feat(simulation): Sprint 14 — NPC mood, trust, routines, conversations, invariants #50

Merged

jpmschweitzer merged 8 commits from server into main 2026-02-20 19:28:54 +01:00

Conversation 2 Commits 8 Files Changed 21

+4322 -17

jpmschweitzer commented 2026-02-20 19:00:17 +01:00

Owner

Copy Link

Copy Source

Summary

Sprint 14 server delivery: NPC social systems that make the simulation breathe independently of the player.

• #247 NPC-to-NPC conversation system with D-078 server-authoritative per-word occlusion (ConversationEvent on ObserverSnapshot, protocol v12)
• #323 NPC mood state machine — 8-state FSM driven by triangle pressure, time of day, and interactions (D-024/D-035)
• #324 Trust progression system — TalkCompleted +1, WalkAway -1, Confrontation -2, clamped [-10,+10]
• #325 Interaction tracking component — InteractionMemory per NPC-pair with count, tick stamp, notable events
• #101 Routine execution system — ActivityState component closes the full PathRequest-to-activity loop (D-031)
• #103 NPC-NPC relationship dynamics — passive trust decay toward neutral after 1h inactivity, Background tier
• #508 Map-agnostic invariant tests — 36 invariants across 4 categories (structural, perception, population, simulation)

Key constraints met: No floats (D-010), no HashMap in simulation, all RNG through SimRng, integer-only Bernoulli trials for occlusion.

644 lib tests pass. 21 files changed, +4,142 lines.

Test plan

• cargo test --lib — 644 tests pass (mood: 31, trust: 12, routine: 8, conversation: 15, invariants: 36+)
• Bridge IPC/TCP integration tests pass
• Determinism: occlusion and mood transitions produce identical output with same seed
• Manual: run gauntlet rooms, verify NPCs transition between routine phases and hold ActivityState
• Manual: verify NPC-to-NPC conversations emit Voice SoundEvents observable on client

## Summary Sprint 14 server delivery: NPC social systems that make the simulation breathe independently of the player. - **#247** NPC-to-NPC conversation system with D-078 server-authoritative per-word occlusion (ConversationEvent on ObserverSnapshot, protocol v12) - **#323** NPC mood state machine — 8-state FSM driven by triangle pressure, time of day, and interactions (D-024/D-035) - **#324** Trust progression system — TalkCompleted +1, WalkAway -1, Confrontation -2, clamped [-10,+10] - **#325** Interaction tracking component — InteractionMemory per NPC-pair with count, tick stamp, notable events - **#101** Routine execution system — ActivityState component closes the full PathRequest-to-activity loop (D-031) - **#103** NPC-NPC relationship dynamics — passive trust decay toward neutral after 1h inactivity, Background tier - **#508** Map-agnostic invariant tests — 36 invariants across 4 categories (structural, perception, population, simulation) **Key constraints met:** No floats (D-010), no HashMap in simulation, all RNG through SimRng, integer-only Bernoulli trials for occlusion. 644 lib tests pass. 21 files changed, +4,142 lines. ## Test plan - [x] `cargo test --lib` — 644 tests pass (mood: 31, trust: 12, routine: 8, conversation: 15, invariants: 36+) - [x] Bridge IPC/TCP integration tests pass - [x] Determinism: occlusion and mood transitions produce identical output with same seed - [ ] Manual: run gauntlet rooms, verify NPCs transition between routine phases and hold ActivityState - [ ] Manual: verify NPC-to-NPC conversations emit Voice SoundEvents observable on client

jpmschweitzer added 7 commits 2026-02-20 19:00:18 +01:00

feat(simulation): NPC-to-NPC conversation system (#247, D-078) ... aa0d8ced37

Implements server-authoritative per-word occlusion for NPC conversations.
ConversationEventBuffer drains into ObserverSnapshot each tick so the client
receives only words audible from the player's position. Updates test fixtures
to include the new conversation_events and conversation_ended fields.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(simulation): NPC mood state machine (#323, D-024, D-035) ... 9eba259fd5

8-state NPC mood FSM driven by stress, time of day, and interaction events.
Adds MoodState component, derive_mood() pure function, and update_mood() Bevy
system wired into NpcPlugin. Adds Focused as 9th Mood content tag (D-035
Sprint 8 amendment). Syncs CurrentMood for Layer 4 dialogue selection.

31 unit tests covering priority ordering, boundary conditions, and system
integration. All random values route through SimRng (D-010 determinism).
No floats. No HashMap.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test(simulation): map-agnostic invariant tests, 36 invariants (#508) ... a14c980aad

Adds test_world::invariants with 29 world-query invariants (structural,
perception, population, simulation) and 7 system-execution tests. All
invariants are gated behind the gauntlet feature and run against the
fully-initialized gauntlet world. Documents StableId ranges through
Sprint 14 rooms.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(simulation): interaction tracking + trust progression (#325, #324) ... dcd15a330f

InteractionMemory component tracks interaction_count, last_interaction_tick,
and notable_events per NPC. Drives D-028 Layer 2 situation activation:
first_meeting (count==0) and repeated_visit (count>=3). warm_active in mood
system now derives from InteractionMemory within a 300-tick window.

Trust progression wired into dialogue systems: talk completion (+1),
walk-away (-1), confrontation (-2) emit TrustEvents consumed by update_trust.
InteractionEvent (WalkAway, Confrontation) recorded in notable_events for
fast per-pair access.

Adds FirstMeeting and RepeatedVisit Situation variants. 18 unit tests in
interaction.rs. All arithmetic integer-only (D-010 determinism). No HashMap.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(simulation): routine execution system with ActivityState (#101, D-031) ... bd60b356f8

ActivityState component tracks the activity an NPC is currently performing
at their routine destination (activity name, phase, started_tick). The
enter_activity system runs after movement validation and sets ActivityState
when an NPC has arrived with no active path. Cleared on phase transitions.

Feeds TellTrigger::DuringActivity and D-028 Layer 2 situation matching.
Unit tests verified via shift_change room integration.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(simulation): NPC-NPC relationship dynamics with passive decay (#103, D-024) ... 8bd65a5662

update_relationship_dynamics runs once per game-minute (10 ticks). Edges
inactive for 1+ game-hours (600 ticks) decay trust 1 point toward 0 per
minute. Creates social texture: NPCs who haven't interacted drift to neutral
without active maintenance. Blocks #249 (social propagation, Sprint 15).

Constants: DECAY_INTERVAL_TICKS=10, DECAY_INACTIVITY_THRESHOLD_TICKS=600,
DECAY_DELTA=1. Integer arithmetic only (D-010 determinism).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test(simulation): additional conversation system QA coverage (Hoshe, S14) ... 019a7b2221

Additional integration tests for conversation cooldown application, distance
termination with both NPCs receiving cooldown, and edge cases in the NPC-to-NPC
conversation pipeline.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

jpmschweitzer commented 2026-02-20 19:09:33 +01:00

Author

Owner

Copy Link

Copy Source

PR Review: server -> main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

The Sprint 14 NPC systems are well-structured, clearly documented, and heavily tested. The state machine logic, relationship graph, and conversation pipeline are solid overall. Three issues need attention before merge.

#	File	Severity	Issue
1	mood.rs:148	warning	current_stress * 100 computed as i16 * i16 — any current_stress above 327 overflows. Widen to i32 before comparison.
2	conversation.rs:403	warning	line_interval = 20 is a magic number defined inline — every other timing constant is a named const. Promote to LINE_INTERVAL_TICKS.
3	invariants.rs (all 36)	warning	No invariants verify the 4 new Sprint 14 components (MoodState, InteractionMemory, ActivityState, ConversationCooldown). Missing: every Active NPC should have MoodState, every NPC should have InteractionMemory, no entity should have both ActivityState and PathRequest.
4	invariants.rs:423	suggestion	Pop3 catches missing-tier NPCs but not double-tagged (both ActiveSim + BackgroundSim would still sum correctly).
5	interaction.rs:68	suggestion	Vec::remove(0) is O(n) — VecDeque::pop_front() would be O(1). Fine at MAX_NOTABLE_EVENTS=16 but inconsistent with codebase performance philosophy.
6	conversation.rs:287	suggestion	O(N^2) pairing loop favors early entities in ECS storage order — first eligible pair that passes 2% roll always wins. Worth a comment.
7	conversation.rs	suggestion	player_query.single_mut() single-player assumption is implicit — relationships.rs has an equivalent TODO comment, this system doesn't.

Tyre (Architecture): REQUEST_CHANGES

Well-structured sprint with strong component discipline and genuine care for D-010 determinism. Separation of concerns is clean — no system reaches into another's internals. Two issues require changes before merge.

#	File	Severity	Issue
1	conversation.rs:271-327	warning	Bevy Query::iter() does not guarantee deterministic entity order across runs. The eligible Vec ordering is not stable, so RNG consumption order diverges between replays. Sort eligible by StableId before the pair loop.
2	conversation.rs:418,505	warning	single_mut() bakes in single-player assumption, violates D-010 principle 3. With two playable characters (D-027), this panics. D-078 requires per-observer occlusion — single_mut() structurally prevents this. Attach ConversationEventBuffer to each observer entity.
3	conversation.rs:430-431	warning	Ambient noise hardcoded to 0 — zone-conspicuousness (D-071) won't wire in cleanly. Function signature should accept zone ambient noise level even if stubbed.
4	conversation.rs:287-291	suggestion	O(N^2) pair scan runs every tick. Acceptable for v0.1 Active-tier counts (30-80 NPCs). Add comment noting the growth limit.
5	mood.rs:50-56	suggestion	Suspicious and Focused are unreachable from derive_mood() — update_mood will overwrite externally-set states next tick. Add guard or manual-set flag before observation pipeline ships.
6	interaction.rs:47	suggestion	Vec::remove(0) should be VecDeque::pop_front() for O(1) amortization.
7	relationships.rs:145	suggestion	who_knows() O(N) full scan — rename to who_knows_full_scan to communicate cost at call site.

Verdict: CHANGES REQUESTED

Key fixes before merge:

1. Sort eligible NPCs by StableId for deterministic conversation pairing (D-010)
2. Replace single_mut() with per-observer iteration (D-010 principle 3, D-078)
3. Widen current_stress * 100 to i32 to prevent overflow
4. Add invariants for Sprint 14 components (MoodState, InteractionMemory)
5. Promote line_interval to named constant
6. Structure ambient noise call site for future zone integration

## PR Review: server -> main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES The Sprint 14 NPC systems are well-structured, clearly documented, and heavily tested. The state machine logic, relationship graph, and conversation pipeline are solid overall. Three issues need attention before merge. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `mood.rs:148` | warning | `current_stress * 100` computed as `i16 * i16` — any `current_stress` above 327 overflows. Widen to `i32` before comparison. | | 2 | `conversation.rs:403` | warning | `line_interval = 20` is a magic number defined inline — every other timing constant is a named `const`. Promote to `LINE_INTERVAL_TICKS`. | | 3 | `invariants.rs` (all 36) | warning | No invariants verify the 4 new Sprint 14 components (MoodState, InteractionMemory, ActivityState, ConversationCooldown). Missing: every Active NPC should have MoodState, every NPC should have InteractionMemory, no entity should have both ActivityState and PathRequest. | | 4 | `invariants.rs:423` | suggestion | Pop3 catches missing-tier NPCs but not double-tagged (both ActiveSim + BackgroundSim would still sum correctly). | | 5 | `interaction.rs:68` | suggestion | `Vec::remove(0)` is O(n) — `VecDeque::pop_front()` would be O(1). Fine at MAX_NOTABLE_EVENTS=16 but inconsistent with codebase performance philosophy. | | 6 | `conversation.rs:287` | suggestion | O(N^2) pairing loop favors early entities in ECS storage order — first eligible pair that passes 2% roll always wins. Worth a comment. | | 7 | `conversation.rs` | suggestion | `player_query.single_mut()` single-player assumption is implicit — relationships.rs has an equivalent TODO comment, this system doesn't. | ### Tyre (Architecture): REQUEST_CHANGES Well-structured sprint with strong component discipline and genuine care for D-010 determinism. Separation of concerns is clean — no system reaches into another's internals. Two issues require changes before merge. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `conversation.rs:271-327` | warning | Bevy `Query::iter()` does not guarantee deterministic entity order across runs. The `eligible` Vec ordering is not stable, so RNG consumption order diverges between replays. Sort `eligible` by StableId before the pair loop. | | 2 | `conversation.rs:418,505` | warning | `single_mut()` bakes in single-player assumption, violates D-010 principle 3. With two playable characters (D-027), this panics. D-078 requires per-observer occlusion — `single_mut()` structurally prevents this. Attach ConversationEventBuffer to each observer entity. | | 3 | `conversation.rs:430-431` | warning | Ambient noise hardcoded to 0 — zone-conspicuousness (D-071) won't wire in cleanly. Function signature should accept zone ambient noise level even if stubbed. | | 4 | `conversation.rs:287-291` | suggestion | O(N^2) pair scan runs every tick. Acceptable for v0.1 Active-tier counts (30-80 NPCs). Add comment noting the growth limit. | | 5 | `mood.rs:50-56` | suggestion | `Suspicious` and `Focused` are unreachable from `derive_mood()` — `update_mood` will overwrite externally-set states next tick. Add guard or manual-set flag before observation pipeline ships. | | 6 | `interaction.rs:47` | suggestion | `Vec::remove(0)` should be `VecDeque::pop_front()` for O(1) amortization. | | 7 | `relationships.rs:145` | suggestion | `who_knows()` O(N) full scan — rename to `who_knows_full_scan` to communicate cost at call site. | ### Verdict: CHANGES REQUESTED **Key fixes before merge:** 1. Sort eligible NPCs by StableId for deterministic conversation pairing (D-010) 2. Replace `single_mut()` with per-observer iteration (D-010 principle 3, D-078) 3. Widen `current_stress * 100` to i32 to prevent overflow 4. Add invariants for Sprint 14 components (MoodState, InteractionMemory) 5. Promote `line_interval` to named constant 6. Structure ambient noise call site for future zone integration

jpmschweitzer commented 2026-02-20 19:11:51 +01:00

Author

Owner

Copy Link

Copy Source

PR Review: server -> main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

The Sprint 14 NPC systems are well-structured, clearly documented, and heavily tested. The state machine logic, relationship graph, and conversation pipeline are solid overall. Three issues need attention before merge.

#	File	Severity	Issue
1	mood.rs:148	warning	current_stress * 100 computed as i16 * i16 — any current_stress above 327 overflows. Widen to i32 before comparison.
2	conversation.rs:403	warning	line_interval = 20 is a magic number defined inline — every other timing constant is a named const. Promote to LINE_INTERVAL_TICKS.
3	invariants.rs (all 36)	warning	No invariants verify the 4 new Sprint 14 components (MoodState, InteractionMemory, ActivityState, ConversationCooldown). Missing: every Active NPC should have MoodState, every NPC should have InteractionMemory, no entity should have both ActivityState and PathRequest.
4	invariants.rs:423	suggestion	Pop3 catches missing-tier NPCs but not double-tagged (both ActiveSim + BackgroundSim would still sum correctly).
5	interaction.rs:68	suggestion	Vec::remove(0) is O(n) — VecDeque::pop_front() would be O(1). Fine at MAX_NOTABLE_EVENTS=16 but inconsistent with codebase performance philosophy.
6	conversation.rs:287	suggestion	O(N^2) pairing loop favors early entities in ECS storage order — first eligible pair that passes 2% roll always wins. Worth a comment.
7	conversation.rs	suggestion	player_query.single_mut() single-player assumption is implicit — relationships.rs has an equivalent TODO comment, this system doesn't.

Tyre (Architecture): REQUEST_CHANGES

Well-structured sprint with strong component discipline and genuine care for D-010 determinism. Separation of concerns is clean — no system reaches into another's internals. Two issues require changes before merge.

#	File	Severity	Issue
1	conversation.rs:271-327	warning	Bevy Query::iter() does not guarantee deterministic entity order across runs. The eligible Vec ordering is not stable, so RNG consumption order diverges between replays. Sort eligible by StableId before the pair loop.
2	conversation.rs:418,505	warning	single_mut() bakes in single-player assumption, violates D-010 principle 3. With two playable characters (D-027), this panics. D-078 requires per-observer occlusion — single_mut() structurally prevents this. Attach ConversationEventBuffer to each observer entity.
3	conversation.rs:430-431	warning	Ambient noise hardcoded to 0 — zone-conspicuousness (D-071) won't wire in cleanly. Function signature should accept zone ambient noise level even if stubbed.
4	conversation.rs:287-291	suggestion	O(N^2) pair scan runs every tick. Acceptable for v0.1 Active-tier counts (30-80 NPCs). Add comment noting the growth limit.
5	mood.rs:50-56	suggestion	Suspicious and Focused are unreachable from derive_mood() — update_mood will overwrite externally-set states next tick. Add guard or manual-set flag before observation pipeline ships.
6	interaction.rs:47	suggestion	Vec::remove(0) should be VecDeque::pop_front() for O(1) amortization.
7	relationships.rs:145	suggestion	who_knows() O(N) full scan — rename to who_knows_full_scan to communicate cost at call site.

Verdict: CHANGES REQUESTED

Key fixes before merge:

1. Sort eligible NPCs by StableId for deterministic conversation pairing (D-010)
2. Replace single_mut() with per-observer iteration (D-010 principle 3, D-078)
3. Widen current_stress * 100 to i32 to prevent overflow
4. Add invariants for Sprint 14 components (MoodState, InteractionMemory)
5. Promote line_interval to named constant
6. Structure ambient noise call site for future zone integration

## PR Review: server -> main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES The Sprint 14 NPC systems are well-structured, clearly documented, and heavily tested. The state machine logic, relationship graph, and conversation pipeline are solid overall. Three issues need attention before merge. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `mood.rs:148` | warning | `current_stress * 100` computed as `i16 * i16` — any `current_stress` above 327 overflows. Widen to `i32` before comparison. | | 2 | `conversation.rs:403` | warning | `line_interval = 20` is a magic number defined inline — every other timing constant is a named `const`. Promote to `LINE_INTERVAL_TICKS`. | | 3 | `invariants.rs` (all 36) | warning | No invariants verify the 4 new Sprint 14 components (MoodState, InteractionMemory, ActivityState, ConversationCooldown). Missing: every Active NPC should have MoodState, every NPC should have InteractionMemory, no entity should have both ActivityState and PathRequest. | | 4 | `invariants.rs:423` | suggestion | Pop3 catches missing-tier NPCs but not double-tagged (both ActiveSim + BackgroundSim would still sum correctly). | | 5 | `interaction.rs:68` | suggestion | `Vec::remove(0)` is O(n) — `VecDeque::pop_front()` would be O(1). Fine at MAX_NOTABLE_EVENTS=16 but inconsistent with codebase performance philosophy. | | 6 | `conversation.rs:287` | suggestion | O(N^2) pairing loop favors early entities in ECS storage order — first eligible pair that passes 2% roll always wins. Worth a comment. | | 7 | `conversation.rs` | suggestion | `player_query.single_mut()` single-player assumption is implicit — relationships.rs has an equivalent TODO comment, this system doesn't. | ### Tyre (Architecture): REQUEST_CHANGES Well-structured sprint with strong component discipline and genuine care for D-010 determinism. Separation of concerns is clean — no system reaches into another's internals. Two issues require changes before merge. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `conversation.rs:271-327` | warning | Bevy `Query::iter()` does not guarantee deterministic entity order across runs. The `eligible` Vec ordering is not stable, so RNG consumption order diverges between replays. Sort `eligible` by StableId before the pair loop. | | 2 | `conversation.rs:418,505` | warning | `single_mut()` bakes in single-player assumption, violates D-010 principle 3. With two playable characters (D-027), this panics. D-078 requires per-observer occlusion — `single_mut()` structurally prevents this. Attach ConversationEventBuffer to each observer entity. | | 3 | `conversation.rs:430-431` | warning | Ambient noise hardcoded to 0 — zone-conspicuousness (D-071) won't wire in cleanly. Function signature should accept zone ambient noise level even if stubbed. | | 4 | `conversation.rs:287-291` | suggestion | O(N^2) pair scan runs every tick. Acceptable for v0.1 Active-tier counts (30-80 NPCs). Add comment noting the growth limit. | | 5 | `mood.rs:50-56` | suggestion | `Suspicious` and `Focused` are unreachable from `derive_mood()` — `update_mood` will overwrite externally-set states next tick. Add guard or manual-set flag before observation pipeline ships. | | 6 | `interaction.rs:47` | suggestion | `Vec::remove(0)` should be `VecDeque::pop_front()` for O(1) amortization. | | 7 | `relationships.rs:145` | suggestion | `who_knows()` O(N) full scan — rename to `who_knows_full_scan` to communicate cost at call site. | ### Verdict: CHANGES REQUESTED **Key fixes before merge:** 1. Sort eligible NPCs by StableId for deterministic conversation pairing (D-010) 2. Replace `single_mut()` with per-observer iteration (D-010 principle 3, D-078) 3. Widen `current_stress * 100` to i32 to prevent overflow 4. Add invariants for Sprint 14 components (MoodState, InteractionMemory) 5. Promote `line_interval` to named constant 6. Structure ambient noise call site for future zone integration

jpmschweitzer referenced this issue from a commit 2026-02-20 19:18:42 +01:00

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

jpmschweitzer added 1 commit 2026-02-20 19:18:43 +01:00

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

- 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>

jpmschweitzer merged commit f1dcdc2ee9 into main 2026-02-20 19:28:54 +01:00

jpmschweitzer referenced this issue from a commit 2026-02-20 19:28:54 +01:00

Merge pull request 'feat(simulation): Sprint 14 — NPC mood, trust, routines, conversations, invariants' (#50) from server into main

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

jpmschweitzer (Jeroen Schweitzer)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#50