From 810f5847a4664f5d33977e5003bef8634ad9bc23 Mon Sep 17 00:00:00 2001 From: Joona Aalto Date: Wed, 19 Jun 2024 17:37:57 +0300 Subject: [PATCH 1/6] Replace `HierarchyEvent` with `OnParentChange` trigger --- crates/bevy_hierarchy/src/child_builder.rs | 188 ++++++++++----------- crates/bevy_hierarchy/src/events.rs | 40 ++--- crates/bevy_hierarchy/src/lib.rs | 2 +- 3 files changed, 106 insertions(+), 124 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 6c32979666e63..a95fc85bdb3af 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -1,19 +1,16 @@ -use crate::{Children, HierarchyEvent, Parent}; +use crate::{Children, OnParentChange, Parent}; use bevy_ecs::{ bundle::Bundle, entity::Entity, - prelude::Events, system::{Commands, EntityCommands}, world::{Command, EntityWorldMut, World}, }; use smallvec::{smallvec, SmallVec}; -// Do not use `world.send_event_batch` as it prints error message when the Events are not available in the world, -// even though it's a valid use case to execute commands on a world without events. Loading a GLTF file for example -fn push_events(world: &mut World, events: impl IntoIterator) { - if let Some(mut moved) = world.get_resource_mut::>() { - moved.extend(events); - } +fn trigger_events(world: &mut World, events: impl IntoIterator) { + events + .into_iter() + .for_each(|(event, target)| world.trigger_targets(event, target)); } /// Adds `child` to `parent`'s [`Children`], without checking if it is already present there. @@ -72,18 +69,18 @@ fn update_old_parent(world: &mut World, child: Entity, parent: Entity) { if previous_parent == parent { return; } + remove_from_children(world, previous_parent, child); - push_events( - world, - [HierarchyEvent::ChildMoved { - child, - previous_parent, - new_parent: parent, - }], + world.trigger_targets( + OnParentChange::Moved { + previous: previous_parent, + new: parent, + }, + child, ); } else { - push_events(world, [HierarchyEvent::ChildAdded { child, parent }]); + world.trigger_targets(OnParentChange::Added(parent), child); } } @@ -96,7 +93,8 @@ fn update_old_parent(world: &mut World, child: Entity, parent: Entity) { /// /// Sends [`HierarchyEvent`]'s. fn update_old_parents(world: &mut World, parent: Entity, children: &[Entity]) { - let mut events: SmallVec<[HierarchyEvent; 8]> = SmallVec::with_capacity(children.len()); + let mut events: SmallVec<[(OnParentChange, Entity); 8]> = + SmallVec::with_capacity(children.len()); for &child in children { if let Some(previous) = update_parent(world, child, parent) { // Do nothing if the entity already has the correct parent. @@ -105,37 +103,39 @@ fn update_old_parents(world: &mut World, parent: Entity, children: &[Entity]) { } remove_from_children(world, previous, child); - events.push(HierarchyEvent::ChildMoved { + events.push(( + OnParentChange::Moved { + previous, + new: parent, + }, child, - previous_parent: previous, - new_parent: parent, - }); + )); } else { - events.push(HierarchyEvent::ChildAdded { child, parent }); + events.push((OnParentChange::Added(parent), child)); } } - push_events(world, events); + trigger_events(world, events); } /// Removes entities in `children` from `parent`'s [`Children`], removing the component if it ends up empty. /// Also removes [`Parent`] component from `children`. fn remove_children(parent: Entity, children: &[Entity], world: &mut World) { - let mut events: SmallVec<[HierarchyEvent; 8]> = SmallVec::new(); + let mut events: SmallVec<[(OnParentChange, Entity); 8]> = SmallVec::new(); if let Some(parent_children) = world.get::(parent) { for &child in children { if parent_children.contains(&child) { - events.push(HierarchyEvent::ChildRemoved { child, parent }); + events.push((OnParentChange::Removed(parent), child)); } } } else { return; } - for event in &events { - if let &HierarchyEvent::ChildRemoved { child, .. } = event { - world.entity_mut(child).remove::(); + for (event, child) in &events { + if let &OnParentChange::Removed(_) = event { + world.entity_mut(*child).remove::(); } } - push_events(world, events); + trigger_events(world, events); let mut parent = world.entity_mut(parent); if let Some(mut parent_children) = parent.get_mut::() { @@ -484,13 +484,8 @@ impl<'w> WorldChildBuilder<'w> { pub fn spawn(&mut self, bundle: impl Bundle) -> EntityWorldMut<'_> { let entity = self.world.spawn((bundle, Parent(self.parent))).id(); push_child_unchecked(self.world, self.parent, entity); - push_events( - self.world, - [HierarchyEvent::ChildAdded { - child: entity, - parent: self.parent, - }], - ); + self.world + .trigger_targets(OnParentChange::Added(self.parent), entity); self.world.entity_mut(entity) } @@ -499,13 +494,8 @@ impl<'w> WorldChildBuilder<'w> { pub fn spawn_empty(&mut self) -> EntityWorldMut<'_> { let entity = self.world.spawn(Parent(self.parent)).id(); push_child_unchecked(self.world, self.parent, entity); - push_events( - self.world, - [HierarchyEvent::ChildAdded { - child: entity, - parent: self.parent, - }], - ); + self.world + .trigger_targets(OnParentChange::Added(self.parent), entity); self.world.entity_mut(entity) } @@ -670,7 +660,7 @@ impl<'w> BuildWorldChildren for EntityWorldMut<'w> { if let Some(parent) = self.take::().map(|p| p.get()) { self.world_scope(|world| { remove_from_children(world, parent, child); - push_events(world, [HierarchyEvent::ChildRemoved { child, parent }]); + world.trigger_targets(OnParentChange::Removed(parent), child); }); } self @@ -694,15 +684,15 @@ mod tests { use super::{BuildChildren, BuildWorldChildren}; use crate::{ components::{Children, Parent}, - HierarchyEvent::{self, ChildAdded, ChildMoved, ChildRemoved}, + OnParentChange, }; use smallvec::{smallvec, SmallVec}; use bevy_ecs::{ component::Component, entity::Entity, - event::Events, - system::Commands, + observer::Trigger, + system::{Commands, ResMut, Resource}, world::{CommandQueue, World}, }; @@ -716,17 +706,21 @@ mod tests { assert_eq!(world.get::(parent).map(|c| &**c), children); } + #[derive(Resource, Default)] + struct TriggeredEvents(Vec<(OnParentChange, Entity)>); + /// Used to omit a number of events that are not relevant to a particular test. fn omit_events(world: &mut World, number: usize) { - let mut events_resource = world.resource_mut::>(); - let mut events: Vec<_> = events_resource.drain().collect(); - events_resource.extend(events.drain(number..)); + let mut events_resource = world.resource_mut::(); + let mut events: Vec<_> = events_resource.0.drain(0..).collect(); + events_resource.0.extend(events.drain(number..)); } - fn assert_events(world: &mut World, expected_events: &[HierarchyEvent]) { + fn assert_triggers(world: &mut World, expected_events: &[(OnParentChange, Entity)]) { let events: Vec<_> = world - .resource_mut::>() - .drain() + .resource_mut::() + .0 + .drain(0..) .collect(); assert_eq!(events, expected_events); } @@ -734,7 +728,16 @@ mod tests { #[test] fn add_child() { let world = &mut World::new(); - world.insert_resource(Events::::default()); + + world.init_resource::(); + + world.observe( + |trigger: Trigger, mut triggered_events: ResMut| { + triggered_events + .0 + .push((trigger.event().clone(), trigger.entity())); + }, + ); let [a, b, c, d] = std::array::from_fn(|_| world.spawn_empty().id()); @@ -742,25 +745,14 @@ mod tests { assert_parent(world, b, Some(a)); assert_children(world, a, Some(&[b])); - assert_events( - world, - &[ChildAdded { - child: b, - parent: a, - }], - ); + assert_triggers(world, &[(OnParentChange::Added(a), b)]); world.entity_mut(a).add_child(c); assert_children(world, a, Some(&[b, c])); assert_parent(world, c, Some(a)); - assert_events( - world, - &[ChildAdded { - child: c, - parent: a, - }], - ); + assert_triggers(world, &[(OnParentChange::Added(a), c)]); + // Children component should be removed when it's empty. world.entity_mut(d).add_child(b).add_child(c); assert_children(world, a, None); @@ -769,7 +761,16 @@ mod tests { #[test] fn set_parent() { let world = &mut World::new(); - world.insert_resource(Events::::default()); + + world.init_resource::(); + + world.observe( + |trigger: Trigger, mut triggered_events: ResMut| { + triggered_events + .0 + .push((trigger.event().clone(), trigger.entity())); + }, + ); let [a, b, c] = std::array::from_fn(|_| world.spawn_empty().id()); @@ -777,26 +778,22 @@ mod tests { assert_parent(world, a, Some(b)); assert_children(world, b, Some(&[a])); - assert_events( - world, - &[ChildAdded { - child: a, - parent: b, - }], - ); + assert_triggers(world, &[(OnParentChange::Added(b), a)]); world.entity_mut(a).set_parent(c); assert_parent(world, a, Some(c)); assert_children(world, b, None); assert_children(world, c, Some(&[a])); - assert_events( + assert_triggers( world, - &[ChildMoved { - child: a, - previous_parent: b, - new_parent: c, - }], + &[( + OnParentChange::Moved { + previous: b, + new: c, + }, + a, + )], ); } @@ -820,7 +817,16 @@ mod tests { #[test] fn remove_parent() { let world = &mut World::new(); - world.insert_resource(Events::::default()); + + world.init_resource::(); + + world.observe( + |trigger: Trigger, mut triggered_events: ResMut| { + triggered_events + .0 + .push((trigger.event().clone(), trigger.entity())); + }, + ); let [a, b, c] = std::array::from_fn(|_| world.spawn_empty().id()); @@ -831,24 +837,12 @@ mod tests { assert_parent(world, c, Some(a)); assert_children(world, a, Some(&[c])); omit_events(world, 2); // Omit ChildAdded events. - assert_events( - world, - &[ChildRemoved { - child: b, - parent: a, - }], - ); + assert_triggers(world, &[(OnParentChange::Removed(a), b)]); world.entity_mut(c).remove_parent(); assert_parent(world, c, None); assert_children(world, a, None); - assert_events( - world, - &[ChildRemoved { - child: c, - parent: a, - }], - ); + assert_triggers(world, &[(OnParentChange::Removed(a), c)]); } #[allow(dead_code)] diff --git a/crates/bevy_hierarchy/src/events.rs b/crates/bevy_hierarchy/src/events.rs index 8c8263cecfe74..e26ae9241f8f6 100644 --- a/crates/bevy_hierarchy/src/events.rs +++ b/crates/bevy_hierarchy/src/events.rs @@ -1,31 +1,19 @@ use bevy_ecs::{event::Event, prelude::Entity}; -/// An [`Event`] that is fired whenever there is a change in the world's hierarchy. +/// A [`Trigger`] emitted whenever there is a change in the world's hierarchy. /// -/// [`Event`]: bevy_ecs::event::Event -#[derive(Event, Debug, Clone, PartialEq, Eq)] -pub enum HierarchyEvent { - /// Fired whenever an [`Entity`] is added as a child to a parent. - ChildAdded { - /// The child that was added - child: Entity, - /// The parent the child was added to - parent: Entity, - }, - /// Fired whenever a child [`Entity`] is removed from its parent. - ChildRemoved { - /// The child that was removed - child: Entity, - /// The parent the child was removed from - parent: Entity, - }, - /// Fired whenever a child [`Entity`] is moved to a new parent. - ChildMoved { - /// The child that was moved - child: Entity, - /// The parent the child was removed from - previous_parent: Entity, - /// The parent the child was added to - new_parent: Entity, +/// [`Trigger`]: bevy_ecs::observer::Trigger +#[derive(Event, Clone, Debug, PartialEq)] +pub enum OnParentChange { + /// Emitted whenever the entity is added as a child to a parent. + Added(Entity), + /// Emitted whenever the child entity is removed from its parent. + Removed(Entity), + /// Emitted whenever the child entity is moved to a new parent. + Moved { + /// The parent the child was removed from. + previous: Entity, + /// The parent the child was added to. + new: Entity, }, } diff --git a/crates/bevy_hierarchy/src/lib.rs b/crates/bevy_hierarchy/src/lib.rs index 0be21f6d16bbf..2fe01292ff52c 100644 --- a/crates/bevy_hierarchy/src/lib.rs +++ b/crates/bevy_hierarchy/src/lib.rs @@ -97,6 +97,6 @@ impl Plugin for HierarchyPlugin { fn build(&self, app: &mut App) { app.register_type::() .register_type::() - .add_event::(); + .add_event::(); } } From 3f6cbab535f69759d13600b1811750c99521f89f Mon Sep 17 00:00:00 2001 From: Joona Aalto Date: Wed, 19 Jun 2024 18:05:52 +0300 Subject: [PATCH 2/6] Remove unnecessary event initialization --- crates/bevy_hierarchy/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/bevy_hierarchy/src/lib.rs b/crates/bevy_hierarchy/src/lib.rs index 2fe01292ff52c..47f5ee5381ef7 100644 --- a/crates/bevy_hierarchy/src/lib.rs +++ b/crates/bevy_hierarchy/src/lib.rs @@ -95,8 +95,6 @@ pub struct HierarchyPlugin; #[cfg(feature = "bevy_app")] impl Plugin for HierarchyPlugin { fn build(&self, app: &mut App) { - app.register_type::() - .register_type::() - .add_event::(); + app.register_type::().register_type::(); } } From 90152d76d112076adb7899b1434a9ba75813352c Mon Sep 17 00:00:00 2001 From: Joona Aalto Date: Wed, 19 Jun 2024 18:07:51 +0300 Subject: [PATCH 3/6] Revert function rename --- crates/bevy_hierarchy/src/child_builder.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index a95fc85bdb3af..b4c0b1372267d 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -716,7 +716,7 @@ mod tests { events_resource.0.extend(events.drain(number..)); } - fn assert_triggers(world: &mut World, expected_events: &[(OnParentChange, Entity)]) { + fn assert_events(world: &mut World, expected_events: &[(OnParentChange, Entity)]) { let events: Vec<_> = world .resource_mut::() .0 @@ -745,13 +745,13 @@ mod tests { assert_parent(world, b, Some(a)); assert_children(world, a, Some(&[b])); - assert_triggers(world, &[(OnParentChange::Added(a), b)]); + assert_events(world, &[(OnParentChange::Added(a), b)]); world.entity_mut(a).add_child(c); assert_children(world, a, Some(&[b, c])); assert_parent(world, c, Some(a)); - assert_triggers(world, &[(OnParentChange::Added(a), c)]); + assert_events(world, &[(OnParentChange::Added(a), c)]); // Children component should be removed when it's empty. world.entity_mut(d).add_child(b).add_child(c); @@ -778,14 +778,14 @@ mod tests { assert_parent(world, a, Some(b)); assert_children(world, b, Some(&[a])); - assert_triggers(world, &[(OnParentChange::Added(b), a)]); + assert_events(world, &[(OnParentChange::Added(b), a)]); world.entity_mut(a).set_parent(c); assert_parent(world, a, Some(c)); assert_children(world, b, None); assert_children(world, c, Some(&[a])); - assert_triggers( + assert_events( world, &[( OnParentChange::Moved { @@ -837,12 +837,12 @@ mod tests { assert_parent(world, c, Some(a)); assert_children(world, a, Some(&[c])); omit_events(world, 2); // Omit ChildAdded events. - assert_triggers(world, &[(OnParentChange::Removed(a), b)]); + assert_events(world, &[(OnParentChange::Removed(a), b)]); world.entity_mut(c).remove_parent(); assert_parent(world, c, None); assert_children(world, a, None); - assert_triggers(world, &[(OnParentChange::Removed(a), c)]); + assert_events(world, &[(OnParentChange::Removed(a), c)]); } #[allow(dead_code)] From a9b4f3c541fc9807fdef511e75c184145781688c Mon Sep 17 00:00:00 2001 From: Joona Aalto Date: Thu, 20 Jun 2024 18:21:36 +0300 Subject: [PATCH 4/6] Remove `trigger_events` --- crates/bevy_hierarchy/src/child_builder.rs | 33 ++++++---------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index b4c0b1372267d..61153ba5f2453 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -7,12 +7,6 @@ use bevy_ecs::{ }; use smallvec::{smallvec, SmallVec}; -fn trigger_events(world: &mut World, events: impl IntoIterator) { - events - .into_iter() - .for_each(|(event, target)| world.trigger_targets(event, target)); -} - /// Adds `child` to `parent`'s [`Children`], without checking if it is already present there. /// /// This might cause unexpected results when removing duplicate children. @@ -93,8 +87,6 @@ fn update_old_parent(world: &mut World, child: Entity, parent: Entity) { /// /// Sends [`HierarchyEvent`]'s. fn update_old_parents(world: &mut World, parent: Entity, children: &[Entity]) { - let mut events: SmallVec<[(OnParentChange, Entity); 8]> = - SmallVec::with_capacity(children.len()); for &child in children { if let Some(previous) = update_parent(world, child, parent) { // Do nothing if the entity already has the correct parent. @@ -103,39 +95,32 @@ fn update_old_parents(world: &mut World, parent: Entity, children: &[Entity]) { } remove_from_children(world, previous, child); - events.push(( + world.trigger_targets( OnParentChange::Moved { previous, new: parent, }, child, - )); + ); } else { - events.push((OnParentChange::Added(parent), child)); + world.trigger_targets(OnParentChange::Added(parent), child); } } - trigger_events(world, events); } /// Removes entities in `children` from `parent`'s [`Children`], removing the component if it ends up empty. /// Also removes [`Parent`] component from `children`. fn remove_children(parent: Entity, children: &[Entity], world: &mut World) { - let mut events: SmallVec<[(OnParentChange, Entity); 8]> = SmallVec::new(); - if let Some(parent_children) = world.get::(parent) { - for &child in children { - if parent_children.contains(&child) { - events.push((OnParentChange::Removed(parent), child)); - } - } - } else { + let Some(parent_children) = world.get::(parent).map(|c| c.0.clone()) else { return; - } - for (event, child) in &events { - if let &OnParentChange::Removed(_) = event { + }; + + for child in children { + if parent_children.contains(child) { + world.trigger_targets(OnParentChange::Removed(parent), *child); world.entity_mut(*child).remove::(); } } - trigger_events(world, events); let mut parent = world.entity_mut(parent); if let Some(mut parent_children) = parent.get_mut::() { From 6a8aff0eb40f30129cc01666b4c789e25050be98 Mon Sep 17 00:00:00 2001 From: Joona Aalto Date: Thu, 5 Dec 2024 18:51:50 +0200 Subject: [PATCH 5/6] Improve docs for `OnParentChange` --- crates/bevy_hierarchy/src/events.rs | 41 ++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/bevy_hierarchy/src/events.rs b/crates/bevy_hierarchy/src/events.rs index 7221d9d2f02c2..83769a66d28b1 100644 --- a/crates/bevy_hierarchy/src/events.rs +++ b/crates/bevy_hierarchy/src/events.rs @@ -2,9 +2,48 @@ use bevy_ecs::{event::Event, prelude::Entity}; #[cfg(feature = "reflect")] use bevy_reflect::Reflect; -/// A [`Trigger`] emitted whenever there is a change in the world's hierarchy. +/// A [`Trigger`] emitted for entities whenever they are added as a child to an entity, +/// removed from their parent, or moved from one parent to another. /// /// [`Trigger`]: bevy_ecs::observer::Trigger +/// +/// # Example +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// # use bevy_hierarchy::{BuildChildren, OnParentChange}; +/// # +/// let mut world = World::new(); +/// +/// // Add an observer to listen for hierarchy changes. +/// world.add_observer(move |trigger: Trigger| { +/// let entity = trigger.entity(); +/// match trigger.event() { +/// OnParentChange::Added(parent) => { +/// println!("Entity {entity} was added as a child to {parent}"); +/// } +/// OnParentChange::Removed(parent) => { +/// println!("Entity {entity} was removed from its parent {parent}"); +/// } +/// OnParentChange::Moved { previous, new } => { +/// println!("Entity {entity} was moved from parent {previous} to {new}"); +/// } +/// } +/// }); +/// +/// let parent1 = world.spawn_empty().id(); +/// let parent2 = world.spawn_empty().id(); +/// let child = world.spawn_empty().id(); +/// +/// // Triggers `OnParentChange::Added`. +/// world.entity_mut(parent1).add_child(child); +/// +/// // Triggers `OnParentChange::Moved`. +/// world.entity_mut(parent2).add_child(child); +/// +/// // Triggers `OnParentChange::Removed`. +/// world.entity_mut(child).remove_parent(); +/// ``` #[derive(Event, Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "reflect", derive(Reflect), reflect(Debug, PartialEq))] pub enum OnParentChange { From fb99eef62a2a61de773c7b0ebbf1b728df7109ce Mon Sep 17 00:00:00 2001 From: Joona Aalto Date: Thu, 5 Dec 2024 19:09:03 +0200 Subject: [PATCH 6/6] Fix links to `HierarchyEvent` --- crates/bevy_hierarchy/src/child_builder.rs | 4 ++-- crates/bevy_hierarchy/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 942a72fab888e..c3f28e8ffae38 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -55,7 +55,7 @@ fn remove_from_children(world: &mut World, parent: Entity, child: Entity) { /// /// Does nothing if `child` was already a child of `parent`. /// -/// Sends [`HierarchyEvent`]'s. +/// Triggers [`OnParentChange`]. fn update_old_parent(world: &mut World, child: Entity, parent: Entity) { let previous = update_parent(world, child, parent); if let Some(previous_parent) = previous { @@ -85,7 +85,7 @@ fn update_old_parent(world: &mut World, child: Entity, parent: Entity) { /// /// Does nothing for a child if it was already a child of `parent`. /// -/// Sends [`HierarchyEvent`]'s. +/// Triggers [`OnParentChange`]. fn update_old_parents(world: &mut World, parent: Entity, children: &[Entity]) { for &child in children { if let Some(previous) = update_parent(world, child, parent) { diff --git a/crates/bevy_hierarchy/src/lib.rs b/crates/bevy_hierarchy/src/lib.rs index 20e14fb516f90..2df0d9e01d51a 100644 --- a/crates/bevy_hierarchy/src/lib.rs +++ b/crates/bevy_hierarchy/src/lib.rs @@ -46,7 +46,7 @@ //! //! [command and world]: BuildChildren //! [diagnostic plugin]: ValidParentCheckPlugin -//! [events]: HierarchyEvent +//! [events]: OnParentChange //! [hierarchical despawn extension methods]: DespawnRecursiveExt //! [plugin]: HierarchyPlugin //! [query extension methods]: HierarchyQueryExt