From e2562d83bdc3631ac797d8cbd909ba5bdf4c8c95 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Thu, 8 Aug 2024 22:07:48 -0500
Subject: [PATCH 01/34] initial implementation of multi event observers
---
crates/bevy_app/src/app.rs | 3 +-
crates/bevy_ecs/src/observer/mod.rs | 37 +++-
crates/bevy_ecs/src/observer/runner.rs | 31 +--
crates/bevy_ecs/src/observer/set.rs | 197 ++++++++++++++++++
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 +-
7 files changed, 258 insertions(+), 37 deletions(-)
create mode 100644 crates/bevy_ecs/src/observer/set.rs
diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs
index 7188102837dea..2d03deff583a9 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::EventSet,
prelude::*,
schedule::{ScheduleBuildSettings, ScheduleLabel},
system::{IntoObserverSystem, SystemId},
@@ -990,7 +991,7 @@ impl App {
}
/// Spawns an [`Observer`] entity, which will watch for and respond to the given event.
- pub fn observe<E: Event, B: Bundle, M>(
+ pub fn observe<E: EventSet, B: Bundle, M>(
&mut self,
observer: impl IntoObserverSystem<E, B, M>,
) -> &mut Self {
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 600384d478438..33779991101b1 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -2,9 +2,11 @@
mod entity_observer;
mod runner;
+mod set;
mod trigger_event;
pub use runner::*;
+pub use set::*;
pub use trigger_event::*;
use crate::observer::entity_observer::ObservedBy;
@@ -17,16 +19,16 @@ use std::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: EventSet, B: Bundle = ()> {
+ event: E::Out<'w>,
propagate: &'w mut bool,
trigger: ObserverTrigger,
_marker: PhantomData<B>,
}
-impl<'w, E, B: Bundle> Trigger<'w, E, B> {
+impl<'w, E: EventSet, 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::Out<'w>, propagate: &'w mut bool, trigger: ObserverTrigger) -> Self {
Self {
event,
propagate,
@@ -41,18 +43,18 @@ 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::OutReadonly<'_> {
+ todo!()
}
/// Returns a mutable reference to the triggered event.
- pub fn event_mut(&mut self) -> &mut E {
- self.event
+ pub fn event_mut(&mut self) -> E::Out<'_> {
+ todo!()
}
/// Returns a pointer to the triggered event.
pub fn event_ptr(&self) -> Ptr {
- Ptr::from(&self.event)
+ todo!()
}
/// Returns the entity that triggered the observer, could be [`Entity::PLACEHOLDER`].
@@ -304,7 +306,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: EventSet, B: Bundle, M>(
&mut self,
system: impl IntoObserverSystem<E, B, M>,
) -> EntityWorldMut {
@@ -602,6 +604,21 @@ mod tests {
#[test]
fn observer_multiple_events() {
+ let mut world = World::new();
+ world.init_resource::<R>();
+ #[derive(Event)]
+ struct Foo;
+ #[derive(Event)]
+ struct Bar;
+ world.observe(|_: Trigger<(Foo, Bar)>, mut res: ResMut<R>| res.0 += 1);
+ world.flush();
+ world.trigger(Foo);
+ world.trigger(Bar);
+ assert_eq!(2, world.resource::<R>().0);
+ }
+
+ #[test]
+ fn observer_multiple_events_unsafe() {
let mut world = World::new();
world.init_resource::<R>();
let on_remove = world.init_component::<OnRemove>();
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index 05afa4f614f76..7ef5362a1cd40 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::{EventSet, ObserverDescriptor, ObserverTrigger},
prelude::*,
query::DebugCheckedUnwrap,
system::{IntoObserverSystem, ObserverSystem},
@@ -260,12 +260,12 @@ pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, propagate:
/// serves as the "source of truth" of the observer.
///
/// [`SystemParam`]: crate::system::SystemParam
-pub struct Observer<T: 'static, B: Bundle> {
- system: BoxedObserverSystem<T, B>,
+pub struct Observer<E: EventSet + 'static, B: Bundle> {
+ system: BoxedObserverSystem<E, B>,
descriptor: ObserverDescriptor,
}
-impl<E: Event, B: Bundle> Observer<E, B> {
+impl<E: EventSet, 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 {
@@ -307,18 +307,21 @@ impl<E: Event, B: Bundle> Observer<E, B> {
}
}
-impl<E: Event, B: Bundle> Component for Observer<E, B> {
+impl<E: EventSet, 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()
};
@@ -354,7 +357,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: EventSet, B: Bundle>(
mut world: DeferredWorld,
observer_trigger: ObserverTrigger,
ptr: PtrMut,
@@ -381,12 +384,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 Some(event) = E::checked_cast(unsafe { world.world() }, &observer_trigger, ptr) else {
+ return;
+ };
+
+ 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/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
new file mode 100644
index 0000000000000..6ac546e0a8538
--- /dev/null
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -0,0 +1,197 @@
+use bevy_ptr::PtrMut;
+
+use crate::component::ComponentId;
+use crate::event::Event;
+use crate::observer::ObserverTrigger;
+use crate::world::World;
+
+/// A set of events that can trigger an observer.
+///
+/// # Safety
+///
+/// Implementor must ensure that [`checked_cast`] and [`init_components`] obey the following:
+/// - Each event type must have a component id registered in [`init_components`].
+/// - [`checked_cast`] must check that the component id matches the event type in order to safely cast a pointer to the output type.
+///
+pub unsafe trait EventSet: 'static {
+ /// The output type that will be passed to the observer.
+ type Out<'trigger>;
+ /// The read-only variant of the output type.
+ type OutReadonly<'trigger>;
+
+ /// Safely casts the pointer to the output type, or a variant of it.
+ /// Returns `None` if the event type does not match.
+ fn checked_cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Option<Self::Out<'trigger>>;
+
+ /// Initialize the components required by the event set.
+ fn init_components(world: &mut World, ids: impl FnMut(ComponentId));
+}
+
+unsafe impl<A: Event> EventSet for A {
+ type Out<'trigger> = &'trigger mut A;
+ type OutReadonly<'trigger> = &'trigger A;
+
+ fn checked_cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Option<Self::Out<'trigger>> {
+ <(A,) as EventSet>::checked_cast(world, observer_trigger, ptr)
+ }
+
+ fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
+ <(A,) as EventSet>::init_components(world, ids)
+ }
+}
+
+unsafe impl<A: Event> EventSet for (A,) {
+ type Out<'trigger> = &'trigger mut A;
+ type OutReadonly<'trigger> = &'trigger A;
+
+ fn checked_cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Option<Self::Out<'trigger>> {
+ let Some(a_id) = world.component_id::<A>() else {
+ return None;
+ };
+
+ if a_id == observer_trigger.event_type {
+ // SAFETY: We just checked that the component id matches the event type
+ let a = unsafe { ptr.deref_mut() };
+ Some(a)
+ } else {
+ None
+ }
+ }
+
+ fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
+ let a_id = world.init_component::<A>();
+ ids(a_id);
+ }
+}
+
+/// The output type of an observer that observes two different event types.
+pub enum Or2<A, B> {
+ /// The first event type.
+ A(A),
+ /// The second event type.
+ B(B),
+}
+
+impl<'a, A, B> From<&'a Or2<&'a mut A, &'a mut B>> for Or2<&'a A, &'a B> {
+ fn from(or: &'a Or2<&'a mut A, &'a mut B>) -> Self {
+ match or {
+ Or2::A(a) => Or2::A(a),
+ Or2::B(b) => Or2::B(b),
+ }
+ }
+}
+
+unsafe impl<A: Event, B: Event> EventSet for (A, B) {
+ type Out<'trigger> = Or2<&'trigger mut A, &'trigger mut B>;
+ type OutReadonly<'trigger> = Or2<&'trigger A, &'trigger B>;
+
+ fn checked_cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Option<Self::Out<'trigger>> {
+ let Some(a_id) = world.component_id::<A>() else {
+ return None;
+ };
+ let Some(b_id) = world.component_id::<B>() else {
+ return None;
+ };
+
+ if a_id == observer_trigger.event_type {
+ // SAFETY: We just checked that the component id matches the event type
+ let a = unsafe { ptr.deref_mut() };
+ Some(Or2::A(a))
+ } else if b_id == observer_trigger.event_type {
+ // SAFETY: We just checked that the component id matches the event type
+ let b = unsafe { ptr.deref_mut() };
+ Some(Or2::B(b))
+ } else {
+ None
+ }
+ }
+
+ fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
+ let a_id = world.init_component::<A>();
+ let b_id = world.init_component::<B>();
+ ids(a_id);
+ ids(b_id);
+ }
+}
+
+/// The output type of an observer that observes three different event types.
+pub enum Or3<A, B, C> {
+ /// The first event type.
+ A(A),
+ /// The second event type.
+ B(B),
+ /// The third event type.
+ C(C),
+}
+
+impl<'a, A, B, C> From<&'a Or3<&'a mut A, &'a mut B, &'a mut C>> for Or3<&'a A, &'a B, &'a C> {
+ fn from(or: &'a Or3<&'a mut A, &'a mut B, &'a mut C>) -> Self {
+ match or {
+ Or3::A(a) => Or3::A(a),
+ Or3::B(b) => Or3::B(b),
+ Or3::C(c) => Or3::C(c),
+ }
+ }
+}
+
+unsafe impl<A: Event, B: Event, C: Event> EventSet for (A, B, C) {
+ type Out<'trigger> = Or3<&'trigger mut A, &'trigger mut B, &'trigger mut C>;
+ type OutReadonly<'trigger> = Or3<&'trigger A, &'trigger B, &'trigger C>;
+
+ fn checked_cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Option<Self::Out<'trigger>> {
+ let Some(a_id) = world.component_id::<A>() else {
+ return None;
+ };
+ let Some(b_id) = world.component_id::<B>() else {
+ return None;
+ };
+ let Some(c_id) = world.component_id::<C>() else {
+ return None;
+ };
+
+ if a_id == observer_trigger.event_type {
+ // SAFETY: We just checked that the component id matches the event type
+ let a = unsafe { ptr.deref_mut() };
+ Some(Or3::A(a))
+ } else if b_id == observer_trigger.event_type {
+ // SAFETY: We just checked that the component id matches the event type
+ let b = unsafe { ptr.deref_mut() };
+ Some(Or3::B(b))
+ } else if c_id == observer_trigger.event_type {
+ // SAFETY: We just checked that the component id matches the event type
+ let c = unsafe { ptr.deref_mut() };
+ Some(Or3::C(c))
+ } else {
+ None
+ }
+ }
+
+ fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
+ let a_id = world.init_component::<A>();
+ let b_id = world.init_component::<B>();
+ let c_id = world.init_component::<C>();
+ ids(a_id);
+ ids(b_id);
+ ids(c_id);
+ }
+}
diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs
index 875e0ae9de8c6..5a27030426ba0 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::{EventSet, Observer, TriggerEvent, TriggerTargets},
system::{RunSystemWithInput, SystemId},
world::{
command_queue::RawCommandQueue, Command, CommandQueue, EntityWorldMut, FromWorld,
@@ -778,7 +778,7 @@ impl<'w, 's> Commands<'w, 's> {
}
/// Spawn 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: EventSet, B: Bundle, M>(
&mut self,
observer: impl IntoObserverSystem<E, B, M>,
) -> EntityCommands {
@@ -1211,7 +1211,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: EventSet, B: Bundle, M>(
&mut self,
system: impl IntoObserverSystem<E, B, M>,
) -> &mut Self {
@@ -1443,7 +1443,7 @@ fn log_components(entity: Entity, world: &mut World) {
info!("Entity {entity}: {debug_infos:?}");
}
-fn observe<E: Event, B: Bundle, M>(
+fn observe<E: EventSet, 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 c5a04f25dd4eb..eae0d663ff2fc 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::EventSet,
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: EventSet + 'static, B: Bundle, Out = ()>:
System<In = Trigger<'static, E, B>, Out = Out> + Send + 'static
{
}
impl<
- E: 'static,
+ E: EventSet + 'static,
B: Bundle,
Out,
T: System<In = Trigger<'static, E, B>, Out = Out> + Send + 'static,
@@ -25,7 +26,9 @@ impl<
}
/// Implemented for systems that convert into [`ObserverSystem`].
-pub trait IntoObserverSystem<E: 'static, B: Bundle, M, Out = ()>: Send + 'static {
+pub trait IntoObserverSystem<E: EventSet + 'static, B: Bundle, M, Out = ()>:
+ Send + 'static
+{
/// The type of [`System`] that this instance converts into.
type System: ObserverSystem<E, B, Out>;
@@ -37,7 +40,7 @@ impl<
S: IntoSystem<Trigger<'static, E, B>, Out, M> + Send + 'static,
M,
Out,
- E: 'static,
+ E: EventSet + 'static,
B: Bundle,
> IntoObserverSystem<E, B, M, Out> for S
where
@@ -53,7 +56,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: EventSet + '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 +
@@ -65,7 +68,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: EventSet + '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 34dc767341ba6..2045ff011bf8e 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::{EventSet, Observer, Observers},
query::Access,
removal_detection::RemovedComponentEvents,
storage::Storages,
@@ -1445,7 +1445,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: EventSet, B: Bundle, M>(
&mut self,
observer: impl IntoObserverSystem<E, B, M>,
) -> &mut Self {
From 41345f0217c6afcceac8cfa38863bfe9dd8ae295 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Thu, 8 Aug 2024 22:58:19 -0500
Subject: [PATCH 02/34] fix some clippy lints
---
crates/bevy_ecs/src/observer/set.rs | 52 ++++++++---------------------
1 file changed, 14 insertions(+), 38 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 6ac546e0a8538..0b01806342c54 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -31,6 +31,7 @@ pub unsafe trait EventSet: 'static {
fn init_components(world: &mut World, ids: impl FnMut(ComponentId));
}
+// SAFETY: Forwards its implementation to `(A,)`, which is itself safe.
unsafe impl<A: Event> EventSet for A {
type Out<'trigger> = &'trigger mut A;
type OutReadonly<'trigger> = &'trigger A;
@@ -44,10 +45,12 @@ unsafe impl<A: Event> EventSet for A {
}
fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
- <(A,) as EventSet>::init_components(world, ids)
+ <(A,) as EventSet>::init_components(world, ids);
}
}
+// SAFETY: All event types have a component id registered in `init_components`,
+// and `checked_cast` checks that the component id matches the event type.
unsafe impl<A: Event> EventSet for (A,) {
type Out<'trigger> = &'trigger mut A;
type OutReadonly<'trigger> = &'trigger A;
@@ -57,9 +60,7 @@ unsafe impl<A: Event> EventSet for (A,) {
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Out<'trigger>> {
- let Some(a_id) = world.component_id::<A>() else {
- return None;
- };
+ let a_id = world.component_id::<A>()?;
if a_id == observer_trigger.event_type {
// SAFETY: We just checked that the component id matches the event type
@@ -84,15 +85,8 @@ pub enum Or2<A, B> {
B(B),
}
-impl<'a, A, B> From<&'a Or2<&'a mut A, &'a mut B>> for Or2<&'a A, &'a B> {
- fn from(or: &'a Or2<&'a mut A, &'a mut B>) -> Self {
- match or {
- Or2::A(a) => Or2::A(a),
- Or2::B(b) => Or2::B(b),
- }
- }
-}
-
+// SAFETY: All event types have a component id registered in `init_components`,
+// and `checked_cast` checks that the component id matches one of the event types before casting.
unsafe impl<A: Event, B: Event> EventSet for (A, B) {
type Out<'trigger> = Or2<&'trigger mut A, &'trigger mut B>;
type OutReadonly<'trigger> = Or2<&'trigger A, &'trigger B>;
@@ -102,12 +96,8 @@ unsafe impl<A: Event, B: Event> EventSet for (A, B) {
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Out<'trigger>> {
- let Some(a_id) = world.component_id::<A>() else {
- return None;
- };
- let Some(b_id) = world.component_id::<B>() else {
- return None;
- };
+ let a_id = world.component_id::<A>()?;
+ let b_id = world.component_id::<B>()?;
if a_id == observer_trigger.event_type {
// SAFETY: We just checked that the component id matches the event type
@@ -140,16 +130,8 @@ pub enum Or3<A, B, C> {
C(C),
}
-impl<'a, A, B, C> From<&'a Or3<&'a mut A, &'a mut B, &'a mut C>> for Or3<&'a A, &'a B, &'a C> {
- fn from(or: &'a Or3<&'a mut A, &'a mut B, &'a mut C>) -> Self {
- match or {
- Or3::A(a) => Or3::A(a),
- Or3::B(b) => Or3::B(b),
- Or3::C(c) => Or3::C(c),
- }
- }
-}
-
+// SAFETY: All event types have a component id registered in `init_components`,
+// and `checked_cast` checks that the component id matches one of the event types before casting.
unsafe impl<A: Event, B: Event, C: Event> EventSet for (A, B, C) {
type Out<'trigger> = Or3<&'trigger mut A, &'trigger mut B, &'trigger mut C>;
type OutReadonly<'trigger> = Or3<&'trigger A, &'trigger B, &'trigger C>;
@@ -159,15 +141,9 @@ unsafe impl<A: Event, B: Event, C: Event> EventSet for (A, B, C) {
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Out<'trigger>> {
- let Some(a_id) = world.component_id::<A>() else {
- return None;
- };
- let Some(b_id) = world.component_id::<B>() else {
- return None;
- };
- let Some(c_id) = world.component_id::<C>() else {
- return None;
- };
+ let a_id = world.component_id::<A>()?;
+ let b_id = world.component_id::<B>()?;
+ let c_id = world.component_id::<C>()?;
if a_id == observer_trigger.event_type {
// SAFETY: We just checked that the component id matches the event type
From 8886f7cd4dd8ebd21a8340532f02067d79843674 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Thu, 8 Aug 2024 23:31:54 -0500
Subject: [PATCH 03/34] make the old multiple event test work again
---
crates/bevy_ecs/src/observer/mod.rs | 2 +-
crates/bevy_ecs/src/observer/runner.rs | 4 +++-
crates/bevy_ecs/src/observer/set.rs | 25 +++++++++----------------
3 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 33779991101b1..f65525e8fcfb8 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -611,7 +611,7 @@ mod tests {
#[derive(Event)]
struct Bar;
world.observe(|_: Trigger<(Foo, Bar)>, mut res: ResMut<R>| res.0 += 1);
- world.flush();
+ world.flush(); // TODO: should we auto-flush after observe?
world.trigger(Foo);
world.trigger(Bar);
assert_eq!(2, world.resource::<R>().0);
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index 7ef5362a1cd40..8098d6670a1a3 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -385,7 +385,9 @@ fn observer_system_runner<E: EventSet, B: Bundle>(
state.last_trigger_id = last_trigger;
// SAFETY: We have immutable access to the world from the passed in DeferredWorld
- let Some(event) = E::checked_cast(unsafe { world.world() }, &observer_trigger, ptr) else {
+ let world_ref = unsafe { world.world() };
+ // SAFETY: Caller ensures `ptr` is castable to `&mut T`, or the function will check it itself. TODO: can we revert this to a safe function?
+ let Some(event) = (unsafe { E::checked_cast(world_ref, &observer_trigger, ptr) }) else {
return;
};
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 0b01806342c54..d2ea43ff36876 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -21,7 +21,7 @@ pub unsafe trait EventSet: 'static {
/// Safely casts the pointer to the output type, or a variant of it.
/// Returns `None` if the event type does not match.
- fn checked_cast<'trigger>(
+ unsafe fn checked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
@@ -36,7 +36,7 @@ unsafe impl<A: Event> EventSet for A {
type Out<'trigger> = &'trigger mut A;
type OutReadonly<'trigger> = &'trigger A;
- fn checked_cast<'trigger>(
+ unsafe fn checked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
@@ -55,20 +55,13 @@ unsafe impl<A: Event> EventSet for (A,) {
type Out<'trigger> = &'trigger mut A;
type OutReadonly<'trigger> = &'trigger A;
- fn checked_cast<'trigger>(
- world: &World,
- observer_trigger: &ObserverTrigger,
+ unsafe fn checked_cast<'trigger>(
+ _world: &World,
+ _observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Out<'trigger>> {
- let a_id = world.component_id::<A>()?;
-
- if a_id == observer_trigger.event_type {
- // SAFETY: We just checked that the component id matches the event type
- let a = unsafe { ptr.deref_mut() };
- Some(a)
- } else {
- None
- }
+ // SAFETY: Caller must ensure that the component id matches the event type
+ Some(unsafe { ptr.deref_mut() })
}
fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
@@ -91,7 +84,7 @@ unsafe impl<A: Event, B: Event> EventSet for (A, B) {
type Out<'trigger> = Or2<&'trigger mut A, &'trigger mut B>;
type OutReadonly<'trigger> = Or2<&'trigger A, &'trigger B>;
- fn checked_cast<'trigger>(
+ unsafe fn checked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
@@ -136,7 +129,7 @@ unsafe impl<A: Event, B: Event, C: Event> EventSet for (A, B, C) {
type Out<'trigger> = Or3<&'trigger mut A, &'trigger mut B, &'trigger mut C>;
type OutReadonly<'trigger> = Or3<&'trigger A, &'trigger B, &'trigger C>;
- fn checked_cast<'trigger>(
+ unsafe fn checked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
From ebc3207336ef8f5aa4df9b1002bf0711f6d34935 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Fri, 9 Aug 2024 01:20:34 -0500
Subject: [PATCH 04/34] reimplement Trigger::event, Trigger::event_mut,
Trigger::event_ptr
---
crates/bevy_ecs/src/observer/mod.rs | 32 ++++---
crates/bevy_ecs/src/observer/set.rs | 120 ++++++++++++++++++++----
crates/bevy_ecs/src/world/entity_ref.rs | 1 -
3 files changed, 123 insertions(+), 30 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index f65525e8fcfb8..67046afc4fe8c 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -20,7 +20,7 @@ use std::marker::PhantomData;
/// [`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: EventSet, B: Bundle = ()> {
- event: E::Out<'w>,
+ event: E::Item<'w>,
propagate: &'w mut bool,
trigger: ObserverTrigger,
_marker: PhantomData<B>,
@@ -28,7 +28,7 @@ pub struct Trigger<'w, E: EventSet, B: Bundle = ()> {
impl<'w, E: EventSet, B: Bundle> Trigger<'w, E, B> {
/// Creates a new trigger for the given event and observer information.
- pub fn new(event: E::Out<'w>, propagate: &'w mut bool, trigger: ObserverTrigger) -> Self {
+ pub fn new(event: E::Item<'w>, propagate: &'w mut bool, trigger: ObserverTrigger) -> Self {
Self {
event,
propagate,
@@ -43,18 +43,18 @@ impl<'w, E: EventSet, B: Bundle> Trigger<'w, E, B> {
}
/// Returns a reference to the triggered event.
- pub fn event(&self) -> E::OutReadonly<'_> {
- todo!()
+ 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) -> E::Out<'_> {
- todo!()
+ pub fn event_mut(&mut self) -> E::Item<'_> {
+ E::shrink(&mut self.event)
}
/// Returns a pointer to the triggered event.
pub fn event_ptr(&self) -> Ptr {
- todo!()
+ E::shrink_ptr(&self.event)
}
/// Returns the entity that triggered the observer, could be [`Entity::PLACEHOLDER`].
@@ -435,7 +435,7 @@ mod tests {
use crate as bevy_ecs;
use crate::observer::{
- EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace,
+ EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace, Or2,
};
use crate::prelude::*;
use crate::traversal::Traversal;
@@ -607,13 +607,19 @@ mod tests {
let mut world = World::new();
world.init_resource::<R>();
#[derive(Event)]
- struct Foo;
+ struct Foo(i32);
#[derive(Event)]
- struct Bar;
- world.observe(|_: Trigger<(Foo, Bar)>, mut res: ResMut<R>| res.0 += 1);
+ struct Bar(bool);
+ world.observe(|t: Trigger<(Foo, Bar)>, mut res: ResMut<R>| {
+ res.0 += 1;
+ match t.event() {
+ Or2::A(Foo(v)) => assert_eq!(5, *v),
+ Or2::B(Bar(v)) => assert_eq!(true, *v),
+ }
+ });
world.flush(); // TODO: should we auto-flush after observe?
- world.trigger(Foo);
- world.trigger(Bar);
+ world.trigger(Foo(5));
+ world.trigger(Bar(true));
assert_eq!(2, world.resource::<R>().0);
}
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index d2ea43ff36876..9a1a9ac096414 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -1,4 +1,4 @@
-use bevy_ptr::PtrMut;
+use bevy_ptr::{Ptr, PtrMut};
use crate::component::ComponentId;
use crate::event::Event;
@@ -15,9 +15,9 @@ use crate::world::World;
///
pub unsafe trait EventSet: 'static {
/// The output type that will be passed to the observer.
- type Out<'trigger>;
+ type Item<'trigger>;
/// The read-only variant of the output type.
- type OutReadonly<'trigger>;
+ type ReadOnlyItem<'trigger>;
/// Safely casts the pointer to the output type, or a variant of it.
/// Returns `None` if the event type does not match.
@@ -25,41 +25,66 @@ pub unsafe trait EventSet: 'static {
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
- ) -> Option<Self::Out<'trigger>>;
+ ) -> Option<Self::Item<'trigger>>;
/// Initialize the components required by the event set.
fn init_components(world: &mut World, ids: impl FnMut(ComponentId));
+
+ /// Shrink the item to a shorter lifetime.
+ fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short>;
+
+ /// Shrink the item to a shorter lifetime, read-only variant.
+ fn shrink_readonly<'long: 'short, 'short>(
+ item: &'short Self::Item<'long>,
+ ) -> Self::ReadOnlyItem<'short>;
+
+ /// Shrink the item to a shorter lifetime, as a type-erased pointer.
+ fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short>;
}
// SAFETY: Forwards its implementation to `(A,)`, which is itself safe.
unsafe impl<A: Event> EventSet for A {
- type Out<'trigger> = &'trigger mut A;
- type OutReadonly<'trigger> = &'trigger A;
+ type Item<'trigger> = &'trigger mut A;
+ type ReadOnlyItem<'trigger> = &'trigger A;
unsafe fn checked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
- ) -> Option<Self::Out<'trigger>> {
+ ) -> Option<Self::Item<'trigger>> {
<(A,) as EventSet>::checked_cast(world, observer_trigger, ptr)
}
fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
<(A,) as EventSet>::init_components(world, ids);
}
+
+ fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
+ <(A,) as EventSet>::shrink(item)
+ }
+
+ fn shrink_readonly<'long: 'short, 'short>(
+ item: &'short Self::Item<'long>,
+ ) -> Self::ReadOnlyItem<'short> {
+ <(A,) as EventSet>::shrink_readonly(item)
+ }
+
+ fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
+ <(A,) as EventSet>::shrink_ptr(item)
+ }
}
// SAFETY: All event types have a component id registered in `init_components`,
// and `checked_cast` checks that the component id matches the event type.
unsafe impl<A: Event> EventSet for (A,) {
- type Out<'trigger> = &'trigger mut A;
- type OutReadonly<'trigger> = &'trigger A;
+ type Item<'trigger> = &'trigger mut A;
+ type ReadOnlyItem<'trigger> = &'trigger A;
unsafe fn checked_cast<'trigger>(
_world: &World,
_observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
- ) -> Option<Self::Out<'trigger>> {
+ ) -> Option<Self::Item<'trigger>> {
// SAFETY: Caller must ensure that the component id matches the event type
Some(unsafe { ptr.deref_mut() })
}
@@ -68,6 +93,20 @@ unsafe impl<A: Event> EventSet for (A,) {
let a_id = world.init_component::<A>();
ids(a_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
+ }
+
+ fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
+ Ptr::from(&**item)
+ }
}
/// The output type of an observer that observes two different event types.
@@ -81,14 +120,14 @@ pub enum Or2<A, B> {
// SAFETY: All event types have a component id registered in `init_components`,
// and `checked_cast` checks that the component id matches one of the event types before casting.
unsafe impl<A: Event, B: Event> EventSet for (A, B) {
- type Out<'trigger> = Or2<&'trigger mut A, &'trigger mut B>;
- type OutReadonly<'trigger> = Or2<&'trigger A, &'trigger B>;
+ type Item<'trigger> = Or2<&'trigger mut A, &'trigger mut B>;
+ type ReadOnlyItem<'trigger> = Or2<&'trigger A, &'trigger B>;
unsafe fn checked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
- ) -> Option<Self::Out<'trigger>> {
+ ) -> Option<Self::Item<'trigger>> {
let a_id = world.component_id::<A>()?;
let b_id = world.component_id::<B>()?;
@@ -111,6 +150,29 @@ unsafe impl<A: Event, B: Event> EventSet for (A, B) {
ids(a_id);
ids(b_id);
}
+
+ fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
+ match item {
+ Or2::A(a) => Or2::A(a),
+ Or2::B(b) => Or2::B(b),
+ }
+ }
+
+ fn shrink_readonly<'long: 'short, 'short>(
+ item: &'short Self::Item<'long>,
+ ) -> Self::ReadOnlyItem<'short> {
+ match item {
+ Or2::A(a) => Or2::A(a),
+ Or2::B(b) => Or2::B(b),
+ }
+ }
+
+ fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
+ match item {
+ Or2::A(a) => Ptr::from(&**a),
+ Or2::B(b) => Ptr::from(&**b),
+ }
+ }
}
/// The output type of an observer that observes three different event types.
@@ -126,14 +188,14 @@ pub enum Or3<A, B, C> {
// SAFETY: All event types have a component id registered in `init_components`,
// and `checked_cast` checks that the component id matches one of the event types before casting.
unsafe impl<A: Event, B: Event, C: Event> EventSet for (A, B, C) {
- type Out<'trigger> = Or3<&'trigger mut A, &'trigger mut B, &'trigger mut C>;
- type OutReadonly<'trigger> = Or3<&'trigger A, &'trigger B, &'trigger C>;
+ type Item<'trigger> = Or3<&'trigger mut A, &'trigger mut B, &'trigger mut C>;
+ type ReadOnlyItem<'trigger> = Or3<&'trigger A, &'trigger B, &'trigger C>;
unsafe fn checked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
- ) -> Option<Self::Out<'trigger>> {
+ ) -> Option<Self::Item<'trigger>> {
let a_id = world.component_id::<A>()?;
let b_id = world.component_id::<B>()?;
let c_id = world.component_id::<C>()?;
@@ -163,4 +225,30 @@ unsafe impl<A: Event, B: Event, C: Event> EventSet for (A, B, C) {
ids(b_id);
ids(c_id);
}
+
+ fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
+ match item {
+ Or3::A(a) => Or3::A(a),
+ Or3::B(b) => Or3::B(b),
+ Or3::C(c) => Or3::C(c),
+ }
+ }
+
+ fn shrink_readonly<'long: 'short, 'short>(
+ item: &'short Self::Item<'long>,
+ ) -> Self::ReadOnlyItem<'short> {
+ match item {
+ Or3::A(a) => Or3::A(a),
+ Or3::B(b) => Or3::B(b),
+ Or3::C(c) => Or3::C(c),
+ }
+ }
+
+ fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
+ match item {
+ Or3::A(a) => Ptr::from(&**a),
+ Or3::B(b) => Ptr::from(&**b),
+ Or3::C(c) => Ptr::from(&**c),
+ }
+ }
}
diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs
index 2045ff011bf8e..bfa23456c6e2e 100644
--- a/crates/bevy_ecs/src/world/entity_ref.rs
+++ b/crates/bevy_ecs/src/world/entity_ref.rs
@@ -4,7 +4,6 @@ use crate::{
change_detection::MutUntyped,
component::{Component, ComponentId, ComponentTicks, Components, StorageType},
entity::{Entities, Entity, EntityLocation},
- event::Event,
observer::{EventSet, Observer, Observers},
query::Access,
removal_detection::RemovedComponentEvents,
From 85178f50d06dc278e197bc8355507e0827c69681 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Fri, 9 Aug 2024 12:50:13 -0500
Subject: [PATCH 05/34] recursive event set support and macro generated impls
up to n=8
---
crates/bevy_ecs/src/observer/runner.rs | 5 +-
crates/bevy_ecs/src/observer/set.rs | 304 +++++++++++--------------
2 files changed, 142 insertions(+), 167 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index 8098d6670a1a3..9955c0ceea309 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -386,8 +386,9 @@ fn observer_system_runner<E: EventSet, B: Bundle>(
// SAFETY: We have immutable access to the world from the passed in DeferredWorld
let world_ref = unsafe { world.world() };
- // SAFETY: Caller ensures `ptr` is castable to `&mut T`, or the function will check it itself. TODO: can we revert this to a safe function?
- let Some(event) = (unsafe { E::checked_cast(world_ref, &observer_trigger, ptr) }) else {
+ // TODO: should we check E::matches here?
+ // SAFETY: Caller ensures `ptr` is castable to `&mut T`, or the function will check it itself.
+ let Some(event) = (unsafe { E::cast(world_ref, &observer_trigger, ptr) }) else {
return;
};
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 9a1a9ac096414..224c42314d59f 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -17,16 +17,22 @@ pub unsafe trait EventSet: 'static {
/// The output type that will be passed to the observer.
type Item<'trigger>;
/// The read-only variant of the output type.
- type ReadOnlyItem<'trigger>;
-
- /// Safely casts the pointer to the output type, or a variant of it.
- /// Returns `None` if the event type does not match.
- unsafe fn checked_cast<'trigger>(
+ type ReadOnlyItem<'trigger>: Copy;
+
+ /// Casts a pointer to the output type.
+ ///
+ /// # Safety
+ ///
+ /// Caller must check that [`matches`] returns `true` before calling this method.
+ unsafe fn cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Item<'trigger>>;
+ /// Checks if the event type matches the observer trigger.
+ fn matches<'trigger>(world: &World, observer_trigger: &ObserverTrigger) -> bool;
+
/// Initialize the components required by the event set.
fn init_components(world: &mut World, ids: impl FnMut(ComponentId));
@@ -38,60 +44,38 @@ pub unsafe trait EventSet: 'static {
item: &'short Self::Item<'long>,
) -> Self::ReadOnlyItem<'short>;
- /// Shrink the item to a shorter lifetime, as a type-erased pointer.
+ /// Shrink the item to a shorter lifetime, as a read-only type-erased pointer.
+ ///
+ /// # Safety
+ ///
+ /// Implementor must give a pointer to the innermost data, not a pointer to any container.
fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short>;
}
-// SAFETY: Forwards its implementation to `(A,)`, which is itself safe.
-unsafe impl<A: Event> EventSet for A {
- type Item<'trigger> = &'trigger mut A;
- type ReadOnlyItem<'trigger> = &'trigger A;
-
- unsafe fn checked_cast<'trigger>(
- world: &World,
- observer_trigger: &ObserverTrigger,
- ptr: PtrMut<'trigger>,
- ) -> Option<Self::Item<'trigger>> {
- <(A,) as EventSet>::checked_cast(world, observer_trigger, ptr)
- }
-
- fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
- <(A,) as EventSet>::init_components(world, ids);
- }
-
- fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
- <(A,) as EventSet>::shrink(item)
- }
-
- fn shrink_readonly<'long: 'short, 'short>(
- item: &'short Self::Item<'long>,
- ) -> Self::ReadOnlyItem<'short> {
- <(A,) as EventSet>::shrink_readonly(item)
- }
-
- fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
- <(A,) as EventSet>::shrink_ptr(item)
- }
-}
-
-// SAFETY: All event types have a component id registered in `init_components`,
-// and `checked_cast` checks that the component id matches the event type.
-unsafe impl<A: Event> EventSet for (A,) {
- type Item<'trigger> = &'trigger mut A;
- type ReadOnlyItem<'trigger> = &'trigger A;
+// SAFETY: The event type has a component id registered in `init_components`,
+// and `matches` checks that the component id matches the event type.
+unsafe impl<E: Event> EventSet for E {
+ type Item<'trigger> = &'trigger mut E;
+ type ReadOnlyItem<'trigger> = &'trigger E;
- unsafe fn checked_cast<'trigger>(
+ unsafe fn cast<'trigger>(
_world: &World,
_observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Item<'trigger>> {
- // SAFETY: Caller must ensure that the component id matches the event type
+ // SAFETY: Caller must ensure that `matches` returns true before calling `cast`
Some(unsafe { ptr.deref_mut() })
}
+ fn matches<'trigger>(world: &World, observer_trigger: &ObserverTrigger) -> bool {
+ world
+ .component_id::<E>()
+ .is_some_and(|id| id == observer_trigger.event_type)
+ }
+
fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
- let a_id = world.init_component::<A>();
- ids(a_id);
+ let id = world.init_component::<E>();
+ ids(id);
}
fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
@@ -109,146 +93,136 @@ unsafe impl<A: Event> EventSet for (A,) {
}
}
-/// The output type of an observer that observes two different event types.
-pub enum Or2<A, B> {
- /// The first event type.
- A(A),
- /// The second event type.
- B(B),
-}
+unsafe impl<A: EventSet> EventSet for (A,) {
+ type Item<'trigger> = A::Item<'trigger>;
+ type ReadOnlyItem<'trigger> = A::ReadOnlyItem<'trigger>;
-// SAFETY: All event types have a component id registered in `init_components`,
-// and `checked_cast` checks that the component id matches one of the event types before casting.
-unsafe impl<A: Event, B: Event> EventSet for (A, B) {
- type Item<'trigger> = Or2<&'trigger mut A, &'trigger mut B>;
- type ReadOnlyItem<'trigger> = Or2<&'trigger A, &'trigger B>;
-
- unsafe fn checked_cast<'trigger>(
+ unsafe fn cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Item<'trigger>> {
- let a_id = world.component_id::<A>()?;
- let b_id = world.component_id::<B>()?;
-
- if a_id == observer_trigger.event_type {
- // SAFETY: We just checked that the component id matches the event type
- let a = unsafe { ptr.deref_mut() };
- Some(Or2::A(a))
- } else if b_id == observer_trigger.event_type {
- // SAFETY: We just checked that the component id matches the event type
- let b = unsafe { ptr.deref_mut() };
- Some(Or2::B(b))
- } else {
- None
- }
+ // SAFETY: Caller must ensure that `matches` returns true before calling `cast`
+ unsafe { A::cast(world, observer_trigger, ptr) }
}
- fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
- let a_id = world.init_component::<A>();
- let b_id = world.init_component::<B>();
- ids(a_id);
- ids(b_id);
+ fn matches<'trigger>(world: &World, observer_trigger: &ObserverTrigger) -> bool {
+ A::matches(world, observer_trigger)
+ }
+
+ fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
+ A::init_components(world, ids);
}
fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
- match item {
- Or2::A(a) => Or2::A(a),
- Or2::B(b) => Or2::B(b),
- }
+ A::shrink(item)
}
fn shrink_readonly<'long: 'short, 'short>(
item: &'short Self::Item<'long>,
) -> Self::ReadOnlyItem<'short> {
- match item {
- Or2::A(a) => Or2::A(a),
- Or2::B(b) => Or2::B(b),
- }
+ A::shrink_readonly(item)
}
fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
- match item {
- Or2::A(a) => Ptr::from(&**a),
- Or2::B(b) => Ptr::from(&**b),
- }
+ A::shrink_ptr(item)
}
}
-/// The output type of an observer that observes three different event types.
-pub enum Or3<A, B, C> {
- /// The first event type.
- A(A),
- /// The second event type.
- B(B),
- /// The third event type.
- C(C),
-}
-
-// SAFETY: All event types have a component id registered in `init_components`,
-// and `checked_cast` checks that the component id matches one of the event types before casting.
-unsafe impl<A: Event, B: Event, C: Event> EventSet for (A, B, C) {
- type Item<'trigger> = Or3<&'trigger mut A, &'trigger mut B, &'trigger mut C>;
- type ReadOnlyItem<'trigger> = Or3<&'trigger A, &'trigger B, &'trigger C>;
-
- unsafe fn checked_cast<'trigger>(
- world: &World,
- observer_trigger: &ObserverTrigger,
- ptr: PtrMut<'trigger>,
- ) -> Option<Self::Item<'trigger>> {
- let a_id = world.component_id::<A>()?;
- let b_id = world.component_id::<B>()?;
- let c_id = world.component_id::<C>()?;
-
- if a_id == observer_trigger.event_type {
- // SAFETY: We just checked that the component id matches the event type
- let a = unsafe { ptr.deref_mut() };
- Some(Or3::A(a))
- } else if b_id == observer_trigger.event_type {
- // SAFETY: We just checked that the component id matches the event type
- let b = unsafe { ptr.deref_mut() };
- Some(Or3::B(b))
- } else if c_id == observer_trigger.event_type {
- // SAFETY: We just checked that the component id matches the event type
- let c = unsafe { ptr.deref_mut() };
- Some(Or3::C(c))
- } else {
- None
- }
- }
-
- fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
- let a_id = world.init_component::<A>();
- let b_id = world.init_component::<B>();
- let c_id = world.init_component::<C>();
- ids(a_id);
- ids(b_id);
- ids(c_id);
- }
-
- fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
- match item {
- Or3::A(a) => Or3::A(a),
- Or3::B(b) => Or3::B(b),
- Or3::C(c) => Or3::C(c),
+macro_rules! impl_event_set {
+ ($Or:ident, $(($P:ident, $p:ident)),*) => {
+ /// An output type of an observer that observes multiple event types.
+ #[derive(Copy, Clone)]
+ pub enum $Or<$($P),*> {
+ $(
+ /// A possible event type.
+ $P($P),
+ )*
}
- }
- fn shrink_readonly<'long: 'short, 'short>(
- item: &'short Self::Item<'long>,
- ) -> Self::ReadOnlyItem<'short> {
- match item {
- Or3::A(a) => Or3::A(a),
- Or3::B(b) => Or3::B(b),
- Or3::C(c) => Or3::C(c),
+ // SAFETY: All event types have a component id registered in `init_components`,
+ // and `cast` calls `matches` before casting the pointer to one of the event types.
+ unsafe impl<$($P: EventSet),*> EventSet for ($($P,)*) {
+ type Item<'trigger> = $Or<$($P::Item<'trigger>),*>;
+ type ReadOnlyItem<'trigger> = $Or<$($P::ReadOnlyItem<'trigger>),*>;
+
+ unsafe fn cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Option<Self::Item<'trigger>> {
+ if false {
+ unreachable!();
+ }
+ $(
+ else if $P::matches(world, observer_trigger) {
+ // SAFETY: We just checked that the component id matches the event type
+ let $p = unsafe { $P::cast(world, observer_trigger, ptr) };
+ if let Some($p) = $p {
+ return Some($Or::$P($p));
+ }
+ }
+ )*
+
+ None
+ }
+
+ fn matches<'trigger>(world: &World, observer_trigger: &ObserverTrigger) -> bool {
+ $(
+ $P::matches(world, observer_trigger) ||
+ )*
+ false
+ }
+
+ fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
+ $(
+ $P::init_components(world, &mut ids);
+ )*
+ }
+
+ fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
+ match item {
+ $(
+ $Or::$P($p) => $Or::$P($P::shrink($p)),
+ )*
+ }
+ }
+
+ fn shrink_readonly<'long: 'short, 'short>(
+ item: &'short Self::Item<'long>,
+ ) -> Self::ReadOnlyItem<'short> {
+ match item {
+ $(
+ $Or::$P($p) => $Or::$P($P::shrink_readonly($p)),
+ )*
+ }
+ }
+
+ fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
+ match item {
+ $(
+ $Or::$P($p) => $P::shrink_ptr($p),
+ )*
+ }
+ }
}
- }
-
- fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
- match item {
- Or3::A(a) => Ptr::from(&**a),
- Or3::B(b) => Ptr::from(&**b),
- Or3::C(c) => Ptr::from(&**c),
- }
- }
+ };
}
+
+impl_event_set!(Or2, (A, a), (B, b));
+impl_event_set!(Or3, (A, a), (B, b), (C, c));
+impl_event_set!(Or4, (A, a), (B, b), (C, c), (D, d));
+impl_event_set!(Or5, (A, a), (B, b), (C, c), (D, d), (E, e));
+impl_event_set!(Or6, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f));
+impl_event_set!(Or7, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g));
+impl_event_set!(
+ Or8,
+ (A, a),
+ (B, b),
+ (C, c),
+ (D, d),
+ (E, e),
+ (F, f),
+ (G, g),
+ (H, h)
+);
From eec60db013ac12bc9e72ba42fb780696d5496479 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Fri, 9 Aug 2024 17:55:24 -0500
Subject: [PATCH 06/34] add untyped wrapper that foregoes casting and safety
checks, replacing some of the unsafe blocks. also added a safe method for
adding events to an observer when it's all untyped
---
crates/bevy_ecs/src/observer/mod.rs | 24 ++++--
crates/bevy_ecs/src/observer/runner.rs | 20 ++++-
crates/bevy_ecs/src/observer/set.rs | 108 ++++++++++++++++++++-----
3 files changed, 124 insertions(+), 28 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 67046afc4fe8c..5a5450f5ee55b 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -435,7 +435,7 @@ mod tests {
use crate as bevy_ecs;
use crate::observer::{
- EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace, Or2,
+ EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace, Or2, Untyped,
};
use crate::prelude::*;
use crate::traversal::Traversal;
@@ -623,17 +623,29 @@ mod tests {
assert_eq!(2, world.resource::<R>().0);
}
+ #[test]
+ fn observer_multiple_events_untyped() {
+ let mut world = World::new();
+ world.init_resource::<R>();
+ world.spawn(Observer::new(
+ |_: Trigger<Untyped<(OnAdd, OnRemove)>, A>, mut res: ResMut<R>| res.0 += 1,
+ ));
+
+ let entity = world.spawn(A).id();
+ world.despawn(entity);
+ assert_eq!(2, world.resource::<R>().0);
+ }
+
#[test]
fn observer_multiple_events_unsafe() {
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<Untyped<()>, A>, mut res: ResMut<R>| res.0 += 1)
+ .with_event_safe(on_add)
+ .with_event_safe(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 9955c0ceea309..88b64da901c32 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::{EventSet, ObserverDescriptor, ObserverTrigger},
+ observer::{EventSet, ObserverDescriptor, ObserverTrigger, Untyped},
prelude::*,
query::DebugCheckedUnwrap,
system::{IntoObserverSystem, ObserverSystem},
@@ -307,6 +307,20 @@ impl<E: EventSet, B: Bundle> Observer<E, B> {
}
}
+impl<E, B: Bundle> Observer<Untyped<E>, B>
+where
+ Untyped<E>: EventSet,
+{
+ /// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
+ /// is triggered.
+ ///
+ /// As opposed to [`Observer::with_event`], this method is safe to use because it does not cast the event to a specific type.
+ pub fn with_event_safe(mut self, event: ComponentId) -> Self {
+ self.descriptor.events.push(event);
+ self
+ }
+}
+
impl<E: EventSet, B: Bundle> Component for Observer<E, B> {
const STORAGE_TYPE: StorageType = StorageType::SparseSet;
fn register_component_hooks(hooks: &mut ComponentHooks) {
@@ -386,9 +400,7 @@ fn observer_system_runner<E: EventSet, B: Bundle>(
// SAFETY: We have immutable access to the world from the passed in DeferredWorld
let world_ref = unsafe { world.world() };
- // TODO: should we check E::matches here?
- // SAFETY: Caller ensures `ptr` is castable to `&mut T`, or the function will check it itself.
- let Some(event) = (unsafe { E::cast(world_ref, &observer_trigger, ptr) }) else {
+ let Some(event) = E::cast(world_ref, &observer_trigger, ptr) else {
return;
};
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 224c42314d59f..9d880065b82b6 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -23,15 +23,15 @@ pub unsafe trait EventSet: 'static {
///
/// # Safety
///
- /// Caller must check that [`matches`] returns `true` before calling this method.
- unsafe fn cast<'trigger>(
+ /// Implementor must ensure that the component id matches the event type before casting the pointer.
+ fn cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Item<'trigger>>;
/// Checks if the event type matches the observer trigger.
- fn matches<'trigger>(world: &World, observer_trigger: &ObserverTrigger) -> bool;
+ fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool;
/// Initialize the components required by the event set.
fn init_components(world: &mut World, ids: impl FnMut(ComponentId));
@@ -58,16 +58,20 @@ unsafe impl<E: Event> EventSet for E {
type Item<'trigger> = &'trigger mut E;
type ReadOnlyItem<'trigger> = &'trigger E;
- unsafe fn cast<'trigger>(
- _world: &World,
- _observer_trigger: &ObserverTrigger,
+ fn cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Item<'trigger>> {
- // SAFETY: Caller must ensure that `matches` returns true before calling `cast`
- Some(unsafe { ptr.deref_mut() })
+ if Self::matches(world, observer_trigger) {
+ // SAFETY: We just checked that the component id matches the event type
+ Some(unsafe { ptr.deref_mut() })
+ } else {
+ None
+ }
}
- fn matches<'trigger>(world: &World, observer_trigger: &ObserverTrigger) -> bool {
+ fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool {
world
.component_id::<E>()
.is_some_and(|id| id == observer_trigger.event_type)
@@ -97,16 +101,15 @@ unsafe impl<A: EventSet> EventSet for (A,) {
type Item<'trigger> = A::Item<'trigger>;
type ReadOnlyItem<'trigger> = A::ReadOnlyItem<'trigger>;
- unsafe fn cast<'trigger>(
+ fn cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Item<'trigger>> {
- // SAFETY: Caller must ensure that `matches` returns true before calling `cast`
- unsafe { A::cast(world, observer_trigger, ptr) }
+ A::cast(world, observer_trigger, ptr)
}
- fn matches<'trigger>(world: &World, observer_trigger: &ObserverTrigger) -> bool {
+ fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool {
A::matches(world, observer_trigger)
}
@@ -146,7 +149,7 @@ macro_rules! impl_event_set {
type Item<'trigger> = $Or<$($P::Item<'trigger>),*>;
type ReadOnlyItem<'trigger> = $Or<$($P::ReadOnlyItem<'trigger>),*>;
- unsafe fn cast<'trigger>(
+ fn cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
@@ -156,9 +159,7 @@ macro_rules! impl_event_set {
}
$(
else if $P::matches(world, observer_trigger) {
- // SAFETY: We just checked that the component id matches the event type
- let $p = unsafe { $P::cast(world, observer_trigger, ptr) };
- if let Some($p) = $p {
+ if let Some($p) = $P::cast(world, observer_trigger, ptr) {
return Some($Or::$P($p));
}
}
@@ -167,7 +168,7 @@ macro_rules! impl_event_set {
None
}
- fn matches<'trigger>(world: &World, observer_trigger: &ObserverTrigger) -> bool {
+ fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool {
$(
$P::matches(world, observer_trigger) ||
)*
@@ -226,3 +227,74 @@ impl_event_set!(
(G, g),
(H, h)
);
+
+/// A wrapper around an [`EventSet`] that foregoes safety checks and casting.
+pub struct Untyped<E>(std::marker::PhantomData<E>);
+
+unsafe impl<E: EventSet> EventSet for Untyped<E> {
+ type Item<'trigger> = PtrMut<'trigger>;
+ type ReadOnlyItem<'trigger> = Ptr<'trigger>;
+
+ fn cast<'trigger>(
+ _world: &World,
+ _observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Option<Self::Item<'trigger>> {
+ Some(ptr)
+ }
+
+ fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
+ true
+ }
+
+ fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
+ E::init_components(world, ids);
+ }
+
+ 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()
+ }
+
+ fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
+ item.as_ref()
+ }
+}
+
+unsafe impl EventSet for Untyped<()> {
+ type Item<'trigger> = PtrMut<'trigger>;
+ type ReadOnlyItem<'trigger> = Ptr<'trigger>;
+
+ fn cast<'trigger>(
+ _world: &World,
+ _observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Option<Self::Item<'trigger>> {
+ Some(ptr)
+ }
+
+ fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
+ true
+ }
+
+ 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()
+ }
+
+ fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
+ item.as_ref()
+ }
+}
From 516a3b7a095f33b25e2d9e7a69d6458a209456fc Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Fri, 9 Aug 2024 18:03:08 -0500
Subject: [PATCH 07/34] The non-empty Untyped<(A, B)> impl should still only
match on A, B, etc
---
crates/bevy_ecs/src/observer/set.rs | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 9d880065b82b6..4144d5ed16ae6 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -231,6 +231,7 @@ impl_event_set!(
/// A wrapper around an [`EventSet`] that foregoes safety checks and casting.
pub struct Untyped<E>(std::marker::PhantomData<E>);
+/// An [`EventSet`] that matches the specified event type(s), but does not cast the pointer.
unsafe impl<E: EventSet> EventSet for Untyped<E> {
type Item<'trigger> = PtrMut<'trigger>;
type ReadOnlyItem<'trigger> = Ptr<'trigger>;
@@ -243,8 +244,8 @@ unsafe impl<E: EventSet> EventSet for Untyped<E> {
Some(ptr)
}
- fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
- true
+ fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool {
+ E::matches(world, observer_trigger)
}
fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
@@ -266,6 +267,7 @@ unsafe impl<E: EventSet> EventSet for Untyped<E> {
}
}
+/// An [`EventSet`] that matches any event type.
unsafe impl EventSet for Untyped<()> {
type Item<'trigger> = PtrMut<'trigger>;
type ReadOnlyItem<'trigger> = Ptr<'trigger>;
From da0792931d0228d54cb49c6e096ad5079e9b287c Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Fri, 9 Aug 2024 18:09:18 -0500
Subject: [PATCH 08/34] Observer::with_event is no longer unsafe, because we
always check that the event type matches the event set before casting
---
crates/bevy_ecs/src/observer/mod.rs | 4 ++--
crates/bevy_ecs/src/observer/runner.rs | 20 +++-----------------
crates/bevy_ecs/src/observer/set.rs | 2 +-
3 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 5a5450f5ee55b..fb88e75756ef3 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -644,8 +644,8 @@ mod tests {
let on_remove = world.init_component::<OnRemove>();
world.spawn(
Observer::new(|_: Trigger<Untyped<()>, A>, mut res: ResMut<R>| res.0 += 1)
- .with_event_safe(on_add)
- .with_event_safe(on_remove),
+ .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 88b64da901c32..381d9e8dd3051 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -296,26 +296,12 @@ impl<E: EventSet, B: Bundle> Observer<E, B> {
self
}
- /// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
- /// is triggered.
- /// # 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 {
- self.descriptor.events.push(event);
- self
- }
-}
-
-impl<E, B: Bundle> Observer<Untyped<E>, B>
-where
- Untyped<E>: EventSet,
-{
/// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
/// is triggered.
///
- /// As opposed to [`Observer::with_event`], this method is safe to use because it does not cast the event to a specific type.
- pub fn with_event_safe(mut self, event: ComponentId) -> Self {
+ /// Note that for any event types that are not matched by `E`, the observer will not be triggered.
+ /// Use [`Untyped<()>`] to match all events added through this method.
+ pub fn with_event(mut self, event: ComponentId) -> Self {
self.descriptor.events.push(event);
self
}
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 4144d5ed16ae6..660bd7f85339e 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -228,7 +228,7 @@ impl_event_set!(
(H, h)
);
-/// A wrapper around an [`EventSet`] that foregoes safety checks and casting.
+/// A wrapper around an [`EventSet`] that foregoes safety checks and casting, and passes the pointer as is.
pub struct Untyped<E>(std::marker::PhantomData<E>);
/// An [`EventSet`] that matches the specified event type(s), but does not cast the pointer.
From be65bf32eaa303ae584730ac9303f0da26093bd1 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Fri, 9 Aug 2024 20:04:37 -0500
Subject: [PATCH 09/34] performance reductions means reintroducing some unsafe
blocks
---
crates/bevy_ecs/src/observer/mod.rs | 6 +++---
crates/bevy_ecs/src/observer/runner.rs | 23 ++++++++++++++++----
crates/bevy_ecs/src/observer/set.rs | 30 +++++++++++---------------
3 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index fb88e75756ef3..553a22140142f 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -642,11 +642,11 @@ mod tests {
world.init_resource::<R>();
let on_add = world.init_component::<OnAdd>();
let on_remove = world.init_component::<OnRemove>();
- world.spawn(
+ world.spawn(unsafe {
Observer::new(|_: Trigger<Untyped<()>, A>, mut res: ResMut<R>| res.0 += 1)
.with_event(on_add)
- .with_event(on_remove),
- );
+ .with_event(on_remove)
+ });
let entity = world.spawn(A).id();
world.despawn(entity);
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index 381d9e8dd3051..e818f9e6a8f20 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -296,12 +296,26 @@ impl<E: EventSet, B: Bundle> Observer<E, B> {
self
}
+ /// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
+ /// is triggered.
+ /// # 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 {
+ self.descriptor.events.push(event);
+ self
+ }
+}
+
+impl<E, B: Bundle> Observer<Untyped<E>, B>
+where
+ Untyped<E>: EventSet,
+{
/// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
/// is triggered.
///
- /// Note that for any event types that are not matched by `E`, the observer will not be triggered.
- /// Use [`Untyped<()>`] to match all events added through this method.
- pub fn with_event(mut self, event: ComponentId) -> Self {
+ /// As opposed to [`Observer::with_event`], this method is safe to use because it does not cast any pointers automatically.
+ pub fn with_event_safe(mut self, event: ComponentId) -> Self {
self.descriptor.events.push(event);
self
}
@@ -386,7 +400,8 @@ fn observer_system_runner<E: EventSet, B: Bundle>(
// SAFETY: We have immutable access to the world from the passed in DeferredWorld
let world_ref = unsafe { world.world() };
- let Some(event) = E::cast(world_ref, &observer_trigger, ptr) else {
+ // SAFETY: Observer was triggered with an event in the set of events it observes, so it must be convertible to E
+ let Some(event) = (unsafe { E::unchecked_cast(world_ref, &observer_trigger, ptr) }) else {
return;
};
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 660bd7f85339e..2cf4d61fafd71 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -23,8 +23,8 @@ pub unsafe trait EventSet: 'static {
///
/// # Safety
///
- /// Implementor must ensure that the component id matches the event type before casting the pointer.
- fn cast<'trigger>(
+ /// Caller must ensure that the component id [`EventSet::matches`] this event set before calling this function.
+ unsafe fn unchecked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
@@ -58,17 +58,13 @@ unsafe impl<E: Event> EventSet for E {
type Item<'trigger> = &'trigger mut E;
type ReadOnlyItem<'trigger> = &'trigger E;
- fn cast<'trigger>(
- world: &World,
- observer_trigger: &ObserverTrigger,
+ unsafe fn unchecked_cast<'trigger>(
+ _world: &World,
+ _observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Item<'trigger>> {
- if Self::matches(world, observer_trigger) {
- // SAFETY: We just checked that the component id matches the event type
- Some(unsafe { ptr.deref_mut() })
- } else {
- None
- }
+ // SAFETY: Caller must ensure that the component id matches the event type
+ Some(unsafe { ptr.deref_mut() })
}
fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool {
@@ -101,12 +97,12 @@ unsafe impl<A: EventSet> EventSet for (A,) {
type Item<'trigger> = A::Item<'trigger>;
type ReadOnlyItem<'trigger> = A::ReadOnlyItem<'trigger>;
- fn cast<'trigger>(
+ unsafe fn unchecked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Option<Self::Item<'trigger>> {
- A::cast(world, observer_trigger, ptr)
+ A::unchecked_cast(world, observer_trigger, ptr)
}
fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool {
@@ -149,7 +145,7 @@ macro_rules! impl_event_set {
type Item<'trigger> = $Or<$($P::Item<'trigger>),*>;
type ReadOnlyItem<'trigger> = $Or<$($P::ReadOnlyItem<'trigger>),*>;
- fn cast<'trigger>(
+ unsafe fn unchecked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
@@ -159,7 +155,7 @@ macro_rules! impl_event_set {
}
$(
else if $P::matches(world, observer_trigger) {
- if let Some($p) = $P::cast(world, observer_trigger, ptr) {
+ if let Some($p) = $P::unchecked_cast(world, observer_trigger, ptr) {
return Some($Or::$P($p));
}
}
@@ -236,7 +232,7 @@ unsafe impl<E: EventSet> EventSet for Untyped<E> {
type Item<'trigger> = PtrMut<'trigger>;
type ReadOnlyItem<'trigger> = Ptr<'trigger>;
- fn cast<'trigger>(
+ unsafe fn unchecked_cast<'trigger>(
_world: &World,
_observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
@@ -272,7 +268,7 @@ unsafe impl EventSet for Untyped<()> {
type Item<'trigger> = PtrMut<'trigger>;
type ReadOnlyItem<'trigger> = Ptr<'trigger>;
- fn cast<'trigger>(
+ unsafe fn unchecked_cast<'trigger>(
_world: &World,
_observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
From a0774d6e58fa7b0cfab0ae0ba96386f4603591db Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Fri, 9 Aug 2024 20:29:21 -0500
Subject: [PATCH 10/34] remove Trigger::event_ptr in favor of UntypedEvent
event set
---
crates/bevy_ecs/src/observer/mod.rs | 22 +++++++--------
crates/bevy_ecs/src/observer/runner.rs | 6 ++---
crates/bevy_ecs/src/observer/set.rs | 37 +++-----------------------
3 files changed, 15 insertions(+), 50 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 553a22140142f..f2ec328c9376b 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -52,11 +52,6 @@ impl<'w, E: EventSet, B: Bundle> Trigger<'w, E, B> {
E::shrink(&mut self.event)
}
- /// Returns a pointer to the triggered event.
- pub fn event_ptr(&self) -> Ptr {
- E::shrink_ptr(&self.event)
- }
-
/// Returns the entity that triggered the observer, could be [`Entity::PLACEHOLDER`].
pub fn entity(&self) -> Entity {
self.trigger.entity
@@ -435,7 +430,8 @@ mod tests {
use crate as bevy_ecs;
use crate::observer::{
- EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace, Or2, Untyped,
+ EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace, Or2,
+ UntypedEvent,
};
use crate::prelude::*;
use crate::traversal::Traversal;
@@ -614,7 +610,7 @@ mod tests {
res.0 += 1;
match t.event() {
Or2::A(Foo(v)) => assert_eq!(5, *v),
- Or2::B(Bar(v)) => assert_eq!(true, *v),
+ Or2::B(Bar(v)) => assert!(*v),
}
});
world.flush(); // TODO: should we auto-flush after observe?
@@ -628,7 +624,7 @@ mod tests {
let mut world = World::new();
world.init_resource::<R>();
world.spawn(Observer::new(
- |_: Trigger<Untyped<(OnAdd, OnRemove)>, A>, mut res: ResMut<R>| res.0 += 1,
+ |_: Trigger<UntypedEvent<(OnAdd, OnRemove)>, A>, mut res: ResMut<R>| res.0 += 1,
));
let entity = world.spawn(A).id();
@@ -642,11 +638,11 @@ mod tests {
world.init_resource::<R>();
let on_add = world.init_component::<OnAdd>();
let on_remove = world.init_component::<OnRemove>();
- world.spawn(unsafe {
- Observer::new(|_: Trigger<Untyped<()>, A>, mut res: ResMut<R>| res.0 += 1)
- .with_event(on_add)
- .with_event(on_remove)
- });
+ world.spawn(
+ Observer::new(|_: Trigger<UntypedEvent, A>, mut res: ResMut<R>| res.0 += 1)
+ .with_event_safe(on_add)
+ .with_event_safe(on_remove),
+ );
let entity = world.spawn(A).id();
world.despawn(entity);
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index e818f9e6a8f20..696aaf9f40be7 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::{EventSet, ObserverDescriptor, ObserverTrigger, Untyped},
+ observer::{EventSet, ObserverDescriptor, ObserverTrigger, UntypedEvent},
prelude::*,
query::DebugCheckedUnwrap,
system::{IntoObserverSystem, ObserverSystem},
@@ -307,9 +307,9 @@ impl<E: EventSet, B: Bundle> Observer<E, B> {
}
}
-impl<E, B: Bundle> Observer<Untyped<E>, B>
+impl<E, B: Bundle> Observer<UntypedEvent<E>, B>
where
- Untyped<E>: EventSet,
+ UntypedEvent<E>: EventSet,
{
/// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
/// is triggered.
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 2cf4d61fafd71..b6a5af362a766 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -43,13 +43,6 @@ pub unsafe trait EventSet: 'static {
fn shrink_readonly<'long: 'short, 'short>(
item: &'short Self::Item<'long>,
) -> Self::ReadOnlyItem<'short>;
-
- /// Shrink the item to a shorter lifetime, as a read-only type-erased pointer.
- ///
- /// # Safety
- ///
- /// Implementor must give a pointer to the innermost data, not a pointer to any container.
- fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short>;
}
// SAFETY: The event type has a component id registered in `init_components`,
@@ -87,10 +80,6 @@ unsafe impl<E: Event> EventSet for E {
) -> Self::ReadOnlyItem<'short> {
item
}
-
- fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
- Ptr::from(&**item)
- }
}
unsafe impl<A: EventSet> EventSet for (A,) {
@@ -122,10 +111,6 @@ unsafe impl<A: EventSet> EventSet for (A,) {
) -> Self::ReadOnlyItem<'short> {
A::shrink_readonly(item)
}
-
- fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
- A::shrink_ptr(item)
- }
}
macro_rules! impl_event_set {
@@ -194,14 +179,6 @@ macro_rules! impl_event_set {
)*
}
}
-
- fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
- match item {
- $(
- $Or::$P($p) => $P::shrink_ptr($p),
- )*
- }
- }
}
};
}
@@ -225,10 +202,10 @@ impl_event_set!(
);
/// A wrapper around an [`EventSet`] that foregoes safety checks and casting, and passes the pointer as is.
-pub struct Untyped<E>(std::marker::PhantomData<E>);
+pub struct UntypedEvent<E = ()>(std::marker::PhantomData<E>);
/// An [`EventSet`] that matches the specified event type(s), but does not cast the pointer.
-unsafe impl<E: EventSet> EventSet for Untyped<E> {
+unsafe impl<E: EventSet> EventSet for UntypedEvent<E> {
type Item<'trigger> = PtrMut<'trigger>;
type ReadOnlyItem<'trigger> = Ptr<'trigger>;
@@ -257,14 +234,10 @@ unsafe impl<E: EventSet> EventSet for Untyped<E> {
) -> Self::ReadOnlyItem<'short> {
item.as_ref()
}
-
- fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
- item.as_ref()
- }
}
/// An [`EventSet`] that matches any event type.
-unsafe impl EventSet for Untyped<()> {
+unsafe impl EventSet for UntypedEvent<()> {
type Item<'trigger> = PtrMut<'trigger>;
type ReadOnlyItem<'trigger> = Ptr<'trigger>;
@@ -291,8 +264,4 @@ unsafe impl EventSet for Untyped<()> {
) -> Self::ReadOnlyItem<'short> {
item.as_ref()
}
-
- fn shrink_ptr<'long: 'short, 'short>(item: &'short Self::Item<'long>) -> Ptr<'short> {
- item.as_ref()
- }
}
From 9a41de53e7898270585edd607755a23d3c117faf Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Fri, 9 Aug 2024 20:31:48 -0500
Subject: [PATCH 11/34] add safety comments
---
crates/bevy_ecs/src/observer/set.rs | 3 +++
1 file changed, 3 insertions(+)
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index b6a5af362a766..94f231d03bb3a 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -82,6 +82,7 @@ unsafe impl<E: Event> EventSet for E {
}
}
+// SAFETY: Inherits the safety of the inner event type.
unsafe impl<A: EventSet> EventSet for (A,) {
type Item<'trigger> = A::Item<'trigger>;
type ReadOnlyItem<'trigger> = A::ReadOnlyItem<'trigger>;
@@ -205,6 +206,7 @@ impl_event_set!(
pub struct UntypedEvent<E = ()>(std::marker::PhantomData<E>);
/// An [`EventSet`] that matches the specified event type(s), but does not cast the pointer.
+// SAFETY: Performs no unsafe operations, returns the pointer as is.
unsafe impl<E: EventSet> EventSet for UntypedEvent<E> {
type Item<'trigger> = PtrMut<'trigger>;
type ReadOnlyItem<'trigger> = Ptr<'trigger>;
@@ -237,6 +239,7 @@ unsafe impl<E: EventSet> EventSet for UntypedEvent<E> {
}
/// An [`EventSet`] that matches any event type.
+// SAFETY: Performs no unsafe operations, returns the pointer as is.
unsafe impl EventSet for UntypedEvent<()> {
type Item<'trigger> = PtrMut<'trigger>;
type ReadOnlyItem<'trigger> = Ptr<'trigger>;
From a2b65e4affec5688455101378407079beb64c254 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Fri, 9 Aug 2024 20:36:46 -0500
Subject: [PATCH 12/34] remove unused import
---
crates/bevy_ecs/src/observer/mod.rs | 1 -
1 file changed, 1 deletion(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index f2ec328c9376b..6b3ed86323e56 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -12,7 +12,6 @@ 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::marker::PhantomData;
From 954b1b7ab5661f4ec55944c4535e33d48bdbf6f2 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Fri, 9 Aug 2024 21:30:34 -0500
Subject: [PATCH 13/34] fix docs
---
crates/bevy_ecs/src/observer/set.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 94f231d03bb3a..3fc8aea3fbcf1 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -9,9 +9,9 @@ use crate::world::World;
///
/// # Safety
///
-/// Implementor must ensure that [`checked_cast`] and [`init_components`] obey the following:
-/// - Each event type must have a component id registered in [`init_components`].
-/// - [`checked_cast`] must check that the component id matches the event type in order to safely cast a pointer to the output type.
+/// Implementor must ensure that:
+/// - [`EventSet::init_components`] must register a component id for each event type in the set.
+/// - [`EventSet::matches`] must return `true` if and only if the event type is in the set.
///
pub unsafe trait EventSet: 'static {
/// The output type that will be passed to the observer.
From 300090632f5a50a27c5cf73df3a2b8ad41a52a54 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 12:40:52 -0500
Subject: [PATCH 14/34] flip the naming for the with_events so that the unsafe
version is longer
---
crates/bevy_ecs/src/observer/mod.rs | 4 ++--
crates/bevy_ecs/src/observer/runner.rs | 8 +++++---
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 6b3ed86323e56..e223b96b64f7f 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -639,8 +639,8 @@ mod tests {
let on_remove = world.init_component::<OnRemove>();
world.spawn(
Observer::new(|_: Trigger<UntypedEvent, A>, mut res: ResMut<R>| res.0 += 1)
- .with_event_safe(on_add)
- .with_event_safe(on_remove),
+ .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 696aaf9f40be7..3f1aeedcab8c9 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -301,7 +301,7 @@ impl<E: EventSet, 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
}
@@ -314,8 +314,10 @@ where
/// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
/// is triggered.
///
- /// As opposed to [`Observer::with_event`], this method is safe to use because it does not cast any pointers automatically.
- pub fn with_event_safe(mut self, event: ComponentId) -> Self {
+ /// # 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(mut self, event: ComponentId) -> Self {
self.descriptor.events.push(event);
self
}
From 26307ec9422b38a5b9380ce30419fa5c57de1cfc Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 12:41:28 -0500
Subject: [PATCH 15/34] add observer propagation tests for multi-events
---
crates/bevy_ecs/src/observer/mod.rs | 71 +++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index e223b96b64f7f..62c6d216a6bd6 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -1006,4 +1006,75 @@ mod tests {
world.flush();
assert_eq!(2, world.resource::<R>().0);
}
+
+ #[test]
+ fn observer_propagating_multi_event_between() {
+ let mut world = World::new();
+ world.init_resource::<R>();
+ #[derive(Event)]
+ struct Foo;
+
+ let grandparent = world
+ .spawn_empty()
+ .observe(|_: Trigger<(EventPropagating,)>, mut res: ResMut<R>| res.0 += 1)
+ .id();
+ let parent = world
+ .spawn(Parent(grandparent))
+ .observe(|_: Trigger<(EventPropagating, Foo)>, mut res: ResMut<R>| res.0 += 1)
+ .id();
+ let child = world
+ .spawn(Parent(parent))
+ .observe(|_: Trigger<(EventPropagating,)>, mut res: ResMut<R>| res.0 += 1)
+ .id();
+
+ // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
+ // and therefore does not automatically flush.
+ world.flush();
+ world.trigger_targets(EventPropagating, child);
+ world.flush();
+ assert_eq!(3, world.resource::<R>().0);
+ }
+
+ #[test]
+ fn observer_propagating_multi_event_all() {
+ let mut world = World::new();
+ world.init_resource::<R>();
+ #[derive(Component)]
+ struct OtherPropagating;
+
+ impl Event for OtherPropagating {
+ type Traversal = Parent;
+
+ const AUTO_PROPAGATE: bool = true;
+ }
+
+ let grandparent = world
+ .spawn_empty()
+ .observe(
+ |_: Trigger<(EventPropagating, OtherPropagating)>, mut res: ResMut<R>| res.0 += 1,
+ )
+ .id();
+ let parent = world
+ .spawn(Parent(grandparent))
+ .observe(
+ |_: Trigger<(EventPropagating, OtherPropagating)>, mut res: ResMut<R>| res.0 += 1,
+ )
+ .id();
+ let child = world
+ .spawn(Parent(parent))
+ .observe(
+ |_: Trigger<(EventPropagating, OtherPropagating)>, mut res: ResMut<R>| res.0 += 1,
+ )
+ .id();
+
+ // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
+ // and therefore does not automatically flush.
+ world.flush();
+ world.trigger_targets(EventPropagating, child);
+ world.flush();
+ assert_eq!(3, world.resource::<R>().0);
+ world.trigger_targets(OtherPropagating, child);
+ world.flush();
+ assert_eq!(6, world.resource::<R>().0);
+ }
}
From 7e32e5c4ed4e66e286ccd695ae1bfc056ea2b4b5 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 13:10:02 -0500
Subject: [PATCH 16/34] add additional check to propagating between test
---
crates/bevy_ecs/src/observer/mod.rs | 3 +++
1 file changed, 3 insertions(+)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 62c6d216a6bd6..085972a00bbf7 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -1033,6 +1033,9 @@ mod tests {
world.trigger_targets(EventPropagating, child);
world.flush();
assert_eq!(3, world.resource::<R>().0);
+ world.trigger_targets(Foo, parent);
+ world.flush();
+ assert_eq!(4, world.resource::<R>().0);
}
#[test]
From b0519ed1ebdf9a868c19d1a337df248a4e01ef24 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 13:10:36 -0500
Subject: [PATCH 17/34] rename untyped tests to be more clear, and change todo
comment
---
crates/bevy_ecs/src/observer/mod.rs | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 085972a00bbf7..070687f6a22b8 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -612,14 +612,16 @@ mod tests {
Or2::B(Bar(v)) => assert!(*v),
}
});
- world.flush(); // TODO: should we auto-flush after observe?
+ // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
+ // and therefore does not automatically flush.
+ world.flush();
world.trigger(Foo(5));
world.trigger(Bar(true));
assert_eq!(2, world.resource::<R>().0);
}
#[test]
- fn observer_multiple_events_untyped() {
+ fn observer_multiple_events_static_untyped() {
let mut world = World::new();
world.init_resource::<R>();
world.spawn(Observer::new(
@@ -632,7 +634,7 @@ mod tests {
}
#[test]
- fn observer_multiple_events_unsafe() {
+ fn observer_multiple_events_dynamic_untyped() {
let mut world = World::new();
world.init_resource::<R>();
let on_add = world.init_component::<OnAdd>();
From 195de902b857ce19c75d5859e1e430e3b422a1b2 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 15:57:58 -0500
Subject: [PATCH 18/34] clarify safety comment and add macro generated impls up
to 15
---
crates/bevy_ecs/src/observer/set.rs | 109 +++++++++++++++++++++++++++-
1 file changed, 107 insertions(+), 2 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 3fc8aea3fbcf1..1711a48914298 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -82,7 +82,7 @@ unsafe impl<E: Event> EventSet for E {
}
}
-// SAFETY: Inherits the safety of the inner event type.
+// SAFETY: Forwards to the inner event type, and inherits its safety properties.
unsafe impl<A: EventSet> EventSet for (A,) {
type Item<'trigger> = A::Item<'trigger>;
type ReadOnlyItem<'trigger> = A::ReadOnlyItem<'trigger>;
@@ -201,6 +201,111 @@ impl_event_set!(
(G, g),
(H, h)
);
+impl_event_set!(
+ Or9,
+ (A, a),
+ (B, b),
+ (C, c),
+ (D, d),
+ (E, e),
+ (F, f),
+ (G, g),
+ (H, h),
+ (I, i)
+);
+impl_event_set!(
+ Or10,
+ (A, a),
+ (B, b),
+ (C, c),
+ (D, d),
+ (E, e),
+ (F, f),
+ (G, g),
+ (H, h),
+ (I, i),
+ (J, j)
+);
+impl_event_set!(
+ Or11,
+ (A, a),
+ (B, b),
+ (C, c),
+ (D, d),
+ (E, e),
+ (F, f),
+ (G, g),
+ (H, h),
+ (I, i),
+ (J, j),
+ (K, k)
+);
+impl_event_set!(
+ Or12,
+ (A, a),
+ (B, b),
+ (C, c),
+ (D, d),
+ (E, e),
+ (F, f),
+ (G, g),
+ (H, h),
+ (I, i),
+ (J, j),
+ (K, k),
+ (L, l)
+);
+impl_event_set!(
+ Or13,
+ (A, a),
+ (B, b),
+ (C, c),
+ (D, d),
+ (E, e),
+ (F, f),
+ (G, g),
+ (H, h),
+ (I, i),
+ (J, j),
+ (K, k),
+ (L, l),
+ (M, m)
+);
+impl_event_set!(
+ Or14,
+ (A, a),
+ (B, b),
+ (C, c),
+ (D, d),
+ (E, e),
+ (F, f),
+ (G, g),
+ (H, h),
+ (I, i),
+ (J, j),
+ (K, k),
+ (L, l),
+ (M, m),
+ (N, n)
+);
+impl_event_set!(
+ Or15,
+ (A, a),
+ (B, b),
+ (C, c),
+ (D, d),
+ (E, e),
+ (F, f),
+ (G, g),
+ (H, h),
+ (I, i),
+ (J, j),
+ (K, k),
+ (L, l),
+ (M, m),
+ (N, n),
+ (O, o)
+);
/// A wrapper around an [`EventSet`] that foregoes safety checks and casting, and passes the pointer as is.
pub struct UntypedEvent<E = ()>(std::marker::PhantomData<E>);
@@ -238,7 +343,7 @@ unsafe impl<E: EventSet> EventSet for UntypedEvent<E> {
}
}
-/// An [`EventSet`] that matches any event type.
+/// An [`EventSet`] that matches any event type, but does not cast the pointer.
// SAFETY: Performs no unsafe operations, returns the pointer as is.
unsafe impl EventSet for UntypedEvent<()> {
type Item<'trigger> = PtrMut<'trigger>;
From 294fd03479224e60e0a48f8415be2cb4e93f308b Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 15:58:40 -0500
Subject: [PATCH 19/34] add benchmarks
---
benches/benches/bevy_ecs/observers/mod.rs | 12 +-
.../benches/bevy_ecs/observers/multievent.rs | 211 ++++++++++++++++++
benches/benches/bevy_ecs/observers/untyped.rs | 45 ++++
3 files changed, 267 insertions(+), 1 deletion(-)
create mode 100644 benches/benches/bevy_ecs/observers/multievent.rs
create mode 100644 benches/benches/bevy_ecs/observers/untyped.rs
diff --git a/benches/benches/bevy_ecs/observers/mod.rs b/benches/benches/bevy_ecs/observers/mod.rs
index 0b8c3f24869ce..3f12158bd880c 100644
--- a/benches/benches/bevy_ecs/observers/mod.rs
+++ b/benches/benches/bevy_ecs/observers/mod.rs
@@ -1,8 +1,18 @@
use criterion::criterion_group;
+mod multievent;
mod propagation;
mod simple;
+mod untyped;
+use multievent::*;
use propagation::*;
use simple::*;
+use untyped::*;
-criterion_group!(observer_benches, event_propagation, observe_simple);
+criterion_group!(
+ observer_benches,
+ event_propagation,
+ observe_simple,
+ observe_multievent,
+ observe_untyped
+);
diff --git a/benches/benches/bevy_ecs/observers/multievent.rs b/benches/benches/bevy_ecs/observers/multievent.rs
new file mode 100644
index 0000000000000..379abfc818138
--- /dev/null
+++ b/benches/benches/bevy_ecs/observers/multievent.rs
@@ -0,0 +1,211 @@
+use bevy_ecs::{
+ component::Component,
+ event::Event,
+ observer::{EventSet, Observer, Trigger},
+ world::World,
+};
+use criterion::{black_box, measurement::WallTime, BenchmarkGroup, Criterion};
+
+pub fn observe_multievent(criterion: &mut Criterion) {
+ let mut group = criterion.benchmark_group("observe_multievent");
+ group.warm_up_time(std::time::Duration::from_millis(500));
+ group.measurement_time(std::time::Duration::from_secs(4));
+
+ bench_in_set::<1, (TestEvent<1>,)>(&mut group);
+ bench_in_set::<2, (TestEvent<1>, TestEvent<2>)>(&mut group);
+ bench_in_set::<3, (TestEvent<1>, TestEvent<2>, TestEvent<3>)>(&mut group);
+ bench_in_set::<4, (TestEvent<1>, TestEvent<2>, TestEvent<3>, TestEvent<4>)>(&mut group);
+ bench_in_set::<
+ 5,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ ),
+ >(&mut group);
+ bench_in_set::<
+ 6,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ TestEvent<6>,
+ ),
+ >(&mut group);
+ bench_in_set::<
+ 7,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ TestEvent<6>,
+ TestEvent<7>,
+ ),
+ >(&mut group);
+ bench_in_set::<
+ 8,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ TestEvent<6>,
+ TestEvent<7>,
+ TestEvent<8>,
+ ),
+ >(&mut group);
+ bench_in_set::<
+ 9,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ TestEvent<6>,
+ TestEvent<7>,
+ TestEvent<8>,
+ TestEvent<9>,
+ ),
+ >(&mut group);
+ bench_in_set::<
+ 10,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ TestEvent<6>,
+ TestEvent<7>,
+ TestEvent<8>,
+ TestEvent<9>,
+ TestEvent<10>,
+ ),
+ >(&mut group);
+ bench_in_set::<
+ 11,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ TestEvent<6>,
+ TestEvent<7>,
+ TestEvent<8>,
+ TestEvent<9>,
+ TestEvent<10>,
+ TestEvent<11>,
+ ),
+ >(&mut group);
+ bench_in_set::<
+ 12,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ TestEvent<6>,
+ TestEvent<7>,
+ TestEvent<8>,
+ TestEvent<9>,
+ TestEvent<10>,
+ TestEvent<11>,
+ TestEvent<12>,
+ ),
+ >(&mut group);
+ bench_in_set::<
+ 13,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ TestEvent<6>,
+ TestEvent<7>,
+ TestEvent<8>,
+ TestEvent<9>,
+ TestEvent<10>,
+ TestEvent<11>,
+ TestEvent<12>,
+ TestEvent<13>,
+ ),
+ >(&mut group);
+ bench_in_set::<
+ 14,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ TestEvent<6>,
+ TestEvent<7>,
+ TestEvent<8>,
+ TestEvent<9>,
+ TestEvent<10>,
+ TestEvent<11>,
+ TestEvent<12>,
+ TestEvent<13>,
+ TestEvent<14>,
+ ),
+ >(&mut group);
+ bench_in_set::<
+ 15,
+ (
+ TestEvent<1>,
+ TestEvent<2>,
+ TestEvent<3>,
+ TestEvent<4>,
+ TestEvent<5>,
+ TestEvent<6>,
+ TestEvent<7>,
+ TestEvent<8>,
+ TestEvent<9>,
+ TestEvent<10>,
+ TestEvent<11>,
+ TestEvent<12>,
+ TestEvent<13>,
+ TestEvent<14>,
+ TestEvent<15>,
+ ),
+ >(&mut group);
+}
+
+fn bench_in_set<const LAST: usize, Set: EventSet>(group: &mut BenchmarkGroup<WallTime>) {
+ group.bench_function(format!("trigger_first/{LAST}"), |bencher| {
+ let mut world = World::new();
+ world.observe(empty_listener_set::<Set>);
+ bencher.iter(|| {
+ for _ in 0..10000 {
+ world.trigger(TestEvent::<1>);
+ }
+ });
+ });
+ group.bench_function(format!("trigger_last/{LAST}"), |bencher| {
+ let mut world = World::new();
+ world.observe(empty_listener_set::<Set>);
+ bencher.iter(|| {
+ for _ in 0..10000 {
+ world.trigger(TestEvent::<LAST>);
+ }
+ });
+ });
+}
+
+#[derive(Event)]
+struct TestEvent<const N: usize>;
+
+fn empty_listener_set<Set: EventSet>(trigger: Trigger<Set>) {
+ black_box(trigger);
+}
diff --git a/benches/benches/bevy_ecs/observers/untyped.rs b/benches/benches/bevy_ecs/observers/untyped.rs
new file mode 100644
index 0000000000000..d4b25d11da632
--- /dev/null
+++ b/benches/benches/bevy_ecs/observers/untyped.rs
@@ -0,0 +1,45 @@
+use bevy_ecs::{
+ event::Event,
+ observer::{EventSet, Observer, Trigger, UntypedEvent},
+ world::World,
+};
+use criterion::{black_box, Criterion};
+
+pub fn observe_untyped(criterion: &mut Criterion) {
+ let mut group = criterion.benchmark_group("observe_untyped");
+ 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::<UntypedEvent>).with_event(event_id_1));
+ bencher.iter(|| {
+ for _ in 0..10000 {
+ world.trigger(TestEvent::<1>);
+ }
+ });
+ });
+ 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::<UntypedEvent>)
+ .with_event(event_id_1)
+ .with_event(event_id_2),
+ );
+ bencher.iter(|| {
+ for _ in 0..10000 {
+ world.trigger(TestEvent::<2>);
+ }
+ });
+ });
+}
+
+#[derive(Event)]
+struct TestEvent<const N: usize>;
+
+fn empty_listener_set<Set: EventSet>(trigger: Trigger<Set>) {
+ black_box(trigger);
+}
From 6434cd874b5f38cde571f81fad4fef6c4a568f49 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 16:23:48 -0500
Subject: [PATCH 20/34] log an error when the user incorrectly used
with_event_unchecked
---
crates/bevy_ecs/src/observer/runner.rs | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index 3f1aeedcab8c9..8bfb6dbfb75e1 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -404,6 +404,10 @@ fn observer_system_runner<E: EventSet, B: Bundle>(
let world_ref = unsafe { world.world() };
// SAFETY: Observer was triggered with an event in the set of events it observes, so it must be convertible to E
let Some(event) = (unsafe { E::unchecked_cast(world_ref, &observer_trigger, ptr) }) else {
+ // This branch is only ever hit if the user called Observer::with_event_unchecked with a component ID not matching the event set E,
+ // EXCEPT when the event set is a singular event type, in which case the event will always match.
+ // This is a user error and should be logged.
+ bevy_utils::tracing::error!("Observer was triggered with an event that does not match the event set. Did you call Observer::with_event_unchecked with the wrong ID? Id: {:?} Set: {:?}", observer_trigger.event_type, std::any::type_name::<E>());
return;
};
From a7cc2c69d9fd9b3da5d4cbf22b916056a4fc22f8 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 16:24:31 -0500
Subject: [PATCH 21/34] Make Observer::with_event call with_event_unchecked
internally, to make it more obvious that it does the same thing
---
crates/bevy_ecs/src/observer/runner.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index 8bfb6dbfb75e1..b88c4202d03ed 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -317,9 +317,9 @@ where
/// # 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(mut self, event: ComponentId) -> Self {
- self.descriptor.events.push(event);
- self
+ pub fn with_event(self, event: ComponentId) -> Self {
+ // SAFETY: UntypedEvent<E> itself does not perform any unsafe operations (like casting), so this is safe.
+ unsafe { self.with_event_unchecked(event) }
}
}
From a5d6f0459fc85d7b284e24f33df1a7d67417076a Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 17:18:28 -0500
Subject: [PATCH 22/34] fix up error message
---
crates/bevy_ecs/src/observer/runner.rs | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index b88c4202d03ed..a711e8fcec245 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -407,7 +407,14 @@ fn observer_system_runner<E: EventSet, B: Bundle>(
// This branch is only ever hit if the user called Observer::with_event_unchecked with a component ID not matching the event set E,
// EXCEPT when the event set is a singular event type, in which case the event will always match.
// This is a user error and should be logged.
- bevy_utils::tracing::error!("Observer was triggered with an event that does not match the event set. Did you call Observer::with_event_unchecked with the wrong ID? Id: {:?} Set: {:?}", observer_trigger.event_type, std::any::type_name::<E>());
+ bevy_utils::tracing::error!(
+ "Observer was triggered with an event that does not match the event set. \
+ Did you call Observer::with_event_unchecked with the wrong ID? \
+ Observer: {:?} Event: {:?} Set: {:?}",
+ observer_trigger.observer,
+ observer_trigger.event_type,
+ std::any::type_name::<E>()
+ );
return;
};
From 5c8c51622a2da575647d7c95065f7e0689fcab84 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 17:30:30 -0500
Subject: [PATCH 23/34] fix safety comment
---
crates/bevy_ecs/src/observer/set.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 1711a48914298..ec95ffbb38c0e 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -126,7 +126,7 @@ macro_rules! impl_event_set {
}
// SAFETY: All event types have a component id registered in `init_components`,
- // and `cast` calls `matches` before casting the pointer to one of the event types.
+ // and `unchecked_cast` calls `matches` before casting to one of the inner event sets.
unsafe impl<$($P: EventSet),*> EventSet for ($($P,)*) {
type Item<'trigger> = $Or<$($P::Item<'trigger>),*>;
type ReadOnlyItem<'trigger> = $Or<$($P::ReadOnlyItem<'trigger>),*>;
From 63790773120d1a096906b2d29c31acc628439b75 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 18:35:41 -0500
Subject: [PATCH 24/34] Both UntypedEvent implementations should match all
event types
---
crates/bevy_ecs/src/observer/set.rs | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index ec95ffbb38c0e..697daf83b77c3 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -308,6 +308,7 @@ impl_event_set!(
);
/// A wrapper around an [`EventSet`] that foregoes safety checks and casting, and passes the pointer as is.
+/// This is useful for observers that do not need to access the event data, or need to do so dynamically.
pub struct UntypedEvent<E = ()>(std::marker::PhantomData<E>);
/// An [`EventSet`] that matches the specified event type(s), but does not cast the pointer.
@@ -325,7 +326,7 @@ unsafe impl<E: EventSet> EventSet for UntypedEvent<E> {
}
fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool {
- E::matches(world, observer_trigger)
+ true
}
fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
From bdd6c33433e182f147a68059b5a25bf252a4cb58 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 19:15:33 -0500
Subject: [PATCH 25/34] fix lints
---
crates/bevy_ecs/src/observer/set.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 697daf83b77c3..32e62b765fbc9 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -325,7 +325,7 @@ unsafe impl<E: EventSet> EventSet for UntypedEvent<E> {
Some(ptr)
}
- fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool {
+ fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
true
}
From bd139ff4c6384118373f885c8d87b71025aaebb3 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 19:19:29 -0500
Subject: [PATCH 26/34] add ability to do mixed statically-known and
runtime-known observers safely
---
crates/bevy_ecs/src/observer/mod.rs | 19 ++-
crates/bevy_ecs/src/observer/runner.rs | 7 +-
crates/bevy_ecs/src/observer/set.rs | 157 +++++++++++++++++++++++++
3 files changed, 176 insertions(+), 7 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 070687f6a22b8..f9b6a300c1267 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -621,7 +621,7 @@ mod tests {
}
#[test]
- fn observer_multiple_events_static_untyped() {
+ fn observer_multiple_events_untyped_autoregister_static() {
let mut world = World::new();
world.init_resource::<R>();
world.spawn(Observer::new(
@@ -634,7 +634,7 @@ mod tests {
}
#[test]
- fn observer_multiple_events_dynamic_untyped() {
+ fn observer_multiple_events_untyped() {
let mut world = World::new();
world.init_resource::<R>();
let on_add = world.init_component::<OnAdd>();
@@ -650,6 +650,21 @@ mod tests {
assert_eq!(2, world.resource::<R>().0);
}
+ #[test]
+ fn observer_multiple_events_mixed() {
+ let mut world = World::new();
+ world.init_resource::<R>();
+ let on_remove = world.init_component::<OnRemove>();
+ world.spawn(
+ Observer::new(|_: Trigger<(OnAdd, UntypedEvent), A>, mut res: ResMut<R>| res.0 += 1)
+ .with_event(on_remove),
+ );
+
+ let entity = world.spawn(A).id();
+ world.despawn(entity);
+ assert_eq!(2, world.resource::<R>().0);
+ }
+
#[test]
fn observer_multiple_components() {
let mut world = World::new();
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index a711e8fcec245..28a339032f623 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::{EventSet, ObserverDescriptor, ObserverTrigger, UntypedEvent},
+ observer::{DynamicEventSafe, EventSet, ObserverDescriptor, ObserverTrigger},
prelude::*,
query::DebugCheckedUnwrap,
system::{IntoObserverSystem, ObserverSystem},
@@ -307,10 +307,7 @@ impl<E: EventSet, B: Bundle> Observer<E, B> {
}
}
-impl<E, B: Bundle> Observer<UntypedEvent<E>, B>
-where
- UntypedEvent<E>: EventSet,
-{
+impl<E: DynamicEventSafe, B: Bundle> Observer<E, B> {
/// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
/// is triggered.
///
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 32e62b765fbc9..d7d4542543f53 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -374,3 +374,160 @@ unsafe impl EventSet for UntypedEvent<()> {
item.as_ref()
}
}
+
+/// This trait is implemented on [`EventSet`]s where it is safe to call
+/// [`Observer::with_event`](crate::observer::Observer::with_event).
+pub unsafe trait DynamicEventSafe: EventSet {}
+
+unsafe impl<E: EventSet> DynamicEventSafe for UntypedEvent<E> {}
+unsafe impl DynamicEventSafe for UntypedEvent<()> {}
+unsafe impl<A: DynamicEventSafe> DynamicEventSafe for (A,) {}
+unsafe impl<A: EventSet, B: DynamicEventSafe> DynamicEventSafe for (A, B) {}
+unsafe impl<A: EventSet, B: EventSet, C: DynamicEventSafe> DynamicEventSafe for (A, B, C) {}
+unsafe impl<A: EventSet, B: EventSet, C: EventSet, D: DynamicEventSafe> DynamicEventSafe
+ for (A, B, C, D)
+{
+}
+unsafe impl<A: EventSet, B: EventSet, C: EventSet, D: EventSet, E: DynamicEventSafe>
+ DynamicEventSafe for (A, B, C, D, E)
+{
+}
+unsafe impl<A: EventSet, B: EventSet, C: EventSet, D: EventSet, E: EventSet, F: DynamicEventSafe>
+ DynamicEventSafe for (A, B, C, D, E, F)
+{
+}
+unsafe impl<
+ A: EventSet,
+ B: EventSet,
+ C: EventSet,
+ D: EventSet,
+ E: EventSet,
+ F: EventSet,
+ G: DynamicEventSafe,
+ > DynamicEventSafe for (A, B, C, D, E, F, G)
+{
+}
+unsafe impl<
+ A: EventSet,
+ B: EventSet,
+ C: EventSet,
+ D: EventSet,
+ E: EventSet,
+ F: EventSet,
+ G: EventSet,
+ H: DynamicEventSafe,
+ > DynamicEventSafe for (A, B, C, D, E, F, G, H)
+{
+}
+unsafe impl<
+ A: EventSet,
+ B: EventSet,
+ C: EventSet,
+ D: EventSet,
+ E: EventSet,
+ F: EventSet,
+ G: EventSet,
+ H: EventSet,
+ I: DynamicEventSafe,
+ > DynamicEventSafe for (A, B, C, D, E, F, G, H, I)
+{
+}
+unsafe impl<
+ A: EventSet,
+ B: EventSet,
+ C: EventSet,
+ D: EventSet,
+ E: EventSet,
+ F: EventSet,
+ G: EventSet,
+ H: EventSet,
+ I: EventSet,
+ J: DynamicEventSafe,
+ > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J)
+{
+}
+unsafe impl<
+ A: EventSet,
+ B: EventSet,
+ C: EventSet,
+ D: EventSet,
+ E: EventSet,
+ F: EventSet,
+ G: EventSet,
+ H: EventSet,
+ I: EventSet,
+ J: EventSet,
+ K: DynamicEventSafe,
+ > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J, K)
+{
+}
+unsafe impl<
+ A: EventSet,
+ B: EventSet,
+ C: EventSet,
+ D: EventSet,
+ E: EventSet,
+ F: EventSet,
+ G: EventSet,
+ H: EventSet,
+ I: EventSet,
+ J: EventSet,
+ K: EventSet,
+ L: DynamicEventSafe,
+ > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J, K, L)
+{
+}
+unsafe impl<
+ A: EventSet,
+ B: EventSet,
+ C: EventSet,
+ D: EventSet,
+ E: EventSet,
+ F: EventSet,
+ G: EventSet,
+ H: EventSet,
+ I: EventSet,
+ J: EventSet,
+ K: EventSet,
+ L: EventSet,
+ M: DynamicEventSafe,
+ > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J, K, L, M)
+{
+}
+unsafe impl<
+ A: EventSet,
+ B: EventSet,
+ C: EventSet,
+ D: EventSet,
+ E: EventSet,
+ F: EventSet,
+ G: EventSet,
+ H: EventSet,
+ I: EventSet,
+ J: EventSet,
+ K: EventSet,
+ L: EventSet,
+ M: EventSet,
+ N: DynamicEventSafe,
+ > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J, K, L, M, N)
+{
+}
+unsafe impl<
+ A: EventSet,
+ B: EventSet,
+ C: EventSet,
+ D: EventSet,
+ E: EventSet,
+ F: EventSet,
+ G: EventSet,
+ H: EventSet,
+ I: EventSet,
+ J: EventSet,
+ K: EventSet,
+ L: EventSet,
+ M: EventSet,
+ N: EventSet,
+ O: DynamicEventSafe,
+ > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J, K, L, M, N, O)
+{
+}
From 2af5feffe7e5288799883110d4f0b82b62dab15b Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sat, 10 Aug 2024 23:33:16 -0500
Subject: [PATCH 27/34] remove footgun by forbiding the nesting of DynamicEvent
or SemiDynamicEvent
---
crates/bevy_ecs/src/observer/mod.rs | 45 +-
crates/bevy_ecs/src/observer/runner.rs | 29 +-
crates/bevy_ecs/src/observer/set.rs | 569 +++++++++----------------
3 files changed, 270 insertions(+), 373 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index f9b6a300c1267..21bd2ac2e6ca5 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -429,8 +429,8 @@ mod tests {
use crate as bevy_ecs;
use crate::observer::{
- EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace, Or2,
- UntypedEvent,
+ DynamicEvent, EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace,
+ Or2, SemiDynamicEvent,
};
use crate::prelude::*;
use crate::traversal::Traversal;
@@ -598,7 +598,7 @@ mod tests {
}
#[test]
- fn observer_multiple_events() {
+ fn observer_multiple_events_static() {
let mut world = World::new();
world.init_resource::<R>();
#[derive(Event)]
@@ -621,11 +621,11 @@ mod tests {
}
#[test]
- fn observer_multiple_events_untyped_autoregister_static() {
+ fn observer_multiple_events_dynamic_autoregister() {
let mut world = World::new();
world.init_resource::<R>();
world.spawn(Observer::new(
- |_: Trigger<UntypedEvent<(OnAdd, OnRemove)>, A>, mut res: ResMut<R>| res.0 += 1,
+ |_: Trigger<DynamicEvent<(OnAdd, OnRemove)>, A>, mut res: ResMut<R>| res.0 += 1,
));
let entity = world.spawn(A).id();
@@ -634,13 +634,13 @@ mod tests {
}
#[test]
- fn observer_multiple_events_untyped() {
+ fn observer_multiple_events_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(
- Observer::new(|_: Trigger<UntypedEvent, A>, mut res: ResMut<R>| res.0 += 1)
+ Observer::new(|_: Trigger<DynamicEvent, A>, mut res: ResMut<R>| res.0 += 1)
.with_event(on_add)
.with_event(on_remove),
);
@@ -651,13 +651,20 @@ mod tests {
}
#[test]
- fn observer_multiple_events_mixed() {
+ fn observer_multiple_events_semidynamic() {
let mut world = World::new();
world.init_resource::<R>();
let on_remove = world.init_component::<OnRemove>();
world.spawn(
- Observer::new(|_: Trigger<(OnAdd, UntypedEvent), A>, mut res: ResMut<R>| res.0 += 1)
- .with_event(on_remove),
+ Observer::new(
+ |trigger: Trigger<SemiDynamicEvent<OnAdd>, A>, mut res: ResMut<R>| {
+ match trigger.event() {
+ Ok(_onadd) => res.assert_order(0),
+ Err(_ptr) => res.assert_order(1),
+ };
+ },
+ )
+ .with_event(on_remove),
);
let entity = world.spawn(A).id();
@@ -665,6 +672,24 @@ mod tests {
assert_eq!(2, world.resource::<R>().0);
}
+ #[test]
+ fn observer_multiple_events_semidynamic_autoregister() {
+ let mut world = World::new();
+ world.init_resource::<R>();
+ world.spawn(Observer::new(
+ |trigger: Trigger<SemiDynamicEvent<OnAdd, OnRemove>, A>, mut res: ResMut<R>| {
+ match trigger.event() {
+ Ok(_onadd) => res.assert_order(0),
+ Err(_ptr) => res.assert_order(1),
+ };
+ },
+ ));
+
+ let entity = world.spawn(A).id();
+ world.despawn(entity);
+ assert_eq!(2, world.resource::<R>().0);
+ }
+
#[test]
fn observer_multiple_components() {
let mut world = World::new();
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index 28a339032f623..44f3f91813ce7 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -1,6 +1,9 @@
use crate::{
component::{ComponentHooks, ComponentId, StorageType},
- observer::{DynamicEventSafe, EventSet, ObserverDescriptor, ObserverTrigger},
+ observer::{
+ DynamicEvent, EventSet, ObserverDescriptor, ObserverTrigger, SemiDynamicEvent,
+ StaticEventSet,
+ },
prelude::*,
query::DebugCheckedUnwrap,
system::{IntoObserverSystem, ObserverSystem},
@@ -307,7 +310,26 @@ impl<E: EventSet, B: Bundle> Observer<E, B> {
}
}
-impl<E: DynamicEventSafe, B: Bundle> Observer<E, B> {
+impl<Register, B: Bundle> Observer<DynamicEvent<Register>, B>
+where
+ DynamicEvent<Register>: EventSet,
+{
+ /// 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: UntypedEvent<E> itself does not perform any unsafe operations (like casting), so this is safe.
+ unsafe { self.with_event_unchecked(event) }
+ }
+}
+
+impl<Static: StaticEventSet, Register, B: Bundle> Observer<SemiDynamicEvent<Static, Register>, B>
+where
+ SemiDynamicEvent<Static, Register>: EventSet,
+{
/// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
/// is triggered.
///
@@ -400,7 +422,8 @@ fn observer_system_runner<E: EventSet, B: Bundle>(
// 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 in the set of events it observes, so it must be convertible to E
- let Some(event) = (unsafe { E::unchecked_cast(world_ref, &observer_trigger, ptr) }) else {
+ // We choose to use unchecked_cast over the safe cast method to avoid the overhead of matching in the single event type case (which is the common case).
+ let Ok(event) = (unsafe { E::unchecked_cast(world_ref, &observer_trigger, ptr) }) else {
// This branch is only ever hit if the user called Observer::with_event_unchecked with a component ID not matching the event set E,
// EXCEPT when the event set is a singular event type, in which case the event will always match.
// This is a user error and should be logged.
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index d7d4542543f53..b227779a72604 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -11,7 +11,7 @@ use crate::world::World;
///
/// Implementor must ensure that:
/// - [`EventSet::init_components`] must register a component id for each event type in the set.
-/// - [`EventSet::matches`] must return `true` if and only if the event type is in the set.
+/// - [`EventSet::matches`] must return `true` if the event type is in the set, or if the set matches any event type.
///
pub unsafe trait EventSet: 'static {
/// The output type that will be passed to the observer.
@@ -19,16 +19,30 @@ pub unsafe trait EventSet: 'static {
/// The read-only variant of the output type.
type ReadOnlyItem<'trigger>: Copy;
+ /// Safely casts a pointer to the output type, checking if the event type matches this event set.
+ fn cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
+ if Self::matches(world, observer_trigger) {
+ // SAFETY: We have checked that the event component id matches the event type
+ unsafe { Self::unchecked_cast(world, observer_trigger, ptr) }
+ } else {
+ Err(ptr)
+ }
+ }
+
/// Casts a pointer to the output type.
///
/// # Safety
///
- /// Caller must ensure that the component id [`EventSet::matches`] this event set before calling this function.
+ /// Caller must ensure that the event component id [`matches`](EventSet::matches) this event set before calling this function.
unsafe fn unchecked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
- ) -> Option<Self::Item<'trigger>>;
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>>;
/// Checks if the event type matches the observer trigger.
fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool;
@@ -45,8 +59,17 @@ pub unsafe trait EventSet: 'static {
) -> Self::ReadOnlyItem<'short>;
}
+/// An [`EventSet`] that matches a statically pre-defined set of event types.
+///
+/// # Safety
+///
+/// Implementors must ensure that [`matches`](EventSet::matches)
+/// returns `true` if and only if the event component id matches the event type,
+/// AND does not match any other event type.
+pub unsafe trait StaticEventSet: EventSet {}
+
// SAFETY: The event type has a component id registered in `init_components`,
-// and `matches` checks that the component id matches the event type.
+// and `matches` checks that the event component id matches the event type.
unsafe impl<E: Event> EventSet for E {
type Item<'trigger> = &'trigger mut E;
type ReadOnlyItem<'trigger> = &'trigger E;
@@ -55,9 +78,9 @@ unsafe impl<E: Event> EventSet for E {
_world: &World,
_observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
- ) -> Option<Self::Item<'trigger>> {
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
// SAFETY: Caller must ensure that the component id matches the event type
- Some(unsafe { ptr.deref_mut() })
+ Ok(unsafe { ptr.deref_mut() })
}
fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool {
@@ -82,16 +105,160 @@ unsafe impl<E: Event> EventSet for E {
}
}
+// SAFETY: The event type is a statically known type.
+unsafe impl<E: Event> StaticEventSet for E {}
+
+/// An [`EventSet`] that matches any event type, but does not cast the pointer. 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.
+pub struct DynamicEvent<Register = ()>(std::marker::PhantomData<Register>);
+
+// SAFETY: Performs no unsafe operations, returns the pointer as is.
+unsafe impl<Register: StaticEventSet> EventSet for DynamicEvent<Register> {
+ type Item<'trigger> = PtrMut<'trigger>;
+ type ReadOnlyItem<'trigger> = Ptr<'trigger>;
+
+ fn cast<'trigger>(
+ _world: &World,
+ _observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
+ Ok(ptr)
+ }
+
+ unsafe fn unchecked_cast<'trigger>(
+ _world: &World,
+ _observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
+ Ok(ptr)
+ }
+
+ fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
+ true
+ }
+
+ fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
+ Register::init_components(world, ids);
+ }
+
+ 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()
+ }
+}
+
+// SAFETY: Performs no unsafe operations, returns the pointer as is.
+unsafe impl EventSet for DynamicEvent<()> {
+ type Item<'trigger> = PtrMut<'trigger>;
+ type ReadOnlyItem<'trigger> = Ptr<'trigger>;
+
+ fn cast<'trigger>(
+ _world: &World,
+ _observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
+ Ok(ptr)
+ }
+
+ unsafe fn unchecked_cast<'trigger>(
+ _world: &World,
+ _observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
+ Ok(ptr)
+ }
+
+ fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
+ true
+ }
+
+ 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()
+ }
+}
+
+/// An [`EventSet`] that matches a statically pre-defined set of event types and casts the pointer to the event type,
+/// or returns the pointer as is if the event type does not match.
+pub struct SemiDynamicEvent<Static: StaticEventSet, Register = ()>(
+ std::marker::PhantomData<(Static, Register)>,
+);
+
+// SAFETY: No unsafe operations are performed. The checked cast variant is used for the static event type.
+unsafe impl<Static: StaticEventSet, Register> EventSet for SemiDynamicEvent<Static, Register>
+where
+ DynamicEvent<Register>: EventSet,
+{
+ type Item<'trigger> = Result<Static::Item<'trigger>, PtrMut<'trigger>>;
+ type ReadOnlyItem<'trigger> = Result<Static::ReadOnlyItem<'trigger>, Ptr<'trigger>>;
+
+ unsafe fn unchecked_cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
+ match Static::cast(world, observer_trigger, ptr) {
+ Ok(item) => Ok(Ok(item)),
+ Err(ptr) => Ok(Err(ptr)),
+ }
+ }
+
+ fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
+ true
+ }
+
+ fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
+ Static::init_components(world, &mut ids);
+ DynamicEvent::<Register>::init_components(world, ids);
+ }
+
+ fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {
+ match item {
+ Ok(item) => Ok(Static::shrink(item)),
+ Err(ptr) => Err(ptr.reborrow()),
+ }
+ }
+
+ fn shrink_readonly<'long: 'short, 'short>(
+ item: &'short Self::Item<'long>,
+ ) -> Self::ReadOnlyItem<'short> {
+ match item {
+ Ok(item) => Ok(Static::shrink_readonly(item)),
+ Err(ptr) => Err(ptr.as_ref()),
+ }
+ }
+}
+
// SAFETY: Forwards to the inner event type, and inherits its safety properties.
-unsafe impl<A: EventSet> EventSet for (A,) {
+unsafe impl<A: StaticEventSet> EventSet for (A,) {
type Item<'trigger> = A::Item<'trigger>;
type ReadOnlyItem<'trigger> = A::ReadOnlyItem<'trigger>;
+ fn cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
+ A::cast(world, observer_trigger, ptr)
+ }
+
unsafe fn unchecked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
- ) -> Option<Self::Item<'trigger>> {
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
A::unchecked_cast(world, observer_trigger, ptr)
}
@@ -114,6 +281,8 @@ unsafe impl<A: EventSet> EventSet for (A,) {
}
}
+unsafe impl<A: StaticEventSet> StaticEventSet for (A,) {}
+
macro_rules! impl_event_set {
($Or:ident, $(($P:ident, $p:ident)),*) => {
/// An output type of an observer that observes multiple event types.
@@ -127,27 +296,37 @@ macro_rules! impl_event_set {
// SAFETY: All event types have a component id registered in `init_components`,
// and `unchecked_cast` calls `matches` before casting to one of the inner event sets.
- unsafe impl<$($P: EventSet),*> EventSet for ($($P,)*) {
+ unsafe impl<$($P: StaticEventSet),*> EventSet for ($($P,)*) {
type Item<'trigger> = $Or<$($P::Item<'trigger>),*>;
type ReadOnlyItem<'trigger> = $Or<$($P::ReadOnlyItem<'trigger>),*>;
+ fn cast<'trigger>(
+ world: &World,
+ observer_trigger: &ObserverTrigger,
+ ptr: PtrMut<'trigger>,
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
+ // SAFETY: Each inner event set is checked in order for a match and then casted.
+ unsafe { Self::unchecked_cast(world, observer_trigger, ptr) }
+ }
+
unsafe fn unchecked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
- ) -> Option<Self::Item<'trigger>> {
+ ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
if false {
unreachable!();
}
$(
else if $P::matches(world, observer_trigger) {
- if let Some($p) = $P::unchecked_cast(world, observer_trigger, ptr) {
- return Some($Or::$P($p));
+ match $P::unchecked_cast(world, observer_trigger, ptr) {
+ Ok($p) => return Ok($Or::$P($p)),
+ Err(ptr) => return Err(ptr),
}
}
)*
- None
+ Err(ptr)
}
fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool {
@@ -181,353 +360,23 @@ macro_rules! impl_event_set {
}
}
}
- };
-}
-
-impl_event_set!(Or2, (A, a), (B, b));
-impl_event_set!(Or3, (A, a), (B, b), (C, c));
-impl_event_set!(Or4, (A, a), (B, b), (C, c), (D, d));
-impl_event_set!(Or5, (A, a), (B, b), (C, c), (D, d), (E, e));
-impl_event_set!(Or6, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f));
-impl_event_set!(Or7, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g));
-impl_event_set!(
- Or8,
- (A, a),
- (B, b),
- (C, c),
- (D, d),
- (E, e),
- (F, f),
- (G, g),
- (H, h)
-);
-impl_event_set!(
- Or9,
- (A, a),
- (B, b),
- (C, c),
- (D, d),
- (E, e),
- (F, f),
- (G, g),
- (H, h),
- (I, i)
-);
-impl_event_set!(
- Or10,
- (A, a),
- (B, b),
- (C, c),
- (D, d),
- (E, e),
- (F, f),
- (G, g),
- (H, h),
- (I, i),
- (J, j)
-);
-impl_event_set!(
- Or11,
- (A, a),
- (B, b),
- (C, c),
- (D, d),
- (E, e),
- (F, f),
- (G, g),
- (H, h),
- (I, i),
- (J, j),
- (K, k)
-);
-impl_event_set!(
- Or12,
- (A, a),
- (B, b),
- (C, c),
- (D, d),
- (E, e),
- (F, f),
- (G, g),
- (H, h),
- (I, i),
- (J, j),
- (K, k),
- (L, l)
-);
-impl_event_set!(
- Or13,
- (A, a),
- (B, b),
- (C, c),
- (D, d),
- (E, e),
- (F, f),
- (G, g),
- (H, h),
- (I, i),
- (J, j),
- (K, k),
- (L, l),
- (M, m)
-);
-impl_event_set!(
- Or14,
- (A, a),
- (B, b),
- (C, c),
- (D, d),
- (E, e),
- (F, f),
- (G, g),
- (H, h),
- (I, i),
- (J, j),
- (K, k),
- (L, l),
- (M, m),
- (N, n)
-);
-impl_event_set!(
- Or15,
- (A, a),
- (B, b),
- (C, c),
- (D, d),
- (E, e),
- (F, f),
- (G, g),
- (H, h),
- (I, i),
- (J, j),
- (K, k),
- (L, l),
- (M, m),
- (N, n),
- (O, o)
-);
-
-/// A wrapper around an [`EventSet`] that foregoes safety checks and casting, and passes the pointer as is.
-/// This is useful for observers that do not need to access the event data, or need to do so dynamically.
-pub struct UntypedEvent<E = ()>(std::marker::PhantomData<E>);
-
-/// An [`EventSet`] that matches the specified event type(s), but does not cast the pointer.
-// SAFETY: Performs no unsafe operations, returns the pointer as is.
-unsafe impl<E: EventSet> EventSet for UntypedEvent<E> {
- type Item<'trigger> = PtrMut<'trigger>;
- type ReadOnlyItem<'trigger> = Ptr<'trigger>;
-
- unsafe fn unchecked_cast<'trigger>(
- _world: &World,
- _observer_trigger: &ObserverTrigger,
- ptr: PtrMut<'trigger>,
- ) -> Option<Self::Item<'trigger>> {
- Some(ptr)
- }
-
- fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
- true
- }
-
- fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
- E::init_components(world, ids);
- }
-
- 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()
- }
-}
-
-/// An [`EventSet`] that matches any event type, but does not cast the pointer.
-// SAFETY: Performs no unsafe operations, returns the pointer as is.
-unsafe impl EventSet for UntypedEvent<()> {
- type Item<'trigger> = PtrMut<'trigger>;
- type ReadOnlyItem<'trigger> = Ptr<'trigger>;
-
- unsafe fn unchecked_cast<'trigger>(
- _world: &World,
- _observer_trigger: &ObserverTrigger,
- ptr: PtrMut<'trigger>,
- ) -> Option<Self::Item<'trigger>> {
- Some(ptr)
- }
-
- fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
- true
- }
- 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()
- }
+ // SAFETY: All inner event types are static event sets.
+ unsafe impl<$($P: StaticEventSet),*> StaticEventSet for ($($P,)*) {}
+ };
}
-/// This trait is implemented on [`EventSet`]s where it is safe to call
-/// [`Observer::with_event`](crate::observer::Observer::with_event).
-pub unsafe trait DynamicEventSafe: EventSet {}
-
-unsafe impl<E: EventSet> DynamicEventSafe for UntypedEvent<E> {}
-unsafe impl DynamicEventSafe for UntypedEvent<()> {}
-unsafe impl<A: DynamicEventSafe> DynamicEventSafe for (A,) {}
-unsafe impl<A: EventSet, B: DynamicEventSafe> DynamicEventSafe for (A, B) {}
-unsafe impl<A: EventSet, B: EventSet, C: DynamicEventSafe> DynamicEventSafe for (A, B, C) {}
-unsafe impl<A: EventSet, B: EventSet, C: EventSet, D: DynamicEventSafe> DynamicEventSafe
- for (A, B, C, D)
-{
-}
-unsafe impl<A: EventSet, B: EventSet, C: EventSet, D: EventSet, E: DynamicEventSafe>
- DynamicEventSafe for (A, B, C, D, E)
-{
-}
-unsafe impl<A: EventSet, B: EventSet, C: EventSet, D: EventSet, E: EventSet, F: DynamicEventSafe>
- DynamicEventSafe for (A, B, C, D, E, F)
-{
-}
-unsafe impl<
- A: EventSet,
- B: EventSet,
- C: EventSet,
- D: EventSet,
- E: EventSet,
- F: EventSet,
- G: DynamicEventSafe,
- > DynamicEventSafe for (A, B, C, D, E, F, G)
-{
-}
-unsafe impl<
- A: EventSet,
- B: EventSet,
- C: EventSet,
- D: EventSet,
- E: EventSet,
- F: EventSet,
- G: EventSet,
- H: DynamicEventSafe,
- > DynamicEventSafe for (A, B, C, D, E, F, G, H)
-{
-}
-unsafe impl<
- A: EventSet,
- B: EventSet,
- C: EventSet,
- D: EventSet,
- E: EventSet,
- F: EventSet,
- G: EventSet,
- H: EventSet,
- I: DynamicEventSafe,
- > DynamicEventSafe for (A, B, C, D, E, F, G, H, I)
-{
-}
-unsafe impl<
- A: EventSet,
- B: EventSet,
- C: EventSet,
- D: EventSet,
- E: EventSet,
- F: EventSet,
- G: EventSet,
- H: EventSet,
- I: EventSet,
- J: DynamicEventSafe,
- > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J)
-{
-}
-unsafe impl<
- A: EventSet,
- B: EventSet,
- C: EventSet,
- D: EventSet,
- E: EventSet,
- F: EventSet,
- G: EventSet,
- H: EventSet,
- I: EventSet,
- J: EventSet,
- K: DynamicEventSafe,
- > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J, K)
-{
-}
-unsafe impl<
- A: EventSet,
- B: EventSet,
- C: EventSet,
- D: EventSet,
- E: EventSet,
- F: EventSet,
- G: EventSet,
- H: EventSet,
- I: EventSet,
- J: EventSet,
- K: EventSet,
- L: DynamicEventSafe,
- > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J, K, L)
-{
-}
-unsafe impl<
- A: EventSet,
- B: EventSet,
- C: EventSet,
- D: EventSet,
- E: EventSet,
- F: EventSet,
- G: EventSet,
- H: EventSet,
- I: EventSet,
- J: EventSet,
- K: EventSet,
- L: EventSet,
- M: DynamicEventSafe,
- > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J, K, L, M)
-{
-}
-unsafe impl<
- A: EventSet,
- B: EventSet,
- C: EventSet,
- D: EventSet,
- E: EventSet,
- F: EventSet,
- G: EventSet,
- H: EventSet,
- I: EventSet,
- J: EventSet,
- K: EventSet,
- L: EventSet,
- M: EventSet,
- N: DynamicEventSafe,
- > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J, K, L, M, N)
-{
-}
-unsafe impl<
- A: EventSet,
- B: EventSet,
- C: EventSet,
- D: EventSet,
- E: EventSet,
- F: EventSet,
- G: EventSet,
- H: EventSet,
- I: EventSet,
- J: EventSet,
- K: EventSet,
- L: EventSet,
- M: EventSet,
- N: EventSet,
- O: DynamicEventSafe,
- > DynamicEventSafe for (A, B, C, D, E, F, G, H, I, J, K, L, M, N, O)
-{
-}
+#[rustfmt::skip] impl_event_set!(Or2, (A, a), (B, b));
+#[rustfmt::skip] impl_event_set!(Or3, (A, a), (B, b), (C, c));
+#[rustfmt::skip] impl_event_set!(Or4, (A, a), (B, b), (C, c), (D, d));
+#[rustfmt::skip] impl_event_set!(Or5, (A, a), (B, b), (C, c), (D, d), (E, e));
+#[rustfmt::skip] impl_event_set!(Or6, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f));
+#[rustfmt::skip] impl_event_set!(Or7, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g));
+#[rustfmt::skip] impl_event_set!(Or8, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g), (H, h));
+#[rustfmt::skip] impl_event_set!(Or9, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g), (H, h), (I, i));
+#[rustfmt::skip] impl_event_set!(Or10, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g), (H, h), (I, i), (J, j));
+#[rustfmt::skip] impl_event_set!(Or11, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g), (H, h), (I, i), (J, j), (K, k));
+#[rustfmt::skip] impl_event_set!(Or12, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g), (H, h), (I, i), (J, j), (K, k), (L, l));
+#[rustfmt::skip] impl_event_set!(Or13, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g), (H, h), (I, i), (J, j), (K, k), (L, l), (M, m));
+#[rustfmt::skip] impl_event_set!(Or14, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g), (H, h), (I, i), (J, j), (K, k), (L, l), (M, m), (N, n));
+#[rustfmt::skip] impl_event_set!(Or15, (A, a), (B, b), (C, c), (D, d), (E, e), (F, f), (G, g), (H, h), (I, i), (J, j), (K, k), (L, l), (M, m), (N, n), (O, o));
From cad10a3da0959b85f2e2294125ef0efecaaa07b7 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 11 Aug 2024 01:12:56 -0500
Subject: [PATCH 28/34] rearrange tests
---
crates/bevy_ecs/src/observer/mod.rs | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 21bd2ac2e6ca5..57a3593ee45bf 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -621,12 +621,16 @@ mod tests {
}
#[test]
- fn observer_multiple_events_dynamic_autoregister() {
+ fn observer_multiple_events_dynamic() {
let mut world = World::new();
world.init_resource::<R>();
- world.spawn(Observer::new(
- |_: Trigger<DynamicEvent<(OnAdd, OnRemove)>, A>, mut res: ResMut<R>| res.0 += 1,
- ));
+ let on_add = world.init_component::<OnAdd>();
+ let on_remove = world.init_component::<OnRemove>();
+ world.spawn(
+ 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();
world.despawn(entity);
@@ -634,16 +638,12 @@ mod tests {
}
#[test]
- fn observer_multiple_events_dynamic() {
+ fn observer_multiple_events_dynamic_autoregister() {
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(
- Observer::new(|_: Trigger<DynamicEvent, A>, mut res: ResMut<R>| res.0 += 1)
- .with_event(on_add)
- .with_event(on_remove),
- );
+ world.spawn(Observer::new(
+ |_: Trigger<DynamicEvent<(OnAdd, OnRemove)>, A>, mut res: ResMut<R>| res.0 += 1,
+ ));
let entity = world.spawn(A).id();
world.despawn(entity);
From b6fb8dc2a729a0d6cdb8cee839345db3a68d6e85 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 11 Aug 2024 02:42:15 -0500
Subject: [PATCH 29/34] update benchmarks
---
.../observers/{untyped.rs => dynamic.rs} | 22 ++-
benches/benches/bevy_ecs/observers/mod.rs | 9 +-
.../benches/bevy_ecs/observers/multievent.rs | 126 +-----------
.../benches/bevy_ecs/observers/semidynamic.rs | 183 ++++++++++++++++++
4 files changed, 213 insertions(+), 127 deletions(-)
rename benches/benches/bevy_ecs/observers/{untyped.rs => dynamic.rs} (59%)
create mode 100644 benches/benches/bevy_ecs/observers/semidynamic.rs
diff --git a/benches/benches/bevy_ecs/observers/untyped.rs b/benches/benches/bevy_ecs/observers/dynamic.rs
similarity index 59%
rename from benches/benches/bevy_ecs/observers/untyped.rs
rename to benches/benches/bevy_ecs/observers/dynamic.rs
index d4b25d11da632..8b0be622420c3 100644
--- a/benches/benches/bevy_ecs/observers/untyped.rs
+++ b/benches/benches/bevy_ecs/observers/dynamic.rs
@@ -1,22 +1,25 @@
use bevy_ecs::{
event::Event,
- observer::{EventSet, Observer, Trigger, UntypedEvent},
- world::World,
+ observer::{DynamicEvent, EmitDynamicTrigger, EventSet, Observer, Trigger},
+ world::{Command, World},
};
use criterion::{black_box, Criterion};
-pub fn observe_untyped(criterion: &mut Criterion) {
- let mut group = criterion.benchmark_group("observe_untyped");
+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::<UntypedEvent>).with_event(event_id_1));
+ world.spawn(Observer::new(empty_listener_set::<DynamicEvent>).with_event(event_id_1));
bencher.iter(|| {
for _ in 0..10000 {
- world.trigger(TestEvent::<1>);
+ unsafe {
+ EmitDynamicTrigger::new_with_id(event_id_1, TestEvent::<1>, ())
+ .apply(&mut world)
+ };
}
});
});
@@ -25,13 +28,16 @@ pub fn observe_untyped(criterion: &mut Criterion) {
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::<UntypedEvent>)
+ Observer::new(empty_listener_set::<DynamicEvent>)
.with_event(event_id_1)
.with_event(event_id_2),
);
bencher.iter(|| {
for _ in 0..10000 {
- world.trigger(TestEvent::<2>);
+ unsafe {
+ EmitDynamicTrigger::new_with_id(event_id_2, TestEvent::<2>, ())
+ .apply(&mut world)
+ };
}
});
});
diff --git a/benches/benches/bevy_ecs/observers/mod.rs b/benches/benches/bevy_ecs/observers/mod.rs
index 3f12158bd880c..ceab56c99e1fb 100644
--- a/benches/benches/bevy_ecs/observers/mod.rs
+++ b/benches/benches/bevy_ecs/observers/mod.rs
@@ -1,18 +1,21 @@
use criterion::criterion_group;
+mod dynamic;
mod multievent;
mod propagation;
+mod semidynamic;
mod simple;
-mod untyped;
+use dynamic::*;
use multievent::*;
use propagation::*;
+use semidynamic::*;
use simple::*;
-use untyped::*;
criterion_group!(
observer_benches,
event_propagation,
observe_simple,
observe_multievent,
- observe_untyped
+ observe_dynamic,
+ observe_semidynamic
);
diff --git a/benches/benches/bevy_ecs/observers/multievent.rs b/benches/benches/bevy_ecs/observers/multievent.rs
index 379abfc818138..4002481a30110 100644
--- a/benches/benches/bevy_ecs/observers/multievent.rs
+++ b/benches/benches/bevy_ecs/observers/multievent.rs
@@ -11,43 +11,19 @@ pub fn observe_multievent(criterion: &mut Criterion) {
group.warm_up_time(std::time::Duration::from_millis(500));
group.measurement_time(std::time::Duration::from_secs(4));
+ group.bench_function("trigger_single", |bencher| {
+ let mut world = World::new();
+ world.observe(empty_listener_set::<TestEvent<1>>);
+ bencher.iter(|| {
+ for _ in 0..10000 {
+ world.trigger(TestEvent::<1>);
+ }
+ });
+ });
+
bench_in_set::<1, (TestEvent<1>,)>(&mut group);
bench_in_set::<2, (TestEvent<1>, TestEvent<2>)>(&mut group);
- bench_in_set::<3, (TestEvent<1>, TestEvent<2>, TestEvent<3>)>(&mut group);
bench_in_set::<4, (TestEvent<1>, TestEvent<2>, TestEvent<3>, TestEvent<4>)>(&mut group);
- bench_in_set::<
- 5,
- (
- TestEvent<1>,
- TestEvent<2>,
- TestEvent<3>,
- TestEvent<4>,
- TestEvent<5>,
- ),
- >(&mut group);
- bench_in_set::<
- 6,
- (
- TestEvent<1>,
- TestEvent<2>,
- TestEvent<3>,
- TestEvent<4>,
- TestEvent<5>,
- TestEvent<6>,
- ),
- >(&mut group);
- bench_in_set::<
- 7,
- (
- TestEvent<1>,
- TestEvent<2>,
- TestEvent<3>,
- TestEvent<4>,
- TestEvent<5>,
- TestEvent<6>,
- TestEvent<7>,
- ),
- >(&mut group);
bench_in_set::<
8,
(
@@ -61,51 +37,6 @@ pub fn observe_multievent(criterion: &mut Criterion) {
TestEvent<8>,
),
>(&mut group);
- bench_in_set::<
- 9,
- (
- TestEvent<1>,
- TestEvent<2>,
- TestEvent<3>,
- TestEvent<4>,
- TestEvent<5>,
- TestEvent<6>,
- TestEvent<7>,
- TestEvent<8>,
- TestEvent<9>,
- ),
- >(&mut group);
- bench_in_set::<
- 10,
- (
- TestEvent<1>,
- TestEvent<2>,
- TestEvent<3>,
- TestEvent<4>,
- TestEvent<5>,
- TestEvent<6>,
- TestEvent<7>,
- TestEvent<8>,
- TestEvent<9>,
- TestEvent<10>,
- ),
- >(&mut group);
- bench_in_set::<
- 11,
- (
- TestEvent<1>,
- TestEvent<2>,
- TestEvent<3>,
- TestEvent<4>,
- TestEvent<5>,
- TestEvent<6>,
- TestEvent<7>,
- TestEvent<8>,
- TestEvent<9>,
- TestEvent<10>,
- TestEvent<11>,
- ),
- >(&mut group);
bench_in_set::<
12,
(
@@ -123,43 +54,6 @@ pub fn observe_multievent(criterion: &mut Criterion) {
TestEvent<12>,
),
>(&mut group);
- bench_in_set::<
- 13,
- (
- TestEvent<1>,
- TestEvent<2>,
- TestEvent<3>,
- TestEvent<4>,
- TestEvent<5>,
- TestEvent<6>,
- TestEvent<7>,
- TestEvent<8>,
- TestEvent<9>,
- TestEvent<10>,
- TestEvent<11>,
- TestEvent<12>,
- TestEvent<13>,
- ),
- >(&mut group);
- bench_in_set::<
- 14,
- (
- TestEvent<1>,
- TestEvent<2>,
- TestEvent<3>,
- TestEvent<4>,
- TestEvent<5>,
- TestEvent<6>,
- TestEvent<7>,
- TestEvent<8>,
- TestEvent<9>,
- TestEvent<10>,
- TestEvent<11>,
- TestEvent<12>,
- TestEvent<13>,
- TestEvent<14>,
- ),
- >(&mut group);
bench_in_set::<
15,
(
diff --git a/benches/benches/bevy_ecs/observers/semidynamic.rs b/benches/benches/bevy_ecs/observers/semidynamic.rs
new file mode 100644
index 0000000000000..a6895cf572e1b
--- /dev/null
+++ b/benches/benches/bevy_ecs/observers/semidynamic.rs
@@ -0,0 +1,183 @@
+use bevy_ecs::{
+ event::Event,
+ observer::{EmitDynamicTrigger, EventSet, Observer, SemiDynamicEvent, Trigger},
+ world::{Command, World},
+};
+use criterion::{black_box, Criterion};
+
+pub fn observe_semidynamic(criterion: &mut Criterion) {
+ let mut group = criterion.benchmark_group("observe_semidynamic");
+ group.warm_up_time(std::time::Duration::from_millis(500));
+ group.measurement_time(std::time::Duration::from_secs(4));
+
+ group.bench_function("static/1s-1d", |bencher| {
+ let mut world = World::new();
+ let event_id_1 = world.init_component::<Dynamic<1>>();
+ world.spawn(
+ Observer::new(empty_listener_set::<SemiDynamicEvent<Static<1>>>).with_event(event_id_1),
+ );
+
+ bencher.iter(|| {
+ for _ in 0..10000 {
+ world.trigger(Static::<1>);
+ }
+ });
+ });
+ group.bench_function("dynamic/1s-1d", |bencher| {
+ let mut world = World::new();
+ let event_id_1 = world.init_component::<Dynamic<1>>();
+ world.spawn(
+ Observer::new(empty_listener_set::<SemiDynamicEvent<Static<1>>>).with_event(event_id_1),
+ );
+
+ bencher.iter(|| {
+ for _ in 0..10000 {
+ unsafe {
+ EmitDynamicTrigger::new_with_id(event_id_1, Dynamic::<1>, ()).apply(&mut world)
+ };
+ }
+ });
+ });
+
+ group.bench_function("static/15s-15d", |bencher| {
+ // Aint she perdy?
+ let mut world = World::new();
+ let event_id_1 = world.init_component::<Dynamic<1>>();
+ let event_id_2 = world.init_component::<Dynamic<2>>();
+ let event_id_3 = world.init_component::<Dynamic<3>>();
+ let event_id_4 = world.init_component::<Dynamic<4>>();
+ let event_id_5 = world.init_component::<Dynamic<5>>();
+ let event_id_6 = world.init_component::<Dynamic<6>>();
+ let event_id_7 = world.init_component::<Dynamic<7>>();
+ let event_id_8 = world.init_component::<Dynamic<8>>();
+ let event_id_9 = world.init_component::<Dynamic<9>>();
+ let event_id_10 = world.init_component::<Dynamic<10>>();
+ let event_id_11 = world.init_component::<Dynamic<11>>();
+ let event_id_12 = world.init_component::<Dynamic<12>>();
+ let event_id_13 = world.init_component::<Dynamic<13>>();
+ let event_id_14 = world.init_component::<Dynamic<14>>();
+ let event_id_15 = world.init_component::<Dynamic<15>>();
+ world.spawn(
+ Observer::new(
+ empty_listener_set::<
+ SemiDynamicEvent<(
+ Static<1>,
+ Static<2>,
+ Static<3>,
+ Static<4>,
+ Static<5>,
+ Static<6>,
+ Static<7>,
+ Static<8>,
+ Static<9>,
+ Static<10>,
+ Static<11>,
+ Static<12>,
+ Static<13>,
+ Static<14>,
+ Static<15>,
+ )>,
+ >,
+ )
+ .with_event(event_id_1)
+ .with_event(event_id_2)
+ .with_event(event_id_3)
+ .with_event(event_id_4)
+ .with_event(event_id_5)
+ .with_event(event_id_6)
+ .with_event(event_id_7)
+ .with_event(event_id_8)
+ .with_event(event_id_9)
+ .with_event(event_id_10)
+ .with_event(event_id_11)
+ .with_event(event_id_12)
+ .with_event(event_id_13)
+ .with_event(event_id_14)
+ .with_event(event_id_15),
+ );
+
+ bencher.iter(|| {
+ for _ in 0..10000 {
+ world.trigger(Static::<14>);
+ }
+ });
+ });
+ group.bench_function("dynamic/15s-15d", |bencher| {
+ // Aint she perdy?
+ let mut world = World::new();
+ let event_id_1 = world.init_component::<Dynamic<1>>();
+ let event_id_2 = world.init_component::<Dynamic<2>>();
+ let event_id_3 = world.init_component::<Dynamic<3>>();
+ let event_id_4 = world.init_component::<Dynamic<4>>();
+ let event_id_5 = world.init_component::<Dynamic<5>>();
+ let event_id_6 = world.init_component::<Dynamic<6>>();
+ let event_id_7 = world.init_component::<Dynamic<7>>();
+ let event_id_8 = world.init_component::<Dynamic<8>>();
+ let event_id_9 = world.init_component::<Dynamic<9>>();
+ let event_id_10 = world.init_component::<Dynamic<10>>();
+ let event_id_11 = world.init_component::<Dynamic<11>>();
+ let event_id_12 = world.init_component::<Dynamic<12>>();
+ let event_id_13 = world.init_component::<Dynamic<13>>();
+ let event_id_14 = world.init_component::<Dynamic<14>>();
+ let event_id_15 = world.init_component::<Dynamic<15>>();
+ world.spawn(
+ Observer::new(
+ empty_listener_set::<
+ SemiDynamicEvent<(
+ Static<1>,
+ Static<2>,
+ Static<3>,
+ Static<4>,
+ Static<5>,
+ Static<6>,
+ Static<7>,
+ Static<8>,
+ Static<9>,
+ Static<10>,
+ Static<11>,
+ Static<12>,
+ Static<13>,
+ Static<14>,
+ Static<15>,
+ )>,
+ >,
+ )
+ .with_event(event_id_1)
+ .with_event(event_id_2)
+ .with_event(event_id_3)
+ .with_event(event_id_4)
+ .with_event(event_id_5)
+ .with_event(event_id_6)
+ .with_event(event_id_7)
+ .with_event(event_id_8)
+ .with_event(event_id_9)
+ .with_event(event_id_10)
+ .with_event(event_id_11)
+ .with_event(event_id_12)
+ .with_event(event_id_13)
+ .with_event(event_id_14)
+ .with_event(event_id_15),
+ );
+
+ bencher.iter(|| {
+ for _ in 0..10000 {
+ unsafe {
+ EmitDynamicTrigger::new_with_id(event_id_14, Dynamic::<14>, ())
+ .apply(&mut world)
+ };
+ }
+ });
+ });
+}
+
+/// Static event type
+#[derive(Event)]
+struct Static<const N: usize>;
+
+/// Dynamic event type
+#[derive(Event)]
+struct Dynamic<const N: usize>;
+
+fn empty_listener_set<Set: EventSet>(trigger: Trigger<Set>) {
+ black_box(trigger);
+}
From 5f9cbeddb2c708150d50035c385898ba6f17927d Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 11 Aug 2024 02:43:23 -0500
Subject: [PATCH 30/34] add safety comment
---
crates/bevy_ecs/src/observer/set.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index b227779a72604..35316ea1d43e5 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -281,6 +281,7 @@ unsafe impl<A: StaticEventSet> EventSet for (A,) {
}
}
+// SAFETY: The inner event set is a static event set.
unsafe impl<A: StaticEventSet> StaticEventSet for (A,) {}
macro_rules! impl_event_set {
From a84c60e6a07b84da33141cc4ed094447a1d45cee Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 11 Aug 2024 03:41:28 -0500
Subject: [PATCH 31/34] clarify safety comment
---
crates/bevy_ecs/src/observer/runner.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index 44f3f91813ce7..fa1a2be6dea32 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -425,7 +425,7 @@ fn observer_system_runner<E: EventSet, B: Bundle>(
// We choose to use unchecked_cast over the safe cast method to avoid the overhead of matching in the single event type case (which is the common case).
let Ok(event) = (unsafe { E::unchecked_cast(world_ref, &observer_trigger, ptr) }) else {
// This branch is only ever hit if the user called Observer::with_event_unchecked with a component ID not matching the event set E,
- // EXCEPT when the event set is a singular event type, in which case the event will always match.
+ // EXCEPT when the event set is a singular event type, in which case the cast will always succeed.
// This is a user error and should be logged.
bevy_utils::tracing::error!(
"Observer was triggered with an event that does not match the event set. \
From 8b82d5905f2fc53f2ec9b411f549c1ba7e3bcd41 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 11 Aug 2024 05:39:29 -0500
Subject: [PATCH 32/34] docs first pass
---
crates/bevy_ecs/src/observer/runner.rs | 9 +--
crates/bevy_ecs/src/observer/set.rs | 95 +++++++++++++++++++++-----
2 files changed, 84 insertions(+), 20 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index fa1a2be6dea32..c6da1f4720806 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -321,7 +321,7 @@ where
/// 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: UntypedEvent<E> itself does not perform any unsafe operations (like casting), so this is safe.
+ // SAFETY: DynamicEvent itself does not perform any unsafe operations (like casting), so this is safe.
unsafe { self.with_event_unchecked(event) }
}
}
@@ -334,10 +334,11 @@ where
/// 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.
+ /// As opposed to [`Observer::with_event_unchecked`], this method is safe to use because no pointer casting is performed automatically
+ /// on event types outside its statically-known set of. That is left to the user to do manually.
pub fn with_event(self, event: ComponentId) -> Self {
- // SAFETY: UntypedEvent<E> itself does not perform any unsafe operations (like casting), so this is safe.
+ // SAFETY: SemiDynamicEvent itself does not perform any unsafe operations (like casting)
+ // for event types outside its statically-known set, so this is safe.
unsafe { self.with_event_unchecked(event) }
}
}
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 35316ea1d43e5..383924deda556 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -5,21 +5,37 @@ use crate::event::Event;
use crate::observer::ObserverTrigger;
use crate::world::World;
-/// A set of events that can trigger an observer.
+/// A set of [`Event`]s that can trigger an observer.
+///
+/// The provided implementations of this trait are:
+///
+/// - All [`Event`]s.
+/// - Any tuple of [`Event`]s, up to 15 types. These can be nested.
+/// - [`DynamicEvent`], which matches any [`Event`]s dynamically added to the observer with [`Observer::with_event`] and does not reify the event data.
+/// - [`SemiDynamicEvent`], which will first try to match a statically-known set of [`Event`]s and reify the event data,
+/// and if no match is found, it will fall back to functioning as a [`DynamicEvent`].
+///
+/// # Example
+///
+/// TODO
///
/// # Safety
///
/// Implementor must ensure that:
-/// - [`EventSet::init_components`] must register a component id for each event type in the set.
-/// - [`EventSet::matches`] must return `true` if the event type is in the set, or if the set matches any event type.
+/// - [`EventSet::init_components`] must register a [`ComponentId`] for each [`Event`] type in the set.
+/// - [`EventSet::matches`] must return `true` if the triggered [`Event`]'s [`ComponentId`] matches a type in the set,
+/// or unambiguously always returns `true` or `false`.
///
+/// [`Observer::with_event`]: crate::observer::Observer::with_event
pub unsafe trait EventSet: 'static {
- /// The output type that will be passed to the observer.
+ /// The item returned by this [`EventSet`] that will be passed to the observer system function.
+ /// Most of the time this will be a mutable reference to an [`Event`] type, a tuple of mutable references, or a [`PtrMut`].
type Item<'trigger>;
- /// The read-only variant of the output type.
+ /// The read-only variant of the [`Item`](EventSet::Item).
type ReadOnlyItem<'trigger>: Copy;
- /// Safely casts a pointer to the output type, checking if the event type matches this event set.
+ /// Safely casts a pointer to the [`Item`](EventSet::Item) type by checking prior
+ /// whether the triggered [`Event`]'s [`ComponentId`] [`matches`](EventSet::matches) a type in this event set.
fn cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
@@ -33,27 +49,35 @@ pub unsafe trait EventSet: 'static {
}
}
- /// Casts a pointer to the output type.
+ /// Casts a pointer to the [`Item`](EventSet::Item) type
+ /// without checking if the [`Event`] type matches this event set.
///
/// # Safety
///
- /// Caller must ensure that the event component id [`matches`](EventSet::matches) this event set before calling this function.
+ /// Caller must ensure that the [`Event`]'s [`ComponentId`] [`matches`](EventSet::matches)
+ /// this event set before calling this function.
unsafe fn unchecked_cast<'trigger>(
world: &World,
observer_trigger: &ObserverTrigger,
ptr: PtrMut<'trigger>,
) -> Result<Self::Item<'trigger>, PtrMut<'trigger>>;
- /// Checks if the event type matches the observer trigger.
+ /// Checks if the [`Event`] type matches the observer trigger.
+ ///
+ /// # Safety
+ ///
+ /// Implementors must ensure that this function returns `true`
+ /// if the triggered [`Event`]'s [`ComponentId`] matches a type in the set,
+ /// or unambiguously always returns `true` or `false`.
fn matches(world: &World, observer_trigger: &ObserverTrigger) -> bool;
- /// Initialize the components required by the event set.
+ /// Initialize the components required by this event set.
fn init_components(world: &mut World, ids: impl FnMut(ComponentId));
- /// Shrink the item to a shorter lifetime.
+ /// Shrink the [`Item`](EventSet::Item) to a shorter lifetime.
fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short>;
- /// Shrink the item to a shorter lifetime, read-only variant.
+ /// Shrink the [`Item`](EventSet::Item) to a shorter lifetime [`ReadOnlyItem`](EventSet::ReadOnlyItem).
fn shrink_readonly<'long: 'short, 'short>(
item: &'short Self::Item<'long>,
) -> Self::ReadOnlyItem<'short>;
@@ -61,11 +85,19 @@ pub unsafe trait EventSet: 'static {
/// An [`EventSet`] that matches a statically pre-defined set of event types.
///
+/// This trait is required in order to prevent a footgun where a user might accidentally specify an `EventSet` similar to
+/// `(DynamicEvent, EventA, EventB)`, which would always match `DynamicEvent` and never `EventA` or `EventB`.
+/// Therefore, we prevent the introduction of `DynamicEvent` in a static `EventSet`,
+/// most notably any `EventSet` tuple made up of normal [`Event`] types.
+///
+/// If you need to support both dynamic and static event types in a single observer,
+/// you can use [`SemiDynamicEvent`] instead.
+///
/// # Safety
///
/// Implementors must ensure that [`matches`](EventSet::matches)
/// returns `true` if and only if the event component id matches the event type,
-/// AND does not match any other event type.
+/// and DOES NOT match any other event type.
pub unsafe trait StaticEventSet: EventSet {}
// SAFETY: The event type has a component id registered in `init_components`,
@@ -108,8 +140,19 @@ unsafe impl<E: Event> EventSet for E {
// SAFETY: The event type is a statically known type.
unsafe impl<E: Event> StaticEventSet for E {}
-/// An [`EventSet`] that matches any event type, but does not cast the pointer. Instead, it returns the pointer as is.
+/// An [`EventSet`] 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.
+///
+/// `DynamicEvent` accepts one type parameter:
+///
+/// - **Register**
+/// The event set that will have its types automatically registered to the observer.
+/// This reduces the boilerplate of registering the event types manually when some or all are statically known.
+/// However, these types will not be matched or casted by the observer.
+///
+/// # Example
+///
+/// TODO
pub struct DynamicEvent<Register = ()>(std::marker::PhantomData<Register>);
// SAFETY: Performs no unsafe operations, returns the pointer as is.
@@ -134,10 +177,12 @@ unsafe impl<Register: StaticEventSet> EventSet for DynamicEvent<Register> {
}
fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
+ // We're treating this as a catch-all event set, so it always matches.
true
}
fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
+ // We'll register the component ids of the `Register` event set, but won't use them for matching.
Register::init_components(world, ids);
}
@@ -174,6 +219,7 @@ unsafe impl EventSet for DynamicEvent<()> {
}
fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
+ // We're treating this as a catch-all event set, so it always matches.
true
}
@@ -190,8 +236,24 @@ unsafe impl EventSet for DynamicEvent<()> {
}
}
-/// An [`EventSet`] that matches a statically pre-defined set of event types and casts the pointer to the event type,
-/// or returns the pointer as is if the event type does not match.
+/// An [`EventSet`] that either matches a statically pre-defined set of event types and casts the pointer to the event type,
+/// or returns the pointer as-is if the event type was not matched.
+/// Basically, it allows you to mix static and dynamic event types in a single observer.
+///
+/// `SemiDynamicEvent` accepts two type parameters:
+///
+/// - **Static**
+/// The static event set that will be matched and casted.
+/// Generally, this should be a tuple of static event types, like `(FooEvent, BarEvent)`.
+/// Must implement [`StaticEventSet`] trait, which means no [`DynamicEvent`] or [`SemiDynamicEvent`] nesting.
+/// - **Register**
+/// The event set that will have its types automatically registered to the observer.
+/// This reduces the boilerplate of registering the event types manually when some or all are statically known.
+/// However, these types will not be matched or casted by the observer.
+///
+/// # Example
+///
+/// TODO
pub struct SemiDynamicEvent<Static: StaticEventSet, Register = ()>(
std::marker::PhantomData<(Static, Register)>,
);
@@ -367,6 +429,7 @@ macro_rules! impl_event_set {
};
}
+// We can't use `all_tuples` here because it doesn't support the extra `OrX` parameter required for each tuple impl.
#[rustfmt::skip] impl_event_set!(Or2, (A, a), (B, b));
#[rustfmt::skip] impl_event_set!(Or3, (A, a), (B, b), (C, c));
#[rustfmt::skip] impl_event_set!(Or4, (A, a), (B, b), (C, c), (D, d));
From 404e4ec9fae4aa2386bbfec77e47b15efc949834 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Sun, 11 Aug 2024 05:42:56 -0500
Subject: [PATCH 33/34] fix indentation
---
crates/bevy_ecs/src/observer/set.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 383924deda556..531f7db6f95f1 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -146,9 +146,9 @@ unsafe impl<E: Event> StaticEventSet for E {}
/// `DynamicEvent` accepts one type parameter:
///
/// - **Register**
-/// The event set that will have its types automatically registered to the observer.
-/// This reduces the boilerplate of registering the event types manually when some or all are statically known.
-/// However, these types will not be matched or casted by the observer.
+/// The event set that will have its types automatically registered to the observer.
+/// This reduces the boilerplate of registering the event types manually when some or all are statically known.
+/// However, these types will not be matched or casted by the observer.
///
/// # Example
///
From caa5dd89d429b4be8c65d63c82a26f594a02e298 Mon Sep 17 00:00:00 2001
From: Christian Hughes <xdotdash@gmail.com>
Date: Mon, 12 Aug 2024 01:04:25 -0500
Subject: [PATCH 34/34] remove unncessary Register parameters from DynamicEvent
and SemiDynamicEvent
---
crates/bevy_ecs/src/observer/mod.rs | 31 ------------
crates/bevy_ecs/src/observer/runner.rs | 10 +---
crates/bevy_ecs/src/observer/set.rs | 67 ++------------------------
3 files changed, 6 insertions(+), 102 deletions(-)
diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index 57a3593ee45bf..c1be95e518b21 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -637,19 +637,6 @@ mod tests {
assert_eq!(2, world.resource::<R>().0);
}
- #[test]
- fn observer_multiple_events_dynamic_autoregister() {
- let mut world = World::new();
- world.init_resource::<R>();
- world.spawn(Observer::new(
- |_: Trigger<DynamicEvent<(OnAdd, OnRemove)>, A>, mut res: ResMut<R>| res.0 += 1,
- ));
-
- let entity = world.spawn(A).id();
- world.despawn(entity);
- assert_eq!(2, world.resource::<R>().0);
- }
-
#[test]
fn observer_multiple_events_semidynamic() {
let mut world = World::new();
@@ -672,24 +659,6 @@ mod tests {
assert_eq!(2, world.resource::<R>().0);
}
- #[test]
- fn observer_multiple_events_semidynamic_autoregister() {
- let mut world = World::new();
- world.init_resource::<R>();
- world.spawn(Observer::new(
- |trigger: Trigger<SemiDynamicEvent<OnAdd, OnRemove>, A>, mut res: ResMut<R>| {
- match trigger.event() {
- Ok(_onadd) => res.assert_order(0),
- Err(_ptr) => res.assert_order(1),
- };
- },
- ));
-
- let entity = world.spawn(A).id();
- world.despawn(entity);
- assert_eq!(2, world.resource::<R>().0);
- }
-
#[test]
fn observer_multiple_components() {
let mut world = World::new();
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index c6da1f4720806..246b0529aa3db 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -310,10 +310,7 @@ impl<E: EventSet, B: Bundle> Observer<E, B> {
}
}
-impl<Register, B: Bundle> Observer<DynamicEvent<Register>, B>
-where
- DynamicEvent<Register>: EventSet,
-{
+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.
///
@@ -326,10 +323,7 @@ where
}
}
-impl<Static: StaticEventSet, Register, B: Bundle> Observer<SemiDynamicEvent<Static, Register>, B>
-where
- SemiDynamicEvent<Static, Register>: EventSet,
-{
+impl<Static: StaticEventSet, B: Bundle> Observer<SemiDynamicEvent<Static>, B> {
/// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
/// is triggered.
///
diff --git a/crates/bevy_ecs/src/observer/set.rs b/crates/bevy_ecs/src/observer/set.rs
index 531f7db6f95f1..932435d080f14 100644
--- a/crates/bevy_ecs/src/observer/set.rs
+++ b/crates/bevy_ecs/src/observer/set.rs
@@ -143,62 +143,13 @@ unsafe impl<E: Event> StaticEventSet for E {}
/// An [`EventSet`] 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.
///
-/// `DynamicEvent` accepts one type parameter:
-///
-/// - **Register**
-/// The event set that will have its types automatically registered to the observer.
-/// This reduces the boilerplate of registering the event types manually when some or all are statically known.
-/// However, these types will not be matched or casted by the observer.
-///
/// # Example
///
/// TODO
-pub struct DynamicEvent<Register = ()>(std::marker::PhantomData<Register>);
-
-// SAFETY: Performs no unsafe operations, returns the pointer as is.
-unsafe impl<Register: StaticEventSet> EventSet for DynamicEvent<Register> {
- type Item<'trigger> = PtrMut<'trigger>;
- type ReadOnlyItem<'trigger> = Ptr<'trigger>;
-
- fn cast<'trigger>(
- _world: &World,
- _observer_trigger: &ObserverTrigger,
- ptr: PtrMut<'trigger>,
- ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
- Ok(ptr)
- }
-
- unsafe fn unchecked_cast<'trigger>(
- _world: &World,
- _observer_trigger: &ObserverTrigger,
- ptr: PtrMut<'trigger>,
- ) -> Result<Self::Item<'trigger>, PtrMut<'trigger>> {
- Ok(ptr)
- }
-
- fn matches(_world: &World, _observer_trigger: &ObserverTrigger) -> bool {
- // We're treating this as a catch-all event set, so it always matches.
- true
- }
-
- fn init_components(world: &mut World, ids: impl FnMut(ComponentId)) {
- // We'll register the component ids of the `Register` event set, but won't use them for matching.
- Register::init_components(world, ids);
- }
-
- 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()
- }
-}
+pub struct DynamicEvent;
// SAFETY: Performs no unsafe operations, returns the pointer as is.
-unsafe impl EventSet for DynamicEvent<()> {
+unsafe impl EventSet for DynamicEvent {
type Item<'trigger> = PtrMut<'trigger>;
type ReadOnlyItem<'trigger> = Ptr<'trigger>;
@@ -246,23 +197,14 @@ unsafe impl EventSet for DynamicEvent<()> {
/// The static event set that will be matched and casted.
/// Generally, this should be a tuple of static event types, like `(FooEvent, BarEvent)`.
/// Must implement [`StaticEventSet`] trait, which means no [`DynamicEvent`] or [`SemiDynamicEvent`] nesting.
-/// - **Register**
-/// The event set that will have its types automatically registered to the observer.
-/// This reduces the boilerplate of registering the event types manually when some or all are statically known.
-/// However, these types will not be matched or casted by the observer.
///
/// # Example
///
/// TODO
-pub struct SemiDynamicEvent<Static: StaticEventSet, Register = ()>(
- std::marker::PhantomData<(Static, Register)>,
-);
+pub struct SemiDynamicEvent<Static: StaticEventSet>(std::marker::PhantomData<Static>);
// SAFETY: No unsafe operations are performed. The checked cast variant is used for the static event type.
-unsafe impl<Static: StaticEventSet, Register> EventSet for SemiDynamicEvent<Static, Register>
-where
- DynamicEvent<Register>: EventSet,
-{
+unsafe impl<Static: StaticEventSet> EventSet for SemiDynamicEvent<Static> {
type Item<'trigger> = Result<Static::Item<'trigger>, PtrMut<'trigger>>;
type ReadOnlyItem<'trigger> = Result<Static::ReadOnlyItem<'trigger>, Ptr<'trigger>>;
@@ -283,7 +225,6 @@ where
fn init_components(world: &mut World, mut ids: impl FnMut(ComponentId)) {
Static::init_components(world, &mut ids);
- DynamicEvent::<Register>::init_components(world, ids);
}
fn shrink<'long: 'short, 'short>(item: &'short mut Self::Item<'long>) -> Self::Item<'short> {