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

feat(simulation): sprint 16 dialogue server #56

Closed
jpmschweitzer wants to merge 0 commits from server into main
pull from: server
merge into: :main
Conversation 1 Commits - Files Changed -
jpmschweitzer commented 2026-02-23 20:02:24 +01:00
Owner
Copy Link
Copy Source

Summary

Sprint 16 server deliverables — dialogue becomes a two-way exchange:

  • #538 Move dialogue system registrations from BridgePlugin to NpcPlugin (game logic in the right module)
  • #539 Implement process_dialogue_response — full D-028 four-layer pipeline for follow-up lines when player picks a dialogue option
  • #171 Verify trust-gated gossip pipeline (D-075): Friendly+KnowsDetails unlocks Secret tier, Suspects stays Surface-only
  • #338 Wire DialogueCooldownTracker — no line repeats within 600-tick cooldown window

Also fixes a latent schedule ambiguity: emit_observation_events now has explicit .before(advance_tick) constraint.

Test plan

  • 857/857 tests pass (cargo test --all)
  • Trust-gated gossip: integration test confirms Secret-tier access with KnowsDetails, Surface-only with Suspects
  • Line variety: regression test confirms no line_id repeats across 10 consecutive Talks within cooldown window
  • DialogueResponse: integration test confirms follow-up selection, conversation end on pool exhaustion, no-profile graceful noop
  • Serialization: DialogueResponse roundtrip test added
  • Golden test check after copy #542 merges (line ID rename may require fixture update)
## Summary Sprint 16 server deliverables — dialogue becomes a two-way exchange: - **#538** Move dialogue system registrations from BridgePlugin to NpcPlugin (game logic in the right module) - **#539** Implement `process_dialogue_response` — full D-028 four-layer pipeline for follow-up lines when player picks a dialogue option - **#171** Verify trust-gated gossip pipeline (D-075): Friendly+KnowsDetails unlocks Secret tier, Suspects stays Surface-only - **#338** Wire DialogueCooldownTracker — no line repeats within 600-tick cooldown window Also fixes a latent schedule ambiguity: `emit_observation_events` now has explicit `.before(advance_tick)` constraint. ## Test plan - [x] 857/857 tests pass (cargo test --all) - [x] Trust-gated gossip: integration test confirms Secret-tier access with KnowsDetails, Surface-only with Suspects - [x] Line variety: regression test confirms no line_id repeats across 10 consecutive Talks within cooldown window - [x] DialogueResponse: integration test confirms follow-up selection, conversation end on pool exhaustion, no-profile graceful noop - [x] Serialization: DialogueResponse roundtrip test added - [ ] Golden test check after copy #542 merges (line ID rename may require fixture update)
jpmschweitzer added 1 commit 2026-02-23 20:02:24 +01:00
feat(simulation): sprint 16 dialogue server — response handler, trust gossip, variety tracker efa2dc543f 
Move dialogue system registrations from BridgePlugin to NpcPlugin (#538):
game logic that depends on NPC-layer resources now registers where it
belongs. BridgePlugin retains only wire protocol concerns.

Implement DialogueResponse verb handler (#539): new process_dialogue_response
system runs the full D-028 four-layer pipeline to select follow-up lines
when the player picks a dialogue option. Clears ActiveDialogue when no
candidates remain. Fix latent schedule ambiguity — emit_observation_events
now has explicit .before(advance_tick) constraint.

Verify trust-gated gossip pipeline (#171): confirmed process_talk_interaction
correctly passes KnowledgeConfidence through relationship_to_trust() per
D-075. Added integration tests for Secret-tier access (Friendly+KnowsDetails)
and Surface-only fallback (Friendly+Suspects).

Wire DialogueCooldownTracker into selection (#338): added regression test
confirming no line_id repeats within the 600-tick cooldown window across
10 consecutive Talk interactions.

Closes #538, #539, #171, #338

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-23 20:12:35 +01:00
Author
Owner
Copy Link
Copy Source

PR Review: server -> main (code)

Hoshe (Code Quality): REQUEST_CHANGES

# File Severity Issue
1 dialogue.rs:~912-971 warning process_dialogue_response never clears/updates ActiveDialogue on follow-up — stale started_tick persists
2 dialogue.rs:325-339 warning Weighted selection fallback is unreachable but would silently misbehave if score_line ever returns 0
3 serialization.rs:176-181 warning assert!(false, ...) should be panic!() — triggers clippy lint
4 serialization.rs:90-127 suggestion SetFacing and TeleportToHub missing from roundtrip test
5 dialogue.rs:2466-2469 suggestion setup_dialogue_response_world is trivial pass-through with unused mut
6 input.rs:476-509 suggestion handle_dialogue_response has no range check (intentional but undocumented)

Tyre (Architecture): REQUEST_CHANGES

# File Severity Issue
1 dialogue.rs:355-577, 797-973 warning ~60 lines duplicated between process_talk_interaction and process_dialogue_response — extract shared run_dialogue_pipeline helper
2 dialogue.rs:78-105 warning DialogueCooldownTracker is per-player-global, not per-NPC — role-pool line IDs will collide across NPCs
3 dialogue.rs:669-682 warning Hardcoded CONFRONTATION_LINES bypasses D-028/D-035 content pipeline
4 dialogue.rs:797-973 suggestion process_dialogue_response doesn't update InteractionMemory — multi-turn convos invisible to history
5 types.rs:383-386 suggestion No DialogueResponse fixture for cross-language GDScript testing
6 input.rs:476-508 suggestion Missing range check on DialogueResponse (D-010 info boundary gap)

Verdict: CHANGES REQUESTED (6 warnings across both reviewers)

## PR Review: server -> main (code) ### Hoshe (Code Quality): REQUEST_CHANGES | # | File | Severity | Issue | |---|------|----------|-------| | 1 | dialogue.rs:~912-971 | warning | `process_dialogue_response` never clears/updates `ActiveDialogue` on follow-up — stale `started_tick` persists | | 2 | dialogue.rs:325-339 | warning | Weighted selection fallback is unreachable but would silently misbehave if `score_line` ever returns 0 | | 3 | serialization.rs:176-181 | warning | `assert!(false, ...)` should be `panic!()` — triggers clippy lint | | 4 | serialization.rs:90-127 | suggestion | `SetFacing` and `TeleportToHub` missing from roundtrip test | | 5 | dialogue.rs:2466-2469 | suggestion | `setup_dialogue_response_world` is trivial pass-through with unused `mut` | | 6 | input.rs:476-509 | suggestion | `handle_dialogue_response` has no range check (intentional but undocumented) | ### Tyre (Architecture): REQUEST_CHANGES | # | File | Severity | Issue | |---|------|----------|-------| | 1 | dialogue.rs:355-577, 797-973 | warning | ~60 lines duplicated between `process_talk_interaction` and `process_dialogue_response` — extract shared `run_dialogue_pipeline` helper | | 2 | dialogue.rs:78-105 | warning | `DialogueCooldownTracker` is per-player-global, not per-NPC — role-pool line IDs will collide across NPCs | | 3 | dialogue.rs:669-682 | warning | Hardcoded `CONFRONTATION_LINES` bypasses D-028/D-035 content pipeline | | 4 | dialogue.rs:797-973 | suggestion | `process_dialogue_response` doesn't update `InteractionMemory` — multi-turn convos invisible to history | | 5 | types.rs:383-386 | suggestion | No `DialogueResponse` fixture for cross-language GDScript testing | | 6 | input.rs:476-508 | suggestion | Missing range check on `DialogueResponse` (D-010 info boundary gap) | ### Verdict: CHANGES REQUESTED (6 warnings across both reviewers)
jpmschweitzer added 1 commit 2026-02-23 20:20:06 +01:00
fix(simulation): address PR #56 review — pipeline extraction, ActiveDialogue, range check 2189a9addd 
Tyre #1: Extract run_dialogue_pipeline() shared helper — eliminates ~60
lines of duplication between process_talk_interaction and
process_dialogue_response (L1-L4 pipeline).

Hoshe #1: process_dialogue_response now updates ActiveDialogue with
current tick on follow-up selection — prevents stale started_tick.

Tyre #4: process_dialogue_response now updates InteractionMemory on
follow-up — multi-turn conversations are visible in history.

Hoshe #6 / Tyre #6: handle_dialogue_response adds server-side range
check (CLOSE_RANGE), matching Talk/Confront pattern (D-010 info
boundary).

Hoshe #2: Weighted selection fallback replaced with unreachable!() —
score_line always returns >= 1, so the fallback was dead code.

Hoshe #3: assert!(false, ...) → panic!() in serialization.rs (clippy).

Hoshe #4: SetFacing and TeleportToHub added to roundtrip test.

Hoshe #5: setup_dialogue_response_world inlined (trivial pass-through).

Tyre #2: Doc comment on DialogueCooldownTracker explains per-player-global
design choice (line IDs are NPC-scoped per D-035, no collision risk).

Tyre #3: CONFRONTATION_LINES comment updated with TODO for D-028/D-035
migration.

Tyre #5: DialogueResponse fixture added for cross-language GDScript
testing (input_dialogue_response.msgpack).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer closed this pull request 2026-02-23 20:25:45 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
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
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#56