From 9ed6094d69d6bcf74f237b246c5c438f4d86477d Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Thu, 5 Mar 2026 10:59:38 +0100 Subject: [PATCH] =?UTF-8?q?fix(simulation):=20address=20PR=20#85=20review?= =?UTF-8?q?=20=E2=80=94=20warnings=20and=20polish=20items?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Ticker rotation: document sliding-window semantics (vs modulus-aligned) - Ticker zone ID: add warning about Gauntlet vs production zone ID mismatch - Proof-room movement profile: respect archetype instead of hardcoding smuggler - Storyteller tie-break: use exact f32 equality (inputs are discrete integers) - Observer: .map().flatten() → .and_then() (clippy strict) - Content loader: remove dangling doc comment before section header - Tests: replace assert!(false, ...) with TODO comments in ignored tests - Tests: add frame limiter note on 302-update loop in tell expiry test Co-Authored-By: Claude Opus 4.6 --- server/src/content/loader.rs | 1 - server/src/main.rs | 5 ++++- server/src/perception/observer/mod.rs | 3 +-- server/src/simulation/ticker.rs | 9 +++++++++ server/src/storyteller/mod.rs | 4 +++- server/tests/news_ticker.rs | 22 +++++++--------------- server/tests/tell_escalation.rs | 4 +++- 7 files changed, 27 insertions(+), 21 deletions(-) diff --git a/server/src/content/loader.rs b/server/src/content/loader.rs index bd111bfbf..b941166a7 100644 --- a/server/src/content/loader.rs +++ b/server/src/content/loader.rs @@ -344,7 +344,6 @@ fn walk_yaml_files(dir: &Path, callback: &mut impl FnMut(&Path)) { } } -/// Check if a YAML file contains only comments and whitespace (stub file). // --------------------------------------------------------------------------- // Tile loading (#577) // --------------------------------------------------------------------------- diff --git a/server/src/main.rs b/server/src/main.rs index 0f17d4cfb..4bcf3897f 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -399,7 +399,10 @@ fn setup_proof_room(app: &mut App, archetype: settled_reach_server::bridge::type let mut registry = EntityRegistry::new(0); // Player at (16,16) — archetype from StartupMessage (#587, D-053) - let profile = MovementProfile::smuggler(); + let profile = match archetype { + settled_reach_server::bridge::types::CharacterArchetype::Smuggler => MovementProfile::smuggler(), + settled_reach_server::bridge::types::CharacterArchetype::Detective => MovementProfile::detective(), + }; let mut monologue_state = MonologueState::default(); monologue_state.character = archetype.as_monologue_key().to_string(); let player = app diff --git a/server/src/perception/observer/mod.rs b/server/src/perception/observer/mod.rs index 30e33574f..ee621042f 100644 --- a/server/src/perception/observer/mod.rs +++ b/server/src/perception/observer/mod.rs @@ -442,8 +442,7 @@ pub fn compute_observer_snapshot( // News ticker (#591): populate when player is in The Last Shift zone. let player_zone = zone_map .as_deref() - .map(|zm| zm.zone_at(observer_pos.x, observer_pos.y, observer_pos.z)) - .flatten(); + .and_then(|zm| zm.zone_at(observer_pos.x, observer_pos.y, observer_pos.z)); let current_ticker = if player_zone == Some(LAST_SHIFT_ZONE_ID) { ticker_pool .as_deref() diff --git a/server/src/simulation/ticker.rs b/server/src/simulation/ticker.rs index ca3a4ed60..7cd453abc 100644 --- a/server/src/simulation/ticker.rs +++ b/server/src/simulation/ticker.rs @@ -16,6 +16,10 @@ use crate::bridge::types::TickerLine; /// /// Must match the zone_id used in the production location YAML /// when the transit district ZoneMap is populated. +/// +/// WARNING: Gauntlet assigns zone IDs sequentially and may differ from +/// production. This constant is for production use only; Gauntlet tests +/// should query the ContentStore for the correct zone ID. pub const LAST_SHIFT_ZONE_ID: u16 = 1; /// How often the displayed headline rotates, in ticks. @@ -56,6 +60,11 @@ impl TickerPool { /// Advance to a new headline using SimRng if `TICKER_ROTATION_TICKS` have elapsed. /// + /// Uses sliding-window timing: the next rotation fires `TICKER_ROTATION_TICKS` + /// after the last rotation, not on a fixed modulus boundary. This means the first + /// headline persists for 200 ticks from tick 0, then each subsequent headline + /// persists for 200 ticks from the moment it was selected. + /// /// Called each tick by `tick_news_ticker`. Deterministic: same seed → same rotation. pub fn maybe_rotate(&mut self, current_tick: u64, rng: &mut crate::simulation::rng::SimRng) { if self.lines.is_empty() { diff --git a/server/src/storyteller/mod.rs b/server/src/storyteller/mod.rs index cf0270449..486caea0b 100644 --- a/server/src/storyteller/mod.rs +++ b/server/src/storyteller/mod.rs @@ -564,9 +564,11 @@ pub fn activation_pass( // Step 5: select best candidate; SimRng tie-break among equal top scores (D-010) candidates.sort_by(|a, b| b.2.partial_cmp(&a.2).unwrap_or(std::cmp::Ordering::Equal)); let top_score = candidates[0].2; + // Exact equality: all inputs are discrete integers cast to f32, so scores + // derived from integer tick counts will compare exactly. No epsilon needed. let top_count = candidates .iter() - .take_while(|(_, _, s)| (*s - top_score).abs() <= f32::EPSILON * top_score.abs().max(1.0)) + .take_while(|(_, _, s)| *s == top_score) .count(); let selected_idx = if top_count > 1 { rng.rng.random_range(0..top_count) diff --git a/server/tests/news_ticker.rs b/server/tests/news_ticker.rs index 45c997d17..4a90915c3 100644 --- a/server/tests/news_ticker.rs +++ b/server/tests/news_ticker.rs @@ -202,49 +202,41 @@ fn ticker_yaml_category_distribution_is_sane() { #[test] #[ignore = "pending TickerPool implementation (#591)"] fn ticker_pool_loads_all_30_headlines() { - // TickerPool::load() should parse the YAML and hold all 30 headlines. + // TODO: TickerPool::load() should parse the YAML and hold all 30 headlines. // Verifies content loader wiring (ticker/ subdirectory is scanned). - assert!(false, "Implement: TickerPool::load() returns pool with len() == 30"); } #[test] #[ignore = "pending TickerPool implementation (#591)"] fn ticker_rotates_at_200_tick_boundary() { - // After TICKER_ROTATION_TICKS (200) ticks, the active headline changes. + // TODO: run 200 ticks, assert current_headline changes. // Must use SimRng — running with same seed must produce same sequence. - assert!(false, "Implement: run 200 ticks, assert current_headline changes"); } #[test] #[ignore = "pending TickerPool implementation (#591)"] fn ticker_rotation_is_deterministic_under_same_seed() { + // TODO: two apps, same seed, assert same ticker at tick 200 and 400. // D-010 principle 4: deterministic simulation. - // Two sessions with the same seed must show the same ticker sequence. - assert!(false, "Implement: two apps, same seed, assert same ticker at tick 200 and 400"); } #[test] #[ignore = "pending current_ticker in ObserverSnapshot (#591)"] fn current_ticker_is_none_when_player_is_not_in_bar_zone() { - // When player is outside "bar" zone, current_ticker must be None. + // TODO: player in terminal zone → snapshot.current_ticker is None. // Edge case: don't leak bar headlines into The Terminal or corridor zones. - assert!(false, "Implement: player in terminal zone → snapshot.current_ticker is None"); } #[test] #[ignore = "pending current_ticker in ObserverSnapshot (#591)"] fn current_ticker_is_some_when_player_is_in_bar_zone() { - // When player is in "bar" zone, current_ticker must be Some. - // Spec: zone_id == "bar" (from zone.rs LAST_SHIFT_BAR_ZONE or equivalent). - assert!(false, "Implement: player in bar zone → snapshot.current_ticker is Some"); + // TODO: player in bar zone → snapshot.current_ticker is Some. + // Spec: zone_id from zone.rs LAST_SHIFT_BAR_ZONE or equivalent. } #[test] #[ignore = "pending current_ticker in ObserverSnapshot (#591)"] fn ticker_line_dual_lens_field_not_in_wire_format() { + // TODO: serialize TickerLine, assert no dual_lens key in msgpack output. // D-036 says dual_lens is authoring metadata ONLY — must not cross the wire. - // TickerLine wire struct must NOT have a dual_lens field. - // This is a security/info-boundary concern: the dual_lens notes contain - // game design commentary that should not be visible to players via the API. - assert!(false, "Implement: serialize TickerLine, assert no dual_lens key in msgpack output"); } diff --git a/server/tests/tell_escalation.rs b/server/tests/tell_escalation.rs index 7fd8f2169..f338cefe9 100644 --- a/server/tests/tell_escalation.rs +++ b/server/tests/tell_escalation.rs @@ -217,7 +217,9 @@ fn routine_deviation_expires_after_duration() { }); } - // Run enough ticks to trigger expiry (301 > TELL_ESCALATION_DURATION_TICKS = 300) + // Run enough ticks to trigger expiry (301 > TELL_ESCALATION_DURATION_TICKS = 300). + // Note: 302 updates is acceptable for a unit test; if this becomes a perf concern + // when un-ignored, consider advancing SimulationTime directly instead of looping. for _ in 0..302 { app.update(); }