From e52c53796e3f0ec0f5e58036d664cf342d997882 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 3 Dec 2022 20:25:02 -0800 Subject: [PATCH 1/5] Allow iterating over with EntityRefs instead of just Entities --- crates/bevy_ecs/src/entity/mod.rs | 8 ++++++++ crates/bevy_ecs/src/world/mod.rs | 12 ++++++++++-- crates/bevy_scene/src/dynamic_scene_builder.rs | 18 ++++++++++++------ 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 7404b5345a6ff..b5f9ea6046f89 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -575,6 +575,14 @@ impl Entities { } } + /// Gets the location of an entity without checking the validity of it's index nor it's generation. + /// + /// # Safety + /// `index` must correspond to an allocated entity. + pub(crate) unsafe fn get_unchecked(&self, index: u32) -> EntityLocation { + self.meta.get_unchecked(index as usize).location + } + /// Updates the location of an [`Entity`]. This must be called when moving the components of /// the entity around in storage. /// diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 6c4bab76ade83..c29d1058e6c39 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -323,11 +323,19 @@ impl World { /// /// This is useful in contexts where you only have read-only access to the [`World`]. #[inline] - pub fn iter_entities(&self) -> impl Iterator + '_ { + pub fn iter_entities(&self) -> impl Iterator> + '_ { self.archetypes .iter() .flat_map(|archetype| archetype.entities().iter()) - .map(|archetype_entity| archetype_entity.entity()) + .map(|archetype_entity| { + // SAFETY: The entity is already located in a Archetype, which means the entity + // must be properly allocated + let location = unsafe { + self.entities + .get_unchecked(archetype_entity.entity().index()) + }; + EntityRef::new(self, archetype_entity.entity(), location) + }) } /// Retrieves an [`EntityMut`] that exposes read and write operations for the given `entity`. diff --git a/crates/bevy_scene/src/dynamic_scene_builder.rs b/crates/bevy_scene/src/dynamic_scene_builder.rs index 841af6756f3ba..56bfb5bc954e1 100644 --- a/crates/bevy_scene/src/dynamic_scene_builder.rs +++ b/crates/bevy_scene/src/dynamic_scene_builder.rs @@ -1,6 +1,9 @@ use crate::{DynamicEntity, DynamicScene}; use bevy_app::AppTypeRegistry; -use bevy_ecs::{prelude::Entity, reflect::ReflectComponent, world::World}; +use bevy_ecs::{ + reflect::ReflectComponent, + world::{EntityRef, World}, +}; use bevy_utils::default; use std::collections::BTreeMap; @@ -67,7 +70,7 @@ impl<'w> DynamicSceneBuilder<'w> { /// Extract one entity from the builder's [`World`]. /// /// Re-extracting an entity that was already extracted will have no effect. - pub fn extract_entity(&mut self, entity: Entity) -> &mut Self { + pub fn extract_entity<'a>(&mut self, entity: EntityRef<'a>) -> &mut Self { self.extract_entities(std::iter::once(entity)) } @@ -96,11 +99,14 @@ impl<'w> DynamicSceneBuilder<'w> { /// builder.extract_entities(query.iter(&world)); /// let scene = builder.build(); /// ``` - pub fn extract_entities(&mut self, entities: impl Iterator) -> &mut Self { + pub fn extract_entities<'a>( + &mut self, + entities: impl Iterator>, + ) -> &mut Self { let type_registry = self.type_registry.read(); for entity in entities { - let index = entity.index(); + let index = entity.id().index(); if self.entities.contains_key(&index) { continue; @@ -111,7 +117,7 @@ impl<'w> DynamicSceneBuilder<'w> { components: Vec::new(), }; - for component_id in self.world.entity(entity).archetype().components() { + for component_id in entity.archetype().components() { let reflect_component = self .world .components() @@ -120,7 +126,7 @@ impl<'w> DynamicSceneBuilder<'w> { .and_then(|registration| registration.data::()); if let Some(reflect_component) = reflect_component { - if let Some(component) = reflect_component.reflect(self.world, entity) { + if let Some(component) = reflect_component.reflect(self.world, entity.id()) { entry.components.push(component.clone_value()); } } From 1932ef35a75c5bf95da579b83c29c52329f4d94f Mon Sep 17 00:00:00 2001 From: James Liu Date: Sun, 4 Dec 2022 15:35:10 -0800 Subject: [PATCH 2/5] grammar Co-authored-by: Alice Cecile --- crates/bevy_ecs/src/entity/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index b5f9ea6046f89..29b5c974e4ca1 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -575,7 +575,7 @@ impl Entities { } } - /// Gets the location of an entity without checking the validity of it's index nor it's generation. + /// Gets the location of an entity without checking the validity of its index nor its generation. /// /// # Safety /// `index` must correspond to an allocated entity. From 5c322676de2e2984d96fb19ff2f1c4621c7f18a7 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 4 Dec 2022 18:36:02 -0800 Subject: [PATCH 3/5] Fix CI and revert dynamic_scene changes --- crates/bevy_ecs/src/world/mod.rs | 2 +- crates/bevy_scene/src/dynamic_scene.rs | 2 +- crates/bevy_scene/src/dynamic_scene_builder.rs | 18 ++++++------------ 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index c29d1058e6c39..6d3504086bcfa 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1878,7 +1878,7 @@ mod tests { let iterate_and_count_entities = |world: &World, entity_counters: &mut HashMap<_, _>| { entity_counters.clear(); for entity in world.iter_entities() { - let counter = entity_counters.entry(entity).or_insert(0); + let counter = entity_counters.entry(entity.id()).or_insert(0); *counter += 1; } }; diff --git a/crates/bevy_scene/src/dynamic_scene.rs b/crates/bevy_scene/src/dynamic_scene.rs index 92f8cae312f4e..c0eb067b2ca8b 100644 --- a/crates/bevy_scene/src/dynamic_scene.rs +++ b/crates/bevy_scene/src/dynamic_scene.rs @@ -46,7 +46,7 @@ impl DynamicScene { let mut builder = DynamicSceneBuilder::from_world_with_type_registry(world, type_registry.clone()); - builder.extract_entities(world.iter_entities()); + builder.extract_entities(world.iter_entities().map(|entity| entity.id())); builder.build() } diff --git a/crates/bevy_scene/src/dynamic_scene_builder.rs b/crates/bevy_scene/src/dynamic_scene_builder.rs index 56bfb5bc954e1..71cbde5983ccb 100644 --- a/crates/bevy_scene/src/dynamic_scene_builder.rs +++ b/crates/bevy_scene/src/dynamic_scene_builder.rs @@ -1,9 +1,6 @@ use crate::{DynamicEntity, DynamicScene}; use bevy_app::AppTypeRegistry; -use bevy_ecs::{ - reflect::ReflectComponent, - world::{EntityRef, World}, -}; +use bevy_ecs::{entity::Entity, reflect::ReflectComponent, world::World}; use bevy_utils::default; use std::collections::BTreeMap; @@ -70,7 +67,7 @@ impl<'w> DynamicSceneBuilder<'w> { /// Extract one entity from the builder's [`World`]. /// /// Re-extracting an entity that was already extracted will have no effect. - pub fn extract_entity<'a>(&mut self, entity: EntityRef<'a>) -> &mut Self { + pub fn extract_entity(&mut self, entity: Entity) -> &mut Self { self.extract_entities(std::iter::once(entity)) } @@ -99,14 +96,11 @@ impl<'w> DynamicSceneBuilder<'w> { /// builder.extract_entities(query.iter(&world)); /// let scene = builder.build(); /// ``` - pub fn extract_entities<'a>( - &mut self, - entities: impl Iterator>, - ) -> &mut Self { + pub fn extract_entities(&mut self, entities: impl Iterator) -> &mut Self { let type_registry = self.type_registry.read(); for entity in entities { - let index = entity.id().index(); + let index = entity.index(); if self.entities.contains_key(&index) { continue; @@ -117,7 +111,7 @@ impl<'w> DynamicSceneBuilder<'w> { components: Vec::new(), }; - for component_id in entity.archetype().components() { + for component_id in self.world.entity(entity).archetype().components() { let reflect_component = self .world .components() @@ -126,7 +120,7 @@ impl<'w> DynamicSceneBuilder<'w> { .and_then(|registration| registration.data::()); if let Some(reflect_component) = reflect_component { - if let Some(component) = reflect_component.reflect(self.world, entity.id()) { + if let Some(component) = reflect_component.reflect(self.world, entity) { entry.components.push(component.clone_value()); } } From 49220b8d17dc1bcf2fdd62e319feb6cdf35d477d Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 4 Dec 2022 18:39:26 -0800 Subject: [PATCH 4/5] Last revert --- crates/bevy_scene/src/dynamic_scene_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_scene/src/dynamic_scene_builder.rs b/crates/bevy_scene/src/dynamic_scene_builder.rs index 71cbde5983ccb..841af6756f3ba 100644 --- a/crates/bevy_scene/src/dynamic_scene_builder.rs +++ b/crates/bevy_scene/src/dynamic_scene_builder.rs @@ -1,6 +1,6 @@ use crate::{DynamicEntity, DynamicScene}; use bevy_app::AppTypeRegistry; -use bevy_ecs::{entity::Entity, reflect::ReflectComponent, world::World}; +use bevy_ecs::{prelude::Entity, reflect::ReflectComponent, world::World}; use bevy_utils::default; use std::collections::BTreeMap; From 5b33067ef8ecdbff7e9585890ae5aefc6537f40b Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 5 Dec 2022 11:50:38 -0800 Subject: [PATCH 5/5] Avoid querying Entities for location information --- crates/bevy_ecs/src/entity/mod.rs | 8 -------- crates/bevy_ecs/src/world/mod.rs | 27 ++++++++++++++------------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 29b5c974e4ca1..7404b5345a6ff 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -575,14 +575,6 @@ impl Entities { } } - /// Gets the location of an entity without checking the validity of its index nor its generation. - /// - /// # Safety - /// `index` must correspond to an allocated entity. - pub(crate) unsafe fn get_unchecked(&self, index: u32) -> EntityLocation { - self.meta.get_unchecked(index as usize).location - } - /// Updates the location of an [`Entity`]. This must be called when moving the components of /// the entity around in storage. /// diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 6d3504086bcfa..a3218f8ff014f 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -14,7 +14,7 @@ use crate::{ component::{ Component, ComponentDescriptor, ComponentId, ComponentInfo, Components, TickCells, }, - entity::{AllocAtWithoutReplacement, Entities, Entity}, + entity::{AllocAtWithoutReplacement, Entities, Entity, EntityLocation}, query::{QueryState, ReadOnlyWorldQuery, WorldQuery}, storage::{ResourceData, SparseSet, Storages}, system::Resource, @@ -324,18 +324,19 @@ impl World { /// This is useful in contexts where you only have read-only access to the [`World`]. #[inline] pub fn iter_entities(&self) -> impl Iterator> + '_ { - self.archetypes - .iter() - .flat_map(|archetype| archetype.entities().iter()) - .map(|archetype_entity| { - // SAFETY: The entity is already located in a Archetype, which means the entity - // must be properly allocated - let location = unsafe { - self.entities - .get_unchecked(archetype_entity.entity().index()) - }; - EntityRef::new(self, archetype_entity.entity(), location) - }) + self.archetypes.iter().flat_map(|archetype| { + archetype + .entities() + .iter() + .enumerate() + .map(|(index, archetype_entity)| { + let location = EntityLocation { + archetype_id: archetype.id(), + index, + }; + EntityRef::new(self, archetype_entity.entity(), location) + }) + }) } /// Retrieves an [`EntityMut`] that exposes read and write operations for the given `entity`.