Skip to content

Fix observers' access to relationship data during despawn_related #20258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

muddxyii
Copy link
Contributor

Objective

Solution

  • Changed despawn_related to use get() instead of take() to preserve relationship components during the despawn process
  • Collect entities into a vector before despawning to ensure the relationship data remains accessible to observers and hooks

Testing

  • Added test despawn_related_observers_can_access_relationship_data that reproduces the issue scenario

@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Jul 23, 2025
@alice-i-cecile alice-i-cecile 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 X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jul 23, 2025
Copy link
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.

I've left a suggestion for a clarifying comment, but this is fundamentally a good fix and I really appreciate the regression test.

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
@Trashtalk217 Trashtalk217 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 Jul 24, 2025
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-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 X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

despawn_related removes relationship before calling observers and hooks
3 participants