diff --git a/crates/eqoxide-core/src/game_state.rs b/crates/eqoxide-core/src/game_state.rs index b2dd6b6f..09e5334c 100644 --- a/crates/eqoxide-core/src/game_state.rs +++ b/crates/eqoxide-core/src/game_state.rs @@ -2044,7 +2044,7 @@ pub(crate) mod tests { let alphabet = [ Ev::Fly(true), Ev::Fly(false), Ev::BuffOn(1), Ev::BuffOff(1), Ev::BuffKnownOther(1), Ev::BuffUnknown(1), - Ev::BuffOn(2), Ev::BuffOff(2), + Ev::BuffOn(2), Ev::BuffOff(2), Ev::BuffUnknown(2), Ev::ZoneIn(true), Ev::ZoneIn(false), Ev::SnapLev, Ev::SnapOther, Ev::SnapUnknown, ]; @@ -2060,7 +2060,7 @@ pub(crate) mod tests { } (ns, nu) }; - // Every sequence of length 3 over the 13-symbol alphabet (2197 orderings). + // Every sequence of length 3 over the 14-symbol alphabet (2744 orderings). for a in alphabet { for b in alphabet { for c in alphabet { let seq = [a, b, c]; let mut st = LevitateState::default(); @@ -2075,9 +2075,18 @@ pub(crate) mod tests { Ev::BuffKnownOther(s) => { st.buff_slot_changed(s, Some(false), false); ref_slots.remove(&s); ref_unresolved.remove(&s); } Ev::BuffOff(s) => { st.buff_slot_changed(s, None, true); ref_slots.remove(&s); ref_unresolved.remove(&s); } // An UNKNOWN spell id never flips the levitate belief, but IS recorded so the - // answer can honestly say Unknown (slot 1, matching the wire event modeled). - Ev::BuffUnknown(_) => { st.buff_slot_changed(1, None, false); - if !ref_slots.contains(&1) { ref_unresolved.insert(1); } } + // answer can honestly say Unknown. + // + // #742: both sides of this arm used to hardcode slot 1 and DISCARD the variant's + // payload, which is what the `dead_code` warning on `BuffUnknown`'s field was + // pointing at. The alphabet therefore could not express "an unresolvable buff in + // a slot OTHER than the one the rest of the sequence acts on": adding + // `BuffUnknown(2)` produced a symbol byte-identical in effect to + // `BuffUnknown(1)`, silently, while reading as extra coverage. The slot now comes + // from the event on BOTH the state under test and the reference model, so the + // symbol means what it says. + Ev::BuffUnknown(s) => { st.buff_slot_changed(s, None, false); + if !ref_slots.contains(&s) { ref_unresolved.insert(s); } } Ev::SnapLev => { st.resync_from_snapshot(&[(1, Some(true))]); let (ns, nu) = ref_snapshot(&ref_slots, &[(1, Some(true))]); ref_slots = ns; ref_unresolved = nu; } Ev::SnapOther => { st.resync_from_snapshot(&[(1, Some(false))]); diff --git a/crates/eqoxide-nav/src/steering.rs b/crates/eqoxide-nav/src/steering.rs index 5f3d8604..5de668ea 100644 --- a/crates/eqoxide-nav/src/steering.rs +++ b/crates/eqoxide-nav/src/steering.rs @@ -1300,9 +1300,19 @@ mod cursor_resync_tests { let span = run.late_x_max - run.late_x_min; // Round 7 (non-blocking 4): the doc above quotes these three. Print them, so the quoted // figures come from THIS run and not from arithmetic on the band's printed 3 decimals. + // + // #742: the WHOLE-run extent is printed alongside. `Run`'s doc says the whole-run and + // settled extents "measure different things, and both are recorded rather than one standing + // in for the other" — but nothing read `x_min`/`x_max`, which is what the `dead_code` warning + // on those two fields was reporting: they were computed and then dropped, so the transient + // the #727 round-5 correction exists to keep separate was invisible in the output. Printing + // them is deliberately NOT an assertion: the transient's width is a diagnostic, and the only + // claim this test makes is about the settled band. eprintln!( - "settled span {span:.4} east of LANDED {:.4} west of LANDED {:.4}", - run.late_x_max - LANDED[0], LANDED[0] - run.late_x_min); + "settled span {span:.4} east of LANDED {:.4} west of LANDED {:.4} \ + whole-run extent [{:.4}, {:.4}] (transient included)", + run.late_x_max - LANDED[0], LANDED[0] - run.late_x_min, + run.x_min, run.x_max); assert!(span < 5.0 * FRAME, "the settled cycle must stay within a few frames of travel — a wider band would be \ drift, not a limit cycle; it was {span:.3} u"); diff --git a/crates/eqoxide-nav/src/traversability.rs b/crates/eqoxide-nav/src/traversability.rs index 5689f525..624879c2 100644 --- a/crates/eqoxide-nav/src/traversability.rs +++ b/crates/eqoxide-nav/src/traversability.rs @@ -673,26 +673,17 @@ mod tests { Collision::build(&ZoneAssets { terrain: meshes, objects: vec![], textures: vec![] }, 32.0) } - /// A corridor (floor + side walls, east-going) with a LINTEL: an overhead barrier spanning the - /// full corridor cross-section from 3.5 u to 6.5 u above the floor. The #386 drift band made - /// exactly this shape a trap: the old planner probes (2.5, 3.0) pass UNDER it, the controller's - /// chest contact ray (4.0) hits it, and its step-up re-probe (4.0 + 2.0) hits it too — clear to - /// A*, solid to the walker. - fn lintel_corridor() -> Collision { - col(vec![ - floor_at(0.0, -40.0, 40.0, -8.0, 8.0), - panel(0.0, -8.0, 8.0, 3.5, 6.5), // the lintel, sealing the corridor at chest height - // side walls so no detour exists — the ONLY way east is under the lintel - mesh(vec![[-8.0, 0.0, -40.0], [-8.0, 10.0, -40.0], [-8.0, 10.0, 40.0], [-8.0, 0.0, 40.0]]), - mesh(vec![[8.0, 0.0, -40.0], [8.0, 10.0, -40.0], [8.0, 10.0, 40.0], [8.0, 0.0, 40.0]]), - ]) - } - - /// The planner's probe heights and the controller's contact heights come from ONE body, and the /// planner's top probe is the controller's chest ray. This is the drift-direction invariant in - /// its cheapest form: if someone re-declares either height locally, the lintel fixture above - /// catches the behaviour; this catches the structure. + /// its cheapest form: if someone re-declares either height locally, the lintel fixture catches + /// the behaviour; this catches the structure. + /// + /// #742: "the lintel fixture" used to mean a `lintel_corridor()` defined immediately above here. + /// #567 moved the tests that consume it to the app crate (they step the real + /// `movement::CharacterController`, which this crate cannot depend on) but left a copy of the + /// fixture behind, unused — that copy is what the `dead_code` warning reported, and it is now + /// deleted. The behaviour pin is alive and unchanged in + /// `tests/walker_sim.rs::planner_never_routes_under_a_lintel_the_walker_collides_with`. #[test] fn planner_probes_are_derived_from_the_controllers_body_on_both_axes() { let planner = PLAYER_BODY.planner_probes(); @@ -703,7 +694,10 @@ mod tests { // the controller's own step-up reach — DERIVED, not a coincident literal. This is the exact // height of the controller's raised step-slide contact ray, so a low obstacle taller than it // blocks BOTH and a shorter one passes BOTH. Re-declaring either as a bare constant re-opens - // the #420 drift; this pins the derivation, and `..._low_wall_...` below pins the behaviour. + // the #420 drift; this pins the derivation. The behaviour is pinned by + // `tests/walker_sim.rs::planner_never_routes_over_a_low_wall_the_walker_cant_step` — NOT by + // anything below in this file (#742: that read "`..._low_wall_...` below", which #567 made + // false when it moved the test out; the fixture it left behind here was dead and is gone). assert_eq!(PLAYER_BODY.step_up, eqoxide_core::physics::STEP_UP, "the body's step-up must be the controller's real step-up capability"); assert_eq!(planner[0], contact[0] + PLAYER_BODY.step_up, @@ -716,22 +710,6 @@ mod tests { assert!(PLAYER_BODY.radius >= eqoxide_core::physics::PLAYER_RADIUS); } - /// A corridor sealed by a thin LOW WALL 3.0 u tall — taller than the controller's step reach - /// (`foot + step_up` = 2.5 u) and with NO walkable top, but shorter than the 4.0 u chest ray. - /// The #420 foot-axis twin of [`lintel_corridor`]: the controller's step-up cannot mount it (its - /// raised contact ray at 2.5 u hits the wall), and the planner's LOW probe (`feet_clr` = 2.5 u) - /// also hits it — so A* refuses the corridor. Drop the low probe (leave only the 4.0 u chest) - /// and the wall passes UNDER it: the planner would then route through a wall the walker cannot - /// step. That is the mutation this fixture pins. - fn low_wall_corridor() -> Collision { - col(vec![ - floor_at(0.0, -40.0, 40.0, -8.0, 8.0), - panel(0.0, -8.0, 8.0, 0.0, 3.0), // 3u wall: above the 2.5u step reach, below the 4.0u chest - mesh(vec![[-8.0, 0.0, -40.0], [-8.0, 10.0, -40.0], [-8.0, 10.0, 40.0], [-8.0, 0.0, 40.0]]), - mesh(vec![[8.0, 0.0, -40.0], [8.0, 10.0, -40.0], [8.0, 10.0, 40.0], [8.0, 0.0, 40.0]]), - ]) - } - /// **THE FIELD IS DETERMINISTIC AND HISTORY-BLIND (the #394 discipline).** A memoised value is /// a pure function of its key: two queries anywhere inside one 2 u key cell get the identical diff --git a/docs/dev-workflow.md b/docs/dev-workflow.md index 221ea5ec..a010acf7 100644 --- a/docs/dev-workflow.md +++ b/docs/dev-workflow.md @@ -171,6 +171,26 @@ Tests that require real zone assets are marked `#[ignore]`: cargo test -- --ignored # run asset-dependent tests (needs ~/eq_assets) ``` +### Capitalised words in test names (`non_snake_case`) — the convention (#742) + +Some test names shout one word to name the axis under test: `..._a_DRY_body_...`, +`..._the_BODY_not_the_feet`, `..._when_a_sync_SUCCEEDS`, `..._when_a_sync_FAILS`. Four such names +exist, across `src/movement.rs` (#730/PR #738) and `src/asset_sync.rs` (#731/PR #755), written by +two different authors — so it is a convention, not a one-off, and #742 asked for it to be written +down rather than rediscovered. + +**The rule: silence `non_snake_case` with a `#[allow(non_snake_case)]` attached to the individual +test function, and never at module or crate scope.** A module- or crate-scope `allow` would also +silence every *accidental* `non_snake_case` in that scope — a typo'd `fn myTest`, a stray +capitalised local — which is precisely the "the warning you meant to keep gets lost with the ones +you meant to suppress" failure mode #723/#730 exist to close. The per-function form suppresses +exactly the one name whose capitalisation is deliberate, and each of the four carries a comment +saying which word is load-bearing and why lower-casing it would erase the signal. Keep that comment +when adding a fifth. + +This is a naming convenience with no runtime meaning: if a name does not have a specific word whose +capitalisation carries information, use plain `snake_case` and no `allow`. + Key test modules: - `src/assets.rs` — collision grid (floor_z, segment_blocked, path_clear) - `src/eq_net/navigation.rs` — the nav walker/planner loop, packet builders (say, target, consider) diff --git a/src/model.rs b/src/model.rs index ff086e74..0a97f8ed 100644 --- a/src/model.rs +++ b/src/model.rs @@ -319,8 +319,13 @@ impl Model for ServerModel { // published snapshot as the sole Model output, so when `WorldState` is split out, the mock's // `publish_snapshot` body is the natural place that constructs it — the scripting surface // (`self_at`/`zone`/`spawn`/`map`) becomes a `WorldState` builder unchanged. +// #742: this re-export used to also name `MockProbe`, which nothing imported — the `unused_imports` +// warning. `MockProbe` is still reachable in practice: every consumer obtains one from +// `MockModel::probe()` and binds it by inference, which is how all four tests below already use it. +// Re-add the name here if a consumer ever needs to write the type out (a helper signature, a struct +// field); it is not re-exported speculatively. #[cfg(test)] -pub(crate) use mock::{MockModel, MockProbe}; +pub(crate) use mock::MockModel; #[cfg(test)] mod mock { @@ -450,11 +455,10 @@ mod mock { self } - /// Override the planning radius (avatar half-width) `find_path_ex` uses. Default 1.0. - pub(crate) fn plan_radius(mut self, r: f32) -> Self { - self.plan_radius = r; - self - } + // #742: a `plan_radius(r)` builder setter lived here and no test ever called it — the + // `dead_code` warning. The FIELD is live (`run` hands it to `find_path_ex`) and keeps its + // documented 1.0 default; only the never-used override is gone. Restoring it is three lines + // if a test needs to vary the avatar half-width. /// Clone out the observation handles BEFORE `run` (which consumes `self`). pub(crate) fn probe(&self) -> MockProbe {