Take data/visuals.ron out of rules_hash (ADR 0003)

CONTRIBUTING.md

@@ -71,7 +71,7 @@ Other recipes in the [justfile](justfile):
 
 Most contributions need no engine knowledge. Every tuning number lives in `data/rules/*.ron` as an integer: time in deciseconds (fields ending in `_ds`) and distance in tiles (fields ending in `_tiles`). The loader converts to ticks once. If you find yourself typing a gameplay constant in Rust, stop and move it to RON. The schema is explained in [docs/DATA_FORMAT.md](docs/DATA_FORMAT.md).
 
-Any edit to a RON file changes `rules_hash`, and every golden replay fixture records the hash it was made with, so without a re-record `cargo test -p sim` fails with `RULES CHANGED since recording`. The data-only path is therefore always the same, and touches no Rust:
+Any edit to a RON file changes `rules_hash` (except the two presentation files: the UI strings under `data/rules/strings/` and, since `rules_version` 25, `data/visuals.ron`, [ADR 0003](docs/adr/0003-visuals-outside-rules-hash.md); `data-check` validates both, but a change to either needs no bump and no re-record), and every golden replay fixture records the hash it was made with, so without a re-record `cargo test -p sim` fails with `RULES CHANGED since recording`. The data-only path is therefore always the same, and touches no Rust:
 
 ```bash
 # 1. Edit a value, for example the Croft gather rate in data/rules/buildings.ron
@@ -117,7 +117,7 @@ $ git status --short
 
 Only `economy_second_town` changes its final hash: it is the only fixture with a worked Croft. A test that hard-codes a number the data controls fails this walkthrough; read the value from `Rules` instead.
 
-Since M4a's rules data (`rules_version` 13) the `OK` line also counts the ages, techs and factions, since M3b it names the checked UI strings, since M5a's rules data (`rules_version` 18) `techs` counts the techs that count toward an age and `side_techs` the Watchtower's Harrying I and II, and since M5b (`rules_version` 21) it counts the bot data under `data/ai/` (`personalities`, `build_orders`), which is in `rules_hash` too, so with the repository at `rules_version` 24 and the bump to 25 of step 2, step 3 now prints a line of the form `OK rules_version=25 rules_hash=0x<16 hex> resources=4 units=8 buildings=8 ages=3 techs=12 side_techs=2 factions=1 personalities=1 build_orders=1 maps=plains_1v1 strings=en` (before the bump it prints `rules_version=24`). An edit under `data/ai/` takes the same path: bump `rules_version`, `just rerecord`, run the tests.
+Since M4a's rules data (`rules_version` 13) the `OK` line also counts the ages, techs and factions, since M3b it names the checked UI strings, since M5a's rules data (`rules_version` 18) `techs` counts the techs that count toward an age and `side_techs` the Watchtower's Harrying I and II, and since M5b (`rules_version` 21) it counts the bot data under `data/ai/` (`personalities`, `build_orders`), which is in `rules_hash` too, so with the repository at `rules_version` 25 and the bump to 26 of step 2, step 3 now prints a line of the form `OK rules_version=26 rules_hash=0x<16 hex> resources=4 units=8 buildings=8 ages=3 techs=12 side_techs=2 factions=1 personalities=1 build_orders=1 maps=plains_1v1 strings=en` (before the bump it prints `rules_version=25`). An edit under `data/ai/` takes the same path: bump `rules_version`, `just rerecord`, run the tests.
 
 ### A build order or personality (`data/ai/`)
 

crates/rules/src/lib.rs

@@ -17,7 +17,7 @@
 //! | `rules/ages.ron` | [`Ages`] |
 //! | `rules/factions.ron` | [`Factions`] |
 //! | `maps/*.ron` | [`map::MapDef`] |
-//! | `visuals.ron` | [`Visuals`] (how each unit kind is drawn; validated here, read by the game) |
+//! | `visuals.ron` | [`Visuals`] (how each unit kind is drawn; validated here, read by the game, not in [`Rules::rules_hash`]: ADR 0003) |
 //! | `ai/personalities.ron`, `ai/build_orders/*.ron` | [`ai::AiRules`] (the bot's knobs and build orders, M5b; validated here, read by `crates/ai`) |
 //!
 //! Every error names the file and, when the data parsed, the field.
@@ -316,13 +316,17 @@ pub struct Rules {
     pub maps: BTreeMap<String, MapDef>,
     /// Contents of `visuals.ron`: a [`Visual`] per unit kind id. Attached by
     /// [`Rules::load`] and validated against `units`; read by the game crate's
-    /// presenter, never by the sim.
-    #[serde(skip_deserializing)]
+    /// presenter, never by the sim. `#[serde(skip)]`, not only
+    /// `skip_deserializing`, so it is the one attached file left out of
+    /// [`Rules::rules_hash`] (ADR 0003, `docs/adr/0003-visuals-outside-rules-hash.md`):
+    /// a look edit needs no `rules_version` bump and no re-record, and a
+    /// bad or missing entry still fails `Rules::load` and `data-check`.
+    #[serde(skip)]
     pub visuals: Visuals,
     /// Everything under `ai/` (M5b): the bot personalities and build
     /// orders. Attached by [`Rules::load`] and validated against the rest
     /// of the rules; read by `crates/ai`, never by the sim, and in
-    /// `rules_hash` like every attached file.
+    /// `rules_hash` like every attached file but `visuals`.
     #[serde(skip_deserializing)]
     pub ai: AiRules,
 }
@@ -1708,9 +1712,11 @@ impl Rules {
     }
 
     /// Content hash of the loaded rules (match rules, resources, units and
-    /// their counter table, buildings, techs, ages, factions, maps and
-    /// visuals); stored in replay headers so a changed RON file is reported
-    /// as `RULES CHANGED` instead of a sim divergence.
+    /// their counter table, buildings, techs, ages, factions, maps and the
+    /// bot data under `ai/`); stored in replay headers so a changed RON file
+    /// is reported as `RULES CHANGED` instead of a sim divergence.
+    /// `visuals.ron` is loaded and validated but not hashed (ADR 0003: the
+    /// sim never reads it), like the UI strings, which are not in `Rules`.
     pub fn rules_hash(&self) -> u64 {
         let bytes = postcard::to_allocvec(self).expect("Rules serialises");
         xxhash_rust::xxh3::xxh3_64(&bytes)
@@ -2046,22 +2052,75 @@ mod tests {
             d.rules_hash(),
             "units.ron is part of the hash"
         );
+        let mut f = Rules::load(data_dir()).unwrap();
+        f.buildings[1].hp += 1;
+        assert_ne!(
+            a.rules_hash(),
+            f.rules_hash(),
+            "buildings.ron is part of the hash"
+        );
+    }
+
+    /// ADR 0003: `visuals.ron` is attached and validated by `load` but left
+    /// out of `rules_hash` (`#[serde(skip)]`), so a look edit needs no
+    /// `rules_version` bump and no re-record. Checked in memory and through
+    /// an edited copy of the file, which must load with the edit in place.
+    #[test]
+    fn rules_hash_ignores_visuals_ron() {
+        let a = Rules::load(data_dir()).unwrap();
         let mut e = Rules::load(data_dir()).unwrap();
         e.visuals.units.insert(
             "yeoman".into(),
             Visual::Primitive(visuals::Primitive::Sphere),
         );
-        assert_ne!(
+        assert_ne!(a.visuals, e.visuals);
+        assert_eq!(
             a.rules_hash(),
             e.rules_hash(),
-            "visuals.ron is part of the hash"
+            "visuals.ron is not part of the hash"
         );
-        let mut f = Rules::load(data_dir()).unwrap();
-        f.buildings[1].hp += 1;
-        assert_ne!(
+        let mut cleared = Rules::load(data_dir()).unwrap();
+        cleared.visuals = Visuals::default();
+        assert_eq!(a.rules_hash(), cleared.rules_hash());
+
+        let edited = loads_after_edits(
+            "visuals-edit",
+            &[(
+                "visuals.ron",
+                r#""skirmisher": Body(shape: Cone, height_tiles_x100: 150)"#,
+                r#""skirmisher": Body(shape: Cone, height_tiles_x100: 160)"#,
+            )],
+        );
+        assert_eq!(
+            edited.visuals.units.get("skirmisher"),
+            Some(&Visual::Body {
+                shape: visuals::Primitive::Cone,
+                height_tiles_x100: 160,
+                lying: false,
+            }),
+            "the edited file is loaded"
+        );
+        assert_eq!(
             a.rules_hash(),
-            f.rules_hash(),
-            "buildings.ron is part of the hash"
+            edited.rules_hash(),
+            "a visuals.ron edit keeps rules_hash"
+        );
+    }
+
+    /// The other half of ADR 0003: leaving the hash does not mean leaving
+    /// validation. A bad entry still fails `load`, naming the file and field.
+    #[test]
+    fn bad_visual_still_fails_to_load_naming_file_and_field() {
+        let msg = load_error_after_edit(
+            "visual-bad-height",
+            "visuals.ron",
+            r#""skirmisher": Body(shape: Cone, height_tiles_x100: 150)"#,
+            r#""skirmisher": Body(shape: Cone, height_tiles_x100: 500)"#,
+        );
+        assert!(msg.contains("visuals.ron"), "{msg}");
+        assert!(
+            msg.contains(r#"units["skirmisher"].height_tiles_x100"#),
+            "{msg}"
         );
     }
 

crates/rules/src/visuals.rs

@@ -2,8 +2,12 @@
 //!
 //! The simulation never reads this file; it is loaded by `Rules::load` so
 //! that it is validated with everything else (every unit kind must have a
-//! visual) and so that it is part of `rules_hash`. The game crate maps a
-//! [`Visual`] to a Bevy mesh and material in `present.rs`.
+//! visual, so `sim-cli data-check` fails on a bad or missing entry). It is
+//! not part of `rules_hash` (ADR 0003,
+//! `docs/adr/0003-visuals-outside-rules-hash.md`; `Rules::visuals` is
+//! `#[serde(skip)]`): changing a look needs no `rules_version` bump and no
+//! re-record, and replays never record which looks were loaded. The game
+//! crate maps a [`Visual`] to a Bevy mesh and material in `present.rs`.
 //!
 //! M2 shipped primitives only; M4b adds [`Visual::Body`], one primitive
 //! with its own height (or length, lying along the unit's facing), so each

crates/sim-cli/tests/data_check.rs

@@ -219,3 +219,77 @@ fn a_cadence_inside_the_command_delay_exits_1_naming_personalities_ron() {
         &["personalities[0].think_every_ds: ", "cmd_delay_ticks"],
     );
 }
+
+/// ADR 0003: `visuals.ron` is out of `rules_hash` but still validated. A
+/// unit kind without a visual fails data-check naming the file and kind.
+#[test]
+fn a_unit_kind_without_a_visual_exits_1_naming_visuals_ron() {
+    rejects_edit(
+        "visual-missing",
+        "visuals.ron",
+        "    \"yeoman\": Primitive(Capsule),\n",
+        "",
+        &[r#"units["yeoman"]: "#, "needs a visual"],
+    );
+}
+
+/// ADR 0003: a look out of range still fails data-check, naming the field.
+#[test]
+fn a_visual_out_of_range_exits_1_naming_visuals_ron() {
+    rejects_edit(
+        "visual-height",
+        "visuals.ron",
+        r#""skirmisher": Body(shape: Cone, height_tiles_x100: 150)"#,
+        r#""skirmisher": Body(shape: Cone, height_tiles_x100: 500)"#,
+        &[r#"units["skirmisher"].height_tiles_x100: "#, "20..=400"],
+    );
+}
+
+/// ADR 0003: a missing `visuals.ron` still fails data-check, naming it.
+#[test]
+fn a_missing_visuals_ron_exits_1_naming_it() {
+    let dir = edited_copy("visuals-gone", "visuals.ron", |text| text);
+    std::fs::remove_file(dir.join("visuals.ron")).unwrap();
+    let out = data_check(&dir);
+    let stderr = text(&out.stderr);
+    assert_eq!(out.status.code(), Some(1), "{}{stderr}", text(&out.stdout));
+    assert!(
+        stderr.starts_with("error: ")
+            && stderr.lines().count() == 1
+            && stderr.contains("visuals.ron"),
+        "{stderr}"
+    );
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
+/// ADR 0003's point: a valid edit of `visuals.ron` passes data-check with
+/// the same `OK` line as the repository data, `rules_hash` included, so it
+/// needs no `rules_version` bump and no re-record.
+#[test]
+fn a_valid_visuals_edit_keeps_the_rules_hash() {
+    let repo = data_check(&repo_data());
+    let from = r#""skirmisher": Body(shape: Cone, height_tiles_x100: 150)"#;
+    let dir = edited_copy("visuals-edit", "visuals.ron", |text| {
+        assert!(
+            text.contains(from),
+            "visuals.ron no longer contains {from:?}"
+        );
+        text.replacen(
+            from,
+            r#""skirmisher": Body(shape: Cone, height_tiles_x100: 160)"#,
+            1,
+        )
+    });
+    let edited = data_check(&dir);
+    let (repo_out, edited_out) = (text(&repo.stdout), text(&edited.stdout));
+    assert_eq!(repo.status.code(), Some(0), "{repo_out}");
+    assert_eq!(
+        edited.status.code(),
+        Some(0),
+        "{edited_out}{}",
+        text(&edited.stderr)
+    );
+    assert!(edited_out.contains(" rules_hash=0x"), "{edited_out}");
+    assert_eq!(repo_out, edited_out);
+    let _ = std::fs::remove_dir_all(&dir);
+}

crates/sim/tests/fixtures/README.md

@@ -29,6 +29,18 @@ The `snapshot_restore` recording snapshots the sim before tick 300, restores
 the bytes into a fresh sim and continues recording on that sim; `verify`
 re-simulates without the swap, so an `OK` proves the restore was exact.
 
+Recorded 2026-10-26 at `rules_version` 25, `SIM_VERSION` 17 with
+`just rerecord` (ADR 0003, `docs/adr/0003-visuals-outside-rules-hash.md`:
+`data/visuals.ron` left `rules_hash`; no value and no sim code changed).
+Each of the eleven files differs from its `rules_version` 24 recording in
+the header's `rules_version` and `rules_hash` only (10 bytes, offsets 8 to
+17; every other byte is equal), and no `.hash` file changed. A per-tick
+`sim-cli hash-dump --every 1` of `move_500`, `combat_skirmish`,
+`capture_seat` and `m5_first_match` as recorded at 24, run by the
+pre-bump binary with its own `data/`, equals that of the re-recorded
+fixture run by the new binary on every tick. From here an edit of
+`data/visuals.ron` alone needs no bump and no re-record.
+
 Recorded 2026-10-23 at `rules_version` 24, `SIM_VERSION` 17 with
 `just rerecord` (the M5b waves slice's review fixes: `data/ai/` gained the
 `defend_guard_pop_ratio` knob, and the Standard bot's defence, build-order

data/README.md

@@ -4,7 +4,9 @@ This directory holds every tuning number, map and (from M5b) AI script that the
 Eonmark simulation reads at start-up. Nothing in here is compiled into the
 binary; `rules::Rules::load("data")` parses and validates it, and the
 resulting `rules_hash` is written into every replay header so a changed file
-is reported as `RULES CHANGED` rather than as a simulation bug. The code
+is reported as `RULES CHANGED` rather than as a simulation bug. The two
+presentation files, `visuals.ron` and `rules/strings/`, are validated too
+but are not in `rules_hash`, since the simulation never reads them. The code
 reads these files; it never defines gameplay constants of its own.
 
 ## Layout
@@ -20,7 +22,7 @@ reads these files; it never defines gameplay constants of its own.
 | `rules/factions.ron` | The factions and their Modifier rows (Freeholders, none) | `crates/rules/src/factions.rs` (`Factions`) | M4a |
 | `maps/plains_1v1.ron` | The one 128x128 mirrored two-player map | `crates/rules/src/map.rs` | M0 |
 | `rules/strings/en.ron` | UI strings: the message line's text for every command rejection, HUD labels and tooltips. Presentation only, outside `rules_hash` (see `crates/rules/src/strings.rs`) | written | M3b |
-| `visuals.ron` | How each unit kind is drawn: `Primitive(Capsule \| Cuboid \| Cylinder \| Sphere \| Cone)` and, from M4b, `Body(shape, height_tiles_x100, lying)` now, `Scene(path, scale_x100, y_offset_tiles_x100, yaw_deg)` from M7; keyed by unit kind id, every kind in `units.ron` must appear | `crates/rules/src/visuals.rs` (`Visuals`) | M2, M4b |
+| `visuals.ron` | How each unit kind is drawn: `Primitive(Capsule \| Cuboid \| Cylinder \| Sphere \| Cone)` and, from M4b, `Body(shape, height_tiles_x100, lying)` now, `Scene(path, scale_x100, y_offset_tiles_x100, yaw_deg)` from M7; keyed by unit kind id, every kind in `units.ron` must appear. Presentation only: validated by `Rules::load`, outside `rules_hash` since `rules_version` 25 ([ADR 0003](../docs/adr/0003-visuals-outside-rules-hash.md)) | `crates/rules/src/visuals.rs` (`Visuals`) | M2, M4b |
 | `ai/personalities.ron` | One row per bot personality: its build order and every knob of the bot (cadence, caps, site policies, army mix, waves, defence and recall); M5b ships `standard`. Loaded last by `Rules::load`, attached as `Rules::ai` and in `rules_hash`, so an edit needs a `rules_version` bump | `crates/rules/src/ai.rs` (`AiRules`) | M5b |
 | `ai/build_orders/*.ron` | One build order per file, its `id` equal to the file stem; M5b ships `standard.ron`. In `rules_hash` like the personalities | `crates/rules/src/ai.rs` (`AiRules`) | M5b |
 | `ai/difficulty.ron` | Easy, Standard, Hard: income interval and aggression | not yet written | M6 |
@@ -33,8 +35,11 @@ reference for contributors is [../docs/DATA_FORMAT.md](../docs/DATA_FORMAT.md).
 ## visuals.ron
 
 Read by the game's presenter, never by the simulation, but loaded and
-validated by `Rules::load` so a missing visual is caught before a window
-opens and so the file is part of `rules_hash`. Schema `crates/rules/src/visuals.rs`.
+validated by `Rules::load`, so a missing or bad visual fails `sim-cli
+data-check` and is caught before a window opens. Since `rules_version` 25
+the file is not part of `rules_hash`
+([ADR 0003](../docs/adr/0003-visuals-outside-rules-hash.md); before that it
+was, from M2). Schema `crates/rules/src/visuals.rs`.
 
 | Field | Meaning |
 |-------|---------|
@@ -43,10 +48,12 @@ opens and so the file is part of `rules_hash`. Schema `crates/rules/src/visuals.
 | `Body(shape, height_tiles_x100, lying)` | The same primitive two radii wide with its own height in hundredths of a tile (M4b): `height_tiles_x100` in 20..=400, a `Capsule` at least two radii. `lying: true` (default `false`, `Capsule` or `Cylinder` only) lays it along the unit's facing, `height_tiles_x100` long and two radii tall. A `Body` is never a `Sphere`. Errors name `units["<id>"].shape`, `.lying` or `.height_tiles_x100`. |
 | `Scene(path, scale_x100, y_offset_tiles_x100, yaw_deg)` | glTF under `assets/` (`.glb` or `.gltf`), uniform scale in hundredths (1..=10000), vertical offset in hundredths of a tile, extra yaw in whole degrees (-360..=360). Parsed and validated now, used from M7. |
 
-The game draws every look low-poly and flat-shaded. A changed look changes
-`rules_hash`, so it needs a `rules_version` bump and `just rerecord` like
-any other data edit; the sim never reads it, so every `.hash` file stays the
-same and only the replay headers change.
+The game draws every look low-poly and flat-shaded. A changed look leaves
+`rules_hash` alone, so it needs no `rules_version` bump and no `just
+rerecord`: every replay still verifies (`OK`, not `RULES CHANGED`), and
+`sim-cli data-check` still rejects an entry that breaks the rules above.
+Anything the simulation or a bot reads does not belong in this file; it
+goes in a hashed file under `rules/` or `ai/`.
 
 ```ron
 (units: {
@@ -82,10 +89,13 @@ exits 1 and prints the file path, the span and the field, for example:
 error: data/rules/rules.ron: 8:3-8:18: Unexpected field named `tick_rate_hertz` in `Rules`, expected one of `rules_version`, `tick_rate_hz`, ... instead
 ```
 
-Every edit here changes `rules_hash`, so a change you keep also bumps
+Every edit here except `visuals.ron` and `rules/strings/` changes
+`rules_hash`, so a change you keep to any other file also bumps
 `rules_version` in `rules/rules.ron` and re-records the golden fixtures
 with `just rerecord` before `cargo test -p sim`; the full data-only path is
-the worked example in [rules/README.md](rules/README.md).
+the worked example in [rules/README.md](rules/README.md). A look or UI
+string edit needs only `data-check`: neither file is hashed, so it takes no
+bump and no re-record.
 
 ## License