diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_0.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_0.msgpack index 148ce3bcd..47a9ea59e 100644 Binary files a/client/tests/fixtures/msgpack/snapshot_boundary_tick_0.msgpack and b/client/tests/fixtures/msgpack/snapshot_boundary_tick_0.msgpack differ diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_127.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_127.msgpack index 53c6b6750..ffd3ba965 100644 Binary files a/client/tests/fixtures/msgpack/snapshot_boundary_tick_127.msgpack and b/client/tests/fixtures/msgpack/snapshot_boundary_tick_127.msgpack differ diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b31m1.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b31m1.msgpack index 91f76e316..d275e542c 100644 Binary files a/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b31m1.msgpack and b/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b31m1.msgpack differ diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b32.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b32.msgpack index 3c851d4c8..888250358 100644 Binary files a/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b32.msgpack and b/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b32.msgpack differ diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_32767.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_32767.msgpack index 304069bcb..13547db7c 100644 Binary files a/client/tests/fixtures/msgpack/snapshot_boundary_tick_32767.msgpack and b/client/tests/fixtures/msgpack/snapshot_boundary_tick_32767.msgpack differ diff --git a/client/tests/fixtures/msgpack/snapshot_empty.msgpack b/client/tests/fixtures/msgpack/snapshot_empty.msgpack index 148ce3bcd..47a9ea59e 100644 Binary files a/client/tests/fixtures/msgpack/snapshot_empty.msgpack and b/client/tests/fixtures/msgpack/snapshot_empty.msgpack differ diff --git a/client/tests/fixtures/msgpack/snapshot_multi_entity.msgpack b/client/tests/fixtures/msgpack/snapshot_multi_entity.msgpack index 7b28637f4..64ffe56ad 100644 Binary files a/client/tests/fixtures/msgpack/snapshot_multi_entity.msgpack and b/client/tests/fixtures/msgpack/snapshot_multi_entity.msgpack differ diff --git a/client/tests/fixtures/msgpack/snapshot_one_npc.msgpack b/client/tests/fixtures/msgpack/snapshot_one_npc.msgpack index a139d15d6..6e7cbb7fa 100644 Binary files a/client/tests/fixtures/msgpack/snapshot_one_npc.msgpack and b/client/tests/fixtures/msgpack/snapshot_one_npc.msgpack differ diff --git a/client/tests/fixtures/msgpack/snapshot_player.msgpack b/client/tests/fixtures/msgpack/snapshot_player.msgpack index 5cd49a1bb..1c6340e12 100644 Binary files a/client/tests/fixtures/msgpack/snapshot_player.msgpack and b/client/tests/fixtures/msgpack/snapshot_player.msgpack differ diff --git a/client/tests/fixtures/msgpack/snapshot_v2_full.msgpack b/client/tests/fixtures/msgpack/snapshot_v2_full.msgpack index 1ff2140fd..09def6a2c 100644 Binary files a/client/tests/fixtures/msgpack/snapshot_v2_full.msgpack and b/client/tests/fixtures/msgpack/snapshot_v2_full.msgpack differ diff --git a/server/src/bridge/tcp.rs b/server/src/bridge/tcp.rs index 5543d8eff..0321fc829 100644 --- a/server/src/bridge/tcp.rs +++ b/server/src/bridge/tcp.rs @@ -45,7 +45,7 @@ impl TcpBridge { ); // Set non-blocking so receive_inputs doesn't stall the game loop. - // read_framed handles WouldBlock by returning Ok(None). + // receive_inputs catches WouldBlock from read_framed and returns Ok(vec![]). stream .set_nonblocking(true) .map_err(|e| BridgeError::Transport(format!("failed to set non-blocking: {}", e)))?; diff --git a/server/src/bridge/types.rs b/server/src/bridge/types.rs index 277882188..6274ea1fa 100644 --- a/server/src/bridge/types.rs +++ b/server/src/bridge/types.rs @@ -17,7 +17,7 @@ pub use crate::simulation::time::{DayPhase, TickRate}; /// negotiation is unnecessary. Client should reject snapshots with version != /// PROTOCOL_VERSION. New fields use #[serde(default)] only during the migration /// period, then the default is removed once both sides are updated. -pub const PROTOCOL_VERSION: u8 = 14; +pub const PROTOCOL_VERSION: u8 = 15; /// Handshake message sent as the very first framed message after connection (#555). /// Client reads this before entering the normal tick loop and validates @@ -53,7 +53,7 @@ pub struct HandshakeMessage { /// Future fields: ambient sound events, HUD state (D-020 expansion). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ObserverSnapshot { - /// Protocol version for forward compatibility. Current: 14. + /// Protocol version for forward compatibility. Current: 15. pub version: u8, /// Simulation tick when this snapshot was produced pub tick: u64, diff --git a/server/src/simulation/input.rs b/server/src/simulation/input.rs index ac0840b9e..b72a7259f 100644 --- a/server/src/simulation/input.rs +++ b/server/src/simulation/input.rs @@ -314,6 +314,11 @@ pub fn process_player_input( } PlayerAction::SaveGame { ref path } => { if let Some(ref mut sl) = save_load { + if sl.pending.is_some() { + tracing::warn!( + "SaveGame overwrites already-pending save/load command (dropped)" + ); + } sl.pending = Some(SaveLoadCommand::Save { path: std::path::PathBuf::from(path), }); @@ -324,6 +329,11 @@ pub fn process_player_input( } PlayerAction::LoadGame { ref path } => { if let Some(ref mut sl) = save_load { + if sl.pending.is_some() { + tracing::warn!( + "LoadGame overwrites already-pending save/load command (dropped)" + ); + } sl.pending = Some(SaveLoadCommand::Load { path: std::path::PathBuf::from(path), }); diff --git a/server/src/simulation/save_io.rs b/server/src/simulation/save_io.rs index a4bb3562a..47b155b57 100644 --- a/server/src/simulation/save_io.rs +++ b/server/src/simulation/save_io.rs @@ -84,9 +84,10 @@ pub fn save_to_file(path: &Path, world: &mut World) -> Result<(), SaveLoadError> // Player knowledge graph — the observer's epistemics at save time let player_knowledge = { let mut q = world.query_filtered::<&KnowledgeGraph, With>(); - q.single(world) - .cloned() - .unwrap_or_else(|_| KnowledgeGraph::new()) + q.single(world).cloned().unwrap_or_else(|_| { + tracing::warn!("save_to_file: no PlayerCharacter with KnowledgeGraph found — saving empty graph"); + KnowledgeGraph::new() + }) }; // Global NPC social web @@ -610,6 +611,32 @@ mod tests { assert!(result.error.is_some(), "error message should be present"); } + // ----------------------------------------------------------------------- + // Overwrite behaviour + // ----------------------------------------------------------------------- + + /// When two commands arrive in the same tick, the second overwrites the first. + /// The warn! in process_player_input fires; here we just confirm last-write-wins. + #[test] + fn pending_command_overwrite_last_write_wins() { + let mut pending = SaveLoadPending::default(); + + pending.pending = Some(SaveLoadCommand::Save { + path: PathBuf::from("/tmp/first.msgpack"), + }); + // Overwrite with a Load command + pending.pending = Some(SaveLoadCommand::Load { + path: PathBuf::from("/tmp/second.msgpack"), + }); + + match pending.pending.unwrap() { + SaveLoadCommand::Load { ref path } => { + assert_eq!(path.to_str().unwrap(), "/tmp/second.msgpack"); + } + other => panic!("expected Load, got {:?}", other), + } + } + // ----------------------------------------------------------------------- // SaveLoadError display // ----------------------------------------------------------------------- diff --git a/server/src/simulation/tier.rs b/server/src/simulation/tier.rs index 3fe5ac4de..9a312fbd8 100644 --- a/server/src/simulation/tier.rs +++ b/server/src/simulation/tier.rs @@ -67,7 +67,12 @@ pub struct LastInteractionTick(pub u64); /// Updated each tick by `evict_excess_active`. #[derive(Resource, Debug, Clone)] pub struct SimSpacePressure { - /// Number of entities currently in `ActiveSim`. + /// Number of entities in `ActiveSim` at the start of the current tick's eviction pass. + /// + /// Set by `evict_excess_active` *before* any evictions run. Eviction commands are + /// deferred (applied after the system), so `active_count` reflects the pre-eviction + /// count, not the post-eviction count. Consumers (e.g., HUD pressure display) should + /// treat this as the high-water mark for the tick. pub active_count: usize, /// Capacity ceiling. pub capacity: usize, @@ -369,6 +374,10 @@ pub fn evict_excess_active( // Min-heap keyed by LastInteractionTick (oldest = smallest = evicted first). // Entities without LastInteractionTick get tick 0 (most stale). + // NOTE: Ties in tick value are broken by Entity index, which is non-deterministic + // across runs (bevy Entity allocation order). For v0.1 this is acceptable — + // deterministic replay (D-010 principle 4) replays inputs, not eviction order. + // If eviction order must be deterministic, key by (tick, StableId) instead. let mut heap: BinaryHeap> = BinaryHeap::new(); for (entity, pos, maybe_tick) in &active_npcs { let tick = maybe_tick.map(|t| t.0).unwrap_or(0); @@ -1213,6 +1222,101 @@ mod tests { assert_eq!(pressure.active_count, 3, "pressure tracks pre-eviction count"); } + #[test] + fn scope_pinned_npcs_survive_eviction_at_scale() { + // Regression: evict_excess_active must never demote a ScopePinned NPC, + // even when many NPCs are over capacity (D-026, #97, #98). + // + // Setup: 85 Active NPCs (capacity = 80 → 5 must be evicted). + // - 10 are ScopePinned (must ALL remain ActiveSim after eviction). + // - 75 are unpinned (5 oldest are eviction targets; 70 survive). + // + // The Without query filter in evict_excess_active is the + // core invariant under test. This test fails immediately if that filter + // is removed or mis-applied. + let mut world = World::new(); + world.insert_resource(SimSpacePressure { + active_count: 0, + capacity: 80, + }); + + // Player at origin — all NPCs are within BACKGROUND_RADIUS. + world.spawn((PlayerCharacter, make_pos(0, 0))); + + // Spawn 10 ScopePinned NPCs. Give them the oldest ticks so they would + // be prime eviction candidates if Without were absent. + let pinned: Vec = (0..10) + .map(|i| { + world + .spawn(( + Npc, + ActiveSim, + ScopePinned, + make_pos(5 + i, 0), + LastInteractionTick(i as u64), + )) + .id() + }) + .collect(); + + // Spawn 5 unpinned NPCs with old ticks — these are the actual eviction targets. + let unpinned_oldest: Vec = (0..5) + .map(|i| { + world + .spawn(( + Npc, + ActiveSim, + make_pos(20 + i, 0), + LastInteractionTick(i as u64), + )) + .id() + }) + .collect(); + + // Spawn 70 unpinned NPCs with newer ticks — these survive. + for i in 0..70i32 { + world.spawn(( + Npc, + ActiveSim, + make_pos(30 + i, 0), + LastInteractionTick(100 + i as u64), + )); + } + + // Total: 10 pinned + 5 oldest-unpinned + 70 newer-unpinned = 85 active. + // cap = 80 → exactly 5 must be evicted. + run_evict_excess_active(&mut world); + + // Core invariant: ALL pinned entities remain ActiveSim. + for (i, &entity) in pinned.iter().enumerate() { + assert!( + world.get::(entity).is_some(), + "ScopePinned NPC {} must remain ActiveSim after eviction (D-026 #98)", + i + ); + assert!( + world.get::(entity).is_none(), + "ScopePinned NPC {} must NOT be demoted to BackgroundSim", + i + ); + assert!( + world.get::(entity).is_none(), + "ScopePinned NPC {} must NOT be demoted to StateSaved", + i + ); + } + + // Sanity: the 5 oldest unpinned were the ones evicted. + let evicted_count = unpinned_oldest + .iter() + .filter(|&&e| world.get::(e).is_none()) + .count(); + assert_eq!( + evicted_count, 5, + "exactly 5 unpinned NPCs (the oldest) should have been evicted to reach capacity" + ); + } + // ----------------------------------------------------------------------- // LastInteractionTick component tests (#97) // ----------------------------------------------------------------------- diff --git a/server/tests/golden/proof_room_tick_10.json b/server/tests/golden/proof_room_tick_10.json index fdbc7e163..3b8bbad72 100644 --- a/server/tests/golden/proof_room_tick_10.json +++ b/server/tests/golden/proof_room_tick_10.json @@ -73,7 +73,7 @@ "scan_events": [], "sound_events": [], "tick": 8, - "version": 14, + "version": 15, "visible_tiles": [ { "tile_kind": "Wall", diff --git a/server/tests/serialization.rs b/server/tests/serialization.rs index 57bae1cf4..8a505e69b 100644 --- a/server/tests/serialization.rs +++ b/server/tests/serialization.rs @@ -346,7 +346,7 @@ fn protocol_version_constant_matches_snapshot() { let snapshot = test_snapshot(0, vec![]); assert_eq!(snapshot.version, PROTOCOL_VERSION); assert_eq!( - PROTOCOL_VERSION, 14, + PROTOCOL_VERSION, 15, "bump this assertion when protocol version changes" ); }