From 1f59ecd310012b7170c71e65cf03450838cc5a25 Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 23 Jun 2024 09:37:05 -0400 Subject: [PATCH 01/20] Add bubbling to entity observers --- crates/bevy_ecs/macros/src/component.rs | 2 + crates/bevy_ecs/src/event/base.rs | 10 +- crates/bevy_ecs/src/lib.rs | 1 + crates/bevy_ecs/src/observer/mod.rs | 285 ++++++++++++++++-- crates/bevy_ecs/src/observer/runner.rs | 7 +- crates/bevy_ecs/src/observer/trigger_event.rs | 8 +- crates/bevy_ecs/src/traversal.rs | 33 ++ crates/bevy_ecs/src/world/deferred_world.rs | 26 +- 8 files changed, 343 insertions(+), 29 deletions(-) create mode 100644 crates/bevy_ecs/src/traversal.rs diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index dbd28e4b28ac6..19db76ba19694 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -17,6 +17,8 @@ pub fn derive_event(input: TokenStream) -> TokenStream { TokenStream::from(quote! { impl #impl_generics #bevy_ecs_path::event::Event for #struct_name #type_generics #where_clause { + type Traverse = #bevy_ecs_path::traversal::TraverseNone; + const SHOULD_BUBBLE: bool = false; } impl #impl_generics #bevy_ecs_path::component::Component for #struct_name #type_generics #where_clause { diff --git a/crates/bevy_ecs/src/event/base.rs b/crates/bevy_ecs/src/event/base.rs index acc830da447bd..621b739f09724 100644 --- a/crates/bevy_ecs/src/event/base.rs +++ b/crates/bevy_ecs/src/event/base.rs @@ -1,4 +1,4 @@ -use crate::component::Component; +use crate::{component::Component, traversal::Traversal}; #[cfg(feature = "bevy_reflect")] use bevy_reflect::Reflect; use std::{ @@ -31,7 +31,13 @@ use std::{ label = "invalid `Event`", note = "consider annotating `{Self}` with `#[derive(Event)]`" )] -pub trait Event: Component {} +pub trait Event: Component { + /// A system param that describes a traversal through the ECS. + type Traverse: Traversal; + + /// Sets the default bubling state of the entity when used with observers. + const SHOULD_BUBBLE: bool; +} /// An `EventId` uniquely identifies an event stored in a specific [`World`]. /// diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 307c7a94a16e5..32f875678b21e 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -29,6 +29,7 @@ pub mod removal_detection; pub mod schedule; pub mod storage; pub mod system; +pub mod traversal; pub mod world; pub use bevy_ptr as ptr; diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index b3d689d449fbf..cc83fc5f26f50 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -8,6 +8,7 @@ pub use runner::*; pub use trigger_event::*; use crate::observer::entity_observer::ObservedBy; +use crate::traversal::Traversal; use crate::{archetype::ArchetypeFlags, system::IntoObserverSystem, world::*}; use crate::{component::ComponentId, prelude::*, world::DeferredWorld}; use bevy_ptr::Ptr; @@ -18,15 +19,17 @@ use std::marker::PhantomData; /// [`Event`] data itself. If it was triggered for a specific [`Entity`], it includes that as well. pub struct Trigger<'w, E, B: Bundle = ()> { event: &'w mut E, + bubble: &'w mut bool, trigger: ObserverTrigger, _marker: PhantomData, } impl<'w, E, B: Bundle> Trigger<'w, E, B> { /// Creates a new trigger for the given event and observer information. - pub fn new(event: &'w mut E, trigger: ObserverTrigger) -> Self { + pub fn new(event: &'w mut E, bubble: &'w mut bool, trigger: ObserverTrigger) -> Self { Self { event, + bubble, trigger, _marker: PhantomData, } @@ -56,6 +59,21 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> { pub fn entity(&self) -> Entity { self.trigger.entity } + + /// Enables event bubbling. + pub fn propigate(&mut self, should_bubble: bool) { + *self.bubble = should_bubble; + } + + /// Get a reference to the event bubling flag. + pub fn should_bubble(&self) -> &bool { + self.bubble + } + + /// Get a mutable reference to the event bubling flag. + pub fn should_bubble_mut(&mut self) -> &mut bool { + self.bubble + } } /// A description of what an [`Observer`] observes. @@ -168,13 +186,16 @@ impl Observers { } /// This will run the observers of the given `event_type`, targeting the given `entity` and `components`. - pub(crate) fn invoke( + pub(crate) fn invoke( mut world: DeferredWorld, event_type: ComponentId, entity: Entity, components: impl Iterator, data: &mut T, - ) { + should_bubble: bool, + ) where + C: Traversal, + { // SAFETY: You cannot get a mutable reference to `observers` from `DeferredWorld` let (mut world, observers) = unsafe { let world = world.as_unsafe_world_cell(); @@ -188,7 +209,8 @@ impl Observers { (world.into_deferred(), observers) }; - let mut trigger_observer = |(&observer, runner): (&Entity, &ObserverRunner)| { + // Trigger observers listening for any kind of this trigger + for (&observer, runner) in observers.map.iter() { (runner)( world.reborrow(), ObserverTrigger { @@ -197,30 +219,82 @@ impl Observers { entity, }, data.into(), + &mut false, ); - }; - - // Trigger observers listening for any kind of this trigger - observers.map.iter().for_each(&mut trigger_observer); + } // Trigger entity observers listening for this kind of trigger if entity != Entity::PLACEHOLDER { - if let Some(map) = observers.entity_observers.get(&entity) { - map.iter().for_each(&mut trigger_observer); + let mut entity = entity; + loop { + let mut bubble = should_bubble; + if let Some(map) = observers.entity_observers.get(&entity) { + for (&observer, runner) in map.iter() { + (runner)( + world.reborrow(), + ObserverTrigger { + observer, + event_type, + entity, + }, + data.into(), + &mut bubble, + ); + } + if !bubble { + break; + } + } + if let Some(next) = world.get_mut::(entity).and_then(|t| t.next()) { + entity = next; + } else { + break; + }; } } // Trigger observers listening to this trigger targeting a specific component components.for_each(|id| { if let Some(component_observers) = observers.component_observers.get(&id) { - component_observers - .map - .iter() - .for_each(&mut trigger_observer); + for (&observer, runner) in component_observers.map.iter() { + (runner)( + world.reborrow(), + ObserverTrigger { + observer, + event_type, + entity, + }, + data.into(), + &mut false, + ); + } if entity != Entity::PLACEHOLDER { - if let Some(map) = component_observers.entity_map.get(&entity) { - map.iter().for_each(&mut trigger_observer); + let mut entity = entity; + loop { + let mut bubble = should_bubble; + if let Some(map) = component_observers.entity_map.get(&entity) { + for (&observer, runner) in map.iter() { + (runner)( + world.reborrow(), + ObserverTrigger { + observer, + event_type, + entity, + }, + data.into(), + &mut bubble, + ); + } + if !bubble { + break; + } + } + if let Some(next) = world.get_mut::(entity).and_then(|t| t.next()) { + entity = next; + } else { + break; + }; } } } @@ -393,6 +467,7 @@ mod tests { use crate as bevy_ecs; use crate::observer::{EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState}; use crate::prelude::*; + use crate::traversal::Traversal; #[derive(Component)] struct A; @@ -421,6 +496,24 @@ mod tests { } } + #[derive(Component)] + struct Parent(Entity); + + impl Traversal for Parent { + fn next(&self) -> Option { + Some(self.0) + } + } + + #[derive(Component)] + struct EventBubling; + + impl Event for EventBubling { + type Traverse = Parent; + + const SHOULD_BUBBLE: bool = true; + } + #[test] fn observer_order_spawn_despawn() { let mut world = World::new(); @@ -649,7 +742,7 @@ mod tests { world.spawn(ObserverState { // SAFETY: we registered `event_a` above and it matches the type of TriggerA descriptor: unsafe { ObserverDescriptor::default().with_events(vec![event_a]) }, - runner: |mut world, _trigger, _ptr| { + runner: |mut world, _trigger, _ptr, _bubble| { world.resource_mut::().0 += 1; }, ..Default::default() @@ -662,4 +755,162 @@ mod tests { world.flush(); assert_eq!(1, world.resource::().0); } + + #[test] + fn observer_bubling() { + let mut world = World::new(); + world.init_resource::(); + + let parent = world + .spawn_empty() + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .id(); + + let child = world + .spawn(Parent(parent)) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubling, child); + world.flush(); + assert_eq!(2, world.resource::().0); + } + + #[test] + fn observer_bubling_redundant_dispatch() { + let mut world = World::new(); + world.init_resource::(); + + let parent = world + .spawn_empty() + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .id(); + + let child = world + .spawn(Parent(parent)) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubling, [child, parent]); + world.flush(); + assert_eq!(3, world.resource::().0); + } + + #[test] + fn observer_bubling_stop_propigation() { + let mut world = World::new(); + world.init_resource::(); + + let parent = world + .spawn_empty() + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .id(); + + let child = world + .spawn(Parent(parent)) + .observe(|mut trigger: Trigger, mut res: ResMut| { + res.0 += 1; + trigger.propigate(false); + }) + .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(EventBubling, child); + world.flush(); + assert_eq!(1, world.resource::().0); + } + + #[test] + fn observer_bubling_join() { + let mut world = World::new(); + world.init_resource::(); + + let parent = world + .spawn_empty() + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .id(); + + let child_a = world + .spawn(Parent(parent)) + .observe(|_: Trigger, mut res: ResMut| { + res.0 += 1; + }) + .id(); + + let child_b = world + .spawn(Parent(parent)) + .observe(|_: Trigger, mut res: ResMut| { + 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(EventBubling, [child_a, child_b]); + world.flush(); + assert_eq!(4, world.resource::().0); + } + + #[test] + fn observer_bubling_no_next() { + let mut world = World::new(); + world.init_resource::(); + + let entity = world + .spawn_empty() + .observe(|_: Trigger, mut res: ResMut| 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(EventBubling, entity); + world.flush(); + assert_eq!(1, world.resource::().0); + } + + #[test] + fn observer_bubling_parallel_propagation() { + let mut world = World::new(); + world.init_resource::(); + + let parent_a = world + .spawn_empty() + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .id(); + + let child_a = world + .spawn(Parent(parent_a)) + .observe(|mut trigger: Trigger, mut res: ResMut| { + res.0 += 1; + trigger.propigate(false); + }) + .id(); + + let parent_b = world + .spawn_empty() + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .id(); + + let child_b = world + .spawn(Parent(parent_b)) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubling, [child_a, child_b]); + world.flush(); + assert_eq!(3, world.resource::().0); + } } diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 40092be5539c7..c2981364059d7 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -20,7 +20,7 @@ pub struct ObserverState { impl Default for ObserverState { fn default() -> Self { Self { - runner: |_, _, _| {}, + runner: |_, _, _, _| {}, last_trigger_id: 0, despawned_watched_entities: 0, descriptor: Default::default(), @@ -86,7 +86,7 @@ impl Component for ObserverState { /// Type for function that is run when an observer is triggered. /// Typically refers to the default runner that runs the system stored in the associated [`ObserverSystemComponent`], /// but can be overridden for custom behaviour. -pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut); +pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, &mut bool); /// An [`Observer`] system. Add this [`Component`] to an [`Entity`] to turn it into an "observer". /// @@ -358,6 +358,7 @@ fn observer_system_runner( mut world: DeferredWorld, observer_trigger: ObserverTrigger, ptr: PtrMut, + bubble: &mut bool, ) { let world = world.as_unsafe_world_cell(); // SAFETY: Observer was triggered so must still exist in world @@ -382,7 +383,7 @@ fn observer_system_runner( state.last_trigger_id = last_trigger; // SAFETY: Caller ensures `ptr` is castable to `&mut T` - let trigger: Trigger = Trigger::new(unsafe { ptr.deref_mut() }, observer_trigger); + let trigger: Trigger = Trigger::new(unsafe { ptr.deref_mut() }, bubble, 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/trigger_event.rs b/crates/bevy_ecs/src/observer/trigger_event.rs index ff4000c8ee688..69edc91941238 100644 --- a/crates/bevy_ecs/src/observer/trigger_event.rs +++ b/crates/bevy_ecs/src/observer/trigger_event.rs @@ -48,7 +48,7 @@ impl Command for EmitDynamicTrigger( +fn trigger_event( world: &mut World, event_type: ComponentId, event_data: &mut E, @@ -58,22 +58,24 @@ fn trigger_event( if targets.entities().len() == 0 { // SAFETY: T is accessible as the type represented by self.trigger, ensured in `Self::new` unsafe { - world.trigger_observers_with_data( + world.trigger_observers_with_data::<_, E::Traverse>( event_type, Entity::PLACEHOLDER, targets.components(), event_data, + E::SHOULD_BUBBLE, ); }; } else { for target in targets.entities() { // SAFETY: T is accessible as the type represented by self.trigger, ensured in `Self::new` unsafe { - world.trigger_observers_with_data( + world.trigger_observers_with_data::<_, E::Traverse>( event_type, target, targets.components(), event_data, + E::SHOULD_BUBBLE, ); }; } diff --git a/crates/bevy_ecs/src/traversal.rs b/crates/bevy_ecs/src/traversal.rs new file mode 100644 index 0000000000000..611fc1298c612 --- /dev/null +++ b/crates/bevy_ecs/src/traversal.rs @@ -0,0 +1,33 @@ +//! A trait for components that let you traverse the ECS + +use crate::{ + component::{Component, StorageType}, + entity::Entity, +}; + +/// A component that holds a pointer to another entity. +/// +/// The implementor is responsible for ensuring that `Traversal::next` cannot produce infinite loops. +pub trait Traversal: Component { + /// Returns the next entity to visit. + fn next(&self) -> Option; +} + +/// A traversial component that dosn't traverse anything. Used to provide a defalt traversal +/// implementation for events. +/// +/// It is not possible to actually construct an instance of this component. +pub struct TraverseNone { + _private: (), +} + +impl Traversal for TraverseNone { + #[inline(always)] + fn next(&self) -> Option { + None + } +} + +impl Component for TraverseNone { + const STORAGE_TYPE: StorageType = StorageType::Table; +} diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index 4147d3644f3ae..260f413026451 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -10,6 +10,7 @@ use crate::{ prelude::{Component, QueryState}, query::{QueryData, QueryFilter}, system::{Commands, Query, Resource}, + traversal::{Traversal, TraverseNone}, }; use super::{ @@ -350,7 +351,14 @@ impl<'w> DeferredWorld<'w> { entity: Entity, components: impl Iterator, ) { - Observers::invoke(self.reborrow(), event, entity, components, &mut ()); + Observers::invoke::<_, TraverseNone>( + self.reborrow(), + event, + entity, + components, + &mut (), + false, + ); } /// Triggers all event observers for [`ComponentId`] in target. @@ -358,14 +366,24 @@ impl<'w> DeferredWorld<'w> { /// # Safety /// Caller must ensure `E` is accessible as the type represented by `event` #[inline] - pub(crate) unsafe fn trigger_observers_with_data( + pub(crate) unsafe fn trigger_observers_with_data( &mut self, event: ComponentId, entity: Entity, components: impl Iterator, data: &mut E, - ) { - Observers::invoke(self.reborrow(), event, entity, components, data); + should_bubble: bool, + ) where + C: Traversal, + { + Observers::invoke::<_, C>( + self.reborrow(), + event, + entity, + components, + data, + should_bubble, + ); } /// Sends a "global" [`Trigger`] without any targets. From bab9f67eb7e8f1cbb999e8e11792c6a4b5c95254 Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 23 Jun 2024 09:46:21 -0400 Subject: [PATCH 02/20] Add Traversal impl to Parent --- crates/bevy_hierarchy/src/components/parent.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index bb2b6abba09b1..8c6407d031a9b 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -3,6 +3,7 @@ use bevy_ecs::reflect::{ReflectComponent, ReflectMapEntities}; use bevy_ecs::{ component::Component, entity::{Entity, EntityMapper, MapEntities}, + traversal::Traversal, world::{FromWorld, World}, }; use std::ops::Deref; @@ -69,3 +70,9 @@ impl Deref for Parent { &self.0 } } + +impl Traversal for Parent { + fn next(&self) -> Option { + Some(self.0) + } +} From 98ff3962ee4893bb75a17205c83edf721c1548ac Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 23 Jun 2024 10:05:44 -0400 Subject: [PATCH 03/20] Fix spelling error --- crates/bevy_ecs/src/observer/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index cc83fc5f26f50..dc586303ec290 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -61,7 +61,7 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> { } /// Enables event bubbling. - pub fn propigate(&mut self, should_bubble: bool) { + pub fn propagate(&mut self, should_bubble: bool) { *self.bubble = should_bubble; } @@ -816,7 +816,7 @@ mod tests { .spawn(Parent(parent)) .observe(|mut trigger: Trigger, mut res: ResMut| { res.0 += 1; - trigger.propigate(false); + trigger.propagate(false); }) .id(); @@ -892,7 +892,7 @@ mod tests { .spawn(Parent(parent_a)) .observe(|mut trigger: Trigger, mut res: ResMut| { res.0 += 1; - trigger.propigate(false); + trigger.propagate(false); }) .id(); From faf63cb2e5c74172f3e75014eac76e78053aaabc Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 23 Jun 2024 10:17:42 -0400 Subject: [PATCH 04/20] Fix more spelling mistakes --- crates/bevy_ecs/src/observer/mod.rs | 2 +- crates/bevy_ecs/src/traversal.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index dc586303ec290..17ac7a364c6ee 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -803,7 +803,7 @@ mod tests { } #[test] - fn observer_bubling_stop_propigation() { + fn observer_bubling_stop_propagation() { let mut world = World::new(); world.init_resource::(); diff --git a/crates/bevy_ecs/src/traversal.rs b/crates/bevy_ecs/src/traversal.rs index 611fc1298c612..46f7370156d92 100644 --- a/crates/bevy_ecs/src/traversal.rs +++ b/crates/bevy_ecs/src/traversal.rs @@ -13,7 +13,7 @@ pub trait Traversal: Component { fn next(&self) -> Option; } -/// A traversial component that dosn't traverse anything. Used to provide a defalt traversal +/// A traversial component that dosn't traverse anything. Used to provide a default traversal /// implementation for events. /// /// It is not possible to actually construct an instance of this component. From 4c4fc74a3b7949c84bf0a474cfa8a4efd1ccca91 Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 23 Jun 2024 11:41:02 -0400 Subject: [PATCH 05/20] Fix even more spelling mistakes --- crates/bevy_ecs/src/observer/mod.rs | 60 ++++++++++++++--------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 17ac7a364c6ee..10c0774a83eb6 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -65,12 +65,12 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> { *self.bubble = should_bubble; } - /// Get a reference to the event bubling flag. + /// Get a reference to the event bubbling flag. pub fn should_bubble(&self) -> &bool { self.bubble } - /// Get a mutable reference to the event bubling flag. + /// Get a mutable reference to the event bubbling flag. pub fn should_bubble_mut(&mut self) -> &mut bool { self.bubble } @@ -506,9 +506,9 @@ mod tests { } #[derive(Component)] - struct EventBubling; + struct EventBubbling; - impl Event for EventBubling { + impl Event for EventBubbling { type Traverse = Parent; const SHOULD_BUBBLE: bool = true; @@ -757,64 +757,64 @@ mod tests { } #[test] - fn observer_bubling() { + fn observer_bubbling() { let mut world = World::new(); world.init_resource::(); let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubling, child); + world.trigger_targets(EventBubbling, child); world.flush(); assert_eq!(2, world.resource::().0); } #[test] - fn observer_bubling_redundant_dispatch() { + fn observer_bubbling_redundant_dispatch() { let mut world = World::new(); world.init_resource::(); let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubling, [child, parent]); + world.trigger_targets(EventBubbling, [child, parent]); world.flush(); assert_eq!(3, world.resource::().0); } #[test] - fn observer_bubling_stop_propagation() { + fn observer_bubbling_stop_propagation() { let mut world = World::new(); world.init_resource::(); let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child = world .spawn(Parent(parent)) - .observe(|mut trigger: Trigger, mut res: ResMut| { + .observe(|mut trigger: Trigger, mut res: ResMut| { res.0 += 1; trigger.propagate(false); }) @@ -823,31 +823,31 @@ mod tests { // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut // and therefore does not automatically flush. world.flush(); - world.trigger_targets(EventBubling, child); + world.trigger_targets(EventBubbling, child); world.flush(); assert_eq!(1, world.resource::().0); } #[test] - fn observer_bubling_join() { + fn observer_bubbling_join() { let mut world = World::new(); world.init_resource::(); let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child_a = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| { + .observe(|_: Trigger, mut res: ResMut| { res.0 += 1; }) .id(); let child_b = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| { + .observe(|_: Trigger, mut res: ResMut| { res.0 += 1; }) .id(); @@ -855,42 +855,42 @@ mod tests { // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut // and therefore does not automatically flush. world.flush(); - world.trigger_targets(EventBubling, [child_a, child_b]); + world.trigger_targets(EventBubbling, [child_a, child_b]); world.flush(); assert_eq!(4, world.resource::().0); } #[test] - fn observer_bubling_no_next() { + fn observer_bubbling_no_next() { let mut world = World::new(); world.init_resource::(); let entity = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubling, entity); + world.trigger_targets(EventBubbling, entity); world.flush(); assert_eq!(1, world.resource::().0); } #[test] - fn observer_bubling_parallel_propagation() { + fn observer_bubbling_parallel_propagation() { let mut world = World::new(); world.init_resource::(); let parent_a = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child_a = world .spawn(Parent(parent_a)) - .observe(|mut trigger: Trigger, mut res: ResMut| { + .observe(|mut trigger: Trigger, mut res: ResMut| { res.0 += 1; trigger.propagate(false); }) @@ -898,18 +898,18 @@ mod tests { let parent_b = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child_b = world .spawn(Parent(parent_b)) - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubling, [child_a, child_b]); + world.trigger_targets(EventBubbling, [child_a, child_b]); world.flush(); assert_eq!(3, world.resource::().0); } From 0b93ca2cb8dc565b4585cbe5c681a10d1e9efb17 Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 30 Jun 2024 14:06:52 -0400 Subject: [PATCH 06/20] Address review comments, update docs, and fix global observer propagation --- crates/bevy_ecs/macros/src/component.rs | 2 +- crates/bevy_ecs/src/event/base.rs | 12 +- crates/bevy_ecs/src/observer/mod.rs | 258 ++++++++++-------- crates/bevy_ecs/src/observer/runner.rs | 14 +- crates/bevy_ecs/src/observer/trigger_event.rs | 75 ++--- crates/bevy_ecs/src/traversal.rs | 20 +- crates/bevy_ecs/src/world/deferred_world.rs | 40 ++- 7 files changed, 236 insertions(+), 185 deletions(-) diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index 19db76ba19694..996d12e291cd0 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -18,7 +18,7 @@ pub fn derive_event(input: TokenStream) -> TokenStream { TokenStream::from(quote! { impl #impl_generics #bevy_ecs_path::event::Event for #struct_name #type_generics #where_clause { type Traverse = #bevy_ecs_path::traversal::TraverseNone; - const SHOULD_BUBBLE: bool = false; + const AUTO_PROPAGATE: bool = false; } impl #impl_generics #bevy_ecs_path::component::Component for #struct_name #type_generics #where_clause { diff --git a/crates/bevy_ecs/src/event/base.rs b/crates/bevy_ecs/src/event/base.rs index 621b739f09724..59708bb03a19d 100644 --- a/crates/bevy_ecs/src/event/base.rs +++ b/crates/bevy_ecs/src/event/base.rs @@ -32,11 +32,17 @@ use std::{ note = "consider annotating `{Self}` with `#[derive(Event)]`" )] pub trait Event: Component { - /// A system param that describes a traversal through the ECS. + /// The component that describes which Entity to propigate this event to next, when [propagation] is enabled. + /// + /// [propagation]: crate::observers::Trigger::enable_propagation type Traverse: Traversal; - /// Sets the default bubling state of the entity when used with observers. - const SHOULD_BUBBLE: bool; + /// When true, this event will always attempt to propigate when [triggered], without requiring a call + /// to [`Trigger::enable_propagation`]. + /// + /// [triggered]: crate::commands::Commands::trigger_targets + /// [`Trigger::enable_propagation`]: crate::observers::Trigger::enable_propagation + const AUTO_PROPAGATE: bool = false; } /// An `EventId` uniquely identifies an event stored in a specific [`World`]. diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 10c0774a83eb6..91c27d5c49e31 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -8,7 +8,6 @@ pub use runner::*; pub use trigger_event::*; use crate::observer::entity_observer::ObservedBy; -use crate::traversal::Traversal; use crate::{archetype::ArchetypeFlags, system::IntoObserverSystem, world::*}; use crate::{component::ComponentId, prelude::*, world::DeferredWorld}; use bevy_ptr::Ptr; @@ -19,17 +18,21 @@ use std::marker::PhantomData; /// [`Event`] data itself. If it was triggered for a specific [`Entity`], it includes that as well. pub struct Trigger<'w, E, B: Bundle = ()> { event: &'w mut E, - bubble: &'w mut bool, + propagation: &'w mut Propagation, trigger: ObserverTrigger, _marker: PhantomData, } impl<'w, E, B: Bundle> Trigger<'w, E, B> { /// Creates a new trigger for the given event and observer information. - pub fn new(event: &'w mut E, bubble: &'w mut bool, trigger: ObserverTrigger) -> Self { + pub fn new( + event: &'w mut E, + propagation: &'w mut Propagation, + trigger: ObserverTrigger, + ) -> Self { Self { event, - bubble, + propagation, trigger, _marker: PhantomData, } @@ -60,19 +63,32 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> { self.trigger.entity } - /// Enables event bubbling. - pub fn propagate(&mut self, should_bubble: bool) { - *self.bubble = should_bubble; + /// Enables or disables event propagation, allowing the same event to trigger observers on a chain of different entities. + /// + /// The path an event will propagate along is specified by it's associated [`Traversal`] component. By default, events + /// use `TraverseNone` which ends the path immediately and prevents propagation. + /// + /// To enable propagation, you must: + /// + Set [`Event::Traverse`] to the component you want to propagate along. + /// + Either call `propagate(true)` in the first observer or set [`Event::AUTO_PROPAGATE`] to `true`. + /// + /// You can prevent an event from propagating further using `propagate(false)`. + pub fn propagate(&mut self, should_propagate: bool) { + *self.propagation = if should_propagate { + Propagation::Continue + } else { + Propagation::Halt + }; } - /// Get a reference to the event bubbling flag. - pub fn should_bubble(&self) -> &bool { - self.bubble + /// Returns a reference to the flag that controls event propagation. See [`propagate`] for more information. + pub fn get_propagation(&self) -> &Propagation { + self.propagation } - /// Get a mutable reference to the event bubbling flag. - pub fn should_bubble_mut(&mut self) -> &mut bool { - self.bubble + /// Returns a mutable reference to the flag that controls event propagation. See [`propagate`] for more information. + pub fn get_propagation_mut(&mut self) -> &mut Propagation { + self.propagation } } @@ -132,6 +148,15 @@ pub struct ObserverTrigger { pub entity: Entity, } +/// Determines if the event should propagate to other entities. +#[derive(Eq, PartialEq, Copy, Clone, Debug)] +pub enum Propagation { + /// Allows propagation to continue. + Continue, + /// Halts propagation. + Halt, +} + // Map between an observer entity and its runner type ObserverMap = EntityHashMap; @@ -186,16 +211,14 @@ impl Observers { } /// This will run the observers of the given `event_type`, targeting the given `entity` and `components`. - pub(crate) fn invoke( + pub(crate) fn invoke( mut world: DeferredWorld, event_type: ComponentId, entity: Entity, components: impl Iterator, data: &mut T, - should_bubble: bool, - ) where - C: Traversal, - { + propagation: &mut Propagation, + ) { // SAFETY: You cannot get a mutable reference to `observers` from `DeferredWorld` let (mut world, observers) = unsafe { let world = world.as_unsafe_world_cell(); @@ -209,8 +232,7 @@ impl Observers { (world.into_deferred(), observers) }; - // Trigger observers listening for any kind of this trigger - for (&observer, runner) in observers.map.iter() { + let mut trigger_observer = |(&observer, runner): (&Entity, &ObserverRunner)| { (runner)( world.reborrow(), ObserverTrigger { @@ -219,82 +241,30 @@ impl Observers { entity, }, data.into(), - &mut false, + propagation, ); - } + }; + // Trigger observers listening for any kind of this trigger + observers.map.iter().for_each(&mut trigger_observer); // Trigger entity observers listening for this kind of trigger if entity != Entity::PLACEHOLDER { - let mut entity = entity; - loop { - let mut bubble = should_bubble; - if let Some(map) = observers.entity_observers.get(&entity) { - for (&observer, runner) in map.iter() { - (runner)( - world.reborrow(), - ObserverTrigger { - observer, - event_type, - entity, - }, - data.into(), - &mut bubble, - ); - } - if !bubble { - break; - } - } - if let Some(next) = world.get_mut::(entity).and_then(|t| t.next()) { - entity = next; - } else { - break; - }; + if let Some(map) = observers.entity_observers.get(&entity) { + map.iter().for_each(&mut trigger_observer); } } // Trigger observers listening to this trigger targeting a specific component components.for_each(|id| { if let Some(component_observers) = observers.component_observers.get(&id) { - for (&observer, runner) in component_observers.map.iter() { - (runner)( - world.reborrow(), - ObserverTrigger { - observer, - event_type, - entity, - }, - data.into(), - &mut false, - ); - } + component_observers + .map + .iter() + .for_each(&mut trigger_observer); if entity != Entity::PLACEHOLDER { - let mut entity = entity; - loop { - let mut bubble = should_bubble; - if let Some(map) = component_observers.entity_map.get(&entity) { - for (&observer, runner) in map.iter() { - (runner)( - world.reborrow(), - ObserverTrigger { - observer, - event_type, - entity, - }, - data.into(), - &mut bubble, - ); - } - if !bubble { - break; - } - } - if let Some(next) = world.get_mut::(entity).and_then(|t| t.next()) { - entity = next; - } else { - break; - }; + if let Some(map) = component_observers.entity_map.get(&entity) { + map.iter().for_each(&mut trigger_observer); } } } @@ -506,12 +476,12 @@ mod tests { } #[derive(Component)] - struct EventBubbling; + struct EventPropagating; - impl Event for EventBubbling { + impl Event for EventPropagating { type Traverse = Parent; - const SHOULD_BUBBLE: bool = true; + const AUTO_PROPAGATE: bool = true; } #[test] @@ -742,7 +712,7 @@ mod tests { world.spawn(ObserverState { // SAFETY: we registered `event_a` above and it matches the type of TriggerA descriptor: unsafe { ObserverDescriptor::default().with_events(vec![event_a]) }, - runner: |mut world, _trigger, _ptr, _bubble| { + runner: |mut world, _trigger, _ptr, _propagation| { world.resource_mut::().0 += 1; }, ..Default::default() @@ -757,97 +727,99 @@ mod tests { } #[test] - fn observer_bubbling() { + fn observer_propagating() { let mut world = World::new(); world.init_resource::(); let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubbling, child); + world.trigger_targets(EventPropagating, child); world.flush(); assert_eq!(2, world.resource::().0); } #[test] - fn observer_bubbling_redundant_dispatch() { + fn observer_propagating_redundant_dispatch() { let mut world = World::new(); world.init_resource::(); let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubbling, [child, parent]); + world.trigger_targets(EventPropagating, [child, parent]); world.flush(); assert_eq!(3, world.resource::().0); } #[test] - fn observer_bubbling_stop_propagation() { + fn observer_propagating_halt() { let mut world = World::new(); world.init_resource::(); let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child = world .spawn(Parent(parent)) - .observe(|mut trigger: Trigger, mut res: ResMut| { - res.0 += 1; - trigger.propagate(false); - }) + .observe( + |mut trigger: Trigger, mut res: ResMut| { + res.0 += 1; + trigger.propagate(false); + }, + ) .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(EventBubbling, child); + world.trigger_targets(EventPropagating, child); world.flush(); assert_eq!(1, world.resource::().0); } #[test] - fn observer_bubbling_join() { + fn observer_propagating_join() { let mut world = World::new(); world.init_resource::(); let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child_a = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| { + .observe(|_: Trigger, mut res: ResMut| { res.0 += 1; }) .id(); let child_b = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| { + .observe(|_: Trigger, mut res: ResMut| { res.0 += 1; }) .id(); @@ -855,62 +827,108 @@ mod tests { // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut // and therefore does not automatically flush. world.flush(); - world.trigger_targets(EventBubbling, [child_a, child_b]); + world.trigger_targets(EventPropagating, [child_a, child_b]); world.flush(); assert_eq!(4, world.resource::().0); } #[test] - fn observer_bubbling_no_next() { + fn observer_propagating_no_next() { let mut world = World::new(); world.init_resource::(); let entity = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubbling, entity); + world.trigger_targets(EventPropagating, entity); world.flush(); assert_eq!(1, world.resource::().0); } #[test] - fn observer_bubbling_parallel_propagation() { + fn observer_propagating_parallel_propagation() { let mut world = World::new(); world.init_resource::(); let parent_a = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child_a = world .spawn(Parent(parent_a)) - .observe(|mut trigger: Trigger, mut res: ResMut| { - res.0 += 1; - trigger.propagate(false); - }) + .observe( + |mut trigger: Trigger, mut res: ResMut| { + res.0 += 1; + trigger.propagate(false); + }, + ) .id(); let parent_b = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) .id(); let child_b = world .spawn(Parent(parent_b)) - .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .observe(|_: Trigger, mut res: ResMut| 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(EventBubbling, [child_a, child_b]); + world.trigger_targets(EventPropagating, [child_a, child_b]); world.flush(); assert_eq!(3, world.resource::().0); } + + #[test] + fn observer_propagating_world() { + let mut world = World::new(); + world.init_resource::(); + + world.observe(|_: Trigger, mut res: ResMut| res.0 += 1); + + let grandparent = world.spawn_empty().id(); + let parent = world.spawn(Parent(grandparent)).id(); + let child = world.spawn(Parent(parent)).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::().0); + } + + #[test] + fn observer_propagating_world_skipping() { + let mut world = World::new(); + world.init_resource::(); + + world.observe( + |trigger: Trigger, query: Query<&A>, mut res: ResMut| { + if query.get(trigger.entity()).is_ok() { + res.0 += 1 + } + }, + ); + + let grandparent = world.spawn(A).id(); + let parent = world.spawn(Parent(grandparent)).id(); + let child = world.spawn((A, Parent(parent))).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!(2, world.resource::().0); + } } diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index c2981364059d7..1279df50dd338 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::{ObserverDescriptor, ObserverTrigger, Propagation}, prelude::*, query::DebugCheckedUnwrap, system::{IntoObserverSystem, ObserverSystem}, @@ -86,7 +86,7 @@ impl Component for ObserverState { /// Type for function that is run when an observer is triggered. /// Typically refers to the default runner that runs the system stored in the associated [`ObserverSystemComponent`], /// but can be overridden for custom behaviour. -pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, &mut bool); +pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, &mut Propagation); /// An [`Observer`] system. Add this [`Component`] to an [`Entity`] to turn it into an "observer". /// @@ -358,7 +358,7 @@ fn observer_system_runner( mut world: DeferredWorld, observer_trigger: ObserverTrigger, ptr: PtrMut, - bubble: &mut bool, + propagation: &mut Propagation, ) { let world = world.as_unsafe_world_cell(); // SAFETY: Observer was triggered so must still exist in world @@ -382,8 +382,12 @@ fn observer_system_runner( } state.last_trigger_id = last_trigger; - // SAFETY: Caller ensures `ptr` is castable to `&mut T` - let trigger: Trigger = Trigger::new(unsafe { ptr.deref_mut() }, bubble, observer_trigger); + let trigger: Trigger = Trigger::new( + // SAFETY: Caller ensures `ptr` is castable to `&mut T` + unsafe { ptr.deref_mut() }, + propagation, + 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/trigger_event.rs b/crates/bevy_ecs/src/observer/trigger_event.rs index 69edc91941238..7c00e404419ca 100644 --- a/crates/bevy_ecs/src/observer/trigger_event.rs +++ b/crates/bevy_ecs/src/observer/trigger_event.rs @@ -2,6 +2,7 @@ use crate::{ component::ComponentId, entity::Entity, event::Event, + observer::Propagation, world::{Command, DeferredWorld, World}, }; @@ -55,7 +56,7 @@ fn trigger_event( targets: Targets, ) { let mut world = DeferredWorld::from(world); - if targets.entities().len() == 0 { + if targets.entities().is_empty() { // SAFETY: T is accessible as the type represented by self.trigger, ensured in `Self::new` unsafe { world.trigger_observers_with_data::<_, E::Traverse>( @@ -63,7 +64,7 @@ fn trigger_event( Entity::PLACEHOLDER, targets.components(), event_data, - E::SHOULD_BUBBLE, + Propagation::Halt, ); }; } else { @@ -72,10 +73,16 @@ fn trigger_event( unsafe { world.trigger_observers_with_data::<_, E::Traverse>( event_type, - target, + *target, targets.components(), event_data, - E::SHOULD_BUBBLE, + const { + if E::AUTO_PROPAGATE { + Propagation::Continue + } else { + Propagation::Halt + } + }, ); }; } @@ -90,78 +97,78 @@ fn trigger_event( /// [`Observer`]: crate::observer::Observer pub trait TriggerTargets: Send + Sync + 'static { /// The components the trigger should target. - fn components(&self) -> impl ExactSizeIterator; + fn components(&self) -> &[ComponentId]; /// The entities the trigger should target. - fn entities(&self) -> impl ExactSizeIterator; + fn entities(&self) -> &[Entity]; } impl TriggerTargets for () { - fn components(&self) -> impl ExactSizeIterator { - [].into_iter() + fn components(&self) -> &[ComponentId] { + &[] } - fn entities(&self) -> impl ExactSizeIterator { - [].into_iter() + fn entities(&self) -> &[Entity] { + &[] } } impl TriggerTargets for Entity { - fn components(&self) -> impl ExactSizeIterator { - [].into_iter() + fn components(&self) -> &[ComponentId] { + &[] } - fn entities(&self) -> impl ExactSizeIterator { - std::iter::once(*self) + fn entities(&self) -> &[Entity] { + std::slice::from_ref(self) } } impl TriggerTargets for Vec { - fn components(&self) -> impl ExactSizeIterator { - [].into_iter() + fn components(&self) -> &[ComponentId] { + &[] } - fn entities(&self) -> impl ExactSizeIterator { - self.iter().copied() + fn entities(&self) -> &[Entity] { + self.as_slice() } } impl TriggerTargets for [Entity; N] { - fn components(&self) -> impl ExactSizeIterator { - [].into_iter() + fn components(&self) -> &[ComponentId] { + &[] } - fn entities(&self) -> impl ExactSizeIterator { - self.iter().copied() + fn entities(&self) -> &[Entity] { + self.as_slice() } } impl TriggerTargets for ComponentId { - fn components(&self) -> impl ExactSizeIterator { - std::iter::once(*self) + fn components(&self) -> &[ComponentId] { + std::slice::from_ref(self) } - fn entities(&self) -> impl ExactSizeIterator { - [].into_iter() + fn entities(&self) -> &[Entity] { + &[] } } impl TriggerTargets for Vec { - fn components(&self) -> impl ExactSizeIterator { - self.iter().copied() + fn components(&self) -> &[ComponentId] { + self.as_slice() } - fn entities(&self) -> impl ExactSizeIterator { - [].into_iter() + fn entities(&self) -> &[Entity] { + &[] } } impl TriggerTargets for [ComponentId; N] { - fn components(&self) -> impl ExactSizeIterator { - self.iter().copied() + fn components(&self) -> &[ComponentId] { + self.as_slice() } - fn entities(&self) -> impl ExactSizeIterator { - [].into_iter() + fn entities(&self) -> &[Entity] { + &[] } } diff --git a/crates/bevy_ecs/src/traversal.rs b/crates/bevy_ecs/src/traversal.rs index 46f7370156d92..d44d05620baf1 100644 --- a/crates/bevy_ecs/src/traversal.rs +++ b/crates/bevy_ecs/src/traversal.rs @@ -1,25 +1,31 @@ -//! A trait for components that let you traverse the ECS +//! A trait for components that let you traverse the ECS. use crate::{ component::{Component, StorageType}, entity::Entity, }; -/// A component that holds a pointer to another entity. +/// A component that can point to another entity, and which can be used to define a path through the ECS. /// -/// The implementor is responsible for ensuring that `Traversal::next` cannot produce infinite loops. +/// Traversals are used to [specify the direction] of [event propagation] in [observers]. By default, +/// events use the [`TraverseNone`] placeholder component, which cannot actually be created or added to +/// an entity and so never causes traversal. +/// +/// The implementer is responsible for ensuring that `Traversal::next` cannot produce infinite loops. +/// +/// [specify the direction]: crate::event::Event::Traverse +/// [event propagation]: crate::observer::Trigger::propagate +/// [observers]: crate::observer::Observer pub trait Traversal: Component { /// Returns the next entity to visit. fn next(&self) -> Option; } -/// A traversial component that dosn't traverse anything. Used to provide a default traversal +/// A traversal component that doesn't traverse anything. Used to provide a default traversal /// implementation for events. /// /// It is not possible to actually construct an instance of this component. -pub struct TraverseNone { - _private: (), -} +pub enum TraverseNone {} impl Traversal for TraverseNone { #[inline(always)] diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index 260f413026451..714a6024d4ab3 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -6,11 +6,11 @@ use crate::{ component::ComponentId, entity::Entity, event::{Event, EventId, Events, SendBatchIds}, - observer::{Observers, TriggerTargets}, + observer::{Observers, Propagation, TriggerTargets}, prelude::{Component, QueryState}, query::{QueryData, QueryFilter}, system::{Commands, Query, Resource}, - traversal::{Traversal, TraverseNone}, + traversal::Traversal, }; use super::{ @@ -351,13 +351,13 @@ impl<'w> DeferredWorld<'w> { entity: Entity, components: impl Iterator, ) { - Observers::invoke::<_, TraverseNone>( + Observers::invoke::<_>( self.reborrow(), event, entity, components, &mut (), - false, + &mut Propagation::Halt, ); } @@ -369,21 +369,31 @@ impl<'w> DeferredWorld<'w> { pub(crate) unsafe fn trigger_observers_with_data( &mut self, event: ComponentId, - entity: Entity, - components: impl Iterator, + mut entity: Entity, + components: &[ComponentId], data: &mut E, - should_bubble: bool, + mut propagation: Propagation, ) where C: Traversal, { - Observers::invoke::<_, C>( - self.reborrow(), - event, - entity, - components, - data, - should_bubble, - ); + loop { + Observers::invoke::<_>( + self.reborrow(), + event, + entity, + components.iter().cloned(), + data, + &mut propagation, + ); + if propagation == Propagation::Halt { + break; + } + if let Some(next) = self.get::(entity).and_then(|t| t.next()) { + entity = next; + } else { + break; + } + } } /// Sends a "global" [`Trigger`] without any targets. From bde5ff0c3fbb87702e9b8d9638b502834366bbd8 Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 30 Jun 2024 14:07:35 -0400 Subject: [PATCH 07/20] Add observer propgation example --- Cargo.toml | 11 +++ examples/ecs/observer_propagation.rs | 126 +++++++++++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 examples/ecs/observer_propagation.rs diff --git a/Cargo.toml b/Cargo.toml index 8211f56657222..18d66d850753f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2549,6 +2549,17 @@ description = "Demonstrates observers that react to events (both built-in life-c category = "ECS (Entity Component System)" wasm = true +[[example]] +name = "observer_propagation" +path = "examples/ecs/observer_propagation.rs" +doc-scrape-examples = true + +[package.metadata.example.observers_propagation] +name = "Observers Propagation" +description = "Demonstrates event propagation with observers" +category = "ECS (Entity Component System)" +wasm = true + [[example]] name = "3d_rotation" path = "examples/transforms/3d_rotation.rs" diff --git a/examples/ecs/observer_propagation.rs b/examples/ecs/observer_propagation.rs new file mode 100644 index 0000000000000..92fd9242f6bb9 --- /dev/null +++ b/examples/ecs/observer_propagation.rs @@ -0,0 +1,126 @@ +//! Demonstrates how to propagate events through the hierarchy with observers. + +use std::time::Duration; + +use bevy::{log::LogPlugin, prelude::*, time::common_conditions::on_timer}; +use rand::{seq::IteratorRandom, thread_rng, Rng}; + +fn main() { + App::new() + .add_plugins(MinimalPlugins) + .add_plugins(LogPlugin::default()) + .add_systems(Startup, setup) + .add_systems( + Update, + attack_armor.run_if(on_timer(Duration::from_millis(200))), + ) + // Add a global observer that will emit a line whenever an attack hits an entity. + .observe(attack_hits) + .run(); +} + +// In this example, we spawn a goblin wearing different pecies of armor. Each piece of armor +// is represented as a child entity, with an `Armor` component. +// +// We're going to model how attack damage can be partally blocked by the goblin's armor using +// event bubling. Our events will target the armor, and if the armor isn't strong enough to block +// the attack it will continue up and hit the goblin. +fn setup(mut commands: Commands) { + commands + .spawn((Name::new("Goblin"), HitPoints(50))) + .observe(take_damage) + .with_children(|parent| { + parent + .spawn((Name::new("Helmet"), Armor(5))) + .observe(block_attack); + parent + .spawn((Name::new("Socks"), Armor(10))) + .observe(block_attack); + parent + .spawn((Name::new("Shirt"), Armor(15))) + .observe(block_attack); + }); +} + +// This event represents an attack we want to "bubble" up from the armor to the goblin . +#[derive(Clone, Component)] +struct Attack { + damage: u16, +} + +// We enable propagation by implementing `Event` manually (rather than using a derive) and specifying +// two important pieces of information: +impl Event for Attack { + // 1. Which component we want to propagate along. In this case, we want to "bubble" (meaning propagate + // from child to parent) so we use the `Parent` component for propagation. The component supplied + // must implement the `Traversal` type. + type Traverse = Parent; + // 2. We can also choose whether or not this event will propagate by default when triggered. If this is + // false, it will only propagate following a call to `Trigger::propagate(true)`. + const AUTO_PROPAGATE: bool = true; +} + +/// An entity that can take damage. +#[derive(Component, Deref, DerefMut)] +struct HitPoints(u16); + +/// For damage to reach the wearer, it must exceed the armor. +#[derive(Component, Deref)] +struct Armor(u16); + +/// A normal bevy system that attacks a piece of the goblin's armor on a timer. +fn attack_armor(entities: Query>, mut commands: Commands) { + let mut rng = rand::thread_rng(); + if let Some(target) = entities.iter().choose(&mut rng) { + let damage = thread_rng().gen_range(1..20); + commands.trigger_targets(Attack { damage }, target); + info!("⚔️ Attack for {} damage", damage); + } +} + +fn attack_hits(mut trigger: Trigger, name: Query<&Name>) { + if let Ok(name) = name.get(trigger.entity()) { + info!("Attack hit {}", name); + } +} + +/// A callback placed on [`Armor`], checking if it absorbed all the [`Attack`] damage. +fn block_attack(mut trigger: Trigger, armor: Query<(&Armor, &Name)>) { + let (armor, name) = armor.get(trigger.entity()).unwrap(); + let attack = trigger.event_mut(); + let damage = attack.damage.saturating_sub(**armor); + if damage > 0 { + info!("🩸 {} damage passed through {}", damage, name); + // The attack isn't stopped by the armor. We reduce the damage of the attack, and allow + // it to continue on to the goblin. + attack.damage = damage; + } else { + info!("🛡️ {} damage blocked by {}", attack.damage, name); + // Armor stopped the attack, the event stops here. + trigger.propagate(false); + info!("(propagation halted early)\n"); + } +} + +/// A callback on the armor wearer, triggered when a piece of armor is not able to block an attack, +/// or the wearer is attacked directly. +fn take_damage( + trigger: Trigger, + mut hp: Query<(&mut HitPoints, &Name)>, + mut commands: Commands, + mut app_exit: EventWriter, +) { + let attack = trigger.event(); + let (mut hp, name) = hp.get_mut(trigger.entity()).unwrap(); + **hp = hp.saturating_sub(attack.damage); + + if **hp > 0 { + info!("{} has {:.1} HP", name, hp.0); + } else { + warn!("💀 {} has died a gruesome death", name); + commands.entity(trigger.entity()).despawn_recursive(); + app_exit.send(bevy::app::AppExit::Success); + } + + info!("(propagation reached root)\n"); +} From 0225a4295a36116fe4655c9de75e6e979135ed9e Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 30 Jun 2024 19:45:19 -0400 Subject: [PATCH 08/20] Fix cargo.toml error and bump MSRV --- Cargo.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 18d66d850753f..f4ff25086ab92 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ keywords = ["game", "engine", "gamedev", "graphics", "bevy"] license = "MIT OR Apache-2.0" repository = "https://github.com/bevyengine/bevy" documentation = "https://docs.rs/bevy" -rust-version = "1.78.0" +rust-version = "1.79.0" [workspace] exclude = [ @@ -2554,8 +2554,8 @@ name = "observer_propagation" path = "examples/ecs/observer_propagation.rs" doc-scrape-examples = true -[package.metadata.example.observers_propagation] -name = "Observers Propagation" +[package.metadata.example.observer_propagation] +name = "Observer Propagation" description = "Demonstrates event propagation with observers" category = "ECS (Entity Component System)" wasm = true From a7811887988731777b442e76430e7b72d1b410ca Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 30 Jun 2024 19:49:02 -0400 Subject: [PATCH 09/20] Update examples/README.md --- examples/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/README.md b/examples/README.md index 4cccac78600f8..42f00c271b83f 100644 --- a/examples/README.md +++ b/examples/README.md @@ -277,6 +277,7 @@ Example | Description [Hierarchy](../examples/ecs/hierarchy.rs) | Creates a hierarchy of parents and children entities [Iter Combinations](../examples/ecs/iter_combinations.rs) | Shows how to iterate over combinations of query results [Nondeterministic System Order](../examples/ecs/nondeterministic_system_order.rs) | Systems run in parallel, but their order isn't always deterministic. Here's how to detect and fix this. +[Observer Propagation](../examples/ecs/observer_propagation.rs) | Demonstrates event propagation with observers [Observers](../examples/ecs/observers.rs) | Demonstrates observers that react to events (both built-in life-cycle events and custom events) [One Shot Systems](../examples/ecs/one_shot_systems.rs) | Shows how to flexibly run systems without scheduling them [Parallel Query](../examples/ecs/parallel_query.rs) | Illustrates parallel queries with `ParallelIterator` From 595f621f5d7c78993388a92fc425f50f921b8a20 Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 30 Jun 2024 19:51:24 -0400 Subject: [PATCH 10/20] Fix typos --- crates/bevy_ecs/src/event/base.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/event/base.rs b/crates/bevy_ecs/src/event/base.rs index 59708bb03a19d..4941df135b420 100644 --- a/crates/bevy_ecs/src/event/base.rs +++ b/crates/bevy_ecs/src/event/base.rs @@ -32,12 +32,12 @@ use std::{ note = "consider annotating `{Self}` with `#[derive(Event)]`" )] pub trait Event: Component { - /// The component that describes which Entity to propigate this event to next, when [propagation] is enabled. + /// The component that describes which Entity to propagate this event to next, when [propagation] is enabled. /// /// [propagation]: crate::observers::Trigger::enable_propagation type Traverse: Traversal; - /// When true, this event will always attempt to propigate when [triggered], without requiring a call + /// When true, this event will always attempt to propagate when [triggered], without requiring a call /// to [`Trigger::enable_propagation`]. /// /// [triggered]: crate::commands::Commands::trigger_targets From f5a6a9981c53b234086473241e79dd51da25abda Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 30 Jun 2024 20:22:45 -0400 Subject: [PATCH 11/20] Fix clippy error in example --- examples/ecs/observer_propagation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/ecs/observer_propagation.rs b/examples/ecs/observer_propagation.rs index 92fd9242f6bb9..e896558696521 100644 --- a/examples/ecs/observer_propagation.rs +++ b/examples/ecs/observer_propagation.rs @@ -78,7 +78,7 @@ fn attack_armor(entities: Query>, mut commands: Commands) { } } -fn attack_hits(mut trigger: Trigger, name: Query<&Name>) { +fn attack_hits(trigger: Trigger, name: Query<&Name>) { if let Ok(name) = name.get(trigger.entity()) { info!("Attack hit {}", name); } From 76530d5bfcc8c5ff92a63ee47995b995432bef9c Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 30 Jun 2024 20:34:32 -0400 Subject: [PATCH 12/20] Fix lint in observers test --- crates/bevy_ecs/src/observer/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 91c27d5c49e31..69eeafa4f4363 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -915,7 +915,7 @@ mod tests { world.observe( |trigger: Trigger, query: Query<&A>, mut res: ResMut| { if query.get(trigger.entity()).is_ok() { - res.0 += 1 + res.0 += 1; } }, ); From 902ec1edc6d8eba1d1777c3ed074f45de0508ae8 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 7 Jul 2024 09:20:47 -0400 Subject: [PATCH 13/20] Apply suggestions from code review Co-authored-by: Torstein Grindvik <52322338+torsteingrindvik@users.noreply.github.com> --- crates/bevy_ecs/src/observer/mod.rs | 2 +- crates/bevy_ecs/src/world/deferred_world.rs | 2 +- examples/ecs/observer_propagation.rs | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 69eeafa4f4363..1dd373c7aef88 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -65,7 +65,7 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> { /// Enables or disables event propagation, allowing the same event to trigger observers on a chain of different entities. /// - /// The path an event will propagate along is specified by it's associated [`Traversal`] component. By default, events + /// The path an event will propagate along is specified by its associated [`Traversal`] component. By default, events /// use `TraverseNone` which ends the path immediately and prevents propagation. /// /// To enable propagation, you must: diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index 714a6024d4ab3..cd45be61687b9 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -381,7 +381,7 @@ impl<'w> DeferredWorld<'w> { self.reborrow(), event, entity, - components.iter().cloned(), + components.iter().copied(), data, &mut propagation, ); diff --git a/examples/ecs/observer_propagation.rs b/examples/ecs/observer_propagation.rs index e896558696521..4db5e28ae6e7a 100644 --- a/examples/ecs/observer_propagation.rs +++ b/examples/ecs/observer_propagation.rs @@ -19,11 +19,11 @@ fn main() { .run(); } -// In this example, we spawn a goblin wearing different pecies of armor. Each piece of armor +// In this example, we spawn a goblin wearing different pieces of armor. Each piece of armor // is represented as a child entity, with an `Armor` component. // // We're going to model how attack damage can be partally blocked by the goblin's armor using -// event bubling. Our events will target the armor, and if the armor isn't strong enough to block +// event bubbling. Our events will target the armor, and if the armor isn't strong enough to block // the attack it will continue up and hit the goblin. fn setup(mut commands: Commands) { commands @@ -42,7 +42,7 @@ fn setup(mut commands: Commands) { }); } -// This event represents an attack we want to "bubble" up from the armor to the goblin . +// This event represents an attack we want to "bubble" up from the armor to the goblin. #[derive(Clone, Component)] struct Attack { damage: u16, @@ -53,7 +53,7 @@ struct Attack { impl Event for Attack { // 1. Which component we want to propagate along. In this case, we want to "bubble" (meaning propagate // from child to parent) so we use the `Parent` component for propagation. The component supplied - // must implement the `Traversal` type. + // must implement the `Traversal` trait. type Traverse = Parent; // 2. We can also choose whether or not this event will propagate by default when triggered. If this is // false, it will only propagate following a call to `Trigger::propagate(true)`. From 3c02e79bd7210ada47c2afc7b32bac375c6eccfd Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 7 Jul 2024 09:28:00 -0400 Subject: [PATCH 14/20] Another typo bites the dust Co-authored-by: Torstein Grindvik <52322338+torsteingrindvik@users.noreply.github.com> --- examples/ecs/observer_propagation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/ecs/observer_propagation.rs b/examples/ecs/observer_propagation.rs index 4db5e28ae6e7a..89105bc55524f 100644 --- a/examples/ecs/observer_propagation.rs +++ b/examples/ecs/observer_propagation.rs @@ -22,7 +22,7 @@ fn main() { // In this example, we spawn a goblin wearing different pieces of armor. Each piece of armor // is represented as a child entity, with an `Armor` component. // -// We're going to model how attack damage can be partally blocked by the goblin's armor using +// We're going to model how attack damage can be partially blocked by the goblin's armor using // event bubbling. Our events will target the armor, and if the armor isn't strong enough to block // the attack it will continue up and hit the goblin. fn setup(mut commands: Commands) { From d89c61c8bcfa5bbf1bcbe372dd70f5ba218da9f6 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 8 Jul 2024 19:06:33 -0700 Subject: [PATCH 15/20] Use replace Propagation enum with `propagate` bool. Event::Traverse -> Event::Traversal (lets keep terminology the same). Add / fix some docs --- crates/bevy_ecs/macros/src/component.rs | 2 +- crates/bevy_ecs/src/event/base.rs | 10 +-- crates/bevy_ecs/src/observer/mod.rs | 71 +++++++------------ crates/bevy_ecs/src/observer/runner.rs | 8 +-- crates/bevy_ecs/src/observer/trigger_event.rs | 15 ++-- crates/bevy_ecs/src/world/deferred_world.rs | 10 +-- examples/ecs/observer_propagation.rs | 5 +- 7 files changed, 45 insertions(+), 76 deletions(-) diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index 996d12e291cd0..a5cab18311336 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -17,7 +17,7 @@ pub fn derive_event(input: TokenStream) -> TokenStream { TokenStream::from(quote! { impl #impl_generics #bevy_ecs_path::event::Event for #struct_name #type_generics #where_clause { - type Traverse = #bevy_ecs_path::traversal::TraverseNone; + type Traversal = #bevy_ecs_path::traversal::TraverseNone; const AUTO_PROPAGATE: bool = false; } diff --git a/crates/bevy_ecs/src/event/base.rs b/crates/bevy_ecs/src/event/base.rs index 4941df135b420..fc23f3b274571 100644 --- a/crates/bevy_ecs/src/event/base.rs +++ b/crates/bevy_ecs/src/event/base.rs @@ -34,14 +34,14 @@ use std::{ pub trait Event: Component { /// The component that describes which Entity to propagate this event to next, when [propagation] is enabled. /// - /// [propagation]: crate::observers::Trigger::enable_propagation - type Traverse: Traversal; + /// [propagation]: crate::observer::Trigger::propagate + type Traversal: Traversal; /// When true, this event will always attempt to propagate when [triggered], without requiring a call - /// to [`Trigger::enable_propagation`]. + /// to [`Trigger::propagate`]. /// - /// [triggered]: crate::commands::Commands::trigger_targets - /// [`Trigger::enable_propagation`]: crate::observers::Trigger::enable_propagation + /// [triggered]: crate::system::Commands::trigger_targets + /// [`Trigger::propagate`]: crate::observer::Trigger::propagate const AUTO_PROPAGATE: bool = false; } diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 1dd373c7aef88..90f3361d5066e 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -15,24 +15,21 @@ use bevy_utils::{EntityHashMap, HashMap}; 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. +/// [`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, - propagation: &'w mut Propagation, + propagate: &'w mut bool, trigger: ObserverTrigger, _marker: PhantomData, } impl<'w, E, B: Bundle> Trigger<'w, E, B> { /// Creates a new trigger for the given event and observer information. - pub fn new( - event: &'w mut E, - propagation: &'w mut Propagation, - trigger: ObserverTrigger, - ) -> Self { + pub fn new(event: &'w mut E, propagate: &'w mut bool, trigger: ObserverTrigger) -> Self { Self { event, - propagation, + propagate, trigger, _marker: PhantomData, } @@ -69,26 +66,19 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> { /// use `TraverseNone` which ends the path immediately and prevents propagation. /// /// To enable propagation, you must: - /// + Set [`Event::Traverse`] to the component you want to propagate along. + /// + Set [`Event::Traversal`] to the component you want to propagate along. /// + Either call `propagate(true)` in the first observer or set [`Event::AUTO_PROPAGATE`] to `true`. /// /// You can prevent an event from propagating further using `propagate(false)`. + /// + /// [`Traversal`]: crate::traversal::Traversal pub fn propagate(&mut self, should_propagate: bool) { - *self.propagation = if should_propagate { - Propagation::Continue - } else { - Propagation::Halt - }; - } - - /// Returns a reference to the flag that controls event propagation. See [`propagate`] for more information. - pub fn get_propagation(&self) -> &Propagation { - self.propagation + *self.propagate = should_propagate; } - /// Returns a mutable reference to the flag that controls event propagation. See [`propagate`] for more information. - pub fn get_propagation_mut(&mut self) -> &mut Propagation { - self.propagation + /// Returns the value of the flag that controls event propagation. See [`propagate`] for more information. + pub fn get_propagate(&self) -> bool { + *self.propagate } } @@ -148,15 +138,6 @@ pub struct ObserverTrigger { pub entity: Entity, } -/// Determines if the event should propagate to other entities. -#[derive(Eq, PartialEq, Copy, Clone, Debug)] -pub enum Propagation { - /// Allows propagation to continue. - Continue, - /// Halts propagation. - Halt, -} - // Map between an observer entity and its runner type ObserverMap = EntityHashMap; @@ -217,7 +198,7 @@ impl Observers { entity: Entity, components: impl Iterator, data: &mut T, - propagation: &mut Propagation, + propagate: &mut bool, ) { // SAFETY: You cannot get a mutable reference to `observers` from `DeferredWorld` let (mut world, observers) = unsafe { @@ -241,7 +222,7 @@ impl Observers { entity, }, data.into(), - propagation, + propagate, ); }; // Trigger observers listening for any kind of this trigger @@ -479,7 +460,7 @@ mod tests { struct EventPropagating; impl Event for EventPropagating { - type Traverse = Parent; + type Traversal = Parent; const AUTO_PROPAGATE: bool = true; } @@ -712,7 +693,7 @@ mod tests { world.spawn(ObserverState { // SAFETY: we registered `event_a` above and it matches the type of TriggerA descriptor: unsafe { ObserverDescriptor::default().with_events(vec![event_a]) }, - runner: |mut world, _trigger, _ptr, _propagation| { + runner: |mut world, _trigger, _ptr, _propagate| { world.resource_mut::().0 += 1; }, ..Default::default() @@ -784,12 +765,10 @@ mod tests { let child = world .spawn(Parent(parent)) - .observe( - |mut trigger: Trigger, mut res: ResMut| { - res.0 += 1; - trigger.propagate(false); - }, - ) + .observe(|trigger: Trigger, mut res: ResMut| { + res.0 += 1; + *trigger.propagate = false; + }) .id(); // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut @@ -862,12 +841,10 @@ mod tests { let child_a = world .spawn(Parent(parent_a)) - .observe( - |mut trigger: Trigger, mut res: ResMut| { - res.0 += 1; - trigger.propagate(false); - }, - ) + .observe(|trigger: Trigger, mut res: ResMut| { + res.0 += 1; + *trigger.propagate = false; + }) .id(); let parent_b = world diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 1279df50dd338..036c54105e36c 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, Propagation}, + observer::{ObserverDescriptor, ObserverTrigger}, prelude::*, query::DebugCheckedUnwrap, system::{IntoObserverSystem, ObserverSystem}, @@ -86,7 +86,7 @@ impl Component for ObserverState { /// Type for function that is run when an observer is triggered. /// Typically refers to the default runner that runs the system stored in the associated [`ObserverSystemComponent`], /// but can be overridden for custom behaviour. -pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, &mut Propagation); +pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, propagate: &mut bool); /// An [`Observer`] system. Add this [`Component`] to an [`Entity`] to turn it into an "observer". /// @@ -358,7 +358,7 @@ fn observer_system_runner( mut world: DeferredWorld, observer_trigger: ObserverTrigger, ptr: PtrMut, - propagation: &mut Propagation, + propagate: &mut bool, ) { let world = world.as_unsafe_world_cell(); // SAFETY: Observer was triggered so must still exist in world @@ -385,7 +385,7 @@ fn observer_system_runner( let trigger: Trigger = Trigger::new( // SAFETY: Caller ensures `ptr` is castable to `&mut T` unsafe { ptr.deref_mut() }, - propagation, + propagate, observer_trigger, ); // SAFETY: the static lifetime is encapsulated in Trigger / cannot leak out. diff --git a/crates/bevy_ecs/src/observer/trigger_event.rs b/crates/bevy_ecs/src/observer/trigger_event.rs index 7c00e404419ca..fea35c0782f2e 100644 --- a/crates/bevy_ecs/src/observer/trigger_event.rs +++ b/crates/bevy_ecs/src/observer/trigger_event.rs @@ -2,7 +2,6 @@ use crate::{ component::ComponentId, entity::Entity, event::Event, - observer::Propagation, world::{Command, DeferredWorld, World}, }; @@ -59,30 +58,24 @@ fn trigger_event( if targets.entities().is_empty() { // SAFETY: T is accessible as the type represented by self.trigger, ensured in `Self::new` unsafe { - world.trigger_observers_with_data::<_, E::Traverse>( + world.trigger_observers_with_data::<_, E::Traversal>( event_type, Entity::PLACEHOLDER, targets.components(), event_data, - Propagation::Halt, + false, ); }; } else { for target in targets.entities() { // SAFETY: T is accessible as the type represented by self.trigger, ensured in `Self::new` unsafe { - world.trigger_observers_with_data::<_, E::Traverse>( + world.trigger_observers_with_data::<_, E::Traversal>( event_type, *target, targets.components(), event_data, - const { - if E::AUTO_PROPAGATE { - Propagation::Continue - } else { - Propagation::Halt - } - }, + E::AUTO_PROPAGATE, ); }; } diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index cd45be61687b9..caebdaa979ad4 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -6,7 +6,7 @@ use crate::{ component::ComponentId, entity::Entity, event::{Event, EventId, Events, SendBatchIds}, - observer::{Observers, Propagation, TriggerTargets}, + observer::{Observers, TriggerTargets}, prelude::{Component, QueryState}, query::{QueryData, QueryFilter}, system::{Commands, Query, Resource}, @@ -357,7 +357,7 @@ impl<'w> DeferredWorld<'w> { entity, components, &mut (), - &mut Propagation::Halt, + &mut false, ); } @@ -372,7 +372,7 @@ impl<'w> DeferredWorld<'w> { mut entity: Entity, components: &[ComponentId], data: &mut E, - mut propagation: Propagation, + mut propagate: bool, ) where C: Traversal, { @@ -383,9 +383,9 @@ impl<'w> DeferredWorld<'w> { entity, components.iter().copied(), data, - &mut propagation, + &mut propagate, ); - if propagation == Propagation::Halt { + if !propagate { break; } if let Some(next) = self.get::(entity).and_then(|t| t.next()) { diff --git a/examples/ecs/observer_propagation.rs b/examples/ecs/observer_propagation.rs index 89105bc55524f..f51b6e5f902c3 100644 --- a/examples/ecs/observer_propagation.rs +++ b/examples/ecs/observer_propagation.rs @@ -7,8 +7,7 @@ use rand::{seq::IteratorRandom, thread_rng, Rng}; fn main() { App::new() - .add_plugins(MinimalPlugins) - .add_plugins(LogPlugin::default()) + .add_plugins((MinimalPlugins, LogPlugin::default())) .add_systems(Startup, setup) .add_systems( Update, @@ -54,7 +53,7 @@ impl Event for Attack { // 1. Which component we want to propagate along. In this case, we want to "bubble" (meaning propagate // from child to parent) so we use the `Parent` component for propagation. The component supplied // must implement the `Traversal` trait. - type Traverse = Parent; + type Traversal = Parent; // 2. We can also choose whether or not this event will propagate by default when triggered. If this is // false, it will only propagate following a call to `Trigger::propagate(true)`. const AUTO_PROPAGATE: bool = true; From 33d2820561d149895f8bd5a5107a21264834d5ed Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 14 Jul 2024 16:25:33 -0400 Subject: [PATCH 16/20] Respond to final round of review feadback --- crates/bevy_ecs/src/observer/mod.rs | 49 ++++++++++++++----- crates/bevy_ecs/src/traversal.rs | 10 ++-- crates/bevy_ecs/src/world/deferred_world.rs | 4 +- .../bevy_hierarchy/src/components/parent.rs | 7 ++- 4 files changed, 53 insertions(+), 17 deletions(-) diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 90f3361d5066e..49f0e89c8b2e2 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -76,7 +76,7 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> { *self.propagate = should_propagate; } - /// Returns the value of the flag that controls event propagation. See [`propagate`] for more information. + /// Returns the value of the flag that controls event propagation. See [`propagate`] for more information. pub fn get_propagate(&self) -> bool { *self.propagate } @@ -451,7 +451,7 @@ mod tests { struct Parent(Entity); impl Traversal for Parent { - fn next(&self) -> Option { + fn traverse(&self) -> Option { Some(self.0) } } @@ -731,7 +731,30 @@ mod tests { } #[test] - fn observer_propagating_redundant_dispatch() { + fn observer_propagating_redundant_dispatch_same_entity() { + let mut world = World::new(); + world.init_resource::(); + + let parent = world + .spawn_empty() + .observe(|_: Trigger, mut res: ResMut| res.0 += 1) + .id(); + + let child = world + .spawn(Parent(parent)) + .observe(|_: Trigger, mut res: ResMut| 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, child]); + world.flush(); + assert_eq!(4, world.resource::().0); + } + + #[test] + fn observer_propagating_redundant_dispatch_parent_child() { let mut world = World::new(); world.init_resource::(); @@ -765,10 +788,12 @@ mod tests { let child = world .spawn(Parent(parent)) - .observe(|trigger: Trigger, mut res: ResMut| { - res.0 += 1; - *trigger.propagate = false; - }) + .observe( + |mut trigger: Trigger, mut res: ResMut| { + res.0 += 1; + trigger.propagate(false); + }, + ) .id(); // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut @@ -841,10 +866,12 @@ mod tests { let child_a = world .spawn(Parent(parent_a)) - .observe(|trigger: Trigger, mut res: ResMut| { - res.0 += 1; - *trigger.propagate = false; - }) + .observe( + |mut trigger: Trigger, mut res: ResMut| { + res.0 += 1; + trigger.propagate(false); + }, + ) .id(); let parent_b = world diff --git a/crates/bevy_ecs/src/traversal.rs b/crates/bevy_ecs/src/traversal.rs index d44d05620baf1..a1dee8cf56a42 100644 --- a/crates/bevy_ecs/src/traversal.rs +++ b/crates/bevy_ecs/src/traversal.rs @@ -11,14 +11,18 @@ use crate::{ /// events use the [`TraverseNone`] placeholder component, which cannot actually be created or added to /// an entity and so never causes traversal. /// -/// The implementer is responsible for ensuring that `Traversal::next` cannot produce infinite loops. +/// Infinite loops are possible, and are not checked for. While looping can be desireable in some contexts +/// (for example, an observer that triggers itself multiple times before stopping), following an infinite +/// traversal loop without an eventual exit will can your application to hang. Each implementer of `Traversal` +/// for documenting possible looping behavior, and consumers of those implementations are responsible for +/// avoiding infinite loops in their code. /// /// [specify the direction]: crate::event::Event::Traverse /// [event propagation]: crate::observer::Trigger::propagate /// [observers]: crate::observer::Observer pub trait Traversal: Component { /// Returns the next entity to visit. - fn next(&self) -> Option; + fn traverse(&self) -> Option; } /// A traversal component that doesn't traverse anything. Used to provide a default traversal @@ -29,7 +33,7 @@ pub enum TraverseNone {} impl Traversal for TraverseNone { #[inline(always)] - fn next(&self) -> Option { + fn traverse(&self) -> Option { None } } diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index caebdaa979ad4..33c39d5266992 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -388,8 +388,8 @@ impl<'w> DeferredWorld<'w> { if !propagate { break; } - if let Some(next) = self.get::(entity).and_then(|t| t.next()) { - entity = next; + if let Some(traverse_to) = self.get::(entity).and_then(|t| t.traverse()) { + entity = traverse_to; } else { break; } diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index 8c6407d031a9b..d9d4e57e1eadc 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -71,8 +71,13 @@ impl Deref for Parent { } } +/// This provides generalized hiarchy traversal for use in [event propagation]. +/// +/// `Parent::traverse` will never form loops in properly-constructed hierarchies. +/// +/// [event propagation]: crate::observer::Trigger::propagate impl Traversal for Parent { - fn next(&self) -> Option { + fn traverse(&self) -> Option { Some(self.0) } } From f7239bbda255b23a10dff666dd3978fe2e2b5626 Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 14 Jul 2024 17:56:11 -0400 Subject: [PATCH 17/20] Port over bevy_eventlistener benchmark --- benches/Cargo.toml | 6 +- benches/benches/bevy_ecs/benches.rs | 2 + benches/benches/bevy_ecs/observers/mod.rs | 6 + .../benches/bevy_ecs/observers/propagation.rs | 151 ++++++++++++++++++ 4 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 benches/benches/bevy_ecs/observers/mod.rs create mode 100644 benches/benches/bevy_ecs/observers/propagation.rs diff --git a/benches/Cargo.toml b/benches/Cargo.toml index 3df074a75c045..d1163b0cd1db8 100644 --- a/benches/Cargo.toml +++ b/benches/Cargo.toml @@ -12,11 +12,13 @@ rand_chacha = "0.3" criterion = { version = "0.3", features = ["html_reports"] } bevy_app = { path = "../crates/bevy_app" } bevy_ecs = { path = "../crates/bevy_ecs", features = ["multi_threaded"] } +bevy_hierarchy = { path = "../crates/bevy_hierarchy" } +bevy_internal = { path = "../crates/bevy_internal" } +bevy_math = { path = "../crates/bevy_math" } bevy_reflect = { path = "../crates/bevy_reflect" } +bevy_render = { path = "../crates/bevy_render" } bevy_tasks = { path = "../crates/bevy_tasks" } bevy_utils = { path = "../crates/bevy_utils" } -bevy_math = { path = "../crates/bevy_math" } -bevy_render = { path = "../crates/bevy_render" } [profile.release] opt-level = 3 diff --git a/benches/benches/bevy_ecs/benches.rs b/benches/benches/bevy_ecs/benches.rs index 6f1e89fb6d316..b3db6abe770d1 100644 --- a/benches/benches/bevy_ecs/benches.rs +++ b/benches/benches/bevy_ecs/benches.rs @@ -3,6 +3,7 @@ use criterion::criterion_main; mod components; mod events; mod iteration; +mod observers; mod scheduling; mod world; @@ -10,6 +11,7 @@ criterion_main!( components::components_benches, events::event_benches, iteration::iterations_benches, + observers::observer_benches, scheduling::scheduling_benches, world::world_benches, ); diff --git a/benches/benches/bevy_ecs/observers/mod.rs b/benches/benches/bevy_ecs/observers/mod.rs new file mode 100644 index 0000000000000..15ac476abc489 --- /dev/null +++ b/benches/benches/bevy_ecs/observers/mod.rs @@ -0,0 +1,6 @@ +use criterion::criterion_group; + +mod propagation; +use propagation::*; + +criterion_group!(observer_benches, event_propagation); diff --git a/benches/benches/bevy_ecs/observers/propagation.rs b/benches/benches/bevy_ecs/observers/propagation.rs new file mode 100644 index 0000000000000..ed22b1b519a06 --- /dev/null +++ b/benches/benches/bevy_ecs/observers/propagation.rs @@ -0,0 +1,151 @@ +use bevy_app::{App, First, Startup}; +use bevy_ecs::{ + component::Component, + entity::Entity, + event::{Event, EventWriter}, + observer::Trigger, + query::{Or, With, Without}, + system::{Commands, EntityCommands, Query}, +}; +use bevy_hierarchy::{BuildChildren, Children, Parent}; +use bevy_internal::MinimalPlugins; + +use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use rand::{seq::IteratorRandom, Rng}; + +const DENSITY: usize = 20; // percent of nodes with listeners +const ENTITY_DEPTH: usize = 64; +const ENTITY_WIDTH: usize = 200; +const N_EVENTS: usize = 500; + +pub fn event_propagation(criterion: &mut Criterion) { + let mut group = criterion.benchmark_group("event_propagation"); + group.warm_up_time(std::time::Duration::from_millis(500)); + group.measurement_time(std::time::Duration::from_secs(4)); + + group.bench_function("baseline", |bencher| { + let mut app = App::new(); + app.add_plugins(MinimalPlugins) + .add_systems(Startup, spawn_listener_hierarchy); + app.update(); + + bencher.iter(|| { + black_box(app.update()); + }); + }); + + group.bench_function("single_event_type", |bencher| { + let mut app = App::new(); + app.add_plugins(MinimalPlugins) + .add_systems( + Startup, + ( + spawn_listener_hierarchy, + add_listeners_to_hierarchy::, + ), + ) + .add_systems(First, send_events::<1, N_EVENTS>); + app.update(); + + bencher.iter(|| { + black_box(app.update()); + }); + }); + + group.bench_function("single_event_type_no_listeners", |bencher| { + let mut app = App::new(); + app.add_plugins(MinimalPlugins) + .add_systems( + Startup, + ( + spawn_listener_hierarchy, + add_listeners_to_hierarchy::, + ), + ) + .add_systems(First, send_events::<9, N_EVENTS>); + app.update(); + + bencher.iter(|| { + black_box(app.update()); + }); + }); + + group.bench_function("four_event_types", |bencher| { + let mut app = App::new(); + const FRAC_N_EVENTS_4: usize = N_EVENTS / 4; + const FRAC_DENSITY_4: usize = DENSITY / 4; + + app.add_plugins(MinimalPlugins) + .add_systems( + Startup, + ( + spawn_listener_hierarchy, + add_listeners_to_hierarchy::, + add_listeners_to_hierarchy::, + add_listeners_to_hierarchy::, + add_listeners_to_hierarchy::, + ), + ) + .add_systems(First, send_events::<1, FRAC_N_EVENTS_4>) + .add_systems(First, send_events::<2, FRAC_N_EVENTS_4>) + .add_systems(First, send_events::<3, FRAC_N_EVENTS_4>) + .add_systems(First, send_events::<4, FRAC_N_EVENTS_4>); + app.update(); + + bencher.iter(|| { + black_box(app.update()); + }); + }); + + group.finish(); +} + +#[derive(Clone, Component)] +struct TestEvent {} + +impl Event for TestEvent { + type Traversal = Parent; + const AUTO_PROPAGATE: bool = true; +} + +fn send_events( + mut commands: Commands, + entities: Query>, +) { + let target = entities.iter().choose(&mut rand::thread_rng()).unwrap(); + (0..N_EVENTS).for_each(|_| { + commands.trigger_targets(TestEvent:: {}, target); + }); +} + +fn spawn_listener_hierarchy(mut commands: Commands) { + for _ in 0..ENTITY_WIDTH { + let mut parent = commands.spawn_empty().id(); + for _ in 0..ENTITY_DEPTH { + let child = commands.spawn_empty().id(); + commands.entity(parent).add_child(child); + parent = child; + } + } +} + +fn empty_listener(_trigger: Trigger>) {} + +fn add_listeners_to_hierarchy( + mut commands: Commands, + roots_and_leaves: Query, Without)>>, + nodes: Query, With)>, +) { + for entity in &roots_and_leaves { + commands.entity(entity).observe(empty_listener::); + } + for entity in &nodes { + maybe_insert_listener::(&mut commands.entity(entity)); + } +} + +fn maybe_insert_listener(commands: &mut EntityCommands) { + if rand::thread_rng().gen_bool(DENSITY as f64 / 100.0) { + commands.observe(empty_listener::); + } +} From 932630d7533bd41f2e1ee633af9fd15659350ca2 Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 14 Jul 2024 17:58:04 -0400 Subject: [PATCH 18/20] Appease clippy --- crates/bevy_ecs/src/world/deferred_world.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index 33c39d5266992..3d2edc020c623 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -388,7 +388,7 @@ impl<'w> DeferredWorld<'w> { if !propagate { break; } - if let Some(traverse_to) = self.get::(entity).and_then(|t| t.traverse()) { + if let Some(traverse_to) = self.get::(entity).and_then(C::traverse) { entity = traverse_to; } else { break; From 116b92dea89950e46176604c82d97653a9877a72 Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 14 Jul 2024 20:45:39 -0400 Subject: [PATCH 19/20] Fixup docs --- crates/bevy_ecs/src/observer/mod.rs | 2 ++ crates/bevy_ecs/src/traversal.rs | 4 ++-- crates/bevy_hierarchy/src/components/parent.rs | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 49f0e89c8b2e2..0ea1e8784667d 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -77,6 +77,8 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> { } /// Returns the value of the flag that controls event propagation. See [`propagate`] for more information. + /// + /// [`propigate`]: Trigger::propagate pub fn get_propagate(&self) -> bool { *self.propagate } diff --git a/crates/bevy_ecs/src/traversal.rs b/crates/bevy_ecs/src/traversal.rs index a1dee8cf56a42..7e4705f1eb097 100644 --- a/crates/bevy_ecs/src/traversal.rs +++ b/crates/bevy_ecs/src/traversal.rs @@ -11,13 +11,13 @@ use crate::{ /// events use the [`TraverseNone`] placeholder component, which cannot actually be created or added to /// an entity and so never causes traversal. /// -/// Infinite loops are possible, and are not checked for. While looping can be desireable in some contexts +/// Infinite loops are possible, and are not checked for. While looping can be desirable in some contexts /// (for example, an observer that triggers itself multiple times before stopping), following an infinite /// traversal loop without an eventual exit will can your application to hang. Each implementer of `Traversal` /// for documenting possible looping behavior, and consumers of those implementations are responsible for /// avoiding infinite loops in their code. /// -/// [specify the direction]: crate::event::Event::Traverse +/// [specify the direction]: crate::event::Event::Traversal /// [event propagation]: crate::observer::Trigger::propagate /// [observers]: crate::observer::Observer pub trait Traversal: Component { diff --git a/crates/bevy_hierarchy/src/components/parent.rs b/crates/bevy_hierarchy/src/components/parent.rs index d9d4e57e1eadc..99d8e45460a8d 100644 --- a/crates/bevy_hierarchy/src/components/parent.rs +++ b/crates/bevy_hierarchy/src/components/parent.rs @@ -71,11 +71,11 @@ impl Deref for Parent { } } -/// This provides generalized hiarchy traversal for use in [event propagation]. +/// This provides generalized hierarchy traversal for use in [event propagation]. /// /// `Parent::traverse` will never form loops in properly-constructed hierarchies. /// -/// [event propagation]: crate::observer::Trigger::propagate +/// [event propagation]: bevy_ecs::observer::Trigger::propagate impl Traversal for Parent { fn traverse(&self) -> Option { Some(self.0) From e29fd6e21c725723d1f96bfec37e4f75319a768e Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Sun, 14 Jul 2024 20:47:26 -0400 Subject: [PATCH 20/20] Fix one last typo --- crates/bevy_ecs/src/observer/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 0ea1e8784667d..82e7268f299ac 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -78,7 +78,7 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> { /// Returns the value of the flag that controls event propagation. See [`propagate`] for more information. /// - /// [`propigate`]: Trigger::propagate + /// [`propagate`]: Trigger::propagate pub fn get_propagate(&self) -> bool { *self.propagate }