fix(simulation): address PR #50 review — determinism, overflow, invariants

- Sort eligible NPCs by StableId for deterministic conversation pairing (D-010) - Replace single_mut() with per-observer iteration (D-010 principle 3, D-027) - Widen current_stress * 100 to i32 in mood derivation to prevent i16 overflow - Promote line_interval to named constant LINE_INTERVAL_TICKS - Add 4 Sprint 14 component invariants (MoodState, InteractionMemory, ActivityState+PathRequest exclusion, double-tagged tier detection) - Attach MoodState to content spawn pipeline and gauntlet fixup - VecDeque for InteractionMemory.notable_events (O(1) pop_front) - Rename who_knows() to who_knows_full_scan() to communicate O(N) cost - Add O(N^2) growth limit comment on conversation pair scan - Document Suspicious/Focused as externally-set moods on enum variants Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-20 19:18:34 +01:00
parent 019a7b2221
commit f0e0673c99
7 changed files with 205 additions and 28 deletions
@@ -38,6 +38,12 @@ use crate::simulation::pathfinding::{ComputedPath, PathBlocked};
 #[cfg(feature = "gauntlet")]
 use crate::simulation::sound::SoundEvent;
 #[cfg(feature = "gauntlet")]
+use crate::npc::interaction::InteractionMemory;
+#[cfg(feature = "gauntlet")]
+use crate::npc::mood::MoodState;
+#[cfg(feature = "gauntlet")]
+use crate::npc::routine::ActivityState;
+#[cfg(feature = "gauntlet")]
 use crate::simulation::tier::{ActiveSim, BackgroundSim, StateSaved};

 #[cfg(feature = "gauntlet")]
@@ -93,6 +99,12 @@ pub fn run_invariants(world: &mut World) {
    inv_sim6_registry_has_entities(world);
    inv_sim7_player_has_monologue_buffer(world);
    inv_sim8_player_has_interaction_buffer(world);
+
+    // --- Sprint 14 component invariants ---
+    inv_s14_active_npcs_have_mood_state(world);
+    inv_s14_npcs_have_interaction_memory(world);
+    inv_s14_no_activity_state_with_path_request(world);
+    inv_pop3b_no_double_tagged_tiers(world);
 }

 // ===========================================================================
@@ -729,6 +741,111 @@ fn inv_sim8_player_has_interaction_buffer(world: &mut World) {
    );
 }

+// ===========================================================================
+// Sprint 14 component invariants
+// ===========================================================================
+
+/// S14-1: Every Active-tier NPC has a MoodState component.
+/// Active NPCs without MoodState are invisible to the mood-driven dialogue pipeline.
+#[cfg(feature = "gauntlet")]
+fn inv_s14_active_npcs_have_mood_state(world: &mut World) {
+    let active = {
+        let mut q = world.query_filtered::<(), (bevy_ecs::prelude::With<Npc>, bevy_ecs::prelude::With<ActiveSim>)>();
+        q.iter(world).count()
+    };
+    let with_mood = {
+        let mut q = world.query_filtered::<(), (bevy_ecs::prelude::With<Npc>, bevy_ecs::prelude::With<ActiveSim>, bevy_ecs::prelude::With<MoodState>)>();
+        q.iter(world).count()
+    };
+    assert_eq!(
+        with_mood,
+        active,
+        "S14-1: all {} Active NPCs must have MoodState; only {} do",
+        active,
+        with_mood
+    );
+}
+
+/// S14-2: Every NPC has an InteractionMemory component.
+/// NPCs without InteractionMemory cannot drive Layer 2 situation activation.
+#[cfg(feature = "gauntlet")]
+fn inv_s14_npcs_have_interaction_memory(world: &mut World) {
+    let total = {
+        let mut q = world.query_filtered::<(), bevy_ecs::prelude::With<Npc>>();
+        q.iter(world).count()
+    };
+    let with_mem = {
+        let mut q = world.query_filtered::<(), (bevy_ecs::prelude::With<Npc>, bevy_ecs::prelude::With<InteractionMemory>)>();
+        q.iter(world).count()
+    };
+    assert_eq!(
+        with_mem,
+        total,
+        "S14-2: all {} NPCs must have InteractionMemory; only {} do",
+        total,
+        with_mem
+    );
+}
+
+/// S14-3: No entity has both ActivityState and PathRequest simultaneously.
+/// ActivityState means "at destination, performing activity". PathRequest means
+/// "needs to move somewhere". Both at once is contradictory.
+#[cfg(feature = "gauntlet")]
+fn inv_s14_no_activity_state_with_path_request(world: &mut World) {
+    use bevy_ecs::prelude::{Entity, With};
+    use crate::simulation::pathfinding::PathRequest;
+
+    let with_activity: Vec<Entity> = {
+        let mut q = world.query_filtered::<Entity, With<ActivityState>>();
+        q.iter(world).collect()
+    };
+    for entity in with_activity {
+        assert!(
+            world.get::<PathRequest>(entity).is_none(),
+            "S14-3: entity {:?} has both ActivityState and PathRequest — mutually exclusive",
+            entity
+        );
+    }
+}
+
+/// Pop3b: No NPC is double-tagged with multiple tier markers.
+/// Pop3 catches missing tiers via sum check, but not double-tagged entities
+/// (e.g. both ActiveSim + BackgroundSim would still sum correctly).
+#[cfg(feature = "gauntlet")]
+fn inv_pop3b_no_double_tagged_tiers(world: &mut World) {
+    use bevy_ecs::prelude::{Entity, With};
+
+    let active_and_bg: Vec<Entity> = {
+        let mut q = world.query_filtered::<Entity, (With<Npc>, With<ActiveSim>, With<BackgroundSim>)>();
+        q.iter(world).collect()
+    };
+    assert!(
+        active_and_bg.is_empty(),
+        "Pop3b: {} NPC(s) are double-tagged with both ActiveSim and BackgroundSim",
+        active_and_bg.len()
+    );
+
+    let active_and_ss: Vec<Entity> = {
+        let mut q = world.query_filtered::<Entity, (With<Npc>, With<ActiveSim>, With<StateSaved>)>();
+        q.iter(world).collect()
+    };
+    assert!(
+        active_and_ss.is_empty(),
+        "Pop3b: {} NPC(s) are double-tagged with both ActiveSim and StateSaved",
+        active_and_ss.len()
+    );
+
+    let bg_and_ss: Vec<Entity> = {
+        let mut q = world.query_filtered::<Entity, (With<Npc>, With<BackgroundSim>, With<StateSaved>)>();
+        q.iter(world).collect()
+    };
+    assert!(
+        bg_and_ss.is_empty(),
+        "Pop3b: {} NPC(s) are double-tagged with both BackgroundSim and StateSaved",
+        bg_and_ss.len()
+    );
+}
+
 // ===========================================================================
 // Additional system-execution tests (7 more invariants, via #[test])
 // Tests 30-36 exercise behaviour that requires running ECS systems.
@@ -495,6 +495,48 @@ pub fn setup_gauntlet(app: &mut App) {
    }

    app.insert_resource(snapshots);
+
+    // --- Sprint 14 component fixup ---
+    // Attach MoodState and InteractionMemory to all Npc entities that are
+    // missing them. Gauntlet room builders don't include these yet — this
+    // ensures invariant S14-1/S14-2 pass and the mood/trust systems have
+    // valid component targets.
+    {
+        use crate::npc::interaction::InteractionMemory;
+        use crate::npc::mood::MoodState;
+        use crate::npc::Npc;
+
+        let missing_mood: Vec<bevy_ecs::prelude::Entity> = {
+            let mut q = app
+                .world_mut()
+                .query_filtered::<bevy_ecs::prelude::Entity, (
+                    bevy_ecs::prelude::With<Npc>,
+                    bevy_ecs::prelude::Without<MoodState>,
+                )>();
+            q.iter(app.world()).collect()
+        };
+        for entity in missing_mood {
+            app.world_mut()
+                .entity_mut(entity)
+                .insert(MoodState::default());
+        }
+
+        let missing_mem: Vec<bevy_ecs::prelude::Entity> = {
+            let mut q = app
+                .world_mut()
+                .query_filtered::<bevy_ecs::prelude::Entity, (
+                    bevy_ecs::prelude::With<Npc>,
+                    bevy_ecs::prelude::Without<InteractionMemory>,
+                )>();
+            q.iter(app.world()).collect()
+        };
+        for entity in missing_mem {
+            app.world_mut()
+                .entity_mut(entity)
+                .insert(InteractionMemory::default());
+        }
+    }
+
    app.insert_resource(registry);
 }