Skip to content

feat: allow for Trigger to provide the Entity on which an event w… #18710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

feat: allow for Trigger to provide the Entity on which an event w… #18710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 63 additions & 7 deletions crates/bevy_ecs/src/observer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,26 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> {
/// be [`Entity::PLACEHOLDER`].
///
/// Observable events can target specific entities. When those events fire, they will trigger
/// any observers on the targeted entities. In this case, the `target()` and `observer()` are
/// the same, because the observer that was triggered is attached to the entity that was
/// targeted by the event.
/// any observers on the targeted entities.
///
/// However, it is also possible for those events to bubble up the entity hierarchy and trigger
/// observers on *different* entities, or trigger a global observer. In these cases, the
/// observing entity is *different* from the entity being targeted by the event.
///
/// This is an important distinction: the entity reacting to an event is not always the same as
/// This is an important distinction: the entity targeted by an event is not always the same as
/// the entity triggered by the event.
pub fn target(&self) -> Entity {
self.trigger.target
}

/// Returns the [`Entity`] that is currently being observed. It may be [`Entity::PLACEHOLDER`]
/// if the observer is a global observer.
///
/// To get the entity that was targeted by the event use [`Trigger::target`].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given how confusing this has been, it might help to explicitly mention event propagation here. Maybe something along the lines of

    /// When [propagation](Trigger::propagate) is used,
    /// an observer may be invoked multiple times for a single event.
    /// [`Trigger::target`] will be the same for each invocation,
    /// while [`Trigger::current_target`] will be different.

pub fn current_target(&self) -> Entity {
self.trigger.current_target
}

/// Returns the components that triggered the observer, out of the
/// components defined in `B`. Does not necessarily include all of them as
/// `B` acts like an `OR` filter rather than an `AND` filter.
Expand Down Expand Up @@ -349,6 +355,8 @@ pub struct ObserverTrigger {
components: SmallVec<[ComponentId; 2]>,
/// The entity the trigger targeted.
pub target: Entity,
/// The entity that is currently being observed.
pub current_target: Entity,
/// The location of the source code that triggered the obserer.
pub caller: MaybeLocation,
}
Expand Down Expand Up @@ -424,6 +432,7 @@ impl Observers {
mut world: DeferredWorld,
event_type: ComponentId,
target: Entity,
current_target: Entity,
components: impl Iterator<Item = ComponentId> + Clone,
data: &mut T,
propagate: &mut bool,
Expand Down Expand Up @@ -452,6 +461,7 @@ impl Observers {
event_type,
components: components.clone().collect(),
target,
current_target,
caller,
},
data.into(),
Expand All @@ -463,7 +473,7 @@ impl Observers {

// Trigger entity observers listening for this kind of trigger
if target != Entity::PLACEHOLDER {
if let Some(map) = observers.entity_observers.get(&target) {
if let Some(map) = observers.entity_observers.get(&current_target) {
map.iter().for_each(&mut trigger_observer);
}
}
Expand All @@ -477,7 +487,7 @@ impl Observers {
.for_each(&mut trigger_observer);

if target != Entity::PLACEHOLDER {
if let Some(map) = component_observers.entity_map.get(&target) {
if let Some(map) = component_observers.entity_map.get(&current_target) {
map.iter().for_each(&mut trigger_observer);
}
}
Expand Down Expand Up @@ -885,6 +895,16 @@ mod tests {
}
}

#[derive(Resource, Default)]
struct OrderEntity(Vec<Entity>);

impl OrderEntity {
#[track_caller]
fn observed(&mut self, id: Entity) {
self.0.push(id);
}
}

#[derive(Component)]
struct ChildOf(Entity);

Expand Down Expand Up @@ -1408,6 +1428,42 @@ mod tests {
assert_eq!(vec!["child", "parent"], world.resource::<Order>().0);
}

#[test]
fn observer_propagating_current_target() {
let mut world = World::new();
world.init_resource::<OrderEntity>();

let parent = world
.spawn_empty()
.observe(
|trigger: Trigger<EventPropagating>, mut res: ResMut<OrderEntity>| {
res.observed(trigger.current_target());
res.observed(trigger.target());
},
)
.id();

let child = world
.spawn(ChildOf(parent))
.observe(
|trigger: Trigger<EventPropagating>, mut res: ResMut<OrderEntity>| {
res.observed(trigger.current_target());
res.observed(trigger.target());
},
)
.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!(
vec![child, child, parent, child],
world.resource::<OrderEntity>().0
);
}

#[test]
fn observer_propagating_redundant_dispatch_same_entity() {
let mut world = World::new();
Expand Down Expand Up @@ -1630,7 +1686,7 @@ mod tests {

world.add_observer(
|trigger: Trigger<EventPropagating>, query: Query<&A>, mut res: ResMut<Order>| {
if query.get(trigger.target()).is_ok() {
if query.get(trigger.current_target()).is_ok() {
res.observed("event");
}
},
Expand Down
10 changes: 7 additions & 3 deletions crates/bevy_ecs/src/world/deferred_world.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,7 @@ impl<'w> DeferredWorld<'w> {
self.reborrow(),
event,
target,
target,
components,
&mut (),
&mut false,
Expand All @@ -761,19 +762,22 @@ impl<'w> DeferredWorld<'w> {
pub(crate) unsafe fn trigger_observers_with_data<E, T>(
&mut self,
event: ComponentId,
mut target: Entity,
target: Entity,
components: impl Iterator<Item = ComponentId> + Clone,
data: &mut E,
mut propagate: bool,
caller: MaybeLocation,
) where
T: Traversal<E>,
{
let mut current_target = target;

loop {
Observers::invoke::<_>(
self.reborrow(),
event,
target,
current_target,
components.clone(),
data,
&mut propagate,
Expand All @@ -783,12 +787,12 @@ impl<'w> DeferredWorld<'w> {
break;
}
if let Some(traverse_to) = self
.get_entity(target)
.get_entity(current_target)
.ok()
.and_then(|entity| entity.get_components::<T>())
.and_then(|item| T::traverse(item, data))
{
target = traverse_to;
current_target = traverse_to;
} else {
break;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_input_focus/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ mod tests {
trigger: Trigger<FocusedInput<KeyboardInput>>,
mut query: Query<&mut GatherKeyboardEvents>,
) {
if let Ok(mut gather) = query.get_mut(trigger.target()) {
if let Ok(mut gather) = query.get_mut(trigger.current_target()) {
if let Key::Character(c) = &trigger.input.logical_key {
gather.0.push_str(c.as_str());
}
Expand Down