From 67cadddaf2023ef6875f718e091ce1578de108e8 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sat, 21 Feb 2026 14:55:21 +0100 Subject: [PATCH] =?UTF-8?q?fix(simulation):=20address=20PR=20#55=20review?= =?UTF-8?q?=20=E2=80=94=20stale=20comment,=20range,=20duplication,=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix protocol version comment (12 → 13) in ObserverSnapshot doc - Widen Want intensity range from 3..=9 to 1..=10 to match spec and test - Replace duplicated pool selection in trigger_recognition_monologue with call to select_pool_line helper (~50 lines removed) - Document NaiveSpatialIndex migration cost for grid/quadtree swap - Remove misleading Default derive from TellCategory (Nervous is not a sensible default for neutral NPCs) - Add caller invariant doc on generate_npc (no TilePosition spawned) Co-Authored-By: Claude Opus 4.6 --- server/src/bridge/types.rs | 2 +- server/src/npc/generate.rs | 7 ++++- server/src/npc/tell_state.rs | 3 +- server/src/simulation/monologue.rs | 50 ++---------------------------- server/src/simulation/spatial.rs | 10 ++++++ 5 files changed, 21 insertions(+), 51 deletions(-) diff --git a/server/src/bridge/types.rs b/server/src/bridge/types.rs index 874f35622..8fd47d332 100644 --- a/server/src/bridge/types.rs +++ b/server/src/bridge/types.rs @@ -37,7 +37,7 @@ pub const PROTOCOL_VERSION: u8 = 13; /// Future fields: ambient sound events, HUD state (D-020 expansion). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ObserverSnapshot { - /// Protocol version for forward compatibility. Current: 12. + /// Protocol version for forward compatibility. Current: 13. pub version: u8, /// Simulation tick when this snapshot was produced pub tick: u64, diff --git a/server/src/npc/generate.rs b/server/src/npc/generate.rs index 0b2870470..395131694 100644 --- a/server/src/npc/generate.rs +++ b/server/src/npc/generate.rs @@ -149,7 +149,7 @@ fn pick_combat_style(idx: usize) -> CombatStyle { fn gen_want(rng: &mut SimRng, role: &RoleDefinition) -> Want { let kind_idx = rng.rng.random_range(0..9_usize); - let intensity = rng.rng.random_range(3_u8..=9); + let intensity = rng.rng.random_range(1_u8..=10); Want { primary: pick_want_kind(kind_idx), intensity, @@ -421,6 +421,11 @@ fn gen_skills(rng: &mut SimRng, role: &RoleDefinition) -> (SkillSet, Option Entity { // Generate all axes before spawning to keep the borrow checker happy. diff --git a/server/src/npc/tell_state.rs b/server/src/npc/tell_state.rs index 2afe99981..6e74342df 100644 --- a/server/src/npc/tell_state.rs +++ b/server/src/npc/tell_state.rs @@ -35,10 +35,9 @@ use crate::simulation::tier::ActiveSim; /// /// Derived each tick from NPC simulation state — not authored per NPC. /// Five categories correspond to the D-024 tell taxonomy. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum TellCategory { /// NPC exhibits nervous behaviour: Major secret + stress exceeds half of threshold. - #[default] Nervous, /// NPC exhibits angry behaviour: low contentment and Hostile mood. Angry, diff --git a/server/src/simulation/monologue.rs b/server/src/simulation/monologue.rs index f5fdff78d..b549567c9 100644 --- a/server/src/simulation/monologue.rs +++ b/server/src/simulation/monologue.rs @@ -352,53 +352,9 @@ pub fn trigger_recognition_monologue( }; // Try content pools for observe_anomaly trigger lines - let line = if let Some(ref content) = content { - let character = state.character.as_str(); - let mut candidates: Vec<(&str, &str)> = Vec::new(); - - for district in content.0.districts.values() { - for pool in &district.monologue_pools { - if pool.character != character { - continue; - } - for line in &pool.lines { - if line.trigger != "observe_anomaly" { - continue; - } - if state.shown_ids.contains(&line.id) { - continue; - } - candidates.push((&line.id, &line.text)); - } - } - } - - if candidates.is_empty() { - // Fallback: allow repeats from content pools - for district in content.0.districts.values() { - for pool in &district.monologue_pools { - if pool.character != character { - continue; - } - for line in &pool.lines { - if line.trigger != "observe_anomaly" { - continue; - } - candidates.push((&line.id, &line.text)); - } - } - } - } - - if !candidates.is_empty() { - let i = rng.rng.random_range(0..candidates.len()); - Some((candidates[i].0.to_string(), candidates[i].1.to_string())) - } else { - None - } - } else { - None - }; + let line = content + .as_deref() + .and_then(|c| select_pool_line("observe_anomaly", &state, c, &mut rng.rng)); // Use content pool line or hardcoded fallback let (id, text) = if let Some((id, text)) = line { diff --git a/server/src/simulation/spatial.rs b/server/src/simulation/spatial.rs index fcf0200e3..3bde13cbb 100644 --- a/server/src/simulation/spatial.rs +++ b/server/src/simulation/spatial.rs @@ -33,6 +33,16 @@ pub trait SpatialIndex: Send + Sync { /// Replace with grid or quadtree when profiling shows this is a bottleneck. /// Deterministic iteration: entries stored in insertion order, but callers /// should not depend on ordering (sort by Entity::to_bits() if needed). +/// +/// ## Migration cost for grid/quadtree swap +/// +/// `sync_spatial_index` takes `ResMut` directly because +/// bevy_ecs cannot store `dyn SpatialIndex` as a Resource. A swap to Grid or +/// BVH requires changing the concrete type in: (1) `sync_spatial_index` system +/// parameter, (2) `SimulationPlugin` resource registration, (3) any system +/// that queries `Res` (currently: `update_follow_state`). +/// The `SpatialIndex` trait ensures the API surface stays identical — only the +/// type name changes at call sites. Estimated: ~5 lines per caller. #[derive(Resource, Debug, Default)] pub struct NaiveSpatialIndex { entries: Vec<(Entity, TilePosition)>,