tree felling optimisations

src/world/generation/forestry.rs

@@ -4,6 +4,8 @@ use bevy_platform::collections::HashSet;
 use bevy_platform::sync::Mutex;
 use bevy_platform::time::Instant;
 use bevy_rand::prelude::*;
+use rustc_hash::FxHashSet;
+use smallvec::SmallVec;
 
 use rand::{RngExt, SeedableRng};
 use std::collections::hash_map::DefaultHasher;
@@ -12,11 +14,13 @@ use std::hash::{Hash, Hasher};
 use crate::entities::item::drop_table::DropTable;
 use crate::{
     constants::{ITILE_SIZE, SEED, TILE_SIZE},
-    world::generation::forestry::constants::{
-        TREE_LEAF_BASE_RADIUS, TREE_LEAF_RADIUS_OFFSET, TREE_LEAF_RADIUS_VARIATION,
-        TREE_MIN_DISTANCE_TILES, TREE_SPAWN_CHANCE, TREE_TRUNK_EXTRA_HEIGHT, TREE_TRUNK_MIN_HEIGHT,
-    },
     world::{
+        chunks::world_to_chunk,
+        generation::forestry::constants::{
+            TREE_LEAF_BASE_RADIUS, TREE_LEAF_RADIUS_OFFSET, TREE_LEAF_RADIUS_VARIATION,
+            TREE_MIN_DISTANCE_TILES, TREE_SPAWN_CHANCE, TREE_TRUNK_EXTRA_HEIGHT,
+            TREE_TRUNK_MIN_HEIGHT,
+        },
         tiles::{
             tile_changed::TileChangedEvent, visibility::TileOcclusionEvent, FixtureTileData,
             TileMap,
@@ -30,11 +34,15 @@ use crate::{
 /// `tile_pos` is the grid-aligned world position of the fixture tile this entity
 /// represents. Used to call remove_fixture without scanning fixture_tiles.
 ///
+/// `chunk_pos` is the chunk this entity lives in. Used by fell_tree to filter
+/// O(world_trees) → O(chunk_trees) when searching for tree parts.
+///
 /// `is_trunk` distinguishes trunk segments (can_stand_in=false, drops wood) from
 /// leaf canopy (can_stand_in=true, drops nothing or leaves).
 #[derive(Component)]
 pub struct TreePart {
     pub tile_pos: IVec3,
+    pub chunk_pos: IVec2,
     pub is_trunk: bool,
 }
 
@@ -105,6 +113,7 @@ pub fn generate_chunk_forrestry(
                                     ChunkOwner(event.chunk_position),
                                     TreePart {
                                         tile_pos: trunk_ivec,
+                                        chunk_pos: world_to_chunk(trunk_ivec),
                                         is_trunk: true,
                                     },
                                 ));
@@ -197,6 +206,7 @@ pub fn generate_chunk_forrestry(
                                                         ChunkOwner(event.chunk_position),
                                                         TreePart {
                                                             tile_pos: ivec,
+                                                            chunk_pos: world_to_chunk(ivec),
                                                             is_trunk: false,
                                                         },
                                                     ));
@@ -256,25 +266,20 @@ pub fn generate_chunk_forrestry(
 /// Fells the entire tree containing the trunk at `trunk_pos`.
 ///
 /// # How trees are identified
-/// Finds all TreePart entities whose XY is within canopy radius of the trunk column.
+/// Finds all TreePart entities in the trunk's chunk whose XY is within canopy radius of
-/// This avoids storing a tree ID — the spatial structure IS the identity.
+/// the trunk column. This avoids storing a tree ID — the spatial structure IS the identity.
 ///
 /// # What this does
 /// For each TreePart entity found:
 /// - Calls `tilemap.remove_fixture(&tree_part.tile_pos)` (clears ChunkData + HashMap)
 /// - Despawns the Bevy entity
 /// - Fires `TileChangedEvent` for path invalidation
-/// - Fires `TileOcclusionEvent` for the column at each removed position
+/// - Fires one `TileOcclusionEvent` per unique column position (deduplicated)
 ///
 /// # chunk_entity_index
 /// Despawned tree entities leave stale entries in `chunk_entity_index`. This is harmless:
 /// `unload_chunk` calls `despawn()` on each indexed entity, which is a no-op on
 /// already-despawned entities.
-///
-/// # Performance
-/// O(tree_size) entity lookups via `Query<(Entity, &TreePart)>`. For a typical tree
-/// (4-8 trunk + ~60 leaf tiles) this is ~68 iterations. Acceptable for an infrequent
-/// action.
 pub fn fell_tree(
     trunk_pos: IVec3,
     tree_parts: &Query<(Entity, &TreePart)>,
@@ -283,40 +288,45 @@ pub fn fell_tree(
     tile_changed: &mut MessageWriter<TileChangedEvent>,
     occlusion: &mut MessageWriter<TileOcclusionEvent>,
 ) {
+    let target_chunk = world_to_chunk(trunk_pos);
     let trunk_x = trunk_pos.x;
     let trunk_y = trunk_pos.y;
     let canopy_radius_world = (TREE_LEAF_BASE_RADIUS * TILE_SIZE) as i32 + ITILE_SIZE;
 
-    let to_remove: Vec<(Entity, IVec3)> = tree_parts
+    let to_remove: SmallVec<[_; 96]> = tree_parts
         .iter()
         .filter(|(_, part)| {
-            let dx = (part.tile_pos.x - trunk_x).abs();
+            part.chunk_pos == target_chunk
-            let dy = (part.tile_pos.y - trunk_y).abs();
+                && (part.tile_pos.x - trunk_x).abs() <= canopy_radius_world
-            dx <= canopy_radius_world && dy <= canopy_radius_world
+                && (part.tile_pos.y - trunk_y).abs() <= canopy_radius_world
         })
         .map(|(entity, part)| (entity, part.tile_pos))
         .collect();
 
-    for (entity, tile_pos) in to_remove {
+    // Collect unique occlusion positions across all removed tiles, then fire once each.
-        tilemap.remove_fixture(&tile_pos);
+    // Adjacent tree parts share column positions, so deduplication cuts event count
-        tile_changed.write(TileChangedEvent { pos: tile_pos });
+    // significantly vs. firing per-tile per-column.
+    let mut dirty_columns: FxHashSet<IVec3> = FxHashSet::default();
+    for (entity, tile_pos) in to_remove.iter() {
+        tilemap.remove_fixture(tile_pos);
+        tile_changed.write(TileChangedEvent { pos: *tile_pos });
+        commands.entity(*entity).despawn();
 
-        // Full column occlusion refresh for each removed tile.
         // calculate_visibility traces up arbitrarily deep, so refresh the full column.
-        // TODO: batch into a dirty-region approach once tree felling is common (~13k events/tree).
+        // Hoist z_depth — constant per call, computed once here.
-        let z_depth =
+        let z_total =
             crate::world::chunks::Z_BELOW as i32 + crate::world::chunks::Z_ABOVE as i32 + 1;
-        for dz in 0..=z_depth {
+        for dz in 0..=z_total {
             for dy in -1..=1i32 {
                 for dx in -1..=1i32 {
-                    occlusion.write(TileOcclusionEvent {
+                    dirty_columns.insert(
-                        tile_position: tile_pos
+                        *tile_pos - IVec3::new(dx * ITILE_SIZE, dy * ITILE_SIZE, dz * ITILE_SIZE),
-                            - IVec3::new(dx * ITILE_SIZE, dy * ITILE_SIZE, dz * ITILE_SIZE),
+                    );
-                    });
                 }
             }
         }
-
+    }
-        commands.entity(entity).despawn();
+    for pos in dirty_columns {
+        occlusion.write(TileOcclusionEvent { tile_position: pos });
     }
 }