Rename observe to observe_entity on EntityWorldMut (#15616)

# Objective

The current observers have some unfortunate footguns where you can end
up confused about what is actually being observed. For apps you can
chain observe like `app.observe(..).observe(..)` which works like you
would expect, but if you try the same with world the first `observe()`
will return the `EntityWorldMut` for the created observer, and the
second `observe()` will only observe on the observer entity. It took
several hours for multiple people on discord to figure this out, which
is not a great experience.

## Solution

Rename `observe` on entities to `observe_entity`. It's slightly more
verbose when you know you have an entity, but it feels right to me that
observers for specific things have more specific naming, and it prevents
this issue completely.

Another possible solution would be to unify `observe` on `App` and
`World` to have the same kind of return type, but I'm not sure exactly
what that would look like.

## Testing

Simple name change, so only concern is docs really.

---


## Migration Guide

The `observe()` method on entities has been renamed to
`observe_entity()` to prevent confusion about what is being observed in
some cases.
This commit is contained in:
Kristoffer Søholm 2024-10-03 19:05:26 +02:00 committed by GitHub
parent 2da8d17a44
commit 336c23c1aa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 52 additions and 36 deletions

View File

@ -1,9 +1,5 @@
use bevy_ecs::{ use bevy_ecs::{
component::Component, component::Component, entity::Entity, event::Event, observer::Trigger, world::World,
entity::Entity,
event::Event,
observer::Trigger,
world::World,
}; };
use bevy_hierarchy::{BuildChildren, Parent}; use bevy_hierarchy::{BuildChildren, Parent};
@ -110,15 +106,15 @@ fn add_listeners_to_hierarchy<const DENSITY: usize, const N: usize>(
world: &mut World, world: &mut World,
) { ) {
for e in roots.iter() { for e in roots.iter() {
world.entity_mut(*e).observe(empty_listener::<N>); world.entity_mut(*e).observe_entity(empty_listener::<N>);
} }
for e in leaves.iter() { for e in leaves.iter() {
world.entity_mut(*e).observe(empty_listener::<N>); world.entity_mut(*e).observe_entity(empty_listener::<N>);
} }
let mut rng = deterministic_rand(); let mut rng = deterministic_rand();
for e in nodes.iter() { for e in nodes.iter() {
if rng.gen_bool(DENSITY as f64 / 100.0) { if rng.gen_bool(DENSITY as f64 / 100.0) {
world.entity_mut(*e).observe(empty_listener::<N>); world.entity_mut(*e).observe_entity(empty_listener::<N>);
} }
} }
} }

View File

@ -29,7 +29,7 @@ pub fn observe_simple(criterion: &mut Criterion) {
let mut world = World::new(); let mut world = World::new();
let mut entities = vec![]; let mut entities = vec![];
for _ in 0..10000 { for _ in 0..10000 {
entities.push(world.spawn_empty().observe(empty_listener_base).id()); entities.push(world.spawn_empty().observe_entity(empty_listener_base).id());
} }
entities.shuffle(&mut deterministic_rand()); entities.shuffle(&mut deterministic_rand());
bencher.iter(|| { bencher.iter(|| {

View File

@ -25,7 +25,7 @@ pub(crate) fn send_events(world: &mut World, mut current_frame: Local<u32>) {
let path = format!("./screenshot-{}.png", *current_frame); let path = format!("./screenshot-{}.png", *current_frame);
world world
.spawn(Screenshot::primary_window()) .spawn(Screenshot::primary_window())
.observe(save_to_disk(path)); .observe_entity(save_to_disk(path));
info!("Took a screenshot at frame {}.", *current_frame); info!("Took a screenshot at frame {}.", *current_frame);
} }
// Custom events are forwarded to the world. // Custom events are forwarded to the world.

View File

@ -839,7 +839,7 @@ mod tests {
world world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventA>| panic!("Trigger routed to non-targeted entity.")); .observe_entity(|_: Trigger<EventA>| panic!("Trigger routed to non-targeted entity."));
world.observe(move |obs: Trigger<EventA>, mut res: ResMut<Order>| { world.observe(move |obs: Trigger<EventA>, mut res: ResMut<Order>| {
assert_eq!(obs.entity(), Entity::PLACEHOLDER); assert_eq!(obs.entity(), Entity::PLACEHOLDER);
res.observed("event_a"); res.observed("event_a");
@ -860,10 +860,10 @@ mod tests {
world world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventA>| panic!("Trigger routed to non-targeted entity.")); .observe_entity(|_: Trigger<EventA>| panic!("Trigger routed to non-targeted entity."));
let entity = world let entity = world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventA>, mut res: ResMut<Order>| res.observed("a_1")) .observe_entity(|_: Trigger<EventA>, mut res: ResMut<Order>| res.observed("a_1"))
.id(); .id();
world.observe(move |obs: Trigger<EventA>, mut res: ResMut<Order>| { world.observe(move |obs: Trigger<EventA>, mut res: ResMut<Order>| {
assert_eq!(obs.entity(), entity); assert_eq!(obs.entity(), entity);
@ -931,12 +931,16 @@ mod tests {
let parent = world let parent = world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("parent")) .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent");
})
.id(); .id();
let child = world let child = world
.spawn(Parent(parent)) .spawn(Parent(parent))
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("child")) .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child");
})
.id(); .id();
// TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
@ -954,12 +958,16 @@ mod tests {
let parent = world let parent = world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("parent")) .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent");
})
.id(); .id();
let child = world let child = world
.spawn(Parent(parent)) .spawn(Parent(parent))
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("child")) .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child");
})
.id(); .id();
// TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
@ -980,12 +988,16 @@ mod tests {
let parent = world let parent = world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("parent")) .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent");
})
.id(); .id();
let child = world let child = world
.spawn(Parent(parent)) .spawn(Parent(parent))
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("child")) .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child");
})
.id(); .id();
// TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
@ -1006,12 +1018,14 @@ mod tests {
let parent = world let parent = world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("parent")) .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent");
})
.id(); .id();
let child = world let child = world
.spawn(Parent(parent)) .spawn(Parent(parent))
.observe( .observe_entity(
|mut trigger: Trigger<EventPropagating>, mut res: ResMut<Order>| { |mut trigger: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child"); res.observed("child");
trigger.propagate(false); trigger.propagate(false);
@ -1034,19 +1048,21 @@ mod tests {
let parent = world let parent = world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("parent")) .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent");
})
.id(); .id();
let child_a = world let child_a = world
.spawn(Parent(parent)) .spawn(Parent(parent))
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| { .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child_a"); res.observed("child_a");
}) })
.id(); .id();
let child_b = world let child_b = world
.spawn(Parent(parent)) .spawn(Parent(parent))
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| { .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child_b"); res.observed("child_b");
}) })
.id(); .id();
@ -1069,7 +1085,9 @@ mod tests {
let entity = world let entity = world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("event")) .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("event");
})
.id(); .id();
// TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
@ -1087,14 +1105,14 @@ mod tests {
let parent_a = world let parent_a = world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| { .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent_a"); res.observed("parent_a");
}) })
.id(); .id();
let child_a = world let child_a = world
.spawn(Parent(parent_a)) .spawn(Parent(parent_a))
.observe( .observe_entity(
|mut trigger: Trigger<EventPropagating>, mut res: ResMut<Order>| { |mut trigger: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child_a"); res.observed("child_a");
trigger.propagate(false); trigger.propagate(false);
@ -1104,14 +1122,16 @@ mod tests {
let parent_b = world let parent_b = world
.spawn_empty() .spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| { .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent_b"); res.observed("parent_b");
}) })
.id(); .id();
let child_b = world let child_b = world
.spawn(Parent(parent_b)) .spawn(Parent(parent_b))
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("child_b")) .observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child_b");
})
.id(); .id();
// TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut

View File

@ -228,12 +228,12 @@ pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, propagate:
/// # let e2 = world.spawn_empty().id(); /// # let e2 = world.spawn_empty().id();
/// # #[derive(Event)] /// # #[derive(Event)]
/// # struct Explode; /// # struct Explode;
/// world.entity_mut(e1).observe(|trigger: Trigger<Explode>, mut commands: Commands| { /// world.entity_mut(e1).observe_entity(|trigger: Trigger<Explode>, mut commands: Commands| {
/// println!("Boom!"); /// println!("Boom!");
/// commands.entity(trigger.entity()).despawn(); /// commands.entity(trigger.entity()).despawn();
/// }); /// });
/// ///
/// world.entity_mut(e2).observe(|trigger: Trigger<Explode>, mut commands: Commands| { /// world.entity_mut(e2).observe_entity(|trigger: Trigger<Explode>, mut commands: Commands| {
/// println!("The explosion fizzles! This entity is immune!"); /// println!("The explosion fizzles! This entity is immune!");
/// }); /// });
/// ``` /// ```
@ -241,7 +241,7 @@ pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, propagate:
/// If all entities watched by a given [`Observer`] are despawned, the [`Observer`] entity will also be despawned. /// If all entities watched by a given [`Observer`] are despawned, the [`Observer`] entity will also be despawned.
/// This protects against observer "garbage" building up over time. /// This protects against observer "garbage" building up over time.
/// ///
/// The examples above calling [`EntityWorldMut::observe`] to add entity-specific observer logic are (once again) /// The examples above calling [`EntityWorldMut::observe_entity`] to add entity-specific observer logic are (once again)
/// just shorthand for spawning an [`Observer`] directly: /// just shorthand for spawning an [`Observer`] directly:
/// ///
/// ``` /// ```

View File

@ -1886,7 +1886,7 @@ fn observe<E: Event, B: Bundle, M>(
) -> impl EntityCommand { ) -> impl EntityCommand {
move |entity, world: &mut World| { move |entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) { if let Some(mut entity) = world.get_entity_mut(entity) {
entity.observe(observer); entity.observe_entity(observer);
} }
} }
} }

View File

@ -1840,7 +1840,7 @@ impl<'w> EntityWorldMut<'w> {
/// Creates an [`Observer`] listening for events of type `E` targeting this entity. /// Creates an [`Observer`] listening for events of type `E` targeting this entity.
/// In order to trigger the callback the entity must also match the query when the event is fired. /// In order to trigger the callback the entity must also match the query when the event is fired.
pub fn observe<E: Event, B: Bundle, M>( pub fn observe_entity<E: Event, B: Bundle, M>(
&mut self, &mut self,
observer: impl IntoObserverSystem<E, B, M>, observer: impl IntoObserverSystem<E, B, M>,
) -> &mut Self { ) -> &mut Self {

View File

@ -11,7 +11,7 @@
//! # use bevy_picking::prelude::*; //! # use bevy_picking::prelude::*;
//! # let mut world = World::default(); //! # let mut world = World::default();
//! world.spawn_empty() //! world.spawn_empty()
//! .observe(|trigger: Trigger<Pointer<Over>>| { //! .observe_entity(|trigger: Trigger<Pointer<Over>>| {
//! println!("I am being hovered over"); //! println!("I am being hovered over");
//! }); //! });
//! ``` //! ```

View File

@ -15,7 +15,7 @@
//! # struct MyComponent; //! # struct MyComponent;
//! # let mut world = World::new(); //! # let mut world = World::new();
//! world.spawn(MyComponent) //! world.spawn(MyComponent)
//! .observe(|mut trigger: Trigger<Pointer<Click>>| { //! .observe_entity(|mut trigger: Trigger<Pointer<Click>>| {
//! // Get the underlying event type //! // Get the underlying event type
//! let click_event: &Pointer<Click> = trigger.event(); //! let click_event: &Pointer<Click> = trigger.event();
//! // Stop the event from bubbling up the entity hierarchjy //! // Stop the event from bubbling up the entity hierarchjy