Bubbling observers traversal should use query data (#15385)

# Objective

Fixes #14331

## Solution

- Make `Traversal` a subtrait of `ReadOnlyQueryData`
- Update implementations and usages

## Testing

- Updated unit tests

## Migration Guide

Update implementations of `Traversal`.

---------

Co-authored-by: Christian Hughes <9044780+ItsDoot@users.noreply.github.com>
This commit is contained in:
Benjamin Brienen 2024-09-23 20:08:36 +02:00 committed by GitHub
parent 83356b12c9
commit 27bea6abf7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 37 additions and 49 deletions

View file

@ -17,7 +17,7 @@ impl<const SIZE: usize> Benchmark<SIZE> {
} }
pub fn run(&mut self) { pub fn run(&mut self) {
let mut reader = self.0.get_reader(); let mut reader = self.0.get_cursor();
for evt in reader.read(&self.0) { for evt in reader.read(&self.0) {
std::hint::black_box(evt); std::hint::black_box(evt);
} }

View file

@ -1,14 +1,14 @@
use bevy_ecs::{ use bevy_ecs::{
component::Component, component::Component,
entity::Entity, entity::Entity,
event::{Event, EventWriter}, event::Event,
observer::Trigger, observer::Trigger,
world::World, world::World,
}; };
use bevy_hierarchy::{BuildChildren, Children, Parent}; use bevy_hierarchy::{BuildChildren, Parent};
use criterion::{black_box, criterion_group, criterion_main, Criterion}; use criterion::{black_box, Criterion};
use rand::{prelude::SliceRandom, SeedableRng}; use rand::SeedableRng;
use rand::{seq::IteratorRandom, Rng}; use rand::{seq::IteratorRandom, Rng};
use rand_chacha::ChaCha8Rng; use rand_chacha::ChaCha8Rng;
@ -71,7 +71,7 @@ pub fn event_propagation(criterion: &mut Criterion) {
struct TestEvent<const N: usize> {} struct TestEvent<const N: usize> {}
impl<const N: usize> Event for TestEvent<N> { impl<const N: usize> Event for TestEvent<N> {
type Traversal = Parent; type Traversal = &'static Parent;
const AUTO_PROPAGATE: bool = true; const AUTO_PROPAGATE: bool = true;
} }

View file

@ -1,6 +1,6 @@
use bevy_ecs::{entity::Entity, event::Event, observer::Trigger, world::World}; use bevy_ecs::{entity::Entity, event::Event, observer::Trigger, world::World};
use criterion::{black_box, criterion_group, criterion_main, Criterion}; use criterion::{black_box, Criterion};
use rand::{prelude::SliceRandom, SeedableRng}; use rand::{prelude::SliceRandom, SeedableRng};
use rand_chacha::ChaCha8Rng; use rand_chacha::ChaCha8Rng;
fn deterministic_rand() -> ChaCha8Rng { fn deterministic_rand() -> ChaCha8Rng {

View file

@ -43,7 +43,7 @@ pub fn spawn_commands(criterion: &mut Criterion) {
bencher.iter(|| { bencher.iter(|| {
let mut commands = Commands::new(&mut command_queue, &world); let mut commands = Commands::new(&mut command_queue, &world);
for i in 0..entity_count { for i in 0..entity_count {
let mut entity = commands let entity = commands
.spawn_empty() .spawn_empty()
.insert_if(A, || black_box(i % 2 == 0)) .insert_if(A, || black_box(i % 2 == 0))
.insert_if(B, || black_box(i % 3 == 0)) .insert_if(B, || black_box(i % 3 == 0))

View file

@ -1,5 +1,4 @@
use bevy_reflect::func::{ArgList, IntoFunction, IntoFunctionMut, TypedFunction}; use bevy_reflect::func::{ArgList, IntoFunction, IntoFunctionMut, TypedFunction};
use bevy_reflect::prelude::*;
use criterion::{criterion_group, criterion_main, BatchSize, Criterion}; use criterion::{criterion_group, criterion_main, BatchSize, Criterion};
criterion_group!(benches, typed, into, call, clone); criterion_group!(benches, typed, into, call, clone);

View file

@ -26,7 +26,7 @@ pub fn derive_event(input: TokenStream) -> TokenStream {
TokenStream::from(quote! { TokenStream::from(quote! {
impl #impl_generics #bevy_ecs_path::event::Event for #struct_name #type_generics #where_clause { impl #impl_generics #bevy_ecs_path::event::Event for #struct_name #type_generics #where_clause {
type Traversal = #bevy_ecs_path::traversal::TraverseNone; type Traversal = ();
const AUTO_PROPAGATE: bool = false; const AUTO_PROPAGATE: bool = false;
} }

View file

@ -91,7 +91,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. /// 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 its 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. /// use `()` which ends the path immediately and prevents propagation.
/// ///
/// To enable propagation, you must: /// To enable propagation, you must:
/// + Set [`Event::Traversal`] to the component you want to propagate along. /// + Set [`Event::Traversal`] to the component you want to propagate along.
@ -565,9 +565,9 @@ mod tests {
#[derive(Component)] #[derive(Component)]
struct Parent(Entity); struct Parent(Entity);
impl Traversal for Parent { impl Traversal for &'_ Parent {
fn traverse(&self) -> Option<Entity> { fn traverse(item: Self::Item<'_>) -> Option<Entity> {
Some(self.0) Some(item.0)
} }
} }
@ -575,7 +575,7 @@ mod tests {
struct EventPropagating; struct EventPropagating;
impl Event for EventPropagating { impl Event for EventPropagating {
type Traversal = Parent; type Traversal = &'static Parent;
const AUTO_PROPAGATE: bool = true; const AUTO_PROPAGATE: bool = true;
} }

View file

@ -1,15 +1,11 @@
//! A trait for components that let you traverse the ECS. //! A trait for components that let you traverse the ECS.
use crate::{ use crate::{entity::Entity, query::ReadOnlyQueryData};
component::{Component, StorageType},
entity::Entity,
};
/// A component that can point to another entity, and which can be used to define a path through the ECS. /// A component that can point to another entity, and which can be used to define a path through the ECS.
/// ///
/// Traversals are used to [specify the direction] of [event propagation] in [observers]. By default, /// Traversals are used to [specify the direction] of [event propagation] in [observers].
/// events use the [`TraverseNone`] placeholder component, which cannot actually be created or added to /// The default query is `()`.
/// an entity and so never causes traversal.
/// ///
/// Infinite loops are possible, and are not checked for. While looping can be desirable 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 /// (for example, an observer that triggers itself multiple times before stopping), following an infinite
@ -20,24 +16,13 @@ use crate::{
/// [specify the direction]: crate::event::Event::Traversal /// [specify the direction]: crate::event::Event::Traversal
/// [event propagation]: crate::observer::Trigger::propagate /// [event propagation]: crate::observer::Trigger::propagate
/// [observers]: crate::observer::Observer /// [observers]: crate::observer::Observer
pub trait Traversal: Component { pub trait Traversal: ReadOnlyQueryData {
/// Returns the next entity to visit. /// Returns the next entity to visit.
fn traverse(&self) -> Option<Entity>; fn traverse(item: Self::Item<'_>) -> Option<Entity>;
} }
/// A traversal component that doesn't traverse anything. Used to provide a default traversal impl Traversal for () {
/// implementation for events. fn traverse(_: Self::Item<'_>) -> Option<Entity> {
///
/// It is not possible to actually construct an instance of this component.
pub enum TraverseNone {}
impl Traversal for TraverseNone {
#[inline(always)]
fn traverse(&self) -> Option<Entity> {
None None
} }
} }
impl Component for TraverseNone {
const STORAGE_TYPE: StorageType = StorageType::Table;
}

View file

@ -148,8 +148,8 @@ impl<'w> DeferredWorld<'w> {
match self.get_resource_mut() { match self.get_resource_mut() {
Some(x) => x, Some(x) => x,
None => panic!( None => panic!(
"Requested resource {} does not exist in the `World`. "Requested resource {} does not exist in the `World`.
Did you forget to add it using `app.insert_resource` / `app.init_resource`? Did you forget to add it using `app.insert_resource` / `app.init_resource`?
Resources are also implicitly added via `app.add_event`, Resources are also implicitly added via `app.add_event`,
and can be added by plugins.", and can be added by plugins.",
std::any::type_name::<R>() std::any::type_name::<R>()
@ -178,8 +178,8 @@ impl<'w> DeferredWorld<'w> {
match self.get_non_send_resource_mut() { match self.get_non_send_resource_mut() {
Some(x) => x, Some(x) => x,
None => panic!( None => panic!(
"Requested non-send resource {} does not exist in the `World`. "Requested non-send resource {} does not exist in the `World`.
Did you forget to add it using `app.insert_non_send_resource` / `app.init_non_send_resource`? Did you forget to add it using `app.insert_non_send_resource` / `app.init_non_send_resource`?
Non-send resources can also be added by plugins.", Non-send resources can also be added by plugins.",
std::any::type_name::<R>() std::any::type_name::<R>()
), ),
@ -387,7 +387,7 @@ impl<'w> DeferredWorld<'w> {
/// # Safety /// # Safety
/// Caller must ensure `E` is accessible as the type represented by `event` /// Caller must ensure `E` is accessible as the type represented by `event`
#[inline] #[inline]
pub(crate) unsafe fn trigger_observers_with_data<E, C>( pub(crate) unsafe fn trigger_observers_with_data<E, T>(
&mut self, &mut self,
event: ComponentId, event: ComponentId,
mut entity: Entity, mut entity: Entity,
@ -395,7 +395,7 @@ impl<'w> DeferredWorld<'w> {
data: &mut E, data: &mut E,
mut propagate: bool, mut propagate: bool,
) where ) where
C: Traversal, T: Traversal,
{ {
loop { loop {
Observers::invoke::<_>( Observers::invoke::<_>(
@ -409,7 +409,11 @@ impl<'w> DeferredWorld<'w> {
if !propagate { if !propagate {
break; break;
} }
if let Some(traverse_to) = self.get::<C>(entity).and_then(C::traverse) { if let Some(traverse_to) = self
.get_entity(entity)
.and_then(|entity| entity.get_components::<T>())
.and_then(T::traverse)
{
entity = traverse_to; entity = traverse_to;
} else { } else {
break; break;

View file

@ -79,8 +79,8 @@ impl Deref for Parent {
/// `Parent::traverse` will never form loops in properly-constructed hierarchies. /// `Parent::traverse` will never form loops in properly-constructed hierarchies.
/// ///
/// [event propagation]: bevy_ecs::observer::Trigger::propagate /// [event propagation]: bevy_ecs::observer::Trigger::propagate
impl Traversal for Parent { impl Traversal for &Parent {
fn traverse(&self) -> Option<Entity> { fn traverse(item: Self::Item<'_>) -> Option<Entity> {
Some(self.0) Some(item.0)
} }
} }

View file

@ -73,7 +73,7 @@ impl<E> Event for Pointer<E>
where where
E: Debug + Clone + Reflect, E: Debug + Clone + Reflect,
{ {
type Traversal = Parent; type Traversal = &'static Parent;
const AUTO_PROPAGATE: bool = true; const AUTO_PROPAGATE: bool = true;
} }

View file

@ -53,7 +53,7 @@ impl Event for Attack {
// 1. Which component we want to propagate along. In this case, we want to "bubble" (meaning propagate // 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 // from child to parent) so we use the `Parent` component for propagation. The component supplied
// must implement the `Traversal` trait. // must implement the `Traversal` trait.
type Traversal = Parent; type Traversal = &'static Parent;
// 2. We can also choose whether or not this event will propagate by default when triggered. If this is // 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)`. // false, it will only propagate following a call to `Trigger::propagate(true)`.
const AUTO_PROPAGATE: bool = true; const AUTO_PROPAGATE: bool = true;