From 6bda4a76209d3c7e9deae736230ebd542b20de76 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 11 Feb 2026 17:53:01 +0100 Subject: [PATCH] fix(simulation): address PR #2 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix Cargo.toml edition 2024 → 2021 (Hoshe #1) - Switch test runner to cargo-nextest in Makefile (Hoshe #2, D-030) - Add debug_assert tick ordering enforcement in InputQueue::push (Hoshe #4) - Add DayPhase Serialize/Deserialize derives (Tyre #6) - Add day phase boundary comments clarifying half-open ranges (Hoshe #3) - Add phase duration adjustability comment (Tyre #5) - Document ObserverSnapshot planned fields as TODO (Tyre #1) - Document entity_id as wire-format ID, not ECS Entity (Tyre #2) - Change Relationship.target_name to target_id: u64 (Tyre #3) - Clarify single app.update() is intentional boilerplate (Hoshe #8, Tyre #7) Co-Authored-By: Claude Opus 4.6 --- Makefile | 2 +- server/Cargo.toml | 2 +- server/src/bridge/types.rs | 7 ++++++- server/src/main.rs | 2 +- server/src/npc/mod.rs | 3 ++- server/src/simulation/input.rs | 24 +++++++++++++++++++++++- server/src/simulation/time.rs | 26 +++++++++++++++++++++++++- 7 files changed, 59 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index dceb5bfab..4a2878f20 100644 --- a/Makefile +++ b/Makefile @@ -69,7 +69,7 @@ client: test: test-server test-client test-server: - cd server && cargo test + cd server && cargo nextest run test-client: @echo "Client tests run via gdUnit4 inside Godot." diff --git a/server/Cargo.toml b/server/Cargo.toml index 9b21c6d68..6afe63cbd 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "settled-reach-server" version = "0.1.0" -edition = "2024" +edition = "2021" [dependencies] bevy_ecs = "0.18" diff --git a/server/src/bridge/types.rs b/server/src/bridge/types.rs index c34466c8f..f7d3811c6 100644 --- a/server/src/bridge/types.rs +++ b/server/src/bridge/types.rs @@ -5,7 +5,10 @@ use serde::{Deserialize, Serialize}; /// The ONLY data structure crossing the client-server boundary (D-020) -/// Contains all information visible to the observer at a given tick +/// Contains all information visible to the observer at a given tick. +/// +/// TODO: Planned fields — fog/visibility data, ambient sound events, +/// internal monologue triggers, HUD state (D-020 expansion). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ObserverSnapshot { /// Simulation tick when this snapshot was produced @@ -17,6 +20,8 @@ pub struct ObserverSnapshot { /// A visible entity in the simulation #[derive(Debug, Clone, Serialize, Deserialize)] pub struct VisibleEntity { + /// Wire-format entity identifier — NOT a bevy ECS Entity. + /// Stable across serialization for client-server boundary (D-020). pub entity_id: u64, pub x: f32, pub y: f32, diff --git a/server/src/main.rs b/server/src/main.rs index b38940cf4..6ddee05cf 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -24,7 +24,7 @@ fn main() { app.add_plugins(SimulationPlugin); app.add_plugins(BridgePlugin); - // Run one update cycle + // Single tick for smoke verification; real game loop in phase 2 app.update(); tracing::info!("Simulation server update complete"); diff --git a/server/src/npc/mod.rs b/server/src/npc/mod.rs index a365c29cc..5498ffae0 100644 --- a/server/src/npc/mod.rs +++ b/server/src/npc/mod.rs @@ -26,7 +26,8 @@ pub struct Relationships { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Relationship { - pub target_name: String, + /// Wire-format entity ID of the relationship target (scales to 10K+ NPCs) + pub target_id: u64, pub kind: String, pub trust_level: f32, } diff --git a/server/src/simulation/input.rs b/server/src/simulation/input.rs index 773992fbc..ea94108ab 100644 --- a/server/src/simulation/input.rs +++ b/server/src/simulation/input.rs @@ -13,8 +13,16 @@ pub struct InputQueue { } impl InputQueue { - /// Add a new input to the queue + /// Add a new input to the queue. + /// Inputs must be pushed in tick order for deterministic processing. + /// Panics in debug builds if tick ordering is violated. pub fn push(&mut self, input: PlayerInput) { + debug_assert!( + self.queue.back().is_none_or(|last| last.tick <= input.tick), + "InputQueue: tick ordering violated (last={}, new={})", + self.queue.back().map_or(0, |last| last.tick), + input.tick, + ); self.queue.push_back(input); } @@ -74,4 +82,18 @@ mod tests { let inputs = queue.drain_for_tick(10); assert!(inputs.is_empty()); } + + #[test] + #[should_panic(expected = "tick ordering violated")] + fn push_rejects_out_of_order_in_debug() { + let mut queue = InputQueue::default(); + queue.push(PlayerInput { + tick: 5, + action: PlayerAction::MoveNorth, + }); + queue.push(PlayerInput { + tick: 2, + action: PlayerAction::MoveSouth, + }); + } } diff --git a/server/src/simulation/time.rs b/server/src/simulation/time.rs index 29b37d0f7..753eefc1c 100644 --- a/server/src/simulation/time.rs +++ b/server/src/simulation/time.rs @@ -3,12 +3,14 @@ // Injectable time resource for deterministic replay (D-030) use bevy_ecs::prelude::*; +use serde::{Deserialize, Serialize}; pub const TICKS_PER_GAME_MINUTE: u64 = 10; pub const MINUTES_PER_PHASE: u64 = 360; pub const MINUTES_PER_DAY: u64 = 1440; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +// 4 equal 6-hour phases (adjustable — D-031 allows rebalancing phase durations) +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum DayPhase { Morning, Afternoon, @@ -33,6 +35,7 @@ impl SimulationTime { } pub fn day_phase(&self) -> DayPhase { let tod = self.time_of_day_minutes(); + // Morning: [0, 360), Afternoon: [360, 720), Evening: [720, 1080), Night: [1080, 1440) match tod { 0..360 => DayPhase::Morning, 360..720 => DayPhase::Afternoon, @@ -114,4 +117,25 @@ mod tests { schedule.run(&mut world); assert_eq!(world.resource::().tick, 1); } + + #[test] + fn day_wraparound_at_midnight() { + // 1440 minutes = 1 full day, should wrap back to Morning + let time = SimulationTime { + tick: MINUTES_PER_DAY * TICKS_PER_GAME_MINUTE, + paused: false, + }; + assert_eq!(time.day_phase(), DayPhase::Morning); + assert_eq!(time.time_of_day_minutes(), 0); + assert_eq!(time.day(), 1); + } + + #[test] + fn day_calculation() { + let time = SimulationTime { + tick: 3 * MINUTES_PER_DAY * TICKS_PER_GAME_MINUTE + 100, + paused: false, + }; + assert_eq!(time.day(), 3); + } }