fix(simulation): determinism fixes — BTreeSet ordering, entity sort, mover sort
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<PlayerCharacter>,
|
||||
>,
|
||||
@@ -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<VisibleEntity>, HashSet<u64>) {
|
||||
) -> (Vec<VisibleEntity>, BTreeSet<u64>) {
|
||||
let mut entities = Vec::new();
|
||||
let mut visible_ids: HashSet<u64> = HashSet::new();
|
||||
let mut visible_ids: BTreeSet<u64> = 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<u64>,
|
||||
visible_positions: &HashSet<(i32, i32)>,
|
||||
visible_ids: &BTreeSet<u64>,
|
||||
visible_positions: &BTreeSet<(i32, i32)>,
|
||||
observer_z: i32,
|
||||
current_tick: u64,
|
||||
entities: &mut Vec<VisibleEntity>,
|
||||
|
||||
@@ -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<VisibleTile>,
|
||||
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<VisibleTile> = 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();
|
||||
|
||||
|
||||
@@ -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<Entity> = 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::<MoveIntent>();
|
||||
@@ -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::<TilePosition>(lower).unwrap();
|
||||
let pos_higher = *world.get::<TilePosition>(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
|
||||
|
||||
Reference in New Issue
Block a user