From 336c23c1aa865c7f329f699e99bd0560ef29cb79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20S=C3=B8holm?= Date: Thu, 3 Oct 2024 19:05:26 +0200 Subject: [PATCH] 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. --- .../benches/bevy_ecs/observers/propagation.rs | 12 ++-- benches/benches/bevy_ecs/observers/simple.rs | 2 +- .../bevy_dev_tools/src/ci_testing/systems.rs | 2 +- crates/bevy_ecs/src/observer/mod.rs | 58 +++++++++++++------ crates/bevy_ecs/src/observer/runner.rs | 6 +- crates/bevy_ecs/src/system/commands/mod.rs | 2 +- crates/bevy_ecs/src/world/entity_ref.rs | 2 +- crates/bevy_picking/src/events.rs | 2 +- crates/bevy_picking/src/lib.rs | 2 +- 9 files changed, 52 insertions(+), 36 deletions(-) diff --git a/benches/benches/bevy_ecs/observers/propagation.rs b/benches/benches/bevy_ecs/observers/propagation.rs index b702662e7d..75fb1f9347 100644 --- a/benches/benches/bevy_ecs/observers/propagation.rs +++ b/benches/benches/bevy_ecs/observers/propagation.rs @@ -1,9 +1,5 @@ use bevy_ecs::{ - component::Component, - entity::Entity, - event::Event, - observer::Trigger, - world::World, + component::Component, entity::Entity, event::Event, observer::Trigger, world::World, }; use bevy_hierarchy::{BuildChildren, Parent}; @@ -110,15 +106,15 @@ fn add_listeners_to_hierarchy( world: &mut World, ) { for e in roots.iter() { - world.entity_mut(*e).observe(empty_listener::); + world.entity_mut(*e).observe_entity(empty_listener::); } for e in leaves.iter() { - world.entity_mut(*e).observe(empty_listener::); + world.entity_mut(*e).observe_entity(empty_listener::); } let mut rng = deterministic_rand(); for e in nodes.iter() { if rng.gen_bool(DENSITY as f64 / 100.0) { - world.entity_mut(*e).observe(empty_listener::); + world.entity_mut(*e).observe_entity(empty_listener::); } } } diff --git a/benches/benches/bevy_ecs/observers/simple.rs b/benches/benches/bevy_ecs/observers/simple.rs index 4d4d5bc2aa..7b7acc5564 100644 --- a/benches/benches/bevy_ecs/observers/simple.rs +++ b/benches/benches/bevy_ecs/observers/simple.rs @@ -29,7 +29,7 @@ pub fn observe_simple(criterion: &mut Criterion) { let mut world = World::new(); let mut entities = vec![]; 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()); bencher.iter(|| { diff --git a/crates/bevy_dev_tools/src/ci_testing/systems.rs b/crates/bevy_dev_tools/src/ci_testing/systems.rs index 20c758a91c..c3d83ec01f 100644 --- a/crates/bevy_dev_tools/src/ci_testing/systems.rs +++ b/crates/bevy_dev_tools/src/ci_testing/systems.rs @@ -25,7 +25,7 @@ pub(crate) fn send_events(world: &mut World, mut current_frame: Local) { let path = format!("./screenshot-{}.png", *current_frame); world .spawn(Screenshot::primary_window()) - .observe(save_to_disk(path)); + .observe_entity(save_to_disk(path)); info!("Took a screenshot at frame {}.", *current_frame); } // Custom events are forwarded to the world. diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index c74d78ae03..5dcc7f70b6 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -839,7 +839,7 @@ mod tests { world .spawn_empty() - .observe(|_: Trigger| panic!("Trigger routed to non-targeted entity.")); + .observe_entity(|_: Trigger| panic!("Trigger routed to non-targeted entity.")); world.observe(move |obs: Trigger, mut res: ResMut| { assert_eq!(obs.entity(), Entity::PLACEHOLDER); res.observed("event_a"); @@ -860,10 +860,10 @@ mod tests { world .spawn_empty() - .observe(|_: Trigger| panic!("Trigger routed to non-targeted entity.")); + .observe_entity(|_: Trigger| panic!("Trigger routed to non-targeted entity.")); let entity = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.observed("a_1")) + .observe_entity(|_: Trigger, mut res: ResMut| res.observed("a_1")) .id(); world.observe(move |obs: Trigger, mut res: ResMut| { assert_eq!(obs.entity(), entity); @@ -931,12 +931,16 @@ mod tests { let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.observed("parent")) + .observe_entity(|_: Trigger, mut res: ResMut| { + res.observed("parent"); + }) .id(); let child = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| res.observed("child")) + .observe_entity(|_: Trigger, mut res: ResMut| { + res.observed("child"); + }) .id(); // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut @@ -954,12 +958,16 @@ mod tests { let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.observed("parent")) + .observe_entity(|_: Trigger, mut res: ResMut| { + res.observed("parent"); + }) .id(); let child = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| res.observed("child")) + .observe_entity(|_: Trigger, mut res: ResMut| { + res.observed("child"); + }) .id(); // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut @@ -980,12 +988,16 @@ mod tests { let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.observed("parent")) + .observe_entity(|_: Trigger, mut res: ResMut| { + res.observed("parent"); + }) .id(); let child = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| res.observed("child")) + .observe_entity(|_: Trigger, mut res: ResMut| { + res.observed("child"); + }) .id(); // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut @@ -1006,12 +1018,14 @@ mod tests { let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.observed("parent")) + .observe_entity(|_: Trigger, mut res: ResMut| { + res.observed("parent"); + }) .id(); let child = world .spawn(Parent(parent)) - .observe( + .observe_entity( |mut trigger: Trigger, mut res: ResMut| { res.observed("child"); trigger.propagate(false); @@ -1034,19 +1048,21 @@ mod tests { let parent = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.observed("parent")) + .observe_entity(|_: Trigger, mut res: ResMut| { + res.observed("parent"); + }) .id(); let child_a = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| { + .observe_entity(|_: Trigger, mut res: ResMut| { res.observed("child_a"); }) .id(); let child_b = world .spawn(Parent(parent)) - .observe(|_: Trigger, mut res: ResMut| { + .observe_entity(|_: Trigger, mut res: ResMut| { res.observed("child_b"); }) .id(); @@ -1069,7 +1085,9 @@ mod tests { let entity = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| res.observed("event")) + .observe_entity(|_: Trigger, mut res: ResMut| { + res.observed("event"); + }) .id(); // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut @@ -1087,14 +1105,14 @@ mod tests { let parent_a = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| { + .observe_entity(|_: Trigger, mut res: ResMut| { res.observed("parent_a"); }) .id(); let child_a = world .spawn(Parent(parent_a)) - .observe( + .observe_entity( |mut trigger: Trigger, mut res: ResMut| { res.observed("child_a"); trigger.propagate(false); @@ -1104,14 +1122,16 @@ mod tests { let parent_b = world .spawn_empty() - .observe(|_: Trigger, mut res: ResMut| { + .observe_entity(|_: Trigger, mut res: ResMut| { res.observed("parent_b"); }) .id(); let child_b = world .spawn(Parent(parent_b)) - .observe(|_: Trigger, mut res: ResMut| res.observed("child_b")) + .observe_entity(|_: Trigger, mut res: ResMut| { + res.observed("child_b"); + }) .id(); // TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index f85725d392..02340b6128 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -228,12 +228,12 @@ pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, propagate: /// # let e2 = world.spawn_empty().id(); /// # #[derive(Event)] /// # struct Explode; -/// world.entity_mut(e1).observe(|trigger: Trigger, mut commands: Commands| { +/// world.entity_mut(e1).observe_entity(|trigger: Trigger, mut commands: Commands| { /// println!("Boom!"); /// commands.entity(trigger.entity()).despawn(); /// }); /// -/// world.entity_mut(e2).observe(|trigger: Trigger, mut commands: Commands| { +/// world.entity_mut(e2).observe_entity(|trigger: Trigger, mut commands: Commands| { /// 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. /// 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: /// /// ``` diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 3d6a6cf536..bec88f51b5 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1886,7 +1886,7 @@ fn observe( ) -> impl EntityCommand { move |entity, world: &mut World| { if let Some(mut entity) = world.get_entity_mut(entity) { - entity.observe(observer); + entity.observe_entity(observer); } } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 023db78f5d..fbc58a9104 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1840,7 +1840,7 @@ impl<'w> EntityWorldMut<'w> { /// Creates an [`Observer`] listening for events of type `E` targeting this entity. /// In order to trigger the callback the entity must also match the query when the event is fired. - pub fn observe( + pub fn observe_entity( &mut self, observer: impl IntoObserverSystem, ) -> &mut Self { diff --git a/crates/bevy_picking/src/events.rs b/crates/bevy_picking/src/events.rs index c61ca42aea..9cbec38592 100644 --- a/crates/bevy_picking/src/events.rs +++ b/crates/bevy_picking/src/events.rs @@ -11,7 +11,7 @@ //! # use bevy_picking::prelude::*; //! # let mut world = World::default(); //! world.spawn_empty() -//! .observe(|trigger: Trigger>| { +//! .observe_entity(|trigger: Trigger>| { //! println!("I am being hovered over"); //! }); //! ``` diff --git a/crates/bevy_picking/src/lib.rs b/crates/bevy_picking/src/lib.rs index 6844149670..fdc710539c 100644 --- a/crates/bevy_picking/src/lib.rs +++ b/crates/bevy_picking/src/lib.rs @@ -15,7 +15,7 @@ //! # struct MyComponent; //! # let mut world = World::new(); //! world.spawn(MyComponent) -//! .observe(|mut trigger: Trigger>| { +//! .observe_entity(|mut trigger: Trigger>| { //! // Get the underlying event type //! let click_event: &Pointer = trigger.event(); //! // Stop the event from bubbling up the entity hierarchjy