From fbde14e107f9b5502450418f3107acab93e592a3 Mon Sep 17 00:00:00 2001 From: Nuutti Kotivuori Date: Sat, 19 Feb 2022 01:54:22 +0200 Subject: [PATCH 01/14] Document `MapEntities` trait. --- crates/bevy_ecs/src/entity/map_entities.rs | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 00080bd0edd2f..271a1d35c83dd 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -8,6 +8,32 @@ pub enum MapEntitiesError { EntityNotFound(Entity), } +/// Operation to map all contained [`Entity`](crate::entity::Entity) fields in +/// a component to new values. +/// +/// If a component contains [`Entity`](crate::entity::Entity) values +/// that refer to other entities in the same world and scene functionality +/// is used to create such components, this trait must be implemented. The +/// is to replace all [`Entity`](crate::entity::Entity) values in the +/// component with values looked up from the given [`EntityMap`]. +/// +/// Implementing this trait is pretty straightforward: +/// +/// ``` +/// #[derive(Component)] +/// struct MyEntityRefs { +/// a: Entity, +/// b: Entity, +/// } +/// +/// impl MapEntities for MyEntityRefs { +/// fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError> { +/// self.a = entity_map.get(self.a)?; +/// self.b = entity_map.get(self.b)?; +/// Ok(()) +/// } +/// } +/// ``` pub trait MapEntities { fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError>; } From eac9a795d2b94b7c9660e5645e1541d01dc707ba Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 01:45:33 -0800 Subject: [PATCH 02/14] Make EntityMeta non-pub and non-pub(crate) --- crates/bevy_ecs/src/bundle.rs | 10 +++++----- crates/bevy_ecs/src/entity/mod.rs | 22 ++++++++++++++++++---- crates/bevy_ecs/src/world/entity_ref.rs | 11 ++++++++--- crates/bevy_ecs/src/world/mod.rs | 5 +---- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 2e5bf03c10e8b..81cc8e74b28c7 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -553,10 +553,10 @@ impl<'a, 'b> BundleInserter<'a, 'b> { InsertBundleResult::NewArchetypeSameTable { new_archetype } => { let result = self.archetype.swap_remove(location.index); if let Some(swapped_entity) = result.swapped_entity { - self.entities.meta[swapped_entity.index as usize].location = location; + self.entities.set(swapped_entity.index, location); } let new_location = new_archetype.allocate(entity, result.table_row); - self.entities.meta[entity.index as usize].location = new_location; + self.entities.set(entity.index, new_location); // PERF: this could be looked up during Inserter construction and stored (but borrowing makes this nasty) let add_bundle = self @@ -581,7 +581,7 @@ impl<'a, 'b> BundleInserter<'a, 'b> { } => { let result = self.archetype.swap_remove(location.index); if let Some(swapped_entity) = result.swapped_entity { - self.entities.meta[swapped_entity.index as usize].location = location; + self.entities.set(swapped_entity.index, location); } // PERF: store "non bundle" components in edge, then just move those to avoid // redundant copies @@ -589,7 +589,7 @@ impl<'a, 'b> BundleInserter<'a, 'b> { .table .move_to_superset_unchecked(result.table_row, new_table); let new_location = new_archetype.allocate(entity, move_result.new_row); - self.entities.meta[entity.index as usize].location = new_location; + self.entities.set(entity.index, new_location); // if an entity was moved into this entity's table spot, update its table row if let Some(swapped_entity) = move_result.swapped_entity { @@ -664,7 +664,7 @@ impl<'a, 'b> BundleSpawner<'a, 'b> { self.change_tick, bundle, ); - self.entities.meta[entity.index as usize].location = location; + self.entities.set(entity.index, location); location } diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 844885413716e..0b6994ef33191 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -109,7 +109,7 @@ pub struct Entity { pub(crate) index: u32, } -pub enum AllocAtWithoutReplacement { +pub(crate) enum AllocAtWithoutReplacement { Exists(EntityLocation), DidNotExist, ExistsWithWrongGeneration, @@ -267,7 +267,7 @@ impl<'a> core::iter::FusedIterator for ReserveEntitiesIterator<'a> {} #[derive(Debug, Default)] pub struct Entities { - pub(crate) meta: Vec, + meta: Vec, /// The `pending` and `free_cursor` fields describe three sets of Entity IDs /// that have been freed or are in the process of being allocated: @@ -447,7 +447,10 @@ impl Entities { /// Allocate a specific entity ID, overwriting its generation. /// /// Returns the location of the entity currently using the given ID, if any. - pub fn alloc_at_without_replacement(&mut self, entity: Entity) -> AllocAtWithoutReplacement { + pub(crate) fn alloc_at_without_replacement( + &mut self, + entity: Entity, + ) -> AllocAtWithoutReplacement { self.verify_flushed(); let result = if entity.index as usize >= self.meta.len() { @@ -544,6 +547,17 @@ impl Entities { } } + /// Updates the location of an [`Entity`]. This must be called when moving the components of + /// the entity around in storage. + /// + /// # Safety + /// - `index` must be a valid entity index. + /// - `location` must be valid for the entity at `index` is location + pub(crate) unsafe fn set(&mut self, index: u32, location: EntityLocation) { + // SAFETY: Caller guarentees that `index` a valid entity index + self.meta.get_unchecked_mut(index as usize).location = location; + } + /// Get the [`Entity`] with a given id, if it exists in this [`Entities`] collection /// Returns `None` if this [`Entity`] is outside of the range of currently reserved Entities /// @@ -666,7 +680,7 @@ impl Entities { // This type must not contain any pointers at any level, and be safe to fully fill with u8::MAX. #[derive(Copy, Clone, Debug)] #[repr(C)] -pub struct EntityMeta { +struct EntityMeta { pub generation: u32, pub location: EntityLocation, } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 37bf71adad1cd..0bb5f19b767af 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -360,7 +360,7 @@ impl<'w> EntityMut<'w> { let old_archetype = &mut archetypes[old_archetype_id]; let remove_result = old_archetype.swap_remove(old_location.index); if let Some(swapped_entity) = remove_result.swapped_entity { - entities.meta[swapped_entity.index as usize].location = old_location; + entities.set(swapped_entity.index, old_location); } let old_table_row = remove_result.table_row; let old_table_id = old_archetype.table_id(); @@ -394,7 +394,8 @@ impl<'w> EntityMut<'w> { }; *self_location = new_location; - entities.meta[entity.index as usize].location = new_location; + // SAFETY: The entity is valid and has been moved to the new location already. + entities.set(entity.index, new_location); } #[deprecated( @@ -490,7 +491,11 @@ impl<'w> EntityMut<'w> { } let remove_result = archetype.swap_remove(location.index); if let Some(swapped_entity) = remove_result.swapped_entity { - world.entities.meta[swapped_entity.index as usize].location = location; + // SAFETY: swapped_entity is valid and the swapped entity's components are + // moved to the new location immediately after. + unsafe { + world.entities.set(swapped_entity.index, location); + } } table_row = remove_result.table_row; diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d42ae7744e127..09b85e22040c6 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -480,10 +480,7 @@ impl World { // empty let location = archetype.allocate(entity, table_row); // SAFETY: entity index was just allocated - self.entities - .meta - .get_unchecked_mut(entity.index() as usize) - .location = location; + self.entities.set(entity.index(), location); EntityMut::new(self, entity, location) } From 86c9f542d691871fb26d40ebca788bed238132e0 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 01:48:46 -0800 Subject: [PATCH 03/14] Make index and generation not pub(crate) --- crates/bevy_ecs/src/bundle.rs | 10 +++++----- crates/bevy_ecs/src/entity/mod.rs | 4 ++-- crates/bevy_ecs/src/world/entity_ref.rs | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 81cc8e74b28c7..407890d5ce601 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -553,10 +553,10 @@ impl<'a, 'b> BundleInserter<'a, 'b> { InsertBundleResult::NewArchetypeSameTable { new_archetype } => { let result = self.archetype.swap_remove(location.index); if let Some(swapped_entity) = result.swapped_entity { - self.entities.set(swapped_entity.index, location); + self.entities.set(swapped_entity.index(), location); } let new_location = new_archetype.allocate(entity, result.table_row); - self.entities.set(entity.index, new_location); + self.entities.set(entity.index(), new_location); // PERF: this could be looked up during Inserter construction and stored (but borrowing makes this nasty) let add_bundle = self @@ -581,7 +581,7 @@ impl<'a, 'b> BundleInserter<'a, 'b> { } => { let result = self.archetype.swap_remove(location.index); if let Some(swapped_entity) = result.swapped_entity { - self.entities.set(swapped_entity.index, location); + self.entities.set(swapped_entity.index(), location); } // PERF: store "non bundle" components in edge, then just move those to avoid // redundant copies @@ -589,7 +589,7 @@ impl<'a, 'b> BundleInserter<'a, 'b> { .table .move_to_superset_unchecked(result.table_row, new_table); let new_location = new_archetype.allocate(entity, move_result.new_row); - self.entities.set(entity.index, new_location); + self.entities.set(entity.index(), new_location); // if an entity was moved into this entity's table spot, update its table row if let Some(swapped_entity) = move_result.swapped_entity { @@ -664,7 +664,7 @@ impl<'a, 'b> BundleSpawner<'a, 'b> { self.change_tick, bundle, ); - self.entities.set(entity.index, location); + self.entities.set(entity.index(), location); location } diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 0b6994ef33191..9b35b1b40885e 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -105,8 +105,8 @@ type IdCursor = isize; /// [`Query::get`]: crate::system::Query::get #[derive(Clone, Copy, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct Entity { - pub(crate) generation: u32, - pub(crate) index: u32, + generation: u32, + index: u32, } pub(crate) enum AllocAtWithoutReplacement { diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 0bb5f19b767af..11ff54232f400 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -360,7 +360,7 @@ impl<'w> EntityMut<'w> { let old_archetype = &mut archetypes[old_archetype_id]; let remove_result = old_archetype.swap_remove(old_location.index); if let Some(swapped_entity) = remove_result.swapped_entity { - entities.set(swapped_entity.index, old_location); + entities.set(swapped_entity.index(), old_location); } let old_table_row = remove_result.table_row; let old_table_id = old_archetype.table_id(); @@ -395,7 +395,7 @@ impl<'w> EntityMut<'w> { *self_location = new_location; // SAFETY: The entity is valid and has been moved to the new location already. - entities.set(entity.index, new_location); + entities.set(entity.index(), new_location); } #[deprecated( @@ -494,7 +494,7 @@ impl<'w> EntityMut<'w> { // SAFETY: swapped_entity is valid and the swapped entity's components are // moved to the new location immediately after. unsafe { - world.entities.set(swapped_entity.index, location); + world.entities.set(swapped_entity.index(), location); } } table_row = remove_result.table_row; From 595415bb70bfdc9a3644f4e91ff41af9a3c4231e Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 01:53:52 -0800 Subject: [PATCH 04/14] Force `Entities` to only be constructed in the context of a `World` --- crates/bevy_ecs/src/entity/mod.rs | 12 +++++++++++- crates/bevy_ecs/src/world/mod.rs | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 9b35b1b40885e..582dc868bda24 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -265,7 +265,7 @@ impl<'a> Iterator for ReserveEntitiesIterator<'a> { impl<'a> core::iter::ExactSizeIterator for ReserveEntitiesIterator<'a> {} impl<'a> core::iter::FusedIterator for ReserveEntitiesIterator<'a> {} -#[derive(Debug, Default)] +#[derive(Debug)] pub struct Entities { meta: Vec, @@ -311,6 +311,15 @@ pub struct Entities { } impl Entities { + pub(crate) const fn new() -> Self { + Entities { + meta: Vec::new(), + pending: Vec::new(), + free_cursor: AtomicIdCursor::new(0), + len: 0, + } + } + /// Reserve entity IDs concurrently. /// /// Storage for entity generation and location is lazily allocated by calling `flush`. @@ -525,6 +534,7 @@ impl Entities { .map_or(false, |e| e.generation() == entity.generation) } + /// Clears all [`Entity`] from the World. pub fn clear(&mut self) { self.meta.clear(); self.pending.clear(); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 09b85e22040c6..14a8d8313caad 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -68,7 +68,7 @@ impl Default for World { fn default() -> Self { Self { id: WorldId::new().expect("More `bevy` `World`s have been created than is supported"), - entities: Default::default(), + entities: Entities::new(), components: Default::default(), archetypes: Default::default(), storages: Default::default(), From 6cad075b76bf60ba3fabf8cd5111824a81e2ba2a Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 02:26:25 -0800 Subject: [PATCH 05/14] Add basic type-level docs to Entities --- crates/bevy_ecs/src/entity/mod.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 582dc868bda24..baf6cda013784 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -265,6 +265,14 @@ impl<'a> Iterator for ReserveEntitiesIterator<'a> { impl<'a> core::iter::ExactSizeIterator for ReserveEntitiesIterator<'a> {} impl<'a> core::iter::FusedIterator for ReserveEntitiesIterator<'a> {} +/// A [`World`]'s internal metadata store on all of its entities. +/// +/// Contains metadata on: +/// - The generation of every entity. +/// - The alive/dead status of a particular entity. (i.e. "has entity 3 been despawned?") +/// - The entity's location in memory (via [`EntityLocation`]) +/// +/// [`World`]: crate::world::World #[derive(Debug)] pub struct Entities { meta: Vec, From fa8eaf1a97349cb9fd6d017c44fe3de61c9af496 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 02:39:59 -0800 Subject: [PATCH 06/14] Fix CI --- crates/bevy_ecs/src/entity/mod.rs | 11 ++++++-- crates/bevy_ecs/src/lib.rs | 47 +++++++------------------------ 2 files changed, 19 insertions(+), 39 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index baf6cda013784..fda6fa2e34fa0 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -116,6 +116,11 @@ pub(crate) enum AllocAtWithoutReplacement { } impl Entity { + #[cfg(test)] + pub(crate) const fn new(index: u32, generation: u32) -> Entity { + Entity { index, generation } + } + /// Creates a new entity reference with the specified `index` and a generation of 0. /// /// # Note @@ -683,11 +688,13 @@ impl Entities { self.meta.len() } + /// The total number of living entities. #[inline] pub fn len(&self) -> u32 { self.len } + /// Checks if any entity is currently alive. #[inline] pub fn is_empty(&self) -> bool { self.len == 0 @@ -739,7 +746,7 @@ mod tests { #[test] fn reserve_entity_len() { - let mut e = Entities::default(); + let mut e = Entities::new(); e.reserve_entity(); // SAFETY: entity_location is left invalid unsafe { e.flush(|_, _| {}) }; @@ -748,7 +755,7 @@ mod tests { #[test] fn get_reserved_and_invalid() { - let mut entities = Entities::default(); + let mut entities = Entities::new(); let e = entities.reserve_entity(); assert!(entities.contains(e)); assert!(entities.get(e).is_none()); diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index a91ed6634f1b6..c88313fafb8a7 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1465,7 +1465,7 @@ mod tests { let e3 = world_a.entities().reserve_entity(); world_a.flush(); - let world_a_max_entities = world_a.entities().meta.len(); + let world_a_max_entities = world_a.entities().len(); world_b .entities .reserve_entities(world_a_max_entities as u32); @@ -1474,10 +1474,7 @@ mod tests { let e4 = world_b.spawn(A(4)).id(); assert_eq!( e4, - Entity { - generation: 0, - index: 3, - }, + Entity::new(3, 0), "new entity is created immediately after world_a's max entity" ); assert!(world_b.get::(e1).is_none()); @@ -1508,10 +1505,7 @@ mod tests { "spawning into existing `world_b` entities works" ); - let e4_mismatched_generation = Entity { - generation: 1, - index: 3, - }; + let e4_mismatched_generation = Entity::new(3, 1); assert!( world_b.get_or_spawn(e4_mismatched_generation).is_none(), "attempting to spawn on top of an entity with a mismatched entity generation fails" @@ -1527,10 +1521,7 @@ mod tests { "failed mismatched spawn doesn't change existing entity" ); - let high_non_existent_entity = Entity { - generation: 0, - index: 6, - }; + let high_non_existent_entity = Entity::new(6, 0); world_b .get_or_spawn(high_non_existent_entity) .unwrap() @@ -1541,10 +1532,7 @@ mod tests { "inserting into newly allocated high / non-continous entity id works" ); - let high_non_existent_but_reserved_entity = Entity { - generation: 0, - index: 5, - }; + let high_non_existent_but_reserved_entity = Entity::new(5, 0); assert!( world_b.get_entity(high_non_existent_but_reserved_entity).is_none(), "entities between high-newly allocated entity and continuous block of existing entities don't exist" @@ -1560,22 +1548,10 @@ mod tests { assert_eq!( reserved_entities, vec![ - Entity { - generation: 0, - index: 5 - }, - Entity { - generation: 0, - index: 4 - }, - Entity { - generation: 0, - index: 7, - }, - Entity { - generation: 0, - index: 8, - }, + Entity::new(5, 0), + Entity::new(4, 0), + Entity::new(7, 0), + Entity::new(8, 0), ], "space between original entities and high entities is used for new entity ids" ); @@ -1624,10 +1600,7 @@ mod tests { let e0 = world.spawn(A(0)).id(); let e1 = Entity::from_raw(1); let e2 = world.spawn_empty().id(); - let invalid_e2 = Entity { - generation: 1, - index: e2.index, - }; + let invalid_e2 = Entity::new(e2.index(), 1); let values = vec![(e0, (B(0), C)), (e1, (B(1), C)), (invalid_e2, (B(2), C))]; From d83fc4c31c256c6fbe497e92914eebc95f94a03a Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 02:44:34 -0800 Subject: [PATCH 07/14] meta_len -> total_count --- crates/bevy_ecs/src/entity/mod.rs | 6 +++--- crates/bevy_render/src/lib.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index fda6fa2e34fa0..416a43a034128 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -682,13 +682,13 @@ impl Entities { self.len = count as u32; } - /// Accessor for getting the length of the vec in `self.meta` + /// The count of all entities in the [`World`], living and dead. #[inline] - pub fn meta_len(&self) -> usize { + pub fn total_count(&self) -> usize { self.meta.len() } - /// The total number of living entities. + /// The count of currently living entities. #[inline] pub fn len(&self) -> u32 { self.len diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index 7b6916a7535a8..f595023588e76 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -220,7 +220,7 @@ impl Plugin for RenderPlugin { // reserve all existing app entities for use in render_app // they can only be spawned using `get_or_spawn()` - let meta_len = app_world.entities().meta_len(); + let total_count = app_world.entities().total_count(); assert_eq!( render_app.world.entities().len(), @@ -233,7 +233,7 @@ impl Plugin for RenderPlugin { render_app .world .entities_mut() - .flush_and_reserve_invalid_assuming_no_entities(meta_len); + .flush_and_reserve_invalid_assuming_no_entities(total_count); } } From f0d3d4605dc918a74e812e21280ed199a631d364 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 02:45:26 -0800 Subject: [PATCH 08/14] Fix doc link --- crates/bevy_ecs/src/entity/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 416a43a034128..4f227f363c560 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -683,6 +683,8 @@ impl Entities { } /// The count of all entities in the [`World`], living and dead. + /// + /// [`World`]: crate::world::World #[inline] pub fn total_count(&self) -> usize { self.meta.len() From a3fd80f016bc71f4489d0e038fe8288f80d10e3f Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 02:59:21 -0800 Subject: [PATCH 09/14] Document MapEntities and MapEntitiesError --- crates/bevy_ecs/src/entity/map_entities.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index ae2d0476c44bc..0a763869548a2 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -2,6 +2,7 @@ use crate::entity::Entity; use bevy_utils::{Entry, HashMap}; use std::fmt; +/// The errors that might be returned while using [`MapEntities::map_entities`]. #[derive(Debug)] pub enum MapEntitiesError { EntityNotFound(Entity), @@ -19,7 +20,12 @@ impl fmt::Display for MapEntitiesError { } } +/// Allows mapping internal [`Entity`] references from one entity space to another. pub trait MapEntities { + /// Updates all [`Entity`] references stored inside using `entity_map`. + /// + /// Implementors should look up any and all [`Entity`] values stored within and + /// update them to the mapped values via `entity_map`. fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError>; } From 4ac6307bd8e26d9811b45e5a9d107e03bc036517 Mon Sep 17 00:00:00 2001 From: James Liu Date: Thu, 24 Nov 2022 05:59:12 -0800 Subject: [PATCH 10/14] Add missing "internal" Co-authored-by: Alice Cecile --- crates/bevy_ecs/src/entity/map_entities.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 0a763869548a2..9213388acb453 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -20,7 +20,7 @@ impl fmt::Display for MapEntitiesError { } } -/// Allows mapping internal [`Entity`] references from one entity space to another. +/// Allows mapping [`Entity`] references from one entity space to another. pub trait MapEntities { /// Updates all [`Entity`] references stored inside using `entity_map`. /// From 9cf5feef66a1e0e2377816bad0073c65919e542c Mon Sep 17 00:00:00 2001 From: James Liu Date: Thu, 24 Nov 2022 07:35:41 -0800 Subject: [PATCH 11/14] Wording on Entities' type level docs Co-authored-by: Jakob Hellermann --- 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 4f227f363c560..a76a4973330af 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -275,7 +275,7 @@ impl<'a> core::iter::FusedIterator for ReserveEntitiesIterator<'a> {} /// Contains metadata on: /// - The generation of every entity. /// - The alive/dead status of a particular entity. (i.e. "has entity 3 been despawned?") -/// - The entity's location in memory (via [`EntityLocation`]) +/// - The location of the entity's components in memory (via [`EntityLocation`]) /// /// [`World`]: crate::world::World #[derive(Debug)] From 693dd03adc7b08a7cb91abd48fff197cc15b96f0 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 07:38:23 -0800 Subject: [PATCH 12/14] Fix docs on total_count --- crates/bevy_ecs/src/entity/mod.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index a76a4973330af..aa115bef5ffab 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -293,8 +293,8 @@ pub struct Entities { /// `reserve_entities` or `reserve_entity()`. They are now waiting for `flush()` to make them /// fully allocated. /// - /// - The count of new IDs that do not yet exist in `self.meta()`, but which we have handed out - /// and reserved. `flush()` will allocate room for them in `self.meta()`. + /// - The count of new IDs that do not yet exist in `self.meta`, but which we have handed out + /// and reserved. `flush()` will allocate room for them in `self.meta`. /// /// The contents of `pending` look like this: /// @@ -682,7 +682,11 @@ impl Entities { self.len = count as u32; } - /// The count of all entities in the [`World`], living and dead. + /// The count of all entities in the [`World`] that have ever been allocated + /// including the entities that are currently freed. + /// + /// This does not include entities that have been reserved but have never been + /// allocated yet. /// /// [`World`]: crate::world::World #[inline] @@ -690,13 +694,13 @@ impl Entities { self.meta.len() } - /// The count of currently living entities. + /// The count of currently allocated entities. #[inline] pub fn len(&self) -> u32 { self.len } - /// Checks if any entity is currently alive. + /// Checks if any entity is currently active. #[inline] pub fn is_empty(&self) -> bool { self.len == 0 From 17ee5378e9193f7f73183861ef07f4167db10b34 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 07:41:13 -0800 Subject: [PATCH 13/14] Update Enities::set's safety docs --- crates/bevy_ecs/src/entity/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index aa115bef5ffab..44c2e0465f10a 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -575,7 +575,8 @@ impl Entities { /// /// # Safety /// - `index` must be a valid entity index. - /// - `location` must be valid for the entity at `index` is location + /// - `location` must be valid for the entity at `index` or immediately made valid afterwards + /// before handing control to unknown code. pub(crate) unsafe fn set(&mut self, index: u32, location: EntityLocation) { // SAFETY: Caller guarentees that `index` a valid entity index self.meta.get_unchecked_mut(index as usize).location = location; @@ -685,7 +686,7 @@ impl Entities { /// The count of all entities in the [`World`] that have ever been allocated /// including the entities that are currently freed. /// - /// This does not include entities that have been reserved but have never been + /// This does not include entities that have been reserved but have never been /// allocated yet. /// /// [`World`]: crate::world::World From ded316fc50ff9fd16e43a1f84f6ac844cab6d114 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Nov 2022 18:08:22 -0800 Subject: [PATCH 14/14] Apply review comments from #3985. --- crates/bevy_ecs/src/entity/map_entities.rs | 32 ++++++++++++---------- crates/bevy_ecs/src/entity/mod.rs | 4 +++ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index d697247347ce8..516588d1597c9 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -8,7 +8,6 @@ pub enum MapEntitiesError { EntityNotFound(Entity), } -<<<<<<< HEAD impl std::error::Error for MapEntitiesError {} impl fmt::Display for MapEntitiesError { @@ -21,25 +20,28 @@ impl fmt::Display for MapEntitiesError { } } -/// Operation to map all contained [`Entity`](crate::entity::Entity) fields in -/// a component to new values. -/// -/// If a component contains [`Entity`](crate::entity::Entity) values -/// that refer to other entities in the same world and scene functionality -/// is used to create such components, this trait must be implemented. The -/// is to replace all [`Entity`](crate::entity::Entity) values in the -/// component with values looked up from the given [`EntityMap`]. +/// Operation to map all contained [`Entity`] fields in a type to new values. +/// +/// As entity IDs are valid only for the [`World`] they're sourced from, using [`Entity`] +/// as references in components copied from another world will be invalid. This trait +/// allows defining custom mappings for these references via [`EntityMap`]. +/// +/// Implementing this trait correctly is required for properly loading components +/// with entity references from scenes. +/// +/// ## Example +/// +/// ```rust +/// use bevy_ecs::prelude::*; +/// use bevy_ecs::entity::{EntityMap, MapEntities, MapEntitiesError}; /// -/// Implementing this trait is pretty straightforward: -/// -/// ``` /// #[derive(Component)] -/// struct MyEntityRefs { +/// struct Spring { /// a: Entity, /// b: Entity, /// } /// -/// impl MapEntities for MyEntityRefs { +/// impl MapEntities for Spring { /// fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError> { /// self.a = entity_map.get(self.a)?; /// self.b = entity_map.get(self.b)?; @@ -47,6 +49,8 @@ impl fmt::Display for MapEntitiesError { /// } /// } /// ``` +/// +/// [`World`]: crate::world::World pub trait MapEntities { /// Updates all [`Entity`] references stored inside using `entity_map`. /// diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 44c2e0465f10a..7614f24912839 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -56,6 +56,9 @@ type IdCursor = isize; /// The identifier is implemented using a [generational index]: a combination of an index and a generation. /// This allows fast insertion after data removal in an array while minimizing loss of spatial locality. /// +/// These identifiers are only valid on the [`World`] it's sourced from. Attempting to use an `Entity` to +/// fetch entity components or metadata from a different world will either fail or return unexpected results. +/// /// [generational index]: https://lucassardois.medium.com/generational-indices-guide-8e3c5f7fd594 /// /// # Usage @@ -103,6 +106,7 @@ type IdCursor = isize; /// [`EntityMut::id`]: crate::world::EntityMut::id /// [`EntityCommands`]: crate::system::EntityCommands /// [`Query::get`]: crate::system::Query::get +/// [`World`]: crate::world::World #[derive(Clone, Copy, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct Entity { generation: u32,