From a233696f00038be935d98c8ae19520dbbfdaf806 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Thu, 8 Aug 2024 22:07:48 -0500
Subject: [PATCH 1/7] Safer dynamic event handling for observers

---
 benches/benches/bevy_ecs/observers/dynamic.rs |  51 ++++++
 benches/benches/bevy_ecs/observers/mod.rs     |  15 +-
 crates/bevy_app/src/app.rs                    |   3 +-
 crates/bevy_ecs/src/observer/data.rs          | 146 ++++++++++++++++++
 crates/bevy_ecs/src/observer/mod.rs           |  44 +++---
 crates/bevy_ecs/src/observer/runner.rs        |  46 ++++--
 crates/bevy_ecs/src/system/commands/mod.rs    |   8 +-
 crates/bevy_ecs/src/system/observer_system.rs |  15 +-
 crates/bevy_ecs/src/world/entity_ref.rs       |   4 +-
 9 files changed, 280 insertions(+), 52 deletions(-)
 create mode 100644 benches/benches/bevy_ecs/observers/dynamic.rs
 create mode 100644 crates/bevy_ecs/src/observer/data.rs

diff --git a/benches/benches/bevy_ecs/observers/dynamic.rs b/benches/benches/bevy_ecs/observers/dynamic.rs
new file mode 100644
index 0000000000000..f9c05e8f9231e
--- /dev/null
+++ b/benches/benches/bevy_ecs/observers/dynamic.rs
@@ -0,0 +1,51 @@
+use bevy_ecs::{
+    event::Event,
+    observer::{DynamicEvent, EmitDynamicTrigger, EventData, Observer, Trigger},
+    world::{Command, World},
+};
+use criterion::{black_box, Criterion};
+
+pub fn observe_dynamic(criterion: &mut Criterion) {
+    let mut group = criterion.benchmark_group("observe_dynamic");
+    group.warm_up_time(std::time::Duration::from_millis(500));
+    group.measurement_time(std::time::Duration::from_secs(4));
+
+    group.bench_function("1", |bencher| {
+        let mut world = World::new();
+        let event_id_1 = world.init_component::<TestEvent<1>>();
+        world.spawn(Observer::new(empty_listener_set::<DynamicEvent>).with_event(event_id_1));
+        bencher.iter(|| {
+            for _ in 0..10000 {
+                unsafe {
+                    EmitDynamicTrigger::new_with_id(event_id_1, TestEvent::<1>, ())
+                        .apply(&mut world)
+                };
+            }
+        });
+    });
+    group.bench_function("2", |bencher| {
+        let mut world = World::new();
+        let event_id_1 = world.init_component::<TestEvent<1>>();
+        let event_id_2 = world.init_component::<TestEvent<2>>();
+        world.spawn(
+            Observer::new(empty_listener_set::<DynamicEvent>)
+                .with_event(event_id_1)
+                .with_event(event_id_2),
+        );
+        bencher.iter(|| {
+            for _ in 0..10000 {
+                unsafe {
+                    EmitDynamicTrigger::new_with_id(event_id_2, TestEvent::<2>, ())
+                        .apply(&mut world)
+                };
+            }
+        });
+    });
+}
+
+#[derive(Event)]
+struct TestEvent<const N: usize>;
+
+fn empty_listener_set<Set: EventData>(trigger: Trigger<Set>) {
+    black_box(trigger);
+}
diff --git a/benches/benches/bevy_ecs/observers/mod.rs b/benches/benches/bevy_ecs/observers/mod.rs
index 0b8c3f24869ce..ceab56c99e1fb 100644
--- a/benches/benches/bevy_ecs/observers/mod.rs
+++ b/benches/benches/bevy_ecs/observers/mod.rs
@@ -1,8 +1,21 @@
 use criterion::criterion_group;
 
+mod dynamic;
+mod multievent;
 mod propagation;
+mod semidynamic;
 mod simple;
+use dynamic::*;
+use multievent::*;
 use propagation::*;
+use semidynamic::*;
 use simple::*;
 
-criterion_group!(observer_benches, event_propagation, observe_simple);
+criterion_group!(
+    observer_benches,
+    event_propagation,
+    observe_simple,
+    observe_multievent,
+    observe_dynamic,
+    observe_semidynamic
+);
diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs
index 07a29800f93df..b0b42a70a0d4a 100644
--- a/crates/bevy_app/src/app.rs
+++ b/crates/bevy_app/src/app.rs
@@ -6,6 +6,7 @@ pub use bevy_derive::AppLabel;
 use bevy_ecs::{
     event::{event_update_system, EventCursor},
     intern::Interned,
+    observer::EventData,
     prelude::*,
     schedule::{ScheduleBuildSettings, ScheduleLabel},
     system::{IntoObserverSystem, SystemId},
@@ -1020,7 +1021,7 @@ impl App {
     ///     }
     /// });
     /// ```
-    pub fn observe<E: Event, B: Bundle, M>(
+    pub fn observe<E: EventData, B: Bundle, M>(
         &mut self,
         observer: impl IntoObserverSystem<E, B, M>,
     ) -> &mut Self {
diff --git a/crates/bevy_ecs/src/observer/data.rs b/crates/bevy_ecs/src/observer/data.rs
new file mode 100644
index 0000000000000..f797a24ab89f8
--- /dev/null
+++ b/crates/bevy_ecs/src/observer/data.rs
@@ -0,0 +1,146 @@
+use bevy_ptr::{Ptr, PtrMut};
+
+use crate::component::ComponentId;
+use crate::event::Event;
+use crate::observer::ObserverTrigger;
+use crate::world::World;
+
+/// The event data that an [`Observer`] is triggered with.
+///
+/// The provided implementations of this trait are:
+///
+/// - All [`Event`] types.
+/// - [`DynamicEvent`], which matches any [`Event`]s dynamically added to the observer with [`Observer::with_event`] and does not reify the event data.
+///
+/// # Safety
+///
+/// Implementor must ensure that:
+/// - [`EventData::init_components`] must register a [`ComponentId`] for each [`Event`] type used in the output type.
+///
+/// [`Observer`]: crate::observer::Observer
+/// [`Observer::with_event`]: crate::observer::Observer::with_event
+pub unsafe trait EventData: 'static {
+    /// The item returned by this [`EventData`] that will be passed to the observer system function.
+    /// Most of the time this will be a mutable reference to an [`Event`] type or a [`PtrMut`].
+    type Item<'trigger>;
+    /// The read-only variant of the [`Item`](EventData::Item).
+    type ReadOnlyItem<'trigger>: Copy;
+
+    /// Casts a pointer to the output [`Item`](EventData::Item) type.
+    ///
+    /// # Safety
+    ///
+    /// Caller must ensure that the given `ptr` can be safely converted to the output [`Item`](EventData::Item) type.
+    unsafe fn cast<'trigger>(
+        world: &World,
+        observer_trigger: &ObserverTrigger,
+        ptr: PtrMut<'trigger>,
+    ) -> Self::Item<'trigger>;
+
+    /// Initialize the components required by this event data.
+    fn init_components(world: &mut World, ids: impl FnMut(ComponentId));
+
+    /// Shrink the [`Item`](EventData::Item) to a shorter lifetime.
+    fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short>;
+
+    /// Shrink the [`Item`](EventData::Item) to a shorter lifetime [`ReadOnlyItem`](EventData::ReadOnlyItem).
+    fn shrink_readonly<'long: 'short, 'short>(
+        item: &'short Self::Item<'long>,
+    ) -> Self::ReadOnlyItem<'short>;
+}
+
+// SAFETY: The event type has a component id registered in `init_components`.
+unsafe impl<E: Event> EventData for E {
+    type Item<'trigger> = &'trigger mut E;
+    type ReadOnlyItem<'trigger> = &'trigger E;
+
+    unsafe fn cast<'trigger>(
+        _world: &World,
+        _observer_trigger: &ObserverTrigger,
+        ptr: PtrMut<'trigger>,
+    ) -> Self::Item<'trigger> {
+        // SAFETY: Caller must ensure that ptr can be safely cast to the Item type.
+        unsafe { ptr.deref_mut() }
+    }
+
+    fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
+        let id = world.init_component::<E>();
+        ids(id);
+    }
+
+    fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
+        item
+    }
+
+    fn shrink_readonly<'long: 'short, 'short>(
+        item: &'short Self::Item<'long>,
+    ) -> Self::ReadOnlyItem<'short> {
+        item
+    }
+}
+
+/// [`EventData`] that matches any event type and performs no casting. Instead, it returns the pointer as is.
+/// This is useful for observers that do not need to access the event data, or need to do so dynamically.
+///
+/// # Example
+///
+/// ## Listen to [`OnAdd`] and [`OnRemove`] events in the same observer
+///
+/// ```
+/// # use crate::prelude::*;
+/// #
+/// /// The component type to listen for on add and remove events.
+/// #[derive(Component)]
+/// struct A;
+///
+/// let mut world = World::new();
+///
+/// // Fetch the component ids for the events
+/// let on_add = world.init_component::<OnAdd>();
+/// let on_remove = world.init_component::<OnRemove>();
+///
+/// world.spawn(
+///     Observer::new(|trigger: Trigger<DynamicEvent, A>| {
+///         // This observer function is called for both OnAdd and OnRemove events!
+///         let ptr_mut = trigger.event_mut();
+///         // do something with the PtrMut, if needed
+///     })
+///     // Safely register the component ids for the events to the observer
+///     .with_event(on_add)
+///     .with_event(on_remove),
+/// );
+///
+/// // The observer will be called twice for these two function calls:
+/// let entity = world.spawn(A).id();
+/// world.despawn(entity);
+/// ```
+///
+/// [`OnAdd`]: crate::event::OnAdd
+/// [`OnRemove`]: crate::event::OnRemove
+pub struct DynamicEvent;
+
+// SAFETY: Performs no unsafe operations, returns the pointer as is.
+unsafe impl EventData for DynamicEvent {
+    type Item<'trigger> = PtrMut<'trigger>;
+    type ReadOnlyItem<'trigger> = Ptr<'trigger>;
+
+    unsafe fn cast<'trigger>(
+        _world: &World,
+        _observer_trigger: &ObserverTrigger,
+        ptr: PtrMut<'trigger>,
+    ) -> Self::Item<'trigger> {
+        ptr
+    }
+
+    fn init_components(_world: &mut World, _ids: impl FnMut(ComponentId)) {}
+
+    fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
+        item.reborrow()
+    }
+
+    fn shrink_readonly<'long: 'short, 'short>(
+        item: &'short Self::Item<'long>,
+    ) -> Self::ReadOnlyItem<'short> {
+        item.as_ref()
+    }
+}
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 3ef2d6b28d63d..93a556736193f 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -1,32 +1,33 @@
 //! Types for creating and storing [`Observer`]s
 
+mod data;
 mod entity_observer;
 mod runner;
 mod trigger_event;
 
+pub use data::*;
 pub use runner::*;
 pub use trigger_event::*;
 
 use crate::observer::entity_observer::ObservedBy;
 use crate::{archetype::ArchetypeFlags, system::IntoObserverSystem, world::*};
 use crate::{component::ComponentId, prelude::*, world::DeferredWorld};
-use bevy_ptr::Ptr;
 use bevy_utils::{EntityHashMap, HashMap};
 use std::{fmt::Debug, marker::PhantomData};
 
 /// Type containing triggered [`Event`] information for a given run of an [`Observer`]. This contains the
 /// [`Event`] data itself. If it was triggered for a specific [`Entity`], it includes that as well. It also
 /// contains event propagation information. See [`Trigger::propagate`] for more information.
-pub struct Trigger<'w, E, B: Bundle = ()> {
-    event: &'w mut E,
+pub struct Trigger<'w, E: EventData, B: Bundle = ()> {
+    event: E::Item<'w>,
     propagate: &'w mut bool,
     trigger: ObserverTrigger,
     _marker: PhantomData<B>,
 }
 
-impl<'w, E, B: Bundle> Trigger<'w, E, B> {
+impl<'w, E: EventData, B: Bundle> Trigger<'w, E, B> {
     /// Creates a new trigger for the given event and observer information.
-    pub fn new(event: &'w mut E, propagate: &'w mut bool, trigger: ObserverTrigger) -> Self {
+    pub fn new(event: E::Item<'w>, propagate: &'w mut bool, trigger: ObserverTrigger) -> Self {
         Self {
             event,
             propagate,
@@ -41,18 +42,13 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> {
     }
 
     /// Returns a reference to the triggered event.
-    pub fn event(&self) -> &E {
-        self.event
+    pub fn event(&self) -> E::ReadOnlyItem<'_> {
+        E::shrink_readonly(&self.event)
     }
 
     /// Returns a mutable reference to the triggered event.
-    pub fn event_mut(&mut self) -> &mut E {
-        self.event
-    }
-
-    /// Returns a pointer to the triggered event.
-    pub fn event_ptr(&self) -> Ptr {
-        Ptr::from(&self.event)
+    pub fn event_mut(&mut self) -> E::Item<'_> {
+        E::shrink(&mut self.event)
     }
 
     /// Returns the entity that triggered the observer, could be [`Entity::PLACEHOLDER`].
@@ -84,7 +80,10 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> {
     }
 }
 
-impl<'w, E: Debug, B: Bundle> Debug for Trigger<'w, E, B> {
+impl<'w, E: EventData, B: Bundle> Debug for Trigger<'w, E, B>
+where
+    <E as EventData>::Item<'w>: Debug,
+{
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("Trigger")
             .field("event", &self.event)
@@ -315,7 +314,7 @@ impl Observers {
 
 impl World {
     /// Spawns a "global" [`Observer`] and returns its [`Entity`].
-    pub fn observe<E: Event, B: Bundle, M>(
+    pub fn observe<E: EventData, B: Bundle, M>(
         &mut self,
         system: impl IntoObserverSystem<E, B, M>,
     ) -> EntityWorldMut {
@@ -444,7 +443,7 @@ mod tests {
 
     use crate as bevy_ecs;
     use crate::observer::{
-        EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace,
+        DynamicEvent, EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace,
     };
     use crate::prelude::*;
     use crate::traversal::Traversal;
@@ -612,16 +611,15 @@ mod tests {
     }
 
     #[test]
-    fn observer_multiple_events() {
+    fn observer_event_data_dynamic() {
         let mut world = World::new();
         world.init_resource::<R>();
+        let on_add = world.init_component::<OnAdd>();
         let on_remove = world.init_component::<OnRemove>();
         world.spawn(
-            // SAFETY: OnAdd and OnRemove are both unit types, so this is safe
-            unsafe {
-                Observer::new(|_: Trigger<OnAdd, A>, mut res: ResMut<R>| res.0 += 1)
-                    .with_event(on_remove)
-            },
+            Observer::new(|_: Trigger<DynamicEvent, A>, mut res: ResMut<R>| res.0 += 1)
+                .with_event(on_add)
+                .with_event(on_remove),
         );
 
         let entity = world.spawn(A).id();
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index 1706ead914e0e..050f46f63d59c 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -1,6 +1,6 @@
 use crate::{
     component::{ComponentHooks, ComponentId, StorageType},
-    observer::{ObserverDescriptor, ObserverTrigger},
+    observer::{DynamicEvent, EventData, ObserverDescriptor, ObserverTrigger},
     prelude::*,
     query::DebugCheckedUnwrap,
     system::{IntoObserverSystem, ObserverSystem},
@@ -261,12 +261,12 @@ pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, propagate:
 /// [`With<ObserverState>`].
 ///
 /// [`SystemParam`]: crate::system::SystemParam
-pub struct Observer<T: 'static, B: Bundle> {
-    system: BoxedObserverSystem<T, B>,
+pub struct Observer<E: EventData + 'static, B: Bundle> {
+    system: BoxedObserverSystem<E, B>,
     descriptor: ObserverDescriptor,
 }
 
-impl<E: Event, B: Bundle> Observer<E, B> {
+impl<E: EventData, B: Bundle> Observer<E, B> {
     /// Creates a new [`Observer`], which defaults to a "global" observer. This means it will run whenever the event `E` is triggered
     /// for _any_ entity (or no entity).
     pub fn new<M>(system: impl IntoObserverSystem<E, B, M>) -> Self {
@@ -302,24 +302,40 @@ impl<E: Event, B: Bundle> Observer<E, B> {
     /// # Safety
     /// The type of the `event` [`ComponentId`] _must_ match the actual value
     /// of the event passed into the observer system.
-    pub unsafe fn with_event(mut self, event: ComponentId) -> Self {
+    pub unsafe fn with_event_unchecked(mut self, event: ComponentId) -> Self {
         self.descriptor.events.push(event);
         self
     }
 }
 
-impl<E: Event, B: Bundle> Component for Observer<E, B> {
+impl<B: Bundle> Observer<DynamicEvent, B> {
+    /// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
+    /// is triggered.
+    ///
+    /// # Note
+    /// As opposed to [`Observer::with_event_unchecked`], this method is safe to use because no pointer casting is performed automatically.
+    /// That is left to the user to do manually.
+    pub fn with_event(self, event: ComponentId) -> Self {
+        // SAFETY: DynamicEvent itself does not perform any unsafe operations (like casting), so this is safe.
+        unsafe { self.with_event_unchecked(event) }
+    }
+}
+
+impl<E: EventData, B: Bundle> Component for Observer<E, B> {
     const STORAGE_TYPE: StorageType = StorageType::SparseSet;
     fn register_component_hooks(hooks: &mut ComponentHooks) {
         hooks.on_add(|mut world, entity, _| {
             world.commands().add(move |world: &mut World| {
-                let event_type = world.init_component::<E>();
+                let mut events = Vec::new();
+                E::init_components(world, |id| {
+                    events.push(id);
+                });
                 let mut components = Vec::new();
                 B::component_ids(&mut world.components, &mut world.storages, &mut |id| {
                     components.push(id);
                 });
                 let mut descriptor = ObserverDescriptor {
-                    events: vec![event_type],
+                    events,
                     components,
                     ..Default::default()
                 };
@@ -355,7 +371,7 @@ impl<E: Event, B: Bundle> Component for Observer<E, B> {
 /// Equivalent to [`BoxedSystem`](crate::system::BoxedSystem) for [`ObserverSystem`].
 pub type BoxedObserverSystem<E = (), B = ()> = Box<dyn ObserverSystem<E, B>>;
 
-fn observer_system_runner<E: Event, B: Bundle>(
+fn observer_system_runner<E: EventData, B: Bundle>(
     mut world: DeferredWorld,
     observer_trigger: ObserverTrigger,
     ptr: PtrMut,
@@ -382,12 +398,12 @@ fn observer_system_runner<E: Event, B: Bundle>(
     }
     state.last_trigger_id = last_trigger;
 
-    let trigger: Trigger<E, B> = Trigger::new(
-        // SAFETY: Caller ensures `ptr` is castable to `&mut T`
-        unsafe { ptr.deref_mut() },
-        propagate,
-        observer_trigger,
-    );
+    // SAFETY: We have immutable access to the world from the passed in DeferredWorld
+    let world_ref = unsafe { world.world() };
+    // SAFETY: Observer was triggered with an event type in its list of valid types, so it must be convertible to E
+    let event = unsafe { E::cast(world_ref, &observer_trigger, ptr) };
+
+    let trigger: Trigger<E, B> = Trigger::new(event, propagate, observer_trigger);
     // SAFETY: the static lifetime is encapsulated in Trigger / cannot leak out.
     // Additionally, IntoObserverSystem is only implemented for functions starting
     // with for<'a> Trigger<'a>, meaning users cannot specify Trigger<'static> manually,
diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs
index 14748ac070eb7..9c250d4748a4e 100644
--- a/crates/bevy_ecs/src/system/commands/mod.rs
+++ b/crates/bevy_ecs/src/system/commands/mod.rs
@@ -9,7 +9,7 @@ use crate::{
     component::{ComponentId, ComponentInfo},
     entity::{Entities, Entity},
     event::Event,
-    observer::{Observer, TriggerEvent, TriggerTargets},
+    observer::{EventData, Observer, TriggerEvent, TriggerTargets},
     system::{RunSystemWithInput, SystemId},
     world::{
         command_queue::RawCommandQueue, Command, CommandQueue, EntityWorldMut, FromWorld,
@@ -784,7 +784,7 @@ impl<'w, 's> Commands<'w, 's> {
     }
 
     /// Spawns an [`Observer`] and returns the [`EntityCommands`] associated with the entity that stores the observer.
-    pub fn observe<E: Event, B: Bundle, M>(
+    pub fn observe<E: EventData, B: Bundle, M>(
         &mut self,
         observer: impl IntoObserverSystem<E, B, M>,
     ) -> EntityCommands {
@@ -1257,7 +1257,7 @@ impl EntityCommands<'_> {
     }
 
     /// Creates an [`Observer`] listening for a trigger of type `T` that targets this entity.
-    pub fn observe<E: Event, B: Bundle, M>(
+    pub fn observe<E: EventData, B: Bundle, M>(
         &mut self,
         system: impl IntoObserverSystem<E, B, M>,
     ) -> &mut Self {
@@ -1492,7 +1492,7 @@ fn log_components(entity: Entity, world: &mut World) {
     info!("Entity {entity}: {debug_infos:?}");
 }
 
-fn observe<E: Event, B: Bundle, M>(
+fn observe<E: EventData, B: Bundle, M>(
     observer: impl IntoObserverSystem<E, B, M>,
 ) -> impl EntityCommand {
     move |entity, world: &mut World| {
diff --git a/crates/bevy_ecs/src/system/observer_system.rs b/crates/bevy_ecs/src/system/observer_system.rs
index 28ea902b1aa4d..880b27cb2d0a6 100644
--- a/crates/bevy_ecs/src/system/observer_system.rs
+++ b/crates/bevy_ecs/src/system/observer_system.rs
@@ -1,6 +1,7 @@
 use bevy_utils::all_tuples;
 
 use crate::{
+    observer::EventData,
     prelude::{Bundle, Trigger},
     system::{System, SystemParam, SystemParamFunction, SystemParamItem},
 };
@@ -10,13 +11,13 @@ use super::IntoSystem;
 /// Implemented for systems that have an [`Observer`] as the first argument.
 ///
 /// [`Observer`]: crate::observer::Observer
-pub trait ObserverSystem<E: 'static, B: Bundle, Out = ()>:
+pub trait ObserverSystem<E: EventData + 'static, B: Bundle, Out = ()>:
     System<In = Trigger<'static, E, B>, Out = Out> + Send + 'static
 {
 }
 
 impl<
-        E: 'static,
+        E: EventData + 'static,
         B: Bundle,
         Out,
         T: System<In = Trigger<'static, E, B>, Out = Out> + Send + 'static,
@@ -30,7 +31,9 @@ impl<
     label = "the trait `IntoObserverSystem` is not implemented",
     note = "for function `ObserverSystem`s, ensure the first argument is a `Trigger<T>` and any subsequent ones are `SystemParam`"
 )]
-pub trait IntoObserverSystem<E: 'static, B: Bundle, M, Out = ()>: Send + 'static {
+pub trait IntoObserverSystem<E: EventData + 'static, B: Bundle, M, Out = ()>:
+    Send + 'static
+{
     /// The type of [`System`] that this instance converts into.
     type System: ObserverSystem<E, B, Out>;
 
@@ -42,7 +45,7 @@ impl<
         S: IntoSystem<Trigger<'static, E, B>, Out, M> + Send + 'static,
         M,
         Out,
-        E: 'static,
+        E: EventData + 'static,
         B: Bundle,
     > IntoObserverSystem<E, B, M, Out> for S
 where
@@ -58,7 +61,7 @@ where
 macro_rules! impl_system_function {
     ($($param: ident),*) => {
         #[allow(non_snake_case)]
-        impl<E: 'static, B: Bundle, Out, Func: Send + Sync + 'static, $($param: SystemParam),*> SystemParamFunction<fn(Trigger<E, B>, $($param,)*)> for Func
+        impl<E: EventData + 'static, B: Bundle, Out, Func: Send + Sync + 'static, $($param: SystemParam),*> SystemParamFunction<fn(Trigger<E, B>, $($param,)*)> for Func
         where
         for <'a> &'a mut Func:
                 FnMut(Trigger<E, B>, $($param),*) -> Out +
@@ -70,7 +73,7 @@ macro_rules! impl_system_function {
             #[inline]
             fn run(&mut self, input: Trigger<'static, E, B>, param_value: SystemParamItem< ($($param,)*)>) -> Out {
                 #[allow(clippy::too_many_arguments)]
-                fn call_inner<E: 'static, B: Bundle, Out, $($param,)*>(
+                fn call_inner<E: EventData + 'static, B: Bundle, Out, $($param,)*>(
                     mut f: impl FnMut(Trigger<'static, E, B>, $($param,)*) -> Out,
                     input: Trigger<'static, E, B>,
                     $($param: $param,)*
diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs
index 0aa28241557e7..53d326b37b2e7 100644
--- a/crates/bevy_ecs/src/world/entity_ref.rs
+++ b/crates/bevy_ecs/src/world/entity_ref.rs
@@ -5,7 +5,7 @@ use crate::{
     component::{Component, ComponentId, ComponentTicks, Components, StorageType},
     entity::{Entities, Entity, EntityLocation},
     event::Event,
-    observer::{Observer, Observers},
+    observer::{EventData, Observer, Observers},
     query::Access,
     removal_detection::RemovedComponentEvents,
     storage::Storages,
@@ -1467,7 +1467,7 @@ impl<'w> EntityWorldMut<'w> {
 
     /// Creates an [`Observer`] listening for events of type `E` targeting this entity.
     /// In order to trigger the callback the entity must also match the query when the event is fired.
-    pub fn observe<E: Event, B: Bundle, M>(
+    pub fn observe<E: EventData, B: Bundle, M>(
         &mut self,
         observer: impl IntoObserverSystem<E, B, M>,
     ) -> &mut Self {

From 1286e94ce0f5babdd8f23672ea923fa44fcec420 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 25 Aug 2024 20:10:55 -0500
Subject: [PATCH 2/7] remove old benchmark definitions from other pr

---
 benches/benches/bevy_ecs/observers/mod.rs | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/benches/benches/bevy_ecs/observers/mod.rs b/benches/benches/bevy_ecs/observers/mod.rs
index ceab56c99e1fb..228ac1265f024 100644
--- a/benches/benches/bevy_ecs/observers/mod.rs
+++ b/benches/benches/bevy_ecs/observers/mod.rs
@@ -1,21 +1,15 @@
 use criterion::criterion_group;
 
 mod dynamic;
-mod multievent;
 mod propagation;
-mod semidynamic;
 mod simple;
 use dynamic::*;
-use multievent::*;
 use propagation::*;
-use semidynamic::*;
 use simple::*;
 
 criterion_group!(
     observer_benches,
     event_propagation,
     observe_simple,
-    observe_multievent,
     observe_dynamic,
-    observe_semidynamic
 );

From bb330e08b58b82efe5fff8c78d9dea15f9dbc3aa Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 25 Aug 2024 21:02:08 -0500
Subject: [PATCH 3/7] add missing derive macro import

---
 crates/bevy_ecs/src/observer/data.rs | 40 ++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/crates/bevy_ecs/src/observer/data.rs b/crates/bevy_ecs/src/observer/data.rs
index f797a24ab89f8..c5e399419ada8 100644
--- a/crates/bevy_ecs/src/observer/data.rs
+++ b/crates/bevy_ecs/src/observer/data.rs
@@ -88,6 +88,7 @@ unsafe impl<E: Event> EventData for E {
 ///
 /// ```
 /// # use crate::prelude::*;
+/// # use bevy_ecs_macros::Component;
 /// #
 /// /// The component type to listen for on add and remove events.
 /// #[derive(Component)]
@@ -144,3 +145,42 @@ unsafe impl EventData for DynamicEvent {
         item.as_ref()
     }
 }
+
+pub struct ReflectEvent;
+
+unsafe impl EventData for ReflectEvent {
+    type Item<'trigger> = &'trigger mut dyn bevy_reflect::Reflect;
+    type ReadOnlyItem<'trigger> = &'trigger dyn bevy_reflect::Reflect;
+
+    unsafe fn cast<'trigger>(
+        world: &World,
+        observer_trigger: &ObserverTrigger,
+        ptr: PtrMut<'trigger>,
+    ) -> Self::Item<'trigger> {
+        let type_id = world
+            .components()
+            .get_info(observer_trigger.event_type)
+            .unwrap()
+            .type_id()
+            .unwrap();
+        let type_registry = world.resource::<crate::reflect::AppTypeRegistry>().read();
+        let reflect_from_ptr = type_registry
+            .get_type_data::<bevy_reflect::ReflectFromPtr>(type_id)
+            .unwrap();
+
+        // SAFETY: The ReflectFromPtr data was fetched based on the observed event type's type id.
+        unsafe { reflect_from_ptr.as_reflect_mut(ptr) }
+    }
+
+    fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {}
+
+    fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
+        *item
+    }
+
+    fn shrink_readonly<'long: 'short, 'short>(
+        item: &'short Self::Item<'long>,
+    ) -> Self::ReadOnlyItem<'short> {
+        *item
+    }
+}

From 3ebfeb862807062dd07ae034614f8ada8a67fed1 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 25 Aug 2024 21:11:46 -0500
Subject: [PATCH 4/7] Partially revert "add missing derive macro import"

This reverts commit bb330e08b58b82efe5fff8c78d9dea15f9dbc3aa.
---
 crates/bevy_ecs/src/observer/data.rs | 39 ----------------------------
 1 file changed, 39 deletions(-)

diff --git a/crates/bevy_ecs/src/observer/data.rs b/crates/bevy_ecs/src/observer/data.rs
index c5e399419ada8..0bd1714b16e6d 100644
--- a/crates/bevy_ecs/src/observer/data.rs
+++ b/crates/bevy_ecs/src/observer/data.rs
@@ -145,42 +145,3 @@ unsafe impl EventData for DynamicEvent {
         item.as_ref()
     }
 }
-
-pub struct ReflectEvent;
-
-unsafe impl EventData for ReflectEvent {
-    type Item<'trigger> = &'trigger mut dyn bevy_reflect::Reflect;
-    type ReadOnlyItem<'trigger> = &'trigger dyn bevy_reflect::Reflect;
-
-    unsafe fn cast<'trigger>(
-        world: &World,
-        observer_trigger: &ObserverTrigger,
-        ptr: PtrMut<'trigger>,
-    ) -> Self::Item<'trigger> {
-        let type_id = world
-            .components()
-            .get_info(observer_trigger.event_type)
-            .unwrap()
-            .type_id()
-            .unwrap();
-        let type_registry = world.resource::<crate::reflect::AppTypeRegistry>().read();
-        let reflect_from_ptr = type_registry
-            .get_type_data::<bevy_reflect::ReflectFromPtr>(type_id)
-            .unwrap();
-
-        // SAFETY: The ReflectFromPtr data was fetched based on the observed event type's type id.
-        unsafe { reflect_from_ptr.as_reflect_mut(ptr) }
-    }
-
-    fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {}
-
-    fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
-        *item
-    }
-
-    fn shrink_readonly<'long: 'short, 'short>(
-        item: &'short Self::Item<'long>,
-    ) -> Self::ReadOnlyItem<'short> {
-        *item
-    }
-}

From fd3706c07be386fc4c8cd25d906fac698980d094 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 25 Aug 2024 21:15:35 -0500
Subject: [PATCH 5/7] fix docstest import

---
 crates/bevy_ecs/src/observer/data.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/bevy_ecs/src/observer/data.rs b/crates/bevy_ecs/src/observer/data.rs
index 0bd1714b16e6d..8568f9917af83 100644
--- a/crates/bevy_ecs/src/observer/data.rs
+++ b/crates/bevy_ecs/src/observer/data.rs
@@ -87,7 +87,7 @@ unsafe impl<E: Event> EventData for E {
 /// ## Listen to [`OnAdd`] and [`OnRemove`] events in the same observer
 ///
 /// ```
-/// # use crate::prelude::*;
+/// # use bevy_ecs::prelude::*;
 /// # use bevy_ecs_macros::Component;
 /// #
 /// /// The component type to listen for on add and remove events.

From 08b8196f29a4f58d559c9351f9688fc46d1ad5a1 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 25 Aug 2024 21:47:43 -0500
Subject: [PATCH 6/7] fix docs and add doctest import

---
 crates/bevy_ecs/src/observer/data.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/crates/bevy_ecs/src/observer/data.rs b/crates/bevy_ecs/src/observer/data.rs
index 8568f9917af83..4854c64dfa1aa 100644
--- a/crates/bevy_ecs/src/observer/data.rs
+++ b/crates/bevy_ecs/src/observer/data.rs
@@ -6,11 +6,12 @@ use crate::observer::ObserverTrigger;
 use crate::world::World;
 
 /// The event data that an [`Observer`] is triggered with.
+/// Ordinarily, this will be a mutable reference to an [`Event`] type.
 ///
 /// The provided implementations of this trait are:
 ///
 /// - All [`Event`] types.
-/// - [`DynamicEvent`], which matches any [`Event`]s dynamically added to the observer with [`Observer::with_event`] and does not reify the event data.
+/// - [`DynamicEvent`].
 ///
 /// # Safety
 ///
@@ -89,6 +90,7 @@ unsafe impl<E: Event> EventData for E {
 /// ```
 /// # use bevy_ecs::prelude::*;
 /// # use bevy_ecs_macros::Component;
+/// # use bevy_ecs::observer::DynamicEvent;
 /// #
 /// /// The component type to listen for on add and remove events.
 /// #[derive(Component)]

From 74f9f11bbc53edb0d741a912eabde68168795d6a Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 25 Aug 2024 22:00:31 -0500
Subject: [PATCH 7/7] fix doctest

---
 crates/bevy_ecs/src/observer/data.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/bevy_ecs/src/observer/data.rs b/crates/bevy_ecs/src/observer/data.rs
index 4854c64dfa1aa..4734d2e104a36 100644
--- a/crates/bevy_ecs/src/observer/data.rs
+++ b/crates/bevy_ecs/src/observer/data.rs
@@ -103,7 +103,7 @@ unsafe impl<E: Event> EventData for E {
 /// let on_remove = world.init_component::<OnRemove>();
 ///
 /// world.spawn(
-///     Observer::new(|trigger: Trigger<DynamicEvent, A>| {
+///     Observer::new(|mut trigger: Trigger<DynamicEvent, A>| {
 ///         // This observer function is called for both OnAdd and OnRemove events!
 ///         let ptr_mut = trigger.event_mut();
 ///         // do something with the PtrMut, if needed