refactor(@projects/@magic-civilization): ♻️ retire GDScript building aggregation, delegate to Rust transform (Rail 1)
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<TacticalBuildingSpec> 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) <noreply@anthropic.com>
This commit is contained in:
Natalie
parent
8a5fb9e8f3
commit
7e2baa25d4
4 changed files with 126 additions and 86 deletions
|
|
@ -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<TacticalBuildingSpec>`-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<TacticalBuildingSpec>`-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:
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
@ -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<TacticalBuildingSpec>` 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<TacticalBuildingSpec>` 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
|
||||
|
|
|
|||
|
|
@ -124,11 +124,19 @@ impl From<BuildingDoc> for TacticalBuildingSpec {
|
|||
pub fn parse_building_catalog(json: &str) -> Result<Vec<TacticalBuildingSpec>, 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<BuildingDoc> = match value {
|
||||
serde_json::Value::Array(_) => serde_json::from_value(value)?,
|
||||
_ => vec![serde_json::from_value(value)?],
|
||||
let elements: Vec<serde_json::Value> = 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::<BuildingDoc>(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": " " }]"#;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue