Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions crates/eqoxide-core/src/game_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
];
Expand All @@ -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();
Expand All @@ -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))]);
Expand Down
14 changes: 12 additions & 2 deletions crates/eqoxide-nav/src/steering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
48 changes: 13 additions & 35 deletions crates/eqoxide-nav/src/traversability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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,
Expand All @@ -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
Expand Down
20 changes: 20 additions & 0 deletions docs/dev-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 10 additions & 6 deletions src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Loading