From af8e20ab9a954a12d6e3e146f5be7b05040a6330 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 17 Feb 2026 17:40:57 +0100 Subject: [PATCH] =?UTF-8?q?fix(simulation):=20determinism=20fixes=20?= =?UTF-8?q?=E2=80=94=20BTreeSet=20ordering,=20entity=20sort,=20mover=20sor?= =?UTF-8?q?t?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace HashSet with BTreeSet for visible_ids, sort visible_tiles by coordinates, sort visible entities in snapshot by entity_id, and sort movers by Entity bits in validate_movement. Required by D-010 principle 4 (deterministic simulation). Fixes #456, #457, #458. Co-Authored-By: Claude Opus 4.6 --- server/src/perception/observer/mod.rs | 18 +++++-- server/src/perception/query.rs | 7 +-- server/src/simulation/movement.rs | 72 ++++++++++++++++++++++++--- 3 files changed, 83 insertions(+), 14 deletions(-) diff --git a/server/src/perception/observer/mod.rs b/server/src/perception/observer/mod.rs index 0adf37aa6..02a9d0d66 100644 --- a/server/src/perception/observer/mod.rs +++ b/server/src/perception/observer/mod.rs @@ -7,7 +7,7 @@ //! D-017 perception modes swap the geometry producer via PerceptionQuery trait. use bevy_ecs::prelude::*; -use std::collections::HashSet; +use std::collections::BTreeSet; use crate::bridge::types::*; use crate::knowledge::types::KnowledgeState; @@ -17,6 +17,7 @@ use crate::perception::query::{ActivePerceptionMode, VisibilityGeometry}; use crate::perception::vision_cone::Facing; use crate::simulation::interaction::NearbyInteractionBuffer; use crate::simulation::inventory::{CarriedBy, InventorySlot, ItemName}; +use crate::simulation::dialogue::DialogueResponseBuffer; use crate::simulation::monologue::{MonologueBuffer, SprintAnomalyQueue}; use crate::simulation::movement::{PlayerCharacter, TilePosition, WalkabilityMap}; use crate::simulation::stance::Stance; @@ -66,6 +67,7 @@ pub fn compute_observer_snapshot( Option<&CharacterArchetype>, Option<&mut SprintAnomalyQueue>, Option<&CognitiveDelay>, + Option<&mut DialogueResponseBuffer>, ), With, >, @@ -89,6 +91,7 @@ pub fn compute_observer_snapshot( archetype_opt, mut anomaly_queue_opt, cognitive_delay_opt, + mut dialogue_response_opt, )) = observer_query.single_mut() else { tracing::error!("compute_observer_snapshot: PlayerCharacter query failed"); @@ -164,6 +167,7 @@ pub fn compute_observer_snapshot( ); let current_monologue = monologue_buffer.take(); + let dialogue_response = dialogue_response_opt.as_mut().and_then(|buf| buf.take()); // Build pending recognitions from CognitiveDelay (#423, D-060) let pending_recognitions = cognitive_delay_opt @@ -186,6 +190,9 @@ pub fn compute_observer_snapshot( }) .unwrap_or_default(); + // Sort entities by entity_id for deterministic snapshot ordering (#457) + entities.sort_by_key(|e| e.entity_id); + buffer.snapshot = Some(ObserverSnapshot { version: crate::bridge::types::PROTOCOL_VERSION, tick: time.tick, @@ -198,6 +205,7 @@ pub fn compute_observer_snapshot( nearby_interactions, current_monologue, pending_recognitions, + dialogue_response, }); } @@ -214,9 +222,9 @@ fn filter_visible_entities( Option<&PlayerCharacter>, Option<&crate::npc::Npc>, )>, -) -> (Vec, HashSet) { +) -> (Vec, BTreeSet) { let mut entities = Vec::new(); - let mut visible_ids: HashSet = HashSet::new(); + let mut visible_ids: BTreeSet = BTreeSet::new(); for (entity, pos, is_player, is_npc) in all_entities.iter() { if pos.z != geometry.observer_z { @@ -281,8 +289,8 @@ fn filter_visible_entities( /// transient Direct-confidence inconsistencies. fn collect_remembered_entities( observer_kg: &KnowledgeGraph, - visible_ids: &HashSet, - visible_positions: &HashSet<(i32, i32)>, + visible_ids: &BTreeSet, + visible_positions: &BTreeSet<(i32, i32)>, observer_z: i32, current_tick: u64, entities: &mut Vec, diff --git a/server/src/perception/query.rs b/server/src/perception/query.rs index 078253dc8..9789f884d 100644 --- a/server/src/perception/query.rs +++ b/server/src/perception/query.rs @@ -5,7 +5,7 @@ //! provide mode-specific FOV and visibility sector computation. //! v0.1 implements only NaturalVision. -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeSet, HashMap}; use bevy_ecs::prelude::*; @@ -21,7 +21,7 @@ use crate::simulation::movement::{TilePosition, WalkabilityMap}; #[derive(Resource, Default)] pub struct VisibilityGeometry { pub visible_tiles: Vec, - pub visible_positions: HashSet<(i32, i32)>, + pub visible_positions: BTreeSet<(i32, i32)>, pub sector_lookup: HashMap<(i32, i32), VisibilitySector>, pub observer_z: i32, } @@ -64,7 +64,7 @@ impl PerceptionQuery for NaturalVision { let cone_tiles = apply_vision_cone(&fov, observer_pos.x, observer_pos.y, facing, &config); - let visible_tiles = cone_tiles + let mut visible_tiles: Vec = cone_tiles .iter() .map(|&(x, y, sector)| { let tile_kind = if walkability.can_move_to(&TilePosition::new(x, y, z)) { @@ -81,6 +81,7 @@ impl PerceptionQuery for NaturalVision { } }) .collect(); + visible_tiles.sort_by_key(|t| (t.x, t.y)); let visible_positions = cone_tiles.iter().map(|&(x, y, _)| (x, y)).collect(); diff --git a/server/src/simulation/movement.rs b/server/src/simulation/movement.rs index 7265b1e59..e0eea10fc 100644 --- a/server/src/simulation/movement.rs +++ b/server/src/simulation/movement.rs @@ -285,12 +285,19 @@ pub fn validate_movement( occupied.insert((*pos, layer), entity); } - for (entity, intent, mut position, presence) in movers.iter_mut() { - let target = &intent.target; - let layer = presence.copied().unwrap_or_default(); - let slot = (*target, layer); + // Sort movers by Entity::to_bits() for deterministic collision resolution (#458) + let mut mover_entities: Vec = movers.iter().map(|(e, _, _, _)| e).collect(); + mover_entities.sort_by_key(|e| e.to_bits()); - if !map.can_move_to(target) { + for entity in mover_entities { + let Ok((_, intent, mut position, presence)) = movers.get_mut(entity) else { + continue; + }; + let target = intent.target; + let layer = presence.copied().unwrap_or_default(); + let slot = (target, layer); + + if !map.can_move_to(&target) { tracing::trace!("Entity {:?} blocked by terrain at {:?}", entity, target); } else if occupied.contains_key(&slot) { tracing::trace!( @@ -309,7 +316,7 @@ pub fn validate_movement( ); // Free old layer slot, claim new one occupied.remove(&(*position, layer)); - *position = *target; + *position = target; occupied.insert(slot, entity); } commands.entity(entity).remove::(); @@ -868,6 +875,59 @@ mod tests { ); } + // ----------------------------------------------------------------------- + // Determinism regression test (#458 — Fix D) + // ----------------------------------------------------------------------- + + #[test] + fn same_tile_movers_resolve_by_entity_bits() { + // Fix D (#458): movers sorted by Entity::to_bits() before collision + // resolution. The entity with the lower bits value processes first + // and wins the tile. This prevents non-deterministic outcomes from + // ECS iteration order. + let mut world = bevy_ecs::world::World::new(); + world.insert_resource(WalkabilityMap::new(10, 10, 1)); + + let target = TilePosition::new(5, 5, 0); + let origin_a = TilePosition::new(5, 4, 0); + let origin_b = TilePosition::new(5, 6, 0); + + let entity_a = world + .spawn((TilePosition::new(5, 4, 0), MoveIntent { target })) + .id(); + + let entity_b = world + .spawn((TilePosition::new(5, 6, 0), MoveIntent { target })) + .id(); + + // Determine which entity has lower bits (not guaranteed by spawn order) + let (lower, higher, _lower_origin, higher_origin) = + if entity_a.to_bits() < entity_b.to_bits() { + (entity_a, entity_b, origin_a, origin_b) + } else { + (entity_b, entity_a, origin_b, origin_a) + }; + + let mut schedule = bevy_ecs::schedule::Schedule::default(); + schedule.add_systems(validate_movement); + schedule.run(&mut world); + + let pos_lower = *world.get::(lower).unwrap(); + let pos_higher = *world.get::(higher).unwrap(); + + // Entity with lower bits processes first and claims the target + assert_eq!( + pos_lower, target, + "entity with lower Entity::to_bits() ({}) should win the tile", + lower.to_bits() + ); + assert_eq!( + pos_higher, higher_origin, + "entity with higher Entity::to_bits() ({}) should stay at origin", + higher.to_bits() + ); + } + #[test] fn all_four_layers_coexist_on_same_tile() { // D-054: Standing + Prone + Seated + Fixture all share one tile