fix(simulation): address PR #37 review — doc corrections, race fix, marker cleanup
Hoshe review (4 items): - types.rs: doc comment "Current: 6" → "Current: 9" - dialogue.rs: walk-away doc duplicated numbering (items 4-5 were 2-3) - test_world/mod.rs: comment "Reset plates at 49-51" → "49-55" - content_scaling.rs: magic number 51 → constants::RESET_PLATE_STABLE_IDS.1 Tyre review (3 items): - knowledge/types.rs: guard comments on decrement() floor at Hostile - input.rs: TeleportToHub now clears ConfrontationDelivered marker - content_scaling.rs: same magic number fix (covered above) Additional: - content_runtime.rs: barrier-based shutdown handshake fixes TCP RST race condition under parallel test execution - dialogue_room.rs: clippy type_complexity allow on NPCS tuple array Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -31,7 +31,7 @@ pub const PROTOCOL_VERSION: u8 = 9;
|
||||
/// Future fields: ambient sound events, HUD state (D-020 expansion).
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct ObserverSnapshot {
|
||||
/// Protocol version for forward compatibility. Current: 6.
|
||||
/// Protocol version for forward compatibility. Current: 9.
|
||||
pub version: u8,
|
||||
/// Simulation tick when this snapshot was produced
|
||||
pub tick: u64,
|
||||
|
||||
@@ -134,14 +134,14 @@ impl RelationshipState {
|
||||
///
|
||||
/// Friendly → Known → PersonOfInterest → Hostile.
|
||||
/// Unknown stays Unknown (can't confront a stranger meaningfully).
|
||||
/// Hostile stays Hostile (already worst state).
|
||||
/// Hostile stays Hostile — floor, does not wrap or panic.
|
||||
pub fn decrement(self) -> Self {
|
||||
match self {
|
||||
Self::Friendly => Self::Known,
|
||||
Self::Known => Self::PersonOfInterest,
|
||||
Self::PersonOfInterest => Self::Hostile,
|
||||
Self::Unknown => Self::Unknown,
|
||||
Self::Hostile => Self::Hostile,
|
||||
Self::Unknown => Self::Unknown, // no-op: can't confront a stranger
|
||||
Self::Hostile => Self::Hostile, // floor: already worst state
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -498,8 +498,8 @@ pub fn process_talk_interaction(
|
||||
/// 1. Target NPC shifts to AnimationTier::Tier2 (D-047 ambiguous animation)
|
||||
/// 2. NPC routine deviation recorded (storyteller hook)
|
||||
/// 3. Emits IncompleteInteraction knowledge event (recorded in KG)
|
||||
/// 2. Clears ActiveDialogue state
|
||||
/// 3. Removes the WalkAwayRequest marker
|
||||
/// 4. Clears ActiveDialogue state
|
||||
/// 5. Removes the WalkAwayRequest marker
|
||||
///
|
||||
/// If no ActiveDialogue is present, removes WalkAwayRequest silently (no-op).
|
||||
///
|
||||
|
||||
@@ -629,12 +629,13 @@ fn handle_teleport_to_hub(
|
||||
// Clear any pending movement
|
||||
commands.entity(player_entity).remove::<MoveIntent>();
|
||||
|
||||
// Clear dialogue/interaction markers
|
||||
// Clear dialogue/interaction markers (including mid-confrontation state)
|
||||
commands
|
||||
.entity(player_entity)
|
||||
.remove::<crate::simulation::dialogue::TalkRequest>()
|
||||
.remove::<crate::simulation::dialogue::ActiveDialogue>()
|
||||
.remove::<crate::simulation::dialogue::WalkAwayRequest>();
|
||||
.remove::<crate::simulation::dialogue::WalkAwayRequest>()
|
||||
.remove::<crate::simulation::dialogue::ConfrontationDelivered>();
|
||||
|
||||
tracing::info!(
|
||||
x = hub_spawn.x,
|
||||
|
||||
@@ -414,7 +414,7 @@ mod tests {
|
||||
assert!(registry.to_entity(&StableId(id)).is_some(), "Crowd Plaza at StableId {}", id);
|
||||
}
|
||||
|
||||
// Reset plates at 49-51
|
||||
// Reset plates at 49-55
|
||||
for id in constants::RESET_PLATE_STABLE_IDS.0..=constants::RESET_PLATE_STABLE_IDS.1 {
|
||||
assert!(
|
||||
registry.to_entity(&StableId(id)).is_some(),
|
||||
|
||||
@@ -28,6 +28,7 @@ const ORIGIN_X: i32 = 36;
|
||||
const ORIGIN_Y: i32 = 104;
|
||||
|
||||
/// NPC definitions: (name, rel_x, rel_y, want_kind, intensity, contentment, stress, threshold, location, role).
|
||||
#[allow(clippy::type_complexity)]
|
||||
const NPCS: &[(&str, i32, i32, WantKind, u8, i16, i16, i16, &str, &str)] = &[
|
||||
(
|
||||
"npc_dialogue_a", 8, 8, WantKind::Connection, 4, 20, 10, 60,
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
use bevy_app::prelude::*;
|
||||
use std::net::{TcpListener, TcpStream};
|
||||
use std::path::PathBuf;
|
||||
use std::sync::{Arc, Barrier};
|
||||
use std::thread;
|
||||
use std::time::Duration;
|
||||
|
||||
@@ -51,6 +52,11 @@ fn content_runtime_boot_tick_10_snapshot() {
|
||||
let listener = TcpListener::bind("127.0.0.1:0").expect("bind listener");
|
||||
let server_addr = listener.local_addr().expect("get local addr");
|
||||
|
||||
// Barrier keeps the server thread alive until the client has finished
|
||||
// reading all snapshots, preventing a TCP RST race under parallel execution.
|
||||
let barrier = Arc::new(Barrier::new(2));
|
||||
let server_barrier = barrier.clone();
|
||||
|
||||
// Server thread: full plugin stack with real content
|
||||
let server_handle = thread::spawn(move || {
|
||||
let bridge = TcpBridge::accept_on(listener).expect("accept connection");
|
||||
@@ -96,6 +102,9 @@ fn content_runtime_boot_tick_10_snapshot() {
|
||||
for _ in 0..10 {
|
||||
app.update();
|
||||
}
|
||||
|
||||
// Wait for client to finish reading before dropping the TCP socket
|
||||
server_barrier.wait();
|
||||
});
|
||||
|
||||
// Client: connect with read timeout and receive 10 snapshots
|
||||
@@ -129,6 +138,9 @@ fn content_runtime_boot_tick_10_snapshot() {
|
||||
drop(reader);
|
||||
drop(writer);
|
||||
|
||||
// Signal server thread that client is done reading
|
||||
barrier.wait();
|
||||
|
||||
// Server thread must not have panicked
|
||||
server_handle
|
||||
.join()
|
||||
|
||||
@@ -233,20 +233,21 @@ fn extra_npcs_dont_affect_baseline_behavior() {
|
||||
assert_eq!(bp.x, sp.x, "player x should match");
|
||||
assert_eq!(bp.y, sp.y, "player y should match");
|
||||
|
||||
// Original entities (entity_id <= 51) visible in baseline should still be
|
||||
// visible in scaled run. Extra NPCs may add to the visible set, but
|
||||
// shouldn't remove baseline visibility.
|
||||
// Original entities (entity_id <= max Gauntlet StableId) visible in baseline
|
||||
// should still be visible in scaled run. Extra NPCs may add to the visible
|
||||
// set, but shouldn't remove baseline visibility.
|
||||
let max_baseline_id = settled_reach_server::test_world::constants::RESET_PLATE_STABLE_IDS.1;
|
||||
let baseline_original_ids: Vec<u64> = baseline_snap
|
||||
.entities
|
||||
.iter()
|
||||
.filter(|e| e.entity_id <= 51)
|
||||
.filter(|e| e.entity_id <= max_baseline_id)
|
||||
.map(|e| e.entity_id)
|
||||
.collect();
|
||||
|
||||
let scaled_original_ids: Vec<u64> = scaled_snap
|
||||
.entities
|
||||
.iter()
|
||||
.filter(|e| e.entity_id <= 51)
|
||||
.filter(|e| e.entity_id <= max_baseline_id)
|
||||
.map(|e| e.entity_id)
|
||||
.collect();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user