Split Component::register_component_hooks into individual methods (#17685)

# Objective

- Fixes #17411

## Solution

- Deprecated `Component::register_component_hooks`
- Added individual methods for each hook which return `None` if the hook
is unused.

## Testing

- CI

---

## Migration Guide

`Component::register_component_hooks` is now deprecated and will be
removed in a future release. When implementing `Component` manually,
also implement the respective hook methods on `Component`.

```rust
// Before
impl Component for Foo {
    // snip
    fn register_component_hooks(hooks: &mut ComponentHooks) {
            hooks.on_add(foo_on_add);
    }
}

// After
impl Component for Foo {
    // snip
    fn on_add() -> Option<ComponentHook> {
            Some(foo_on_add)
    }
}
```

## Notes

I've chosen to deprecate `Component::register_component_hooks` rather
than outright remove it to ease the migration guide. While it is in a
state of deprecation, it must be used by
`Components::register_component_internal` to ensure users who haven't
migrated to the new hook definition scheme aren't left behind. For users
of the new scheme, a default implementation of
`Component::register_component_hooks` is provided which forwards the new
individual hook implementations.

Personally, I think this is a cleaner API to work with, and would allow
the documentation for hooks to exist on the respective `Component`
methods (e.g., documentation for `OnAdd` can exist on
`Component::on_add`). Ideally, `Component::on_add` would be the hook
itself rather than a getter for the hook, but it is the only way to
early-out for a no-op hook, which is important for performance.

## Migration Guide

`Component::register_component_hooks` has been deprecated. If you are
manually implementing the `Component` trait and registering hooks there,
use the individual methods such as `on_add` instead for increased
clarity.
This commit is contained in:
Zachary Harrold 2025-02-06 06:33:05 +11:00 committed by GitHub
parent ff466a37df
commit d0c0bad7b4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 140 additions and 68 deletions

View File

@ -69,8 +69,8 @@ impl Component for OrderIndependentTransparencySettings {
const STORAGE_TYPE: StorageType = StorageType::SparseSet;
type Mutability = Mutable;
fn register_component_hooks(hooks: &mut ComponentHooks) {
hooks.on_add(|world, context| {
fn on_add() -> Option<ComponentHook> {
Some(|world, context| {
if let Some(value) = world.get::<OrderIndependentTransparencySettings>(context.entity) {
if value.layer_count > 32 {
warn!("{}OrderIndependentTransparencySettings layer_count set to {} might be too high.",
@ -79,7 +79,7 @@ impl Component for OrderIndependentTransparencySettings {
);
}
}
});
})
}
}

View File

@ -71,15 +71,11 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
let storage = storage_path(&bevy_ecs_path, attrs.storage);
let on_add = hook_register_function_call(quote! {on_add}, attrs.on_add);
let mut on_insert = hook_register_function_call(quote! {on_insert}, attrs.on_insert);
let mut on_replace = hook_register_function_call(quote! {on_replace}, attrs.on_replace);
let on_remove: Option<TokenStream2> =
hook_register_function_call(quote! {on_remove}, attrs.on_remove);
let mut on_despawn = hook_register_function_call(quote! {on_despawn}, attrs.on_despawn);
let on_add_path = attrs.on_add.map(|path| path.to_token_stream());
let on_remove_path = attrs.on_remove.map(|path| path.to_token_stream());
if relationship.is_some() {
if on_insert.is_some() {
let on_insert_path = if relationship.is_some() {
if attrs.on_insert.is_some() {
return syn::Error::new(
ast.span(),
"Custom on_insert hooks are not supported as relationships already define an on_insert hook",
@ -88,11 +84,13 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
.into();
}
on_insert = Some(
quote!(hooks.on_insert(<Self as #bevy_ecs_path::relationship::Relationship>::on_insert);),
);
Some(quote!(<Self as #bevy_ecs_path::relationship::Relationship>::on_insert))
} else {
attrs.on_insert.map(|path| path.to_token_stream())
};
if on_replace.is_some() {
let on_replace_path = if relationship.is_some() {
if attrs.on_replace.is_some() {
return syn::Error::new(
ast.span(),
"Custom on_replace hooks are not supported as Relationships already define an on_replace hook",
@ -101,13 +99,9 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
.into();
}
on_replace = Some(
quote!(hooks.on_replace(<Self as #bevy_ecs_path::relationship::Relationship>::on_replace);),
);
}
if let Some(relationship_target) = &attrs.relationship_target {
if on_replace.is_some() {
Some(quote!(<Self as #bevy_ecs_path::relationship::Relationship>::on_replace))
} else if attrs.relationship_target.is_some() {
if attrs.on_replace.is_some() {
return syn::Error::new(
ast.span(),
"Custom on_replace hooks are not supported as RelationshipTarget already defines an on_replace hook",
@ -116,12 +110,16 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
.into();
}
on_replace = Some(
quote!(hooks.on_replace(<Self as #bevy_ecs_path::relationship::RelationshipTarget>::on_replace);),
);
Some(quote!(<Self as #bevy_ecs_path::relationship::RelationshipTarget>::on_replace))
} else {
attrs.on_replace.map(|path| path.to_token_stream())
};
if relationship_target.despawn_descendants {
if on_despawn.is_some() {
let on_despawn_path = if attrs
.relationship_target
.is_some_and(|target| target.despawn_descendants)
{
if attrs.on_despawn.is_some() {
return syn::Error::new(
ast.span(),
"Custom on_despawn hooks are not supported as this RelationshipTarget already defines an on_despawn hook, via the despawn_descendants attribute",
@ -130,11 +128,18 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
.into();
}
on_despawn = Some(
quote!(hooks.on_despawn(<Self as #bevy_ecs_path::relationship::RelationshipTarget>::on_despawn);),
);
}
}
Some(quote!(<Self as #bevy_ecs_path::relationship::RelationshipTarget>::on_despawn))
} else {
attrs.on_despawn.map(|path| path.to_token_stream())
};
let on_add = hook_register_function_call(&bevy_ecs_path, quote! {on_add}, on_add_path);
let on_insert = hook_register_function_call(&bevy_ecs_path, quote! {on_insert}, on_insert_path);
let on_replace =
hook_register_function_call(&bevy_ecs_path, quote! {on_replace}, on_replace_path);
let on_remove = hook_register_function_call(&bevy_ecs_path, quote! {on_remove}, on_remove_path);
let on_despawn =
hook_register_function_call(&bevy_ecs_path, quote! {on_despawn}, on_despawn_path);
ast.generics
.make_where_clause()
@ -232,14 +237,11 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
recursion_check_stack.pop();
}
#[allow(unused_variables)]
fn register_component_hooks(hooks: &mut #bevy_ecs_path::component::ComponentHooks) {
#on_add
#on_insert
#on_replace
#on_remove
#on_despawn
}
fn get_component_clone_handler() -> #bevy_ecs_path::component::ComponentCloneHandler {
#clone_handler
@ -444,10 +446,17 @@ fn storage_path(bevy_ecs_path: &Path, ty: StorageTy) -> TokenStream2 {
}
fn hook_register_function_call(
bevy_ecs_path: &Path,
hook: TokenStream2,
function: Option<ExprPath>,
function: Option<TokenStream2>,
) -> Option<TokenStream2> {
function.map(|meta| quote! { hooks. #hook (#meta); })
function.map(|meta| {
quote! {
fn #hook() -> ::core::option::Option<#bevy_ecs_path::component::ComponentHook> {
::core::option::Option::Some(#meta)
}
}
})
}
impl Parse for Relationship {

View File

@ -401,7 +401,38 @@ pub trait Component: Send + Sync + 'static {
type Mutability: ComponentMutability;
/// Called when registering this component, allowing mutable access to its [`ComponentHooks`].
fn register_component_hooks(_hooks: &mut ComponentHooks) {}
#[deprecated(
since = "0.16.0",
note = "Use the individual hook methods instead (e.g., `Component::on_add`, etc.)"
)]
fn register_component_hooks(hooks: &mut ComponentHooks) {
hooks.update_from_component::<Self>();
}
/// Gets the `on_add` [`ComponentHook`] for this [`Component`] if one is defined.
fn on_add() -> Option<ComponentHook> {
None
}
/// Gets the `on_insert` [`ComponentHook`] for this [`Component`] if one is defined.
fn on_insert() -> Option<ComponentHook> {
None
}
/// Gets the `on_replace` [`ComponentHook`] for this [`Component`] if one is defined.
fn on_replace() -> Option<ComponentHook> {
None
}
/// Gets the `on_remove` [`ComponentHook`] for this [`Component`] if one is defined.
fn on_remove() -> Option<ComponentHook> {
None
}
/// Gets the `on_despawn` [`ComponentHook`] for this [`Component`] if one is defined.
fn on_despawn() -> Option<ComponentHook> {
None
}
/// Registers required components.
fn register_required_components(
@ -575,6 +606,26 @@ pub struct ComponentHooks {
}
impl ComponentHooks {
pub(crate) fn update_from_component<C: Component + ?Sized>(&mut self) -> &mut Self {
if let Some(hook) = C::on_add() {
self.on_add(hook);
}
if let Some(hook) = C::on_insert() {
self.on_insert(hook);
}
if let Some(hook) = C::on_replace() {
self.on_replace(hook);
}
if let Some(hook) = C::on_remove() {
self.on_remove(hook);
}
if let Some(hook) = C::on_despawn() {
self.on_despawn(hook);
}
self
}
/// Register a [`ComponentHook`] that will be run when this component is added to an entity.
/// An `on_add` hook will always run before `on_insert` hooks. Spawning an entity counts as
/// adding all of its components.
@ -1181,7 +1232,14 @@ impl Components {
recursion_check_stack,
);
let info = &mut self.components[id.index()];
#[expect(
deprecated,
reason = "need to use this method until it is removed to ensure user defined components register hooks correctly"
)]
// TODO: Replace with `info.hooks.update_from_component::<T>();` once `Component::register_component_hooks` is removed
T::register_component_hooks(&mut info.hooks);
info.required_components = required_components;
let clone_handler = T::get_component_clone_handler();
self.component_clone_handlers

View File

@ -1,6 +1,6 @@
use crate::{
component::{
Component, ComponentCloneHandler, ComponentHooks, HookContext, Mutable, StorageType,
Component, ComponentCloneHandler, ComponentHook, HookContext, Mutable, StorageType,
},
entity::{ComponentCloneCtx, Entity, EntityCloneBuilder},
observer::ObserverState,
@ -16,8 +16,8 @@ impl Component for ObservedBy {
const STORAGE_TYPE: StorageType = StorageType::SparseSet;
type Mutability = Mutable;
fn register_component_hooks(hooks: &mut ComponentHooks) {
hooks.on_remove(|mut world, HookContext { entity, .. }| {
fn on_remove() -> Option<ComponentHook> {
Some(|mut world, HookContext { entity, .. }| {
let observed_by = {
let mut component = world.get_mut::<ObservedBy>(entity).unwrap();
core::mem::take(&mut component.0)
@ -42,7 +42,7 @@ impl Component for ObservedBy {
world.commands().entity(e).despawn();
}
}
});
})
}
fn get_component_clone_handler() -> ComponentCloneHandler {

View File

@ -2,7 +2,7 @@ use alloc::{boxed::Box, vec, vec::Vec};
use core::any::Any;
use crate::{
component::{ComponentHook, ComponentHooks, ComponentId, HookContext, Mutable, StorageType},
component::{ComponentHook, ComponentId, HookContext, Mutable, StorageType},
observer::{ObserverDescriptor, ObserverTrigger},
prelude::*,
query::DebugCheckedUnwrap,
@ -65,13 +65,16 @@ impl Component for ObserverState {
const STORAGE_TYPE: StorageType = StorageType::SparseSet;
type Mutability = Mutable;
fn register_component_hooks(hooks: &mut ComponentHooks) {
hooks.on_add(|mut world, HookContext { entity, .. }| {
fn on_add() -> Option<ComponentHook> {
Some(|mut world, HookContext { entity, .. }| {
world.commands().queue(move |world: &mut World| {
world.register_observer(entity);
});
});
hooks.on_remove(|mut world, HookContext { entity, .. }| {
})
}
fn on_remove() -> Option<ComponentHook> {
Some(|mut world, HookContext { entity, .. }| {
let descriptor = core::mem::take(
&mut world
.entity_mut(entity)
@ -83,7 +86,7 @@ impl Component for ObserverState {
world.commands().queue(move |world: &mut World| {
world.unregister_observer(entity, descriptor);
});
});
})
}
}
@ -322,14 +325,14 @@ impl Observer {
impl Component for Observer {
const STORAGE_TYPE: StorageType = StorageType::SparseSet;
type Mutability = Mutable;
fn register_component_hooks(hooks: &mut ComponentHooks) {
hooks.on_add(|world, context| {
fn on_add() -> Option<ComponentHook> {
Some(|world, context| {
let Some(observe) = world.get::<Self>(context.entity) else {
return;
};
let hook = observe.hook_on_add;
hook(world, context);
});
})
}
}
@ -389,7 +392,7 @@ fn observer_system_runner<E: Event, B: Bundle, S: ObserverSystem<E, B>>(
}
}
/// A [`ComponentHook`] used by [`Observer`] to handle its [`on-add`](`ComponentHooks::on_add`).
/// A [`ComponentHook`] used by [`Observer`] to handle its [`on-add`](`crate::component::ComponentHooks::on_add`).
///
/// This function exists separate from [`Observer`] to allow [`Observer`] to have its type parameters
/// erased.

View File

@ -14,7 +14,7 @@
//! between components (like hierarchies or parent-child links) and need to maintain correctness.
use bevy::{
ecs::component::{ComponentHooks, HookContext, Mutable, StorageType},
ecs::component::{ComponentHook, HookContext, Mutable, StorageType},
prelude::*,
};
use std::collections::HashMap;
@ -33,9 +33,11 @@ impl Component for MyComponent {
type Mutability = Mutable;
/// Hooks can also be registered during component initialization by
/// implementing `register_component_hooks`
fn register_component_hooks(_hooks: &mut ComponentHooks) {
// Register hooks...
/// implementing the associated method
fn on_add() -> Option<ComponentHook> {
// We don't have an `on_add` hook so we'll just return None.
// Note that this is the default behavior when not implementing a hook.
None
}
}