jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(simulation): address PR #42 review — 8 items from Hoshe and Tyre

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
jpmschweitzer
2026-02-19 15:08:32 +01:00
co-authored by Claude Opus 4.6
parent 8e00db2b2d
commit 03c44bebaf
7 changed files with 51 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/bridge/types.rs
+1 -1
View File
@@ -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,
server/src/knowledge/graph.rs
+34 -1
View File
@@ -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::<u64>().ok())
.and_then(|v| match v.parse::<u64>() {
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"
);
}
}
server/src/npc/routine.rs
+3
View File
@@ -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 {
server/src/perception/observer/mod.rs
+2 -2
View File
@@ -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()
};
server/src/perception/observer/tests.rs
+1
View File
@@ -15,6 +15,7 @@ fn setup_world(width: i32, height: i32) -> World {
world.init_resource::<EntityRegistry>();
world.init_resource::<VisibilityGeometry>();
world.init_resource::<ActivePerceptionMode>();
world.init_resource::<crate::simulation::sound::SoundEventQueue>();
world
}
server/src/simulation/sound.rs
+10 -1
View File
@@ -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<SoundEventQueue>,
server/src/simulation/tier.rs
-110
View File
@@ -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<ScopeKind>,
}
#[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::<SimulationTier>(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::<SimulationTier>(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<ActiveSim> / With<BackgroundSim> / With<StateSaved>
// filter correctly — the core guarantee that behavior systems only run for the
@@ -338,57 +279,6 @@ mod tests {
assert!(world.get::<ActiveSim>(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::<LastInteraction>(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::<LastInteraction>(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::<ScopeTag>(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]