Skip to content

Fix observer/hook OnReplace and OnRemove triggering when removing a bundle even when the component is not present on the entity - #17942

Merged
alice-i-cecile merged 2 commits into
bevyengine:mainfrom
andriyDev:observers
Feb 24, 2025
Merged

Conversation

@andriyDev

@andriyDev andriyDev commented Feb 20, 2025

Copy link
Copy Markdown
Contributor

Objective

Solution

  • When removing components, we filter the list of components in the removed bundle based on whether they are actually in the archetype.

Testing

  • Added a test.

@andriyDev andriyDev added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events S-Needs-Review Needs reviewer attention (from anyone!) to move forward D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Feb 20, 2025
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Feb 20, 2025
@andriyDev andriyDev changed the title Observers Fix observer/hook OnReplace and OnRemove triggering when removing a bundle even when the component is not present on the entity Feb 20, 2025
fjkorf
fjkorf reviewed Feb 20, 2025
View reviewed changes

@fjkorf fjkorf left a comment

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.

this makes sense. and the test makes it clear what we're up to.

@andriyDev

andriyDev commented Feb 20, 2025

Copy link
Copy Markdown
Contributor Author

@fjkorf Did you intend to approve? 😅 A second approval to move to this into final review would be great!

@andriyDev andriyDev 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 Feb 20, 2025
@alice-i-cecile
alice-i-cecile added this pull request to the merge queue Feb 24, 2025
Merged via the queue into bevyengine:main with commit bb09751 Feb 24, 2025
@ItsDoot ItsDoot mentioned this pull request Apr 4, 2025
Closed
@andriyDev
andriyDev deleted the observers branch May 2, 2026 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@fjkorf fjkorf fjkorf approved these changes
@mnmaita mnmaita mnmaita approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Milestone

0.16

Development

Successfully merging this pull request may close these issues.

All observers triggered when any element of a bundle is removed.

4 participants

@andriyDev @fjkorf @mnmaita @alice-i-cecile