Skip to content

Commit

Permalink
Rename observe to observe_entity on EntityWorldMut (bevyengine#15616)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
kristoff3r authored Oct 3, 2024
1 parent 2da8d17 commit 336c23c
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 36 deletions.
12 changes: 4 additions & 8 deletions benches/benches/bevy_ecs/observers/propagation.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -110,15 +106,15 @@ fn add_listeners_to_hierarchy<const DENSITY: usize, const N: usize>(
world: &mut World,
) {
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() {
world.entity_mut(*e).observe(empty_listener::<N>);
world.entity_mut(*e).observe_entity(empty_listener::<N>);
}
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::<N>);
world.entity_mut(*e).observe_entity(empty_listener::<N>);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion benches/benches/bevy_ecs/observers/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(|| {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_dev_tools/src/ci_testing/systems.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub(crate) fn send_events(world: &mut World, mut current_frame: Local<u32>) {
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.
Expand Down
58 changes: 39 additions & 19 deletions crates/bevy_ecs/src/observer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ mod tests {

world
.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>| {
assert_eq!(obs.entity(), Entity::PLACEHOLDER);
res.observed("event_a");
Expand All @@ -860,10 +860,10 @@ mod tests {

world
.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
.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();
world.observe(move |obs: Trigger<EventA>, mut res: ResMut<Order>| {
assert_eq!(obs.entity(), entity);
Expand Down Expand Up @@ -931,12 +931,16 @@ mod tests {

let parent = world
.spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("parent"))
.observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent");
})
.id();

let child = world
.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();

// TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
Expand All @@ -954,12 +958,16 @@ mod tests {

let parent = world
.spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("parent"))
.observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent");
})
.id();

let child = world
.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();

// TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
Expand All @@ -980,12 +988,16 @@ mod tests {

let parent = world
.spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("parent"))
.observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent");
})
.id();

let child = world
.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();

// TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
Expand All @@ -1006,12 +1018,14 @@ mod tests {

let parent = world
.spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("parent"))
.observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent");
})
.id();

let child = world
.spawn(Parent(parent))
.observe(
.observe_entity(
|mut trigger: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child");
trigger.propagate(false);
Expand All @@ -1034,19 +1048,21 @@ mod tests {

let parent = world
.spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("parent"))
.observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent");
})
.id();

let child_a = world
.spawn(Parent(parent))
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
.observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child_a");
})
.id();

let child_b = world
.spawn(Parent(parent))
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
.observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child_b");
})
.id();
Expand All @@ -1069,7 +1085,9 @@ mod tests {

let entity = world
.spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| res.observed("event"))
.observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("event");
})
.id();

// TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
Expand All @@ -1087,14 +1105,14 @@ mod tests {

let parent_a = world
.spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
.observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent_a");
})
.id();

let child_a = world
.spawn(Parent(parent_a))
.observe(
.observe_entity(
|mut trigger: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("child_a");
trigger.propagate(false);
Expand All @@ -1104,14 +1122,16 @@ mod tests {

let parent_b = world
.spawn_empty()
.observe(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
.observe_entity(|_: Trigger<EventPropagating>, mut res: ResMut<Order>| {
res.observed("parent_b");
})
.id();

let child_b = world
.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();

// TODO: ideally this flush is not necessary, but right now observe() returns WorldEntityMut
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/observer/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,20 +228,20 @@ 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<Explode>, mut commands: Commands| {
/// world.entity_mut(e1).observe_entity(|trigger: Trigger<Explode>, mut commands: Commands| {
/// println!("Boom!");
/// 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!");
/// });
/// ```
///
/// 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:
///
/// ```
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1886,7 +1886,7 @@ fn observe<E: Event, B: Bundle, M>(
) -> impl EntityCommand {
move |entity, world: &mut World| {
if let Some(mut entity) = world.get_entity_mut(entity) {
entity.observe(observer);
entity.observe_entity(observer);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/world/entity_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<E: Event, B: Bundle, M>(
pub fn observe_entity<E: Event, B: Bundle, M>(
&mut self,
observer: impl IntoObserverSystem<E, B, M>,
) -> &mut Self {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_picking/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//! # use bevy_picking::prelude::*;
//! # let mut world = World::default();
//! world.spawn_empty()
//! .observe(|trigger: Trigger<Pointer<Over>>| {
//! .observe_entity(|trigger: Trigger<Pointer<Over>>| {
//! println!("I am being hovered over");
//! });
//! ```
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_picking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
//! # struct MyComponent;
//! # let mut world = World::new();
//! world.spawn(MyComponent)
//! .observe(|mut trigger: Trigger<Pointer<Click>>| {
//! .observe_entity(|mut trigger: Trigger<Pointer<Click>>| {
//! // Get the underlying event type
//! let click_event: &Pointer<Click> = trigger.event();
//! // Stop the event from bubbling up the entity hierarchjy
Expand Down

0 comments on commit 336c23c

Please sign in to comment.