From 6ed8d11502f1bd7ace0259655296a4dcdbfe6a00 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 25 Feb 2026 12:13:03 +0100 Subject: [PATCH 1/3] =?UTF-8?q?feat(simulation):=20Sprint=2019=20=E2=80=94?= =?UTF-8?q?=207=20server=20systems?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protocol handshake (#555): HandshakeMessage as first IPC frame, HandshakeState resource, forward-compatible input handling. State serialization (#96): serialize_npc_to_frozen/deserialize with full D-024 axis coverage (10 new optional fields on NpcSaveState). Scope tags (#98): ScopeTagKind enum, ScopePinned marker, automatic assignment from KnowledgeGraph and RelationshipGraph. Timestamp eviction (#97): LastInteractionTick, SimSpacePressure, BinaryHeap LRU eviction respecting ScopePinned entities. Save/load (#553): save_to_file/load_from_file via MessagePack, SaveGame/LoadGame IPC commands, SaveLoadResultWire on snapshot. Test infrastructure (#200): Layer 3 integration test entry point, three-layer architecture documented per D-030. Information boundary tests (#272): 4 negative tests proving no passive KG leakage, LOS fog holds, tier boundary holds, save isolation per NPC. 1063 tests passing, 0 failures. Co-Authored-By: Claude Opus 4.6 --- server/Cargo.toml | 10 + server/src/bridge/local.rs | 15 + server/src/bridge/mod.rs | 35 + server/src/bridge/tcp.rs | 20 + server/src/bridge/text_renderer.rs | 4 +- server/src/bridge/types.rs | 86 ++- server/src/knowledge/registry.rs | 32 + server/src/main.rs | 15 +- server/src/perception/observer/mod.rs | 13 +- server/src/simulation/input.rs | 34 +- server/src/simulation/mod.rs | 8 + server/src/simulation/save_io.rs | 629 ++++++++++++++++ server/src/simulation/save_state.rs | 486 +++++++++++- server/src/simulation/tier.rs | 790 ++++++++++++++++++++ server/tests/bridge_ipc.rs | 2 +- server/tests/bridge_tcp.rs | 2 +- server/tests/gen_fixtures.rs | 4 +- server/tests/golden/proof_room_tick_10.json | 1 - server/tests/information_boundaries.rs | 314 ++++++++ server/tests/integration/mod.rs | 60 ++ server/tests/layer3.rs | 20 +- server/tests/serialization.rs | 16 +- 22 files changed, 2561 insertions(+), 35 deletions(-) create mode 100644 server/src/simulation/save_io.rs create mode 100644 server/tests/information_boundaries.rs create mode 100644 server/tests/integration/mod.rs diff --git a/server/Cargo.toml b/server/Cargo.toml index d3864c8ce..1afc640c4 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -24,3 +24,13 @@ gauntlet = [] [dev-dependencies] serde_json = "1" + +# --------------------------------------------------------------------------- +# Explicit test target for the Layer 3 integration module (D-030, ticket #200). +# tests/integration/mod.rs cannot be auto-discovered by cargo (only top-level +# *.rs files in tests/ are auto-discovered). This declaration makes it a named +# test binary: `cargo test --test integration_layer3`. +# --------------------------------------------------------------------------- +[[test]] +name = "integration_layer3" +path = "tests/integration/mod.rs" diff --git a/server/src/bridge/local.rs b/server/src/bridge/local.rs index af5b395b3..61ab95073 100644 --- a/server/src/bridge/local.rs +++ b/server/src/bridge/local.rs @@ -80,6 +80,21 @@ impl LocalBridge { } impl SimBridge for LocalBridge { + fn send_handshake(&self) -> Result<(), BridgeError> { + use super::types::{HandshakeMessage, PROTOCOL_VERSION}; + let msg = HandshakeMessage { + protocol_version: PROTOCOL_VERSION, + }; + let payload = rmp_serde::to_vec_named(&msg)?; + let mut writer = self + .writer + .lock() + .map_err(|e| BridgeError::MutexPoisoned(format!("writer: {}", e)))?; + write_framed(writer.get_mut(), &payload)?; + tracing::info!("sent handshake: protocol_version={}", PROTOCOL_VERSION); + Ok(()) + } + fn send_snapshot(&self, snapshot: &ObserverSnapshot) -> Result<(), BridgeError> { let payload = rmp_serde::to_vec_named(snapshot)?; diff --git a/server/src/bridge/mod.rs b/server/src/bridge/mod.rs index a09fcaf8f..89ea3c363 100644 --- a/server/src/bridge/mod.rs +++ b/server/src/bridge/mod.rs @@ -35,6 +35,11 @@ pub enum BridgeError { /// Abstracts transport layer (D-020) /// Implemented by LocalBridge (stdio) and future NetworkBridge pub trait SimBridge: Send + Sync { + /// Send the protocol handshake as the first framed message (#555). + /// Must be called exactly once, immediately after connection, before + /// any ObserverSnapshot is sent. + fn send_handshake(&self) -> Result<(), BridgeError>; + /// Send an observer snapshot to the client fn send_snapshot(&self, snapshot: &ObserverSnapshot) -> Result<(), BridgeError>; @@ -55,6 +60,10 @@ impl BridgeResource { } } + pub fn send_handshake(&self) -> Result<(), BridgeError> { + self.inner.send_handshake() + } + pub fn send_snapshot(&self, snapshot: &ObserverSnapshot) -> Result<(), BridgeError> { self.inner.send_snapshot(snapshot) } @@ -64,15 +73,40 @@ impl BridgeResource { } } +/// Tracks whether the protocol handshake has been sent (#555). +/// Inserted by BridgePlugin as Pending. Set to Complete in main.rs after +/// `send_handshake()` succeeds. `receive_bridge_inputs` logs a warning +/// if inputs arrive while still Pending. +#[derive(Resource, Debug, Clone, Copy, PartialEq, Eq)] +pub enum HandshakeState { + /// Handshake not yet sent. Inputs arriving in this state trigger a warning. + Pending, + /// Handshake sent. Normal operation. + Complete, +} + +impl Default for HandshakeState { + fn default() -> Self { + Self::Pending + } +} + /// Receive inputs from bridge and push to InputQueue pub fn receive_bridge_inputs( bridge: Option>, mut input_queue: ResMut, mut running: ResMut, + handshake: Res, ) { let Some(bridge) = bridge else { return }; match bridge.receive_inputs() { Ok(inputs) => { + if !inputs.is_empty() && *handshake == HandshakeState::Pending { + tracing::warn!( + "Received {} input(s) before handshake completed — processing anyway (forward-compatible)", + inputs.len() + ); + } for input in &inputs { tracing::trace!( "Received input: tick={} action={:?}", @@ -156,6 +190,7 @@ impl Plugin for BridgePlugin { fn build(&self, app: &mut App) { app.init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .add_systems( diff --git a/server/src/bridge/tcp.rs b/server/src/bridge/tcp.rs index d4ddf2a88..5543d8eff 100644 --- a/server/src/bridge/tcp.rs +++ b/server/src/bridge/tcp.rs @@ -129,6 +129,26 @@ impl TcpBridge { } impl SimBridge for TcpBridge { + fn send_handshake(&self) -> Result<(), BridgeError> { + use super::types::{HandshakeMessage, PROTOCOL_VERSION}; + let msg = HandshakeMessage { + protocol_version: PROTOCOL_VERSION, + }; + let payload = rmp_serde::to_vec_named(&msg)?; + let mut writer = self + .writer + .lock() + .map_err(|e| BridgeError::MutexPoisoned(format!("writer: {}", e)))?; + // Toggle to blocking for reliable handshake delivery. + let stream = writer.get_mut(); + stream.set_nonblocking(false).map_err(BridgeError::Io)?; + let result = write_framed(stream, &payload); + stream.set_nonblocking(true).map_err(BridgeError::Io)?; + result?; + tracing::info!("sent handshake: protocol_version={}", PROTOCOL_VERSION); + Ok(()) + } + fn send_snapshot(&self, snapshot: &ObserverSnapshot) -> Result<(), BridgeError> { let payload = rmp_serde::to_vec_named(snapshot)?; diff --git a/server/src/bridge/text_renderer.rs b/server/src/bridge/text_renderer.rs index df789e107..777c32e50 100644 --- a/server/src/bridge/text_renderer.rs +++ b/server/src/bridge/text_renderer.rs @@ -306,13 +306,13 @@ mod tests { conversation_events: vec![], conversation_ended: vec![], follow_state: None, - examine_result: None, character_pressure: None, sound_events: vec![], rng_seed: None, poi_list: vec![], examine_result: None, player_knowledge: None, + save_result: None, } } @@ -441,13 +441,13 @@ mod tests { conversation_events: vec![], conversation_ended: vec![], follow_state: None, - examine_result: None, character_pressure: None, sound_events: vec![], rng_seed: None, poi_list: vec![], examine_result: None, player_knowledge: None, + save_result: None, }; let text = format_snapshot_text(&snap); assert!(text.contains("Tick 0")); diff --git a/server/src/bridge/types.rs b/server/src/bridge/types.rs index fdec7a988..277882188 100644 --- a/server/src/bridge/types.rs +++ b/server/src/bridge/types.rs @@ -19,6 +19,16 @@ pub use crate::simulation::time::{DayPhase, TickRate}; /// period, then the default is removed once both sides are updated. pub const PROTOCOL_VERSION: u8 = 14; +/// Handshake message sent as the very first framed message after connection (#555). +/// Client reads this before entering the normal tick loop and validates +/// `protocol_version` against its own `PROTOCOL_VERSION` constant. +/// Wire format: MessagePack, same 4-byte length-prefixed framing as ObserverSnapshot. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct HandshakeMessage { + /// Must match client's PROTOCOL_VERSION or the client should disconnect. + pub protocol_version: u8, +} + /// The ONLY data structure crossing the client-server boundary (D-020) /// Contains all information visible to the observer at a given tick. /// @@ -39,6 +49,7 @@ pub const PROTOCOL_VERSION: u8 = 14; /// v14 adds: poi_list (#151, discovered POIs for minimap rendering), /// examine_result (#242, character-filtered examine observation text), /// player_knowledge (#264, partial KG dump for journal/knowledge panel). +/// v15 adds: save_result (#553, save/load operation result for client confirmation). /// Future fields: ambient sound events, HUD state (D-020 expansion). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ObserverSnapshot { @@ -110,11 +121,6 @@ pub struct ObserverSnapshot { /// Client shows follow indicator with distance, LOS, and tension. #[serde(default)] pub follow_state: Option, - /// Examine result from Examine verb interaction (#242). - /// Present when the player examined an NPC or object this tick. - /// Client displays character-filtered detail text in an observation panel. - #[serde(default)] - pub examine_result: Option, /// Character pressure state for client HUD widget (#248). /// Present when pressure is non-zero. Client renders tension indicator. #[serde(default)] @@ -140,6 +146,11 @@ pub struct ObserverSnapshot { /// Client renders as a read-only journal grouped by entity. #[serde(default, skip_serializing_if = "Option::is_none")] pub player_knowledge: Option, + /// Result of the most recently completed save or load (#553, D-085). + /// Present for exactly one tick after the operation completes. + /// Client shows a confirmation toast (success) or error modal (failure). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub save_result: Option, } /// Game time data for client display (D-031) @@ -414,6 +425,14 @@ pub enum PlayerAction { target_entity_id: u64, response_id: String, }, + /// Save the current game state to `path` (#553, D-085). + /// Client sends this when the player activates the save UI. + /// Server executes save_to_file and sends SaveLoadResultWire confirmation. + SaveGame { path: String }, + /// Load a previously saved game from `path` (#553, D-085). + /// Client sends this when the player selects a save file to load. + /// Server executes load_from_file and sends SaveLoadResultWire confirmation. + LoadGame { path: String }, } impl PlayerAction { @@ -642,8 +661,65 @@ pub struct KnownFactWire { pub acquired_tick: u64, } +/// Save/load operation result for client confirmation (#553, D-085). +/// +/// Included in `ObserverSnapshot.save_result` for exactly one tick after the +/// operation completes. `success=false` carries a human-readable `error` string. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SaveLoadResultWire { + /// Whether the save or load succeeded. + pub success: bool, + /// "save" or "load" — identifies which operation completed. + pub kind: String, + /// Error message if `success` is false. None on success. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub error: Option, +} + /// Snapshot buffer resource for staging outgoing ObserverSnapshots #[derive(Resource, Debug, Default)] pub struct SnapshotBuffer { pub snapshot: Option, + /// Pending save/load result, consumed once by `compute_observer_snapshot` (#553). + pub pending_save_result: Option, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn handshake_message_roundtrip() { + let msg = HandshakeMessage { + protocol_version: PROTOCOL_VERSION, + }; + let bytes = rmp_serde::to_vec_named(&msg).expect("serialize"); + let decoded: HandshakeMessage = rmp_serde::from_slice(&bytes).expect("deserialize"); + assert_eq!(decoded, msg); + assert_eq!(decoded.protocol_version, PROTOCOL_VERSION); + } + + #[test] + fn handshake_message_rejects_wrong_version() { + let msg = HandshakeMessage { + protocol_version: PROTOCOL_VERSION, + }; + let bytes = rmp_serde::to_vec_named(&msg).expect("serialize"); + let decoded: HandshakeMessage = rmp_serde::from_slice(&bytes).expect("deserialize"); + // Simulate client-side validation: version mismatch should be detectable + let wrong_version = PROTOCOL_VERSION.wrapping_add(1); + assert_ne!(decoded.protocol_version, wrong_version); + } + + #[test] + fn handshake_is_distinct_from_snapshot() { + // HandshakeMessage and ObserverSnapshot are different types on the wire. + // A HandshakeMessage should NOT deserialize as an ObserverSnapshot. + let msg = HandshakeMessage { + protocol_version: PROTOCOL_VERSION, + }; + let bytes = rmp_serde::to_vec_named(&msg).expect("serialize"); + let result = rmp_serde::from_slice::(&bytes); + assert!(result.is_err(), "HandshakeMessage must not deserialize as ObserverSnapshot"); + } } diff --git a/server/src/knowledge/registry.rs b/server/src/knowledge/registry.rs index 915d308d2..7f67b32b1 100644 --- a/server/src/knowledge/registry.rs +++ b/server/src/knowledge/registry.rs @@ -83,6 +83,38 @@ impl EntityRegistry { self.next_id = target; } + /// Register an entity with a specific pre-existing StableId (used during save/load). + /// + /// Unlike `register`, this does NOT advance `next_id`. After bulk-registering + /// all restored entities, call `advance_past(max_stable_id)` so future `register()` + /// calls produce IDs that don't conflict with the restored set. + /// + /// No-op if the entity is already mapped to the same `stable_id`. + /// Panics in debug builds if `stable_id` is already mapped to a different entity. + pub fn register_existing(&mut self, entity: Entity, stable_id: StableId) { + if let Some(&existing) = self.by_stable_id.get(&stable_id) { + debug_assert_eq!( + existing, entity, + "register_existing: StableId {:?} already mapped to a different entity", + stable_id + ); + return; + } + self.by_stable_id.insert(stable_id, entity); + self.by_entity.insert(entity, stable_id); + } + + /// Advance `next_id` past `id` so future `register()` calls don't conflict. + /// + /// Unlike `reserve_up_to`, this never panics: if the counter is already past `id`, + /// this is a no-op. Use after `register_existing` bulk-load to position the counter. + pub fn advance_past(&mut self, id: u64) { + let target = id.saturating_add(1); + if target > self.next_id { + self.next_id = target; + } + } + /// Number of registered entities. pub fn len(&self) -> usize { self.by_entity.len() diff --git a/server/src/main.rs b/server/src/main.rs index 8913c7b54..61b66fd2d 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -11,7 +11,7 @@ use bevy_app::prelude::*; use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; use settled_reach_server::bridge::tcp::TcpBridge; -use settled_reach_server::bridge::{BridgePlugin, BridgeResource, ServerRunning}; +use settled_reach_server::bridge::{BridgePlugin, BridgeResource, HandshakeState, ServerRunning}; use settled_reach_server::simulation::SimulationPlugin; fn main() { @@ -121,7 +121,17 @@ fn main() { tracing::error!("Failed to accept: {}", e); std::process::exit(1); }); - tracing::info!("Client connected, initializing simulation"); + tracing::info!("Client connected, sending protocol handshake"); + + // Protocol handshake: first framed message on the wire (#555). + // Client reads this and validates protocol_version before sending any input. + use settled_reach_server::bridge::SimBridge; + bridge.send_handshake().unwrap_or_else(|e| { + tracing::error!("Failed to send handshake: {}", e); + std::process::exit(1); + }); + + tracing::info!("Handshake sent, initializing simulation"); // RNG seed: test-mode defaults to 42 for deterministic replay let seed = seed_flag.unwrap_or(if test_mode { 42 } else { 0 }); @@ -138,6 +148,7 @@ fn main() { }); app.add_plugins(settled_reach_server::content::ContentPlugin); app.insert_resource(BridgeResource::new(bridge)); + app.insert_resource(HandshakeState::Complete); // Override SimRng with the chosen seed (SimulationPlugin defaults to seed 0) app.insert_resource(settled_reach_server::simulation::rng::SimRng::new(seed)); diff --git a/server/src/perception/observer/mod.rs b/server/src/perception/observer/mod.rs index bf82678bf..6d07f11fa 100644 --- a/server/src/perception/observer/mod.rs +++ b/server/src/perception/observer/mod.rs @@ -378,6 +378,9 @@ pub fn compute_observer_snapshot( None }; + // Consume pending save/load result for this tick (#553). + let save_result = buffer.pending_save_result.take(); + buffer.snapshot = Some(ObserverSnapshot { version: crate::bridge::types::PROTOCOL_VERSION, tick: time.tick, @@ -396,15 +399,21 @@ pub fn compute_observer_snapshot( conversation_events, conversation_ended, follow_state, - examine_result, character_pressure: pressure_query.iter().next().map(|p| { crate::simulation::pressure::CharacterPressureWire::from(p) }), sound_events, rng_seed: sim_rng.as_deref().map(|r| r.seed()), poi_list, - examine_result: None, // Populated by examine system when #242 lands + examine_result: examine_result.map(|e| { + crate::bridge::types::ExamineResultWire { + entity_id: e.target_entity_id, + text: e.text, + confidence: crate::bridge::types::KnowledgeConfidence::Direct, + } + }), player_knowledge, + save_result, }); } diff --git a/server/src/simulation/input.rs b/server/src/simulation/input.rs index a04c233a3..ac0840b9e 100644 --- a/server/src/simulation/input.rs +++ b/server/src/simulation/input.rs @@ -9,6 +9,7 @@ use crate::simulation::inventory::{ find_next_slot, occupied_slots_for, CarriedBy, InventorySlot, ItemName, MAX_INVENTORY_SLOTS, }; use crate::simulation::movement::{MoveIntent, PlayerCharacter, TilePosition}; +use crate::simulation::save_io::{SaveLoadCommand, SaveLoadPending}; use crate::simulation::stance::{PlayerMoveCooldown, Stance}; use crate::simulation::time::{SimulationTime, TickRate}; use crate::test_world::reset::{RoomResetTrigger, RoomSnapshots}; @@ -74,7 +75,8 @@ impl InputQueue { } /// Drains InputQueue for the current tick, converts PlayerActions to ECS components. -/// Handles stance toggling (D-053), movement cooldown, and Take/Place verbs (#424). +/// Handles stance toggling (D-053), movement cooldown, Take/Place verbs (#424), +/// and save/load commands (#553). #[allow(clippy::type_complexity, clippy::too_many_arguments)] pub fn process_player_input( mut input_queue: ResMut, @@ -94,6 +96,7 @@ pub fn process_player_input( all_positions: Query<&TilePosition>, reset_triggers: Query<&RoomResetTrigger>, mut room_snapshots: Option>, + mut save_load: Option>, ) { let current_tick = time.tick; let paused = time.paused(); @@ -104,12 +107,15 @@ pub fn process_player_input( for input in inputs { // Discard all gameplay actions while paused (D-052, R2-OQ-01). - // Only Pause/Unpause/TeleportToHub are processed — everything else is discarded. - // TeleportToHub is exempted because it's a Gauntlet QA action (#491). + // SaveGame/LoadGame are also exempted — saving while paused is valid (#553). if paused && !matches!( input.action, - PlayerAction::Pause | PlayerAction::Unpause | PlayerAction::TeleportToHub + PlayerAction::Pause + | PlayerAction::Unpause + | PlayerAction::TeleportToHub + | PlayerAction::SaveGame { .. } + | PlayerAction::LoadGame { .. } ) { continue; @@ -306,6 +312,26 @@ pub fn process_player_input( PlayerAction::UsePerceptionMode(ref mode) => { tracing::trace!("UsePerceptionMode({}) — no-op for Sprint 1", mode); } + PlayerAction::SaveGame { ref path } => { + if let Some(ref mut sl) = save_load { + sl.pending = Some(SaveLoadCommand::Save { + path: std::path::PathBuf::from(path), + }); + tracing::info!("SaveGame queued: {:?}", path); + } else { + tracing::warn!("SaveGame received but SaveLoadPending resource not registered"); + } + } + PlayerAction::LoadGame { ref path } => { + if let Some(ref mut sl) = save_load { + sl.pending = Some(SaveLoadCommand::Load { + path: std::path::PathBuf::from(path), + }); + tracing::info!("LoadGame queued: {:?}", path); + } else { + tracing::warn!("LoadGame received but SaveLoadPending resource not registered"); + } + } } } diff --git a/server/src/simulation/mod.rs b/server/src/simulation/mod.rs index 43edc59f5..de0f3436b 100644 --- a/server/src/simulation/mod.rs +++ b/server/src/simulation/mod.rs @@ -22,6 +22,7 @@ pub mod poi; pub mod poi_discovery; pub mod pressure; pub mod rng; +pub mod save_io; pub mod save_state; pub mod sound; pub mod spatial; @@ -43,6 +44,7 @@ impl Plugin for SimulationPlugin { app.init_resource::() .insert_resource(rng::SimRng::new(0)) .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -61,6 +63,12 @@ impl Plugin for SimulationPlugin { Update, ( input::process_player_input, + // execute_save_load is an exclusive system (takes &mut World). + // Must run after process_player_input (which queues the command) + // and before compute_observer_snapshot (which consumes the result). + save_io::execute_save_load + .after(input::process_player_input) + .before(crate::perception::observer::compute_observer_snapshot), pathfinding::compute_paths.after(input::process_player_input), path_follow::follow_paths.after(pathfinding::compute_paths), movement::validate_movement.after(path_follow::follow_paths), diff --git a/server/src/simulation/save_io.rs b/server/src/simulation/save_io.rs new file mode 100644 index 000000000..a4bb3562a --- /dev/null +++ b/server/src/simulation/save_io.rs @@ -0,0 +1,629 @@ +// Save/load ECS extraction (#553) +// Implements D-020 MessagePack format for save files, D-010 determinism. +// +// Two entry points: +// save_to_file: queries ECS, builds SaveStateV1, writes MessagePack to path. +// load_from_file: reads path, deserialises SaveStateV1, re-injects ECS state. +// +// IPC: SaveGame / LoadGame PlayerAction variants queue commands here. +// execute_save_load: exclusive system that drains the queue and writes the result +// to SnapshotBuffer.pending_save_result for client feedback. + +use std::path::{Path, PathBuf}; + +use bevy_ecs::prelude::*; +use thiserror::Error; + +use crate::bridge::types::SaveLoadResultWire; +use crate::bridge::types::SnapshotBuffer; +use crate::knowledge::graph::KnowledgeGraph; +use crate::knowledge::registry::EntityRegistry; +use crate::npc::Npc; +use crate::npc::relationships::RelationshipGraph; +use crate::simulation::movement::PlayerCharacter; +use crate::simulation::rng::SimRng; +use crate::simulation::save_state::{ + deserialize_npc_from_frozen, serialize_npc_to_frozen, SaveStateV1, SAVE_FORMAT_VERSION, +}; +use crate::simulation::tier::BackgroundSim; +use crate::simulation::time::SimulationTime; + +/// Errors from save/load operations (#553). +#[derive(Debug, Error)] +pub enum SaveLoadError { + #[error("I/O error: {0}")] + Io(String), + #[error("serialization error: {0}")] + Serialize(String), + #[error("deserialization error: {0}")] + Deserialize(String), + #[error("format version mismatch: expected {expected}, found {found}")] + VersionMismatch { expected: u8, found: u8 }, +} + +/// A queued save or load command (#553). +#[derive(Debug, Clone)] +pub enum SaveLoadCommand { + Save { path: PathBuf }, + Load { path: PathBuf }, +} + +/// Pending save/load command resource (#553). +/// +/// `process_player_input` queues commands here when it encounters +/// `PlayerAction::SaveGame` or `PlayerAction::LoadGame`. The +/// `execute_save_load` exclusive system drains this queue each tick. +#[derive(Resource, Debug, Default)] +pub struct SaveLoadPending { + /// Pending command (at most one; new commands overwrite pending ones). + pub pending: Option, +} + +/// Extract world state into `SaveStateV1` and write MessagePack bytes to `path` (#553). +/// +/// Queries all NPC entities, the player knowledge graph, global relationship graph, +/// simulation time, and RNG seed. Builds `SaveStateV1` and writes to disk. +/// +/// NPC states are sorted by `stable_id` ascending for determinism (D-010). +/// NPCs without `StableEntityId` trigger a panic (caller invariant — all live +/// NPCs must be registered before save). +/// +/// # Errors +/// `SaveLoadError::Io` on filesystem failure. +/// `SaveLoadError::Serialize` on MessagePack encoding failure. +pub fn save_to_file(path: &Path, world: &mut World) -> Result<(), SaveLoadError> { + // Simulation clock + let (tick, tick_rate) = { + let t = world.resource::(); + (t.tick, t.tick_rate) + }; + + // RNG seed for deterministic replay (D-010) + let seed = world.resource::().seed(); + + // 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()) + }; + + // Global NPC social web + let relationship_graph = world.resource::().clone(); + + // Per-NPC states: collect then sort by stable_id (D-010 determinism) + let npc_entities: Vec = { + let mut q = world.query_filtered::>(); + q.iter(world).collect() + }; + let mut npc_states: Vec<_> = npc_entities + .iter() + .map(|&entity| serialize_npc_to_frozen(entity, world)) + .collect(); + npc_states.sort_by_key(|s| s.stable_id.0); + + let npc_count = npc_states.len(); + let state = SaveStateV1 { + format_version: SAVE_FORMAT_VERSION, + tick, + tick_rate, + seed, + player_knowledge, + relationship_graph, + npc_states, + }; + + let bytes = state + .to_bytes() + .map_err(|e| SaveLoadError::Serialize(e.to_string()))?; + + std::fs::write(path, &bytes).map_err(|e| SaveLoadError::Io(e.to_string()))?; + + tracing::info!( + "save_to_file: {:?} (tick={}, npcs={}, {} bytes)", + path, + tick, + npc_count, + bytes.len() + ); + Ok(()) +} + +/// Read `path`, deserialise `SaveStateV1`, and re-inject state into the ECS (#553). +/// +/// Steps: +/// 1. Read and deserialise bytes; reject if `format_version != SAVE_FORMAT_VERSION`. +/// 2. Despawn all existing NPC entities and unregister them from `EntityRegistry`. +/// 3. Re-spawn each NPC via `deserialize_npc_from_frozen`; register with +/// `register_existing`; insert `BackgroundSim` tier marker. +/// 4. Advance `EntityRegistry` counter past all restored IDs. +/// 5. Restore `RelationshipGraph`, `SimulationTime`, and `SimRng` resources. +/// 6. Update the player entity's `KnowledgeGraph` if a player entity exists. +/// +/// **Gotcha (D-010):** Bevy `Entity` handles are generational. `NpcSaveState` uses +/// `StableId(u64)` throughout — `EntityRegistry` maps restored `StableId`s to the +/// new `Entity` handles after re-spawn. +/// +/// # Errors +/// `SaveLoadError::Io` on filesystem failure. +/// `SaveLoadError::Deserialize` on MessagePack decoding failure. +/// `SaveLoadError::VersionMismatch` when the save file predates the current schema. +pub fn load_from_file(path: &Path, world: &mut World) -> Result<(), SaveLoadError> { + let bytes = std::fs::read(path).map_err(|e| SaveLoadError::Io(e.to_string()))?; + let state = + SaveStateV1::from_bytes(&bytes).map_err(|e| SaveLoadError::Deserialize(e.to_string()))?; + + if state.format_version != SAVE_FORMAT_VERSION { + return Err(SaveLoadError::VersionMismatch { + expected: SAVE_FORMAT_VERSION, + found: state.format_version, + }); + } + + let npc_count = state.npc_states.len(); + + // Despawn all existing NPC entities and clear their registry entries. + let npc_entities: Vec = { + let mut q = world.query_filtered::>(); + q.iter(world).collect() + }; + for entity in npc_entities { + world.resource_mut::().unregister(entity); + world.despawn(entity); + } + + // Track the highest restored StableId so we can advance the counter. + let mut max_id: u64 = 0; + + // Re-spawn NPCs, assign tier marker, register pre-existing StableIds. + for npc_state in &state.npc_states { + let entity = deserialize_npc_from_frozen(npc_state, world); + + // Loaded NPCs start in BackgroundSim; the distance system promotes as needed. + world.entity_mut(entity).insert(BackgroundSim); + + let stable_id = npc_state.stable_id; + world + .resource_mut::() + .register_existing(entity, stable_id); + + max_id = max_id.max(stable_id.0); + } + + // Advance the registry counter past all restored IDs so future register() + // calls produce non-conflicting IDs. + if npc_count > 0 { + world.resource_mut::().advance_past(max_id); + } + + // Restore simulation resources. + world.insert_resource(state.relationship_graph); + { + let mut t = world.resource_mut::(); + t.tick = state.tick; + t.tick_rate = state.tick_rate; + } + world.insert_resource(SimRng::new(state.seed)); + + // Update the player entity's KnowledgeGraph if a player exists. + let player_entity = { + let mut q = world.query_filtered::>(); + q.single(world).ok() + }; + if let Some(player_entity) = player_entity { + world + .entity_mut(player_entity) + .insert(state.player_knowledge); + } + + tracing::info!( + "load_from_file: {:?} (tick={}, npcs={})", + path, + state.tick, + npc_count, + ); + Ok(()) +} + +/// Exclusive system: drain `SaveLoadPending` and execute queued save/load (#553). +/// +/// Runs each tick, after `process_player_input`. If a command is pending, +/// executes it and writes `SaveLoadResultWire` to `SnapshotBuffer.pending_save_result` +/// for consumption by `compute_observer_snapshot` the same tick. +pub fn execute_save_load(world: &mut World) { + // Take the pending command (releases the borrow before we use world again). + let command = { + let mut pending = world.resource_mut::(); + pending.pending.take() + }; + + let Some(command) = command else { + return; + }; + + let (kind_str, result) = match &command { + SaveLoadCommand::Save { path } => { + let r = save_to_file(path, world); + ("save", r) + } + SaveLoadCommand::Load { path } => { + let r = load_from_file(path, world); + ("load", r) + } + }; + + let wire_result = match result { + Ok(()) => { + tracing::info!("execute_save_load: {} completed", kind_str); + SaveLoadResultWire { + success: true, + kind: kind_str.to_string(), + error: None, + } + } + Err(ref e) => { + tracing::error!("execute_save_load: {} failed: {}", kind_str, e); + SaveLoadResultWire { + success: false, + kind: kind_str.to_string(), + error: Some(e.to_string()), + } + } + }; + + // Write result to SnapshotBuffer for client feedback (one tick only — consumed by + // compute_observer_snapshot via pending_save_result.take()). + if let Some(mut buf) = world.get_resource_mut::() { + buf.pending_save_result = Some(wire_result); + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + use crate::knowledge::graph::KnowledgeGraph; + use crate::knowledge::registry::{EntityRegistry, StableEntityId}; + use crate::knowledge::types::StableId; + use crate::npc::Npc; + use crate::npc::relationships::RelationshipGraph; + use crate::simulation::movement::TilePosition; + use crate::simulation::rng::SimRng; + use crate::simulation::save_state::{SaveStateV1, SAVE_FORMAT_VERSION}; + use crate::simulation::time::{SimulationTime, TickRate}; + use bevy_ecs::world::World; + use std::sync::atomic::{AtomicU64, Ordering}; + + static COUNTER: AtomicU64 = AtomicU64::new(0); + + fn temp_path() -> PathBuf { + let id = COUNTER.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!("settled_reach_save_io_test_{}.msgpack", id)) + } + + fn minimal_world() -> World { + let mut w = World::new(); + w.insert_resource(SimulationTime::default()); + w.insert_resource(SimRng::new(42)); + w.insert_resource(RelationshipGraph::new()); + w.init_resource::(); + w + } + + fn spawn_test_npc(world: &mut World, stable_id: u64) -> Entity { + world + .spawn(( + Npc, + StableEntityId(StableId(stable_id)), + TilePosition::new(stable_id as i32, 0, 0), + )) + .id() + } + + // ----------------------------------------------------------------------- + // save_to_file + // ----------------------------------------------------------------------- + + #[test] + fn save_to_file_creates_valid_msgpack() { + let mut world = minimal_world(); + spawn_test_npc(&mut world, 1); + spawn_test_npc(&mut world, 2); + + let path = temp_path(); + save_to_file(&path, &mut world).expect("save should succeed"); + + let bytes = std::fs::read(&path).expect("file should exist"); + let state = SaveStateV1::from_bytes(&bytes).expect("bytes must be valid msgpack"); + assert_eq!(state.format_version, SAVE_FORMAT_VERSION); + assert_eq!(state.npc_states.len(), 2); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn save_to_file_sorts_npc_states_by_stable_id() { + let mut world = minimal_world(); + // Spawn in reverse order — save should still sort ascending + spawn_test_npc(&mut world, 50); + spawn_test_npc(&mut world, 10); + spawn_test_npc(&mut world, 30); + + let path = temp_path(); + save_to_file(&path, &mut world).expect("save should succeed"); + + let bytes = std::fs::read(&path).expect("read saved file"); + let state = SaveStateV1::from_bytes(&bytes).unwrap(); + let ids: Vec = state.npc_states.iter().map(|n| n.stable_id.0).collect(); + assert_eq!(ids, vec![10, 30, 50], "npc_states must be sorted by stable_id"); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn save_to_file_preserves_tick_and_seed() { + let mut world = minimal_world(); + { + let mut t = world.resource_mut::(); + t.tick = 9999; + t.tick_rate = TickRate::Half; + } + world.insert_resource(SimRng::new(0xDEADBEEF)); + + let path = temp_path(); + save_to_file(&path, &mut world).expect("save"); + + let bytes = std::fs::read(&path).unwrap(); + let state = SaveStateV1::from_bytes(&bytes).unwrap(); + assert_eq!(state.tick, 9999); + assert_eq!(state.tick_rate, TickRate::Half); + assert_eq!(state.seed, 0xDEADBEEF); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn save_to_file_returns_io_error_on_bad_path() { + let mut world = minimal_world(); + let bad_path = std::path::Path::new("/nonexistent/directory/save.msgpack"); + let result = save_to_file(bad_path, &mut world); + assert!( + matches!(result, Err(SaveLoadError::Io(_))), + "expected Io error for bad path" + ); + } + + // ----------------------------------------------------------------------- + // load_from_file + // ----------------------------------------------------------------------- + + #[test] + fn load_from_file_restores_npc_count() { + let mut world = minimal_world(); + spawn_test_npc(&mut world, 1); + spawn_test_npc(&mut world, 2); + + let path = temp_path(); + save_to_file(&path, &mut world).expect("save"); + + // Spawn an extra NPC — loading should despawn the old NPCs and restore exactly 2 + spawn_test_npc(&mut world, 99); + let pre_load_count = { + let mut q = world.query_filtered::>(); + q.iter(&world).count() + }; + assert_eq!(pre_load_count, 3, "three NPCs before load"); + + load_from_file(&path, &mut world).expect("load"); + + let post_load_count = { + let mut q = world.query_filtered::>(); + q.iter(&world).count() + }; + assert_eq!(post_load_count, 2, "exactly the two saved NPCs after load"); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn load_from_file_restores_stable_ids_in_registry() { + let mut world = minimal_world(); + spawn_test_npc(&mut world, 10); + spawn_test_npc(&mut world, 20); + + let path = temp_path(); + save_to_file(&path, &mut world).expect("save"); + load_from_file(&path, &mut world).expect("load"); + + let registry = world.resource::(); + assert!( + registry.to_entity(&StableId(10)).is_some(), + "StableId(10) must be in registry after load" + ); + assert!( + registry.to_entity(&StableId(20)).is_some(), + "StableId(20) must be in registry after load" + ); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn load_from_file_restores_tick_and_seed() { + let mut world = minimal_world(); + { + let mut t = world.resource_mut::(); + t.tick = 5000; + } + world.insert_resource(SimRng::new(12345)); + + let path = temp_path(); + save_to_file(&path, &mut world).expect("save"); + + // Change time and seed, then load + { + let mut t = world.resource_mut::(); + t.tick = 1; + } + world.insert_resource(SimRng::new(0)); + + load_from_file(&path, &mut world).expect("load"); + + let t = world.resource::(); + assert_eq!(t.tick, 5000, "tick restored from save"); + assert_eq!( + world.resource::().seed(), + 12345, + "seed restored from save" + ); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn load_from_file_rejects_wrong_format_version() { + // Craft a save with a wrong format_version + let bad_state = SaveStateV1 { + format_version: 0xFF, // deliberately wrong + tick: 0, + tick_rate: TickRate::Full, + seed: 0, + player_knowledge: KnowledgeGraph::new(), + relationship_graph: RelationshipGraph::new(), + npc_states: vec![], + }; + let bytes = bad_state.to_bytes().expect("serialize"); + let path = temp_path(); + std::fs::write(&path, &bytes).expect("write"); + + let mut world = minimal_world(); + let result = load_from_file(&path, &mut world); + assert!( + matches!(result, Err(SaveLoadError::VersionMismatch { .. })), + "expected VersionMismatch error, got {:?}", + result + ); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn load_from_file_returns_io_error_for_missing_file() { + let mut world = minimal_world(); + let missing = std::path::Path::new("/tmp/settled_reach_nonexistent_42.msgpack"); + let result = load_from_file(missing, &mut world); + assert!( + matches!(result, Err(SaveLoadError::Io(_))), + "expected Io error for missing file" + ); + } + + #[test] + fn load_from_file_assigns_background_sim_tier() { + let mut world = minimal_world(); + spawn_test_npc(&mut world, 1); + + let path = temp_path(); + save_to_file(&path, &mut world).expect("save"); + load_from_file(&path, &mut world).expect("load"); + + let has_background: bool = { + let mut q = world.query_filtered::, With)>(); + q.iter(&world).count() > 0 + }; + assert!( + has_background, + "loaded NPC should be in BackgroundSim tier" + ); + + let _ = std::fs::remove_file(&path); + } + + // ----------------------------------------------------------------------- + // execute_save_load + // ----------------------------------------------------------------------- + + #[test] + fn execute_save_load_noop_when_no_pending() { + let mut world = minimal_world(); + world.init_resource::(); + world.init_resource::(); + + execute_save_load(&mut world); + + // No result written when no pending command + let buf = world.resource::(); + assert!( + buf.pending_save_result.is_none(), + "no pending_save_result when no command was queued" + ); + } + + #[test] + fn execute_save_load_writes_success_result() { + let mut world = minimal_world(); + world.init_resource::(); + + let path = temp_path(); + world.insert_resource(SaveLoadPending { + pending: Some(SaveLoadCommand::Save { path: path.clone() }), + }); + + execute_save_load(&mut world); + + let buf = world.resource::(); + let result = buf + .pending_save_result + .as_ref() + .expect("result must be written after execute"); + assert!(result.success, "save should succeed"); + assert_eq!(result.kind, "save"); + assert!(result.error.is_none()); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn execute_save_load_writes_error_result_on_bad_path() { + let mut world = minimal_world(); + world.init_resource::(); + + world.insert_resource(SaveLoadPending { + pending: Some(SaveLoadCommand::Save { + path: PathBuf::from("/nonexistent/dir/save.msgpack"), + }), + }); + + execute_save_load(&mut world); + + let buf = world.resource::(); + let result = buf + .pending_save_result + .as_ref() + .expect("result must be written even on failure"); + assert!(!result.success, "save should fail with bad path"); + assert_eq!(result.kind, "save"); + assert!(result.error.is_some(), "error message should be present"); + } + + // ----------------------------------------------------------------------- + // SaveLoadError display + // ----------------------------------------------------------------------- + + #[test] + fn save_load_error_display() { + let e = SaveLoadError::Io("disk full".into()); + assert!(e.to_string().contains("disk full")); + + let e2 = SaveLoadError::VersionMismatch { + expected: 1, + found: 2, + }; + assert!(e2.to_string().contains("expected 1")); + assert!(e2.to_string().contains("found 2")); + } +} diff --git a/server/src/simulation/save_state.rs b/server/src/simulation/save_state.rs index 672cd0681..c9d8a2936 100644 --- a/server/src/simulation/save_state.rs +++ b/server/src/simulation/save_state.rs @@ -2,7 +2,8 @@ //! //! `SaveStateV1` is the versioned serialization envelope for full game state. //! Shares architecture with #96 (state serialization system) — this module -//! defines the data model only; ECS extraction and injection is #257. +//! defines the data model AND the per-NPC serialization primitives for tier +//! eviction freeze/thaw (#96). //! //! ## Write format: MessagePack //! @@ -34,12 +35,22 @@ //! - `NpcMemory` (intentionally excluded — stale inferences would be wrong after //! reload; memory degrades naturally over time so reset-on-load is acceptable) +use bevy_ecs::entity::Entity; +use bevy_ecs::world::World; use serde::{Deserialize, Serialize}; use crate::knowledge::graph::KnowledgeGraph; +use crate::knowledge::registry::StableEntityId; use crate::knowledge::types::StableId; -use crate::npc::{SecretSeverity, Relationships}; +use crate::npc::{ + CombatCapability, Contentment, DailyRoutine, InformationInventory, JobPerformance, Npc, + PersonalityTraits, Relationships, Secret, SecretSeverity, SkillSet, TellSystem, + ToleranceThreshold, Want, WantKind, +}; +use crate::npc::awareness::PlayerAwareness; +use crate::npc::mood::MoodState; use crate::npc::relationships::RelationshipGraph; +use crate::npc::vision::{NpcMemory, NpcVisionState}; use crate::simulation::movement::TilePosition; use crate::simulation::time::TickRate; @@ -76,9 +87,36 @@ pub struct SaveStateV1 { /// Per-NPC state snapshot for `SaveStateV1`. /// -/// Captures the D-024 axis values and position. On load, the full NPC entity -/// is reconstructed by injecting these values into the appropriate components. -/// Field order matches the 10-axis model (D-024) for readability. +/// Two usage contexts: +/// 1. **Whole-game save** (`SaveStateV1.npc_states`): populated by #553 ECS extraction. +/// Only the core axis fields need to be populated for this use case. +/// 2. **Tier eviction freeze** (produced by `serialize_npc_to_frozen`): captures ALL +/// components needed for full NPC reconstruction from `StateSaved` tier. +/// The extended optional fields (#96) carry all 10 D-024 axes. +/// +/// All fields added post-#256 use `#[serde(default)]` for forward compatibility +/// with older save files that predate these fields. +/// +/// ## D-024 axis coverage +/// | Axis | Field | Status | +/// |------|-------|--------| +/// | 1: Want | `want` | Full (optional for backward compat) | +/// | 2: Secret | `secret_severity` (legacy) + `secret` | Full | +/// | 3: Relationships | `relationships` | Full | +/// | 4: Tolerance | `current_stress` + `tolerance_threshold` | Full | +/// | 5: Daily routine | `routine` | Full (optional) | +/// | 6: Information inventory | `information_inventory` | Full (optional) | +/// | 7: Contentment | `contentment` | Full | +/// | Supporting 1: Personality | `personality_traits` | Full (optional) | +/// | Supporting 2: Tells | `tell_system` | Full (optional) | +/// | Supporting 3: Skills | `skill_set` + `combat_capability` | Full (optional) | +/// +/// ## Components intentionally NOT serialized +/// - `NpcVisionState`: runtime LOS state, reset to default on reactivation +/// - `NpcMemory`: stale inferences would be wrong after reload (intentional drop) +/// - `PlayerAwareness`: runtime derived state, reset to default on reactivation +/// - `AnimationTier`: resets to `Tier1` on reactivation (no persistent state) +/// - `RoutineDeviation`: transient event marker, acceptable to drop on reload #[derive(Debug, Clone, Serialize, Deserialize)] pub struct NpcSaveState { /// Stable entity identifier (survives serialization — D-020). @@ -86,7 +124,8 @@ pub struct NpcSaveState { /// Last known tile position. pub position: TilePosition, - // Axis 2: Secret severity (description is regenerated from content on load) + // Axis 2: Secret severity (legacy field — description regenerated from content on load). + // Kept for backward compatibility. Prefer `secret` field when doing full reconstruction. pub secret_severity: SecretSeverity, // Axis 3: Per-NPC relationship slots pub relationships: Option, @@ -99,6 +138,52 @@ pub struct NpcSaveState { /// Per-NPC knowledge graph (if present — Active-tier NPCs carry KG). pub knowledge_graph: Option, + + // --- Full reconstruction fields (added #96, for tier eviction freeze) --- + // All fields below use serde(default) for backward compatibility with saves + // created before #96 shipped. + + /// Axis 1: Want (primary drive, intensity, and description). + #[serde(default)] + pub want: Option, + + /// Axis 2: Full secret (description + known_by list). + /// Supersedes `secret_severity` for full reconstruction. + #[serde(default)] + pub secret: Option, + + /// Axis 5: Daily routine (phase → location schedule). + #[serde(default)] + pub routine: Option, + + /// Axis 6: Information inventory (facts this NPC carries). + #[serde(default)] + pub information_inventory: Option, + + /// Supporting axis 1: Personality traits (2–3 traits, no contradictory pairs). + #[serde(default)] + pub personality_traits: Option, + + /// Supporting axis 2: Tell system (behavioral tells tied to stress/personality). + #[serde(default)] + pub tell_system: Option, + + /// Supporting axis 3: Skill set (proficiency BTreeMap). + #[serde(default)] + pub skill_set: Option, + + /// Optional combat capability (only present for combat-trained NPCs). + #[serde(default)] + pub combat_capability: Option, + + /// Mood state at save time. Derived from stress but worth preserving across + /// tier transitions to avoid jarring state resets on reactivation. + #[serde(default)] + pub mood_state: Option, + + /// Job performance score — drifts over time, persist across tier transitions. + #[serde(default)] + pub job_performance: Option, } impl SaveStateV1 { @@ -113,6 +198,166 @@ impl SaveStateV1 { } } +// --------------------------------------------------------------------------- +// Per-NPC tier eviction serialization primitives (#96) +// --------------------------------------------------------------------------- + +/// Serialize a live NPC entity to a `NpcSaveState` frozen struct. +/// +/// Used by the tier eviction system when demoting an entity to `StateSaved`: +/// instead of keeping all ECS components live, the entity is frozen and despawned. +/// The caller should despawn the entity after calling this function. +/// +/// **Caller invariant:** The entity must have a `StableEntityId` component. +/// All other components are optional — missing components produce sensible defaults +/// in the output (and will be reconstructed as defaults by `deserialize_npc_from_frozen`). +/// +/// # Panics +/// Panics if the entity has no `StableEntityId` component. +pub fn serialize_npc_to_frozen(entity: Entity, world: &World) -> NpcSaveState { + let position = world + .get::(entity) + .copied() + .unwrap_or_else(|| TilePosition::new(0, 0, 0)); + + let stable_id = world + .get::(entity) + .map(|s| s.0) + .expect("NPC entity must have StableEntityId before serialization (#96)"); + + let secret = world.get::(entity).cloned(); + let secret_severity = secret + .as_ref() + .map(|s| s.severity) + .unwrap_or(SecretSeverity::Minor); + + let (current_stress, tolerance_threshold) = world + .get::(entity) + .map(|t| (t.current_stress, t.threshold)) + .unwrap_or((0, 50)); + + NpcSaveState { + stable_id, + position, + secret_severity, + relationships: world.get::(entity).cloned(), + current_stress, + tolerance_threshold, + contentment: world + .get::(entity) + .map(|c| c.level) + .unwrap_or(0), + knowledge_graph: world.get::(entity).cloned(), + want: world.get::(entity).cloned(), + secret, + routine: world.get::(entity).cloned(), + information_inventory: world.get::(entity).cloned(), + personality_traits: world.get::(entity).cloned(), + tell_system: world.get::(entity).cloned(), + skill_set: world.get::(entity).cloned(), + combat_capability: world.get::(entity).cloned(), + mood_state: world.get::(entity).cloned(), + job_performance: world.get::(entity).cloned(), + } +} + +/// Deserialize a frozen `NpcSaveState` and re-spawn a full NPC entity. +/// +/// Used by the tier eviction system when reactivating an entity from `StateSaved`. +/// Reconstructs all D-024 axis components from the frozen state. +/// +/// **Caller responsibilities after calling this function:** +/// 1. Register the returned `Entity` with `EntityRegistry` (StableId→Entity mapping). +/// 2. Assign the appropriate tier marker (`ActiveSim` or `BackgroundSim`). +/// +/// Optional fields that are absent in `state` are reconstructed with sensible defaults: +/// - `Want`: defaults to `Safety` at intensity 5 (conservative non-disruptive default) +/// - `Secret`: reconstructed from `secret_severity` with empty description +/// - `MoodState`, `JobPerformance`: their `Default` implementations +/// +/// Components excluded from reconstruction (see `NpcSaveState` doc for rationale): +/// `NpcVisionState`, `NpcMemory`, `PlayerAwareness` are reset to their `Default` states. +pub fn deserialize_npc_from_frozen(state: &NpcSaveState, world: &mut World) -> Entity { + let want = state.want.clone().unwrap_or(Want { + primary: WantKind::Safety, // conservative fallback — see flag comment above + intensity: 5, + description: String::new(), + }); + + let secret = state.secret.clone().unwrap_or(crate::npc::Secret { + description: String::new(), + severity: state.secret_severity, + known_by: vec![], + }); + + let relationships = state + .relationships + .clone() + .unwrap_or(Relationships { entries: vec![] }); + + let tolerance = ToleranceThreshold { + current_stress: state.current_stress, + threshold: state.tolerance_threshold, + }; + + let contentment = Contentment { + level: state.contentment, + }; + + let kg = state + .knowledge_graph + .clone() + .unwrap_or_else(KnowledgeGraph::new); + + // Spawn the entity with all required components. Tier marker (ActiveSim / + // BackgroundSim) is NOT added here — the caller assigns it after registration. + let entity = world + .spawn(( + Npc, + state.position, + StableEntityId(state.stable_id), + want, + secret, + relationships, + tolerance, + contentment, + kg, + state.mood_state.clone().unwrap_or_default(), + state.job_performance.clone().unwrap_or_default(), + // Runtime-computed components: reset to default on reactivation. + NpcVisionState::default(), + NpcMemory::default(), + PlayerAwareness::default(), + )) + .id(); + + // Optional axis components — insert only if present in frozen state. + { + let mut em = world.entity_mut(entity); + + if let Some(routine) = state.routine.clone() { + em.insert(routine); + } + if let Some(inventory) = state.information_inventory.clone() { + em.insert(inventory); + } + if let Some(traits) = state.personality_traits.clone() { + em.insert(traits); + } + if let Some(tells) = state.tell_system.clone() { + em.insert(tells); + } + if let Some(skills) = state.skill_set.clone() { + em.insert(skills); + } + if let Some(combat) = state.combat_capability.clone() { + em.insert(combat); + } + } + + entity +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -121,6 +366,7 @@ impl SaveStateV1 { mod tests { use super::*; use crate::knowledge::graph::KnowledgeGraph; + use crate::knowledge::registry::StableEntityId; use crate::knowledge::types::{FactId, FactKnowledge, KnowledgeConfidence, StableId}; use crate::npc::relationships::RelationshipGraph; use crate::simulation::movement::TilePosition; @@ -193,6 +439,16 @@ mod tests { tolerance_threshold: 80, contentment: -15, knowledge_graph: None, + want: None, + secret: None, + routine: None, + information_inventory: None, + personality_traits: None, + tell_system: None, + skill_set: None, + combat_capability: None, + mood_state: None, + job_performance: None, }, NpcSaveState { stable_id: StableId(202), @@ -203,6 +459,16 @@ mod tests { tolerance_threshold: 60, contentment: 30, knowledge_graph: None, + want: None, + secret: None, + routine: None, + information_inventory: None, + personality_traits: None, + tell_system: None, + skill_set: None, + combat_capability: None, + mood_state: None, + job_performance: None, }, ]; @@ -301,6 +567,16 @@ mod tests { tolerance_threshold: 50, contentment: 0, knowledge_graph: Some(npc_kg), + want: None, + secret: None, + routine: None, + information_inventory: None, + personality_traits: None, + tell_system: None, + skill_set: None, + combat_capability: None, + mood_state: None, + job_performance: None, }]; let bytes = state.to_bytes().expect("serialize"); @@ -317,4 +593,202 @@ mod tests { // Document the version explicitly so CI catches unintentional bumps. assert_eq!(SAVE_FORMAT_VERSION, 1); } + + // ----------------------------------------------------------------------- + // Tier eviction serialization primitives (#96) + // ----------------------------------------------------------------------- + + fn spawn_minimal_npc(world: &mut World, stable_id: StableId) -> Entity { + use crate::npc::{ + Contentment, Relationships, Secret, SecretSeverity, ToleranceThreshold, Want, WantKind, + }; + use crate::npc::mood::MoodState; + use crate::npc::vision::{NpcMemory, NpcVisionState}; + use crate::npc::awareness::PlayerAwareness; + + world + .spawn(( + Npc, + StableEntityId(stable_id), + TilePosition::new(5, 10, 0), + Want { + primary: WantKind::Safety, + intensity: 7, + description: "wants safety".into(), + }, + Secret { + description: "has a minor secret".into(), + severity: SecretSeverity::Minor, + known_by: vec![], + }, + Relationships { entries: vec![] }, + ToleranceThreshold { + current_stress: 30, + threshold: 70, + }, + Contentment { level: 15 }, + MoodState::default(), + crate::npc::JobPerformance::default(), + KnowledgeGraph::new(), + NpcVisionState::default(), + NpcMemory::default(), + PlayerAwareness::default(), + )) + .id() + } + + #[test] + fn serialize_npc_to_frozen_captures_stable_id_and_position() { + let mut world = World::new(); + let entity = spawn_minimal_npc(&mut world, StableId(42)); + + let frozen = serialize_npc_to_frozen(entity, &world); + + assert_eq!(frozen.stable_id, StableId(42)); + assert_eq!(frozen.position, TilePosition::new(5, 10, 0)); + } + + #[test] + fn serialize_npc_to_frozen_captures_axes() { + use crate::npc::SecretSeverity; + let mut world = World::new(); + let entity = spawn_minimal_npc(&mut world, StableId(1)); + + let frozen = serialize_npc_to_frozen(entity, &world); + + assert_eq!(frozen.secret_severity, SecretSeverity::Minor); + assert_eq!(frozen.current_stress, 30); + assert_eq!(frozen.tolerance_threshold, 70); + assert_eq!(frozen.contentment, 15); + + // Full optional axes should be populated when components exist + assert!(frozen.want.is_some(), "want should be captured"); + assert!(frozen.secret.is_some(), "secret should be captured"); + } + + #[test] + fn serialize_deserialize_roundtrip_produces_identical_component_values() { + // Spec (#96): serialize + deserialize produces an entity with identical values. + let mut world = World::new(); + let original = spawn_minimal_npc(&mut world, StableId(77)); + + // Serialize + let frozen = serialize_npc_to_frozen(original, &world); + + // Deserialize into a new entity + let restored = deserialize_npc_from_frozen(&frozen, &mut world); + + // Verify StableEntityId matches + let orig_stable = world.get::(original).unwrap().0; + let rest_stable = world.get::(restored).unwrap().0; + assert_eq!(orig_stable, rest_stable, "StableId must match"); + + // Position + let orig_pos = world.get::(original).copied().unwrap(); + let rest_pos = world.get::(restored).copied().unwrap(); + assert_eq!(orig_pos, rest_pos, "position must match"); + + // Tolerance + let orig_tol = world.get::(original).cloned().unwrap(); + let rest_tol = world.get::(restored).cloned().unwrap(); + assert_eq!(orig_tol.current_stress, rest_tol.current_stress); + assert_eq!(orig_tol.threshold, rest_tol.threshold); + + // Contentment + let orig_con = world.get::(original).cloned().unwrap(); + let rest_con = world.get::(restored).cloned().unwrap(); + assert_eq!(orig_con.level, rest_con.level, "contentment must match"); + + // Want + let orig_want = world.get::(original).cloned().unwrap(); + let rest_want = world.get::(restored).cloned().unwrap(); + assert_eq!(orig_want.primary, rest_want.primary, "want.primary must match"); + assert_eq!(orig_want.intensity, rest_want.intensity, "want.intensity must match"); + + // Secret severity + let orig_secret = world.get::(original).cloned().unwrap(); + let rest_secret = world.get::(restored).cloned().unwrap(); + assert_eq!(orig_secret.severity, rest_secret.severity, "secret severity must match"); + } + + #[test] + fn deserialize_npc_without_optional_axes_uses_safe_defaults() { + // Spec (#96): optional fields absent in frozen state produce sensible defaults. + use crate::npc::SecretSeverity; + let frozen = NpcSaveState { + stable_id: StableId(999), + position: TilePosition::new(0, 0, 0), + secret_severity: SecretSeverity::Moderate, + relationships: None, + current_stress: 10, + tolerance_threshold: 50, + contentment: 0, + knowledge_graph: None, + want: None, + secret: None, + routine: None, + information_inventory: None, + personality_traits: None, + tell_system: None, + skill_set: None, + combat_capability: None, + mood_state: None, + job_performance: None, + }; + + let mut world = World::new(); + let entity = deserialize_npc_from_frozen(&frozen, &mut world); + + // Entity must exist with required components + assert!(world.get::(entity).is_some()); + assert!(world.get::(entity).is_some()); + assert!(world.get::(entity).is_some()); + assert!(world.get::(entity).is_some()); + assert!(world.get::(entity).is_some(), "Want defaults to Safety"); + assert!(world.get::(entity).is_some(), "Secret built from secret_severity"); + + // Secret severity must be preserved from the legacy field + let secret = world.get::(entity).unwrap(); + assert_eq!(secret.severity, SecretSeverity::Moderate); + + // Optional axes absent in frozen state → not inserted or use defaults + assert!(world.get::(entity).is_none(), "routine absent when not frozen"); + } + + #[test] + fn frozen_npc_roundtrips_via_messagepack() { + // Spec (#96): NpcSaveState must survive MessagePack roundtrip. + let mut world = World::new(); + let entity = spawn_minimal_npc(&mut world, StableId(55)); + let frozen = serialize_npc_to_frozen(entity, &world); + + // Wrap in SaveStateV1 for MessagePack encoding + let save = SaveStateV1 { + format_version: SAVE_FORMAT_VERSION, + tick: 100, + tick_rate: TickRate::Full, + seed: 12, + player_knowledge: KnowledgeGraph::new(), + relationship_graph: RelationshipGraph::new(), + npc_states: vec![frozen], + }; + + let bytes = save.to_bytes().expect("serialize"); + let recovered = SaveStateV1::from_bytes(&bytes).expect("deserialize"); + let bytes2 = recovered.to_bytes().expect("re-serialize"); + assert_eq!(bytes, bytes2, "frozen NPC state must roundtrip via MessagePack"); + } + + #[test] + fn serialize_npc_panics_without_stable_entity_id() { + // Spec (#96): StableEntityId is required — missing it is a programmer error. + let mut world = World::new(); + let entity = world.spawn((Npc, TilePosition::new(0, 0, 0))).id(); + + // World doesn't implement UnwindSafe — wrap in AssertUnwindSafe. + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + serialize_npc_to_frozen(entity, &world); + })); + assert!(result.is_err(), "must panic without StableEntityId"); + } } diff --git a/server/src/simulation/tier.rs b/server/src/simulation/tier.rs index bcb645bce..3fe5ac4de 100644 --- a/server/src/simulation/tier.rs +++ b/server/src/simulation/tier.rs @@ -1,9 +1,21 @@ // Simulation tier system // Implements D-026: Active/Background/State-saved/Ungenerated tiers // Tier transitions based on player approach distance (#99). +// Scope tag system: NPCs with active scope tags stay pinned to ActiveSim (#98). +// Timestamp-based eviction: LRU eviction when ActiveSim exceeds capacity (#97). + +use std::collections::{BTreeSet, BinaryHeap}; +use std::cmp::Reverse; use bevy_app::prelude::*; use bevy_ecs::prelude::*; +use serde::{Deserialize, Serialize}; + +use crate::knowledge::graph::KnowledgeGraph; +use crate::knowledge::registry::StableEntityId; +use crate::knowledge::types::KnowledgeConfidence; +use crate::npc::{Npc, RelationshipKind}; +use crate::npc::relationships::RelationshipGraph; use crate::simulation::movement::{PlayerCharacter, TilePosition}; // --- Tier radius constants (D-026) --- @@ -38,20 +50,356 @@ pub struct BackgroundSim; #[derive(Component, Debug, Clone, Copy, Default)] pub struct StateSaved; +// --------------------------------------------------------------------------- +// Eviction system (D-026, #97) +// --------------------------------------------------------------------------- + +/// Maximum number of entities in `ActiveSim` before LRU eviction kicks in (D-026). +pub const ACTIVE_SIM_CAPACITY: usize = 80; + +/// Tracks the tick at which the player last interacted with or observed an NPC (#97). +/// Updated by `update_last_interaction_tick` when an NPC is in the player's LOS. +/// Used by `evict_excess_active` as the LRU sort key. +#[derive(Component, Debug, Clone, Copy, Default, Serialize, Deserialize)] +pub struct LastInteractionTick(pub u64); + +/// Tracks current `ActiveSim` entity count vs. capacity (#97, D-026). +/// Updated each tick by `evict_excess_active`. +#[derive(Resource, Debug, Clone)] +pub struct SimSpacePressure { + /// Number of entities currently in `ActiveSim`. + pub active_count: usize, + /// Capacity ceiling. + pub capacity: usize, +} + +impl Default for SimSpacePressure { + fn default() -> Self { + Self { + active_count: 0, + capacity: ACTIVE_SIM_CAPACITY, + } + } +} + +// --------------------------------------------------------------------------- +// Scope tag system (D-026, #98) +// --------------------------------------------------------------------------- + +/// Scope tag kinds: reasons why an NPC stays pinned to `ActiveSim` (D-026). +/// +/// Four variants track distinct reasons for pinning. An NPC may have multiple +/// reasons simultaneously — all are tracked in `ScopeTag`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +pub enum ScopeTagKind { + /// NPC is in the player's immediate neighborhood. + /// Set at session start for NPCs within `ACTIVE_RADIUS`. Managed by + /// `assign_neighborhood_tags_on_start` (deferred: future sprint). + Neighborhood, + /// NPC is involved in an active quest. + /// Reserved for the quest system (deferred: future sprint). + ActiveQuest, + /// NPC has a `Friend` or `Colleague` relationship with the player character. + /// Assigned by `assign_scope_tags` each tick from `RelationshipGraph`. + Colleague, + /// NPC is known to the player with confidence >= `KnowsOf`. + /// Assigned by `assign_scope_tags` each tick from player `KnowledgeGraph`. + KnownContact, +} + +/// Scope tag component: which scope tags currently apply to this NPC (D-026). +/// +/// NPCs carrying at least one scope tag are kept in `ActiveSim` regardless of +/// distance or LRU eviction pressure. `ScopePinned` is the eviction guard; +/// this component is the source of truth. +/// +/// Assignment: +/// - `KnownContact` and `Colleague`: recomputed by `assign_scope_tags` each tick. +/// - `Neighborhood`: set at session start (see `ScopeTagKind::Neighborhood`). +/// - `ActiveQuest`: reserved for future quest system. +#[derive(Component, Debug, Clone, Default, Serialize, Deserialize)] +pub struct ScopeTag { + pub tags: BTreeSet, +} + +impl ScopeTag { + /// Create a `ScopeTag` with a single initial kind. + pub fn with(kind: ScopeTagKind) -> Self { + let mut tags = BTreeSet::new(); + tags.insert(kind); + Self { tags } + } + + /// Add a scope tag kind. + pub fn add(&mut self, kind: ScopeTagKind) { + self.tags.insert(kind); + } + + /// Remove a scope tag kind. + pub fn remove(&mut self, kind: ScopeTagKind) { + self.tags.remove(&kind); + } + + /// True if this NPC carries at least one scope tag. + pub fn is_pinned(&self) -> bool { + !self.tags.is_empty() + } + + /// True if this specific kind is present. + pub fn contains(&self, kind: ScopeTagKind) -> bool { + self.tags.contains(&kind) + } +} + +/// Marker component: this NPC is scope-pinned — the eviction system must skip it. +/// +/// Kept in sync with `ScopeTag` by `sync_scope_pins`. Always use `ScopeTag` +/// as the source of truth; treat `ScopePinned` as a query-optimisation cache. +#[derive(Component, Debug, Clone, Copy, Default)] +pub struct ScopePinned; + /// Plugin registering the tier marker components and the tier transition system. pub struct TierPlugin; impl Plugin for TierPlugin { fn build(&self, app: &mut App) { + app.init_resource::(); + // Tier transition runs after movement so positions are current. app.add_systems( Update, update_tier_markers.after(crate::simulation::movement::validate_movement), ); + // Scope tag assignment runs each tick to keep KnownContact / Colleague current. + // Must run before sync_scope_pins so pins are correct before eviction checks. + // Eviction runs after scope pins are synced (respects ScopePinned). + // LastInteractionTick update runs after visibility geometry. + app.add_systems( + Update, + ( + assign_scope_tags, + sync_scope_pins.after(assign_scope_tags), + update_last_interaction_tick + .after(crate::perception::observer::compute_visibility_geometry), + evict_excess_active + .after(sync_scope_pins) + .after(update_tier_markers), + ), + ); tracing::debug!("TierPlugin initialized"); } } +// --------------------------------------------------------------------------- +// Scope tag systems (D-026, #98) +// --------------------------------------------------------------------------- + +/// System: assign `KnownContact` and `Colleague` scope tags from player epistemics. +/// +/// Runs each tick. Clears and recomputes `KnownContact` and `Colleague` tags for all +/// NPCs based on: +/// - `KnownContact`: player `KnowledgeGraph` has an entry for this NPC with +/// confidence >= `KnowsOf`. +/// - `Colleague`: global `RelationshipGraph` has an edge from the player to this NPC +/// with kind `Friend` or `Colleague`. +/// +/// `Neighborhood` and `ActiveQuest` tags are NOT touched by this system: +/// - `Neighborhood` is set at session start and persists (future sprint). +/// - `ActiveQuest` is reserved for the quest system (future sprint). +/// +/// No-op when there is no `PlayerCharacter` entity. +pub fn assign_scope_tags( + player_query: Query<(&KnowledgeGraph, &StableEntityId), With>, + rel_graph: Res, + mut npcs: Query<(Entity, &StableEntityId, Option<&mut ScopeTag>), With>, + mut commands: Commands, +) { + let Ok((player_kg, player_stable)) = player_query.single() else { + return; + }; + let player_id = player_stable.0; + + // Collect KnownContact set: entities in player KG with confidence >= KnowsOf. + // BTreeSet for deterministic iteration (D-010). + let known_contacts: BTreeSet<_> = player_kg + .entities + .iter() + .filter(|(_, ek)| ek.confidence >= KnowledgeConfidence::KnowsOf) + .map(|(id, _)| *id) + .collect(); + + // Collect Colleague set: player → NPC relationship edges with Friend/Colleague kind. + let colleagues: BTreeSet<_> = rel_graph + .relationships_of(&player_id) + .into_iter() + .filter(|(_, edge)| { + matches!(edge.kind, RelationshipKind::Friend | RelationshipKind::Colleague) + }) + .map(|(target_id, _)| *target_id) + .collect(); + + for (entity, npc_stable, maybe_scope_tag) in &mut npcs { + let npc_id = npc_stable.0; + let is_known = known_contacts.contains(&npc_id); + let is_colleague = colleagues.contains(&npc_id); + + match maybe_scope_tag { + Some(mut scope_tag) => { + // Remove computed tags, then re-add if still applicable. + scope_tag.remove(ScopeTagKind::KnownContact); + scope_tag.remove(ScopeTagKind::Colleague); + if is_known { + scope_tag.add(ScopeTagKind::KnownContact); + } + if is_colleague { + scope_tag.add(ScopeTagKind::Colleague); + } + } + None if is_known || is_colleague => { + // Create a new ScopeTag component for this NPC. + let mut scope_tag = ScopeTag::default(); + if is_known { + scope_tag.add(ScopeTagKind::KnownContact); + } + if is_colleague { + scope_tag.add(ScopeTagKind::Colleague); + } + commands.entity(entity).insert(scope_tag); + } + None => {} // NPC not known or related — no scope tag needed. + } + } +} + +/// System: keep `ScopePinned` markers in sync with `ScopeTag` components. +/// +/// Runs after `assign_scope_tags`. For each NPC: +/// - `ScopeTag` present and non-empty → add `ScopePinned` (if not already present). +/// - `ScopeTag` absent or empty → remove `ScopePinned` (if present). +/// +/// The eviction system (#97) queries `Without` to skip pinned NPCs. +pub fn sync_scope_pins( + mut commands: Commands, + needs_pin: Query<(Entity, &ScopeTag), Without>, + may_need_unpin: Query<(Entity, Option<&ScopeTag>), With>, +) { + // Add ScopePinned to NPCs that have a non-empty ScopeTag. + for (entity, scope_tag) in &needs_pin { + if scope_tag.is_pinned() { + commands.entity(entity).insert(ScopePinned); + } + } + + // Remove ScopePinned from NPCs whose ScopeTag is absent or empty. + for (entity, maybe_scope_tag) in &may_need_unpin { + let still_pinned = maybe_scope_tag.map(|s| s.is_pinned()).unwrap_or(false); + if !still_pinned { + commands.entity(entity).remove::(); + } + } +} + +// --------------------------------------------------------------------------- +// Eviction systems (D-026, #97) +// --------------------------------------------------------------------------- + +/// System: update `LastInteractionTick` for NPCs visible to the player (#97). +/// +/// Runs after visibility geometry is computed. Any NPC at a visible position +/// (in the player's LOS) gets its `LastInteractionTick` set to the current tick. +/// NPCs without this component get it inserted on first observation. +pub fn update_last_interaction_tick( + time: Res, + vis_geo: Res, + mut npcs_with_tick: Query<(&TilePosition, &mut LastInteractionTick), With>, + npcs_without_tick: Query<(Entity, &TilePosition), (With, Without)>, + mut commands: Commands, +) { + let current_tick = time.tick; + + // Update existing LastInteractionTick for visible NPCs. + for (pos, mut last_tick) in &mut npcs_with_tick { + if pos.z == vis_geo.observer_z + && vis_geo.visible_positions.contains(&(pos.x, pos.y)) + { + last_tick.0 = current_tick; + } + } + + // Insert LastInteractionTick for NPCs that don't have it yet but are visible. + for (entity, pos) in &npcs_without_tick { + if pos.z == vis_geo.observer_z + && vis_geo.visible_positions.contains(&(pos.x, pos.y)) + { + commands.entity(entity).insert(LastInteractionTick(current_tick)); + } + } +} + +/// System: evict excess `ActiveSim` entities when count exceeds capacity (#97). +/// +/// When more than `ACTIVE_SIM_CAPACITY` entities are in `ActiveSim`: +/// 1. Skip all `ScopePinned` entities (they stay Active regardless). +/// 2. Sort remaining by `LastInteractionTick` (oldest first) via min-heap. +/// 3. Demote the oldest N entities to `BackgroundSim` (or `StateSaved` if beyond +/// background radius). +/// +/// Updates `SimSpacePressure` resource with current counts. +pub fn evict_excess_active( + mut commands: Commands, + player_query: Query<&TilePosition, With>, + active_npcs: Query< + (Entity, &TilePosition, Option<&LastInteractionTick>), + (With, With, Without), + >, + active_count_query: Query<(), With>, + mut pressure: ResMut, +) { + let total_active = active_count_query.iter().count(); + pressure.active_count = total_active; + + if total_active <= pressure.capacity { + return; + } + + let excess = total_active - pressure.capacity; + let Ok(player_pos) = player_query.single() else { + return; + }; + + // Min-heap keyed by LastInteractionTick (oldest = smallest = evicted first). + // Entities without LastInteractionTick get tick 0 (most stale). + 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); + heap.push(Reverse((tick, entity, *pos))); + } + + let mut evicted = 0; + while evicted < excess { + let Some(Reverse((_, entity, pos))) = heap.pop() else { + break; + }; + + let dist = tile_distance(player_pos, &pos); + if dist > BACKGROUND_RADIUS { + commands.entity(entity).remove::().insert(StateSaved); + } else { + commands.entity(entity).remove::().insert(BackgroundSim); + } + evicted += 1; + } + + if evicted > 0 { + tracing::debug!( + "evicted {} excess ActiveSim entities (was {}, cap {})", + evicted, + total_active, + pressure.capacity, + ); + } +} + // --- Tier transition system (D-026, #99) --- /// Manhattan tile distance between two positions, returning `u32::MAX` for @@ -432,4 +780,446 @@ mod tests { assert!(world.get::(npc).is_none(), "demoted to Background"); assert!(world.get::(npc).is_some()); } + + // ----------------------------------------------------------------------- + // ScopeTag component tests (#98, D-026) + // ----------------------------------------------------------------------- + + #[test] + fn scope_tag_with_creates_single_kind() { + let tag = ScopeTag::with(ScopeTagKind::KnownContact); + assert!(tag.contains(ScopeTagKind::KnownContact)); + assert!(!tag.contains(ScopeTagKind::Colleague)); + assert!(tag.is_pinned()); + } + + #[test] + fn scope_tag_add_and_remove() { + let mut tag = ScopeTag::default(); + assert!(!tag.is_pinned(), "new ScopeTag is empty"); + + tag.add(ScopeTagKind::Colleague); + assert!(tag.is_pinned()); + assert!(tag.contains(ScopeTagKind::Colleague)); + + tag.add(ScopeTagKind::KnownContact); + assert!(tag.contains(ScopeTagKind::KnownContact)); + + tag.remove(ScopeTagKind::Colleague); + assert!(!tag.contains(ScopeTagKind::Colleague)); + assert!(tag.is_pinned(), "still pinned by KnownContact"); + + tag.remove(ScopeTagKind::KnownContact); + assert!(!tag.is_pinned(), "unpinned when all tags removed"); + } + + #[test] + fn scope_tag_multiple_kinds_coexist() { + let mut tag = ScopeTag::default(); + tag.add(ScopeTagKind::Neighborhood); + tag.add(ScopeTagKind::ActiveQuest); + tag.add(ScopeTagKind::Colleague); + tag.add(ScopeTagKind::KnownContact); + + assert_eq!(tag.tags.len(), 4, "all four kinds present"); + assert!(tag.is_pinned()); + } + + // ----------------------------------------------------------------------- + // sync_scope_pins system tests (#98) + // ----------------------------------------------------------------------- + + fn run_sync_scope_pins(world: &mut World) { + let mut schedule = bevy_ecs::schedule::Schedule::default(); + schedule.add_systems(sync_scope_pins); + schedule.run(world); + } + + #[test] + fn sync_scope_pins_adds_scope_pinned_for_non_empty_tag() { + let mut world = World::new(); + let npc = world + .spawn((Npc, ScopeTag::with(ScopeTagKind::KnownContact))) + .id(); + + run_sync_scope_pins(&mut world); + + assert!( + world.get::(npc).is_some(), + "ScopePinned added for non-empty ScopeTag" + ); + } + + #[test] + fn sync_scope_pins_does_not_add_for_empty_tag() { + let mut world = World::new(); + let npc = world.spawn((Npc, ScopeTag::default())).id(); + + run_sync_scope_pins(&mut world); + + assert!( + world.get::(npc).is_none(), + "ScopePinned must NOT be added for empty ScopeTag" + ); + } + + #[test] + fn sync_scope_pins_removes_scope_pinned_when_tag_emptied() { + let mut world = World::new(); + // Start with ScopePinned already set but ScopeTag now empty. + let npc = world.spawn((Npc, ScopePinned, ScopeTag::default())).id(); + + run_sync_scope_pins(&mut world); + + assert!( + world.get::(npc).is_none(), + "ScopePinned removed when ScopeTag is empty" + ); + } + + #[test] + fn sync_scope_pins_removes_scope_pinned_when_tag_absent() { + let mut world = World::new(); + // NPC has ScopePinned but no ScopeTag component at all. + let npc = world.spawn((Npc, ScopePinned)).id(); + + run_sync_scope_pins(&mut world); + + assert!( + world.get::(npc).is_none(), + "ScopePinned removed when ScopeTag absent" + ); + } + + #[test] + fn sync_scope_pins_keeps_existing_scope_pinned() { + // An NPC that already has ScopePinned AND a non-empty ScopeTag should remain pinned. + let mut world = World::new(); + let npc = world + .spawn((Npc, ScopePinned, ScopeTag::with(ScopeTagKind::Colleague))) + .id(); + + run_sync_scope_pins(&mut world); + + // After sync, the NPC should still have ScopePinned (it was already there + // AND the scope tag is non-empty — so no change needed). + assert!( + world.get::(npc).is_some(), + "ScopePinned preserved for non-empty ScopeTag" + ); + } + + // ----------------------------------------------------------------------- + // assign_scope_tags system tests (#98) + // ----------------------------------------------------------------------- + + fn run_assign_scope_tags(world: &mut World) { + let mut schedule = bevy_ecs::schedule::Schedule::default(); + schedule.add_systems(assign_scope_tags); + schedule.run(world); + } + + #[test] + fn assign_scope_tags_no_op_without_player() { + let mut world = World::new(); + world.init_resource::(); + + // NPC exists but no PlayerCharacter + let npc = world.spawn((Npc, StableEntityId(crate::knowledge::types::StableId(1)))).id(); + + run_assign_scope_tags(&mut world); + + // No ScopeTag should be assigned — no player + assert!(world.get::(npc).is_none()); + } + + #[test] + fn assign_scope_tags_known_contact_from_player_kg() { + use crate::knowledge::types::StableId; + + let mut world = World::new(); + world.init_resource::(); + + let npc_stable = StableId(10); + let player_stable = StableId(1); + + // Set up player with a KnowledgeGraph that knows the NPC at KnowsOf level. + let mut player_kg = KnowledgeGraph::new(); + player_kg.observe_entity(npc_stable, make_pos(5, 5), 0); + + world.spawn(( + PlayerCharacter, + make_pos(0, 0), + player_kg, + StableEntityId(player_stable), + )); + + // Spawn the NPC + let npc = world + .spawn((Npc, make_pos(10, 0), StableEntityId(npc_stable))) + .id(); + + run_assign_scope_tags(&mut world); + + let scope_tag = world.get::(npc).expect("ScopeTag should be assigned"); + assert!( + scope_tag.contains(ScopeTagKind::KnownContact), + "NPC known at KnowsOf level should get KnownContact tag" + ); + } + + #[test] + fn assign_scope_tags_colleague_from_relationship_graph() { + use crate::knowledge::types::StableId; + use crate::npc::relationships::RelationshipEdge; + + let mut world = World::new(); + + let npc_stable = StableId(20); + let player_stable = StableId(1); + + // Player KG is empty — no KnownContact. + let player_kg = KnowledgeGraph::new(); + + world.spawn(( + PlayerCharacter, + make_pos(0, 0), + player_kg, + StableEntityId(player_stable), + )); + + // Set up RelationshipGraph with player → NPC as Friend. + let mut rel_graph = RelationshipGraph::new(); + rel_graph.set_relationship( + player_stable, + npc_stable, + RelationshipEdge { + kind: RelationshipKind::Friend, + trust: 5, + history: vec![], + last_interaction_tick: 0, + }, + ); + world.insert_resource(rel_graph); + + let npc = world + .spawn((Npc, make_pos(0, 5), StableEntityId(npc_stable))) + .id(); + + run_assign_scope_tags(&mut world); + + let scope_tag = world.get::(npc).expect("ScopeTag assigned for colleague"); + assert!( + scope_tag.contains(ScopeTagKind::Colleague), + "Friend relationship should grant Colleague scope tag" + ); + } + + #[test] + fn assign_scope_tags_does_not_affect_unknown_npcs() { + use crate::knowledge::types::StableId; + + let mut world = World::new(); + world.init_resource::(); + + let player_stable = StableId(1); + let player_kg = KnowledgeGraph::new(); // empty — knows nobody + + world.spawn(( + PlayerCharacter, + make_pos(0, 0), + player_kg, + StableEntityId(player_stable), + )); + + // NPC that the player doesn't know + let npc = world + .spawn((Npc, make_pos(10, 0), StableEntityId(StableId(99)))) + .id(); + + run_assign_scope_tags(&mut world); + + assert!( + world.get::(npc).is_none(), + "unknown NPC should not receive ScopeTag" + ); + } + + #[test] + fn scope_pinned_npc_in_query_without_scope_pinned_marker() { + // Verify that ScopePinned is a separate marker and Without + // correctly excludes pinned NPCs from eviction queries. + let mut world = World::new(); + let pinned = world.spawn((Npc, ScopePinned)).id(); + let unpinned = world.spawn(Npc).id(); + + let mut query = world.query_filtered::, Without)>(); + let unpinned_results: Vec = query.iter(&world).collect(); + + assert_eq!(unpinned_results.len(), 1, "only one unpinned NPC"); + assert_eq!(unpinned_results[0], unpinned); + assert!(!unpinned_results.contains(&pinned), "pinned NPC excluded from eviction query"); + } + + // ----------------------------------------------------------------------- + // Eviction system tests (#97, D-026) + // ----------------------------------------------------------------------- + + fn run_evict_excess_active(world: &mut World) { + let mut schedule = bevy_ecs::schedule::Schedule::default(); + schedule.add_systems(evict_excess_active); + schedule.run(world); + } + + #[test] + fn no_eviction_when_under_capacity() { + let mut world = World::new(); + world.insert_resource(SimSpacePressure { + active_count: 0, + capacity: 5, + }); + world.spawn((PlayerCharacter, make_pos(0, 0))); + + // Spawn 3 active NPCs (under cap of 5) + let npc1 = world.spawn((Npc, ActiveSim, make_pos(5, 0), LastInteractionTick(10))).id(); + let npc2 = world.spawn((Npc, ActiveSim, make_pos(6, 0), LastInteractionTick(20))).id(); + let npc3 = world.spawn((Npc, ActiveSim, make_pos(7, 0), LastInteractionTick(30))).id(); + + run_evict_excess_active(&mut world); + + // All should remain Active + assert!(world.get::(npc1).is_some()); + assert!(world.get::(npc2).is_some()); + assert!(world.get::(npc3).is_some()); + } + + #[test] + fn evicts_oldest_when_over_capacity() { + let mut world = World::new(); + world.insert_resource(SimSpacePressure { + active_count: 0, + capacity: 2, + }); + world.spawn((PlayerCharacter, make_pos(0, 0))); + + // 3 NPCs, cap=2 → must evict 1 (the oldest: tick 10) + let oldest = world.spawn((Npc, ActiveSim, make_pos(5, 0), LastInteractionTick(10))).id(); + let mid = world.spawn((Npc, ActiveSim, make_pos(6, 0), LastInteractionTick(20))).id(); + let newest = world.spawn((Npc, ActiveSim, make_pos(7, 0), LastInteractionTick(30))).id(); + + run_evict_excess_active(&mut world); + + assert!(world.get::(oldest).is_none(), "oldest evicted"); + assert!(world.get::(oldest).is_some(), "oldest → Background"); + assert!(world.get::(mid).is_some(), "mid stays Active"); + assert!(world.get::(newest).is_some(), "newest stays Active"); + } + + #[test] + fn eviction_skips_scope_pinned() { + let mut world = World::new(); + world.insert_resource(SimSpacePressure { + active_count: 0, + capacity: 1, + }); + world.spawn((PlayerCharacter, make_pos(0, 0))); + + // 2 NPCs, cap=1. The oldest is ScopePinned → skip it, evict the other. + let pinned = world.spawn(( + Npc, ActiveSim, ScopePinned, + ScopeTag::with(ScopeTagKind::KnownContact), + make_pos(5, 0), LastInteractionTick(5), + )).id(); + let unpinned = world.spawn(( + Npc, ActiveSim, + make_pos(6, 0), LastInteractionTick(20), + )).id(); + + run_evict_excess_active(&mut world); + + assert!(world.get::(pinned).is_some(), "pinned NPC stays Active"); + assert!(world.get::(unpinned).is_none(), "unpinned NPC evicted"); + assert!(world.get::(unpinned).is_some()); + } + + #[test] + fn eviction_demotes_to_state_saved_if_beyond_background_radius() { + let mut world = World::new(); + world.insert_resource(SimSpacePressure { + active_count: 0, + capacity: 1, + }); + world.spawn((PlayerCharacter, make_pos(0, 0))); + + // NPC at distance 200 (beyond BACKGROUND_RADIUS=120) → StateSaved + let far = world.spawn(( + Npc, ActiveSim, + make_pos(200, 0), LastInteractionTick(5), + )).id(); + // NPC at distance 5 (within ACTIVE_RADIUS) → stays + let near = world.spawn(( + Npc, ActiveSim, + make_pos(5, 0), LastInteractionTick(50), + )).id(); + + run_evict_excess_active(&mut world); + + assert!(world.get::(far).is_none(), "far NPC evicted"); + assert!(world.get::(far).is_some(), "far NPC → StateSaved"); + assert!(world.get::(near).is_some(), "near NPC stays Active"); + } + + #[test] + fn eviction_handles_npcs_without_last_interaction_tick() { + let mut world = World::new(); + world.insert_resource(SimSpacePressure { + active_count: 0, + capacity: 1, + }); + world.spawn((PlayerCharacter, make_pos(0, 0))); + + // NPC without LastInteractionTick defaults to tick 0 (most stale) + let no_tick = world.spawn((Npc, ActiveSim, make_pos(5, 0))).id(); + let with_tick = world.spawn(( + Npc, ActiveSim, + make_pos(6, 0), LastInteractionTick(100), + )).id(); + + run_evict_excess_active(&mut world); + + assert!(world.get::(no_tick).is_none(), "no-tick NPC evicted first"); + assert!(world.get::(no_tick).is_some()); + assert!(world.get::(with_tick).is_some(), "with-tick NPC stays"); + } + + #[test] + fn sim_space_pressure_updated_after_eviction() { + let mut world = World::new(); + world.insert_resource(SimSpacePressure { + active_count: 0, + capacity: 2, + }); + world.spawn((PlayerCharacter, make_pos(0, 0))); + + world.spawn((Npc, ActiveSim, make_pos(5, 0), LastInteractionTick(10))); + world.spawn((Npc, ActiveSim, make_pos(6, 0), LastInteractionTick(20))); + world.spawn((Npc, ActiveSim, make_pos(7, 0), LastInteractionTick(30))); + + run_evict_excess_active(&mut world); + + let pressure = world.resource::(); + // active_count is set BEFORE eviction runs (it reads the pre-eviction count). + // The actual count changes via deferred commands, which apply after the system. + assert_eq!(pressure.active_count, 3, "pressure tracks pre-eviction count"); + } + + // ----------------------------------------------------------------------- + // LastInteractionTick component tests (#97) + // ----------------------------------------------------------------------- + + #[test] + fn last_interaction_tick_defaults_to_zero() { + let tick = LastInteractionTick::default(); + assert_eq!(tick.0, 0); + } } diff --git a/server/tests/bridge_ipc.rs b/server/tests/bridge_ipc.rs index ecb056bf8..c84a98273 100644 --- a/server/tests/bridge_ipc.rs +++ b/server/tests/bridge_ipc.rs @@ -66,12 +66,12 @@ fn snapshot_roundtrip_over_unix_socket() { conversation_events: vec![], conversation_ended: vec![], follow_state: None, - examine_result: None, character_pressure: None, rng_seed: None, poi_list: vec![], examine_result: None, player_knowledge: None, + save_result: None, }; bridge diff --git a/server/tests/bridge_tcp.rs b/server/tests/bridge_tcp.rs index 3ea63a72e..f608c3ba7 100644 --- a/server/tests/bridge_tcp.rs +++ b/server/tests/bridge_tcp.rs @@ -52,12 +52,12 @@ fn snapshot_roundtrip_over_tcp() { conversation_events: vec![], conversation_ended: vec![], follow_state: None, - examine_result: None, character_pressure: None, rng_seed: None, poi_list: vec![], examine_result: None, player_knowledge: None, + save_result: None, }; bridge diff --git a/server/tests/gen_fixtures.rs b/server/tests/gen_fixtures.rs index b32a4f1ae..642906ad0 100644 --- a/server/tests/gen_fixtures.rs +++ b/server/tests/gen_fixtures.rs @@ -41,12 +41,12 @@ fn fixture_snapshot(tick: u64, entities: Vec) -> ObserverSnapshot conversation_events: vec![], conversation_ended: vec![], follow_state: None, - examine_result: None, character_pressure: None, rng_seed: None, poi_list: vec![], examine_result: None, player_knowledge: None, + save_result: None, } } @@ -232,12 +232,12 @@ fn generate_msgpack_fixtures() { conversation_events: vec![], conversation_ended: vec![], follow_state: None, - examine_result: None, character_pressure: None, rng_seed: None, poi_list: vec![], examine_result: None, player_knowledge: None, + save_result: None, }; write_fixture( "snapshot_v2_full", diff --git a/server/tests/golden/proof_room_tick_10.json b/server/tests/golden/proof_room_tick_10.json index 95752ac95..fdbc7e163 100644 --- a/server/tests/golden/proof_room_tick_10.json +++ b/server/tests/golden/proof_room_tick_10.json @@ -39,7 +39,6 @@ "z": 0 } ], - "examine_result": null, "follow_state": null, "game_time": { "day": 0, diff --git a/server/tests/information_boundaries.rs b/server/tests/information_boundaries.rs new file mode 100644 index 000000000..1f6d11cb7 --- /dev/null +++ b/server/tests/information_boundaries.rs @@ -0,0 +1,314 @@ +//! Information boundary negative test suite (D-010, D-030, ticket #272). +//! +//! THE core asymmetric information claim: entity X cannot see what entity Y +//! knows, unless the observation system explicitly grants it. +//! +//! These are NEGATIVE tests — they assert that information does NOT cross +//! boundaries. Each test uses `assert!(x.is_none())` or equivalent absence +//! patterns, not just "test passed because nothing happened." +//! +//! ## Test layers (D-030) +//! +//! Layer 1 (pure unit, no ECS): +//! - `player_kg_has_no_passive_npc_leakage` — KG starts empty, stays empty +//! - `save_state_npc_kg_isolation` — per-NPC KG serialization isolation +//! - `snapshot_excludes_entities_outside_los` — FOV geometry excludes far tiles +//! +//! Layer 2 (minimal ECS world, no subprocess): +//! - `background_npc_kg_not_updated_by_active_tier_events` — tier boundary holds +//! +//! Spec references: D-010 (info boundaries), D-026 (tiers), D-030 (testability), +//! D-041 (knowledge graph), Q-029 (save format) + +use bevy_ecs::prelude::*; +use bevy_ecs::schedule::Schedule; + +use settled_reach_server::knowledge::events::{ + process_knowledge_events, KnowledgeEvent, KnowledgeEventQueue, KnowledgeEventType, +}; +use settled_reach_server::knowledge::{ + ContradictionDetectedQueue, EntityRegistry, KnowledgeGraph, +}; +use settled_reach_server::knowledge::types::StableId; +use settled_reach_server::npc::{Npc, SecretSeverity}; +use settled_reach_server::npc::relationships::RelationshipGraph; +use settled_reach_server::perception::query::{NaturalVision, PerceptionQuery}; +use settled_reach_server::simulation::movement::{TilePosition, WalkabilityMap}; +use settled_reach_server::simulation::save_state::{NpcSaveState, SaveStateV1, SAVE_FORMAT_VERSION}; +use settled_reach_server::simulation::tier::{ActiveSim, BackgroundSim}; +use settled_reach_server::simulation::time::TickRate; +use settled_reach_server::bridge::types::FacingDirection; + +// =========================================================================== +// Layer 1 — Pure unit: no ECS world, no subprocess +// =========================================================================== + +/// IB-1 (Layer 1): A fresh KnowledgeGraph contains no entries for any entity. +/// +/// Core claim: player knowledge is never passively populated. The KG starts +/// empty and can only be written by `observe_entity()`, `record_knowledge()`, +/// or knowledge events processed by `process_knowledge_events`. Simply +/// existing in the simulation world does not leak an NPC's existence into +/// the player's knowledge graph. +/// +/// Spec reference: D-010 principle 2 (information boundaries as first-class system) +#[test] +fn player_kg_has_no_passive_npc_leakage() { + let player_kg = KnowledgeGraph::new(); + let npc_id = StableId(42); + + // Negative assertion: a freshly created KG contains no entity references. + assert!( + player_kg.entities.get(&npc_id).is_none(), + "IB-1: fresh KnowledgeGraph must not contain any entity (passive leakage — D-010 principle 2)" + ); + assert!( + player_kg.is_empty(), + "IB-1: KnowledgeGraph::new() must be completely empty" + ); + + // Negative assertion: spawning a bare ECS entity doesn't populate a KG. + // The knowledge graph is a component, not a global shared resource. + let mut world = World::new(); + let player = world + .spawn(KnowledgeGraph::new()) + .id(); + + // Spawn an NPC in the same world — no observation system runs. + let _npc = world.spawn((Npc, TilePosition::new(50, 50, 0))).id(); + + // Player's KG must be empty regardless of NPCs existing nearby. + let kg = world.get::(player).unwrap(); + assert!( + kg.entities.get(&npc_id).is_none(), + "IB-1: spawning an NPC in the world must not passively populate the player's KG" + ); + assert!( + kg.is_empty(), + "IB-1: player KG must stay empty until an observation system explicitly populates it" + ); +} + +/// IB-2 (Layer 1): FOV geometry excludes positions beyond the vision range. +/// +/// The observer snapshot system (compute_observer_snapshot) includes entities +/// by testing whether their tile position is in `VisibilityGeometry.visible_positions`. +/// This test verifies that the FOV computation — the upstream source of that set — +/// correctly excludes positions far from the observer, so no entity outside LOS +/// can ever appear in the snapshot. +/// +/// Spec reference: D-010 principle 2, D-011 (symmetric shadowcasting), D-030 Layer 1 +#[test] +fn snapshot_excludes_entities_outside_los() { + // All-walkable 100×100 map at z=0 — no walls to cast shadows. + let walkability = WalkabilityMap::new(100, 100, 1); + let observer_pos = TilePosition::new(5, 5, 0); + let facing = FacingDirection::North; + + let geometry = NaturalVision.compute_geometry(&observer_pos, facing, &walkability); + + // --- Far entity: 45 tiles away, well outside FOV range (~12 tiles) --- + let far_npc_pos = TilePosition::new(50, 5, 0); + assert!( + !geometry.visible_positions.contains(&(far_npc_pos.x, far_npc_pos.y)), + "IB-2: entity at {:?} (45 tiles from observer) must NOT be in FOV — \ + observer snapshot would exclude this entity (fog of perception, D-010 principle 2)", + far_npc_pos + ); + + // --- Sanity check: the observer's own position is visible --- + assert!( + geometry.visible_positions.contains(&(observer_pos.x, observer_pos.y)), + "IB-2 sanity: observer's own position must always be in the FOV set" + ); + + // --- Additional sanity: an immediately adjacent tile (1 step) is visible --- + let adjacent_pos = TilePosition::new(6, 5, 0); + assert!( + geometry.visible_positions.contains(&(adjacent_pos.x, adjacent_pos.y)), + "IB-2 sanity: tile immediately adjacent to observer must be visible" + ); +} + +/// IB-4 (Layer 1): NPC save states do not bleed each other's KnowledgeGraphs. +/// +/// `SaveStateV1.npc_states` is a flat `Vec`. Each `NpcSaveState` +/// has its own optional `knowledge_graph: Option`. After a +/// serialise → deserialise roundtrip: +/// - NPC_A's `NpcSaveState.knowledge_graph` contains ONLY NPC_A's own KG. +/// - NPC_B's `NpcSaveState.knowledge_graph` is `None` (Background tier, +/// no KG carried) — it must not be overwritten by NPC_A's KG data. +/// +/// Spec reference: D-010 principle 2, D-026 (tier serialization), Q-029 (save format) +#[test] +fn save_state_npc_kg_isolation() { + let npc_a_id = StableId(1); + let npc_b_id = StableId(2); + + // NPC_A (Active tier) carries a KG that has observed NPC_B. + let mut npc_a_kg = KnowledgeGraph::new(); + // NPC_A has observed NPC_B at some position — this puts NPC_B in NPC_A's KG. + let _ = npc_a_kg.observe_entity(npc_b_id, TilePosition::new(10, 10, 0), 5); + + let npc_a_state = NpcSaveState { + stable_id: npc_a_id, + position: TilePosition::new(5, 5, 0), + secret_severity: SecretSeverity::Minor, + relationships: None, + current_stress: 0, + tolerance_threshold: 20, + contentment: 50, + knowledge_graph: Some(npc_a_kg), // Active NPC carries KG + want: None, + secret: None, + routine: None, + information_inventory: None, + personality_traits: None, + tell_system: None, + skill_set: None, + combat_capability: None, + mood_state: None, + job_performance: None, + }; + + // NPC_B (Background tier) does not carry a KG. + let npc_b_state = NpcSaveState { + stable_id: npc_b_id, + position: TilePosition::new(20, 20, 0), + secret_severity: SecretSeverity::Minor, + relationships: None, + current_stress: 0, + tolerance_threshold: 20, + contentment: 50, + knowledge_graph: None, // Background NPC carries no KG + want: None, + secret: None, + routine: None, + information_inventory: None, + personality_traits: None, + tell_system: None, + skill_set: None, + combat_capability: None, + mood_state: None, + job_performance: None, + }; + + let save = SaveStateV1 { + format_version: SAVE_FORMAT_VERSION, + tick: 10, + tick_rate: TickRate::Full, + seed: 42, + player_knowledge: KnowledgeGraph::new(), + relationship_graph: RelationshipGraph::new(), + npc_states: vec![npc_a_state, npc_b_state], + }; + + // Roundtrip: serialize → deserialize. + let bytes = save.to_bytes().expect("IB-4: serialize SaveStateV1"); + let recovered = SaveStateV1::from_bytes(&bytes).expect("IB-4: deserialize SaveStateV1"); + + // --- Negative assertion: NPC_B's state must NOT contain a KnowledgeGraph --- + let npc_b_recovered = recovered + .npc_states + .iter() + .find(|s| s.stable_id == npc_b_id) + .expect("IB-4: NPC_B must be present in recovered npc_states"); + + assert!( + npc_b_recovered.knowledge_graph.is_none(), + "IB-4: NPC_B's recovered state must not contain a KnowledgeGraph — \ + serialization must not bleed NPC_A's KG data into NPC_B's entry (D-010 principle 2)" + ); + + // --- Sanity: NPC_A's state must contain its own KG (not lost in roundtrip) --- + let npc_a_recovered = recovered + .npc_states + .iter() + .find(|s| s.stable_id == npc_a_id) + .expect("IB-4: NPC_A must be present in recovered npc_states"); + + let kg = npc_a_recovered + .knowledge_graph + .as_ref() + .expect("IB-4: NPC_A's KG must survive roundtrip"); + + // NPC_A's KG entry for NPC_B is NPC_A's OBSERVATION DATA (where NPC_A saw NPC_B). + // This is not NPC_B's own KG — it's NPC_A's record of NPC_B's position. + assert!( + kg.entities.get(&npc_b_id).is_some(), + "IB-4 sanity: NPC_A's KG should still contain its observation of NPC_B after roundtrip" + ); +} + +// =========================================================================== +// Layer 2 — Minimal ECS world (no subprocess) +// =========================================================================== + +/// IB-3 (Layer 2): `process_knowledge_events` only modifies the observer entity. +/// +/// Background-tier NPC KnowledgeGraphs must not be modified when Active-tier +/// events are processed. The `process_knowledge_events` system routes events +/// via `event.observer` (an ECS Entity handle) — only the targeted entity's KG +/// is written. This test confirms that a Background-tier NPC, not named in any +/// event's `observer` field, has its KG left completely unchanged. +/// +/// Spec reference: D-010 principle 2, D-026 (tier boundary), D-030 Layer 2 +#[test] +fn background_npc_kg_not_updated_by_active_tier_events() { + let mut world = World::new(); + + // Required resources for process_knowledge_events. + world.init_resource::(); + world.init_resource::(); + world.init_resource::(); + + // Active-tier NPC: will be the observer in the knowledge event. + let active_npc = world + .spawn((Npc, ActiveSim, KnowledgeGraph::new())) + .id(); + + // Background-tier NPC: must NOT be affected. + let background_npc = world + .spawn((Npc, BackgroundSim, KnowledgeGraph::new())) + .id(); + + // A separate "observed" entity (the target of the DirectObservation). + // Register it in the EntityRegistry so process_knowledge_events can resolve its StableId. + let observed_entity = world.spawn_empty().id(); + { + let mut registry = world.resource_mut::(); + registry.register(observed_entity); + } + + // Push a DirectObservation event targeting only the Active NPC as observer. + // The Background NPC is not mentioned anywhere in this event. + world + .resource_mut::() + .push(KnowledgeEvent { + observer: active_npc, + tick: 1, + event_type: KnowledgeEventType::DirectObservation { + target: observed_entity, + position: TilePosition::new(5, 5, 0), + }, + }); + + // Run the knowledge event processing system. + let mut schedule = Schedule::default(); + schedule.add_systems(process_knowledge_events); + schedule.run(&mut world); + + // --- Negative assertion: Background NPC's KG must be completely unchanged --- + let bg_kg = world + .get::(background_npc) + .expect("IB-3: BackgroundSim NPC must still have KnowledgeGraph component"); + + assert!( + bg_kg.is_empty(), + "IB-3: Background-tier NPC KG must not be modified by Active-tier events. \ + process_knowledge_events must only update the event.observer entity (D-026 tier boundary, \ + D-010 principle 2). Found {} entity entries and {} fact entries.", + bg_kg.entity_count(), + bg_kg.fact_count() + ); +} diff --git a/server/tests/integration/mod.rs b/server/tests/integration/mod.rs new file mode 100644 index 000000000..bf630cd42 --- /dev/null +++ b/server/tests/integration/mod.rs @@ -0,0 +1,60 @@ +//! Layer 3 integration test entry point (D-030, ticket #200). +//! +//! ## Three-layer test architecture (D-030 sub-decision 3) +//! +//! ```text +//! Layer 1 — Fixture-based serialization (FAST, run on every edit) +//! Scope: Pure unit tests. No ECS world. No subprocess. +//! Tools: Rust #[test] + data structures directly. +//! Speed: <1ms each. +//! Files: tests/serialization.rs, tests/information_boundaries.rs (Layer 1 tests), +//! #[cfg(test)] mod tests within src/ modules +//! +//! Layer 2 — Mock subprocess / minimal ECS world (MEDIUM, run on every PR) +//! Scope: Minimal bevy App or World. Real systems, no real subprocess. +//! IPC roundtrip over Unix socket without spawning the binary. +//! Tools: bevy_ecs World + Schedule, or LocalBridge with in-process simulation. +//! Speed: 1ms–100ms each. +//! Files: tests/bridge_ipc.rs, tests/bridge_tcp.rs, +//! tests/information_boundaries.rs (Layer 2 tests), +//! tests/determinism.rs, tests/movement.rs, tests/smoke.rs +//! +//! Layer 3 — Real subprocess integration (SLOW, run daily / pre-merge) +//! Scope: Full binary spawned as a child process. No mocks. Real IPC. +//! Exercises the complete path: spawn → handshake → tick → snapshot. +//! Tools: std::process::Command, TcpStream. +//! Speed: 1s–15s each (process startup dominates). +//! Files: tests/layer3.rs, tests/integration/ (this module) +//! ``` +//! +//! ## Layer 3 test guidelines +//! +//! - Always set a deadline for server startup (`LISTEN_TIMEOUT`). +//! - Always kill the child process in teardown (even on test failure — use a +//! RAII guard or drop the handle at end of test). +//! - Use `--port 0` to get a kernel-assigned port; parse `LISTENING:{port}` from +//! stdout to obtain the actual port. +//! - Serialize `PlayerInput` via `rmp_serde`, frame with `bridge::framing::write_framed`. +//! - Deserialize `ObserverSnapshot` via `rmp_serde` after `bridge::framing::read_framed`. +//! +//! Spec reference: D-030 (testability architecture), D-020 (subprocess IPC protocol) + +// --------------------------------------------------------------------------- +// Stub: Layer 3 startup smoke test +// --------------------------------------------------------------------------- + +/// Placeholder for future Layer 3 tests that require full subprocess setup. +/// +/// Non-blocking tests that exercise the simulation binary end-to-end live in +/// `tests/layer3.rs`. This module is the organisational entry point for tests +/// that exercise multi-message Layer 3 scenarios (multi-tick sequences, +/// save/load roundtrip over IPC, protocol version negotiation). +/// +/// See `tests/layer3.rs::server_subprocess_sends_snapshot_on_connect` for the +/// canonical Layer 3 pattern. +#[test] +fn layer3_module_entry_point_placeholder() { + // This test exists to verify the integration module compiles and is + // discovered by cargo test. Real Layer 3 scenario tests replace this. + // D-030 Layer 3 stubs are acceptable until the IPC handshake (#555) lands. +} diff --git a/server/tests/layer3.rs b/server/tests/layer3.rs index 07cc6eaf1..fc8524a1b 100644 --- a/server/tests/layer3.rs +++ b/server/tests/layer3.rs @@ -72,7 +72,19 @@ fn server_subprocess_sends_snapshot_on_connect() { let mut reader = BufReader::new(stream.try_clone().expect("clone stream for reader")); let mut writer = BufWriter::new(stream); - // 4. Send one PlayerInput (idle tick 0) + // 4. Read the protocol handshake (first framed message, #555) + let handshake_frame = read_framed(&mut reader) + .expect("read handshake frame") + .expect("server closed connection before sending handshake"); + let handshake: HandshakeMessage = + rmp_serde::from_slice(&handshake_frame).expect("deserialize HandshakeMessage"); + assert_eq!( + handshake.protocol_version, PROTOCOL_VERSION, + "handshake protocol_version mismatch: got {}, expected {}", + handshake.protocol_version, PROTOCOL_VERSION + ); + + // 5. Send one PlayerInput (idle tick 0) let inputs = vec![PlayerInput { tick: 0, action: PlayerAction::MoveNorth, @@ -80,14 +92,14 @@ fn server_subprocess_sends_snapshot_on_connect() { let payload = rmp_serde::to_vec_named(&inputs).expect("serialize PlayerInput"); write_framed(&mut writer, &payload).expect("send PlayerInput to server"); - // 5. Read one ObserverSnapshot + // 6. Read one ObserverSnapshot let response = read_framed(&mut reader) .expect("read snapshot frame") .expect("server closed connection before sending snapshot"); let snapshot: ObserverSnapshot = rmp_serde::from_slice(&response).expect("deserialize ObserverSnapshot"); - // 6. Assert protocol correctness (D-020) + // 7. Assert protocol correctness (D-020) assert_eq!( snapshot.version, PROTOCOL_VERSION, "protocol version mismatch: got {}, expected {}", @@ -105,7 +117,7 @@ fn server_subprocess_sends_snapshot_on_connect() { .any(|e| matches!(e.kind, EntityKind::Player)); assert!(has_player, "snapshot must contain a Player entity"); - // 7. Clean up: drop connection so the server exits its game loop + // 8. Clean up: drop connection so the server exits its game loop drop(reader); drop(writer); diff --git a/server/tests/serialization.rs b/server/tests/serialization.rs index 91d0c6bf4..57bae1cf4 100644 --- a/server/tests/serialization.rs +++ b/server/tests/serialization.rs @@ -30,12 +30,12 @@ fn test_snapshot(tick: u64, entities: Vec) -> ObserverSnapshot { conversation_events: vec![], conversation_ended: vec![], follow_state: None, - examine_result: None, character_pressure: None, rng_seed: None, poi_list: vec![], examine_result: None, player_knowledge: None, + save_result: None, } } @@ -120,6 +120,12 @@ fn all_player_action_variants_roundtrip() { target_entity_id: 42, response_id: "kael-davan_d_001".to_string(), }, + PlayerAction::SaveGame { + path: "/tmp/test.msgpack".to_string(), + }, + PlayerAction::LoadGame { + path: "/tmp/test.msgpack".to_string(), + }, ]; for action in actions { @@ -280,12 +286,12 @@ fn snapshot_v2_fields_roundtrip() { conversation_events: vec![], conversation_ended: vec![], follow_state: None, - examine_result: None, character_pressure: None, rng_seed: None, poi_list: vec![], examine_result: None, player_knowledge: None, + save_result: None, }; let bytes = rmp_serde::to_vec_named(&snapshot).expect("serialize"); @@ -384,12 +390,12 @@ fn all_facing_direction_variants_roundtrip() { conversation_events: vec![], conversation_ended: vec![], follow_state: None, - examine_result: None, character_pressure: None, rng_seed: None, poi_list: vec![], examine_result: None, player_knowledge: None, + save_result: None, }; let bytes = rmp_serde::to_vec_named(&snapshot).expect("serialize"); let decoded: ObserverSnapshot = rmp_serde::from_slice(&bytes).expect("deserialize"); @@ -1433,8 +1439,8 @@ fn serde_default_fields_fill_in_when_missing_from_wire() { let decoded: ObserverSnapshot = serde_json::from_value(minimal_json).expect("minimal JSON must deserialize"); - // Version matches what was in the wire (13, simulating older server) - assert_eq!(decoded.version, 13); + // Version matches what was in the wire + assert_eq!(decoded.version, 14); assert_eq!(decoded.tick, 42); assert_eq!(decoded.entities.len(), 1); -- 2.54.0 From 740e97b75db632c7a41596db7dc4f0dde9e88628 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 25 Feb 2026 12:13:23 +0100 Subject: [PATCH 2/3] chore(meta): update changelog Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91170718e..578cf79e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ Format based on [Keep a Changelog](https://keepachangelog.com/). ## [Unreleased] ### Added +- Protocol version handshake — `HandshakeMessage` as first IPC frame before tick loop, forward-compatible input handling (#555, D-020) +- State serialization primitives — `serialize_npc_to_frozen`/`deserialize_npc_from_frozen` with full D-024 10-axis coverage for tier eviction freeze/thaw (#96, D-026) +- Scope tag system — `ScopeTagKind` (Neighborhood, ActiveQuest, Colleague, KnownContact), `ScopePinned` marker, automatic assignment from KnowledgeGraph and RelationshipGraph (#98, D-026) +- Timestamp-based eviction — `LastInteractionTick` LRU tracking, `SimSpacePressure` resource, BinaryHeap eviction respecting scope-pinned entities, Active cap 80 (#97, D-026) +- Save/load ECS extraction — `save_to_file`/`load_from_file` via MessagePack, `SaveGame`/`LoadGame` IPC commands, `SaveLoadResultWire` on ObserverSnapshot (#553, D-085) +- Test infrastructure — Layer 3 integration test entry point, three-layer architecture per D-030 (#200) +- Information boundary negative tests — 4 tests proving no passive KG leakage, LOS fog holds, tier boundary holds, per-NPC save isolation (#272, D-010) - Protocol v14 — `poi_list`, `examine_result`, `player_knowledge` ObserverSnapshot wire types with live KG serialization (#151, #174, #264) - Minimap rendering — circular 160px diegetic insert overlay with POI dots (colored by category), border arrows for distant POIs, player-centered fixed-north (#151) - Dialogue UI hardening — confrontation italic voice (D-063), examine result overlay with 5s auto-dismiss and confidence coloring (#174) -- 2.54.0 From 23fbfdbfc537b7728ded510e533176dbc5139410 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 25 Feb 2026 12:32:35 +0100 Subject: [PATCH 3/3] =?UTF-8?q?fix(simulation):=20PR=20#68=20review=20?= =?UTF-8?q?=E2=80=94=20version=20bump,=20tracing=20warns,=20test=20coverag?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bump PROTOCOL_VERSION 14 → 15 for save_result field addition - Add tracing::warn on SaveLoadPending command overwrite (double-tap F5) - Add tracing::warn on KnowledgeGraph::new() fallback during save - Fix misleading WouldBlock comment in tcp.rs - Document SimSpacePressure.active_count pre-eviction timing - Document entity-based eviction tie-breaking non-determinism - Add ScopePinned eviction survival regression test - Regenerate msgpack fixtures for protocol v15 Co-Authored-By: Claude Opus 4.6 --- .../msgpack/snapshot_boundary_tick_0.msgpack | Bin 379 -> 373 bytes .../snapshot_boundary_tick_127.msgpack | Bin 379 -> 373 bytes .../snapshot_boundary_tick_2b31m1.msgpack | Bin 383 -> 377 bytes .../snapshot_boundary_tick_2b32.msgpack | Bin 387 -> 381 bytes .../snapshot_boundary_tick_32767.msgpack | Bin 381 -> 375 bytes .../fixtures/msgpack/snapshot_empty.msgpack | Bin 379 -> 373 bytes .../msgpack/snapshot_multi_entity.msgpack | Bin 784 -> 778 bytes .../fixtures/msgpack/snapshot_one_npc.msgpack | Bin 477 -> 471 bytes .../fixtures/msgpack/snapshot_player.msgpack | Bin 480 -> 474 bytes .../fixtures/msgpack/snapshot_v2_full.msgpack | Bin 644 -> 638 bytes server/src/bridge/tcp.rs | 2 +- server/src/bridge/types.rs | 4 +- server/src/simulation/input.rs | 10 ++ server/src/simulation/save_io.rs | 33 +++++- server/src/simulation/tier.rs | 106 +++++++++++++++++- server/tests/golden/proof_room_tick_10.json | 2 +- server/tests/serialization.rs | 2 +- 17 files changed, 150 insertions(+), 9 deletions(-) diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_0.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_0.msgpack index 148ce3bcd558bbbf951cc30443441e6a809957b2..47a9ea59e8cb8ee53852956860f437296c0785de 100644 GIT binary patch delta 52 zcmey(^p%O{9)rm8vecsD%=|q5jXd6ra+{Jf5{nX(OHzyC3yM;Ui%W}A53DH2&y3H> IEH0S<03jk3XaE2J delta 58 zcmey$^qYz29)rm8vecsD%=|pQjXd6rD(g}!5_2>2QsawKi%WA#4s1%!NGwWBE=etl NF8~S^mlmZS006Bu7$pDz diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_127.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_127.msgpack index 53c6b67504295d83e92e02d3dd0005b0e6263592..ffd3ba965b44e8ee593e53e92e9015cb9f85691c 100644 GIT binary patch delta 52 zcmey(^p%O{9)rm8vecsD%=|q5jXd6ra+{Jf5{nX(OHzyC3yM;Ui%W}A53DH2&y3H> IEH0S<03jk3XaE2J delta 58 zcmey$^qYz29)rm8vecsD%=|pQjXd6rD(g}!5_2>2QsawKi%WA#4s1%!NGwWBE=etl NF8~S^mlmZS006Bu7$pDz diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b31m1.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b31m1.msgpack index 91f76e316921c6f2c06ace082708a12c04b63ab6..d275e542c04d342859503bc0ea49092d49227ece 100644 GIT binary patch delta 52 zcmey*^plC_9)rm8vecsD%=|q5jXeI0a+{Jf5{nX(OHzyC3yM;Ui%W}A53DH2&y3H> IEH0S<03$vXbN~PV delta 58 zcmey#^q-069)rm8vecsD%=|pQjXeI0D(g}!5_2>2QsawKi%WA#4s1%!NGwWBE=etl NF8~S^mlmZS006I<7%>0< diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b32.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b32.msgpack index 3c851d4c8e1cbb0880e5571a5efe514e084e2166..88825035891121739ac9e6ea42a798c9987773bc 100644 GIT binary patch delta 52 zcmZo>{>#L3k3nR4S!z*nW_}+3MxJ0sxlPF#iA9OYC8VBo-wmm!uZO N7XXEdON&wu005RW7rp=h diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_32767.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_32767.msgpack index 304069bcb2e90efaa885cbc3123132cee31db23a..13547db7c220f3b5852b7b5a5b7bc0798e168acc 100644 GIT binary patch delta 52 zcmey%^qq<49)rm8vecsD%=|q5jXb`La+{Jf5{nX(OHzyC3yM;Ui%W}A53DH2&y3H> IEH0S<03t9IZU6uP delta 58 zcmey)^p}a}9)rm8vecsD%=|pQjXb`LD(g}!5_2>2QsawKi%WA#4s1%!NGwWBE=etl NF8~S^mlmZS006FM7%Kn( diff --git a/client/tests/fixtures/msgpack/snapshot_empty.msgpack b/client/tests/fixtures/msgpack/snapshot_empty.msgpack index 148ce3bcd558bbbf951cc30443441e6a809957b2..47a9ea59e8cb8ee53852956860f437296c0785de 100644 GIT binary patch delta 52 zcmey(^p%O{9)rm8vecsD%=|q5jXd6ra+{Jf5{nX(OHzyC3yM;Ui%W}A53DH2&y3H> IEH0S<03jk3XaE2J delta 58 zcmey$^qYz29)rm8vecsD%=|pQjXd6rD(g}!5_2>2QsawKi%WA#4s1%!NGwWBE=etl NF8~S^mlmZS006Bu7$pDz diff --git a/client/tests/fixtures/msgpack/snapshot_multi_entity.msgpack b/client/tests/fixtures/msgpack/snapshot_multi_entity.msgpack index 7b28637f49e1b89fbdb8664041c0fb7331c49986..64ffe56ad347262e4f3a9326ccd1c900979304b9 100644 GIT binary patch delta 52 zcmbQh*2Ttik3nR4S!z*nW_}+3MxF;ua+{Jf5{nX(OHzyC3yM;Ui%W}A53DH2&y3H> IEH0S<01IUmFaQ7m delta 58 zcmeBTo503%k3nR4S!z*nW_}*uMxF;uD(g}!5_2>2QsawKi%WA#4s1%!NGwWBE=etl NF8~S^mlmZS005RK7wrH5 diff --git a/client/tests/fixtures/msgpack/snapshot_one_npc.msgpack b/client/tests/fixtures/msgpack/snapshot_one_npc.msgpack index a139d15d635f7f4ab7d103e3041ea3a2674895e5..6e7cbb7fa741051c66ce7d2d5013ec9b16048ae6 100644 GIT binary patch delta 52 zcmcc1e4Ux+9)rm8vecsD%=|q5jXY}^ { 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" ); } -- 2.54.0