fix(simulation): address PR #85 review — warnings and polish items
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
+4
-1
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user