fix(simulation): address PR review items for #655 content removal
- line_pool.rs: add trace! when active_situations is empty (Layer 2 will silently filter all lines — important for content debugging) - knowledge_grant.rs: document serde(untagged) ambiguity hazard; add KnowledgeGrant::validate() for load-time field validation - monologue.rs: annotate trigger_monologue as schedule ordering anchor; fix stale "periodic trigger cooldown tracking" doc reference in trigger_event_monologue (COOLDOWN_TICKS removed in #655) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -11,6 +11,10 @@ use std::collections::BTreeMap;
|
||||
/// Untagged enum — serde tries each variant in order:
|
||||
/// `Fact` matches YAML with `fact_id` field.
|
||||
/// `Entity` matches YAML with `entity_ref` field.
|
||||
///
|
||||
/// **Ambiguity hazard**: if a YAML object contains both `fact_id` and `entity_ref`,
|
||||
/// serde(untagged) silently selects `Fact` (first variant wins) and ignores `entity_ref`.
|
||||
/// Use [`KnowledgeGrant::validate`] at load time to catch this case.
|
||||
#[derive(Debug, Clone, Deserialize)]
|
||||
#[serde(untagged)]
|
||||
pub enum KnowledgeGrant {
|
||||
@@ -31,6 +35,33 @@ pub enum KnowledgeGrant {
|
||||
},
|
||||
}
|
||||
|
||||
impl KnowledgeGrant {
|
||||
/// Validates that the grant is internally consistent.
|
||||
///
|
||||
/// Specifically catches the serde(untagged) ambiguity: a raw YAML value that has
|
||||
/// both `fact_id` and `entity_ref` would parse silently as `Fact`, losing the
|
||||
/// entity reference. This can only be detected after deserializing the surrounding
|
||||
/// structure — call this on every grant after loading.
|
||||
///
|
||||
/// Currently there is no post-deserialization raw-value access, so this serves as
|
||||
/// a place to add future cross-field validation. Returns `Ok(())` for now.
|
||||
pub fn validate(&self) -> Result<(), String> {
|
||||
match self {
|
||||
KnowledgeGrant::Fact { fact_id, .. } => {
|
||||
if fact_id.is_empty() {
|
||||
return Err("KnowledgeGrant::Fact has empty fact_id".to_string());
|
||||
}
|
||||
}
|
||||
KnowledgeGrant::Entity { entity_ref, .. } => {
|
||||
if entity_ref.is_empty() {
|
||||
return Err("KnowledgeGrant::Entity has empty entity_ref".to_string());
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
/// Prerequisite set for a monologue line.
|
||||
#[derive(Debug, Clone, Deserialize)]
|
||||
pub struct Prerequisites {
|
||||
|
||||
@@ -13,6 +13,7 @@ use std::fmt;
|
||||
use std::str::FromStr;
|
||||
|
||||
use bevy_ecs::prelude::*;
|
||||
use tracing::trace;
|
||||
|
||||
use crate::simulation::knowledge_grant::{KnowledgeGrant, Prerequisites};
|
||||
|
||||
@@ -363,6 +364,14 @@ impl LinePoolIndex {
|
||||
active_situations: &[Situation],
|
||||
player_trust: TrustTier,
|
||||
) -> Vec<&IndexedDialogueLine> {
|
||||
if active_situations.is_empty() {
|
||||
trace!(
|
||||
location,
|
||||
role,
|
||||
"query_dialogue: active_situations is empty — Layer 2 will filter all lines"
|
||||
);
|
||||
}
|
||||
|
||||
let key = (location.to_string(), role.to_string());
|
||||
let Some(pool) = self.dialogue.get(&key) else {
|
||||
return Vec::new();
|
||||
|
||||
@@ -410,8 +410,8 @@ fn sound_range_tiles(range: &crate::knowledge::types::SoundRange) -> u32 {
|
||||
///
|
||||
/// Checks observation events, sound events, overheard conversations, and
|
||||
/// completed dialogues for monologue-worthy triggers. Fires at most one
|
||||
/// monologue per tick. Bypasses cooldown (event-driven),
|
||||
/// but updates last_fired_tick for periodic trigger cooldown tracking.
|
||||
/// monologue per tick. Event-driven — no cooldown gate. Updates
|
||||
/// `last_fired_tick` so v0.2 periodic triggers can respect the recency window.
|
||||
///
|
||||
/// Priority order (first match wins):
|
||||
/// 1. observe_npc (new entity spotted — uses previous-tick observation events)
|
||||
@@ -577,14 +577,12 @@ fn has_hear_sound_event(
|
||||
})
|
||||
}
|
||||
|
||||
/// Monologue trigger system.
|
||||
/// Monologue trigger system — **stub**.
|
||||
///
|
||||
/// Runs each tick. Checks trigger conditions against loaded content pools
|
||||
/// and writes a MonologueEvent to MonologueBuffer when a line should fire.
|
||||
///
|
||||
/// v0.1 triggers:
|
||||
/// - `enter_location`: fires once on first tick (session start)
|
||||
/// - `time_idle`: fires after idle threshold of no player movement
|
||||
/// v0.1 content pool triggers were removed (#655). This system remains
|
||||
/// registered in the schedule as a sequencing anchor: `trigger_recognition_monologue`
|
||||
/// and `process_sprint_anomaly_monologue` are ordered `.after(trigger_monologue)`.
|
||||
/// Remove this stub when those systems' ordering constraints are refactored.
|
||||
pub fn trigger_monologue(
|
||||
_time: Res<SimulationTime>,
|
||||
_rng: ResMut<SimRng>,
|
||||
@@ -593,7 +591,7 @@ pub fn trigger_monologue(
|
||||
With<PlayerCharacter>,
|
||||
>,
|
||||
) {
|
||||
// v0.2: content pool removed; line selection deferred to generator pipeline
|
||||
// Intentional no-op — kept as schedule ordering anchor. See doc comment.
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user