Change Map to Visit, split Reflect

crates/bevy_ecs/src/entity/mod.rs

@@ -35,12 +35,12 @@
 //! [`World::despawn`]: crate::world::World::despawn
 //! [`EntityWorldMut::insert`]: crate::world::EntityWorldMut::insert
 //! [`EntityWorldMut::remove`]: crate::world::EntityWorldMut::remove
-mod map_entities;
+mod visit_entities;
 #[cfg(feature = "bevy_reflect")]
 use bevy_reflect::Reflect;
 #[cfg(all(feature = "bevy_reflect", feature = "serialize"))]
 use bevy_reflect::{ReflectDeserialize, ReflectSerialize};
-pub use map_entities::*;
+pub use visit_entities::*;
 
 mod hash;
 pub use hash::*;

crates/bevy_ecs/src/entity/map_entities.rs → crates/bevy_ecs/src/entity/visit_entities.rs

@@ -16,21 +16,21 @@ use super::EntityHashMap;
 ///
 /// It may be useful to implement directly for types that can't produce an
 /// iterator for lifetime reasons, such as those involving internal mutexes.
-pub trait MapEntities {
+pub trait VisitEntities {
     /// Apply an operation to all contained entities.
-    fn map_entities<F: FnMut(Entity)>(&self, f: F);
+    fn visit_entities<F: FnMut(Entity)>(&self, f: F);
     /// Apply an operation to mutable references to all entities.
-    fn map_entities_mut<F: FnMut(&mut Entity)>(&mut self, f: F);
+    fn visit_entities_mut<F: FnMut(&mut Entity)>(&mut self, f: F);
 }
 
-impl<T> MapEntities for T
+impl<T> VisitEntities for T
 where
     T: IterEntities,
 {
-    fn map_entities<F: FnMut(Entity)>(&self, f: F) {
+    fn visit_entities<F: FnMut(Entity)>(&self, f: F) {
         self.iter_entities().for_each(f);
     }
-    fn map_entities_mut<F: FnMut(&mut Entity)>(&mut self, f: F) {
+    fn visit_entities_mut<F: FnMut(&mut Entity)>(&mut self, f: F) {
         self.iter_entities_mut().for_each(f);
     }
 }
@@ -179,8 +179,7 @@ impl EntityMapper for SceneEntityMapper<'_> {
 /// world. These newly allocated references are guaranteed to never point to any living entity in that world.
 ///
 /// 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.
+/// [`Entity`]. After it is finished being used, this entity is despawned and the requisite number of generations reserved.
 pub struct SceneEntityMapper<'m> {
     /// A mapping from one set of entities to another.
     ///

crates/bevy_ecs/src/reflect/map_entities.rs

@@ -1,118 +1,85 @@
 use crate::{
     component::Component,
-    entity::{Entity, EntityHashMap, EntityMapper, MapEntities, SceneEntityMapper},
+    entity::{Entity, EntityHashMap, EntityMapper, SceneEntityMapper, VisitEntities},
     world::World,
 };
-use bevy_reflect::{FromReflect, FromType, PartialReflect};
+use bevy_reflect::{FromReflect, FromType};
 
 /// For a specific type of component, this maps any fields with values of type [`Entity`] to a new world.
 ///
 /// 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 [`SceneEntityMapper`] and [`MapEntities`] for more information.
-///
-/// [`MapEntities`]: crate::entity::MapEntities
+/// See [`SceneEntityMapper`] for more information.
 #[derive(Clone)]
 pub struct ReflectMapEntities {
-    map_all_world_entities: fn(&mut World, &mut SceneEntityMapper),
-    map_world_entities: fn(&mut World, &mut SceneEntityMapper, &[Entity]),
-    map_entities_mut: fn(&mut dyn PartialReflect, &mut dyn FnMut(&mut Entity)),
-    map_entities: fn(&dyn PartialReflect, &mut dyn FnMut(Entity)),
+    map_all_entities: fn(&mut World, &mut SceneEntityMapper),
+    map_entities: fn(&mut World, &mut SceneEntityMapper, &[Entity]),
 }
 
 impl ReflectMapEntities {
-    /// A general method for applying [`MapEntities`] behavior to all elements in an [`EntityHashMap<Entity>`].
+    /// A general method for applying [`VisitEntities`] behavior to all elements in an [`EntityHashMap<Entity>`].
     ///
     /// Be mindful in its usage: Works best in situations where the entities in the [`EntityHashMap<Entity>`] are newly
     /// created, before systems have a chance to add new components. If some of the entities referred to
     /// by the [`EntityHashMap<Entity>`] might already contain valid entity references, you should use
-    /// [`map_world_entities`](Self::map_entities).
+    /// [`map_entities`](Self::map_entities).
     ///
     /// 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_world_entities`](Self::map_all_world_entities), those `Parent`
+    /// 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.
     ///
-    /// [`MapEntities`]: crate::entity::MapEntities
-    pub fn map_all_world_entities(
-        &self,
-        world: &mut World,
-        entity_map: &mut EntityHashMap<Entity>,
-    ) {
-        SceneEntityMapper::world_scope(entity_map, world, self.map_all_world_entities);
+    /// [`VisitEntities`]: crate::entity::VisitEntities
+    pub fn map_all_entities(&self, world: &mut World, entity_map: &mut EntityHashMap<Entity>) {
+        SceneEntityMapper::world_scope(entity_map, world, self.map_all_entities);
     }
 
-    /// A general method for applying [`MapEntities`] behavior to elements in an [`EntityHashMap<Entity>`]. Unlike
-    /// [`map_all_world_entities`](Self::map_all_world_entities), this is applied to specific entities, not all values
+    /// A general method for applying [`VisitEntities`] behavior to elements in an [`EntityHashMap<Entity>`]. Unlike
+    /// [`map_all_entities`](Self::map_all_entities), this is applied to specific entities, not all values
     /// in the [`EntityHashMap<Entity>`].
     ///
     /// This is useful mostly for when you need to be careful not to update components that already contain valid entity
-    /// values. See [`map_all_world_entities`](Self::map_all_world_entities) for more details.
+    /// values. See [`map_all_entities`](Self::map_all_entities) for more details.
     ///
-    /// [`MapEntities`]: crate::entity::MapEntities
-    pub fn map_world_entities(
+    /// [`VisitEntities`]: crate::entity::VisitEntities
+    pub fn map_entities(
         &self,
         world: &mut World,
         entity_map: &mut EntityHashMap<Entity>,
         entities: &[Entity],
     ) {
         SceneEntityMapper::world_scope(entity_map, world, |world, mapper| {
-            (self.map_world_entities)(world, mapper, entities);
+            (self.map_entities)(world, mapper, entities);
         });
     }
-
-    /// A general method for applying an operation to all entities in a
-    /// reflected component.
-    pub fn map_entities(&self, component: &dyn PartialReflect, f: &mut dyn FnMut(Entity)) {
-        (self.map_entities)(component, f);
-    }
-
-    /// A general method for applying an operation that may modify entities in a
-    /// reflected component.
-    pub fn map_entities_mut(
-        &self,
-        component: &mut dyn PartialReflect,
-        f: &mut dyn FnMut(&mut Entity),
-    ) {
-        (self.map_entities_mut)(component, f);
-    }
 }
 
-impl<C: Component + FromReflect + MapEntities> FromType<C> for ReflectMapEntities {
+impl<C: Component + FromReflect + VisitEntities> FromType<C> for ReflectMapEntities {
     fn from_type() -> Self {
         ReflectMapEntities {
-            map_world_entities: |world, entity_mapper, entities| {
+            map_entities: |world, entity_mapper, entities| {
                 for &entity in entities {
                     if let Some(mut component) = world.get_mut::<C>(entity) {
-                        component.map_entities_mut(|entity| {
+                        component.visit_entities_mut(|entity| {
                             *entity = entity_mapper.map_entity(*entity);
                         });
                     }
                 }
             },
-            map_all_world_entities: |world, entity_mapper| {
+            map_all_entities: |world, entity_mapper| {
                 let entities = entity_mapper
                     .get_map()
                     .values()
                     .copied()
                     .collect::<Vec<Entity>>();
                 for entity in &entities {
                     if let Some(mut component) = world.get_mut::<C>(*entity) {
-                        component.map_entities_mut(|entity| {
+                        component.visit_entities_mut(|entity| {
                             *entity = entity_mapper.map_entity(*entity);
                         });
                     }
                 }
             },
-            map_entities: |component, f| {
-                let mut concrete = C::from_reflect(component).unwrap();
-                concrete.map_entities_mut(|entity| f(*entity));
-            },
-            map_entities_mut: |component, f| {
-                let mut concrete = C::from_reflect(component).unwrap();
-                concrete.map_entities_mut(f);
-                component.apply(&concrete);
-            },
         }
     }
 }
@@ -122,64 +89,33 @@ impl<C: Component + FromReflect + MapEntities> FromType<C> for ReflectMapEntitie
 /// 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 [`SceneEntityMapper`] and [`MapEntities`] for more information.
-///
-/// [`MapEntities`]: crate::entity::MapEntities
+/// See [`SceneEntityMapper`] for more information.
 #[derive(Clone)]
 pub struct ReflectMapEntitiesResource {
-    map_world_entities: fn(&mut World, &mut SceneEntityMapper),
-    map_entities_mut: fn(&mut dyn PartialReflect, &mut dyn FnMut(&mut Entity)),
-    map_entities: fn(&dyn PartialReflect, &mut dyn FnMut(Entity)),
+    map_entities: fn(&mut World, &mut SceneEntityMapper),
 }
 
 impl ReflectMapEntitiesResource {
-    /// A method for applying [`MapEntities`] behavior to elements in a [`EntityHashMap<Entity>`].
-    ///
-    /// [`MapEntities`]: crate::entity::MapEntities
-    pub fn map_world_entities(&self, world: &mut World, entity_map: &mut EntityHashMap<Entity>) {
+    /// A method for remapping entities via a [`EntityHashMap<Entity>`].
+    pub fn map_entities(&self, world: &mut World, entity_map: &mut EntityHashMap<Entity>) {
         SceneEntityMapper::world_scope(entity_map, world, |world, mapper| {
-            (self.map_world_entities)(world, mapper);
+            (self.map_entities)(world, mapper);
         });
     }
-
-    /// A general method for applying an operation to all entities in a
-    /// reflected component.
-    pub fn map_entities(&self, component: &dyn PartialReflect, f: &mut dyn FnMut(Entity)) {
-        (self.map_entities)(component, f);
-    }
-
-    /// A general method for applying an operation that may modify entities in a
-    /// reflected component.
-    pub fn map_entities_mut(
-        &self,
-        component: &mut dyn PartialReflect,
-        f: &mut dyn FnMut(&mut Entity),
-    ) {
-        (self.map_entities_mut)(component, f);
-    }
 }
 
-impl<R: crate::system::Resource + FromReflect + MapEntities> FromType<R>
+impl<R: crate::system::Resource + FromReflect + VisitEntities> FromType<R>
     for ReflectMapEntitiesResource
 {
     fn from_type() -> Self {
         ReflectMapEntitiesResource {
-            map_world_entities: |world, entity_mapper| {
+            map_entities: |world, entity_mapper| {
                 if let Some(mut resource) = world.get_resource_mut::<R>() {
-                    resource.map_entities_mut(|entity| {
+                    resource.visit_entities_mut(|entity| {
                         *entity = entity_mapper.map_entity(*entity);
                     });
                 }
             },
-            map_entities: |component, f| {
-                let mut concrete = R::from_reflect(component).unwrap();
-                concrete.map_entities_mut(|entity| f(*entity));
-            },
-            map_entities_mut: |component, f| {
-                let mut concrete = R::from_reflect(component).unwrap();
-                concrete.map_entities_mut(f);
-                component.apply(&concrete);
-            },
         }
     }
 }

crates/bevy_ecs/src/reflect/mod.rs

@@ -18,13 +18,15 @@ mod entity_commands;
 mod from_world;
 mod map_entities;
 mod resource;
+mod visit_entities;
 
 pub use bundle::{ReflectBundle, ReflectBundleFns};
 pub use component::{ReflectComponent, ReflectComponentFns};
 pub use entity_commands::ReflectCommandExt;
 pub use from_world::{ReflectFromWorld, ReflectFromWorldFns};
 pub use map_entities::{ReflectMapEntities, ReflectMapEntitiesResource};
 pub use resource::{ReflectResource, ReflectResourceFns};
+pub use visit_entities::ReflectVisitEntities;
 
 /// A [`Resource`] storing [`TypeRegistry`] for
 /// type registrations relevant to a whole app.

crates/bevy_ecs/src/reflect/visit_entities.rs

@@ -0,0 +1,45 @@
+use crate::entity::{Entity, VisitEntities};
+use bevy_reflect::{FromReflect, FromType, PartialReflect};
+
+/// For a reflected value, apply an operation to all contained entities.
+///
+/// See [`VisitEntities`] for more details.
+#[derive(Clone)]
+pub struct ReflectVisitEntities {
+    visit_entities_mut: fn(&mut dyn PartialReflect, &mut dyn FnMut(&mut Entity)),
+    visit_entities: fn(&dyn PartialReflect, &mut dyn FnMut(Entity)),
+}
+
+impl ReflectVisitEntities {
+    /// A general method for applying an operation to all entities in a
+    /// reflected component.
+    pub fn visit_entities(&self, component: &dyn PartialReflect, f: &mut dyn FnMut(Entity)) {
+        (self.visit_entities)(component, f);
+    }
+
+    /// A general method for applying an operation that may modify entities in a
+    /// reflected component.
+    pub fn visit_entities_mut(
+        &self,
+        component: &mut dyn PartialReflect,
+        f: &mut dyn FnMut(&mut Entity),
+    ) {
+        (self.visit_entities_mut)(component, f);
+    }
+}
+
+impl<C: FromReflect + VisitEntities> FromType<C> for ReflectVisitEntities {
+    fn from_type() -> Self {
+        ReflectVisitEntities {
+            visit_entities: |component, f| {
+                let mut concrete = C::from_reflect(component).unwrap();
+                concrete.visit_entities_mut(|entity| f(*entity));
+            },
+            visit_entities_mut: |component, f| {
+                let mut concrete = C::from_reflect(component).unwrap();
+                concrete.visit_entities_mut(f);
+                component.apply(&concrete);
+            },
+        }
+    }
+}

crates/bevy_hierarchy/src/components/children.rs

@@ -163,11 +163,11 @@ impl<'a> IntoIterator for &'a Children {
 
 #[cfg(test)]
 mod test {
-    use bevy_ecs::entity::MapEntities;
+    use bevy_ecs::entity::VisitEntities;
 
     use super::*;
 
-    fn assert_impls_map_entities<M: MapEntities>() {}
+    fn assert_impls_map_entities<M: VisitEntities>() {}
 
     #[test]
     fn children_impls_map_entities() {

crates/bevy_scene/src/dynamic_scene.rs

@@ -130,7 +130,7 @@ impl DynamicScene {
                 "we should be getting TypeId from this TypeRegistration in the first place",
             );
             if let Some(map_entities_reflect) = registration.data::<ReflectMapEntities>() {
-                map_entities_reflect.map_world_entities(world, entity_map, &entities);
+                map_entities_reflect.map_entities(world, entity_map, &entities);
             }
         }
 
@@ -159,7 +159,7 @@ impl DynamicScene {
 
             // Map entities in the resource if it implements [`MapEntities`].
             if let Some(map_entities_reflect) = registration.data::<ReflectMapEntitiesResource>() {
-                map_entities_reflect.map_world_entities(world, entity_map);
+                map_entities_reflect.map_entities(world, entity_map);
             }
         }
 

crates/bevy_scene/src/scene.rs

@@ -127,7 +127,7 @@ impl Scene {
 
         for registration in type_registry.iter() {
             if let Some(map_entities_reflect) = registration.data::<ReflectMapEntities>() {
-                map_entities_reflect.map_all_world_entities(world, entity_map);
+                map_entities_reflect.map_all_entities(world, entity_map);
             }
         }
 

crates/bevy_window/src/window.rs

@@ -1219,12 +1219,12 @@ pub struct ClosingWindow;
 
 #[cfg(test)]
 mod tests {
-    use bevy_ecs::entity::MapEntities;
+    use bevy_ecs::entity::VisitEntities;
 
     use super::*;
 
     // Compile-time assertion that a type implements MapEntitiesMut
-    fn impl_map_entities<M: MapEntities>() {}
+    fn impl_map_entities<M: VisitEntities>() {}
 
     // Ensure that WidowRef implements MapEntitiesMut
     #[test]