Skip to content
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

Update the Children component of the parent entity when a scene gets deleted #12710

Merged

Conversation

cBournhonesque
Copy link
Contributor

@cBournhonesque cBournhonesque commented Mar 25, 2024

Objective

  • A scene usually gets created using the SceneBundle or DynamicSceneBundle. This means that the scene's entities get added as children of the root entity (the entity on which the SceneBundle gets added)
  • When the scene gets deleted using the SceneSpawner, the scene's entities are deleted, but the Children component of the root entity doesn't get updated. This means that the hierarchy becomes unsound, with Children linking to non-existing components.

Solution

  • Update the despawn_sync logic to also update the Children from any parents of the scene, if there are any
  • Adds a test where a Scene gets despawned and checks for dangling Children references on the parent. The test fails on main but works here.

Alternative implementations

  • One option could be to add a parent: Option<Entity> on the InstanceInfo struct that tracks if the SceneInstance was added as a child of a root entity

@cBournhonesque cBournhonesque added the A-Scenes Serialized ECS data stored on the disk label Mar 25, 2024
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Hierarchy Parent-child entity hierarchies labels Mar 25, 2024
@alice-i-cecile
Copy link
Member

#12235 may obsolete this PR.

Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

Thanks for adding an test for this!

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 29, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 29, 2024
Merged via the queue into bevyengine:main with commit afff818 Mar 29, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Hierarchy Parent-child entity hierarchies A-Scenes Serialized ECS data stored on the disk C-Bug An unexpected or incorrect behavior 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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants