Handle TriggerTargets that are combinations for components/entities (#18024)

# Objective

* Fixes https://github.com/bevyengine/bevy/issues/14074
* Applies CI fixes for #16326

It is currently not possible to issues a trigger that targets a specific
list of components AND a specific list of entities

## Solution

We can now use `((A, B), (entity_1, entity_2))` as a trigger target, as
well as the reverse

## Testing

Added a unit test.

The triggering rules for observers are quite confusing:

Triggers once per entity target

For each entity target, an observer system triggers if any of its
components matches the trigger target components (but it triggers at
most once, since we use an internal counter to make sure that an
observer can run at most once per entity target)

(copied from #14563)
(copied from #16326)

## Notes

All credit to @BenjaminBrienen and @cBournhonesque! Just applying a
small fix to this PR so it can be merged.

---------

Co-authored-by: Benjamin Brienen <Benjamin.Brienen@outlook.com>
Co-authored-by: Christian Hughes <xdotdash@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
This commit is contained in:
Zachary Harrold 2025-02-25 10:57:34 +11:00 committed by GitHub
parent ed1143b26b
commit 20813aed64
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 195 additions and 60 deletions

View File

@ -1,6 +1,10 @@
use core::hint::black_box;
use bevy_ecs::{entity::Entity, event::Event, observer::Trigger, world::World};
use bevy_ecs::{
event::Event,
observer::{Trigger, TriggerTargets},
world::World,
};
use criterion::Criterion;
use rand::{prelude::SliceRandom, SeedableRng};
@ -46,6 +50,6 @@ fn empty_listener_base(trigger: Trigger<EventBase>) {
black_box(trigger);
}
fn send_base_event(world: &mut World, entities: &Vec<Entity>) {
fn send_base_event(world: &mut World, entities: impl TriggerTargets) {
world.trigger_targets(EventBase, entities);
}

View File

@ -5,6 +5,7 @@ mod runner;
pub use entity_observer::ObservedBy;
pub use runner::*;
use variadics_please::all_tuples;
use crate::{
archetype::ArchetypeFlags,
@ -177,92 +178,108 @@ impl<'w, E, B: Bundle> DerefMut for Trigger<'w, E, B> {
/// will run.
pub trait TriggerTargets {
/// The components the trigger should target.
fn components(&self) -> &[ComponentId];
fn components(&self) -> impl Iterator<Item = ComponentId> + Clone + '_;
/// The entities the trigger should target.
fn entities(&self) -> &[Entity];
fn entities(&self) -> impl Iterator<Item = Entity> + Clone + '_;
}
impl TriggerTargets for () {
fn components(&self) -> &[ComponentId] {
&[]
impl<T: TriggerTargets + ?Sized> TriggerTargets for &T {
fn components(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
(**self).components()
}
fn entities(&self) -> &[Entity] {
&[]
fn entities(&self) -> impl Iterator<Item = Entity> + Clone + '_ {
(**self).entities()
}
}
impl TriggerTargets for Entity {
fn components(&self) -> &[ComponentId] {
&[]
fn components(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
[].into_iter()
}
fn entities(&self) -> &[Entity] {
core::slice::from_ref(self)
}
}
impl TriggerTargets for Vec<Entity> {
fn components(&self) -> &[ComponentId] {
&[]
}
fn entities(&self) -> &[Entity] {
self.as_slice()
}
}
impl<const N: usize> TriggerTargets for [Entity; N] {
fn components(&self) -> &[ComponentId] {
&[]
}
fn entities(&self) -> &[Entity] {
self.as_slice()
fn entities(&self) -> impl Iterator<Item = Entity> + Clone + '_ {
core::iter::once(*self)
}
}
impl TriggerTargets for ComponentId {
fn components(&self) -> &[ComponentId] {
core::slice::from_ref(self)
fn components(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
core::iter::once(*self)
}
fn entities(&self) -> &[Entity] {
&[]
fn entities(&self) -> impl Iterator<Item = Entity> + Clone + '_ {
[].into_iter()
}
}
impl TriggerTargets for Vec<ComponentId> {
fn components(&self) -> &[ComponentId] {
self.as_slice()
impl<T: TriggerTargets> TriggerTargets for Vec<T> {
fn components(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
self.iter().flat_map(T::components)
}
fn entities(&self) -> &[Entity] {
&[]
fn entities(&self) -> impl Iterator<Item = Entity> + Clone + '_ {
self.iter().flat_map(T::entities)
}
}
impl<const N: usize> TriggerTargets for [ComponentId; N] {
fn components(&self) -> &[ComponentId] {
self.as_slice()
impl<const N: usize, T: TriggerTargets> TriggerTargets for [T; N] {
fn components(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
self.iter().flat_map(T::components)
}
fn entities(&self) -> &[Entity] {
&[]
fn entities(&self) -> impl Iterator<Item = Entity> + Clone + '_ {
self.iter().flat_map(T::entities)
}
}
impl TriggerTargets for &Vec<Entity> {
fn components(&self) -> &[ComponentId] {
&[]
impl<T: TriggerTargets> TriggerTargets for [T] {
fn components(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
self.iter().flat_map(T::components)
}
fn entities(&self) -> &[Entity] {
self.as_slice()
fn entities(&self) -> impl Iterator<Item = Entity> + Clone + '_ {
self.iter().flat_map(T::entities)
}
}
macro_rules! impl_trigger_targets_tuples {
($(#[$meta:meta])* $($trigger_targets: ident),*) => {
#[expect(clippy::allow_attributes, reason = "can't guarantee violation of non_snake_case")]
#[allow(non_snake_case, reason = "`all_tuples!()` generates non-snake-case variable names.")]
$(#[$meta])*
impl<$($trigger_targets: TriggerTargets),*> TriggerTargets for ($($trigger_targets,)*)
{
fn components(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
let iter = [].into_iter();
let ($($trigger_targets,)*) = self;
$(
let iter = iter.chain($trigger_targets.components());
)*
iter
}
fn entities(&self) -> impl Iterator<Item = Entity> + Clone + '_ {
let iter = [].into_iter();
let ($($trigger_targets,)*) = self;
$(
let iter = iter.chain($trigger_targets.entities());
)*
iter
}
}
}
}
all_tuples!(
#[doc(fake_variadic)]
impl_trigger_targets_tuples,
0,
15,
T
);
/// A description of what an [`Observer`] observes.
#[derive(Default, Clone)]
pub struct ObserverDescriptor {
@ -673,7 +690,8 @@ impl World {
caller: MaybeLocation,
) {
let mut world = DeferredWorld::from(self);
if targets.entities().is_empty() {
let mut entity_targets = targets.entities().peekable();
if entity_targets.peek().is_none() {
// SAFETY: `event_data` is accessible as the type represented by `event_id`
unsafe {
world.trigger_observers_with_data::<_, E::Traversal>(
@ -686,12 +704,12 @@ impl World {
);
};
} else {
for target in targets.entities() {
for target_entity in entity_targets {
// SAFETY: `event_data` is accessible as the type represented by `event_id`
unsafe {
world.trigger_observers_with_data::<_, E::Traversal>(
event_id,
*target,
target_entity,
targets.components(),
event_data,
E::AUTO_PROPAGATE,
@ -1209,6 +1227,119 @@ mod tests {
assert_eq!(vec!["a_2", "a_1"], world.resource::<Order>().0);
}
#[test]
fn observer_multiple_targets() {
#[derive(Resource, Default)]
struct R(i32);
let mut world = World::new();
let component_a = world.register_component::<A>();
let component_b = world.register_component::<B>();
world.init_resource::<R>();
// targets (entity_1, A)
let entity_1 = world
.spawn_empty()
.observe(|_: Trigger<EventA, A>, mut res: ResMut<R>| res.0 += 1)
.id();
// targets (entity_2, B)
let entity_2 = world
.spawn_empty()
.observe(|_: Trigger<EventA, B>, mut res: ResMut<R>| res.0 += 10)
.id();
// targets any entity or component
world.add_observer(|_: Trigger<EventA>, mut res: ResMut<R>| res.0 += 100);
// targets any entity, and components A or B
world.add_observer(|_: Trigger<EventA, (A, B)>, mut res: ResMut<R>| res.0 += 1000);
// test all tuples
world.add_observer(|_: Trigger<EventA, (A, B, (A, B))>, mut res: ResMut<R>| res.0 += 10000);
world.add_observer(
|_: Trigger<EventA, (A, B, (A, B), ((A, B), (A, B)))>, mut res: ResMut<R>| {
res.0 += 100000;
},
);
world.add_observer(
|_: Trigger<EventA, (A, B, (A, B), (B, A), (A, B, ((A, B), (B, A))))>,
mut res: ResMut<R>| res.0 += 1000000,
);
// WorldEntityMut does not automatically flush.
world.flush();
// trigger for an entity and a component
world.trigger_targets(EventA, (entity_1, component_a));
world.flush();
// only observer that doesn't trigger is the one only watching entity_2
assert_eq!(1111101, world.resource::<R>().0);
world.resource_mut::<R>().0 = 0;
// trigger for both entities, but no components: trigger once per entity target
world.trigger_targets(EventA, (entity_1, entity_2));
world.flush();
// only the observer that doesn't require components triggers - once per entity
assert_eq!(200, world.resource::<R>().0);
world.resource_mut::<R>().0 = 0;
// trigger for both components, but no entities: trigger once
world.trigger_targets(EventA, (component_a, component_b));
world.flush();
// all component observers trigger, entities are not observed
assert_eq!(1111100, world.resource::<R>().0);
world.resource_mut::<R>().0 = 0;
// trigger for both entities and both components: trigger once per entity target
// we only get 2222211 because a given observer can trigger only once per entity target
world.trigger_targets(EventA, ((component_a, component_b), (entity_1, entity_2)));
world.flush();
assert_eq!(2222211, world.resource::<R>().0);
world.resource_mut::<R>().0 = 0;
// trigger to test complex tuples: (A, B, (A, B))
world.trigger_targets(
EventA,
(component_a, component_b, (component_a, component_b)),
);
world.flush();
// the duplicate components in the tuple don't cause multiple triggers
assert_eq!(1111100, world.resource::<R>().0);
world.resource_mut::<R>().0 = 0;
// trigger to test complex tuples: (A, B, (A, B), ((A, B), (A, B)))
world.trigger_targets(
EventA,
(
component_a,
component_b,
(component_a, component_b),
((component_a, component_b), (component_a, component_b)),
),
);
world.flush();
// the duplicate components in the tuple don't cause multiple triggers
assert_eq!(1111100, world.resource::<R>().0);
world.resource_mut::<R>().0 = 0;
// trigger to test the most complex tuple: (A, B, (A, B), (B, A), (A, B, ((A, B), (B, A))))
world.trigger_targets(
EventA,
(
component_a,
component_b,
(component_a, component_b),
(component_b, component_a),
(
component_a,
component_b,
((component_a, component_b), (component_b, component_a)),
),
),
);
world.flush();
// the duplicate components in the tuple don't cause multiple triggers
assert_eq!(1111100, world.resource::<R>().0);
world.resource_mut::<R>().0 = 0;
}
#[test]
fn observer_dynamic_component() {
let mut world = World::new();

View File

@ -722,10 +722,10 @@ mod tests {
assert_eq!(sets.len(), 0);
assert!(sets.is_empty());
init_component::<TestComponent1>(&mut sets, 1);
register_component::<TestComponent1>(&mut sets, 1);
assert_eq!(sets.len(), 1);
init_component::<TestComponent2>(&mut sets, 2);
register_component::<TestComponent2>(&mut sets, 2);
assert_eq!(sets.len(), 2);
// check its shape by iter
@ -739,7 +739,7 @@ mod tests {
vec![(ComponentId::new(1), 0), (ComponentId::new(2), 0),]
);
fn init_component<T: Component>(sets: &mut SparseSets, id: usize) {
fn register_component<T: Component>(sets: &mut SparseSets, id: usize) {
let descriptor = ComponentDescriptor::new::<T>();
let id = ComponentId::new(id);
let info = ComponentInfo::new(id, descriptor);

View File

@ -674,7 +674,7 @@ impl<'w> DeferredWorld<'w> {
&mut self,
event: ComponentId,
mut target: Entity,
components: &[ComponentId],
components: impl Iterator<Item = ComponentId> + Clone,
data: &mut E,
mut propagate: bool,
caller: MaybeLocation,
@ -686,7 +686,7 @@ impl<'w> DeferredWorld<'w> {
self.reborrow(),
event,
target,
components.iter().copied(),
components.clone(),
data,
&mut propagate,
caller,