From 86bd6bb40b75d1ac57d403e4136e786cfa2b18ea Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 19 Jan 2024 20:57:54 -0500 Subject: [PATCH 01/21] add simple entity mapper --- crates/bevy_ecs/src/entity/map_entities.rs | 37 ++++++++++++++++--- crates/bevy_ecs/src/entity/mod.rs | 12 ++++++ crates/bevy_ecs/src/reflect/map_entities.rs | 20 +++++----- .../bevy_hierarchy/src/components/children.rs | 11 +++++- .../bevy_hierarchy/src/components/parent.rs | 9 ++++- crates/bevy_render/src/mesh/mesh/skinning.rs | 11 +++++- crates/bevy_scene/src/dynamic_scene.rs | 2 +- crates/bevy_scene/src/scene.rs | 2 +- crates/bevy_scene/src/serde.rs | 10 +++-- crates/bevy_window/src/window.rs | 14 ++++++- 10 files changed, 100 insertions(+), 28 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 4c99ff69d5f51..8ed5373ef43f4 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -18,7 +18,7 @@ use bevy_utils::EntityHashMap; /// /// ``` /// use bevy_ecs::prelude::*; -/// use bevy_ecs::entity::{EntityMapper, MapEntities}; +/// use bevy_ecs::entity::{EntityMapper, MapEntities, SimpleEntityMapper}; /// /// #[derive(Component)] /// struct Spring { @@ -27,9 +27,13 @@ use bevy_utils::EntityHashMap; /// } /// /// impl MapEntities for Spring { -/// fn map_entities(&mut self, entity_mapper: &mut EntityMapper) { -/// self.a = entity_mapper.get_or_reserve(self.a); -/// self.b = entity_mapper.get_or_reserve(self.b); +/// fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { +/// self.a.map_or_gen_entities(entity_mapper); +/// self.b.map_or_gen_entities(entity_mapper); +/// } +/// fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { +/// self.a.map_entities(entity_mapper); +/// self.b.map_entities(entity_mapper) /// } /// } /// ``` @@ -39,7 +43,30 @@ pub trait MapEntities { /// /// Implementors should look up any and all [`Entity`] values stored within and /// update them to the mapped values via `entity_mapper`. - fn map_entities(&mut self, entity_mapper: &mut EntityMapper); + fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper); + + fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper); +} + +/// Similar to `EntityMapper`, but does not allocate new [`Entity`] references in case we couldn't map the entity. +pub struct SimpleEntityMapper<'m> { + map: &'m EntityHashMap, +} + +impl<'m> SimpleEntityMapper<'m> { + pub fn new(map: &'m EntityHashMap) -> Self { + Self { map } + } + + /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. + pub fn get(&mut self, entity: Entity) -> Option { + self.map.get(&entity).copied() + } + + /// Gets a reference to the underlying [`EntityHashMap`]. + pub fn get_map(&'m self) -> &'m EntityHashMap { + self.map + } } /// A wrapper for [`EntityHashMap`], augmenting it with the ability to allocate new [`Entity`] references in a destination diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index ab7d167cff67c..a10600ffa8a19 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -136,6 +136,18 @@ pub struct Entity { index: u32, } +impl MapEntities for Entity { + fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { + *self = entity_mapper.get_or_reserve(*self); + } + + fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + if let Some(mapped) = entity_mapper.get(*self) { + *self = mapped; + } + } +} + // By not short-circuiting in comparisons, we get better codegen. // See impl PartialEq for Entity { diff --git a/crates/bevy_ecs/src/reflect/map_entities.rs b/crates/bevy_ecs/src/reflect/map_entities.rs index 27bcb03653546..6c6e2d5804ce9 100644 --- a/crates/bevy_ecs/src/reflect/map_entities.rs +++ b/crates/bevy_ecs/src/reflect/map_entities.rs @@ -13,8 +13,8 @@ use bevy_utils::EntityHashMap; /// See [`MapEntities`] for more information. #[derive(Clone)] pub struct ReflectMapEntities { - map_all_entities: fn(&mut World, &mut EntityMapper), - map_entities: fn(&mut World, &mut EntityMapper, &[Entity]), + map_or_gen_all_entities: fn(&mut World, &mut EntityMapper), + map_or_gen_entities: fn(&mut World, &mut EntityMapper, &[Entity]), } impl ReflectMapEntities { @@ -27,12 +27,12 @@ impl ReflectMapEntities { /// An example of this: A scene can be loaded with `Parent` components, but then a `Parent` component can be added /// to these entities after they have been loaded. If you reload the scene using [`map_all_entities`](Self::map_all_entities), those `Parent` /// components with already valid entity references could be updated to point at something else entirely. - pub fn map_all_entities( + pub fn map_or_gen_all_entities( &self, world: &mut World, entity_map: &mut EntityHashMap, ) { - EntityMapper::world_scope(entity_map, world, self.map_all_entities); + EntityMapper::world_scope(entity_map, world, self.map_or_gen_all_entities); } /// A general method for applying [`MapEntities`] behavior to elements in an [`EntityHashMap`]. Unlike @@ -41,14 +41,14 @@ impl ReflectMapEntities { /// /// This is useful mostly for when you need to be careful not to update components that already contain valid entity /// values. See [`map_all_entities`](Self::map_all_entities) for more details. - pub fn map_entities( + pub fn map_or_gen_entities( &self, world: &mut World, entity_map: &mut EntityHashMap, entities: &[Entity], ) { EntityMapper::world_scope(entity_map, world, |world, mapper| { - (self.map_entities)(world, mapper, entities); + (self.map_or_gen_entities)(world, mapper, entities); }); } } @@ -56,14 +56,14 @@ impl ReflectMapEntities { impl FromType for ReflectMapEntities { fn from_type() -> Self { ReflectMapEntities { - map_entities: |world, entity_mapper, entities| { + map_or_gen_entities: |world, entity_mapper, entities| { for &entity in entities { if let Some(mut component) = world.get_mut::(entity) { - component.map_entities(entity_mapper); + component.map_or_gen_entities(entity_mapper); } } }, - map_all_entities: |world, entity_mapper| { + map_or_gen_all_entities: |world, entity_mapper| { let entities = entity_mapper .get_map() .values() @@ -71,7 +71,7 @@ impl FromType for ReflectMapEntities { .collect::>(); for entity in &entities { if let Some(mut component) = world.get_mut::(*entity) { - component.map_entities(entity_mapper); + component.map_or_gen_entities(entity_mapper); } } }, diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index 57556747fcef5..fb33925715dc0 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -9,6 +9,7 @@ use bevy_ecs::{ use bevy_utils::smallvec::SmallVec; use core::slice; use std::ops::Deref; +use bevy_ecs::entity::SimpleEntityMapper; /// Contains references to the child entities of this entity. /// @@ -29,9 +30,15 @@ use std::ops::Deref; pub struct Children(pub(crate) SmallVec<[Entity; 8]>); impl MapEntities for Children { - fn map_entities(&mut self, entity_mapper: &mut EntityMapper) { + fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { for entity in &mut self.0 { - *entity = entity_mapper.get_or_reserve(*entity); + entity.map_or_gen_entities(entity_mapper); + } + } + + fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + for entity in &mut self.0 { + entity.map_entities(entity_mapper); } } } diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index 2ba6a10ccb779..2c5b19f8475b6 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -6,6 +6,7 @@ use bevy_ecs::{ world::{FromWorld, World}, }; use std::ops::Deref; +use bevy_ecs::entity::SimpleEntityMapper; /// Holds a reference to the parent entity of this entity. /// This component should only be present on entities that actually have a parent entity. @@ -56,8 +57,12 @@ impl FromWorld for Parent { } impl MapEntities for Parent { - fn map_entities(&mut self, entity_mapper: &mut EntityMapper) { - self.0 = entity_mapper.get_or_reserve(self.0); + fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { + self.0.map_or_gen_entities(entity_mapper); + } + + fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + self.0.map_entities(entity_mapper); } } diff --git a/crates/bevy_render/src/mesh/mesh/skinning.rs b/crates/bevy_render/src/mesh/mesh/skinning.rs index e5c4cd9fcc925..523d935fd021d 100644 --- a/crates/bevy_render/src/mesh/mesh/skinning.rs +++ b/crates/bevy_render/src/mesh/mesh/skinning.rs @@ -8,6 +8,7 @@ use bevy_ecs::{ use bevy_math::Mat4; use bevy_reflect::{Reflect, TypePath}; use std::ops::Deref; +use bevy_ecs::entity::SimpleEntityMapper; #[derive(Component, Debug, Default, Clone, Reflect)] #[reflect(Component, MapEntities)] @@ -17,9 +18,15 @@ pub struct SkinnedMesh { } impl MapEntities for SkinnedMesh { - fn map_entities(&mut self, entity_mapper: &mut EntityMapper) { + fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { for joint in &mut self.joints { - *joint = entity_mapper.get_or_reserve(*joint); + *joint.map_or_gen_entities(entity_mapper); + } + } + + fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + for joint in &mut self.joints { + *joint.map_entities(entity_mapper); } } } diff --git a/crates/bevy_scene/src/dynamic_scene.rs b/crates/bevy_scene/src/dynamic_scene.rs index 676fb9fdcd834..d880b5255aed4 100644 --- a/crates/bevy_scene/src/dynamic_scene.rs +++ b/crates/bevy_scene/src/dynamic_scene.rs @@ -149,7 +149,7 @@ impl DynamicScene { "we should be getting TypeId from this TypeRegistration in the first place", ); if let Some(map_entities_reflect) = registration.data::() { - map_entities_reflect.map_entities(world, entity_map, &entities); + map_entities_reflect.map_or_gen_entities(world, entity_map, &entities); } } diff --git a/crates/bevy_scene/src/scene.rs b/crates/bevy_scene/src/scene.rs index 28cf824aa061c..b4c28ba3ad3a0 100644 --- a/crates/bevy_scene/src/scene.rs +++ b/crates/bevy_scene/src/scene.rs @@ -130,7 +130,7 @@ impl Scene { for registration in type_registry.iter() { if let Some(map_entities_reflect) = registration.data::() { - map_entities_reflect.map_all_entities(world, &mut instance_info.entity_map); + map_entities_reflect.map_or_gen_all_entities(world, &mut instance_info.entity_map); } } diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 0567a9451b617..080d2f55b0d6e 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -499,7 +499,7 @@ mod tests { use crate::ron; use crate::serde::{SceneDeserializer, SceneSerializer}; use crate::{DynamicScene, DynamicSceneBuilder}; - use bevy_ecs::entity::{Entity, EntityMapper, MapEntities}; + use bevy_ecs::entity::{Entity, EntityMapper, MapEntities, SimpleEntityMapper}; use bevy_ecs::prelude::{Component, ReflectComponent, ReflectResource, Resource, World}; use bevy_ecs::query::{With, Without}; use bevy_ecs::reflect::{AppTypeRegistry, ReflectMapEntities}; @@ -550,8 +550,12 @@ mod tests { struct MyEntityRef(Entity); impl MapEntities for MyEntityRef { - fn map_entities(&mut self, entity_mapper: &mut EntityMapper) { - self.0 = entity_mapper.get_or_reserve(self.0); + fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { + self.0.map_or_gen_entities(entity_mapper); + } + + fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + self.0.map_entities(entity_mapper); } } diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index ee05635fac6e1..73732f67f6c9b 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -2,6 +2,7 @@ use bevy_ecs::{ entity::{Entity, EntityMapper, MapEntities}, prelude::{Component, ReflectComponent}, }; +use bevy_ecs::entity::SimpleEntityMapper; use bevy_math::{DVec2, IVec2, Vec2}; use bevy_reflect::{std_traits::ReflectDefault, Reflect}; @@ -59,10 +60,19 @@ impl WindowRef { } impl MapEntities for WindowRef { - fn map_entities(&mut self, entity_mapper: &mut EntityMapper) { + fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { match self { Self::Entity(entity) => { - *entity = entity_mapper.get_or_reserve(*entity); + *entity.map_entities(entity_mapper); + } + Self::Primary => {} + }; + } + + fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + match self { + Self::Entity(entity) => { + *entity.map_or_gen_entities(entity_mapper); } Self::Primary => {} }; From bb81f220a78e8f5428d957fea80e19729f18536a Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 19 Jan 2024 21:04:56 -0500 Subject: [PATCH 02/21] add docstring --- crates/bevy_ecs/src/entity/map_entities.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 8ed5373ef43f4..9dc680cbf9201 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -45,6 +45,9 @@ pub trait MapEntities { /// update them to the mapped values via `entity_mapper`. fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper); + /// Updates all [`Entity`] references stored inside using `entity_mapper`. + /// + /// Only updates the references for which there is a mapping. fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper); } @@ -58,7 +61,7 @@ impl<'m> SimpleEntityMapper<'m> { Self { map } } - /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. + /// Returns the corresponding mapped entity or None if it is absent. pub fn get(&mut self, entity: Entity) -> Option { self.map.get(&entity).copied() } From a966db99b4fb016bd5e7692c8667c4ac686a2046 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 20 Jan 2024 10:45:00 -0500 Subject: [PATCH 03/21] address comments on mtuability --- crates/bevy_ecs/src/entity/map_entities.rs | 6 +++--- crates/bevy_ecs/src/entity/mod.rs | 2 +- crates/bevy_hierarchy/src/components/children.rs | 2 +- crates/bevy_hierarchy/src/components/parent.rs | 2 +- crates/bevy_render/src/mesh/mesh/skinning.rs | 2 +- crates/bevy_scene/src/serde.rs | 2 +- crates/bevy_window/src/window.rs | 6 +++--- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 9dc680cbf9201..dd2cce89ca578 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -31,7 +31,7 @@ use bevy_utils::EntityHashMap; /// self.a.map_or_gen_entities(entity_mapper); /// self.b.map_or_gen_entities(entity_mapper); /// } -/// fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { +/// fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { /// self.a.map_entities(entity_mapper); /// self.b.map_entities(entity_mapper) /// } @@ -48,7 +48,7 @@ pub trait MapEntities { /// Updates all [`Entity`] references stored inside using `entity_mapper`. /// /// Only updates the references for which there is a mapping. - fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper); + fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper); } /// Similar to `EntityMapper`, but does not allocate new [`Entity`] references in case we couldn't map the entity. @@ -62,7 +62,7 @@ impl<'m> SimpleEntityMapper<'m> { } /// Returns the corresponding mapped entity or None if it is absent. - pub fn get(&mut self, entity: Entity) -> Option { + pub fn get(&self, entity: Entity) -> Option { self.map.get(&entity).copied() } diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index a10600ffa8a19..0792ce728ca52 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -141,7 +141,7 @@ impl MapEntities for Entity { *self = entity_mapper.get_or_reserve(*self); } - fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { if let Some(mapped) = entity_mapper.get(*self) { *self = mapped; } diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index fb33925715dc0..6df75d6248c21 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -36,7 +36,7 @@ impl MapEntities for Children { } } - fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { for entity in &mut self.0 { entity.map_entities(entity_mapper); } diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index 2c5b19f8475b6..1ea9836fe2b9c 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -61,7 +61,7 @@ impl MapEntities for Parent { self.0.map_or_gen_entities(entity_mapper); } - fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { self.0.map_entities(entity_mapper); } } diff --git a/crates/bevy_render/src/mesh/mesh/skinning.rs b/crates/bevy_render/src/mesh/mesh/skinning.rs index 523d935fd021d..86323a26e7e48 100644 --- a/crates/bevy_render/src/mesh/mesh/skinning.rs +++ b/crates/bevy_render/src/mesh/mesh/skinning.rs @@ -24,7 +24,7 @@ impl MapEntities for SkinnedMesh { } } - fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { for joint in &mut self.joints { *joint.map_entities(entity_mapper); } diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 080d2f55b0d6e..adad62b1798ca 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -554,7 +554,7 @@ mod tests { self.0.map_or_gen_entities(entity_mapper); } - fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { self.0.map_entities(entity_mapper); } } diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index 73732f67f6c9b..a09d2d2440d0c 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -63,16 +63,16 @@ impl MapEntities for WindowRef { fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { match self { Self::Entity(entity) => { - *entity.map_entities(entity_mapper); + *entity.map_or_gen_entities(entity_mapper); } Self::Primary => {} }; } - fn map_entities(&mut self, entity_mapper: &mut SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { match self { Self::Entity(entity) => { - *entity.map_or_gen_entities(entity_mapper); + *entity.map_entities(entity_mapper); } Self::Primary => {} }; From 709cc6915e347d05059b525357473757b023f214 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 20 Jan 2024 11:20:32 -0500 Subject: [PATCH 04/21] make MapEntities work with any mapper --- crates/bevy_ecs/src/entity/map_entities.rs | 34 ++++++++++++------- crates/bevy_ecs/src/entity/mod.rs | 10 ++---- crates/bevy_ecs/src/reflect/map_entities.rs | 16 ++++----- .../bevy_hierarchy/src/components/children.rs | 9 ++--- .../bevy_hierarchy/src/components/parent.rs | 8 ++--- crates/bevy_render/src/mesh/mesh/skinning.rs | 9 ++--- crates/bevy_scene/src/serde.rs | 8 ++--- crates/bevy_window/src/window.rs | 12 ++----- 8 files changed, 41 insertions(+), 65 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index dd2cce89ca578..2159b079adec2 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -18,7 +18,7 @@ use bevy_utils::EntityHashMap; /// /// ``` /// use bevy_ecs::prelude::*; -/// use bevy_ecs::entity::{EntityMapper, MapEntities, SimpleEntityMapper}; +/// use bevy_ecs::entity::{EntityMapper, MapEntities, Mapper, SimpleEntityMapper}; /// /// #[derive(Component)] /// struct Spring { @@ -27,11 +27,7 @@ use bevy_utils::EntityHashMap; /// } /// /// impl MapEntities for Spring { -/// fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { -/// self.a.map_or_gen_entities(entity_mapper); -/// self.b.map_or_gen_entities(entity_mapper); -/// } -/// fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { +/// fn map_entities(&mut self, entity_mapper: &mut M) { /// self.a.map_entities(entity_mapper); /// self.b.map_entities(entity_mapper) /// } @@ -39,16 +35,16 @@ use bevy_utils::EntityHashMap; /// ``` /// pub trait MapEntities { - /// Updates all [`Entity`] references stored inside using `entity_mapper`. - /// - /// Implementors should look up any and all [`Entity`] values stored within and - /// update them to the mapped values via `entity_mapper`. - fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper); - /// Updates all [`Entity`] references stored inside using `entity_mapper`. /// /// Only updates the references for which there is a mapping. - fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper); + fn map_entities(&mut self, entity_mapper: &mut M); +} + +pub trait Mapper { + /// Map an entity to another entity + fn map(&mut self, entity: Entity) -> Entity; + } /// Similar to `EntityMapper`, but does not allocate new [`Entity`] references in case we couldn't map the entity. @@ -56,6 +52,12 @@ pub struct SimpleEntityMapper<'m> { map: &'m EntityHashMap, } +impl Mapper for SimpleEntityMapper<'_> { + fn map(&mut self, entity: Entity) -> Entity { + self.get(entity).unwrap_or(entity) + } +} + impl<'m> SimpleEntityMapper<'m> { pub fn new(map: &'m EntityHashMap) -> Self { Self { map } @@ -72,6 +74,12 @@ impl<'m> SimpleEntityMapper<'m> { } } +impl Mapper for EntityMapper { + fn map(&mut self, entity: Entity) -> Entity { + self.get_or_reserve(entity) + } +} + /// A wrapper for [`EntityHashMap`], augmenting it with the ability to allocate new [`Entity`] references in a destination /// world. These newly allocated references are guaranteed to never point to any living entity in that world. /// diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 0792ce728ca52..21023c36a641d 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -137,14 +137,8 @@ pub struct Entity { } impl MapEntities for Entity { - fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { - *self = entity_mapper.get_or_reserve(*self); - } - - fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { - if let Some(mapped) = entity_mapper.get(*self) { - *self = mapped; - } + fn map_entities(&mut self, mut entity_mapper: M) { + entity_mapper.map(*self); } } diff --git a/crates/bevy_ecs/src/reflect/map_entities.rs b/crates/bevy_ecs/src/reflect/map_entities.rs index 6c6e2d5804ce9..4043b3f36caf0 100644 --- a/crates/bevy_ecs/src/reflect/map_entities.rs +++ b/crates/bevy_ecs/src/reflect/map_entities.rs @@ -13,8 +13,8 @@ use bevy_utils::EntityHashMap; /// See [`MapEntities`] for more information. #[derive(Clone)] pub struct ReflectMapEntities { - map_or_gen_all_entities: fn(&mut World, &mut EntityMapper), - map_or_gen_entities: fn(&mut World, &mut EntityMapper, &[Entity]), + map_all_entities: fn(&mut World, &mut EntityMapper), + map_entities: fn(&mut World, &mut EntityMapper, &[Entity]), } impl ReflectMapEntities { @@ -32,7 +32,7 @@ impl ReflectMapEntities { world: &mut World, entity_map: &mut EntityHashMap, ) { - EntityMapper::world_scope(entity_map, world, self.map_or_gen_all_entities); + EntityMapper::world_scope(entity_map, world, self.map_all_entities); } /// A general method for applying [`MapEntities`] behavior to elements in an [`EntityHashMap`]. Unlike @@ -48,7 +48,7 @@ impl ReflectMapEntities { entities: &[Entity], ) { EntityMapper::world_scope(entity_map, world, |world, mapper| { - (self.map_or_gen_entities)(world, mapper, entities); + (self.map_entities)(world, mapper, entities); }); } } @@ -56,14 +56,14 @@ impl ReflectMapEntities { impl FromType for ReflectMapEntities { fn from_type() -> Self { ReflectMapEntities { - map_or_gen_entities: |world, entity_mapper, entities| { + map_entities: |world, entity_mapper, entities| { for &entity in entities { if let Some(mut component) = world.get_mut::(entity) { - component.map_or_gen_entities(entity_mapper); + component.map_entities(entity_mapper); } } }, - map_or_gen_all_entities: |world, entity_mapper| { + map_all_entities: |world, entity_mapper| { let entities = entity_mapper .get_map() .values() @@ -71,7 +71,7 @@ impl FromType for ReflectMapEntities { .collect::>(); for entity in &entities { if let Some(mut component) = world.get_mut::(*entity) { - component.map_or_gen_entities(entity_mapper); + component.map_entities(entity_mapper); } } }, diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index 6df75d6248c21..97eed59944e8e 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -9,7 +9,7 @@ use bevy_ecs::{ use bevy_utils::smallvec::SmallVec; use core::slice; use std::ops::Deref; -use bevy_ecs::entity::SimpleEntityMapper; +use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; /// Contains references to the child entities of this entity. /// @@ -30,13 +30,8 @@ use bevy_ecs::entity::SimpleEntityMapper; pub struct Children(pub(crate) SmallVec<[Entity; 8]>); impl MapEntities for Children { - fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { - for entity in &mut self.0 { - entity.map_or_gen_entities(entity_mapper); - } - } - fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &mut M) { for entity in &mut self.0 { entity.map_entities(entity_mapper); } diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index 1ea9836fe2b9c..a6e82290290ee 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -6,7 +6,7 @@ use bevy_ecs::{ world::{FromWorld, World}, }; use std::ops::Deref; -use bevy_ecs::entity::SimpleEntityMapper; +use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; /// Holds a reference to the parent entity of this entity. /// This component should only be present on entities that actually have a parent entity. @@ -57,11 +57,7 @@ impl FromWorld for Parent { } impl MapEntities for Parent { - fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { - self.0.map_or_gen_entities(entity_mapper); - } - - fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &mut M) { self.0.map_entities(entity_mapper); } } diff --git a/crates/bevy_render/src/mesh/mesh/skinning.rs b/crates/bevy_render/src/mesh/mesh/skinning.rs index 86323a26e7e48..982f7275aa7b0 100644 --- a/crates/bevy_render/src/mesh/mesh/skinning.rs +++ b/crates/bevy_render/src/mesh/mesh/skinning.rs @@ -8,7 +8,7 @@ use bevy_ecs::{ use bevy_math::Mat4; use bevy_reflect::{Reflect, TypePath}; use std::ops::Deref; -use bevy_ecs::entity::SimpleEntityMapper; +use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; #[derive(Component, Debug, Default, Clone, Reflect)] #[reflect(Component, MapEntities)] @@ -18,13 +18,8 @@ pub struct SkinnedMesh { } impl MapEntities for SkinnedMesh { - fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { - for joint in &mut self.joints { - *joint.map_or_gen_entities(entity_mapper); - } - } - fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &mut M) { for joint in &mut self.joints { *joint.map_entities(entity_mapper); } diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index adad62b1798ca..779b0fb1d73cd 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -499,7 +499,7 @@ mod tests { use crate::ron; use crate::serde::{SceneDeserializer, SceneSerializer}; use crate::{DynamicScene, DynamicSceneBuilder}; - use bevy_ecs::entity::{Entity, EntityMapper, MapEntities, SimpleEntityMapper}; + use bevy_ecs::entity::{Entity, EntityMapper, MapEntities, Mapper, SimpleEntityMapper}; use bevy_ecs::prelude::{Component, ReflectComponent, ReflectResource, Resource, World}; use bevy_ecs::query::{With, Without}; use bevy_ecs::reflect::{AppTypeRegistry, ReflectMapEntities}; @@ -550,11 +550,7 @@ mod tests { struct MyEntityRef(Entity); impl MapEntities for MyEntityRef { - fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { - self.0.map_or_gen_entities(entity_mapper); - } - - fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &M) { self.0.map_entities(entity_mapper); } } diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index a09d2d2440d0c..5dcfcb56ff154 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -2,7 +2,7 @@ use bevy_ecs::{ entity::{Entity, EntityMapper, MapEntities}, prelude::{Component, ReflectComponent}, }; -use bevy_ecs::entity::SimpleEntityMapper; +use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; use bevy_math::{DVec2, IVec2, Vec2}; use bevy_reflect::{std_traits::ReflectDefault, Reflect}; @@ -60,16 +60,8 @@ impl WindowRef { } impl MapEntities for WindowRef { - fn map_or_gen_entities(&mut self, entity_mapper: &mut EntityMapper) { - match self { - Self::Entity(entity) => { - *entity.map_or_gen_entities(entity_mapper); - } - Self::Primary => {} - }; - } - fn map_entities(&mut self, entity_mapper: &SimpleEntityMapper) { + fn map_entities(&mut self, entity_mapper: &mut M) { match self { Self::Entity(entity) => { *entity.map_entities(entity_mapper); From c463aed174c9e3c44ae279e759abae8fc0bf932e Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 20 Jan 2024 12:23:58 -0500 Subject: [PATCH 05/21] lint --- crates/bevy_ecs/src/entity/map_entities.rs | 45 ++++++++++++++++--- crates/bevy_ecs/src/entity/mod.rs | 3 +- crates/bevy_ecs/src/reflect/map_entities.rs | 4 +- .../bevy_hierarchy/src/components/children.rs | 3 +- .../bevy_hierarchy/src/components/parent.rs | 2 +- crates/bevy_render/src/mesh/mesh/skinning.rs | 3 +- crates/bevy_scene/src/dynamic_scene.rs | 2 +- crates/bevy_scene/src/scene.rs | 2 +- crates/bevy_window/src/window.rs | 3 +- 9 files changed, 49 insertions(+), 18 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 2159b079adec2..c6cf95f876031 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -9,7 +9,11 @@ use bevy_utils::EntityHashMap; /// /// 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 [`EntityHashMap`] +/// allows defining custom mappings for these references via a [`Mapper`], which is a type that knows +/// how to perform entity mapping. (usually by using a [`EntityHashMap`]) +/// +/// [`MapEntities`] is already implemented for [`Entity`], so you just need to call [`Entity::map_entities`] +/// for all the [`Entity`] fields in your type. /// /// Implementing this trait correctly is required for properly loading components /// with entity references from scenes. @@ -36,15 +40,17 @@ use bevy_utils::EntityHashMap; /// pub trait MapEntities { /// Updates all [`Entity`] references stored inside using `entity_mapper`. - /// - /// Only updates the references for which there is a mapping. fn map_entities(&mut self, entity_mapper: &mut M); } +/// This traits defines a type that knows how to map [`Entity`] references. +/// +/// Two implementations are provided: +/// - `SimpleEntityMapper`: tries to map the [`Entity`] reference, but if it can't, it returns the same [`Entity`] reference. +/// - `EntityMapper`: tries to map the [`Entity`] reference, but if it can't, it allocates a new [`Entity`] reference. pub trait Mapper { /// Map an entity to another entity fn map(&mut self, entity: Entity) -> Entity; - } /// Similar to `EntityMapper`, but does not allocate new [`Entity`] references in case we couldn't map the entity. @@ -53,6 +59,7 @@ pub struct SimpleEntityMapper<'m> { } impl Mapper for SimpleEntityMapper<'_> { + /// Map the entity to another entity, or return the same entity if we couldn't map it. fn map(&mut self, entity: Entity) -> Entity { self.get(entity).unwrap_or(entity) } @@ -74,7 +81,8 @@ impl<'m> SimpleEntityMapper<'m> { } } -impl Mapper for EntityMapper { +impl Mapper for EntityMapper<'_> { + /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. fn map(&mut self, entity: Entity) -> Entity { self.get_or_reserve(entity) } @@ -177,10 +185,35 @@ mod tests { use bevy_utils::EntityHashMap; use crate::{ - entity::{Entity, EntityMapper}, + entity::map_entities::Mapper, + entity::{Entity, EntityMapper, SimpleEntityMapper}, world::World, }; + #[test] + fn simple_entity_mapper() { + const FIRST_IDX: u32 = 1; + const SECOND_IDX: u32 = 2; + + const MISSING_IDX: u32 = 10; + + let mut map = EntityHashMap::default(); + map.insert(Entity::from_raw(FIRST_IDX), Entity::from_raw(SECOND_IDX)); + let mut mapper = SimpleEntityMapper::new(&map); + + // entity is mapped correctly if it exists in the map + assert_eq!( + mapper.map(Entity::from_raw(FIRST_IDX)), + Entity::from_raw(SECOND_IDX) + ); + + // entity is just returned as is if it does not exist in the map + assert_eq!( + mapper.map(Entity::from_raw(MISSING_IDX)), + Entity::from_raw(MISSING_IDX) + ); + } + #[test] fn entity_mapper() { const FIRST_IDX: u32 = 1; diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 21023c36a641d..23e7c3cefa1dd 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -137,7 +137,8 @@ pub struct Entity { } impl MapEntities for Entity { - fn map_entities(&mut self, mut entity_mapper: M) { + // we just map the entity itself + fn map_entities(&mut self, entity_mapper: &mut M) { entity_mapper.map(*self); } } diff --git a/crates/bevy_ecs/src/reflect/map_entities.rs b/crates/bevy_ecs/src/reflect/map_entities.rs index 4043b3f36caf0..27bcb03653546 100644 --- a/crates/bevy_ecs/src/reflect/map_entities.rs +++ b/crates/bevy_ecs/src/reflect/map_entities.rs @@ -27,7 +27,7 @@ impl ReflectMapEntities { /// An example of this: A scene can be loaded with `Parent` components, but then a `Parent` component can be added /// to these entities after they have been loaded. If you reload the scene using [`map_all_entities`](Self::map_all_entities), those `Parent` /// components with already valid entity references could be updated to point at something else entirely. - pub fn map_or_gen_all_entities( + pub fn map_all_entities( &self, world: &mut World, entity_map: &mut EntityHashMap, @@ -41,7 +41,7 @@ impl ReflectMapEntities { /// /// This is useful mostly for when you need to be careful not to update components that already contain valid entity /// values. See [`map_all_entities`](Self::map_all_entities) for more details. - pub fn map_or_gen_entities( + pub fn map_entities( &self, world: &mut World, entity_map: &mut EntityHashMap, diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index 97eed59944e8e..08b474ec60b23 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -1,3 +1,4 @@ +use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; #[cfg(feature = "reflect")] use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ @@ -9,7 +10,6 @@ use bevy_ecs::{ use bevy_utils::smallvec::SmallVec; use core::slice; use std::ops::Deref; -use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; /// Contains references to the child entities of this entity. /// @@ -30,7 +30,6 @@ use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; pub struct Children(pub(crate) SmallVec<[Entity; 8]>); impl MapEntities for Children { - fn map_entities(&mut self, entity_mapper: &mut M) { for entity in &mut self.0 { entity.map_entities(entity_mapper); diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index a6e82290290ee..83a064c26185f 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -1,3 +1,4 @@ +use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; #[cfg(feature = "reflect")] use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ @@ -6,7 +7,6 @@ use bevy_ecs::{ world::{FromWorld, World}, }; use std::ops::Deref; -use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; /// Holds a reference to the parent entity of this entity. /// This component should only be present on entities that actually have a parent entity. diff --git a/crates/bevy_render/src/mesh/mesh/skinning.rs b/crates/bevy_render/src/mesh/mesh/skinning.rs index 982f7275aa7b0..c1f1b13646425 100644 --- a/crates/bevy_render/src/mesh/mesh/skinning.rs +++ b/crates/bevy_render/src/mesh/mesh/skinning.rs @@ -1,4 +1,5 @@ use bevy_asset::{Asset, Handle}; +use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; use bevy_ecs::{ component::Component, entity::{Entity, EntityMapper, MapEntities}, @@ -8,7 +9,6 @@ use bevy_ecs::{ use bevy_math::Mat4; use bevy_reflect::{Reflect, TypePath}; use std::ops::Deref; -use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; #[derive(Component, Debug, Default, Clone, Reflect)] #[reflect(Component, MapEntities)] @@ -18,7 +18,6 @@ pub struct SkinnedMesh { } impl MapEntities for SkinnedMesh { - fn map_entities(&mut self, entity_mapper: &mut M) { for joint in &mut self.joints { *joint.map_entities(entity_mapper); diff --git a/crates/bevy_scene/src/dynamic_scene.rs b/crates/bevy_scene/src/dynamic_scene.rs index d880b5255aed4..676fb9fdcd834 100644 --- a/crates/bevy_scene/src/dynamic_scene.rs +++ b/crates/bevy_scene/src/dynamic_scene.rs @@ -149,7 +149,7 @@ impl DynamicScene { "we should be getting TypeId from this TypeRegistration in the first place", ); if let Some(map_entities_reflect) = registration.data::() { - map_entities_reflect.map_or_gen_entities(world, entity_map, &entities); + map_entities_reflect.map_entities(world, entity_map, &entities); } } diff --git a/crates/bevy_scene/src/scene.rs b/crates/bevy_scene/src/scene.rs index b4c28ba3ad3a0..28cf824aa061c 100644 --- a/crates/bevy_scene/src/scene.rs +++ b/crates/bevy_scene/src/scene.rs @@ -130,7 +130,7 @@ impl Scene { for registration in type_registry.iter() { if let Some(map_entities_reflect) = registration.data::() { - map_entities_reflect.map_or_gen_all_entities(world, &mut instance_info.entity_map); + map_entities_reflect.map_all_entities(world, &mut instance_info.entity_map); } } diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index 5dcfcb56ff154..a2f419443c0d9 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -1,8 +1,8 @@ +use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; use bevy_ecs::{ entity::{Entity, EntityMapper, MapEntities}, prelude::{Component, ReflectComponent}, }; -use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; use bevy_math::{DVec2, IVec2, Vec2}; use bevy_reflect::{std_traits::ReflectDefault, Reflect}; @@ -60,7 +60,6 @@ impl WindowRef { } impl MapEntities for WindowRef { - fn map_entities(&mut self, entity_mapper: &mut M) { match self { Self::Entity(entity) => { From 39b3ddb621ad8d416b0cc203b9332426093e55f5 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 20 Jan 2024 12:31:05 -0500 Subject: [PATCH 06/21] fix tests --- crates/bevy_ecs/src/entity/map_entities.rs | 1 + crates/bevy_hierarchy/src/components/children.rs | 4 ++-- crates/bevy_hierarchy/src/components/parent.rs | 4 ++-- crates/bevy_render/src/mesh/mesh/skinning.rs | 6 +++--- crates/bevy_scene/src/serde.rs | 4 ++-- crates/bevy_window/src/window.rs | 6 +++--- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index c6cf95f876031..a7da9bd04baab 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -66,6 +66,7 @@ impl Mapper for SimpleEntityMapper<'_> { } impl<'m> SimpleEntityMapper<'m> { + /// Creates a new `SimpleEntityMapper` from an [`EntityHashMap`]. pub fn new(map: &'m EntityHashMap) -> Self { Self { map } } diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index 08b474ec60b23..317093a41e5cb 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -1,9 +1,9 @@ -use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; +use bevy_ecs::entity::{Mapper}; #[cfg(feature = "reflect")] use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ component::Component, - entity::{Entity, EntityMapper, MapEntities}, + entity::{Entity, MapEntities}, prelude::FromWorld, world::World, }; diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index 83a064c26185f..6d66ce8354b7e 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -1,9 +1,9 @@ -use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; +use bevy_ecs::entity::{Mapper}; #[cfg(feature = "reflect")] use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ component::Component, - entity::{Entity, EntityMapper, MapEntities}, + entity::{Entity, MapEntities}, world::{FromWorld, World}, }; use std::ops::Deref; diff --git a/crates/bevy_render/src/mesh/mesh/skinning.rs b/crates/bevy_render/src/mesh/mesh/skinning.rs index c1f1b13646425..2d16a48f9e51a 100644 --- a/crates/bevy_render/src/mesh/mesh/skinning.rs +++ b/crates/bevy_render/src/mesh/mesh/skinning.rs @@ -1,8 +1,8 @@ use bevy_asset::{Asset, Handle}; -use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; +use bevy_ecs::entity::{Mapper}; use bevy_ecs::{ component::Component, - entity::{Entity, EntityMapper, MapEntities}, + entity::{Entity, MapEntities}, prelude::ReflectComponent, reflect::ReflectMapEntities, }; @@ -20,7 +20,7 @@ pub struct SkinnedMesh { impl MapEntities for SkinnedMesh { fn map_entities(&mut self, entity_mapper: &mut M) { for joint in &mut self.joints { - *joint.map_entities(entity_mapper); + joint.map_entities(entity_mapper); } } } diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 779b0fb1d73cd..a6a1c89e193a8 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -499,7 +499,7 @@ mod tests { use crate::ron; use crate::serde::{SceneDeserializer, SceneSerializer}; use crate::{DynamicScene, DynamicSceneBuilder}; - use bevy_ecs::entity::{Entity, EntityMapper, MapEntities, Mapper, SimpleEntityMapper}; + use bevy_ecs::entity::{Entity, MapEntities, Mapper}; use bevy_ecs::prelude::{Component, ReflectComponent, ReflectResource, Resource, World}; use bevy_ecs::query::{With, Without}; use bevy_ecs::reflect::{AppTypeRegistry, ReflectMapEntities}; @@ -550,7 +550,7 @@ mod tests { struct MyEntityRef(Entity); impl MapEntities for MyEntityRef { - fn map_entities(&mut self, entity_mapper: &M) { + fn map_entities(&mut self, entity_mapper: &mut M) { self.0.map_entities(entity_mapper); } } diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index a2f419443c0d9..1f8aaee6c061f 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -1,6 +1,6 @@ -use bevy_ecs::entity::{Mapper, SimpleEntityMapper}; +use bevy_ecs::entity::{Mapper}; use bevy_ecs::{ - entity::{Entity, EntityMapper, MapEntities}, + entity::{Entity, MapEntities}, prelude::{Component, ReflectComponent}, }; use bevy_math::{DVec2, IVec2, Vec2}; @@ -63,7 +63,7 @@ impl MapEntities for WindowRef { fn map_entities(&mut self, entity_mapper: &mut M) { match self { Self::Entity(entity) => { - *entity.map_entities(entity_mapper); + entity.map_entities(entity_mapper); } Self::Primary => {} }; From a5046be76df13e4e01f603aaa6a6d2317dd528f9 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 20 Jan 2024 12:34:55 -0500 Subject: [PATCH 07/21] fmt --- crates/bevy_hierarchy/src/components/children.rs | 2 +- crates/bevy_hierarchy/src/components/parent.rs | 2 +- crates/bevy_render/src/mesh/mesh/skinning.rs | 2 +- crates/bevy_window/src/window.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index 317093a41e5cb..c5348697da14f 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -1,4 +1,4 @@ -use bevy_ecs::entity::{Mapper}; +use bevy_ecs::entity::Mapper; #[cfg(feature = "reflect")] use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index 6d66ce8354b7e..0f8aad57fd246 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -1,4 +1,4 @@ -use bevy_ecs::entity::{Mapper}; +use bevy_ecs::entity::Mapper; #[cfg(feature = "reflect")] use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ diff --git a/crates/bevy_render/src/mesh/mesh/skinning.rs b/crates/bevy_render/src/mesh/mesh/skinning.rs index 2d16a48f9e51a..4c6f2a2348cb9 100644 --- a/crates/bevy_render/src/mesh/mesh/skinning.rs +++ b/crates/bevy_render/src/mesh/mesh/skinning.rs @@ -1,5 +1,5 @@ use bevy_asset::{Asset, Handle}; -use bevy_ecs::entity::{Mapper}; +use bevy_ecs::entity::Mapper; use bevy_ecs::{ component::Component, entity::{Entity, MapEntities}, diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index 1f8aaee6c061f..ed779ec700cb4 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -1,4 +1,4 @@ -use bevy_ecs::entity::{Mapper}; +use bevy_ecs::entity::Mapper; use bevy_ecs::{ entity::{Entity, MapEntities}, prelude::{Component, ReflectComponent}, From f14bdf03bed2b16409b0b9d861c53a92d257f76e Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 20 Jan 2024 13:23:33 -0500 Subject: [PATCH 08/21] fix entity map_entities impl --- crates/bevy_ecs/src/entity/mod.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 23e7c3cefa1dd..49524e356d3b0 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -139,7 +139,7 @@ pub struct Entity { impl MapEntities for Entity { // we just map the entity itself fn map_entities(&mut self, entity_mapper: &mut M) { - entity_mapper.map(*self); + *self = entity_mapper.map(*self); } } @@ -1125,4 +1125,22 @@ mod tests { assert_ne!(hash, first_hash); } } + + #[test] + fn entity_map() { + use crate::entity::map_entities::SimpleEntityMapper; + use bevy_utils::EntityHashMap; + + pub const FIRST_IDX: u32 = 0; + pub const SECOND_IDX: u32 = 0; + + let mut map = EntityHashMap::default(); + map.insert(Entity::from_raw(FIRST_IDX), Entity::from_raw(SECOND_IDX)); + let mut mapper = SimpleEntityMapper::new(&map); + + let mut entity = Entity::from_raw(FIRST_IDX); + entity.map_entities(&mut mapper); + + assert_eq!(entity, Entity::from_raw(SECOND_IDX)); + } } From 629e4db858ca0c5d3e1c9c95d67dd176c8aa1334 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 26 Jan 2024 11:54:31 -0500 Subject: [PATCH 09/21] remove simple entity mapper --- crates/bevy_ecs/src/entity/map_entities.rs | 53 ++++------------------ crates/bevy_ecs/src/entity/mod.rs | 18 -------- 2 files changed, 8 insertions(+), 63 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index a7da9bd04baab..e0970dc2ef6f8 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -22,7 +22,7 @@ use bevy_utils::EntityHashMap; /// /// ``` /// use bevy_ecs::prelude::*; -/// use bevy_ecs::entity::{EntityMapper, MapEntities, Mapper, SimpleEntityMapper}; +/// use bevy_ecs::entity::{EntityMapper, MapEntities, Mapper}; /// /// #[derive(Component)] /// struct Spring { @@ -43,45 +43,15 @@ pub trait MapEntities { fn map_entities(&mut self, entity_mapper: &mut M); } -/// This traits defines a type that knows how to map [`Entity`] references. +/// Any implementor of this trait knows how to map an [`Entity`] into another [`Entity`]. /// -/// Two implementations are provided: -/// - `SimpleEntityMapper`: tries to map the [`Entity`] reference, but if it can't, it returns the same [`Entity`] reference. -/// - `EntityMapper`: tries to map the [`Entity`] reference, but if it can't, it allocates a new [`Entity`] reference. +/// Usually this is done by using a [`EntityHashMap`] to map the [`Entity`] references. +/// This can be used to map [`Entity`] references from one [`World`] to another. pub trait Mapper { /// Map an entity to another entity fn map(&mut self, entity: Entity) -> Entity; } -/// Similar to `EntityMapper`, but does not allocate new [`Entity`] references in case we couldn't map the entity. -pub struct SimpleEntityMapper<'m> { - map: &'m EntityHashMap, -} - -impl Mapper for SimpleEntityMapper<'_> { - /// Map the entity to another entity, or return the same entity if we couldn't map it. - fn map(&mut self, entity: Entity) -> Entity { - self.get(entity).unwrap_or(entity) - } -} - -impl<'m> SimpleEntityMapper<'m> { - /// Creates a new `SimpleEntityMapper` from an [`EntityHashMap`]. - pub fn new(map: &'m EntityHashMap) -> Self { - Self { map } - } - - /// Returns the corresponding mapped entity or None if it is absent. - pub fn get(&self, entity: Entity) -> Option { - self.map.get(&entity).copied() - } - - /// Gets a reference to the underlying [`EntityHashMap`]. - pub fn get_map(&'m self) -> &'m EntityHashMap { - self.map - } -} - impl Mapper for EntityMapper<'_> { /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. fn map(&mut self, entity: Entity) -> Entity { @@ -187,32 +157,25 @@ mod tests { use crate::{ entity::map_entities::Mapper, - entity::{Entity, EntityMapper, SimpleEntityMapper}, + entity::{Entity, EntityMapper}, world::World, }; #[test] - fn simple_entity_mapper() { + fn mapper_trait() { const FIRST_IDX: u32 = 1; const SECOND_IDX: u32 = 2; - const MISSING_IDX: u32 = 10; - let mut map = EntityHashMap::default(); map.insert(Entity::from_raw(FIRST_IDX), Entity::from_raw(SECOND_IDX)); - let mut mapper = SimpleEntityMapper::new(&map); + let mut world = World::new(); + let mut mapper = EntityMapper::new(&mut map, &mut world); // entity is mapped correctly if it exists in the map assert_eq!( mapper.map(Entity::from_raw(FIRST_IDX)), Entity::from_raw(SECOND_IDX) ); - - // entity is just returned as is if it does not exist in the map - assert_eq!( - mapper.map(Entity::from_raw(MISSING_IDX)), - Entity::from_raw(MISSING_IDX) - ); } #[test] diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 49524e356d3b0..c6e4f3525b5b9 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -1125,22 +1125,4 @@ mod tests { assert_ne!(hash, first_hash); } } - - #[test] - fn entity_map() { - use crate::entity::map_entities::SimpleEntityMapper; - use bevy_utils::EntityHashMap; - - pub const FIRST_IDX: u32 = 0; - pub const SECOND_IDX: u32 = 0; - - let mut map = EntityHashMap::default(); - map.insert(Entity::from_raw(FIRST_IDX), Entity::from_raw(SECOND_IDX)); - let mut mapper = SimpleEntityMapper::new(&map); - - let mut entity = Entity::from_raw(FIRST_IDX); - entity.map_entities(&mut mapper); - - assert_eq!(entity, Entity::from_raw(SECOND_IDX)); - } } From 94a886ade5aabde9239122fd61e02e77d84c3c91 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 26 Jan 2024 15:01:49 -0500 Subject: [PATCH 10/21] deprecate get_or_reserve --- crates/bevy_ecs/src/entity/map_entities.rs | 43 +++++++++++----------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index e0970dc2ef6f8..59ae062c17fe9 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -55,7 +55,22 @@ pub trait Mapper { impl Mapper for EntityMapper<'_> { /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. fn map(&mut self, entity: Entity) -> Entity { - self.get_or_reserve(entity) + if let Some(&mapped) = self.map.get(&entity) { + return mapped; + } + + // this new entity reference is specifically designed to never represent any living entity + let new = Entity::from_raw_and_generation( + self.dead_start.index(), + IdentifierMask::inc_masked_high_by(self.dead_start.generation, self.generations), + ); + + // Prevent generations counter from being a greater value than HIGH_MASK. + self.generations = (self.generations + 1) & HIGH_MASK; + + self.map.insert(entity, new); + + new } } @@ -82,24 +97,10 @@ pub struct EntityMapper<'m> { } impl<'m> EntityMapper<'m> { + #[deprecated(since="0.13.0", note="please use `EntityMapper::Map` instead")] /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. pub fn get_or_reserve(&mut self, entity: Entity) -> Entity { - if let Some(&mapped) = self.map.get(&entity) { - return mapped; - } - - // this new entity reference is specifically designed to never represent any living entity - let new = Entity::from_raw_and_generation( - self.dead_start.index(), - IdentifierMask::inc_masked_high_by(self.dead_start.generation, self.generations), - ); - - // Prevent generations counter from being a greater value than HIGH_MASK. - self.generations = (self.generations + 1) & HIGH_MASK; - - self.map.insert(entity, new); - - new + self.map(entity) } /// Gets a reference to the underlying [`EntityHashMap`]. @@ -188,15 +189,15 @@ mod tests { let mut mapper = EntityMapper::new(&mut map, &mut world); let mapped_ent = Entity::from_raw(FIRST_IDX); - let dead_ref = mapper.get_or_reserve(mapped_ent); + let dead_ref = mapper.map(mapped_ent); assert_eq!( dead_ref, - mapper.get_or_reserve(mapped_ent), + mapper.map(mapped_ent), "should persist the allocated mapping from the previous line" ); assert_eq!( - mapper.get_or_reserve(Entity::from_raw(SECOND_IDX)).index(), + mapper.map(Entity::from_raw(SECOND_IDX)).index(), dead_ref.index(), "should re-use the same index for further dead refs" ); @@ -214,7 +215,7 @@ mod tests { let mut world = World::new(); let dead_ref = EntityMapper::world_scope(&mut map, &mut world, |_, mapper| { - mapper.get_or_reserve(Entity::from_raw(0)) + mapper.map(Entity::from_raw(0)) }); // Next allocated entity should be a further generation on the same index From 2c9dbfea5d16dc68e2f1e49b31c24bbce6557627 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 26 Jan 2024 15:11:52 -0500 Subject: [PATCH 11/21] revert implementing MapEntities for Entity --- crates/bevy_ecs/src/entity/map_entities.rs | 6 +++--- crates/bevy_ecs/src/entity/mod.rs | 7 ------- crates/bevy_hierarchy/src/components/children.rs | 2 +- crates/bevy_hierarchy/src/components/parent.rs | 2 +- crates/bevy_render/src/mesh/mesh/skinning.rs | 2 +- crates/bevy_scene/src/serde.rs | 2 +- crates/bevy_window/src/window.rs | 2 +- 7 files changed, 8 insertions(+), 15 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 59ae062c17fe9..04d929338d906 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -32,8 +32,8 @@ use bevy_utils::EntityHashMap; /// /// impl MapEntities for Spring { /// fn map_entities(&mut self, entity_mapper: &mut M) { -/// self.a.map_entities(entity_mapper); -/// self.b.map_entities(entity_mapper) +/// self.a = entity_mapper.map(self.a); +/// self.b = entity_mapper.map(self.b); /// } /// } /// ``` @@ -97,7 +97,7 @@ pub struct EntityMapper<'m> { } impl<'m> EntityMapper<'m> { - #[deprecated(since="0.13.0", note="please use `EntityMapper::Map` instead")] + #[deprecated(since = "0.13.0", note = "please use `EntityMapper::Map` instead")] /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. pub fn get_or_reserve(&mut self, entity: Entity) -> Entity { self.map(entity) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index c6e4f3525b5b9..ab7d167cff67c 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -136,13 +136,6 @@ pub struct Entity { index: u32, } -impl MapEntities for Entity { - // we just map the entity itself - fn map_entities(&mut self, entity_mapper: &mut M) { - *self = entity_mapper.map(*self); - } -} - // By not short-circuiting in comparisons, we get better codegen. // See impl PartialEq for Entity { diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index c5348697da14f..175860c262c4c 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -32,7 +32,7 @@ pub struct Children(pub(crate) SmallVec<[Entity; 8]>); impl MapEntities for Children { fn map_entities(&mut self, entity_mapper: &mut M) { for entity in &mut self.0 { - entity.map_entities(entity_mapper); + *entity = entity_mapper.map(*entity); } } } diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index 0f8aad57fd246..487a5232c713c 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -58,7 +58,7 @@ impl FromWorld for Parent { impl MapEntities for Parent { fn map_entities(&mut self, entity_mapper: &mut M) { - self.0.map_entities(entity_mapper); + self.0 = entity_mapper.map(self.0); } } diff --git a/crates/bevy_render/src/mesh/mesh/skinning.rs b/crates/bevy_render/src/mesh/mesh/skinning.rs index 4c6f2a2348cb9..975452a1a3134 100644 --- a/crates/bevy_render/src/mesh/mesh/skinning.rs +++ b/crates/bevy_render/src/mesh/mesh/skinning.rs @@ -20,7 +20,7 @@ pub struct SkinnedMesh { impl MapEntities for SkinnedMesh { fn map_entities(&mut self, entity_mapper: &mut M) { for joint in &mut self.joints { - joint.map_entities(entity_mapper); + *joint = entity_mapper.map(*joint); } } } diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index a6a1c89e193a8..2f7604717bd92 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -551,7 +551,7 @@ mod tests { impl MapEntities for MyEntityRef { fn map_entities(&mut self, entity_mapper: &mut M) { - self.0.map_entities(entity_mapper); + self.0 = entity_mapper.map(self.0); } } diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index ed779ec700cb4..3d5ce5401f074 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -63,7 +63,7 @@ impl MapEntities for WindowRef { fn map_entities(&mut self, entity_mapper: &mut M) { match self { Self::Entity(entity) => { - entity.map_entities(entity_mapper); + *entity = entity_mapper.map(*entity); } Self::Primary => {} }; From 286386be6f9b2d422428a8a5e85ba9ff0549417a Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 26 Jan 2024 15:14:03 -0500 Subject: [PATCH 12/21] clean imports --- crates/bevy_hierarchy/src/components/children.rs | 3 +-- crates/bevy_hierarchy/src/components/parent.rs | 3 +-- crates/bevy_render/src/mesh/mesh/skinning.rs | 3 +-- crates/bevy_window/src/window.rs | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index 175860c262c4c..ccab7c82a2c7d 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -1,9 +1,8 @@ -use bevy_ecs::entity::Mapper; #[cfg(feature = "reflect")] use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ component::Component, - entity::{Entity, MapEntities}, + entity::{Entity, MapEntities, Mapper}, prelude::FromWorld, world::World, }; diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index 487a5232c713c..b0a57060de9a6 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -1,9 +1,8 @@ -use bevy_ecs::entity::Mapper; #[cfg(feature = "reflect")] use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ component::Component, - entity::{Entity, MapEntities}, + entity::{Entity, MapEntities, Mapper}, world::{FromWorld, World}, }; use std::ops::Deref; diff --git a/crates/bevy_render/src/mesh/mesh/skinning.rs b/crates/bevy_render/src/mesh/mesh/skinning.rs index 975452a1a3134..4860fe7f83dcc 100644 --- a/crates/bevy_render/src/mesh/mesh/skinning.rs +++ b/crates/bevy_render/src/mesh/mesh/skinning.rs @@ -1,8 +1,7 @@ use bevy_asset::{Asset, Handle}; -use bevy_ecs::entity::Mapper; use bevy_ecs::{ component::Component, - entity::{Entity, MapEntities}, + entity::{Entity, MapEntities, Mapper}, prelude::ReflectComponent, reflect::ReflectMapEntities, }; diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index 3d5ce5401f074..936ca41518afe 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -1,6 +1,5 @@ -use bevy_ecs::entity::Mapper; use bevy_ecs::{ - entity::{Entity, MapEntities}, + entity::{Entity, MapEntities, Mapper}, prelude::{Component, ReflectComponent}, }; use bevy_math::{DVec2, IVec2, Vec2}; From a2269a3afdf8d153c62e9c8069ece5a9cc0cb422 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 26 Jan 2024 15:40:15 -0500 Subject: [PATCH 13/21] address comments --- crates/bevy_ecs/src/entity/map_entities.rs | 25 ++-------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 04d929338d906..1dfac785c0d78 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -10,10 +10,7 @@ use bevy_utils::EntityHashMap; /// 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 a [`Mapper`], which is a type that knows -/// how to perform entity mapping. (usually by using a [`EntityHashMap`]) -/// -/// [`MapEntities`] is already implemented for [`Entity`], so you just need to call [`Entity::map_entities`] -/// for all the [`Entity`] fields in your type. +/// how to perform entity mapping (usually by using an [`EntityHashMap`]). /// /// Implementing this trait correctly is required for properly loading components /// with entity references from scenes. @@ -97,7 +94,7 @@ pub struct EntityMapper<'m> { } impl<'m> EntityMapper<'m> { - #[deprecated(since = "0.13.0", note = "please use `EntityMapper::Map` instead")] + #[deprecated(since = "0.13.0", note = "please use `EntityMapper::map` instead")] /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. pub fn get_or_reserve(&mut self, entity: Entity) -> Entity { self.map(entity) @@ -157,28 +154,10 @@ mod tests { use bevy_utils::EntityHashMap; use crate::{ - entity::map_entities::Mapper, entity::{Entity, EntityMapper}, world::World, }; - #[test] - fn mapper_trait() { - const FIRST_IDX: u32 = 1; - const SECOND_IDX: u32 = 2; - - let mut map = EntityHashMap::default(); - map.insert(Entity::from_raw(FIRST_IDX), Entity::from_raw(SECOND_IDX)); - let mut world = World::new(); - let mut mapper = EntityMapper::new(&mut map, &mut world); - - // entity is mapped correctly if it exists in the map - assert_eq!( - mapper.map(Entity::from_raw(FIRST_IDX)), - Entity::from_raw(SECOND_IDX) - ); - } - #[test] fn entity_mapper() { const FIRST_IDX: u32 = 1; From a0f311e89626ae2580921f4c8b70d660eb8cc8d6 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 26 Jan 2024 15:48:37 -0500 Subject: [PATCH 14/21] rename EntityMapper -> SceneEntityMapper, Mapper -> EntityMapper --- crates/bevy_ecs/src/entity/map_entities.rs | 30 +++++++++---------- crates/bevy_ecs/src/reflect/map_entities.rs | 10 +++---- .../bevy_hierarchy/src/components/children.rs | 4 +-- .../bevy_hierarchy/src/components/parent.rs | 4 +-- crates/bevy_render/src/mesh/mesh/skinning.rs | 4 +-- crates/bevy_scene/src/serde.rs | 4 +-- crates/bevy_window/src/window.rs | 4 +-- 7 files changed, 30 insertions(+), 30 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 1dfac785c0d78..c9096f5eca579 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -9,7 +9,7 @@ use bevy_utils::EntityHashMap; /// /// 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 a [`Mapper`], which is a type that knows +/// allows defining custom mappings for these references via a [`EntityMapper`], which is a type that knows /// how to perform entity mapping (usually by using an [`EntityHashMap`]). /// /// Implementing this trait correctly is required for properly loading components @@ -19,7 +19,7 @@ use bevy_utils::EntityHashMap; /// /// ``` /// use bevy_ecs::prelude::*; -/// use bevy_ecs::entity::{EntityMapper, MapEntities, Mapper}; +/// use bevy_ecs::entity::{SceneEntityMapper, MapEntities, EntityMapper}; /// /// #[derive(Component)] /// struct Spring { @@ -28,7 +28,7 @@ use bevy_utils::EntityHashMap; /// } /// /// impl MapEntities for Spring { -/// fn map_entities(&mut self, entity_mapper: &mut M) { +/// fn map_entities(&mut self, entity_mapper: &mut M) { /// self.a = entity_mapper.map(self.a); /// self.b = entity_mapper.map(self.b); /// } @@ -37,19 +37,19 @@ use bevy_utils::EntityHashMap; /// pub trait MapEntities { /// Updates all [`Entity`] references stored inside using `entity_mapper`. - fn map_entities(&mut self, entity_mapper: &mut M); + fn map_entities(&mut self, entity_mapper: &mut M); } /// Any implementor of this trait knows how to map an [`Entity`] into another [`Entity`]. /// /// Usually this is done by using a [`EntityHashMap`] to map the [`Entity`] references. /// This can be used to map [`Entity`] references from one [`World`] to another. -pub trait Mapper { +pub trait EntityMapper { /// Map an entity to another entity fn map(&mut self, entity: Entity) -> Entity; } -impl Mapper for EntityMapper<'_> { +impl EntityMapper for SceneEntityMapper<'_> { /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. fn map(&mut self, entity: Entity) -> Entity { if let Some(&mapped) = self.map.get(&entity) { @@ -77,7 +77,7 @@ impl Mapper for EntityMapper<'_> { /// References are allocated by returning increasing generations starting from an internally initialized base /// [`Entity`]. After it is finished being used by [`MapEntities`] implementations, this entity is despawned and the /// requisite number of generations reserved. -pub struct EntityMapper<'m> { +pub struct SceneEntityMapper<'m> { /// A mapping from one set of entities to another. /// /// This is typically used to coordinate data transfer between sets of entities, such as between a scene and the world @@ -85,7 +85,7 @@ pub struct EntityMapper<'m> { /// identifiers directly. /// /// On its own, a [`EntityHashMap`] is not capable of allocating new entity identifiers, which is needed to map references - /// to entities that lie outside the source entity set. This functionality can be accessed through [`EntityMapper::world_scope()`]. + /// to entities that lie outside the source entity set. This functionality can be accessed through [`SceneEntityMapper::world_scope()`]. map: &'m mut EntityHashMap, /// A base [`Entity`] used to allocate new references. dead_start: Entity, @@ -93,7 +93,7 @@ pub struct EntityMapper<'m> { generations: u32, } -impl<'m> EntityMapper<'m> { +impl<'m> SceneEntityMapper<'m> { #[deprecated(since = "0.13.0", note = "please use `EntityMapper::map` instead")] /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. pub fn get_or_reserve(&mut self, entity: Entity) -> Entity { @@ -110,7 +110,7 @@ impl<'m> EntityMapper<'m> { self.map } - /// Creates a new [`EntityMapper`], spawning a temporary base [`Entity`] in the provided [`World`] + /// Creates a new [`SceneEntityMapper`], spawning a temporary base [`Entity`] in the provided [`World`] fn new(map: &'m mut EntityHashMap, world: &mut World) -> Self { Self { map, @@ -122,7 +122,7 @@ impl<'m> EntityMapper<'m> { /// Reserves the allocated references to dead entities within the world. This frees the temporary base /// [`Entity`] while reserving extra generations via [`crate::entity::Entities::reserve_generations`]. Because this - /// renders the [`EntityMapper`] unable to safely allocate any more references, this method takes ownership of + /// renders the [`SceneEntityMapper`] unable to safely allocate any more references, this method takes ownership of /// `self` in order to render it unusable. fn finish(self, world: &mut World) { // SAFETY: Entities data is kept in a valid state via `EntityMap::world_scope` @@ -131,7 +131,7 @@ impl<'m> EntityMapper<'m> { assert!(entities.reserve_generations(self.dead_start.index(), self.generations)); } - /// Creates an [`EntityMapper`] from a provided [`World`] and [`EntityHashMap`], then calls the + /// Creates an [`SceneEntityMapper`] from a provided [`World`] and [`EntityHashMap`], then calls the /// provided function with it. This allows one to allocate new entity references in this [`World`] that are /// guaranteed to never point at a living entity now or in the future. This functionality is useful for safely /// mapping entity identifiers that point at entities outside the source world. The passed function, `f`, is called @@ -154,7 +154,7 @@ mod tests { use bevy_utils::EntityHashMap; use crate::{ - entity::{Entity, EntityMapper}, + entity::{Entity, SceneEntityMapper}, world::World, }; @@ -165,7 +165,7 @@ mod tests { let mut map = EntityHashMap::default(); let mut world = World::new(); - let mut mapper = EntityMapper::new(&mut map, &mut world); + let mut mapper = SceneEntityMapper::new(&mut map, &mut world); let mapped_ent = Entity::from_raw(FIRST_IDX); let dead_ref = mapper.map(mapped_ent); @@ -193,7 +193,7 @@ mod tests { let mut map = EntityHashMap::default(); let mut world = World::new(); - let dead_ref = EntityMapper::world_scope(&mut map, &mut world, |_, mapper| { + let dead_ref = SceneEntityMapper::world_scope(&mut map, &mut world, |_, mapper| { mapper.map(Entity::from_raw(0)) }); diff --git a/crates/bevy_ecs/src/reflect/map_entities.rs b/crates/bevy_ecs/src/reflect/map_entities.rs index 27bcb03653546..5c7805dd68d8f 100644 --- a/crates/bevy_ecs/src/reflect/map_entities.rs +++ b/crates/bevy_ecs/src/reflect/map_entities.rs @@ -1,6 +1,6 @@ use crate::{ component::Component, - entity::{Entity, EntityMapper, MapEntities}, + entity::{Entity, MapEntities, SceneEntityMapper}, world::World, }; use bevy_reflect::FromType; @@ -13,8 +13,8 @@ use bevy_utils::EntityHashMap; /// See [`MapEntities`] for more information. #[derive(Clone)] pub struct ReflectMapEntities { - map_all_entities: fn(&mut World, &mut EntityMapper), - map_entities: fn(&mut World, &mut EntityMapper, &[Entity]), + map_all_entities: fn(&mut World, &mut SceneEntityMapper), + map_entities: fn(&mut World, &mut SceneEntityMapper, &[Entity]), } impl ReflectMapEntities { @@ -32,7 +32,7 @@ impl ReflectMapEntities { world: &mut World, entity_map: &mut EntityHashMap, ) { - EntityMapper::world_scope(entity_map, world, self.map_all_entities); + SceneEntityMapper::world_scope(entity_map, world, self.map_all_entities); } /// A general method for applying [`MapEntities`] behavior to elements in an [`EntityHashMap`]. Unlike @@ -47,7 +47,7 @@ impl ReflectMapEntities { entity_map: &mut EntityHashMap, entities: &[Entity], ) { - EntityMapper::world_scope(entity_map, world, |world, mapper| { + SceneEntityMapper::world_scope(entity_map, world, |world, mapper| { (self.map_entities)(world, mapper, entities); }); } diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index ccab7c82a2c7d..aafda1d5c4088 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -2,7 +2,7 @@ use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ component::Component, - entity::{Entity, MapEntities, Mapper}, + entity::{Entity, EntityMapper, MapEntities}, prelude::FromWorld, world::World, }; @@ -29,7 +29,7 @@ use std::ops::Deref; pub struct Children(pub(crate) SmallVec<[Entity; 8]>); impl MapEntities for Children { - fn map_entities(&mut self, entity_mapper: &mut M) { + fn map_entities(&mut self, entity_mapper: &mut M) { for entity in &mut self.0 { *entity = entity_mapper.map(*entity); } diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index b0a57060de9a6..5b41ddbf96682 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -2,7 +2,7 @@ use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ component::Component, - entity::{Entity, MapEntities, Mapper}, + entity::{Entity, EntityMapper, MapEntities}, world::{FromWorld, World}, }; use std::ops::Deref; @@ -56,7 +56,7 @@ impl FromWorld for Parent { } impl MapEntities for Parent { - fn map_entities(&mut self, entity_mapper: &mut M) { + fn map_entities(&mut self, entity_mapper: &mut M) { self.0 = entity_mapper.map(self.0); } } diff --git a/crates/bevy_render/src/mesh/mesh/skinning.rs b/crates/bevy_render/src/mesh/mesh/skinning.rs index 4860fe7f83dcc..a876e4dcb896c 100644 --- a/crates/bevy_render/src/mesh/mesh/skinning.rs +++ b/crates/bevy_render/src/mesh/mesh/skinning.rs @@ -1,7 +1,7 @@ use bevy_asset::{Asset, Handle}; use bevy_ecs::{ component::Component, - entity::{Entity, MapEntities, Mapper}, + entity::{Entity, EntityMapper, MapEntities}, prelude::ReflectComponent, reflect::ReflectMapEntities, }; @@ -17,7 +17,7 @@ pub struct SkinnedMesh { } impl MapEntities for SkinnedMesh { - fn map_entities(&mut self, entity_mapper: &mut M) { + fn map_entities(&mut self, entity_mapper: &mut M) { for joint in &mut self.joints { *joint = entity_mapper.map(*joint); } diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 2f7604717bd92..2dad234c35fd5 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -499,7 +499,7 @@ mod tests { use crate::ron; use crate::serde::{SceneDeserializer, SceneSerializer}; use crate::{DynamicScene, DynamicSceneBuilder}; - use bevy_ecs::entity::{Entity, MapEntities, Mapper}; + use bevy_ecs::entity::{Entity, EntityMapper, MapEntities}; use bevy_ecs::prelude::{Component, ReflectComponent, ReflectResource, Resource, World}; use bevy_ecs::query::{With, Without}; use bevy_ecs::reflect::{AppTypeRegistry, ReflectMapEntities}; @@ -550,7 +550,7 @@ mod tests { struct MyEntityRef(Entity); impl MapEntities for MyEntityRef { - fn map_entities(&mut self, entity_mapper: &mut M) { + fn map_entities(&mut self, entity_mapper: &mut M) { self.0 = entity_mapper.map(self.0); } } diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index 936ca41518afe..c65db2a0b073e 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -1,5 +1,5 @@ use bevy_ecs::{ - entity::{Entity, MapEntities, Mapper}, + entity::{Entity, EntityMapper, MapEntities}, prelude::{Component, ReflectComponent}, }; use bevy_math::{DVec2, IVec2, Vec2}; @@ -59,7 +59,7 @@ impl WindowRef { } impl MapEntities for WindowRef { - fn map_entities(&mut self, entity_mapper: &mut M) { + fn map_entities(&mut self, entity_mapper: &mut M) { match self { Self::Entity(entity) => { *entity = entity_mapper.map(*entity); From 64108c3b5fc241aaea4fd8dc7df1544f26dbaaf0 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 26 Jan 2024 16:00:28 -0500 Subject: [PATCH 15/21] add EntityMapper to prelude and readd comment --- crates/bevy_ecs/src/entity/map_entities.rs | 5 ++++- crates/bevy_ecs/src/lib.rs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index c9096f5eca579..a4422525db029 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -37,6 +37,9 @@ use bevy_utils::EntityHashMap; /// pub trait MapEntities { /// Updates all [`Entity`] references stored inside using `entity_mapper`. + /// + /// Implementors should look up any and all [`Entity`] values stored within and + /// update them to the mapped values via `entity_mapper`. fn map_entities(&mut self, entity_mapper: &mut M); } @@ -154,7 +157,7 @@ mod tests { use bevy_utils::EntityHashMap; use crate::{ - entity::{Entity, SceneEntityMapper}, + entity::{Entity, EntityMapper, SceneEntityMapper}, world::World, }; diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 0260860c75e27..dfe2ccab0a769 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -36,7 +36,7 @@ pub mod prelude { bundle::Bundle, change_detection::{DetectChanges, DetectChangesMut, Mut, Ref}, component::Component, - entity::Entity, + entity::{Entity, EntityMapper}, event::{Event, EventReader, EventWriter, Events}, query::{Added, AnyOf, Changed, Has, Or, QueryBuilder, QueryState, With, Without}, removal_detection::RemovedComponents, From 410f0cd005481bb79dbc4338362e63fff0275494 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 26 Jan 2024 16:07:23 -0500 Subject: [PATCH 16/21] rename EntityMapper::map to EntityMapper::map_entity --- crates/bevy_ecs/src/entity/map_entities.rs | 18 +++++++++--------- .../bevy_hierarchy/src/components/children.rs | 2 +- crates/bevy_hierarchy/src/components/parent.rs | 2 +- crates/bevy_render/src/mesh/mesh/skinning.rs | 2 +- crates/bevy_scene/src/serde.rs | 2 +- crates/bevy_window/src/window.rs | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index a4422525db029..c0faa202a78fe 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -29,8 +29,8 @@ use bevy_utils::EntityHashMap; /// /// impl MapEntities for Spring { /// fn map_entities(&mut self, entity_mapper: &mut M) { -/// self.a = entity_mapper.map(self.a); -/// self.b = entity_mapper.map(self.b); +/// self.a = entity_mapper.map_entity(self.a); +/// self.b = entity_mapper.map_entity(self.b); /// } /// } /// ``` @@ -49,12 +49,12 @@ pub trait MapEntities { /// This can be used to map [`Entity`] references from one [`World`] to another. pub trait EntityMapper { /// Map an entity to another entity - fn map(&mut self, entity: Entity) -> Entity; + fn map_entity(&mut self, entity: Entity) -> Entity; } impl EntityMapper for SceneEntityMapper<'_> { /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. - fn map(&mut self, entity: Entity) -> Entity { + fn map_entity(&mut self, entity: Entity) -> Entity { if let Some(&mapped) = self.map.get(&entity) { return mapped; } @@ -100,7 +100,7 @@ impl<'m> SceneEntityMapper<'m> { #[deprecated(since = "0.13.0", note = "please use `EntityMapper::map` instead")] /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. pub fn get_or_reserve(&mut self, entity: Entity) -> Entity { - self.map(entity) + self.map_entity(entity) } /// Gets a reference to the underlying [`EntityHashMap`]. @@ -171,15 +171,15 @@ mod tests { let mut mapper = SceneEntityMapper::new(&mut map, &mut world); let mapped_ent = Entity::from_raw(FIRST_IDX); - let dead_ref = mapper.map(mapped_ent); + let dead_ref = mapper.map_entity(mapped_ent); assert_eq!( dead_ref, - mapper.map(mapped_ent), + mapper.map_entity(mapped_ent), "should persist the allocated mapping from the previous line" ); assert_eq!( - mapper.map(Entity::from_raw(SECOND_IDX)).index(), + mapper.map_entity(Entity::from_raw(SECOND_IDX)).index(), dead_ref.index(), "should re-use the same index for further dead refs" ); @@ -197,7 +197,7 @@ mod tests { let mut world = World::new(); let dead_ref = SceneEntityMapper::world_scope(&mut map, &mut world, |_, mapper| { - mapper.map(Entity::from_raw(0)) + mapper.map_entity(Entity::from_raw(0)) }); // Next allocated entity should be a further generation on the same index diff --git a/crates/bevy_hierarchy/src/components/children.rs b/crates/bevy_hierarchy/src/components/children.rs index aafda1d5c4088..d980289abca79 100644 --- a/crates/bevy_hierarchy/src/components/children.rs +++ b/crates/bevy_hierarchy/src/components/children.rs @@ -31,7 +31,7 @@ pub struct Children(pub(crate) SmallVec<[Entity; 8]>); impl MapEntities for Children { fn map_entities(&mut self, entity_mapper: &mut M) { for entity in &mut self.0 { - *entity = entity_mapper.map(*entity); + *entity = entity_mapper.map_entity(*entity); } } } diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index 5b41ddbf96682..bb2b6abba09b1 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -57,7 +57,7 @@ impl FromWorld for Parent { impl MapEntities for Parent { fn map_entities(&mut self, entity_mapper: &mut M) { - self.0 = entity_mapper.map(self.0); + self.0 = entity_mapper.map_entity(self.0); } } diff --git a/crates/bevy_render/src/mesh/mesh/skinning.rs b/crates/bevy_render/src/mesh/mesh/skinning.rs index a876e4dcb896c..616f2a5472abf 100644 --- a/crates/bevy_render/src/mesh/mesh/skinning.rs +++ b/crates/bevy_render/src/mesh/mesh/skinning.rs @@ -19,7 +19,7 @@ pub struct SkinnedMesh { impl MapEntities for SkinnedMesh { fn map_entities(&mut self, entity_mapper: &mut M) { for joint in &mut self.joints { - *joint = entity_mapper.map(*joint); + *joint = entity_mapper.map_entity(*joint); } } } diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index 2dad234c35fd5..81f7692fbfe37 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -551,7 +551,7 @@ mod tests { impl MapEntities for MyEntityRef { fn map_entities(&mut self, entity_mapper: &mut M) { - self.0 = entity_mapper.map(self.0); + self.0 = entity_mapper.map_entity(self.0); } } diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index c65db2a0b073e..611371b586232 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -62,7 +62,7 @@ impl MapEntities for WindowRef { fn map_entities(&mut self, entity_mapper: &mut M) { match self { Self::Entity(entity) => { - *entity = entity_mapper.map(*entity); + *entity = entity_mapper.map_entity(*entity); } Self::Primary => {} }; From 4e02d4f92da13062e741219984adbb403f416745 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 26 Jan 2024 16:14:38 -0500 Subject: [PATCH 17/21] update doc comments --- crates/bevy_ecs/src/entity/map_entities.rs | 4 ++-- crates/bevy_ecs/src/reflect/map_entities.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index c0faa202a78fe..dc090f3812c97 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -19,7 +19,7 @@ use bevy_utils::EntityHashMap; /// /// ``` /// use bevy_ecs::prelude::*; -/// use bevy_ecs::entity::{SceneEntityMapper, MapEntities, EntityMapper}; +/// use bevy_ecs::entity::{MapEntities}; /// /// #[derive(Component)] /// struct Spring { @@ -97,7 +97,7 @@ pub struct SceneEntityMapper<'m> { } impl<'m> SceneEntityMapper<'m> { - #[deprecated(since = "0.13.0", note = "please use `EntityMapper::map` instead")] + #[deprecated(since = "0.13.0", note = "please use `EntityMapper::map_entity` instead")] /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. pub fn get_or_reserve(&mut self, entity: Entity) -> Entity { self.map_entity(entity) diff --git a/crates/bevy_ecs/src/reflect/map_entities.rs b/crates/bevy_ecs/src/reflect/map_entities.rs index 5c7805dd68d8f..af9d1ef451f9d 100644 --- a/crates/bevy_ecs/src/reflect/map_entities.rs +++ b/crates/bevy_ecs/src/reflect/map_entities.rs @@ -10,7 +10,7 @@ use bevy_utils::EntityHashMap; /// Since a given `Entity` ID is only valid for the world it came from, when performing deserialization /// any stored IDs need to be re-allocated in the destination world. /// -/// See [`MapEntities`] for more information. +/// See [`SceneEntityMapper`] and [`MapEntities`] for more information. #[derive(Clone)] pub struct ReflectMapEntities { map_all_entities: fn(&mut World, &mut SceneEntityMapper), From 24726207dbca39936be6a10454201f3aafa13eee Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 26 Jan 2024 16:34:16 -0500 Subject: [PATCH 18/21] fmt --- crates/bevy_ecs/src/entity/map_entities.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index dc090f3812c97..766e6d603369d 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -97,7 +97,10 @@ pub struct SceneEntityMapper<'m> { } impl<'m> SceneEntityMapper<'m> { - #[deprecated(since = "0.13.0", note = "please use `EntityMapper::map_entity` instead")] + #[deprecated( + since = "0.13.0", + note = "please use `EntityMapper::map_entity` instead" + )] /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. pub fn get_or_reserve(&mut self, entity: Entity) -> Entity { self.map_entity(entity) From ec2d3cbd632539176aa1a7fe596bda2cb38b1374 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sun, 28 Jan 2024 14:23:09 -0500 Subject: [PATCH 19/21] update docs --- crates/bevy_ecs/src/entity/map_entities.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 766e6d603369d..4e479e933eb6b 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -19,7 +19,7 @@ use bevy_utils::EntityHashMap; /// /// ``` /// use bevy_ecs::prelude::*; -/// use bevy_ecs::entity::{MapEntities}; +/// use bevy_ecs::entity::MapEntities; /// /// #[derive(Component)] /// struct Spring { @@ -38,22 +38,24 @@ use bevy_utils::EntityHashMap; pub trait MapEntities { /// Updates all [`Entity`] references stored inside using `entity_mapper`. /// - /// Implementors should look up any and all [`Entity`] values stored within and + /// Implementors should look up any and all [`Entity`] values stored within `self` and /// update them to the mapped values via `entity_mapper`. fn map_entities(&mut self, entity_mapper: &mut M); } -/// Any implementor of this trait knows how to map an [`Entity`] into another [`Entity`]. +/// An implementor of this trait knows how to map an [`Entity`] into another [`Entity`]. /// -/// Usually this is done by using a [`EntityHashMap`] to map the [`Entity`] references. -/// This can be used to map [`Entity`] references from one [`World`] to another. +/// Usually this is done by using an [`EntityHashMap`] to map source entities +/// (mapper inputs) to the current world's entities (mapper outputs). +/// +/// More generally, this can be used to map [`Entity`] references between any two [`Worlds`](World). pub trait EntityMapper { /// Map an entity to another entity fn map_entity(&mut self, entity: Entity) -> Entity; } impl EntityMapper for SceneEntityMapper<'_> { - /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. + /// Returns the corresponding mapped entity or reserves a new dead entity ID in the current world if it is absent. fn map_entity(&mut self, entity: Entity) -> Entity { if let Some(&mapped) = self.map.get(&entity) { return mapped; @@ -101,7 +103,7 @@ impl<'m> SceneEntityMapper<'m> { since = "0.13.0", note = "please use `EntityMapper::map_entity` instead" )] - /// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent. + /// Returns the corresponding mapped entity or reserves a new dead entity ID in the current world if it is absent. pub fn get_or_reserve(&mut self, entity: Entity) -> Entity { self.map_entity(entity) } From 97d225ecb0ab7106b483516a86582343200f5ee3 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sun, 28 Jan 2024 14:29:09 -0500 Subject: [PATCH 20/21] add doctest example --- crates/bevy_ecs/src/entity/map_entities.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 4e479e933eb6b..253f1e85e1690 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -49,6 +49,25 @@ pub trait MapEntities { /// (mapper inputs) to the current world's entities (mapper outputs). /// /// More generally, this can be used to map [`Entity`] references between any two [`Worlds`](World). +/// +/// ## Example +/// +/// ``` +/// # use bevy_ecs::entity::{Entity, EntityMapper}; +/// # use bevy_utils::EntityHashMap; +/// +/// pub struct SimpleEntityMapper { +/// map: EntityHashMap, +/// } +/// +/// // Example implementation of EntityMapper where we map an entity to another entity if it exists +/// // in the underlying `EntityHashMap`, otherwise we just return the original entity. +/// impl EntityMapper for SimpleEntityMapper { +/// fn map_entity(&mut self, entity: Entity) -> Entity { +/// self.map.get(&entity).copied().unwrap_or(entity) +/// } +/// } +/// ``` pub trait EntityMapper { /// Map an entity to another entity fn map_entity(&mut self, entity: Entity) -> Entity; From c53e01741577b7ce34413f12f8c6aa85550faa96 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 29 Jan 2024 11:43:55 -0500 Subject: [PATCH 21/21] Fix whitespace in example Co-authored-by: Hennadii Chernyshchyk --- 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 69f3ac7546523..59949c1603615 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -57,7 +57,7 @@ pub trait MapEntities { /// ``` /// # use bevy_ecs::entity::{Entity, EntityMapper}; /// # use bevy_utils::EntityHashMap; -/// +/// # /// pub struct SimpleEntityMapper { /// map: EntityHashMap, /// }