Replace VisitEntities with MapEntities (#18432)

# Objective

There are currently too many disparate ways to handle entity mapping,
especially after #17687. We now have MapEntities, VisitEntities,
VisitEntitiesMut, Component::visit_entities,
Component::visit_entities_mut.

Our only known use case at the moment for these is entity mapping. This
means we have significant consolidation potential.

Additionally, VisitEntitiesMut cannot be implemented for map-style
collections like HashSets, as you cant "just" mutate a `&mut Entity`.
Our current approach to Component mapping requires VisitEntitiesMut,
meaning this category of entity collection isn't mappable. `MapEntities`
is more generally applicable. Additionally, the _existence_ of the
blanket From impl on VisitEntitiesMut blocks us from implementing
MapEntities for HashSets (or any types we don't own), because the owner
could always add a conflicting impl in the future.

## Solution

Use `MapEntities` everywhere and remove all "visit entities" usages.

* Add `Component::map_entities`
* Remove `Component::visit_entities`, `Component::visit_entities_mut`,
`VisitEntities`, and `VisitEntitiesMut`
* Support deriving `Component::map_entities` in `#[derive(Coomponent)]`
* Add `#[derive(MapEntities)]`, and share logic with the
`Component::map_entities` derive.
* Add `ComponentCloneCtx::queue_deferred`, which is command-like logic
that runs immediately after normal clones. Reframe `FromWorld` fallback
logic in the "reflect clone" impl to use it. This cuts out a lot of
unnecessary work and I think justifies the existence of a pseudo-command
interface (given how niche, yet performance sensitive this is).

Note that we no longer auto-impl entity mapping for ` IntoIterator<Item
= &'a Entity>` types, as this would block our ability to implement cases
like `HashMap`. This means the onus is on us (or type authors) to add
explicit support for types that should be mappable.

Also note that the Component-related changes do not require a migration
guide as there hasn't been a release with them yet.

## Migration Guide

If you were previously implementing `VisitEntities` or
`VisitEntitiesMut` (likely via a derive), instead use `MapEntities`.
Those were almost certainly used in the context of Bevy Scenes or
reflection via `ReflectMapEntities`. If you have a case that uses
`VisitEntities` or `VisitEntitiesMut` directly, where `MapEntities` is
not a viable replacement, please let us know!

```rust
// before
#[derive(VisitEntities, VisitEntitiesMut)]
struct Inventory {
  items: Vec<Entity>,
  #[visit_entities(ignore)]
  label: String,
}

// after
#[derive(MapEntities)]
struct Inventory {
  #[entities]
  items: Vec<Entity>,
  label: String,
}
```

Author: cart

crates/bevy_ecs/macros/src/component.rs

@@ -92,12 +92,17 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
         Err(err) => err.into_compile_error().into(),
     };
 
-    let visit_entities = visit_entities(
+    let map_entities = map_entities(
         &ast.data,
-        &bevy_ecs_path,
+        Ident::new("this", Span::call_site()),
         relationship.is_some(),
         relationship_target.is_some(),
-    );
+    ).map(|map_entities_impl| quote! {
+        fn map_entities<M: #bevy_ecs_path::entity::EntityMapper>(this: &mut Self, mapper: &mut M) {
+            use #bevy_ecs_path::entity::MapEntities;
+            #map_entities_impl
+        }
+    });
 
     let storage = storage_path(&bevy_ecs_path, attrs.storage);
 
@@ -295,7 +300,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
                 #clone_behavior
             }
 
-            #visit_entities
+            #map_entities
         }
 
         #relationship
@@ -306,19 +311,18 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
 
 const ENTITIES: &str = "entities";
 
-fn visit_entities(
+pub(crate) fn map_entities(
     data: &Data,
-    bevy_ecs_path: &Path,
+    self_ident: Ident,
     is_relationship: bool,
     is_relationship_target: bool,
-) -> TokenStream2 {
+) -> Option<TokenStream2> {
     match data {
         Data::Struct(DataStruct { fields, .. }) => {
-            let mut visit = Vec::with_capacity(fields.len());
-            let mut visit_mut = Vec::with_capacity(fields.len());
+            let mut map = Vec::with_capacity(fields.len());
 
             let relationship = if is_relationship || is_relationship_target {
-                relationship_field(fields, "VisitEntities", fields.span()).ok()
+                relationship_field(fields, "MapEntities", fields.span()).ok()
             } else {
                 None
             };
@@ -335,27 +339,17 @@ fn visit_entities(
                         .clone()
                         .map_or(Member::from(index), Member::Named);
 
-                    visit.push(quote!(this.#field_member.visit_entities(&mut func);));
-                    visit_mut.push(quote!(this.#field_member.visit_entities_mut(&mut func);));
+                    map.push(quote!(#self_ident.#field_member.map_entities(mapper);));
                 });
-            if visit.is_empty() {
-                return quote!();
+            if map.is_empty() {
+                return None;
             };
-            quote!(
-                fn visit_entities(this: &Self, mut func: impl FnMut(#bevy_ecs_path::entity::Entity)) {
-                    use #bevy_ecs_path::entity::VisitEntities;
-                    #(#visit)*
-                }
-
-                fn visit_entities_mut(this: &mut Self, mut func: impl FnMut(&mut #bevy_ecs_path::entity::Entity)) {
-                    use #bevy_ecs_path::entity::VisitEntitiesMut;
-                    #(#visit_mut)*
-                }
-            )
+            Some(quote!(
+                #(#map)*
+            ))
         }
         Data::Enum(DataEnum { variants, .. }) => {
-            let mut visit = Vec::with_capacity(variants.len());
-            let mut visit_mut = Vec::with_capacity(variants.len());
+            let mut map = Vec::with_capacity(variants.len());
 
             for variant in variants.iter() {
                 let field_members = variant
@@ -377,40 +371,25 @@ fn visit_entities(
                     .map(|member| format_ident!("__self_{}", member))
                     .collect::<Vec<_>>();
 
-                visit.push(
+                map.push(
                     quote!(Self::#ident {#(#field_members: #field_idents,)* ..} => {
-                        #(#field_idents.visit_entities(&mut func);)*
-                    }),
-                );
-                visit_mut.push(
-                    quote!(Self::#ident {#(#field_members: #field_idents,)* ..} => {
-                        #(#field_idents.visit_entities_mut(&mut func);)*
+                        #(#field_idents.map_entities(mapper);)*
                     }),
                 );
             }
 
-            if visit.is_empty() {
-                return quote!();
+            if map.is_empty() {
+                return None;
             };
-            quote!(
-                fn visit_entities(this: &Self, mut func: impl FnMut(#bevy_ecs_path::entity::Entity)) {
-                    use #bevy_ecs_path::entity::VisitEntities;
-                    match this {
-                        #(#visit,)*
-                        _ => {}
-                    }
-                }
 
-                fn visit_entities_mut(this: &mut Self, mut func: impl FnMut(&mut #bevy_ecs_path::entity::Entity)) {
-                    use #bevy_ecs_path::entity::VisitEntitiesMut;
-                    match this {
-                        #(#visit_mut,)*
-                        _ => {}
-                    }
+            Some(quote!(
+                match #self_ident {
+                    #(#map,)*
+                    _ => {}
                 }
-            )
+            ))
         }
-        Data::Union(_) => quote!(),
+        Data::Union(_) => None,
     }
 }
 

crates/bevy_ecs/macros/src/lib.rs

@@ -9,10 +9,13 @@ mod query_filter;
 mod states;
 mod world_query;
 
-use crate::{query_data::derive_query_data_impl, query_filter::derive_query_filter_impl};
+use crate::{
+    component::map_entities, query_data::derive_query_data_impl,
+    query_filter::derive_query_filter_impl,
+};
 use bevy_macro_utils::{derive_label, ensure_no_collision, get_struct_fields, BevyManifest};
 use proc_macro::TokenStream;
-use proc_macro2::{Span, TokenStream as TokenStream2};
+use proc_macro2::{Ident, Span};
 use quote::{format_ident, quote};
 use syn::{
     parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, token::Comma,
@@ -185,105 +188,22 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
     })
 }
 
-fn derive_visit_entities_base(
-    input: TokenStream,
-    trait_name: TokenStream2,
-    gen_methods: impl FnOnce(Vec<TokenStream2>) -> TokenStream2,
-) -> TokenStream {
+#[proc_macro_derive(MapEntities, attributes(entities))]
+pub fn derive_map_entities(input: TokenStream) -> TokenStream {
     let ast = parse_macro_input!(input as DeriveInput);
     let ecs_path = bevy_ecs_path();
-
-    let named_fields = match get_struct_fields(&ast.data) {
-        Ok(fields) => fields,
-        Err(e) => return e.into_compile_error().into(),
-    };
-
-    let field = named_fields
-        .iter()
-        .filter_map(|field| {
-            if let Some(attr) = field
-                .attrs
-                .iter()
-                .find(|a| a.path().is_ident("visit_entities"))
-            {
-                let ignore = attr.parse_nested_meta(|meta| {
-                    if meta.path.is_ident("ignore") {
-                        Ok(())
-                    } else {
-                        Err(meta.error("Invalid visit_entities attribute. Use `ignore`"))
-                    }
-                });
-                return match ignore {
-                    Ok(()) => None,
-                    Err(e) => Some(Err(e)),
-                };
-            }
-            Some(Ok(field))
-        })
-        .map(|res| res.map(|field| field.ident.as_ref()))
-        .collect::<Result<Vec<_>, _>>();
-
-    let field = match field {
-        Ok(field) => field,
-        Err(e) => return e.into_compile_error().into(),
-    };
-
-    if field.is_empty() {
-        return syn::Error::new(
-            ast.span(),
-            format!("Invalid `{}` type: at least one field", trait_name),
-        )
-        .into_compile_error()
-        .into();
-    }
-
-    let field_access = field
-        .iter()
-        .enumerate()
-        .map(|(n, f)| {
-            if let Some(ident) = f {
-                quote! {
-                    self.#ident
-                }
-            } else {
-                let idx = Index::from(n);
-                quote! {
-                    self.#idx
-                }
-            }
-        })
-        .collect::<Vec<_>>();
-
-    let methods = gen_methods(field_access);
-
-    let generics = ast.generics;
-    let (impl_generics, ty_generics, _) = generics.split_for_impl();
+    let map_entities_impl = map_entities(
+        &ast.data,
+        Ident::new("self", Span::call_site()),
+        false,
+        false,
+    );
     let struct_name = &ast.ident;
-
+    let (impl_generics, type_generics, where_clause) = &ast.generics.split_for_impl();
     TokenStream::from(quote! {
-        impl #impl_generics #ecs_path::entity:: #trait_name for #struct_name #ty_generics {
-            #methods
-        }
-    })
-}
-
-#[proc_macro_derive(VisitEntitiesMut, attributes(visit_entities))]
-pub fn derive_visit_entities_mut(input: TokenStream) -> TokenStream {
-    derive_visit_entities_base(input, quote! { VisitEntitiesMut }, |field| {
-        quote! {
-            fn visit_entities_mut<F: FnMut(&mut Entity)>(&mut self, mut f: F) {
-                #(#field.visit_entities_mut(&mut f);)*
-            }
-        }
-    })
-}
-
-#[proc_macro_derive(VisitEntities, attributes(visit_entities))]
-pub fn derive_visit_entities(input: TokenStream) -> TokenStream {
-    derive_visit_entities_base(input, quote! { VisitEntities }, |field| {
-        quote! {
-            fn visit_entities<F: FnMut(Entity)>(&self, mut f: F) {
-                #(#field.visit_entities(&mut f);)*
+        impl #impl_generics #ecs_path::entity::MapEntities for #struct_name #type_generics #where_clause {
+            fn map_entities<M: #ecs_path::entity::EntityMapper>(&mut self, mapper: &mut M) {
+                #map_entities_impl
             }
         }
     })