From efa2dc543f0d1f52e97dc90ab5243f115079c6bd Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 23 Feb 2026 20:01:39 +0100 Subject: [PATCH 1/2] =?UTF-8?q?feat(simulation):=20sprint=2016=20dialogue?= =?UTF-8?q?=20server=20=E2=80=94=20response=20handler,=20trust=20gossip,?= =?UTF-8?q?=20variety=20tracker?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- server/Cargo.lock | 2 +- server/src/bridge/mod.rs | 11 +- server/src/bridge/types.rs | 7 + server/src/npc/mod.rs | 12 + server/src/simulation/dialogue.rs | 681 ++++++++++++++++++++++++++++++ server/src/simulation/input.rs | 53 ++- server/tests/serialization.rs | 4 + 7 files changed, 760 insertions(+), 10 deletions(-) diff --git a/server/Cargo.lock b/server/Cargo.lock index b31b7ea23..6b618aa1d 100644 --- a/server/Cargo.lock +++ b/server/Cargo.lock @@ -1092,7 +1092,7 @@ dependencies = [ [[package]] name = "settled-reach-server" -version = "0.1.14" +version = "0.1.15" dependencies = [ "bevy_app", "bevy_ecs", diff --git a/server/src/bridge/mod.rs b/server/src/bridge/mod.rs index 83c872f20..76783ea3d 100644 --- a/server/src/bridge/mod.rs +++ b/server/src/bridge/mod.rs @@ -178,13 +178,6 @@ impl Plugin for BridgePlugin { .after(crate::simulation::sound::collect_sound_events) .after(crate::simulation::conversation::run_npc_conversations) .after(crate::simulation::dialogue::process_walk_away), - crate::simulation::dialogue::process_talk_interaction - .after(crate::simulation::input::process_player_input), - crate::simulation::dialogue::process_walk_away - .after(crate::simulation::input::process_player_input) - .after(crate::simulation::dialogue::process_talk_interaction), - crate::simulation::dialogue::process_confrontation_response - .after(crate::simulation::input::process_player_input), crate::simulation::follow::update_follow_state .after(crate::perception::observer::compute_visibility_geometry) .after(crate::simulation::movement::validate_movement) @@ -195,9 +188,11 @@ impl Plugin for BridgePlugin { .after(crate::simulation::monologue::trigger_event_monologue) .after(crate::simulation::dialogue::process_talk_interaction) .after(crate::simulation::dialogue::process_confrontation_response) + .after(crate::simulation::dialogue::process_dialogue_response) .before(crate::simulation::time::advance_tick), crate::perception::observation::emit_observation_events - .after(crate::perception::observer::compute_observer_snapshot), + .after(crate::perception::observer::compute_observer_snapshot) + .before(crate::simulation::time::advance_tick), send_bridge_snapshot .after(crate::perception::observer::compute_observer_snapshot), ), diff --git a/server/src/bridge/types.rs b/server/src/bridge/types.rs index 8fd47d332..8c2697358 100644 --- a/server/src/bridge/types.rs +++ b/server/src/bridge/types.rs @@ -377,6 +377,13 @@ pub enum PlayerAction { /// Clears dialogue, monologue, and interaction buffers. /// Rejected with a log warning on non-Gauntlet maps. TeleportToHub, + /// Player chose a dialogue option (#539, D-028 follow-up). + /// response_id is the line_id that was displayed; target_entity_id is the NPC's wire ID. + /// Server runs the same 4-layer pipeline to select a follow-up line. + DialogueResponse { + target_entity_id: u64, + response_id: String, + }, } impl PlayerAction { diff --git a/server/src/npc/mod.rs b/server/src/npc/mod.rs index 5e0188613..f049c45e8 100644 --- a/server/src/npc/mod.rs +++ b/server/src/npc/mod.rs @@ -27,6 +27,7 @@ impl Plugin for NpcPlugin { fn build(&self, app: &mut App) { app.init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -42,6 +43,7 @@ impl Plugin for NpcPlugin { .after(crate::simulation::dialogue::process_talk_interaction) .after(crate::simulation::dialogue::process_walk_away) .after(crate::simulation::dialogue::process_confrontation_response) + .after(crate::simulation::dialogue::process_dialogue_response) .before(crate::simulation::time::advance_tick), relationships::update_relationship_dynamics .after(relationships::update_trust) @@ -59,6 +61,16 @@ impl Plugin for NpcPlugin { .after(mood::update_mood) .after(routine::detect_routine_deviation) .before(crate::perception::observer::compute_observer_snapshot), + crate::simulation::dialogue::process_talk_interaction + .after(crate::simulation::input::process_player_input), + crate::simulation::dialogue::process_walk_away + .after(crate::simulation::input::process_player_input) + .after(crate::simulation::dialogue::process_talk_interaction), + crate::simulation::dialogue::process_confrontation_response + .after(crate::simulation::input::process_player_input), + crate::simulation::dialogue::process_dialogue_response + .after(crate::simulation::input::process_player_input) + .after(crate::simulation::dialogue::process_talk_interaction), ), ); diff --git a/server/src/simulation/dialogue.rs b/server/src/simulation/dialogue.rs index 18e35edcd..a542af343 100644 --- a/server/src/simulation/dialogue.rs +++ b/server/src/simulation/dialogue.rs @@ -116,6 +116,16 @@ pub struct ActiveDialogue { pub started_tick: u64, } +/// Marker: player submitted a dialogue response this tick (#539). +/// +/// Set by process_player_input when PlayerAction::DialogueResponse is received. +/// Consumed and removed by process_dialogue_response each tick. +#[derive(Component, Debug)] +pub struct DialogueResponseRequest { + pub target: Entity, + pub response_id: String, +} + /// Marker: player walked away during active dialogue this tick (D-064). /// /// Set by process_player_input when PlayerAction::WalkAway is received. @@ -767,6 +777,201 @@ pub fn process_confrontation_response( .remove::(); } +// --------------------------------------------------------------------------- +// System: process_dialogue_response (#539) +// --------------------------------------------------------------------------- + +/// Process DialogueResponse actions through the full D-028 four-layer pipeline (#539). +/// +/// Called when the player picks a dialogue option. Re-runs the same pipeline as +/// process_talk_interaction to select a follow-up line. Clears ActiveDialogue if +/// no candidates remain after cooldown filtering (conversation ends naturally). +/// +/// The response_id is the line_id that was shown; it's already on cooldown from +/// process_talk_interaction, ensuring the follow-up is a different line. +/// +/// System ordering: after process_player_input, after process_talk_interaction, +/// before compute_observer_snapshot. +#[tracing::instrument(level = "debug", skip_all)] +#[allow(clippy::type_complexity, clippy::too_many_arguments)] +pub fn process_dialogue_response( + mut commands: Commands, + time: Res, + line_pool: Option>, + registry: Res, + mut rng: ResMut, + mut trust_queue: ResMut, + mut player_query: Query< + ( + Entity, + &KnowledgeGraph, + &DialogueResponseRequest, + &mut DialogueResponseBuffer, + &mut DialogueCooldownTracker, + Option<&ActiveDialogue>, + ), + With, + >, + npc_query: Query<( + &DialogueProfile, + Option<&CurrentMood>, + Option<&NpcName>, + Option<&NpcColorIndex>, + )>, +) { + let Ok(( + player_entity, + observer_kg, + response_req, + mut response_buffer, + mut cooldown, + active_dialogue_opt, + )) = player_query.single_mut() + else { + return; + }; + + let target = response_req.target; + let response_id = response_req.response_id.clone(); + + // Always remove the marker regardless of outcome — request is consumed this tick. + commands + .entity(player_entity) + .remove::(); + + let Some(line_pool) = line_pool else { return }; + + // Look up NPC dialogue profile, mood, name, and color + let Ok((profile, mood_opt, npc_name_opt, color_idx_opt)) = npc_query.get(target) else { + tracing::debug!( + "DialogueResponse target {:?} has no DialogueProfile — cannot select follow-up", + target + ); + return; + }; + + // Resolve target's StableId for KG lookup + let target_stable = registry.to_stable(target); + let relationship = target_stable + .map(|sid| observer_kg.relationship_with(&sid)) + .unwrap_or(RelationshipState::Unknown); + + // Layer 1: Access tiers from relationship + let access_tiers = available_access_tiers(relationship); + + // Layer 2: Derive active situations from game state + let situations = derive_situations(time.day_phase(), relationship); + + // Layer 3: Trust tier from relationship + confidence (D-075) + let confidence = target_stable + .and_then(|sid| observer_kg.confidence_of(&sid)) + .unwrap_or(crate::knowledge::types::KnowledgeConfidence::Suspects); + let trust = relationship_to_trust(relationship, confidence); + + // Query Layers 1-3: collect candidates across all available access tiers + let mut candidates: Vec<&IndexedDialogueLine> = Vec::new(); + let mut seen_ids: BTreeSet<&str> = BTreeSet::new(); + + for access in &access_tiers { + let results = line_pool.0.query_dialogue( + &profile.location, + &profile.role, + *access, + &situations, + trust, + ); + for line in results { + if seen_ids.insert(&line.id) { + candidates.push(line); + } + } + } + + tracing::debug!( + response_id = response_id.as_str(), + candidate_count = candidates.len(), + "DialogueResponse: running follow-up pipeline" + ); + + // Layer 4: Topic + mood weighted selection + let npc_mood = mood_opt.map(|m| m.0); + let active_topics: Vec = Vec::new(); // v0.1: no topic context + + // Prune old cooldown entries + cooldown.prune(time.tick); + + let selected = select_dialogue_line( + &candidates, + npc_mood, + &active_topics, + &cooldown, + time.tick, + &mut rng.rng, + ); + + if let Some(line) = selected { + let Some(speaker_stable) = registry.to_stable(target) else { + tracing::warn!( + "DialogueResponse target {:?} not in EntityRegistry — skipping follow-up", + target + ); + return; + }; + + let speaker_display_name = { + let known = observer_kg + .entity_knowledge(&speaker_stable) + .map(|e| e.known_attributes.contains_key("name")) + .unwrap_or(false); + if known { + npc_name_opt + .map(|n| n.0.clone()) + .unwrap_or_else(|| "Unknown".to_string()) + } else { + display_label_for_role(&profile.role) + } + }; + let speaker_color = color_idx_opt.map(|c| c.0).unwrap_or(0u8); + + response_buffer.response = Some(DialogueResponseEvent { + line_id: line.id.clone(), + text: line.text.clone(), + speaker_entity_id: speaker_stable.0, + speaker_color_index: speaker_color, + speaker_name: speaker_display_name, + }); + + cooldown.record(&line.id, time.tick); + + // Trust progression: follow-up dialogue warms the NPC + trust_queue.push(TrustEvent::TalkCompleted { + npc: target, + player: player_entity, + }); + + tracing::debug!( + "Follow-up selected: id={}, response_id={}, location={}, role={}", + line.id, + response_id, + profile.location, + profile.role, + ); + } else { + // No follow-up lines — conversation ends naturally (D-062: invisible locks) + tracing::debug!( + "No follow-up lines for response_id={} at {}/{} — ending conversation", + response_id, + profile.location, + profile.role, + ); + // Clear active dialogue state + if active_dialogue_opt.is_some() { + commands.entity(player_entity).remove::(); + } + } + +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -1787,6 +1992,304 @@ mod tests { use rand::SeedableRng; + // -- Trust-gated gossip tests (#171, D-075) ---------------------------------- + + /// Build a pool with a Surface-tier Public line and a Secret-tier Insider line. + /// Used to verify that KnowledgeConfidence gates Secret access correctly. + fn build_trust_tier_pool() -> LinePoolIndex { + let mut index = LinePoolIndex::default(); + let pool = IndexedDialoguePool { + location: "the-terminal".to_string(), + role: "dock-worker".to_string(), + lines: vec![ + IndexedDialogueLine { + id: "trust_surface_001".to_string(), + text: "Just another day at the terminal.".to_string(), + role: "dock-worker".to_string(), + access: vec![AccessTier::Public], + trust: TrustTier::Surface, + situation: vec![Situation::Routine], + topic: vec![], + mood: vec![], + tags: vec![], + knowledge_grant: None, + }, + IndexedDialogueLine { + id: "trust_secret_001".to_string(), + text: "The manifests don't match. You didn't hear that from me.".to_string(), + role: "dock-worker".to_string(), + access: vec![AccessTier::Insider], // requires Friendly relationship + trust: TrustTier::Secret, // requires Friendly + KnowsDetails+ + situation: vec![Situation::Routine], + topic: vec![], + mood: vec![], + tags: vec![], + knowledge_grant: None, + }, + ], + }; + index.dialogue.insert( + ("the-terminal".to_string(), "dock-worker".to_string()), + pool, + ); + index + } + + #[test] + fn trust_gated_knows_details_can_get_secret_tier_line() { + // D-075: Friendly + KnowsDetails → Secret trust tier → secret lines available. + // Spec ref: #171, D-075 "Secret: Friendly + KnowsDetails+" + use crate::knowledge::types::KnowledgeConfidence; + + let mut world = setup_dialogue_world(); + world.insert_resource(LinePoolIndexResource(build_trust_tier_pool())); + + let npc = world + .spawn(( + Npc, + TilePosition::new(5, 5, 0), + DialogueProfile { + location: "the-terminal".to_string(), + role: "dock-worker".to_string(), + }, + )) + .id(); + let npc_sid = world.resource_mut::().register(npc); + + // observe_entity → Direct; observe_entity_leaving_los → KnowsDetails + let mut kg = KnowledgeGraph::new(); + kg.observe_entity(npc_sid, TilePosition::new(5, 5, 0), 0); + kg.observe_entity_leaving_los(&npc_sid, 1); + kg.set_relationship(&npc_sid, RelationshipState::Friendly); + + assert_eq!( + kg.confidence_of(&npc_sid), + Some(KnowledgeConfidence::KnowsDetails), + "precondition: KG must have KnowsDetails confidence" + ); + + let player = world + .spawn(( + PlayerCharacter, + TilePosition::new(5, 6, 0), + kg, + TalkRequest { target: npc }, + DialogueResponseBuffer::default(), + DialogueCooldownTracker::default(), + )) + .id(); + world.resource_mut::().register(player); + + // Run with multiple seeds — Secret-tier line must appear at least once + let mut saw_secret_line = false; + for seed in 0u64..50 { + world.get_mut::(player).unwrap().response = None; + world.entity_mut(player).insert(TalkRequest { target: npc }); + // Reset cooldown so the pool is not exhausted between iterations + world.entity_mut(player).insert(DialogueCooldownTracker::default()); + world.insert_resource(SimRng::new(seed)); + + let mut schedule = bevy_ecs::schedule::Schedule::default(); + schedule.add_systems(process_talk_interaction); + schedule.run(&mut world); + world.flush(); + + if let Some(resp) = &world + .get::(player) + .unwrap() + .response + { + if resp.line_id == "trust_secret_001" { + saw_secret_line = true; + break; + } + } + } + + assert!( + saw_secret_line, + "Friendly + KnowsDetails player should be able to access Secret-tier lines (D-075 #171)" + ); + } + + #[test] + fn trust_gated_suspects_only_gets_surface_tier() { + // D-075: Friendly + Suspects → Surface trust tier → Secret lines invisible. + // Spec ref: #171, D-075 "Surface: any relationship + any confidence" + use crate::knowledge::types::KnowledgeConfidence; + + let mut world = setup_dialogue_world(); + world.insert_resource(LinePoolIndexResource(build_trust_tier_pool())); + + let npc = world + .spawn(( + Npc, + TilePosition::new(5, 5, 0), + DialogueProfile { + location: "the-terminal".to_string(), + role: "dock-worker".to_string(), + }, + )) + .id(); + let npc_sid = world.resource_mut::().register(npc); + + // Friendly relationship but Suspects confidence → Surface trust only + let mut kg = KnowledgeGraph::new(); + kg.observe_entity(npc_sid, TilePosition::new(5, 5, 0), 0); + kg.set_relationship(&npc_sid, RelationshipState::Friendly); + // Patch down to Suspects (observe_entity sets Direct — too high) + kg.entities.get_mut(&npc_sid).unwrap().confidence = KnowledgeConfidence::Suspects; + + assert_eq!( + kg.confidence_of(&npc_sid), + Some(KnowledgeConfidence::Suspects), + "precondition: KG must have Suspects confidence" + ); + + let player = world + .spawn(( + PlayerCharacter, + TilePosition::new(5, 6, 0), + kg, + TalkRequest { target: npc }, + DialogueResponseBuffer::default(), + DialogueCooldownTracker::default(), + )) + .id(); + world.resource_mut::().register(player); + + let mut saw_secret = false; + for seed in 0u64..50 { + world.get_mut::(player).unwrap().response = None; + world.entity_mut(player).insert(TalkRequest { target: npc }); + world.entity_mut(player).insert(DialogueCooldownTracker::default()); + world.insert_resource(SimRng::new(seed)); + + let mut schedule = bevy_ecs::schedule::Schedule::default(); + schedule.add_systems(process_talk_interaction); + schedule.run(&mut world); + world.flush(); + + if let Some(resp) = &world + .get::(player) + .unwrap() + .response + { + if resp.line_id == "trust_secret_001" { + saw_secret = true; + break; + } + } + } + + assert!( + !saw_secret, + "Friendly + Suspects player must NOT access Secret-tier lines (D-075 #171)" + ); + } + + // -- Line variety regression test (#338, D-028) ------------------------------ + + /// Pool with 12 distinct Public/Surface/Routine lines for variety testing. + fn build_variety_pool() -> LinePoolIndex { + let mut index = LinePoolIndex::default(); + let lines: Vec = (1u32..=12) + .map(|n| IndexedDialogueLine { + id: format!("variety_{:03}", n), + text: format!("Line number {}.", n), + role: "dock-worker".to_string(), + access: vec![AccessTier::Public], + trust: TrustTier::Surface, + situation: vec![Situation::Routine], + topic: vec![], + mood: vec![], + tags: vec![], + knowledge_grant: None, + }) + .collect(); + let pool = IndexedDialoguePool { + location: "the-terminal".to_string(), + role: "dock-worker".to_string(), + lines, + }; + index.dialogue.insert( + ("the-terminal".to_string(), "dock-worker".to_string()), + pool, + ); + index + } + + #[test] + fn line_variety_no_repeats_within_cooldown_window() { + // #338, D-028: No line_id should repeat within LINE_COOLDOWN_TICKS. + // Regression: Talk 10 times at tick 0 (well within the 600-tick window). + // Each selected line must be distinct — cooldown tracker enforces this. + let mut world = setup_dialogue_world(); + world.insert_resource(LinePoolIndexResource(build_variety_pool())); + + let npc = world + .spawn(( + Npc, + TilePosition::new(5, 5, 0), + DialogueProfile { + location: "the-terminal".to_string(), + role: "dock-worker".to_string(), + }, + )) + .id(); + world.resource_mut::().register(npc); + + // Unknown player — Public access only; tick stays at 0 throughout + let player = world + .spawn(( + PlayerCharacter, + TilePosition::new(5, 6, 0), + KnowledgeGraph::new(), + TalkRequest { target: npc }, + DialogueResponseBuffer::default(), + DialogueCooldownTracker::default(), + )) + .id(); + world.resource_mut::().register(player); + + let mut seen_ids: Vec = Vec::new(); + + for seed in 0u64..10 { + world.get_mut::(player).unwrap().response = None; + world.entity_mut(player).insert(TalkRequest { target: npc }); + // NOTE: SimulationTime is NOT advanced — all 10 talks happen within tick 0 + world.insert_resource(SimRng::new(seed)); + + let mut schedule = bevy_ecs::schedule::Schedule::default(); + schedule.add_systems(process_talk_interaction); + schedule.run(&mut world); + world.flush(); + + if let Some(resp) = &world + .get::(player) + .unwrap() + .response + { + let id = resp.line_id.clone(); + assert!( + !seen_ids.contains(&id), + "Line '{}' was repeated within the {}-tick cooldown window (iteration {}). \ + Cooldown tracker must prevent repeats. (#338)", + id, + LINE_COOLDOWN_TICKS, + seed, + ); + seen_ids.push(id); + } + } + + assert_eq!( + seen_ids.len(), + 10, + "Should have selected 10 distinct lines across 10 consecutive Talks (#338)" + ); + } + // === Confrontation Response Tests (#520, D-063) === #[test] @@ -1957,4 +2460,182 @@ mod tests { "Marker should be removed after processing" ); } + + // === DialogueResponse Tests (#539) === + + fn setup_dialogue_response_world() -> World { + let mut world = setup_dialogue_world(); + world + } + + #[test] + fn process_dialogue_response_selects_follow_up_line() { + let mut world = setup_dialogue_response_world(); + let index = build_test_line_pool(); + world.insert_resource(LinePoolIndexResource(index)); + + let npc = world + .spawn(( + Npc, + TilePosition::new(5, 5, 0), + DialogueProfile { + location: "the-terminal".to_string(), + role: "dock-worker".to_string(), + }, + CurrentMood(Mood::Content), + )) + .id(); + world.resource_mut::().register(npc); + + let player = world + .spawn(( + PlayerCharacter, + TilePosition::new(5, 6, 0), + KnowledgeGraph::new(), + // Simulate that first line was already selected (on cooldown) + DialogueResponseRequest { + target: npc, + response_id: "test_d_001".to_string(), + }, + DialogueResponseBuffer::default(), + DialogueCooldownTracker::default(), + )) + .id(); + world.resource_mut::().register(player); + + let mut schedule = bevy_ecs::schedule::Schedule::default(); + schedule.add_systems(process_dialogue_response); + schedule.run(&mut world); + world.flush(); + + // Should have consumed the marker + assert!( + world.get::(player).is_none(), + "DialogueResponseRequest should be consumed" + ); + } + + #[test] + fn process_dialogue_response_clears_active_dialogue_when_no_lines() { + let mut world = setup_dialogue_response_world(); + + // Build pool with only one line — it will be on cooldown + let mut index = LinePoolIndex::default(); + let pool = IndexedDialoguePool { + location: "test".to_string(), + role: "worker".to_string(), + lines: vec![IndexedDialogueLine { + id: "only_line".to_string(), + text: "Only thing I can say.".to_string(), + role: "worker".to_string(), + access: vec![AccessTier::Public], + trust: TrustTier::Surface, + situation: vec![Situation::Routine], + topic: vec![], + mood: vec![], + tags: vec![], + knowledge_grant: None, + }], + }; + index + .dialogue + .insert(("test".to_string(), "worker".to_string()), pool); + world.insert_resource(LinePoolIndexResource(index)); + + let npc = world + .spawn(( + Npc, + TilePosition::new(5, 5, 0), + DialogueProfile { + location: "test".to_string(), + role: "worker".to_string(), + }, + )) + .id(); + world.resource_mut::().register(npc); + + // Set the only line on cooldown — so no follow-up can be selected + let mut cooldown = DialogueCooldownTracker::default(); + cooldown.record("only_line", 0); + + let player = world + .spawn(( + PlayerCharacter, + TilePosition::new(5, 6, 0), + KnowledgeGraph::new(), + DialogueResponseRequest { + target: npc, + response_id: "only_line".to_string(), + }, + DialogueResponseBuffer::default(), + cooldown, + ActiveDialogue { + target: npc, + interaction_type: crate::knowledge::events::InteractionType::Talk, + started_tick: 0, + }, + )) + .id(); + world.resource_mut::().register(player); + + let mut schedule = bevy_ecs::schedule::Schedule::default(); + schedule.add_systems(process_dialogue_response); + schedule.run(&mut world); + world.flush(); + + // No follow-up lines — ActiveDialogue should be cleared + assert!( + world.get::(player).is_none(), + "ActiveDialogue should be cleared when no follow-up lines available" + ); + assert!( + world.get::(player).is_none(), + "DialogueResponseRequest should be consumed" + ); + // Buffer should remain empty + let buffer = world.get::(player).unwrap(); + assert!( + buffer.response.is_none(), + "No response when all lines on cooldown" + ); + } + + #[test] + fn process_dialogue_response_no_profile_is_noop() { + let mut world = setup_dialogue_response_world(); + + // NPC without DialogueProfile + let npc = world.spawn((Npc, TilePosition::new(5, 5, 0))).id(); + world.resource_mut::().register(npc); + + let player = world + .spawn(( + PlayerCharacter, + TilePosition::new(5, 6, 0), + KnowledgeGraph::new(), + DialogueResponseRequest { + target: npc, + response_id: "some_line".to_string(), + }, + DialogueResponseBuffer::default(), + DialogueCooldownTracker::default(), + )) + .id(); + world.resource_mut::().register(player); + + let mut schedule = bevy_ecs::schedule::Schedule::default(); + schedule.add_systems(process_dialogue_response); + schedule.run(&mut world); + world.flush(); + + assert!( + world.get::(player).is_none(), + "DialogueResponseRequest consumed even with no profile" + ); + let buffer = world.get::(player).unwrap(); + assert!( + buffer.response.is_none(), + "No response for NPC without DialogueProfile" + ); + } } diff --git a/server/src/simulation/input.rs b/server/src/simulation/input.rs index 953e4531e..b91a6a0be 100644 --- a/server/src/simulation/input.rs +++ b/server/src/simulation/input.rs @@ -280,6 +280,18 @@ pub fn process_player_input( PlayerAction::TeleportToHub => { handle_teleport_to_hub(&mut player_query, &mut commands); } + PlayerAction::DialogueResponse { + target_entity_id, + ref response_id, + } => { + handle_dialogue_response( + &mut commands, + ®istry, + &player_query, + target_entity_id, + response_id, + ); + } PlayerAction::UsePerceptionMode(ref mode) => { tracing::trace!("UsePerceptionMode({}) — no-op for Sprint 1", mode); } @@ -458,6 +470,44 @@ fn handle_talk( tracing::debug!(target_id, "Talk: TalkRequest marker set on player"); } +/// Handle DialogueResponse action: set DialogueResponseRequest marker (#539). +/// The follow-up dialogue pipeline runs in process_dialogue_response (dialogue.rs). +#[allow(clippy::type_complexity)] +fn handle_dialogue_response( + commands: &mut Commands, + registry: &EntityRegistry, + player_query: &Query< + ( + Entity, + &TilePosition, + Option<&mut Stance>, + Option<&mut PlayerMoveCooldown>, + ), + With, + >, + target_entity_id: u64, + response_id: &str, +) { + let Ok((player_entity, _, _, _)) = player_query.single() else { + return; + }; + + let target_stable = StableId(target_entity_id); + let Some(target_entity) = registry.to_entity(&target_stable) else { + tracing::warn!(target_entity_id, "DialogueResponse: target entity not in registry"); + return; + }; + + commands + .entity(player_entity) + .insert(crate::simulation::dialogue::DialogueResponseRequest { + target: target_entity, + response_id: response_id.to_string(), + }); + + tracing::debug!(target_entity_id, response_id, "DialogueResponse: marker set on player"); +} + /// Handle Confront verb: set ConfrontationDelivered marker on the player entity (#520, D-063). /// The confrontation response system runs in process_confrontation_response (dialogue.rs). /// Server-side range check: Confront requires CLOSE_RANGE (same as Talk). @@ -741,7 +791,8 @@ fn handle_teleport_to_hub( .remove::() .remove::() .remove::() - .remove::(); + .remove::() + .remove::(); tracing::info!( x = hub_spawn.x, diff --git a/server/tests/serialization.rs b/server/tests/serialization.rs index f1b508086..f93478dc5 100644 --- a/server/tests/serialization.rs +++ b/server/tests/serialization.rs @@ -107,6 +107,10 @@ fn all_player_action_variants_roundtrip() { PlayerAction::ToggleStanceUp, PlayerAction::ToggleStanceDown, PlayerAction::WalkAway, + PlayerAction::DialogueResponse { + target_entity_id: 42, + response_id: "kael-davan_d_001".to_string(), + }, ]; for action in actions { From 2189a9addd05dcf6265de763b03a1fe6f8c32ea4 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 23 Feb 2026 20:20:00 +0100 Subject: [PATCH 2/2] =?UTF-8?q?fix(simulation):=20address=20PR=20#56=20rev?= =?UTF-8?q?iew=20=E2=80=94=20pipeline=20extraction,=20ActiveDialogue,=20ra?= =?UTF-8?q?nge=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../msgpack/input_dialogue_response.msgpack | 1 + server/src/simulation/dialogue.rs | 221 +++++++++--------- server/src/simulation/input.rs | 24 +- server/tests/gen_fixtures.rs | 13 ++ server/tests/serialization.rs | 7 +- 5 files changed, 151 insertions(+), 115 deletions(-) create mode 100644 client/tests/fixtures/msgpack/input_dialogue_response.msgpack diff --git a/client/tests/fixtures/msgpack/input_dialogue_response.msgpack b/client/tests/fixtures/msgpack/input_dialogue_response.msgpack new file mode 100644 index 000000000..593da0708 --- /dev/null +++ b/client/tests/fixtures/msgpack/input_dialogue_response.msgpack @@ -0,0 +1 @@ +tick,actionDialogueResponsetarget_entity_id*response_idkael-davan_d_001 \ No newline at end of file diff --git a/server/src/simulation/dialogue.rs b/server/src/simulation/dialogue.rs index a542af343..0a6b6cd42 100644 --- a/server/src/simulation/dialogue.rs +++ b/server/src/simulation/dialogue.rs @@ -79,6 +79,11 @@ impl Default for CurrentMood { /// /// Prevents the same line from being selected within LINE_COOLDOWN_TICKS. /// Entries older than the cooldown window are pruned each query. +/// +/// Design note: this is player-global, not per-NPC. Line IDs are NPC-scoped +/// per D-035 (`{template}_{d|m|e}_{###}`), so cross-NPC collisions don't occur +/// in practice. If a future sprint introduces shared line IDs across roles, +/// the key should become `(StableId, line_id)` instead. #[derive(Component, Debug, Default)] pub struct DialogueCooldownTracker { used: std::collections::BTreeMap, // line_id → tick_used (D-041) @@ -321,11 +326,9 @@ pub fn select_dialogue_line<'a>( return None; } - // Weighted random selection + // Weighted random selection — score_line always returns >= 1 (base score), + // so total_weight > 0 is guaranteed when scored is non-empty. let total_weight: u32 = scored.iter().map(|(_, s)| s).sum(); - if total_weight == 0 { - return None; - } let mut roll = rng.random_range(0..total_weight); for (line, weight) in &scored { @@ -335,8 +338,70 @@ pub fn select_dialogue_line<'a>( roll -= weight; } - // Fallback (shouldn't reach here with valid weights) - Some(scored.last().unwrap().0) + unreachable!("weighted selection with total_weight > 0 must select a line") +} + +// --------------------------------------------------------------------------- +// Shared pipeline: Layers 1-4 +// --------------------------------------------------------------------------- + +/// Run the full D-028 four-layer dialogue pipeline and return a selected line. +/// +/// Shared by `process_talk_interaction` and `process_dialogue_response` to +/// avoid duplicating the L1-L4 query + scoring logic. Callers handle the +/// result differently (initial Talk sets ActiveDialogue; follow-up may clear it). +fn run_dialogue_pipeline<'a>( + line_pool: &'a crate::content::line_pool::LinePoolIndex, + location: &str, + role: &str, + relationship: RelationshipState, + confidence: crate::knowledge::types::KnowledgeConfidence, + day_phase: crate::simulation::time::DayPhase, + interaction_mem: Option<&InteractionMemory>, + npc_mood: Option, + cooldown: &DialogueCooldownTracker, + tick: u64, + rng: &mut impl Rng, +) -> Option<&'a IndexedDialogueLine> { + // Layer 1: Access tiers from relationship + let access_tiers = available_access_tiers(relationship); + + // Layer 2: Derive active situations from game state + let mut situations = derive_situations(day_phase, relationship); + + // Layer 2 extension: first_meeting / repeated_visit from InteractionMemory (#325, D-028) + if let Some(mem) = interaction_mem { + if mem.is_first_meeting() { + situations.push(Situation::FirstMeeting); + } else if mem.is_repeated_visit() { + situations.push(Situation::RepeatedVisit); + } + } + + // Layer 3: Trust tier from relationship + confidence (D-075) + let trust = relationship_to_trust(relationship, confidence); + + // Query Layers 1-3: collect candidates across all available access tiers + let mut candidates: Vec<&IndexedDialogueLine> = Vec::new(); + let mut seen_ids: BTreeSet<&str> = BTreeSet::new(); + + for access in &access_tiers { + let results = line_pool.query_dialogue(location, role, *access, &situations, trust); + for line in results { + if seen_ids.insert(&line.id) { + candidates.push(line); + } + } + } + + if candidates.is_empty() { + return None; + } + + // Layer 4: Topic + mood weighted selection + let active_topics: Vec = Vec::new(); // v0.1: no topic context yet + + select_dialogue_line(&candidates, npc_mood, &active_topics, cooldown, tick, rng) } // --------------------------------------------------------------------------- @@ -415,73 +480,26 @@ pub fn process_talk_interaction( .map(|sid| observer_kg.relationship_with(&sid)) .unwrap_or(RelationshipState::Unknown); - // Layer 1: Access tiers from relationship - let access_tiers = available_access_tiers(relationship); - - // Layer 2: Derive active situations from game state - let mut situations = derive_situations(time.day_phase(), relationship); - - // Layer 2 extension: first_meeting / repeated_visit from InteractionMemory (#325, D-028) - if let Some(ref mem) = interaction_mem_opt { - if mem.is_first_meeting() { - situations.push(Situation::FirstMeeting); - } else if mem.is_repeated_visit() { - situations.push(Situation::RepeatedVisit); - } - } - - // Layer 3: Trust tier from relationship + confidence (D-075) // Default to Suspects for unknown NPCs — no KG entry means no basis for // deeper dialogue, which correctly yields Surface trust tier. let confidence = target_stable .and_then(|sid| observer_kg.confidence_of(&sid)) .unwrap_or(crate::knowledge::types::KnowledgeConfidence::Suspects); - let trust = relationship_to_trust(relationship, confidence); - // Query Layers 1-3: collect candidates across all available access tiers - let mut candidates: Vec<&IndexedDialogueLine> = Vec::new(); - let mut seen_ids: BTreeSet<&str> = BTreeSet::new(); - - for access in &access_tiers { - let results = line_pool.0.query_dialogue( - &profile.location, - &profile.role, - *access, - &situations, - trust, - ); - for line in results { - // Deduplicate across access tiers (BTreeSet for deterministic iteration) - if seen_ids.insert(&line.id) { - candidates.push(line); - } - } - } - - if candidates.is_empty() { - tracing::debug!( - "No dialogue lines available for {}/{} (access={:?}, situations={:?}, trust={:?})", - profile.location, - profile.role, - access_tiers, - situations, - trust, - ); - commands.entity(player_entity).remove::(); - return; - } - - // Layer 4: Topic + mood weighted selection let npc_mood = mood_opt.map(|m| m.0); - let active_topics: Vec = Vec::new(); // v0.1: no topic context yet // Prune old cooldown entries cooldown.prune(time.tick); - let selected = select_dialogue_line( - &candidates, + let selected = run_dialogue_pipeline( + &line_pool.0, + &profile.location, + &profile.role, + relationship, + confidence, + time.day_phase(), + interaction_mem_opt.as_deref(), npc_mood, - &active_topics, &cooldown, time.tick, &mut rng.rng, @@ -668,7 +686,7 @@ pub fn process_walk_away( /// Hardcoded confrontation monologue lines (D-063). /// Fired as a monologue spike when the player delivers a confrontation. -/// Future: move to content pools with trigger="confrontation_delivered". +/// TODO: move to content pools with trigger="confrontation_delivered" (D-028/D-035). const CONFRONTATION_LINES: &[(&str, &str)] = &[ ( "confront_01", @@ -812,9 +830,10 @@ pub fn process_dialogue_response( ), With, >, - npc_query: Query<( + mut npc_query: Query<( &DialogueProfile, Option<&CurrentMood>, + Option<&mut InteractionMemory>, Option<&NpcName>, Option<&NpcColorIndex>, )>, @@ -841,8 +860,10 @@ pub fn process_dialogue_response( let Some(line_pool) = line_pool else { return }; - // Look up NPC dialogue profile, mood, name, and color - let Ok((profile, mood_opt, npc_name_opt, color_idx_opt)) = npc_query.get(target) else { + // Look up NPC dialogue profile, mood, interaction history, name, and color + let Ok((profile, mood_opt, mut interaction_mem_opt, npc_name_opt, color_idx_opt)) = + npc_query.get_mut(target) + else { tracing::debug!( "DialogueResponse target {:?} has no DialogueProfile — cannot select follow-up", target @@ -856,54 +877,24 @@ pub fn process_dialogue_response( .map(|sid| observer_kg.relationship_with(&sid)) .unwrap_or(RelationshipState::Unknown); - // Layer 1: Access tiers from relationship - let access_tiers = available_access_tiers(relationship); - - // Layer 2: Derive active situations from game state - let situations = derive_situations(time.day_phase(), relationship); - - // Layer 3: Trust tier from relationship + confidence (D-075) let confidence = target_stable .and_then(|sid| observer_kg.confidence_of(&sid)) .unwrap_or(crate::knowledge::types::KnowledgeConfidence::Suspects); - let trust = relationship_to_trust(relationship, confidence); - // Query Layers 1-3: collect candidates across all available access tiers - let mut candidates: Vec<&IndexedDialogueLine> = Vec::new(); - let mut seen_ids: BTreeSet<&str> = BTreeSet::new(); - - for access in &access_tiers { - let results = line_pool.0.query_dialogue( - &profile.location, - &profile.role, - *access, - &situations, - trust, - ); - for line in results { - if seen_ids.insert(&line.id) { - candidates.push(line); - } - } - } - - tracing::debug!( - response_id = response_id.as_str(), - candidate_count = candidates.len(), - "DialogueResponse: running follow-up pipeline" - ); - - // Layer 4: Topic + mood weighted selection let npc_mood = mood_opt.map(|m| m.0); - let active_topics: Vec = Vec::new(); // v0.1: no topic context // Prune old cooldown entries cooldown.prune(time.tick); - let selected = select_dialogue_line( - &candidates, + let selected = run_dialogue_pipeline( + &line_pool.0, + &profile.location, + &profile.role, + relationship, + confidence, + time.day_phase(), + interaction_mem_opt.as_deref(), npc_mood, - &active_topics, &cooldown, time.tick, &mut rng.rng, @@ -943,12 +934,24 @@ pub fn process_dialogue_response( cooldown.record(&line.id, time.tick); + // Update ActiveDialogue with current tick — prevents stale started_tick + commands.entity(player_entity).insert(ActiveDialogue { + target, + interaction_type: crate::knowledge::events::InteractionType::Talk, + started_tick: time.tick, + }); + // Trust progression: follow-up dialogue warms the NPC trust_queue.push(TrustEvent::TalkCompleted { npc: target, player: player_entity, }); + // Interaction tracking: record follow-up as a talk event + if let Some(ref mut mem) = interaction_mem_opt { + mem.record_talk(time.tick); + } + tracing::debug!( "Follow-up selected: id={}, response_id={}, location={}, role={}", line.id, @@ -969,7 +972,6 @@ pub fn process_dialogue_response( commands.entity(player_entity).remove::(); } } - } // --------------------------------------------------------------------------- @@ -2463,14 +2465,9 @@ mod tests { // === DialogueResponse Tests (#539) === - fn setup_dialogue_response_world() -> World { - let mut world = setup_dialogue_world(); - world - } - #[test] fn process_dialogue_response_selects_follow_up_line() { - let mut world = setup_dialogue_response_world(); + let mut world = setup_dialogue_world(); let index = build_test_line_pool(); world.insert_resource(LinePoolIndexResource(index)); @@ -2517,7 +2514,7 @@ mod tests { #[test] fn process_dialogue_response_clears_active_dialogue_when_no_lines() { - let mut world = setup_dialogue_response_world(); + let mut world = setup_dialogue_world(); // Build pool with only one line — it will be on cooldown let mut index = LinePoolIndex::default(); @@ -2602,7 +2599,7 @@ mod tests { #[test] fn process_dialogue_response_no_profile_is_noop() { - let mut world = setup_dialogue_response_world(); + let mut world = setup_dialogue_world(); // NPC without DialogueProfile let npc = world.spawn((Npc, TilePosition::new(5, 5, 0))).id(); diff --git a/server/src/simulation/input.rs b/server/src/simulation/input.rs index b91a6a0be..91457a830 100644 --- a/server/src/simulation/input.rs +++ b/server/src/simulation/input.rs @@ -288,6 +288,7 @@ pub fn process_player_input( &mut commands, ®istry, &player_query, + &all_positions, target_entity_id, response_id, ); @@ -472,6 +473,10 @@ fn handle_talk( /// Handle DialogueResponse action: set DialogueResponseRequest marker (#539). /// The follow-up dialogue pipeline runs in process_dialogue_response (dialogue.rs). +/// +/// Range check: same CLOSE_RANGE as Talk/Confront (D-010 info boundary). The player +/// must still be near the NPC to continue a conversation — walking away mid-dialogue +/// should not allow remote follow-ups. #[allow(clippy::type_complexity)] fn handle_dialogue_response( commands: &mut Commands, @@ -485,10 +490,11 @@ fn handle_dialogue_response( ), With, >, + all_positions: &Query<&TilePosition>, target_entity_id: u64, response_id: &str, ) { - let Ok((player_entity, _, _, _)) = player_query.single() else { + let Ok((player_entity, player_pos, _, _)) = player_query.single() else { return; }; @@ -498,6 +504,22 @@ fn handle_dialogue_response( return; }; + // Server-side range check: reject DialogueResponse if target moved out of range + if let Ok(target_pos) = all_positions.get(target_entity) { + let distance = player_pos + .manhattan_distance(target_pos) + .unwrap_or(u32::MAX); + if distance > crate::simulation::interaction::CLOSE_RANGE { + tracing::info!( + target_entity_id, + distance, + "DialogueResponse: target out of range (max {})", + crate::simulation::interaction::CLOSE_RANGE, + ); + return; + } + } + commands .entity(player_entity) .insert(crate::simulation::dialogue::DialogueResponseRequest { diff --git a/server/tests/gen_fixtures.rs b/server/tests/gen_fixtures.rs index 51d1f03c5..86a7346a9 100644 --- a/server/tests/gen_fixtures.rs +++ b/server/tests/gen_fixtures.rs @@ -253,6 +253,19 @@ fn generate_msgpack_fixtures() { &rmp_serde::to_vec_named(&input_batch).unwrap(), ); + // PlayerInput: DialogueResponse (#539) + let input_dialogue_response = PlayerInput { + tick: 300, + action: PlayerAction::DialogueResponse { + target_entity_id: 42, + response_id: "kael-davan_d_001".to_string(), + }, + }; + write_fixture( + "input_dialogue_response", + &rmp_serde::to_vec_named(&input_dialogue_response).unwrap(), + ); + // Diagonal movement fixtures (clockwise: NE, SE, SW, NW) for (name, action) in [ ("input_move_northeast", PlayerAction::MoveNortheast), diff --git a/server/tests/serialization.rs b/server/tests/serialization.rs index f93478dc5..e3bdb38e7 100644 --- a/server/tests/serialization.rs +++ b/server/tests/serialization.rs @@ -107,6 +107,10 @@ fn all_player_action_variants_roundtrip() { PlayerAction::ToggleStanceUp, PlayerAction::ToggleStanceDown, PlayerAction::WalkAway, + PlayerAction::SetFacing { + facing: "north".to_string(), + }, + PlayerAction::TeleportToHub, PlayerAction::DialogueResponse { target_entity_id: 42, response_id: "kael-davan_d_001".to_string(), @@ -173,8 +177,7 @@ fn all_fixtures_deserialize() { rmp_serde::from_slice::(&bytes) .unwrap_or_else(|e| panic!("deserialize boundary raw fixture {}: {}", name, e)); } else { - assert!( - false, + panic!( "unknown fixture naming convention: {} — add a deserialization branch for this prefix", name );