Skip to content

Allow all hooks on relationship components#24000

Merged
alice-i-cecile merged 3 commits intobevyengine:mainfrom
SpecificProtagonist:push-nnlsqowsnvsk
Apr 28, 2026
Merged

Allow all hooks on relationship components#24000
alice-i-cecile merged 3 commits intobevyengine:mainfrom
SpecificProtagonist:push-nnlsqowsnvsk

Conversation

@SpecificProtagonist
Copy link
Copy Markdown
Contributor

@SpecificProtagonist SpecificProtagonist commented Apr 27, 2026

Objective

Fixes #23990

Solution

If both the user and the relationship define a hook, call both.

Testing

Added a test checking hooks don't interfere with relationships.


Showcase

#[derive(Component)]
#[relationship(relationship_target = LikedBy)]
#[component(on_add = on_add)]
struct Likes(Entity);

@SpecificProtagonist SpecificProtagonist added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events S-Needs-Review Needs reviewer attention (from anyone!) to move forward D-Macros Code that generates Rust code labels Apr 27, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in ECS Apr 27, 2026
struct RelTarget(Entity);

fn on_add(world: DeferredWorld, context: HookContext) {
assert!(!world.entity(context.entity).contains::<RelTarget>());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is a point of discussion: do we want the user defined hook to run before or after the relationship one?

Copy link
Copy Markdown
Contributor Author

@SpecificProtagonist SpecificProtagonist Apr 27, 2026

Choose a reason for hiding this comment

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

I think it doesn't matter a lot because structural changes are deferred. Currently the order in which hooks are called when inserting/removing multiple components at once isn't specified either (and they all run before the world is flushed).
I suppose this assert is misleading in this regard as it's independent of the order.

Copy link
Copy Markdown
Contributor

@chescock chescock left a comment

Choose a reason for hiding this comment

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

Oh, nice, you managed to get this down to a pretty small change!

Comment thread crates/bevy_ecs/macros/src/component.rs Outdated
Comment thread crates/bevy_ecs/macros/src/component.rs
Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Wow, that's incredibly nice. Love to see it!

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 28, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 28, 2026
@alice-i-cecile alice-i-cecile mentioned this pull request Apr 28, 2026
1 task
Merged via the queue into bevyengine:main with commit a536222 Apr 28, 2026
40 checks passed
@github-project-automation github-project-automation Bot moved this from Needs SME Triage to Done in ECS Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Macros Code that generates Rust code S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Relationship components cannot define their own lifecycle hooks

4 participants