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

fix(simulation): address PR #56 review — pipeline extraction, ActiveDialogue, range check

Browse Source 
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>
This commit is contained in:
jpmschweitzer
2026-02-23 20:20:00 +01:00
co-authored by Claude Opus 4.6
parent efa2dc543f
commit 2189a9addd
5 changed files with 151 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/tests/fixtures/msgpack/input_dialogue_response.msgpack
Vendored
+1
View File
@@ -0,0 +1 @@
うtickﾍ,ｦaction�DialogueResponseげtarget_entity_id*ｫresponse_idｰkael-davan_d_001
server/src/simulation/dialogue.rs
+109 -112
View File
@@ -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<String, u64>, // 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<Mood>,
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<Topic> = 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::<TalkRequest>();
return;
}
// Layer 4: Topic + mood weighted selection
let npc_mood = mood_opt.map(|m| m.0);
let active_topics: Vec<Topic> = 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<PlayerCharacter>,
>,
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<Topic> = 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::<ActiveDialogue>();
}
}
}
// ---------------------------------------------------------------------------
@@ -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();
server/src/simulation/input.rs
+23 -1
View File
@@ -288,6 +288,7 @@ pub fn process_player_input(
&mut commands,
&registry,
&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<PlayerCharacter>,
>,
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 {
server/tests/gen_fixtures.rs
+13
View File
@@ -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),
server/tests/serialization.rs
+5 -2
View File
@@ -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::<u64>(&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
);