diff --git a/docs/architecture/line-pool-format.md b/docs/architecture/line-pool-format.md index 138ff7b27..d04c110a8 100644 --- a/docs/architecture/line-pool-format.md +++ b/docs/architecture/line-pool-format.md @@ -336,6 +336,19 @@ lines: | `character` | single char | `s` = smuggler, `d` = detective. **Monologue only.** | `s`, `d` | | `sequence` | 3-digit zero-padded | `001`–`999` | `001`, `042` | +**General validation regex** (matches both dialogue and monologue IDs): + +``` +^[a-z0-9-]+_(d|m)(_[a-z])?_\d{3}$ +``` + +- `[a-z0-9-]+` — location slug (kebab-case, at least one character) +- `(d|m)` — pool type: `d` for dialogue, `m` for monologue +- `(_[a-z])?` — optional character segment (monologue only): `_s` or `_d` +- `\d{3}` — three-digit zero-padded sequence number + +Use the pool-specific regexes in [Section 5.2](#52-regex-patterns) for strict per-type validation. This general regex is useful for quick format checks that accept either type. + > **D-035 deviation note:** D-035 specifies `{template}_{d|m|e}_{###}`. This spec refines that to `{location-slug}_{d|m}_{###}` (dialogue) and `{location-slug}_m_{s|d}_{###}` (monologue) for two reasons: (1) location-slug is more precise than template name and matches the directory hierarchy, and (2) monologue IDs include a character segment (`s`/`d`) to ensure uniqueness across the hard character partition (D-032). All existing authored content already uses this refined format. D-035 should be updated to reflect the implemented convention. ### 5.2 Regex patterns diff --git a/server/src/content/hot_reload.rs b/server/src/content/hot_reload.rs index 85ebc0d65..257924499 100644 --- a/server/src/content/hot_reload.rs +++ b/server/src/content/hot_reload.rs @@ -21,20 +21,34 @@ use crate::content::{ContentConfig, ContentStoreResource, LinePoolIndexResource} /// At 10 tps (D-031), 20 ticks = 2 seconds. const CHECK_INTERVAL_TICKS: u64 = 20; +/// Consecutive reload failures before escalating to a warning. +const FAILURE_WARN_THRESHOLD: u32 = 5; + /// Resource tracking content file timestamps for change detection. #[derive(Resource, Debug)] pub struct ContentWatcher { file_timestamps: BTreeMap, ticks_since_check: u64, + /// Consecutive reload failures. Resets on success. + consecutive_failures: u32, } impl ContentWatcher { /// Create a new watcher and perform initial timestamp scan. + /// Returns a watcher with no tracked files if content_root is invalid. pub fn new(content_root: &Path) -> Self { let mut watcher = Self { file_timestamps: BTreeMap::new(), ticks_since_check: 0, + consecutive_failures: 0, }; + if content_root.as_os_str().is_empty() || !content_root.is_dir() { + tracing::warn!( + "ContentWatcher: invalid content root {:?}, hot-reload disabled", + content_root, + ); + return watcher; + } watcher.scan(content_root); watcher } @@ -42,7 +56,7 @@ impl ContentWatcher { /// Scan content directory tree and record all YAML file timestamps. fn scan(&mut self, content_root: &Path) { self.file_timestamps.clear(); - walk_yaml(content_root, &mut self.file_timestamps); + walk_yaml(content_root, &mut self.file_timestamps, 0); tracing::debug!( "ContentWatcher: tracking {} content files", self.file_timestamps.len() @@ -52,7 +66,7 @@ impl ContentWatcher { /// Check for changes and rescan. Returns true if any files changed. fn check_and_rescan(&mut self, content_root: &Path) -> bool { let mut new_timestamps = BTreeMap::new(); - walk_yaml(content_root, &mut new_timestamps); + walk_yaml(content_root, &mut new_timestamps, 0); let changed = new_timestamps != self.file_timestamps; if changed { self.file_timestamps = new_timestamps; @@ -66,15 +80,23 @@ impl ContentWatcher { } } +/// Maximum recursion depth for directory walking (guards against symlink loops). +const MAX_WALK_DEPTH: usize = 100; + /// Recursively walk a directory, recording .yaml file modification timestamps. -fn walk_yaml(dir: &Path, timestamps: &mut BTreeMap) { +/// Stops recursing at MAX_WALK_DEPTH to guard against symlink loops. +fn walk_yaml(dir: &Path, timestamps: &mut BTreeMap, depth: usize) { + if depth >= MAX_WALK_DEPTH { + tracing::warn!("walk_yaml: max depth {} reached at {:?}, stopping", MAX_WALK_DEPTH, dir); + return; + } let Ok(entries) = std::fs::read_dir(dir) else { return; }; for entry in entries.filter_map(|e| e.ok()) { let path = entry.path(); if path.is_dir() { - walk_yaml(&path, timestamps); + walk_yaml(&path, timestamps, depth + 1); } else if path.extension().and_then(|e| e.to_str()) == Some("yaml") { if let Ok(meta) = std::fs::metadata(&path) { if let Ok(modified) = meta.modified() { @@ -124,6 +146,7 @@ pub fn hot_reload_content( let m_count = index.monologue_line_count(); store_res.0 = store; index_res.0 = index; + watcher.consecutive_failures = 0; tracing::info!( "Content hot-reloaded: {} dialogue lines, {} monologue lines", d_count, @@ -131,7 +154,16 @@ pub fn hot_reload_content( ); } Err(e) => { - tracing::warn!("Content hot-reload failed (keeping previous): {}", e); + watcher.consecutive_failures += 1; + if watcher.consecutive_failures >= FAILURE_WARN_THRESHOLD { + tracing::warn!( + "Content hot-reload failed {} consecutive times (keeping previous): {}", + watcher.consecutive_failures, + e, + ); + } else { + tracing::warn!("Content hot-reload failed (keeping previous): {}", e); + } } } } diff --git a/server/src/content/line_pool.rs b/server/src/content/line_pool.rs index eee384e9c..2078ae9b6 100644 --- a/server/src/content/line_pool.rs +++ b/server/src/content/line_pool.rs @@ -349,6 +349,14 @@ impl LinePoolIndex { } fn index_dialogue_pool(&mut self, pool: &types::DialoguePool) { + if pool.location.is_empty() { + tracing::warn!( + "Skipping dialogue pool with empty location (role={})", + pool.role, + ); + return; + } + let key = (pool.location.clone(), pool.role.clone()); let indexed_lines: Vec = @@ -366,6 +374,14 @@ impl LinePoolIndex { } fn index_monologue_pool(&mut self, pool: &types::MonologuePool) { + if pool.location.is_empty() { + tracing::warn!( + "Skipping monologue pool with empty location (character={})", + pool.character, + ); + return; + } + let character = match pool.character.parse::() { Ok(c) => c, Err(e) => { @@ -534,8 +550,28 @@ fn parse_dialogue_line(line: &types::DialogueLine) -> Option = line.topic.iter().filter_map(|s| s.parse().ok()).collect(); - let mood: Vec = line.mood.iter().filter_map(|s| s.parse().ok()).collect(); + let topic: Vec = line + .topic + .iter() + .filter_map(|s| { + s.parse() + .map_err(|e: ParseEnumError| { + tracing::warn!("Line {}: {}", line.id, e); + }) + .ok() + }) + .collect(); + let mood: Vec = line + .mood + .iter() + .filter_map(|s| { + s.parse() + .map_err(|e: ParseEnumError| { + tracing::warn!("Line {}: {}", line.id, e); + }) + .ok() + }) + .collect(); Some(IndexedDialogueLine { id: line.id.clone(), @@ -1009,6 +1045,100 @@ mod tests { assert!(results.is_empty()); } + #[test] + fn query_monologue_line_with_prerequisite_excluded_when_fact_absent() { + // H10: Verify prerequisite field is populated through query so callers + // can filter. query_monologue returns ALL matching lines (prerequisite + // evaluation is caller's responsibility per D-028), but a line with a + // prerequisite should carry that data through for the caller to check. + let mut store = ContentStore::default(); + let monologue_pools = vec![types::MonologuePool { + character: "smuggler".to_string(), + location: "the-terminal".to_string(), + lines: vec![ + // Line WITHOUT prerequisite — should always be available + types::MonologueLine { + id: "prereq_none".to_string(), + text: "No prereq line".to_string(), + trigger: "enter_location".to_string(), + prerequisites: None, + priority: Some(5), + cooldown: None, + tags: vec![], + }, + // Line WITH prerequisite — caller must check before using + types::MonologueLine { + id: "prereq_fact".to_string(), + text: "Requires cargo_manifest_seen".to_string(), + trigger: "enter_location".to_string(), + prerequisites: Some(types::Prerequisites { + facts: vec![types::FactPrerequisite { + fact_id: "cargo_manifest_seen".to_string(), + min_confidence: "confirmed".to_string(), + }], + entity_attributes: vec![], + relationship: None, + }), + priority: Some(8), + cooldown: None, + tags: vec![], + }, + ], + }]; + let district = DistrictContent { + monologue_pools, + ..Default::default() + }; + store + .districts + .insert("test.district".to_string(), district); + + let index = LinePoolIndex::build(&store); + let results = + index.query_monologue(Character::Smuggler, "the-terminal", Trigger::EnterLocation); + + // Both lines are returned (query doesn't filter prerequisites) + assert_eq!(results.len(), 2); + + // Verify the prerequisite-bearing line carries its prerequisites through + let prereq_line = results.iter().find(|l| l.id == "prereq_fact").unwrap(); + assert!( + prereq_line.prerequisites.is_some(), + "prerequisite field should be populated for caller to evaluate" + ); + let prereqs = prereq_line.prerequisites.as_ref().unwrap(); + assert_eq!(prereqs.facts.len(), 1); + assert_eq!(prereqs.facts[0].fact_id, "cargo_manifest_seen"); + + // The no-prerequisite line should have None + let no_prereq_line = results.iter().find(|l| l.id == "prereq_none").unwrap(); + assert!( + no_prereq_line.prerequisites.is_none(), + "line without prerequisites should have None" + ); + + // Simulate caller-side filtering: if fact is absent, exclude the line + let player_known_facts: Vec<&str> = vec![]; // empty — fact not known + let available: Vec<_> = results + .iter() + .filter(|line| { + match &line.prerequisites { + None => true, // no prerequisites = always available + Some(prereqs) => prereqs + .facts + .iter() + .all(|f| player_known_facts.contains(&f.fact_id.as_str())), + } + }) + .collect(); + assert_eq!( + available.len(), + 1, + "only the no-prerequisite line should pass when fact is absent" + ); + assert_eq!(available[0].id, "prereq_none"); + } + // -- Parse edge cases ---------------------------------------------------- #[test] diff --git a/server/src/content/loader.rs b/server/src/content/loader.rs index b381dca48..aba68da37 100644 --- a/server/src/content/loader.rs +++ b/server/src/content/loader.rs @@ -93,8 +93,11 @@ fn discover_districts(campaign_path: &Path) -> Vec { if systems_path.is_dir() { walk_for_districts(&systems_path, &mut districts); } - // BTreeMap ordering guarantees deterministic district processing, - // but sort the discovery order too for consistency. + // Discovery order from fs::read_dir is platform-dependent. Sort the Vec + // here so districts load in a deterministic order regardless of OS. + // ContentStore.districts uses BTreeMap for deterministic *iteration* later, + // but sorted discovery ensures deterministic *load* order (and thus + // deterministic ID derivation and log output). districts.sort(); districts } diff --git a/server/src/npc/relationships.rs b/server/src/npc/relationships.rs index 7ed764ec8..9d4188598 100644 --- a/server/src/npc/relationships.rs +++ b/server/src/npc/relationships.rs @@ -26,11 +26,8 @@ pub struct RelationshipEdge { /// BTreeMap<(subject, target), edge> for deterministic iteration (D-010). /// Directed graph: edge (A, B) represents how A feels about B. /// -/// TODO(v0.2): This is a global omniscient resource — all entities share one -/// graph. This violates information boundaries (D-009/D-010) because any -/// system can read any relationship. For multiplayer, this needs per-observer -/// projection so each entity only sees relationships they should know about. -/// Acceptable for v0.1 single-player where the server is authoritative. +/// TODO(v0.2): RelationshipGraph is per-world. Multiplayer needs per-observer +/// relationship views (D-010). #[derive(Resource, Debug, Clone, Default, Serialize, Deserialize)] pub struct RelationshipGraph { edges: BTreeMap<(StableId, StableId), RelationshipEdge>, diff --git a/server/src/perception/cognitive_delay.rs b/server/src/perception/cognitive_delay.rs index 35977aca2..1fba0c2ae 100644 --- a/server/src/perception/cognitive_delay.rs +++ b/server/src/perception/cognitive_delay.rs @@ -19,10 +19,22 @@ use crate::simulation::time::SimulationTime; /// Base cognitive delay: 0.6 seconds = 6 ticks at 10 tps (D-031, D-060). /// Tunable: expect playtesting adjustments. +/// +/// Playtesting expectation: 0.6s should feel like a brief "processing" beat — +/// noticeable enough that new entities register as grey blobs before resolving, +/// but short enough to not feel sluggish. If playtesters report recognition +/// feels instant (reduce to test), or laggy (current value may be too high for +/// fast-paced encounters), adjust in 2-tick increments. The 2:1 ratio with +/// URGENT_DELAY_TICKS should be preserved. pub const NORMAL_DELAY_TICKS: u64 = 6; /// Urgent cognitive delay: 0.3 seconds = 3 ticks at 10 tps (D-031, D-060). /// Triggered when observe_anomaly context is active. +/// +/// Playtesting expectation: 0.3s should feel nearly instant but still register +/// visually as a "snap to attention" moment. If playtesters don't notice the +/// delay at all, consider whether the grey blob phase is too brief to read. +/// Must remain strictly less than NORMAL_DELAY_TICKS. pub const URGENT_DELAY_TICKS: u64 = 3; /// How the recognition was triggered, determines delay duration. @@ -76,6 +88,8 @@ impl CognitiveDelay { } /// Check if an entity is already pending recognition. + // Note: O(n) scan over pending vec. Fine for v0.1 (typically <10 pending). + // If NPC cognitive delay is added, consider HashSet index. pub fn is_pending(&self, stable_id: &StableId) -> bool { self.pending.iter().any(|p| p.stable_id == *stable_id) } diff --git a/server/src/perception/observation.rs b/server/src/perception/observation.rs index a2bdd9902..5f29c3394 100644 --- a/server/src/perception/observation.rs +++ b/server/src/perception/observation.rs @@ -78,7 +78,7 @@ pub fn emit_observation_events( } else if let Some(ref mut delay) = cognitive_delay { // New entity + cognitive delay available: buffer recognition if !delay.is_pending(&stable_id) { - // TODO: wire RecognitionTrigger::Urgent for observe_anomaly triggers + // TODO(#450): wire RecognitionTrigger::Urgent for observe_anomaly triggers let trigger = RecognitionTrigger::Normal; delay.push(PendingRecognition { target: entity, @@ -135,8 +135,9 @@ pub fn emit_observation_events( delay.pending().iter().map(|p| p.stable_id).collect(); for sid in pending_ids { if !visible_stable_ids.contains(&sid.0) { - delay.cancel(&sid); - tracing::debug!("Cognitive delay cancelled: stable_id={} left LOS", sid.0); + if delay.cancel(&sid).is_some() { + tracing::debug!("Cognitive delay cancelled: stable_id={} left LOS", sid.0); + } } } } diff --git a/server/src/perception/observer/tests.rs b/server/src/perception/observer/tests.rs index 6d6ddcea7..4e594377e 100644 --- a/server/src/perception/observer/tests.rs +++ b/server/src/perception/observer/tests.rs @@ -1817,3 +1817,101 @@ fn sprint_anomaly_multiple_contradicted_npcs_only_first_queued() { let queue = query.single(&world).unwrap(); assert!(queue.has_pending(), "one anomaly should be queued"); } + +// ----------------------------------------------------------------------- +// Pending recognitions in observer snapshot (#423, D-060) +// ----------------------------------------------------------------------- + +#[test] +fn pending_recognitions_appear_in_snapshot() { + // H11: When a player entity has a CognitiveDelay component with pending + // recognitions, compute_observer_snapshot should include them in + // pending_recognitions for the client to render as grey blobs. + use crate::perception::cognitive_delay::{ + CognitiveDelay, PendingRecognition, RecognitionTrigger, + }; + + let mut world = setup_world(32, 32); + let mut registry = EntityRegistry::new(0); + + // Target entity that is being "recognized" + let target = world.spawn_empty().id(); + let target_sid = registry.register(target); + + // Player with CognitiveDelay containing a pending recognition + let mut cd = CognitiveDelay::default(); + cd.push(PendingRecognition { + target, + stable_id: target_sid, + position: TilePosition::new(16, 14, 0), + delay_until_tick: 110, // will complete at tick 110 + trigger: RecognitionTrigger::Normal, + }); + + let player = world + .spawn(( + PlayerCharacter, + TilePosition::new(16, 16, 0), + Facing::default(), + KnowledgeGraph::new(), + NearbyInteractionBuffer::default(), + MonologueBuffer::default(), + cd, + )) + .id(); + registry.register(player); + world.insert_resource(registry); + world.insert_resource({ + let mut t = SimulationTime::default(); + t.tick = 106; // 4 ticks remaining until recognition + t + }); + + run_observer_pipeline(&mut world); + + let buffer = world.resource::(); + let snapshot = buffer.snapshot.as_ref().expect("snapshot should exist"); + + assert_eq!( + snapshot.pending_recognitions.len(), + 1, + "should have one pending recognition in snapshot" + ); + let pending = &snapshot.pending_recognitions[0]; + assert_eq!(pending.entity_id, target_sid.0); + assert_eq!(pending.remaining_ticks, 4, "110 - 106 = 4 remaining"); + assert_eq!( + pending.total_delay_ticks, + crate::perception::cognitive_delay::NORMAL_DELAY_TICKS, + "total delay should match Normal trigger" + ); + // Position should be render coords of (16, 14, 0) + let (expected_x, expected_y, expected_z) = TilePosition::new(16, 14, 0).to_render_coords(); + assert_eq!(pending.x, expected_x); + assert_eq!(pending.y, expected_y); + assert_eq!(pending.z, expected_z); +} + +#[test] +fn no_cognitive_delay_component_means_empty_pending_recognitions() { + // H11 complement: player WITHOUT CognitiveDelay should produce + // an empty pending_recognitions vec (backward compatibility). + let mut world = setup_world(32, 32); + world.spawn(( + PlayerCharacter, + TilePosition::new(16, 16, 0), + Facing::default(), + KnowledgeGraph::new(), + NearbyInteractionBuffer::default(), + MonologueBuffer::default(), + )); + + run_observer_pipeline(&mut world); + + let buffer = world.resource::(); + let snapshot = buffer.snapshot.as_ref().unwrap(); + assert!( + snapshot.pending_recognitions.is_empty(), + "no CognitiveDelay component should produce empty pending_recognitions" + ); +} diff --git a/server/src/simulation/input.rs b/server/src/simulation/input.rs index 65542deb5..fb8a34fac 100644 --- a/server/src/simulation/input.rs +++ b/server/src/simulation/input.rs @@ -14,6 +14,10 @@ use crate::simulation::time::{SimulationTime, TickRate}; use bevy_ecs::prelude::*; use std::collections::VecDeque; +/// Maximum number of inputs the queue will hold before dropping oldest. +/// Prevents unbounded memory growth from input flooding. +pub const INPUT_QUEUE_CAPACITY: usize = 1000; + /// Queue of pending player inputs, ordered by tick #[derive(Resource, Debug, Default)] pub struct InputQueue { @@ -24,6 +28,7 @@ impl InputQueue { /// Add a new input to the queue. /// Inputs must be pushed in tick order for deterministic processing. /// Panics in debug builds if tick ordering is violated. + /// Drops oldest inputs when capacity is exceeded. pub fn push(&mut self, input: PlayerInput) { debug_assert!( self.queue.back().is_none_or(|last| last.tick <= input.tick), @@ -31,6 +36,14 @@ impl InputQueue { self.queue.back().map_or(0, |last| last.tick), input.tick, ); + if self.queue.len() >= INPUT_QUEUE_CAPACITY { + let dropped = self.queue.pop_front(); + tracing::warn!( + "InputQueue at capacity ({}), dropping oldest input (tick={})", + INPUT_QUEUE_CAPACITY, + dropped.map_or(0, |d| d.tick), + ); + } self.queue.push_back(input); } diff --git a/server/src/simulation/listening.rs b/server/src/simulation/listening.rs index 458293246..a1105eb07 100644 --- a/server/src/simulation/listening.rs +++ b/server/src/simulation/listening.rs @@ -91,6 +91,13 @@ pub fn update_listening_focus( // Sprint stance: stationary but too high-alert to listen let Some(threshold) = ListeningFocus::threshold_for_stance(stance) else { + if focus.eavesdrop_target.is_some() || focus.stationary_ticks > 0 { + tracing::debug!( + "Sprint stance resets eavesdrop: stationary_ticks={}, had_target={}", + focus.stationary_ticks, + focus.eavesdrop_target.is_some(), + ); + } focus.stationary_ticks = 0; focus.eavesdrop_target = None; continue; @@ -426,6 +433,23 @@ mod tests { assert_eq!(focus.stationary_ticks, EAVESDROP_THRESHOLD); } + // ----------------------------------------------------------------------- + // Constant invariants + // ----------------------------------------------------------------------- + + #[test] + fn eavesdrop_threshold_careful_less_than_normal() { + // T4: The careful threshold MUST be strictly less than the normal + // threshold — careful stance rewards patience with faster eavesdrop + // activation (D-053, D-018). + assert!( + EAVESDROP_THRESHOLD_CAREFUL < EAVESDROP_THRESHOLD, + "EAVESDROP_THRESHOLD_CAREFUL ({}) must be < EAVESDROP_THRESHOLD ({})", + EAVESDROP_THRESHOLD_CAREFUL, + EAVESDROP_THRESHOLD, + ); + } + // ----------------------------------------------------------------------- // Edge cases // ----------------------------------------------------------------------- diff --git a/server/tests/serialization.rs b/server/tests/serialization.rs index e91583325..5f08b8435 100644 --- a/server/tests/serialization.rs +++ b/server/tests/serialization.rs @@ -480,6 +480,43 @@ fn snapshot_v6_full_inventory_roundtrip() { assert_eq!(decoded.player_inventory[8].slot, 8); } +/// PendingRecognitionWire round-trips through MessagePack (#423, D-060). +/// Guards against cognitive delay wire data corruption during serialization. +#[test] +fn pending_recognition_wire_roundtrip() { + let mut snapshot = test_snapshot(0, vec![]); + snapshot.pending_recognitions = vec![ + PendingRecognitionWire { + entity_id: 42, + x: 10.5, + y: 20.0, + z: 0, + remaining_ticks: 4, + total_delay_ticks: 6, + }, + PendingRecognitionWire { + entity_id: 99, + x: 15.0, + y: 8.5, + z: 1, + remaining_ticks: 1, + total_delay_ticks: 3, + }, + ]; + + let bytes = rmp_serde::to_vec_named(&snapshot).expect("serialize"); + let decoded: ObserverSnapshot = rmp_serde::from_slice(&bytes).expect("deserialize"); + + assert_eq!(decoded.pending_recognitions.len(), 2); + assert_eq!(decoded.pending_recognitions[0].entity_id, 42); + assert_eq!(decoded.pending_recognitions[0].remaining_ticks, 4); + assert_eq!(decoded.pending_recognitions[0].total_delay_ticks, 6); + assert!((decoded.pending_recognitions[0].x - 10.5).abs() < f32::EPSILON); + assert_eq!(decoded.pending_recognitions[1].entity_id, 99); + assert_eq!(decoded.pending_recognitions[1].z, 1); + assert_eq!(decoded.pending_recognitions[1].total_delay_ticks, 3); +} + /// All VerbKind variants must survive MessagePack round-trip (#421, D-057). /// Guards against serde mapping breakage when new verbs are added. #[test]