From 41e79ae826e1af2c510da442211386da05dcb920 Mon Sep 17 00:00:00 2001 From: Zachary Harrold Date: Thu, 23 Jan 2025 13:45:24 +1100 Subject: [PATCH] Refactored `ComponentHook` Parameters into `HookContext` (#17503) # Objective - Make the function signature for `ComponentHook` less verbose ## Solution - Refactored `Entity`, `ComponentId`, and `Option<&Location>` into a new `HookContext` struct. ## Testing - CI --- ## Migration Guide Update the function signatures for your component hooks to only take 2 arguments, `world` and `context`. Note that because `HookContext` is plain data with all members public, you can use de-structuring to simplify migration. ```rust // Before fn my_hook( mut world: DeferredWorld, entity: Entity, component_id: ComponentId, ) { ... } // After fn my_hook( mut world: DeferredWorld, HookContext { entity, component_id, caller }: HookContext, ) { ... } ``` Likewise, if you were discarding certain parameters, you can use `..` in the de-structuring: ```rust // Before fn my_hook( mut world: DeferredWorld, entity: Entity, _: ComponentId, ) { ... } // After fn my_hook( mut world: DeferredWorld, HookContext { entity, .. }: HookContext, ) { ... } ``` --- crates/bevy_core_pipeline/src/oit/mod.rs | 6 +- crates/bevy_ecs/src/bundle.rs | 77 ++++++----------- crates/bevy_ecs/src/component.rs | 31 ++++--- crates/bevy_ecs/src/hierarchy.rs | 8 +- crates/bevy_ecs/src/lib.rs | 8 +- .../bevy_ecs/src/observer/entity_observer.rs | 6 +- crates/bevy_ecs/src/observer/runner.rs | 17 ++-- crates/bevy_ecs/src/relationship/mod.rs | 31 ++----- crates/bevy_ecs/src/world/deferred_world.rs | 72 +++++++++------- crates/bevy_ecs/src/world/entity_ref.rs | 78 +++++------------ crates/bevy_input_focus/src/autofocus.rs | 11 +-- crates/bevy_input_focus/src/lib.rs | 10 +-- crates/bevy_render/src/camera/camera.rs | 10 +-- crates/bevy_render/src/sync_component.rs | 10 +-- crates/bevy_render/src/view/visibility/mod.rs | 7 +- crates/bevy_scene/src/dynamic_scene.rs | 2 +- crates/bevy_scene/src/lib.rs | 14 +-- examples/ecs/component_hooks.rs | 85 +++++++++++-------- examples/ecs/immutable_components.rs | 17 +--- 19 files changed, 210 insertions(+), 290 deletions(-) diff --git a/crates/bevy_core_pipeline/src/oit/mod.rs b/crates/bevy_core_pipeline/src/oit/mod.rs index 0942331bb8..ea2f7dbd48 100644 --- a/crates/bevy_core_pipeline/src/oit/mod.rs +++ b/crates/bevy_core_pipeline/src/oit/mod.rs @@ -70,11 +70,11 @@ impl Component for OrderIndependentTransparencySettings { type Mutability = Mutable; fn register_component_hooks(hooks: &mut ComponentHooks) { - hooks.on_add(|world, entity, _, caller| { - if let Some(value) = world.get::(entity) { + hooks.on_add(|world, context| { + if let Some(value) = world.get::(context.entity) { if value.layer_count > 32 { warn!("{}OrderIndependentTransparencySettings layer_count set to {} might be too high.", - caller.map(|location|format!("{location}: ")).unwrap_or_default(), + context.caller.map(|location|format!("{location}: ")).unwrap_or_default(), value.layer_count ); } diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 2d83cfc668..8b3a5bd909 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1704,9 +1704,8 @@ fn sorted_remove(source: &mut Vec, remove: &[T]) { #[cfg(test)] mod tests { use crate as bevy_ecs; - use crate::{component::ComponentId, prelude::*, world::DeferredWorld}; + use crate::{component::HookContext, prelude::*, world::DeferredWorld}; use alloc::vec; - use core::panic::Location; #[derive(Component)] struct A; @@ -1715,39 +1714,19 @@ mod tests { #[component(on_add = a_on_add, on_insert = a_on_insert, on_replace = a_on_replace, on_remove = a_on_remove)] struct AMacroHooks; - fn a_on_add( - mut world: DeferredWorld, - _: Entity, - _: ComponentId, - _: Option<&'static Location<'static>>, - ) { + fn a_on_add(mut world: DeferredWorld, _: HookContext) { world.resource_mut::().assert_order(0); } - fn a_on_insert( - mut world: DeferredWorld, - _: T1, - _: T2, - _: Option<&'static Location<'static>>, - ) { + fn a_on_insert(mut world: DeferredWorld, _: HookContext) { world.resource_mut::().assert_order(1); } - fn a_on_replace( - mut world: DeferredWorld, - _: T1, - _: T2, - _: Option<&'static Location<'static>>, - ) { + fn a_on_replace(mut world: DeferredWorld, _: HookContext) { world.resource_mut::().assert_order(2); } - fn a_on_remove( - mut world: DeferredWorld, - _: T1, - _: T2, - _: Option<&'static Location<'static>>, - ) { + fn a_on_remove(mut world: DeferredWorld, _: HookContext) { world.resource_mut::().assert_order(3); } @@ -1780,10 +1759,10 @@ mod tests { world.init_resource::(); world .register_component_hooks::() - .on_add(|mut world, _, _, _| world.resource_mut::().assert_order(0)) - .on_insert(|mut world, _, _, _| world.resource_mut::().assert_order(1)) - .on_replace(|mut world, _, _, _| world.resource_mut::().assert_order(2)) - .on_remove(|mut world, _, _, _| world.resource_mut::().assert_order(3)); + .on_add(|mut world, _| world.resource_mut::().assert_order(0)) + .on_insert(|mut world, _| world.resource_mut::().assert_order(1)) + .on_replace(|mut world, _| world.resource_mut::().assert_order(2)) + .on_remove(|mut world, _| world.resource_mut::().assert_order(3)); let entity = world.spawn(A).id(); world.despawn(entity); @@ -1807,10 +1786,10 @@ mod tests { world.init_resource::(); world .register_component_hooks::() - .on_add(|mut world, _, _, _| world.resource_mut::().assert_order(0)) - .on_insert(|mut world, _, _, _| world.resource_mut::().assert_order(1)) - .on_replace(|mut world, _, _, _| world.resource_mut::().assert_order(2)) - .on_remove(|mut world, _, _, _| world.resource_mut::().assert_order(3)); + .on_add(|mut world, _| world.resource_mut::().assert_order(0)) + .on_insert(|mut world, _| world.resource_mut::().assert_order(1)) + .on_replace(|mut world, _| world.resource_mut::().assert_order(2)) + .on_remove(|mut world, _| world.resource_mut::().assert_order(3)); let mut entity = world.spawn_empty(); entity.insert(A); @@ -1824,8 +1803,8 @@ mod tests { let mut world = World::new(); world .register_component_hooks::() - .on_replace(|mut world, _, _, _| world.resource_mut::().assert_order(0)) - .on_insert(|mut world, _, _, _| { + .on_replace(|mut world, _| world.resource_mut::().assert_order(0)) + .on_insert(|mut world, _| { if let Some(mut r) = world.get_resource_mut::() { r.assert_order(1); } @@ -1846,22 +1825,22 @@ mod tests { world.init_resource::(); world .register_component_hooks::() - .on_add(|mut world, entity, _, _| { + .on_add(|mut world, context| { world.resource_mut::().assert_order(0); - world.commands().entity(entity).insert(B); + world.commands().entity(context.entity).insert(B); }) - .on_remove(|mut world, entity, _, _| { + .on_remove(|mut world, context| { world.resource_mut::().assert_order(2); - world.commands().entity(entity).remove::(); + world.commands().entity(context.entity).remove::(); }); world .register_component_hooks::() - .on_add(|mut world, entity, _, _| { + .on_add(|mut world, context| { world.resource_mut::().assert_order(1); - world.commands().entity(entity).remove::(); + world.commands().entity(context.entity).remove::(); }) - .on_remove(|mut world, _, _, _| { + .on_remove(|mut world, _| { world.resource_mut::().assert_order(3); }); @@ -1878,27 +1857,27 @@ mod tests { world.init_resource::(); world .register_component_hooks::() - .on_add(|mut world, entity, _, _| { + .on_add(|mut world, context| { world.resource_mut::().assert_order(0); - world.commands().entity(entity).insert(B).insert(C); + world.commands().entity(context.entity).insert(B).insert(C); }); world .register_component_hooks::() - .on_add(|mut world, entity, _, _| { + .on_add(|mut world, context| { world.resource_mut::().assert_order(1); - world.commands().entity(entity).insert(D); + world.commands().entity(context.entity).insert(D); }); world .register_component_hooks::() - .on_add(|mut world, _, _, _| { + .on_add(|mut world, _| { world.resource_mut::().assert_order(3); }); world .register_component_hooks::() - .on_add(|mut world, _, _, _| { + .on_add(|mut world, _| { world.resource_mut::().assert_order(2); }); diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index a6817bf6ef..ffa270b5f1 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -299,7 +299,7 @@ pub use bevy_ecs_macros::require; /// - `#[component(on_remove = on_remove_function)]` /// /// ``` -/// # use bevy_ecs::component::Component; +/// # use bevy_ecs::component::{Component, HookContext}; /// # use bevy_ecs::world::DeferredWorld; /// # use bevy_ecs::entity::Entity; /// # use bevy_ecs::component::ComponentId; @@ -315,12 +315,12 @@ pub use bevy_ecs_macros::require; /// // #[component(on_replace = my_on_replace_hook, on_remove = my_on_remove_hook)] /// struct ComponentA; /// -/// fn my_on_add_hook(world: DeferredWorld, entity: Entity, id: ComponentId, caller: Option<&Location>) { +/// fn my_on_add_hook(world: DeferredWorld, context: HookContext) { /// // ... /// } /// -/// // You can also omit writing some types using generics. -/// fn my_on_insert_hook(world: DeferredWorld, _: T1, _: T2, caller: Option<&Location>) { +/// // You can also destructure items directly in the signature +/// fn my_on_insert_hook(world: DeferredWorld, HookContext { caller, .. }: HookContext) { /// // ... /// } /// ``` @@ -498,9 +498,18 @@ pub enum StorageType { } /// The type used for [`Component`] lifecycle hooks such as `on_add`, `on_insert` or `on_remove`. -/// The caller location is `Some` if the `track_caller` feature is enabled. -pub type ComponentHook = - for<'w> fn(DeferredWorld<'w>, Entity, ComponentId, Option<&'static Location<'static>>); +pub type ComponentHook = for<'w> fn(DeferredWorld<'w>, HookContext); + +/// Context provided to a [`ComponentHook`]. +#[derive(Clone, Copy, Debug)] +pub struct HookContext { + /// The [`Entity`] this hook was invoked for. + pub entity: Entity, + /// The [`ComponentId`] this hook was invoked for. + pub component_id: ComponentId, + /// The caller location is `Some` if the `track_caller` feature is enabled. + pub caller: Option<&'static Location<'static>>, +} /// [`World`]-mutating functions that run as part of lifecycle events of a [`Component`]. /// @@ -537,14 +546,14 @@ pub type ComponentHook = /// let mut tracked_component_query = world.query::<&MyTrackedComponent>(); /// assert!(tracked_component_query.iter(&world).next().is_none()); /// -/// world.register_component_hooks::().on_add(|mut world, entity, _component_id, _caller| { +/// world.register_component_hooks::().on_add(|mut world, context| { /// let mut tracked_entities = world.resource_mut::(); -/// tracked_entities.0.insert(entity); +/// tracked_entities.0.insert(context.entity); /// }); /// -/// world.register_component_hooks::().on_remove(|mut world, entity, _component_id, _caller| { +/// world.register_component_hooks::().on_remove(|mut world, context| { /// let mut tracked_entities = world.resource_mut::(); -/// tracked_entities.0.remove(&entity); +/// tracked_entities.0.remove(&context.entity); /// }); /// /// let entity = world.spawn(MyTrackedComponent).id(); diff --git a/crates/bevy_ecs/src/hierarchy.rs b/crates/bevy_ecs/src/hierarchy.rs index bd77d3f721..d4eb2475b9 100644 --- a/crates/bevy_ecs/src/hierarchy.rs +++ b/crates/bevy_ecs/src/hierarchy.rs @@ -14,7 +14,7 @@ use crate::reflect::{ use crate::{ self as bevy_ecs, bundle::Bundle, - component::{Component, ComponentId}, + component::{Component, HookContext}, entity::{Entity, VisitEntities}, relationship::{RelatedSpawner, RelatedSpawnerCommands}, system::EntityCommands, @@ -22,8 +22,8 @@ use crate::{ }; use alloc::{format, string::String, vec::Vec}; use bevy_ecs_macros::VisitEntitiesMut; +use core::ops::Deref; use core::slice; -use core::{ops::Deref, panic::Location}; use disqualified::ShortName; use log::warn; @@ -268,9 +268,7 @@ impl<'a> EntityCommands<'a> { /// contains component `C`. This will print a warning if the parent does not contain `C`. pub fn validate_parent_has_component( world: DeferredWorld, - entity: Entity, - _: ComponentId, - caller: Option<&'static Location<'static>>, + HookContext { entity, caller, .. }: HookContext, ) { let entity_ref = world.entity(entity); let Some(child_of) = entity_ref.get::() else { diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index c59c5515b1..b79ac90f46 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2030,8 +2030,8 @@ mod tests { world.insert_resource(I(0)); world .register_component_hooks::() - .on_add(|mut world, _, _, _| world.resource_mut::().0 += 1) - .on_insert(|mut world, _, _, _| world.resource_mut::().0 += 1); + .on_add(|mut world, _| world.resource_mut::().0 += 1) + .on_insert(|mut world, _| world.resource_mut::().0 += 1); // Spawn entity and ensure Y was added assert!(world.spawn(X).contains::()); @@ -2060,8 +2060,8 @@ mod tests { world.insert_resource(I(0)); world .register_component_hooks::() - .on_add(|mut world, _, _, _| world.resource_mut::().0 += 1) - .on_insert(|mut world, _, _, _| world.resource_mut::().0 += 1); + .on_add(|mut world, _| world.resource_mut::().0 += 1) + .on_insert(|mut world, _| world.resource_mut::().0 += 1); // Spawn entity and ensure Y was added assert!(world.spawn_empty().insert(X).contains::()); diff --git a/crates/bevy_ecs/src/observer/entity_observer.rs b/crates/bevy_ecs/src/observer/entity_observer.rs index 8edc6d2874..1ad30890e4 100644 --- a/crates/bevy_ecs/src/observer/entity_observer.rs +++ b/crates/bevy_ecs/src/observer/entity_observer.rs @@ -1,5 +1,7 @@ use crate::{ - component::{Component, ComponentCloneHandler, ComponentHooks, Mutable, StorageType}, + component::{ + Component, ComponentCloneHandler, ComponentHooks, HookContext, Mutable, StorageType, + }, entity::{ComponentCloneCtx, Entity, EntityCloneBuilder}, observer::ObserverState, world::{DeferredWorld, World}, @@ -15,7 +17,7 @@ impl Component for ObservedBy { type Mutability = Mutable; fn register_component_hooks(hooks: &mut ComponentHooks) { - hooks.on_remove(|mut world, entity, _, _| { + hooks.on_remove(|mut world, HookContext { entity, .. }| { let observed_by = { let mut component = world.get_mut::(entity).unwrap(); core::mem::take(&mut component.0) diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 96b5838c19..6990d5023b 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -1,9 +1,8 @@ use alloc::{boxed::Box, vec, vec::Vec}; use core::any::Any; -use core::panic::Location; use crate::{ - component::{ComponentHook, ComponentHooks, ComponentId, Mutable, StorageType}, + component::{ComponentHook, ComponentHooks, ComponentId, HookContext, Mutable, StorageType}, observer::{ObserverDescriptor, ObserverTrigger}, prelude::*, query::DebugCheckedUnwrap, @@ -67,12 +66,12 @@ impl Component for ObserverState { type Mutability = Mutable; fn register_component_hooks(hooks: &mut ComponentHooks) { - hooks.on_add(|mut world, entity, _, _| { + hooks.on_add(|mut world, HookContext { entity, .. }| { world.commands().queue(move |world: &mut World| { world.register_observer(entity); }); }); - hooks.on_remove(|mut world, entity, _, _| { + hooks.on_remove(|mut world, HookContext { entity, .. }| { let descriptor = core::mem::take( &mut world .entity_mut(entity) @@ -319,12 +318,12 @@ impl Component for Observer { const STORAGE_TYPE: StorageType = StorageType::SparseSet; type Mutability = Mutable; fn register_component_hooks(hooks: &mut ComponentHooks) { - hooks.on_add(|world, entity, id, caller| { - let Some(observe) = world.get::(entity) else { + hooks.on_add(|world, context| { + let Some(observe) = world.get::(context.entity) else { return; }; let hook = observe.hook_on_add; - hook(world, entity, id, caller); + hook(world, context); }); } } @@ -395,9 +394,7 @@ fn observer_system_runner>( /// ensure type parameters match. fn hook_on_add>( mut world: DeferredWorld<'_>, - entity: Entity, - _: ComponentId, - _: Option<&'static Location<'static>>, + HookContext { entity, .. }: HookContext, ) { world.commands().queue(move |world: &mut World| { let event_id = E::register_component_id(world); diff --git a/crates/bevy_ecs/src/relationship/mod.rs b/crates/bevy_ecs/src/relationship/mod.rs index 10c8bc302c..b04b4f2b58 100644 --- a/crates/bevy_ecs/src/relationship/mod.rs +++ b/crates/bevy_ecs/src/relationship/mod.rs @@ -5,14 +5,13 @@ mod relationship_query; mod relationship_source_collection; use alloc::format; -use core::panic::Location; pub use related_methods::*; pub use relationship_query::*; pub use relationship_source_collection::*; use crate::{ - component::{Component, ComponentId, Mutable}, + component::{Component, HookContext, Mutable}, entity::Entity, system::{ command::HandleError, @@ -74,12 +73,7 @@ pub trait Relationship: Component + Sized { fn from(entity: Entity) -> Self; /// The `on_insert` component hook that maintains the [`Relationship`] / [`RelationshipTarget`] connection. - fn on_insert( - mut world: DeferredWorld, - entity: Entity, - _: ComponentId, - caller: Option<&'static Location<'static>>, - ) { + fn on_insert(mut world: DeferredWorld, HookContext { entity, caller, .. }: HookContext) { let target_entity = world.entity(entity).get::().unwrap().get(); if target_entity == entity { warn!( @@ -113,12 +107,7 @@ pub trait Relationship: Component + Sized { /// The `on_replace` component hook that maintains the [`Relationship`] / [`RelationshipTarget`] connection. // note: think of this as "on_drop" - fn on_replace( - mut world: DeferredWorld, - entity: Entity, - _: ComponentId, - _: Option<&'static Location<'static>>, - ) { + fn on_replace(mut world: DeferredWorld, HookContext { entity, .. }: HookContext) { let target_entity = world.entity(entity).get::().unwrap().get(); if let Ok(mut target_entity_mut) = world.get_entity_mut(target_entity) { if let Some(mut relationship_target) = @@ -179,12 +168,7 @@ pub trait RelationshipTarget: Component + Sized { /// The `on_replace` component hook that maintains the [`Relationship`] / [`RelationshipTarget`] connection. // note: think of this as "on_drop" - fn on_replace( - mut world: DeferredWorld, - entity: Entity, - _: ComponentId, - caller: Option<&'static Location<'static>>, - ) { + fn on_replace(mut world: DeferredWorld, HookContext { entity, caller, .. }: HookContext) { // NOTE: this unsafe code is an optimization. We could make this safe, but it would require // copying the RelationshipTarget collection // SAFETY: This only reads the Self component and queues Remove commands @@ -215,12 +199,7 @@ pub trait RelationshipTarget: Component + Sized { /// The `on_despawn` component hook that despawns entities stored in an entity's [`RelationshipTarget`] when /// that entity is despawned. // note: think of this as "on_drop" - fn on_despawn( - mut world: DeferredWorld, - entity: Entity, - _: ComponentId, - caller: Option<&'static Location<'static>>, - ) { + fn on_despawn(mut world: DeferredWorld, HookContext { entity, caller, .. }: HookContext) { // NOTE: this unsafe code is an optimization. We could make this safe, but it would require // copying the RelationshipTarget collection // SAFETY: This only reads the Self component and queues despawn commands diff --git a/crates/bevy_ecs/src/world/deferred_world.rs b/crates/bevy_ecs/src/world/deferred_world.rs index 6cdead4ce5..35b38fd198 100644 --- a/crates/bevy_ecs/src/world/deferred_world.rs +++ b/crates/bevy_ecs/src/world/deferred_world.rs @@ -5,7 +5,7 @@ use core::panic::Location; use crate::{ archetype::Archetype, change_detection::MutUntyped, - component::{ComponentId, Mutable}, + component::{ComponentId, HookContext, Mutable}, entity::Entity, event::{Event, EventId, Events, SendBatchIds}, observer::{Observers, TriggerTargets}, @@ -538,12 +538,14 @@ impl<'w> DeferredWorld<'w> { if let Some(hook) = hooks.on_add { hook( DeferredWorld { world: self.world }, - entity, - component_id, - #[cfg(feature = "track_location")] - Some(caller), - #[cfg(not(feature = "track_location"))] - None, + HookContext { + entity, + component_id, + #[cfg(feature = "track_location")] + caller: Some(caller), + #[cfg(not(feature = "track_location"))] + caller: None, + }, ); } } @@ -569,12 +571,14 @@ impl<'w> DeferredWorld<'w> { if let Some(hook) = hooks.on_insert { hook( DeferredWorld { world: self.world }, - entity, - component_id, - #[cfg(feature = "track_location")] - Some(caller), - #[cfg(not(feature = "track_location"))] - None, + HookContext { + entity, + component_id, + #[cfg(feature = "track_location")] + caller: Some(caller), + #[cfg(not(feature = "track_location"))] + caller: None, + }, ); } } @@ -600,12 +604,14 @@ impl<'w> DeferredWorld<'w> { if let Some(hook) = hooks.on_replace { hook( DeferredWorld { world: self.world }, - entity, - component_id, - #[cfg(feature = "track_location")] - Some(caller), - #[cfg(not(feature = "track_location"))] - None, + HookContext { + entity, + component_id, + #[cfg(feature = "track_location")] + caller: Some(caller), + #[cfg(not(feature = "track_location"))] + caller: None, + }, ); } } @@ -631,12 +637,14 @@ impl<'w> DeferredWorld<'w> { if let Some(hook) = hooks.on_remove { hook( DeferredWorld { world: self.world }, - entity, - component_id, - #[cfg(feature = "track_location")] - Some(caller), - #[cfg(not(feature = "track_location"))] - None, + HookContext { + entity, + component_id, + #[cfg(feature = "track_location")] + caller: Some(caller), + #[cfg(not(feature = "track_location"))] + caller: None, + }, ); } } @@ -662,12 +670,14 @@ impl<'w> DeferredWorld<'w> { if let Some(hook) = hooks.on_despawn { hook( DeferredWorld { world: self.world }, - entity, - component_id, - #[cfg(feature = "track_location")] - Some(caller), - #[cfg(not(feature = "track_location"))] - None, + HookContext { + entity, + component_id, + #[cfg(feature = "track_location")] + caller: Some(caller), + #[cfg(not(feature = "track_location"))] + caller: None, + }, ); } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 00ed616c5b..bc47197228 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -4401,10 +4401,10 @@ mod tests { use bevy_ptr::{OwningPtr, Ptr}; use core::panic::AssertUnwindSafe; - use core::panic::Location; #[cfg(feature = "track_location")] - use std::sync::OnceLock; + use {core::panic::Location, std::sync::OnceLock}; + use crate::component::HookContext; use crate::{ self as bevy_ecs, change_detection::MutUntyped, @@ -5482,22 +5482,12 @@ mod tests { #[component(on_add = ord_a_hook_on_add, on_insert = ord_a_hook_on_insert, on_replace = ord_a_hook_on_replace, on_remove = ord_a_hook_on_remove)] struct OrdA; - fn ord_a_hook_on_add( - mut world: DeferredWorld, - entity: Entity, - _id: ComponentId, - _caller: Option<&'static Location<'static>>, - ) { + fn ord_a_hook_on_add(mut world: DeferredWorld, HookContext { entity, .. }: HookContext) { world.resource_mut::().0.push("OrdA hook on_add"); world.commands().entity(entity).insert(OrdB); } - fn ord_a_hook_on_insert( - mut world: DeferredWorld, - entity: Entity, - _id: ComponentId, - _caller: Option<&'static Location<'static>>, - ) { + fn ord_a_hook_on_insert(mut world: DeferredWorld, HookContext { entity, .. }: HookContext) { world .resource_mut::() .0 @@ -5506,24 +5496,14 @@ mod tests { world.commands().entity(entity).remove::(); } - fn ord_a_hook_on_replace( - mut world: DeferredWorld, - _entity: Entity, - _id: ComponentId, - _caller: Option<&'static Location<'static>>, - ) { + fn ord_a_hook_on_replace(mut world: DeferredWorld, _: HookContext) { world .resource_mut::() .0 .push("OrdA hook on_replace"); } - fn ord_a_hook_on_remove( - mut world: DeferredWorld, - _entity: Entity, - _id: ComponentId, - _caller: Option<&'static Location<'static>>, - ) { + fn ord_a_hook_on_remove(mut world: DeferredWorld, _: HookContext) { world .resource_mut::() .0 @@ -5550,12 +5530,7 @@ mod tests { #[component(on_add = ord_b_hook_on_add, on_insert = ord_b_hook_on_insert, on_replace = ord_b_hook_on_replace, on_remove = ord_b_hook_on_remove)] struct OrdB; - fn ord_b_hook_on_add( - mut world: DeferredWorld, - _entity: Entity, - _id: ComponentId, - _caller: Option<&'static Location<'static>>, - ) { + fn ord_b_hook_on_add(mut world: DeferredWorld, _: HookContext) { world.resource_mut::().0.push("OrdB hook on_add"); world.commands().queue(|world: &mut World| { world @@ -5565,36 +5540,21 @@ mod tests { }); } - fn ord_b_hook_on_insert( - mut world: DeferredWorld, - _entity: Entity, - _id: ComponentId, - _caller: Option<&'static Location<'static>>, - ) { + fn ord_b_hook_on_insert(mut world: DeferredWorld, _: HookContext) { world .resource_mut::() .0 .push("OrdB hook on_insert"); } - fn ord_b_hook_on_replace( - mut world: DeferredWorld, - _entity: Entity, - _id: ComponentId, - _caller: Option<&'static Location<'static>>, - ) { + fn ord_b_hook_on_replace(mut world: DeferredWorld, _: HookContext) { world .resource_mut::() .0 .push("OrdB hook on_replace"); } - fn ord_b_hook_on_remove( - mut world: DeferredWorld, - _entity: Entity, - _id: ComponentId, - _caller: Option<&'static Location<'static>>, - ) { + fn ord_b_hook_on_remove(mut world: DeferredWorld, _: HookContext) { world .resource_mut::() .0 @@ -5734,7 +5694,7 @@ mod tests { struct C; static TRACKED: OnceLock<&'static Location<'static>> = OnceLock::new(); - fn get_tracked(world: DeferredWorld, entity: Entity, _: ComponentId, _: Option<&Location>) { + fn get_tracked(world: DeferredWorld, HookContext { entity, .. }: HookContext) { TRACKED.get_or_init(|| { world .entities @@ -5795,35 +5755,35 @@ mod tests { world.register_component::(); world .register_component_hooks::() - .on_add(|world, entity, _, _| { + .on_add(|world, context| { ADD_COUNT.fetch_add(1, Ordering::Relaxed); assert_eq!( - world.get(entity), + world.get(context.entity), Some(&Foo(EXPECTED_VALUE.load(Ordering::Relaxed))) ); }) - .on_remove(|world, entity, _, _| { + .on_remove(|world, context| { REMOVE_COUNT.fetch_add(1, Ordering::Relaxed); assert_eq!( - world.get(entity), + world.get(context.entity), Some(&Foo(EXPECTED_VALUE.load(Ordering::Relaxed))) ); }) - .on_replace(|world, entity, _, _| { + .on_replace(|world, context| { REPLACE_COUNT.fetch_add(1, Ordering::Relaxed); assert_eq!( - world.get(entity), + world.get(context.entity), Some(&Foo(EXPECTED_VALUE.load(Ordering::Relaxed))) ); }) - .on_insert(|world, entity, _, _| { + .on_insert(|world, context| { INSERT_COUNT.fetch_add(1, Ordering::Relaxed); assert_eq!( - world.get(entity), + world.get(context.entity), Some(&Foo(EXPECTED_VALUE.load(Ordering::Relaxed))) ); }); diff --git a/crates/bevy_input_focus/src/autofocus.rs b/crates/bevy_input_focus/src/autofocus.rs index e91833adb1..dc3b0919d2 100644 --- a/crates/bevy_input_focus/src/autofocus.rs +++ b/crates/bevy_input_focus/src/autofocus.rs @@ -1,8 +1,6 @@ //! Contains the [`AutoFocus`] component and related machinery. -use core::panic::Location; - -use bevy_ecs::{component::ComponentId, prelude::*, world::DeferredWorld}; +use bevy_ecs::{component::HookContext, prelude::*, world::DeferredWorld}; use crate::InputFocus; @@ -25,12 +23,7 @@ use bevy_reflect::{prelude::*, Reflect}; #[component(on_add = on_auto_focus_added)] pub struct AutoFocus; -fn on_auto_focus_added( - mut world: DeferredWorld, - entity: Entity, - _: ComponentId, - _: Option<&'static Location<'static>>, -) { +fn on_auto_focus_added(mut world: DeferredWorld, HookContext { entity, .. }: HookContext) { if let Some(mut input_focus) = world.get_resource_mut::() { input_focus.set(entity); } diff --git a/crates/bevy_input_focus/src/lib.rs b/crates/bevy_input_focus/src/lib.rs index 28640d20de..0ec0b42897 100644 --- a/crates/bevy_input_focus/src/lib.rs +++ b/crates/bevy_input_focus/src/lib.rs @@ -361,26 +361,20 @@ mod tests { use alloc::string::String; use bevy_ecs::{ - component::ComponentId, observer::Trigger, system::RunSystemOnce, world::DeferredWorld, + component::HookContext, observer::Trigger, system::RunSystemOnce, world::DeferredWorld, }; use bevy_input::{ keyboard::{Key, KeyCode}, ButtonState, InputPlugin, }; use bevy_window::WindowResolution; - use core::panic::Location; use smol_str::SmolStr; #[derive(Component)] #[component(on_add = set_focus_on_add)] struct SetFocusOnAdd; - fn set_focus_on_add( - mut world: DeferredWorld, - entity: Entity, - _: ComponentId, - _: Option<&Location>, - ) { + fn set_focus_on_add(mut world: DeferredWorld, HookContext { entity, .. }: HookContext) { let mut input_focus = world.resource_mut::(); input_focus.set(entity); } diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index 5b48dc4fec..c5db6e3ad6 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -22,7 +22,7 @@ use bevy_asset::{AssetEvent, AssetId, Assets, Handle}; use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{ change_detection::DetectChanges, - component::{Component, ComponentId}, + component::{Component, HookContext}, entity::{Entity, EntityBorrow}, event::EventReader, prelude::{require, With}, @@ -43,7 +43,6 @@ use bevy_window::{ WindowScaleFactorChanged, }; use core::ops::Range; -use core::panic::Location; use derive_more::derive::From; use tracing::warn; use wgpu::{BlendState, TextureFormat, TextureUsages}; @@ -333,12 +332,7 @@ pub struct Camera { pub sub_camera_view: Option, } -fn warn_on_no_render_graph( - world: DeferredWorld, - entity: Entity, - _: ComponentId, - caller: Option<&'static Location<'static>>, -) { +fn warn_on_no_render_graph(world: DeferredWorld, HookContext { entity, caller, .. }: HookContext) { if !world.entity(entity).contains::() { warn!("{}Entity {entity} has a `Camera` component, but it doesn't have a render graph configured. Consider adding a `Camera2d` or `Camera3d` component, or manually adding a `CameraRenderGraph` component if you need a custom render graph.", caller.map(|location|format!("{location}: ")).unwrap_or_default()); } diff --git a/crates/bevy_render/src/sync_component.rs b/crates/bevy_render/src/sync_component.rs index f4f0f9a5e4..dd7eca1a28 100644 --- a/crates/bevy_render/src/sync_component.rs +++ b/crates/bevy_render/src/sync_component.rs @@ -32,11 +32,11 @@ impl Plugin for SyncComponentPlugin { fn build(&self, app: &mut App) { app.register_required_components::(); - app.world_mut().register_component_hooks::().on_remove( - |mut world, entity, _component_id, _caller| { + app.world_mut() + .register_component_hooks::() + .on_remove(|mut world, context| { let mut pending = world.resource_mut::(); - pending.push(EntityRecord::ComponentRemoved(entity)); - }, - ); + pending.push(EntityRecord::ComponentRemoved(context.entity)); + }); } } diff --git a/crates/bevy_render/src/view/visibility/mod.rs b/crates/bevy_render/src/view/visibility/mod.rs index 6660e66280..33c6639266 100644 --- a/crates/bevy_render/src/view/visibility/mod.rs +++ b/crates/bevy_render/src/view/visibility/mod.rs @@ -2,9 +2,8 @@ mod range; mod render_layers; use core::any::TypeId; -use core::panic::Location; -use bevy_ecs::component::ComponentId; +use bevy_ecs::component::HookContext; use bevy_ecs::entity::hash_set::EntityHashSet; use bevy_ecs::world::DeferredWorld; use derive_more::derive::{Deref, DerefMut}; @@ -635,9 +634,7 @@ pub fn check_visibility( /// ``` pub fn add_visibility_class( mut world: DeferredWorld<'_>, - entity: Entity, - _: ComponentId, - _: Option<&Location>, + HookContext { entity, .. }: HookContext, ) where C: 'static, { diff --git a/crates/bevy_scene/src/dynamic_scene.rs b/crates/bevy_scene/src/dynamic_scene.rs index 312826663e..a40053ed5f 100644 --- a/crates/bevy_scene/src/dynamic_scene.rs +++ b/crates/bevy_scene/src/dynamic_scene.rs @@ -365,7 +365,7 @@ mod tests { let mut dst_world = World::new(); dst_world .register_component_hooks::() - .on_add(|mut world, _, _, _| { + .on_add(|mut world, _| { world.commands().spawn_empty(); }); dst_world.insert_resource(reg.clone()); diff --git a/crates/bevy_scene/src/lib.rs b/crates/bevy_scene/src/lib.rs index a68d63b302..507f18f63a 100644 --- a/crates/bevy_scene/src/lib.rs +++ b/crates/bevy_scene/src/lib.rs @@ -68,12 +68,14 @@ impl Plugin for ScenePlugin { // Register component hooks for DynamicSceneRoot app.world_mut() .register_component_hooks::() - .on_remove(|mut world, entity, _, _| { - let Some(handle) = world.get::(entity) else { + .on_remove(|mut world, context| { + let Some(handle) = world.get::(context.entity) else { return; }; let id = handle.id(); - if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { + if let Some(&SceneInstance(scene_instance)) = + world.get::(context.entity) + { let Some(mut scene_spawner) = world.get_resource_mut::() else { return; }; @@ -87,8 +89,10 @@ impl Plugin for ScenePlugin { // Register component hooks for SceneRoot app.world_mut() .register_component_hooks::() - .on_remove(|mut world, entity, _, _| { - if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { + .on_remove(|mut world, context| { + if let Some(&SceneInstance(scene_instance)) = + world.get::(context.entity) + { let Some(mut scene_spawner) = world.get_resource_mut::() else { return; }; diff --git a/examples/ecs/component_hooks.rs b/examples/ecs/component_hooks.rs index 26136965d7..aeb2a1f284 100644 --- a/examples/ecs/component_hooks.rs +++ b/examples/ecs/component_hooks.rs @@ -14,7 +14,7 @@ //! between components (like hierarchies or parent-child links) and need to maintain correctness. use bevy::{ - ecs::component::{ComponentHooks, Mutable, StorageType}, + ecs::component::{ComponentHooks, HookContext, Mutable, StorageType}, prelude::*, }; use std::collections::HashMap; @@ -63,54 +63,69 @@ fn setup(world: &mut World) { world .register_component_hooks::() // There are 4 component lifecycle hooks: `on_add`, `on_insert`, `on_replace` and `on_remove` - // A hook has 4 arguments: + // A hook has 2 arguments: // - a `DeferredWorld`, this allows access to resource and component data as well as `Commands` - // - the entity that triggered the hook - // - the component id of the triggering component, this is mostly used for dynamic components - // - the location of the code that caused the hook to trigger + // - a `HookContext`, this provides access to the following contextual information: + // - the entity that triggered the hook + // - the component id of the triggering component, this is mostly used for dynamic components + // - the location of the code that caused the hook to trigger // // `on_add` will trigger when a component is inserted onto an entity without it - .on_add(|mut world, entity, component_id, caller| { - // You can access component data from within the hook - let value = world.get::(entity).unwrap().0; - println!( - "{component_id:?} added to {entity} with value {value:?}{}", - caller - .map(|location| format!("due to {location}")) - .unwrap_or_default() - ); - // Or access resources - world - .resource_mut::() - .insert(value, entity); - // Or send events - world.send_event(MyEvent); - }) + .on_add( + |mut world, + HookContext { + entity, + component_id, + caller, + }| { + // You can access component data from within the hook + let value = world.get::(entity).unwrap().0; + println!( + "{component_id:?} added to {entity} with value {value:?}{}", + caller + .map(|location| format!("due to {location}")) + .unwrap_or_default() + ); + // Or access resources + world + .resource_mut::() + .insert(value, entity); + // Or send events + world.send_event(MyEvent); + }, + ) // `on_insert` will trigger when a component is inserted onto an entity, // regardless of whether or not it already had it and after `on_add` if it ran - .on_insert(|world, _, _, _| { + .on_insert(|world, _| { println!("Current Index: {:?}", world.resource::()); }) // `on_replace` will trigger when a component is inserted onto an entity that already had it, // and runs before the value is replaced. // Also triggers when a component is removed from an entity, and runs before `on_remove` - .on_replace(|mut world, entity, _, _| { - let value = world.get::(entity).unwrap().0; + .on_replace(|mut world, context| { + let value = world.get::(context.entity).unwrap().0; world.resource_mut::().remove(&value); }) // `on_remove` will trigger when a component is removed from an entity, // since it runs before the component is removed you can still access the component data - .on_remove(|mut world, entity, component_id, caller| { - let value = world.get::(entity).unwrap().0; - println!( - "{component_id:?} removed from {entity} with value {value:?}{}", - caller - .map(|location| format!("due to {location}")) - .unwrap_or_default() - ); - // You can also issue commands through `.commands()` - world.commands().entity(entity).despawn(); - }); + .on_remove( + |mut world, + HookContext { + entity, + component_id, + caller, + }| { + let value = world.get::(entity).unwrap().0; + println!( + "{component_id:?} removed from {entity} with value {value:?}{}", + caller + .map(|location| format!("due to {location}")) + .unwrap_or_default() + ); + // You can also issue commands through `.commands()` + world.commands().entity(entity).despawn(); + }, + ); } fn trigger_hooks( diff --git a/examples/ecs/immutable_components.rs b/examples/ecs/immutable_components.rs index ca5682f362..bde5cc9d95 100644 --- a/examples/ecs/immutable_components.rs +++ b/examples/ecs/immutable_components.rs @@ -2,7 +2,7 @@ use bevy::{ ecs::{ - component::{ComponentDescriptor, ComponentId, StorageType}, + component::{ComponentDescriptor, ComponentId, HookContext, StorageType}, world::DeferredWorld, }, prelude::*, @@ -10,7 +10,6 @@ use bevy::{ utils::HashMap, }; use core::alloc::Layout; -use core::panic::Location; /// This component is mutable, the default case. This is indicated by components /// implementing [`Component`] where [`Component::Mutability`] is [`Mutable`](bevy::ecs::component::Mutable). @@ -74,12 +73,7 @@ impl NameIndex { /// /// Since all mutations to [`Name`] are captured by hooks, we know it is not currently /// inserted in the index, and its value will not change without triggering a hook. -fn on_insert_name( - mut world: DeferredWorld<'_>, - entity: Entity, - _component: ComponentId, - _caller: Option<&'static Location<'static>>, -) { +fn on_insert_name(mut world: DeferredWorld<'_>, HookContext { entity, .. }: HookContext) { let Some(&name) = world.entity(entity).get::() else { unreachable!("OnInsert hook guarantees `Name` is available on entity") }; @@ -94,12 +88,7 @@ fn on_insert_name( /// /// Since all mutations to [`Name`] are captured by hooks, we know it is currently /// inserted in the index. -fn on_replace_name( - mut world: DeferredWorld<'_>, - entity: Entity, - _component: ComponentId, - _caller: Option<&'static Location<'static>>, -) { +fn on_replace_name(mut world: DeferredWorld<'_>, HookContext { entity, .. }: HookContext) { let Some(&name) = world.entity(entity).get::() else { unreachable!("OnReplace hook guarantees `Name` is available on entity") };