Refactor EntityEvent to support ContainsEntity, unlocking the use of kinded entities with observers (#21408)

This is related to #21384

# Objective

The goal of this change is to enable `EntityEvent` to work with any
Entity-like type which implements `ContainsEntity`.

## Solution

- Change return type of `EntityEvent::event_target` from `Entity` to
`&impl ContainsEntity`
- Change all appropriate call sites to use `event_target().entity()`
instead of `event_target()`
- Refactor `EntityEvent::event_target_mut` into
`EntityEvent::set_event_target`

I'm not fully happy with this solution yet, but I'm opening the PR to
discuss the options from here.

Mainly the issue revolves around `set_event_target`. To make this work,
we need the underlying type to be constructible from an entity. This may
not always be safe from user's perspective (it wouldn't be with
`Instance<T>` for example).

Based on my understanding of the system, an event's target is only
mutated in the case of event propagation, which makes sense.

To work around this, I propose that we flag `EntityEvents` as
**immutable** or not; or "not propagatable" or not (I'm open to other
terminology! :P).

This would allow us to implement `set_event_target` as `unreachable()!`
and throw an error instead if the user tries to propagate such an event.
We could even maybe enforce this compile time but it'll require
additional complexity (mostly in form of different permutations of
`trigger_*` method family).

## Testing

- Added `test_derive_entity_event` to cover all permutations

Author: Zeenobit

crates/bevy_ecs/macros/src/event.rs

@@ -134,21 +134,31 @@ pub fn derive_entity_event(input: TokenStream) -> TokenStream {
     } else {
         quote! {#bevy_ecs_path::event::EntityTrigger}
     };
+
+    let set_entity_event_target_impl = if propagate {
+        quote! {
+            impl #impl_generics #bevy_ecs_path::event::SetEntityEventTarget for #struct_name #type_generics #where_clause {
+                fn set_event_target(&mut self, entity: #bevy_ecs_path::entity::Entity) {
+                    self.#entity_field = Into::into(entity);
+                }
+            }
+        }
+    } else {
+        quote! {}
+    };
+
     TokenStream::from(quote! {
         impl #impl_generics #bevy_ecs_path::event::Event for #struct_name #type_generics #where_clause {
             type Trigger<'a> = #trigger;
         }
 
         impl #impl_generics #bevy_ecs_path::event::EntityEvent for #struct_name #type_generics #where_clause {
             fn event_target(&self) -> #bevy_ecs_path::entity::Entity {
-                self.#entity_field
-            }
-
-            fn event_target_mut(&mut self) -> &mut #bevy_ecs_path::entity::Entity {
-                &mut self.#entity_field
+                #bevy_ecs_path::entity::ContainsEntity::entity(&self.#entity_field)
             }
         }
 
+        #set_entity_event_target_impl
     })
 }
 

crates/bevy_ecs/src/event/mod.rs

@@ -135,6 +135,33 @@ pub trait Event: Send + Sync + Sized + 'static {
 /// struct Explode(#[event_target] Entity);
 /// ```
 ///
+/// You may also use any type which implements [`ContainsEntity`](crate::entity::ContainsEntity) as the event target:
+///
+/// ```
+/// # use bevy_ecs::prelude::*;
+/// struct Bomb(Entity);
+///
+/// impl ContainsEntity for Bomb {
+///     fn entity(&self) -> Entity {
+///         self.0
+///     }
+/// }
+///
+/// #[derive(EntityEvent)]
+/// struct Explode(Bomb);
+/// ```
+///
+/// By default, an [`EntityEvent`] is immutable. This means the event data, including the target, does not change while the event
+/// is triggered. However, to support event propagation, your event must also implement the [`SetEntityEventTarget`] trait.
+///
+/// This trait is automatically implemented for you if you enable event propagation:
+/// ```
+/// # use bevy_ecs::prelude::*;
+/// #[derive(EntityEvent)]
+/// #[entity_event(propagate)]
+/// struct Explode(Entity);
+/// ```
+///
 /// ## Trigger Behavior
 ///
 /// When derived, [`EntityEvent`] defaults to setting [`Event::Trigger`] to [`EntityTrigger`], which will run all normal "untargeted"
@@ -284,11 +311,21 @@ pub trait Event: Send + Sync + Sized + 'static {
 pub trait EntityEvent: Event {
     /// The [`Entity`] "target" of this [`EntityEvent`]. When triggered, this will run observers that watch for this specific entity.
     fn event_target(&self) -> Entity;
-    /// Returns a mutable reference to the [`Entity`] "target" of this [`EntityEvent`]. When triggered, this will run observers that watch for this specific entity.
+}
+
+/// A trait which is used to set the target of an [`EntityEvent`].
+///
+/// By default, entity events are immutable; meaning their target does not change during the lifetime of the event. However, some events
+/// may require mutable access to provide features such as event propagation.
+///
+/// You should never need to implement this trait manually if you use `#[derive(EntityEvent)]`. It is automatically implemented for you if you
+/// use `#[entity_event(propagate)]`.
+pub trait SetEntityEventTarget: EntityEvent {
+    /// Sets the [`Entity`] "target" of this [`EntityEvent`]. When triggered, this will run observers that watch for this specific entity.
     ///
-    /// Note: In general, this should not be mutated from within an [`Observer`](crate::observer::Observer), as this will not "retarget"
+    /// Note: In general, this should not be called from within an [`Observer`](crate::observer::Observer), as this will not "retarget"
     /// the event in any of Bevy's built-in [`Trigger`] implementations.
-    fn event_target_mut(&mut self) -> &mut Entity;
+    fn set_event_target(&mut self, entity: Entity);
 }
 
 impl World {
@@ -931,4 +968,127 @@ mod tests {
         });
         schedule.run(&mut world);
     }
+
+    #[test]
+    fn test_derive_entity_event() {
+        use bevy_ecs::prelude::*;
+
+        struct Entitoid(Entity);
+
+        impl ContainsEntity for Entitoid {
+            fn entity(&self) -> Entity {
+                self.0
+            }
+        }
+
+        struct MutableEntitoid(Entity);
+
+        impl ContainsEntity for MutableEntitoid {
+            fn entity(&self) -> Entity {
+                self.0
+            }
+        }
+
+        impl From<Entity> for MutableEntitoid {
+            fn from(value: Entity) -> Self {
+                Self(value)
+            }
+        }
+
+        #[derive(EntityEvent)]
+        struct A(Entity);
+
+        #[derive(EntityEvent)]
+        #[entity_event(propagate)]
+        struct AP(Entity);
+
+        #[derive(EntityEvent)]
+        struct B {
+            entity: Entity,
+        }
+
+        #[derive(EntityEvent)]
+        #[entity_event(propagate)]
+        struct BP {
+            entity: Entity,
+        }
+
+        #[derive(EntityEvent)]
+        struct C {
+            #[event_target]
+            target: Entity,
+        }
+
+        #[derive(EntityEvent)]
+        #[entity_event(propagate)]
+        struct CP {
+            #[event_target]
+            target: Entity,
+        }
+
+        #[derive(EntityEvent)]
+        struct D(Entitoid);
+
+        // SHOULD NOT COMPILE:
+        // #[derive(EntityEvent)]
+        // #[entity_event(propagate)]
+        // struct DP(Entitoid);
+
+        #[derive(EntityEvent)]
+        struct E {
+            entity: Entitoid,
+        }
+
+        // SHOULD NOT COMPILE:
+        // #[derive(EntityEvent)]
+        // #[entity_event(propagate)]
+        // struct EP {
+        //     entity: Entitoid,
+        // }
+
+        #[derive(EntityEvent)]
+        struct F {
+            #[event_target]
+            target: Entitoid,
+        }
+
+        // SHOULD NOT COMPILE:
+        // #[derive(EntityEvent)]
+        // #[entity_event(propagate)]
+        // struct FP {
+        //     #[event_target]
+        //     target: Entitoid,
+        // }
+
+        #[derive(EntityEvent)]
+        #[entity_event(propagate)]
+        struct G(MutableEntitoid);
+
+        impl From<Entity> for G {
+            fn from(value: Entity) -> Self {
+                Self(value.into())
+            }
+        }
+
+        let mut world = World::new();
+        let entity = world.spawn_empty().id();
+
+        world.entity_mut(entity).trigger(A);
+        world.entity_mut(entity).trigger(AP);
+        world.trigger(B { entity });
+        world.trigger(BP { entity });
+        world.trigger(C { target: entity });
+        world.trigger(CP { target: entity });
+        world.trigger(D(Entitoid(entity)));
+        world.trigger(E {
+            entity: Entitoid(entity),
+        });
+        world.trigger(F {
+            target: Entitoid(entity),
+        });
+        world.trigger(G(MutableEntitoid(entity)));
+        world.entity_mut(entity).trigger(G::from);
+
+        // No asserts; test just needs to compile
+    }
 }

crates/bevy_ecs/src/event/trigger.rs

@@ -1,3 +1,4 @@
+use crate::event::SetEntityEventTarget;
 use crate::{
     component::ComponentId,
     entity::Entity,
@@ -267,7 +268,7 @@ impl<const AUTO_PROPAGATE: bool, E: EntityEvent, T: Traversal<E>> fmt::Debug
 // - `E`'s [`Event::Trigger`] is constrained to [`PropagateEntityTrigger<E>`]
 unsafe impl<
         const AUTO_PROPAGATE: bool,
-        E: EntityEvent + for<'a> Event<Trigger<'a> = Self>,
+        E: EntityEvent + SetEntityEventTarget + for<'a> Event<Trigger<'a> = Self>,
         T: Traversal<E>,
     > Trigger<E> for PropagateEntityTrigger<AUTO_PROPAGATE, E, T>
 {
@@ -309,7 +310,7 @@ unsafe impl<
                 break;
             }
 
-            *event.event_target_mut() = current_entity;
+            event.set_event_target(current_entity);
             // SAFETY:
             // - `observers` come from `world` and match the event type `E`, enforced by the call to `trigger`
             // - the passed in event pointer comes from `event`, which is an `Event`

release-content/migration-guides/immutable-entity-events.md

@@ -0,0 +1,11 @@
+---
+title: "Immutable Entity Events"
+pull_requests: [21408]
+---
+
+The mutable methods of `EntityEvent` (`EntityEvent::from` and `EntityEvent::event_target_mut`)
+have been moved to a separate trait: `SetEntityEventTarget`
+
+This makes all `EntityEvents` immutable by default.
+
+`SetEntityEventTarget` is implemented automatically for propagated events (e.g. `#[entity_event(propagate)]`).