Fix all Clippy warnings across the server codebase (2411 insertions, 1341 deletions). Raise type-complexity-threshold to 750 and too-many-arguments to 12 in .clippy.toml for idiomatic Bevy ECS system signatures. The server now passes `cargo clippy -- --deny warnings` cleanly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
540 lines
18 KiB
Rust
540 lines
18 KiB
Rust
// SpatialIndex trait + naive Vec implementation (#340)
|
|
// Provides proximity queries for follow mechanic (#241), NPC vision (#115, deferred).
|
|
// Trait abstraction allows grid/quadtree replacement without touching callers.
|
|
//
|
|
// Uses bevy_ecs Entity handles (not StableId) — this is a simulation-layer
|
|
// spatial optimization, not a knowledge graph concern.
|
|
|
|
use bevy_ecs::prelude::*;
|
|
|
|
use crate::simulation::movement::TilePosition;
|
|
|
|
/// Trait for spatial proximity queries over entities with TilePosition.
|
|
///
|
|
/// Implementations must be registered as a Bevy Resource.
|
|
/// All methods operate on the same z-level — cross-z queries return empty.
|
|
pub trait SpatialIndex: Send + Sync {
|
|
/// Return all entities within Manhattan distance `radius` of `position` on the same z-level.
|
|
/// Does NOT include entities exactly at `position` — use `entities_at` for that.
|
|
fn entities_in_range(&self, position: &TilePosition, radius: u32) -> Vec<Entity>;
|
|
|
|
/// Return all entities at the exact `position`.
|
|
fn entities_at(&self, position: &TilePosition) -> Vec<Entity>;
|
|
|
|
/// Return all entities within Manhattan distance `radius` of `position`,
|
|
/// **including** entities exactly at `position`. Single-pass alternative to
|
|
/// `entities_in_range` + `entities_at`.
|
|
fn entities_within(&self, position: &TilePosition, radius: u32) -> Vec<Entity> {
|
|
let mut result = self.entities_in_range(position, radius);
|
|
result.extend(self.entities_at(position));
|
|
result
|
|
}
|
|
|
|
/// Insert or update an entity's position in the index.
|
|
fn update(&mut self, entity: Entity, position: TilePosition);
|
|
|
|
/// Remove an entity from the index (e.g. on despawn or tier transition).
|
|
fn remove(&mut self, entity: Entity);
|
|
}
|
|
|
|
/// Naive Vec-backed spatial index — O(n) queries, sufficient for Active tier (30-80 NPCs).
|
|
///
|
|
/// Replace with grid or quadtree when profiling shows this is a bottleneck.
|
|
/// Deterministic iteration: entries stored in insertion order, but callers
|
|
/// should not depend on ordering (sort by Entity::to_bits() if needed).
|
|
///
|
|
/// ## Migration cost for grid/quadtree swap
|
|
///
|
|
/// `sync_spatial_index` takes `ResMut<NaiveSpatialIndex>` directly because
|
|
/// bevy_ecs cannot store `dyn SpatialIndex` as a Resource. A swap to Grid or
|
|
/// BVH requires changing the concrete type in: (1) `sync_spatial_index` system
|
|
/// parameter, (2) `SimulationPlugin` resource registration, (3) any system
|
|
/// that queries `Res<NaiveSpatialIndex>` (currently: `update_follow_state`).
|
|
/// The `SpatialIndex` trait ensures the API surface stays identical — only the
|
|
/// type name changes at call sites. Estimated: ~5 lines per caller.
|
|
#[derive(Resource, Debug, Default)]
|
|
pub struct NaiveSpatialIndex {
|
|
entries: Vec<(Entity, TilePosition)>,
|
|
}
|
|
|
|
impl NaiveSpatialIndex {
|
|
pub fn new() -> Self {
|
|
Self {
|
|
entries: Vec::new(),
|
|
}
|
|
}
|
|
}
|
|
|
|
impl SpatialIndex for NaiveSpatialIndex {
|
|
fn entities_in_range(&self, position: &TilePosition, radius: u32) -> Vec<Entity> {
|
|
self.entries
|
|
.iter()
|
|
.filter(|(_, pos)| {
|
|
pos != position
|
|
&& pos
|
|
.manhattan_distance(position)
|
|
.is_some_and(|d| d <= radius)
|
|
})
|
|
.map(|(entity, _)| *entity)
|
|
.collect()
|
|
}
|
|
|
|
fn entities_at(&self, position: &TilePosition) -> Vec<Entity> {
|
|
self.entries
|
|
.iter()
|
|
.filter(|(_, pos)| pos == position)
|
|
.map(|(entity, _)| *entity)
|
|
.collect()
|
|
}
|
|
|
|
fn entities_within(&self, position: &TilePosition, radius: u32) -> Vec<Entity> {
|
|
self.entries
|
|
.iter()
|
|
.filter(|(_, pos)| {
|
|
pos == position
|
|
|| pos
|
|
.manhattan_distance(position)
|
|
.is_some_and(|d| d <= radius)
|
|
})
|
|
.map(|(entity, _)| *entity)
|
|
.collect()
|
|
}
|
|
|
|
fn update(&mut self, entity: Entity, position: TilePosition) {
|
|
if let Some(entry) = self.entries.iter_mut().find(|(e, _)| *e == entity) {
|
|
entry.1 = position;
|
|
} else {
|
|
self.entries.push((entity, position));
|
|
}
|
|
}
|
|
|
|
fn remove(&mut self, entity: Entity) {
|
|
self.entries.retain(|(e, _)| *e != entity);
|
|
}
|
|
}
|
|
|
|
/// System: sync TilePosition changes into the NaiveSpatialIndex each tick.
|
|
///
|
|
/// Runs after movement validation so positions are final for the tick.
|
|
/// Only tracks entities with TilePosition — entities without it are not indexed.
|
|
pub fn sync_spatial_index(
|
|
mut index: ResMut<NaiveSpatialIndex>,
|
|
query: Query<(Entity, &TilePosition), Changed<TilePosition>>,
|
|
mut removed: RemovedComponents<TilePosition>,
|
|
) {
|
|
for (entity, pos) in query.iter() {
|
|
index.update(entity, *pos);
|
|
}
|
|
for entity in removed.read() {
|
|
index.remove(entity);
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
use bevy_ecs::world::World;
|
|
|
|
fn make_entity(world: &mut World) -> Entity {
|
|
world.spawn_empty().id()
|
|
}
|
|
|
|
#[test]
|
|
fn entities_at_exact_position() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e1 = make_entity(&mut world);
|
|
let e2 = make_entity(&mut world);
|
|
let e3 = make_entity(&mut world);
|
|
|
|
let pos = TilePosition::new(5, 5, 0);
|
|
index.update(e1, pos);
|
|
index.update(e2, pos);
|
|
index.update(e3, TilePosition::new(6, 5, 0));
|
|
|
|
let at = index.entities_at(&pos);
|
|
assert_eq!(at.len(), 2);
|
|
assert!(at.contains(&e1));
|
|
assert!(at.contains(&e2));
|
|
assert!(!at.contains(&e3));
|
|
}
|
|
|
|
#[test]
|
|
fn entities_in_range_excludes_exact_position() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e_at = make_entity(&mut world);
|
|
let e_near = make_entity(&mut world);
|
|
|
|
let center = TilePosition::new(5, 5, 0);
|
|
index.update(e_at, center);
|
|
index.update(e_near, TilePosition::new(5, 6, 0));
|
|
|
|
let in_range = index.entities_in_range(¢er, 2);
|
|
assert!(
|
|
!in_range.contains(&e_at),
|
|
"entity at center should be excluded"
|
|
);
|
|
assert!(in_range.contains(&e_near));
|
|
}
|
|
|
|
#[test]
|
|
fn entities_in_range_manhattan_distance() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e_close = make_entity(&mut world);
|
|
let e_boundary = make_entity(&mut world);
|
|
let e_far = make_entity(&mut world);
|
|
|
|
let center = TilePosition::new(5, 5, 0);
|
|
index.update(e_close, TilePosition::new(5, 6, 0)); // distance 1
|
|
index.update(e_boundary, TilePosition::new(7, 5, 0)); // distance 2
|
|
index.update(e_far, TilePosition::new(8, 5, 0)); // distance 3
|
|
|
|
let in_range = index.entities_in_range(¢er, 2);
|
|
assert!(in_range.contains(&e_close));
|
|
assert!(in_range.contains(&e_boundary));
|
|
assert!(!in_range.contains(&e_far));
|
|
}
|
|
|
|
#[test]
|
|
fn different_z_level_excluded() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e = make_entity(&mut world);
|
|
index.update(e, TilePosition::new(5, 5, 1));
|
|
|
|
let center = TilePosition::new(5, 5, 0);
|
|
assert!(index.entities_in_range(¢er, 10).is_empty());
|
|
assert!(index.entities_at(¢er).is_empty());
|
|
}
|
|
|
|
#[test]
|
|
fn update_moves_existing_entity() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e = make_entity(&mut world);
|
|
let old_pos = TilePosition::new(5, 5, 0);
|
|
let new_pos = TilePosition::new(10, 10, 0);
|
|
|
|
index.update(e, old_pos);
|
|
assert_eq!(index.entities_at(&old_pos).len(), 1);
|
|
|
|
index.update(e, new_pos);
|
|
assert!(index.entities_at(&old_pos).is_empty());
|
|
assert_eq!(index.entities_at(&new_pos).len(), 1);
|
|
}
|
|
|
|
#[test]
|
|
fn remove_entity() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e = make_entity(&mut world);
|
|
let pos = TilePosition::new(5, 5, 0);
|
|
|
|
index.update(e, pos);
|
|
assert_eq!(index.entities_at(&pos).len(), 1);
|
|
|
|
index.remove(e);
|
|
assert!(index.entities_at(&pos).is_empty());
|
|
}
|
|
|
|
#[test]
|
|
fn remove_nonexistent_entity_is_noop() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e = make_entity(&mut world);
|
|
index.remove(e); // should not panic
|
|
}
|
|
|
|
#[test]
|
|
fn empty_index_returns_empty() {
|
|
let index = NaiveSpatialIndex::new();
|
|
let pos = TilePosition::new(5, 5, 0);
|
|
|
|
assert!(index.entities_at(&pos).is_empty());
|
|
assert!(index.entities_in_range(&pos, 10).is_empty());
|
|
}
|
|
|
|
#[test]
|
|
fn zero_radius_returns_nothing() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e = make_entity(&mut world);
|
|
let center = TilePosition::new(5, 5, 0);
|
|
index.update(e, TilePosition::new(5, 6, 0)); // distance 1
|
|
|
|
// Radius 0: only exact position would match, but entities_in_range excludes center
|
|
assert!(index.entities_in_range(¢er, 0).is_empty());
|
|
}
|
|
|
|
// -----------------------------------------------------------------------
|
|
// Additional QA correctness tests (Hoshe, Sprint 15)
|
|
// -----------------------------------------------------------------------
|
|
|
|
/// Verify that updating an entity twice does not insert duplicates.
|
|
/// Callers of update() rely on the index having at most one entry per entity.
|
|
#[test]
|
|
fn update_does_not_duplicate_entity() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e = make_entity(&mut world);
|
|
let pos = TilePosition::new(5, 5, 0);
|
|
|
|
index.update(e, pos);
|
|
index.update(e, pos); // same position again
|
|
|
|
assert_eq!(
|
|
index.entries.len(),
|
|
1,
|
|
"update with same position must not insert a duplicate entry"
|
|
);
|
|
assert_eq!(index.entities_at(&pos).len(), 1);
|
|
}
|
|
|
|
/// Moving an entity does not accumulate stale entries.
|
|
#[test]
|
|
fn update_to_new_position_does_not_leave_old_entry() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e = make_entity(&mut world);
|
|
index.update(e, TilePosition::new(5, 5, 0));
|
|
index.update(e, TilePosition::new(10, 10, 0));
|
|
index.update(e, TilePosition::new(15, 15, 0));
|
|
|
|
// Entry list should still have exactly one entry for this entity
|
|
assert_eq!(index.entries.len(), 1);
|
|
}
|
|
|
|
/// `entities_in_range` with radius 1: includes distance-1, excludes distance-2.
|
|
#[test]
|
|
fn entities_in_range_radius_1_boundary() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e_dist1_x = make_entity(&mut world);
|
|
let e_dist1_y = make_entity(&mut world);
|
|
let e_dist2 = make_entity(&mut world);
|
|
|
|
let center = TilePosition::new(5, 5, 0);
|
|
index.update(e_dist1_x, TilePosition::new(6, 5, 0)); // Manhattan = 1
|
|
index.update(e_dist1_y, TilePosition::new(5, 4, 0)); // Manhattan = 1
|
|
index.update(e_dist2, TilePosition::new(7, 5, 0)); // Manhattan = 2
|
|
|
|
let in_range = index.entities_in_range(¢er, 1);
|
|
assert!(in_range.contains(&e_dist1_x));
|
|
assert!(in_range.contains(&e_dist1_y));
|
|
assert!(
|
|
!in_range.contains(&e_dist2),
|
|
"distance 2 must not appear in radius-1 result"
|
|
);
|
|
}
|
|
|
|
/// Diagonal: Manhattan distance covers all 4 orthogonal directions.
|
|
#[test]
|
|
fn entities_in_range_all_four_directions() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let north = make_entity(&mut world);
|
|
let south = make_entity(&mut world);
|
|
let east = make_entity(&mut world);
|
|
let west = make_entity(&mut world);
|
|
let corner = make_entity(&mut world); // distance 2 via diagonal (Manhattan = 2)
|
|
|
|
let center = TilePosition::new(10, 10, 0);
|
|
index.update(north, TilePosition::new(10, 11, 0)); // distance 1
|
|
index.update(south, TilePosition::new(10, 9, 0)); // distance 1
|
|
index.update(east, TilePosition::new(11, 10, 0)); // distance 1
|
|
index.update(west, TilePosition::new(9, 10, 0)); // distance 1
|
|
index.update(corner, TilePosition::new(11, 11, 0)); // distance 2
|
|
|
|
let in_range = index.entities_in_range(¢er, 2);
|
|
assert!(in_range.contains(&north));
|
|
assert!(in_range.contains(&south));
|
|
assert!(in_range.contains(&east));
|
|
assert!(in_range.contains(&west));
|
|
assert!(in_range.contains(&corner));
|
|
}
|
|
|
|
/// Entities just outside radius are excluded even when within Euclidean distance.
|
|
/// (5, 5) vs (8, 8): Manhattan = 6, Euclidean ≈ 4.2. Radius 5 → excluded.
|
|
#[test]
|
|
fn manhattan_excludes_diagonal_entity_within_euclidean() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e = make_entity(&mut world);
|
|
let center = TilePosition::new(5, 5, 0);
|
|
index.update(e, TilePosition::new(8, 8, 0)); // Manhattan = 6
|
|
|
|
let in_range = index.entities_in_range(¢er, 5);
|
|
assert!(
|
|
!in_range.contains(&e),
|
|
"Manhattan distance 6 must not appear in radius-5 result"
|
|
);
|
|
}
|
|
|
|
/// `entities_at` returns empty when no entity is at the queried position.
|
|
#[test]
|
|
fn entities_at_returns_empty_for_unoccupied_position() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let e = make_entity(&mut world);
|
|
index.update(e, TilePosition::new(5, 5, 0));
|
|
|
|
assert!(index.entities_at(&TilePosition::new(6, 5, 0)).is_empty());
|
|
}
|
|
|
|
/// Multiple entities at the same position — all returned by entities_at.
|
|
#[test]
|
|
fn entities_at_handles_multiple_entities_same_tile() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let pos = TilePosition::new(5, 5, 0);
|
|
let entities: Vec<Entity> = (0..5).map(|_| make_entity(&mut world)).collect();
|
|
for &e in &entities {
|
|
index.update(e, pos);
|
|
}
|
|
|
|
let at = index.entities_at(&pos);
|
|
assert_eq!(at.len(), 5, "all entities at same tile must be returned");
|
|
for e in &entities {
|
|
assert!(at.contains(e));
|
|
}
|
|
}
|
|
|
|
/// Large radius includes all entities in the index (except those at center).
|
|
#[test]
|
|
fn large_radius_includes_all_entities() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let center = TilePosition::new(50, 50, 0);
|
|
let mut entities = vec![];
|
|
for i in 0..10_i32 {
|
|
let e = make_entity(&mut world);
|
|
index.update(e, TilePosition::new(i, i, 0)); // All far from center
|
|
entities.push(e);
|
|
}
|
|
|
|
let in_range = index.entities_in_range(¢er, 200);
|
|
assert_eq!(
|
|
in_range.len(),
|
|
10,
|
|
"radius 200 should include all 10 entities"
|
|
);
|
|
}
|
|
|
|
/// Remove one entity from a multi-entity index, others remain.
|
|
#[test]
|
|
fn remove_one_entity_others_intact() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let pos = TilePosition::new(5, 5, 0);
|
|
let e1 = make_entity(&mut world);
|
|
let e2 = make_entity(&mut world);
|
|
let e3 = make_entity(&mut world);
|
|
|
|
index.update(e1, pos);
|
|
index.update(e2, pos);
|
|
index.update(e3, TilePosition::new(6, 5, 0));
|
|
|
|
index.remove(e1);
|
|
|
|
let at = index.entities_at(&pos);
|
|
assert_eq!(at.len(), 1, "only e2 should remain at the position");
|
|
assert!(at.contains(&e2));
|
|
assert!(!at.contains(&e1));
|
|
// e3 unaffected
|
|
assert_eq!(index.entities_at(&TilePosition::new(6, 5, 0)).len(), 1);
|
|
}
|
|
|
|
/// Stress test: 100 entities, correctness at scale.
|
|
#[test]
|
|
fn stress_100_entities_correctness() {
|
|
let mut world = World::new();
|
|
let mut index = NaiveSpatialIndex::new();
|
|
|
|
let center = TilePosition::new(0, 0, 0);
|
|
let mut in_range_expected = 0u32;
|
|
|
|
for i in 0..100_i32 {
|
|
let e = make_entity(&mut world);
|
|
let pos = TilePosition::new(i, 0, 0); // distance = i from center
|
|
index.update(e, pos);
|
|
if i > 0 && i <= 10 {
|
|
in_range_expected += 1;
|
|
}
|
|
}
|
|
|
|
let result = index.entities_in_range(¢er, 10);
|
|
assert_eq!(
|
|
result.len(),
|
|
in_range_expected as usize,
|
|
"radius-10 from origin should include exactly 10 entities (distance 1..10)"
|
|
);
|
|
}
|
|
|
|
/// sync_spatial_index system test: Changed<TilePosition> updates the index.
|
|
#[test]
|
|
fn sync_system_tracks_position_changes() {
|
|
use super::sync_spatial_index;
|
|
|
|
let mut world = World::new();
|
|
world.init_resource::<NaiveSpatialIndex>();
|
|
|
|
let start = TilePosition::new(5, 5, 0);
|
|
let dest = TilePosition::new(10, 10, 0);
|
|
|
|
let entity = world.spawn(start).id();
|
|
|
|
let mut schedule = bevy_ecs::schedule::Schedule::default();
|
|
schedule.add_systems(sync_spatial_index);
|
|
|
|
// First run: entity inserted with initial position
|
|
schedule.run(&mut world);
|
|
|
|
{
|
|
let idx = world.resource::<NaiveSpatialIndex>();
|
|
assert_eq!(
|
|
idx.entities_at(&start).len(),
|
|
1,
|
|
"entity should be at start after first sync"
|
|
);
|
|
}
|
|
|
|
// Update position
|
|
world.entity_mut(entity).insert(dest);
|
|
|
|
// Second run: entity moved
|
|
schedule.run(&mut world);
|
|
|
|
{
|
|
let idx = world.resource::<NaiveSpatialIndex>();
|
|
assert!(
|
|
idx.entities_at(&start).is_empty(),
|
|
"old position should be cleared"
|
|
);
|
|
assert_eq!(
|
|
idx.entities_at(&dest).len(),
|
|
1,
|
|
"entity should be at new position"
|
|
);
|
|
}
|
|
}
|
|
}
|