From 03c44bebaf6461a5fc22ff6f0cc9e175c0997863 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Thu, 19 Feb 2026 15:08:32 +0100 Subject: [PATCH] =?UTF-8?q?fix(simulation):=20address=20PR=20#42=20review?= =?UTF-8?q?=20=E2=80=94=208=20items=20from=20Hoshe=20and=20Tyre?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Sound producers: document empty v0.1 pipeline explicitly (critical) 2. Routine tests: add ActiveSim to 3 tests that passed trivially 3. Rename _observer_pos → observer_pos (used at line 191) 4. Add FactionOnly positive test case (matching faction_id) 5. Fix stale doc comment "Current: 9" → 10 in ObserverSnapshot 6. Remove orphaned SimulationTier/LastInteraction/ScopeTag types 7. Add tracing::warn on FactionOnly non-numeric parse failure 8. Document Medium-range occlusion gap as TODO in audible_at 9. Insert SoundEventQueue in observer test setup_world Co-Authored-By: Claude Opus 4.6 --- server/src/bridge/types.rs | 2 +- server/src/knowledge/graph.rs | 35 +++++++- server/src/npc/routine.rs | 3 + server/src/perception/observer/mod.rs | 4 +- server/src/perception/observer/tests.rs | 1 + server/src/simulation/sound.rs | 11 ++- server/src/simulation/tier.rs | 110 ------------------------ 7 files changed, 51 insertions(+), 115 deletions(-) diff --git a/server/src/bridge/types.rs b/server/src/bridge/types.rs index 8bef80d07..4f83b46e4 100644 --- a/server/src/bridge/types.rs +++ b/server/src/bridge/types.rs @@ -31,7 +31,7 @@ pub const PROTOCOL_VERSION: u8 = 10; /// v10 adds: sound_events (#124, D-038 server sound event pipeline). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ObserverSnapshot { - /// Protocol version for forward compatibility. Current: 9. + /// Protocol version for forward compatibility. Current: 10. pub version: u8, /// Simulation tick when this snapshot was produced pub tick: u64, diff --git a/server/src/knowledge/graph.rs b/server/src/knowledge/graph.rs index 9ffc52464..77e725155 100644 --- a/server/src/knowledge/graph.rs +++ b/server/src/knowledge/graph.rs @@ -268,7 +268,17 @@ pub fn filter_by_access( .entities .get(&target_id) .and_then(|k| k.known_attributes.get("faction_id")) - .and_then(|v| v.parse::().ok()) + .and_then(|v| match v.parse::() { + Ok(id) => Some(id), + Err(_) => { + tracing::warn!( + target_id = target_id.0, + value = %v, + "FactionOnly: non-numeric faction_id attribute, denying access" + ); + None + } + }) .is_some_and(|id| id == faction_id.0), // RelationshipGated: observer must have a relationship score >= threshold. @@ -713,4 +723,27 @@ mod tests { "FactionOnly must block when faction attribute is not known" ); } + + #[test] + fn filter_by_access_faction_only_passes_with_matching_faction() { + let observer = StableId(1); + let target = StableId(2); + let mut kg = KnowledgeGraph::new(); + + // Observer knows target's faction via known_attributes + kg.observe_entity(target, make_position(5, 5), 10); + kg.entities + .get_mut(&target) + .unwrap() + .known_attributes + .insert("faction_id".into(), "99".into()); + + let faction = StableId(99); + let rule = ObserverAccess::FactionOnly(faction); + + assert!( + filter_by_access(observer, target, &rule, &kg), + "FactionOnly must pass when observer knows the matching faction_id" + ); + } } diff --git a/server/src/npc/routine.rs b/server/src/npc/routine.rs index 86c24b35d..4a97e8adf 100644 --- a/server/src/npc/routine.rs +++ b/server/src/npc/routine.rs @@ -125,6 +125,7 @@ mod tests { let entity = world .spawn(( Npc, + ActiveSim, TilePosition::new(5, 5, 0), DailyRoutine { entries: vec![RoutineEntry { @@ -153,6 +154,7 @@ mod tests { let entity = world .spawn(( Npc, + ActiveSim, loc, // Already at afternoon location DailyRoutine { entries: vec![RoutineEntry { @@ -181,6 +183,7 @@ mod tests { let entity = world .spawn(( Npc, + ActiveSim, TilePosition::new(5, 5, 0), DailyRoutine { entries: vec![RoutineEntry { diff --git a/server/src/perception/observer/mod.rs b/server/src/perception/observer/mod.rs index 66f69e830..2fe1619ad 100644 --- a/server/src/perception/observer/mod.rs +++ b/server/src/perception/observer/mod.rs @@ -88,7 +88,7 @@ pub fn compute_observer_snapshot( ) { let Ok(( observer_entity, - _observer_pos, + observer_pos, facing_opt, observer_kg, mut interaction_buffer, @@ -188,7 +188,7 @@ pub fn compute_observer_snapshot( // Collect sound events audible to the observer (D-038, #124). // Filter by D-018 range: only events the player can hear based on distance. let sound_events = if let Some(ref queue) = sound_queue { - queue.audible_at(_observer_pos).cloned().collect() + queue.audible_at(observer_pos).cloned().collect() } else { Vec::new() }; diff --git a/server/src/perception/observer/tests.rs b/server/src/perception/observer/tests.rs index e5c3c2914..f2b1d3024 100644 --- a/server/src/perception/observer/tests.rs +++ b/server/src/perception/observer/tests.rs @@ -15,6 +15,7 @@ fn setup_world(width: i32, height: i32) -> World { world.init_resource::(); world.init_resource::(); world.init_resource::(); + world.init_resource::(); world } diff --git a/server/src/simulation/sound.rs b/server/src/simulation/sound.rs index a2f359011..3183fb1ad 100644 --- a/server/src/simulation/sound.rs +++ b/server/src/simulation/sound.rs @@ -83,7 +83,9 @@ impl SoundEvent { } /// Whether this sound is audible at `listener_pos`. - /// Simple tile-distance check; occlusion is a future concern (D-018 note). + /// Simple tile-distance check — no wall/obstruction occlusion. + /// TODO: Medium-range sounds should be attenuated or blocked by walls + /// per D-018. Requires LOS integration (backlog — not in v0.1 scope). pub fn audible_at(&self, listener_pos: &TilePosition) -> bool { let ceil = Self::max_range_tiles(self.range); let dx = (self.x.floor() as i32).abs_diff(listener_pos.x); @@ -157,6 +159,13 @@ impl SoundEventQueue { /// Runs each tick after movement/monologue/dialogue systems have fired. /// Removes the emitter component after draining. Ordering: after movement, /// before `compute_observer_snapshot`. +/// +/// NOTE: v0.1 has no sound producers — no system currently inserts +/// SoundEventEmitter components. The pipeline (emitter → queue → snapshot → +/// client bridge) is fully wired but produces zero events at runtime. +/// Sound producers (Footstep on movement, Voice on dialogue) are backlog +/// scope and will be added when the client audio bus routing (#125) is +/// integrated. See D-018 for the sound model specification. pub fn collect_sound_events( mut commands: Commands, mut queue: ResMut, diff --git a/server/src/simulation/tier.rs b/server/src/simulation/tier.rs index 2ecef4c9a..bcb645bce 100644 --- a/server/src/simulation/tier.rs +++ b/server/src/simulation/tier.rs @@ -4,8 +4,6 @@ use bevy_app::prelude::*; use bevy_ecs::prelude::*; -use serde::{Deserialize, Serialize}; - use crate::simulation::movement::{PlayerCharacter, TilePosition}; // --- Tier radius constants (D-026) --- @@ -131,68 +129,11 @@ pub fn update_tier_markers( } } -#[derive(Component, Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub enum SimulationTier { - Active, - Background, - StateSaved, - Ungenerated, -} - -#[derive(Component, Debug, Clone)] -pub struct LastInteraction { - pub tick: u64, -} - -#[derive(Component, Debug, Clone)] -pub struct ScopeTag { - pub tags: Vec, -} - -#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub enum ScopeKind { - Neighborhood, - ActiveQuest, - Colleague, - KnownContact, -} - #[cfg(test)] mod tests { use super::*; use bevy_ecs::world::World; - // --- SimulationTier enum tests --- - - #[test] - fn tier_can_be_added_and_queried() { - let mut world = World::new(); - let entity = world.spawn(SimulationTier::Active).id(); - assert_eq!( - *world.get::(entity).unwrap(), - SimulationTier::Active - ); - } - - #[test] - fn tier_can_transition() { - let mut world = World::new(); - let entity = world.spawn(SimulationTier::Active).id(); - world.entity_mut(entity).insert(SimulationTier::Background); - assert_eq!( - *world.get::(entity).unwrap(), - SimulationTier::Background - ); - } - - #[test] - fn all_tier_variants_are_distinct() { - assert_ne!(SimulationTier::Active, SimulationTier::Background); - assert_ne!(SimulationTier::Background, SimulationTier::StateSaved); - assert_ne!(SimulationTier::StateSaved, SimulationTier::Ungenerated); - assert_ne!(SimulationTier::Active, SimulationTier::Ungenerated); - } - // --- Marker component query correctness (D-026, #94) --- // These tests verify that With / With / With // filter correctly — the core guarantee that behavior systems only run for the @@ -338,57 +279,6 @@ mod tests { assert!(world.get::(entity).is_none()); } - // --- LastInteraction and ScopeTag --- - - #[test] - fn last_interaction_records_tick() { - let mut world = World::new(); - let entity = world.spawn(LastInteraction { tick: 42 }).id(); - - let interaction = world.get::(entity).unwrap(); - assert_eq!(interaction.tick, 42); - } - - #[test] - fn last_interaction_tick_can_be_updated() { - let mut world = World::new(); - let entity = world.spawn(LastInteraction { tick: 1 }).id(); - - world.entity_mut(entity).insert(LastInteraction { tick: 100 }); - - let interaction = world.get::(entity).unwrap(); - assert_eq!(interaction.tick, 100); - } - - #[test] - fn scope_tag_neighborhood_kind() { - let mut world = World::new(); - let entity = world - .spawn(ScopeTag { - tags: vec![ScopeKind::Neighborhood], - }) - .id(); - - let tag = world.get::(entity).unwrap(); - assert!(tag.tags.contains(&ScopeKind::Neighborhood)); - assert!(!tag.tags.contains(&ScopeKind::ActiveQuest)); - } - - #[test] - fn scope_tag_multiple_kinds() { - let entity = ScopeTag { - tags: vec![ - ScopeKind::Neighborhood, - ScopeKind::Colleague, - ScopeKind::KnownContact, - ], - }; - assert_eq!(entity.tags.len(), 3); - assert!(entity.tags.contains(&ScopeKind::Colleague)); - assert!(entity.tags.contains(&ScopeKind::KnownContact)); - assert!(!entity.tags.contains(&ScopeKind::ActiveQuest)); - } - // --- TierPlugin smoke test --- #[test]