Update the Children component of the parent entity when a scene gets deleted (#12710)
# 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](df15cd7dcc/crates/bevy_scene/src/scene_spawner.rs (L27))
struct that tracks if the SceneInstance was added as a child of a root
entity
This commit is contained in:
parent
bcdb20d4f3
commit
afff818e5c
@ -98,3 +98,78 @@ pub fn scene_spawner(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::{DynamicScene, DynamicSceneBundle, ScenePlugin, SceneSpawner};
|
||||
use bevy_app::{App, ScheduleRunnerPlugin};
|
||||
use bevy_asset::{AssetPlugin, Assets};
|
||||
use bevy_ecs::component::Component;
|
||||
use bevy_ecs::entity::Entity;
|
||||
use bevy_ecs::prelude::{AppTypeRegistry, ReflectComponent, World};
|
||||
use bevy_hierarchy::{Children, HierarchyPlugin};
|
||||
use bevy_reflect::Reflect;
|
||||
use bevy_utils::default;
|
||||
|
||||
#[derive(Component, Reflect, Default)]
|
||||
#[reflect(Component)]
|
||||
struct ComponentA {
|
||||
pub x: f32,
|
||||
pub y: f32,
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spawn_and_delete() {
|
||||
let mut app = App::new();
|
||||
|
||||
app.add_plugins(ScheduleRunnerPlugin::default())
|
||||
.add_plugins(HierarchyPlugin)
|
||||
.add_plugins(AssetPlugin::default())
|
||||
.add_plugins(ScenePlugin)
|
||||
.register_type::<ComponentA>();
|
||||
app.update();
|
||||
|
||||
let mut scene_world = World::new();
|
||||
|
||||
// create a new DynamicScene manually
|
||||
let type_registry = app.world.resource::<AppTypeRegistry>().clone();
|
||||
scene_world.insert_resource(type_registry);
|
||||
scene_world.spawn(ComponentA { x: 3.0, y: 4.0 });
|
||||
let scene = DynamicScene::from_world(&scene_world);
|
||||
let scene_handle = app.world.resource_mut::<Assets<DynamicScene>>().add(scene);
|
||||
|
||||
// spawn the scene as a child of `entity` using the `DynamicSceneBundle`
|
||||
let entity = app
|
||||
.world
|
||||
.spawn(DynamicSceneBundle {
|
||||
scene: scene_handle.clone(),
|
||||
..default()
|
||||
})
|
||||
.id();
|
||||
|
||||
// run the app's schedule once, so that the scene gets spawned
|
||||
app.update();
|
||||
|
||||
// make sure that the scene was added as a child of the root entity
|
||||
let (scene_entity, scene_component_a) = app
|
||||
.world
|
||||
.query::<(Entity, &ComponentA)>()
|
||||
.single(&app.world);
|
||||
assert_eq!(scene_component_a.x, 3.0);
|
||||
assert_eq!(scene_component_a.y, 4.0);
|
||||
assert_eq!(app.world.entity(entity).get::<Children>().unwrap().len(), 1);
|
||||
|
||||
// let's try to delete the scene
|
||||
let mut scene_spawner = app.world.resource_mut::<SceneSpawner>();
|
||||
scene_spawner.despawn(&scene_handle);
|
||||
|
||||
// run the scene spawner system to despawn the scene
|
||||
app.update();
|
||||
|
||||
// the scene entity does not exist anymore
|
||||
assert!(app.world.get_entity(scene_entity).is_none());
|
||||
|
||||
// the root entity does not have any children anymore
|
||||
assert!(app.world.entity(entity).get::<Children>().is_none());
|
||||
}
|
||||
}
|
||||
|
||||
@ -8,7 +8,7 @@ use bevy_ecs::{
|
||||
system::Resource,
|
||||
world::{Command, Mut, World},
|
||||
};
|
||||
use bevy_hierarchy::{Parent, PushChild};
|
||||
use bevy_hierarchy::{BuildWorldChildren, DespawnRecursiveExt, Parent, PushChild};
|
||||
use bevy_utils::{tracing::error, HashMap, HashSet};
|
||||
use thiserror::Error;
|
||||
use uuid::Uuid;
|
||||
@ -189,7 +189,10 @@ impl SceneSpawner {
|
||||
pub fn despawn_instance_sync(&mut self, world: &mut World, instance_id: &InstanceId) {
|
||||
if let Some(instance) = self.spawned_instances.remove(instance_id) {
|
||||
for &entity in instance.entity_map.values() {
|
||||
let _ = world.despawn(entity);
|
||||
if let Some(mut entity_mut) = world.get_entity_mut(entity) {
|
||||
entity_mut.remove_parent();
|
||||
entity_mut.despawn_recursive();
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user