From 1d536aeaa8f7efd79d499d17d38cb63c42414a6d Mon Sep 17 00:00:00 2001 From: Natalie Date: Wed, 24 Jun 2026 23:24:46 -0400 Subject: [PATCH] =?UTF-8?q?test(@projects/@magic-civilization):=20?= =?UTF-8?q?=F0=9F=A7=AA=20full-content=20round-trip=20guard=20+=20parity?= =?UTF-8?q?=20fixes=20it=20surfaced?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a regression guard that loads the ENTIRE authored content store through the Rust source-of-truth types — every public/resources/units/*.json into UnitStats AND TacticalUnitSpec, every buildings/*.json through parse_building_catalog — not just the 7-file bench subset. The `game data JSON schemas` step validates against schemas; this validates against the structs the simulator actually runs on, so a file can no longer satisfy a schema yet break the sim. Runs under `cargo test --workspace`, so verify auto-enforces it; a drifting file fails the gate with its filename. The guard immediately caught two parser-parity bugs the bench never exercised: - Building `effects[]` may carry a boolean value (`{"type":"enables_naval", "value":true}`); `BuildingEffect.value: f64` rejected it, dropping the whole building (harbor, stable, deep_*, …) from the catalog. Add a lenient_number deserializer that coerces non-numbers to 0.0 — parity with the GDScript `value is int or is float` guard. (NB: the dylib from 7e2baa25d had the strict parser; rebuilt so the live game re-includes these buildings.) - TacticalUnitSpec.tier had no serde default while the GDScript builder defaults it to 1; a unit JSON omitting tier (founder.json) failed to deserialize. Add #[serde(default = "default_tier")] for path parity. Test excludes the *.schema.json / *_categories.json sidecars that live in the content dirs. Validated: mc-core+mc-ai+mc-player-api 822/0; rebuilt aarch64 dylib; headless GUT 728/0/13. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../mc-ai/src/tactical/building_catalog.rs | 21 ++- .../crates/mc-core/src/tactical_types.rs | 5 +- .../tests/canonical_content_round_trip.rs | 129 ++++++++++++++++++ 3 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 src/simulator/crates/mc-player-api/tests/canonical_content_round_trip.rs 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 11589c66..319c3183 100644 --- a/src/simulator/crates/mc-ai/src/tactical/building_catalog.rs +++ b/src/simulator/crates/mc-ai/src/tactical/building_catalog.rs @@ -27,10 +27,29 @@ use crate::tactical::state::TacticalBuildingSpec; struct BuildingEffect { #[serde(rename = "type", default)] effect_type: String, - #[serde(default)] + /// Authored `value` is usually a number, but some effects carry a boolean + /// flag instead (e.g. `{"type":"enables_naval","value":true}`). The GDScript + /// builder reads `value` ONLY when it is int/float and treats anything else + /// as 0, so a non-numeric value never rejects the whole building. Mirror + /// that here — without it a single boolean-valued effect would drop the + /// entire building from the catalog. + #[serde(default, deserialize_with = "lenient_number")] value: f64, } +/// Deserialize any JSON scalar as `f64`, coercing non-numbers (bool, string, +/// null) to `0.0` — parity with the GDScript `eff["value"] is int or is float` +/// guard. +fn lenient_number<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + use serde::Deserialize; + Ok(serde_json::Value::deserialize(deserializer)? + .as_f64() + .unwrap_or(0.0)) +} + /// Subset of a `buildings/.json` document needed to derive a tactical spec. /// Unknown JSON keys (sprite, adjacency, encyclopedia, …) are ignored. #[derive(Debug, Clone, Deserialize)] diff --git a/src/simulator/crates/mc-core/src/tactical_types.rs b/src/simulator/crates/mc-core/src/tactical_types.rs index 0d29cf5e..bb06eaaf 100644 --- a/src/simulator/crates/mc-core/src/tactical_types.rs +++ b/src/simulator/crates/mc-core/src/tactical_types.rs @@ -163,7 +163,10 @@ pub struct BuildingPriors { pub struct TacticalUnitSpec { /// Unit id (e.g. `"warrior"`, `"pikeman"`). pub id: String, - /// Tier on the 1..N content ladder. + /// Tier on the 1..N content ladder. Missing `tier` defaults to 1, matching + /// the GDScript `build_unit_catalog` builder (`tier_val = 1` unless authored) + /// so a unit file that omits it loads identically on both paths. + #[serde(default = "default_tier")] pub tier: u32, /// Tech gate — unit buildable when the player has researched this id. pub tech_required: Option, diff --git a/src/simulator/crates/mc-player-api/tests/canonical_content_round_trip.rs b/src/simulator/crates/mc-player-api/tests/canonical_content_round_trip.rs new file mode 100644 index 00000000..14dfe35d --- /dev/null +++ b/src/simulator/crates/mc-player-api/tests/canonical_content_round_trip.rs @@ -0,0 +1,129 @@ +//! Regression guard — the ENTIRE authored content store round-trips through the +//! canonical Rust structs / transforms, not just the handful the bench harness +//! loads. +//! +//! Why this exists: the `game data JSON schemas` verify step validates the +//! authored files against JSON *schemas*, but a file can satisfy a schema and +//! still fail to deserialize into the Rust types the simulator actually runs on +//! (a renamed field, a type the schema leaves loose, a `#[serde(default)]` that +//! silently drops a real value). The bench fixtures only exercise +//! `dwarf_warrior` / `dwarf_founder` / `dwarf_berserker` + four buildings, so +//! the other ~180 units / ~200 buildings were never checked against the source +//! of truth. The session that added this guard had three separate drift bugs +//! (`unit_type`, `build_cost`, building `effects`) hide precisely because no +//! test loaded the real data through the real Rust path. +//! +//! These tests assert that EVERY `public/resources/{units,buildings}/*.json` +//! file deserializes into the same `UnitStats` / `TacticalUnitSpec` the engine +//! consumes and parses through the one canonical `parse_building_catalog` +//! transform. A single drifting file fails the whole `cargo test --workspace` +//! gate with the offending filename, so content and code can never silently +//! diverge again. + +use std::fs; +use std::path::{Path, PathBuf}; + +use mc_ai::tactical::parse_building_catalog; +use mc_ai::tactical::state::TacticalUnitSpec; +use mc_units::UnitStats; + +/// `public/resources//` resolved from this crate's manifest dir +/// (`.../src/simulator/crates/mc-player-api`). +fn resources_dir(category: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../../../public/resources") + .join(category) +} + +/// Sorted list of `*.json` files in a content category (deterministic order so +/// failures are reproducible). +fn json_files(category: &str) -> Vec { + let dir = resources_dir(category); + let mut out: Vec = fs::read_dir(&dir) + .unwrap_or_else(|e| panic!("read_dir {} failed: {e}", dir.display())) + .filter_map(|entry| entry.ok().map(|e| e.path())) + .filter(|p| p.extension().is_some_and(|x| x == "json")) + // Skip non-content files that live alongside the data: JSON schemas and + // the `*_categories.json` taxonomy/metadata sidecars. + .filter(|p| { + let n = p.file_name().unwrap().to_string_lossy(); + !n.ends_with(".schema.json") && !n.ends_with("_categories.json") + }) + .collect(); + out.sort(); + out +} + +/// A unit document is either a single object or a legacy array of objects; +/// normalise to the list of unit objects either way. +fn unit_objects(raw: &str, file: &str) -> Vec { + match serde_json::from_str::(raw) { + Ok(serde_json::Value::Array(a)) => a, + Ok(other) => vec![other], + Err(e) => panic!("{file}: not valid JSON: {e}"), + } +} + +fn name(path: &Path) -> String { + path.file_name().unwrap().to_string_lossy().into_owned() +} + +#[test] +fn every_unit_json_deserializes_into_unit_stats_and_tactical_spec() { + let files = json_files("units"); + assert!( + files.len() > 50, + "expected the full unit roster under public/resources/units, got {}", + files.len() + ); + + let mut failures: Vec = Vec::new(); + for path in &files { + let raw = fs::read_to_string(path).unwrap_or_else(|e| panic!("read {}: {e}", name(path))); + for (i, obj) in unit_objects(&raw, &name(path)).into_iter().enumerate() { + let tag = format!("{}[{i}]", name(path)); + // The runtime stat-line the sim spawns from (cost/movement/combat). + if let Err(e) = serde_json::from_value::(obj.clone()) { + failures.push(format!("{tag} -> UnitStats: {e}")); + } + // The tactical-AI buildable spec (tier/gates/archetype). + if let Err(e) = serde_json::from_value::(obj) { + failures.push(format!("{tag} -> TacticalUnitSpec: {e}")); + } + } + } + + assert!( + failures.is_empty(), + "{} unit document(s) drifted from the Rust source-of-truth structs:\n {}", + failures.len(), + failures.join("\n ") + ); +} + +#[test] +fn every_building_json_parses_through_the_canonical_transform() { + let files = json_files("buildings"); + assert!( + files.len() > 50, + "expected the full building roster under public/resources/buildings, got {}", + files.len() + ); + + let mut failures: Vec = Vec::new(); + for path in &files { + let raw = fs::read_to_string(path).unwrap_or_else(|e| panic!("read {}: {e}", name(path))); + match parse_building_catalog(&raw) { + Ok(specs) if !specs.is_empty() => {} + Ok(_) => failures.push(format!("{}: produced 0 specs", name(path))), + Err(e) => failures.push(format!("{}: {e}", name(path))), + } + } + + assert!( + failures.is_empty(), + "{} building document(s) drifted from parse_building_catalog:\n {}", + failures.len(), + failures.join("\n ") + ); +}