From 7e2baa25d4c2a6e2676e61365b8779dcaaa0f4ce Mon Sep 17 00:00:00 2001 From: Natalie Date: Wed, 24 Jun 2026 23:12:52 -0400 Subject: [PATCH] =?UTF-8?q?refactor(@projects/@magic-civilization):=20?= =?UTF-8?q?=E2=99=BB=EF=B8=8F=20retire=20GDScript=20building=20aggregation?= =?UTF-8?q?,=20delegate=20to=20Rust=20transform=20(Rail=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The effects→yield aggregation existed in two places: GDScript (ai_turn_bridge_state.gd::build_building_catalog) and Rust (mc_ai::tactical::parse_building_catalog). Both were byte-equivalent but a duplicated transform that could drift. Per Rail 1 (simulation logic in Rust), the GDScript copy is now retired. - api-gdext: GdItemSystem gains `aggregate_building_catalog_json(raw)` — a thin #[func] over parse_building_catalog that takes the raw authored building docs and returns the aggregated Vec JSON (reuses the existing lightweight stateless bridge class — no new registered class, so no plum class-cache churn). - ai_turn_bridge_state.gd: build_building_catalog now marshals DataLoader.get_data("buildings").values() to that method instead of summing the effects[] array in GDScript. The ~80-line aggregation loop is deleted. - parse_building_catalog: made resilient (skips malformed entries instead of failing the whole catalog) to match the GDScript builder's has("id") filter. Validation: cargo mc-ai building_catalog 4/4; rebuilt the aarch64 dylib; full headless GUT 728 passing / 0 failing / 13 pending, including 2 new tests that exercise the GDScript→Rust→GDScript round-trip (forge production→yield_production, research→science, trade→gold) and the malformed-input empty-catalog path. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/modules/ai/ai_turn_bridge_state.gd | 100 ++++-------------- .../test_building_catalog_rust_delegation.gd | 57 ++++++++++ src/simulator/api-gdext/src/lib.rs | 26 +++++ .../mc-ai/src/tactical/building_catalog.rs | 29 ++++- 4 files changed, 126 insertions(+), 86 deletions(-) create mode 100644 src/game/engine/tests/unit/ai/test_building_catalog_rust_delegation.gd diff --git a/src/game/engine/src/modules/ai/ai_turn_bridge_state.gd b/src/game/engine/src/modules/ai/ai_turn_bridge_state.gd index c7ee4358..8180d200 100644 --- a/src/game/engine/src/modules/ai/ai_turn_bridge_state.gd +++ b/src/game/engine/src/modules/ai/ai_turn_bridge_state.gd @@ -219,91 +219,27 @@ static func build_unit_catalog() -> Array: ## p1-42: build the catalog of producible buildings consumed by -## `mc_ai::tactical::production::pick_building_from_catalog`. Mirrors the -## `build_unit_catalog` pattern — flattens DataLoader's `buildings` dict -## into a `Vec`-shaped Array. Empty entries / index -## files are skipped. Effect aggregation walks the `effects[]` array, -## summing per-yield-type values into the spec's typed fields. +## `mc_ai::tactical::production::pick_building_from_catalog`. The authored +## `effects[]` → typed-yield aggregation is NOT performed here any more: the raw +## building docs are handed to the canonical Rust transform +## (`mc_ai::tactical::parse_building_catalog`, exposed via +## `GdItemSystem.aggregate_building_catalog_json`) so the effect→yield mapping +## lives in exactly ONE place (Rail 1: simulation logic in Rust, no GDScript +## copy to drift). Returns a `Vec`-shaped Array. static func build_building_catalog() -> Array: - var out: Array = [] var data: Dictionary = DataLoader.get_data("buildings") if data == null: - return out - for bid: String in data.keys(): - if not (data[bid] is Dictionary): - continue - var entry: Dictionary = data[bid] - if entry.is_empty(): - continue - if not entry.has("id"): - continue - var id_val: String = dict_string_field(entry, "id") - if id_val.is_empty(): - id_val = bid - var tier_val: int = 1 - if entry.has("tier") and (entry["tier"] is int or entry["tier"] is float): - tier_val = int(entry["tier"]) - var cost_val: int = 0 - if entry.has("cost") and (entry["cost"] is int or entry["cost"] is float): - cost_val = int(entry["cost"]) - var category: String = dict_string_field(entry, "category") - var tech_raw: String = dict_string_field(entry, "tech_required") - var race_raw: String = dict_string_field(entry, "race_required") - var wonder_raw: String = dict_string_field(entry, "wonder_type") - var resource_raw: String = dict_string_field(entry, "requires_resource") - var requires_existing_raw: String = dict_string_field(entry, "requires_existing") - var item: Dictionary = { - "id": id_val, - "tier": tier_val, - "category": category, - "cost": cost_val, - "tech_required": (tech_raw if not tech_raw.is_empty() else null), - "race_required": (race_raw if not race_raw.is_empty() else null), - "wonder_type": (wonder_raw if not wonder_raw.is_empty() else null), - "requires_resource": (resource_raw if not resource_raw.is_empty() else null), - "requires_existing": (requires_existing_raw if not requires_existing_raw.is_empty() else null), - "yield_food": 0, - "yield_production": 0, - "yield_gold": 0, - "yield_science": 0, - "yield_culture": 0, - "yield_defense": 0, - "yield_gpp": 0, - "great_work_slots": 0, - "yield_happiness": 0, - } - # Aggregate the authored `effects` array into the typed yield fields. - if entry.has("effects") and entry["effects"] is Array: - for eff: Variant in entry["effects"]: - if not (eff is Dictionary): - continue - var etype: String = dict_string_field(eff, "type") - var evalue: int = 0 - if eff.has("value") and (eff["value"] is int or eff["value"] is float): - evalue = int(eff["value"]) - match etype: - "food": - item["yield_food"] += evalue - "production": - item["yield_production"] += evalue - "gold", "trade": - item["yield_gold"] += evalue - "science", "research": - item["yield_science"] += evalue - "culture": - item["yield_culture"] += evalue - "defense", "city_hp", "wall_hp": - item["yield_defense"] += evalue - "happiness": - item["yield_happiness"] += evalue - _: - # GPP / great_work_slots — coarse aggregation across all channels. - if etype.begins_with("gpp_"): - item["yield_gpp"] += evalue - elif etype.begins_with("great_work_slots_"): - item["great_work_slots"] += evalue - out.append(item) - return out + return [] + # `data.values()` is the array of authored building docs — exactly the shape + # `parse_building_catalog` accepts (it skips any malformed entry). + var raw_json: String = JSON.stringify(data.values()) + var gd: RefCounted = ClassDB.instantiate("GdItemSystem") as RefCounted + if gd == null: + push_error("build_building_catalog: GdItemSystem unavailable") + return [] + var aggregated_json: String = gd.call("aggregate_building_catalog_json", raw_json) + var parsed: Array = JSON.parse_string(aggregated_json) as Array + return parsed static func dict_string_field(entry: Dictionary, key: String) -> String: diff --git a/src/game/engine/tests/unit/ai/test_building_catalog_rust_delegation.gd b/src/game/engine/tests/unit/ai/test_building_catalog_rust_delegation.gd new file mode 100644 index 00000000..2e467076 --- /dev/null +++ b/src/game/engine/tests/unit/ai/test_building_catalog_rust_delegation.gd @@ -0,0 +1,57 @@ +extends GutTest +## Verifies the GDScript→Rust delegation for building-catalog aggregation. +## +## `AiTurnBridgeState.build_building_catalog()` no longer sums the authored +## `effects[]` array in GDScript — it marshals the raw building docs to +## `GdItemSystem.aggregate_building_catalog_json`, which runs the canonical +## `mc_ai::tactical::parse_building_catalog` transform (Rail 1: one +## implementation, in Rust). This test exercises that round-trip directly with a +## fixed input (no DataLoader dependency), proving the effect→yield mapping +## crosses the GDExtension boundary intact. + + +func test_aggregate_building_catalog_json_round_trip() -> void: + var gd: RefCounted = ClassDB.instantiate("GdItemSystem") as RefCounted + assert_not_null(gd, "GdItemSystem must instantiate (GDExtension dylib loaded)") + if gd == null: + return + var raw_json: String = ( + '[{"id":"forge","category":"production","cost":60,' + + '"effects":[{"type":"production","value":2}]},' + + '{"id":"grand_archive","category":"research","cost":120,' + + '"effects":[{"type":"research","value":3},{"type":"trade","value":1}]}]' + ) + var out_json: String = gd.call("aggregate_building_catalog_json", raw_json) + var parsed: Array = JSON.parse_string(out_json) as Array + assert_eq(parsed.size(), 2, "both building specs returned by the Rust transform") + if parsed.size() != 2: + return + + var forge: Dictionary = parsed[0] + assert_eq(String(forge.get("id", "")), "forge", "id preserved across the boundary") + assert_eq(int(forge.get("cost", -1)), 60, "cost preserved") + assert_eq( + int(forge.get("yield_production", -1)), + 2, + "forge production effect aggregated to yield_production=2 by Rust" + ) + + var archive: Dictionary = parsed[1] + assert_eq( + int(archive.get("yield_science", -1)), + 3, + "research effect aliases to yield_science" + ) + assert_eq(int(archive.get("yield_gold", -1)), 1, "trade effect aliases to yield_gold") + + +func test_malformed_raw_json_yields_empty_array() -> void: + var gd: RefCounted = ClassDB.instantiate("GdItemSystem") as RefCounted + if gd == null: + return + var out_json: String = gd.call("aggregate_building_catalog_json", "not json at all") + # The Rust method logs a godot_error! on the parse failure; mark it expected + # so GUT's unexpected-error tracker treats this defensive path as handled. + assert_engine_error("aggregate_building_catalog_json parse failed") + var parsed: Array = JSON.parse_string(out_json) as Array + assert_eq(parsed.size(), 0, "invalid input returns an empty catalog, never crashes") diff --git a/src/simulator/api-gdext/src/lib.rs b/src/simulator/api-gdext/src/lib.rs index 82a6eb2c..a80c82cf 100644 --- a/src/simulator/api-gdext/src/lib.rs +++ b/src/simulator/api-gdext/src/lib.rs @@ -2855,6 +2855,32 @@ impl GdItemSystem { mc_items::ItemSystem::rush_buy_cost(base_production_cost as i32) as i64 } + /// Aggregate a raw building-catalog JSON into the tactical + /// `Vec` wire form, via the canonical + /// [`mc_ai::tactical::parse_building_catalog`] transform. + /// + /// `raw_json` is the array of authored `buildings/*.json` documents — what + /// GDScript gets from `DataLoader.get_data("buildings").values()`. This is + /// the single home of the `effects[] → yield_*` aggregation (Rail 1): the AI + /// bridge calls this instead of re-implementing the sum in GDScript. Returns + /// the serialised `Vec` JSON, or `"[]"` on error. + #[func] + fn aggregate_building_catalog_json(&self, raw_json: GString) -> GString { + match mc_ai::tactical::parse_building_catalog(&raw_json.to_string()) { + Ok(specs) => match serde_json::to_string(&specs) { + Ok(s) => GString::from(s), + Err(e) => { + godot_error!("aggregate_building_catalog_json serialise failed: {e}"); + GString::from("[]") + } + }, + Err(e) => { + godot_error!("aggregate_building_catalog_json parse failed: {e}"); + GString::from("[]") + } + } + } + #[func] fn decay_turns_from_balance_json(raw: GString) -> i64 { mc_items::ItemSystem::decay_turns_from_balance_json(&raw.to_string()) as i64 diff --git a/src/simulator/crates/mc-ai/src/tactical/building_catalog.rs b/src/simulator/crates/mc-ai/src/tactical/building_catalog.rs index 36637656..11589c66 100644 --- a/src/simulator/crates/mc-ai/src/tactical/building_catalog.rs +++ b/src/simulator/crates/mc-ai/src/tactical/building_catalog.rs @@ -124,11 +124,19 @@ impl From for TacticalBuildingSpec { pub fn parse_building_catalog(json: &str) -> Result, serde_json::Error> { // Tolerate both shapes: a bare object and an array of objects. let value: serde_json::Value = serde_json::from_str(json)?; - let docs: Vec = match value { - serde_json::Value::Array(_) => serde_json::from_value(value)?, - _ => vec![serde_json::from_value(value)?], + let elements: Vec = match value { + serde_json::Value::Array(a) => a, + other => vec![other], }; - Ok(docs.into_iter().map(TacticalBuildingSpec::from).collect()) + // Skip elements that aren't valid building documents (no `id`, not an + // object, …) rather than failing the whole catalog — mirrors the GDScript + // builder's `if not entry.has("id"): continue` defensive filtering, so a + // single malformed authored file never blanks the AI's roster. + Ok(elements + .into_iter() + .filter_map(|v| serde_json::from_value::(v).ok()) + .map(TacticalBuildingSpec::from) + .collect()) } #[cfg(test)] @@ -183,6 +191,19 @@ mod tests { assert_eq!(b.yield_food + b.yield_production + b.yield_culture, 0); } + #[test] + fn skips_malformed_entries_without_failing() { + // One valid building + one object missing the required `id` → only the + // valid one survives (mirrors the GDScript `has("id")` skip). + let json = r#"[ + { "id": "forge", "effects": [{ "type": "production", "value": 2 }] }, + { "name": "no id here", "category": "junk" } + ]"#; + let specs = parse_building_catalog(json).expect("array parses"); + assert_eq!(specs.len(), 1, "malformed entry skipped, valid kept"); + assert_eq!(specs[0].id, "forge"); + } + #[test] fn empty_gate_strings_become_none() { let json = r#"[{ "id": "hut", "tech_required": "", "race_required": " " }]"#;