From 0ca3c938b6a2a1b2e587a896ff39fa7452faf206 Mon Sep 17 00:00:00 2001 From: FpguDhk <3411015214@qq.com> Date: Fri, 29 Mar 2024 17:47:25 +0800 Subject: [PATCH 1/9] Remove `InstanceId` when Scene Despawn --- crates/bevy_scene/src/bundle.rs | 2 +- crates/bevy_scene/src/lib.rs | 22 ++++++++++++++++++++-- crates/bevy_scene/src/scene_spawner.rs | 11 ++++------- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/crates/bevy_scene/src/bundle.rs b/crates/bevy_scene/src/bundle.rs index a0b27b44d0a0e..aa626808110f1 100644 --- a/crates/bevy_scene/src/bundle.rs +++ b/crates/bevy_scene/src/bundle.rs @@ -16,7 +16,7 @@ use crate::{DynamicScene, InstanceId, Scene, SceneSpawner}; /// [`InstanceId`] of a spawned scene. It can be used with the [`SceneSpawner`] to /// interact with the spawned scene. #[derive(Component, Deref, DerefMut)] -pub struct SceneInstance(InstanceId); +pub struct SceneInstance(pub(crate) InstanceId); /// A component bundle for a [`Scene`] root. /// diff --git a/crates/bevy_scene/src/lib.rs b/crates/bevy_scene/src/lib.rs index 0c1a89785412f..15eeda0ef31f1 100644 --- a/crates/bevy_scene/src/lib.rs +++ b/crates/bevy_scene/src/lib.rs @@ -25,7 +25,7 @@ pub mod serde; /// Rusty Object Notation, a crate used to serialize and deserialize bevy scenes. pub use bevy_asset::ron; -use bevy_ecs::schedule::IntoSystemConfigs; +use bevy_ecs::{schedule::IntoSystemConfigs, world::World}; pub use bundle::*; pub use dynamic_scene::*; pub use dynamic_scene_builder::*; @@ -44,7 +44,7 @@ pub mod prelude { } use bevy_app::prelude::*; -use bevy_asset::AssetApp; +use bevy_asset::{AssetApp, Handle}; /// Plugin that provides scene functionality to an [`App`]. #[derive(Default)] @@ -58,6 +58,7 @@ impl Plugin for ScenePlugin { .init_asset_loader::() .add_event::() .init_resource::() + .add_systems(Startup, setup) .add_systems(SpawnScene, (scene_spawner, scene_spawner_system).chain()); } } @@ -66,3 +67,20 @@ impl Plugin for ScenePlugin { impl Plugin for ScenePlugin { fn build(&self, _: &mut App) {} } + +fn setup(world: &mut World) { + world + .register_component_hooks::>() + .on_remove(|mut world, entity, _| { + let id = world.get::>(entity).unwrap().id(); + if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { + let mut scene_spawner = world.resource_mut::(); + scene_spawner + .spawned_dynamic_scenes + .get_mut(&id) + .map(|instance_ids| { + instance_ids.remove(&scene_instance); + }); + } + }); +} diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index 08a9695f79a10..47cf9f6a74d37 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -60,8 +60,7 @@ impl InstanceId { /// - [`despawn_instance`](Self::despawn_instance) #[derive(Default, Resource)] pub struct SceneSpawner { - spawned_scenes: HashMap, Vec>, - spawned_dynamic_scenes: HashMap, Vec>, + pub(crate) spawned_dynamic_scenes: HashMap, HashSet>, spawned_instances: HashMap, scene_asset_event_reader: ManualEventReader>, dynamic_scenes_to_spawn: Vec<(Handle, InstanceId)>, @@ -207,7 +206,7 @@ impl SceneSpawner { self.spawned_instances .insert(instance_id, InstanceInfo { entity_map }); let spawned = self.spawned_dynamic_scenes.entry(id).or_default(); - spawned.push(instance_id); + spawned.insert(instance_id); Ok(instance_id) } @@ -248,8 +247,6 @@ impl SceneSpawner { scene.write_to_world_with(world, &world.resource::().clone())?; self.spawned_instances.insert(instance_id, instance_info); - let spawned = self.spawned_scenes.entry(id).or_default(); - spawned.push(instance_id); Ok(instance_id) }) } @@ -307,8 +304,8 @@ impl SceneSpawner { let spawned = self .spawned_dynamic_scenes .entry(handle.id()) - .or_insert_with(Vec::new); - spawned.push(instance_id); + .or_insert_with(HashSet::new); + spawned.insert(instance_id); } Err(SceneSpawnError::NonExistentScene { .. }) => { self.dynamic_scenes_to_spawn.push((handle, instance_id)); From 0f9cc3030b14acbc3730f123251bb73f88b97723 Mon Sep 17 00:00:00 2001 From: FpguDhk <3411015214@qq.com> Date: Fri, 29 Mar 2024 19:11:57 +0800 Subject: [PATCH 2/9] Fix leak in `spawned_instances` --- crates/bevy_scene/src/lib.rs | 10 ++++------ crates/bevy_scene/src/scene_spawner.rs | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/crates/bevy_scene/src/lib.rs b/crates/bevy_scene/src/lib.rs index 15eeda0ef31f1..7bdc1584dddf4 100644 --- a/crates/bevy_scene/src/lib.rs +++ b/crates/bevy_scene/src/lib.rs @@ -75,12 +75,10 @@ fn setup(world: &mut World) { let id = world.get::>(entity).unwrap().id(); if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { let mut scene_spawner = world.resource_mut::(); - scene_spawner - .spawned_dynamic_scenes - .get_mut(&id) - .map(|instance_ids| { - instance_ids.remove(&scene_instance); - }); + if let Some(instance_ids) = scene_spawner.spawned_dynamic_scenes.get_mut(&id) { + instance_ids.remove(&scene_instance); + } + scene_spawner.despawn_instance(scene_instance); } }); } diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index 47cf9f6a74d37..8c3b65877c506 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -61,7 +61,7 @@ impl InstanceId { #[derive(Default, Resource)] pub struct SceneSpawner { pub(crate) spawned_dynamic_scenes: HashMap, HashSet>, - spawned_instances: HashMap, + pub(crate) spawned_instances: HashMap, scene_asset_event_reader: ManualEventReader>, dynamic_scenes_to_spawn: Vec<(Handle, InstanceId)>, scenes_to_spawn: Vec<(Handle, InstanceId)>, From 96110ec9d7430ba00b14e30721e46195828d6b19 Mon Sep 17 00:00:00 2001 From: FpguDhk <3411015214@qq.com> Date: Fri, 29 Mar 2024 20:02:29 +0800 Subject: [PATCH 3/9] Add remove hook for normal scenes --- crates/bevy_scene/src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/bevy_scene/src/lib.rs b/crates/bevy_scene/src/lib.rs index 7bdc1584dddf4..b80237a6def3b 100644 --- a/crates/bevy_scene/src/lib.rs +++ b/crates/bevy_scene/src/lib.rs @@ -81,4 +81,12 @@ fn setup(world: &mut World) { scene_spawner.despawn_instance(scene_instance); } }); + world + .register_component_hooks::>() + .on_remove(|mut world, entity, _| { + if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { + let mut scene_spawner = world.resource_mut::(); + scene_spawner.despawn_instance(scene_instance); + } + }); } From 9c5249beec892eb4dbe4d30ce988022caf66bd0a Mon Sep 17 00:00:00 2001 From: FpguDhk <3411015214@qq.com> Date: Fri, 29 Mar 2024 20:09:05 +0800 Subject: [PATCH 4/9] use `get_entity_mut` to despawn entities --- crates/bevy_scene/src/scene_spawner.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index 8c3b65877c506..9e955889c403f 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -188,7 +188,9 @@ 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(entity) = world.get_entity_mut(entity) { + entity.despawn(); + } } } } From 6b9cb55f47cd26b09efd7a10ddcb937e16690232 Mon Sep 17 00:00:00 2001 From: FpguDhk <3411015214@qq.com> Date: Sat, 30 Mar 2024 01:39:43 +0800 Subject: [PATCH 5/9] Add component hooks via `App::world` --- crates/bevy_scene/src/lib.rs | 48 ++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/crates/bevy_scene/src/lib.rs b/crates/bevy_scene/src/lib.rs index b80237a6def3b..73b856630baeb 100644 --- a/crates/bevy_scene/src/lib.rs +++ b/crates/bevy_scene/src/lib.rs @@ -58,8 +58,31 @@ impl Plugin for ScenePlugin { .init_asset_loader::() .add_event::() .init_resource::() - .add_systems(Startup, setup) .add_systems(SpawnScene, (scene_spawner, scene_spawner_system).chain()); + + // Register component hooks for DynamicScene + app.world + .register_component_hooks::>() + .on_remove(|mut world, entity, _| { + let id = world.get::>(entity).unwrap().id(); + if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { + let mut scene_spawner = world.resource_mut::(); + if let Some(instance_ids) = scene_spawner.spawned_dynamic_scenes.get_mut(&id) { + instance_ids.remove(&scene_instance); + } + scene_spawner.despawn_instance(scene_instance); + } + }); + + // Register component hooks for Scene + app.world + .register_component_hooks::>() + .on_remove(|mut world, entity, _| { + if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { + let mut scene_spawner = world.resource_mut::(); + scene_spawner.despawn_instance(scene_instance); + } + }); } } @@ -67,26 +90,3 @@ impl Plugin for ScenePlugin { impl Plugin for ScenePlugin { fn build(&self, _: &mut App) {} } - -fn setup(world: &mut World) { - world - .register_component_hooks::>() - .on_remove(|mut world, entity, _| { - let id = world.get::>(entity).unwrap().id(); - if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { - let mut scene_spawner = world.resource_mut::(); - if let Some(instance_ids) = scene_spawner.spawned_dynamic_scenes.get_mut(&id) { - instance_ids.remove(&scene_instance); - } - scene_spawner.despawn_instance(scene_instance); - } - }); - world - .register_component_hooks::>() - .on_remove(|mut world, entity, _| { - if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { - let mut scene_spawner = world.resource_mut::(); - scene_spawner.despawn_instance(scene_instance); - } - }); -} From c10dbbb81f899ff300497ced4ba678ef189a196b Mon Sep 17 00:00:00 2001 From: FpguDhk <3411015214@qq.com> Date: Sat, 30 Mar 2024 01:44:50 +0800 Subject: [PATCH 6/9] Remove unused import --- crates/bevy_scene/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_scene/src/lib.rs b/crates/bevy_scene/src/lib.rs index 73b856630baeb..ddce38970c5e9 100644 --- a/crates/bevy_scene/src/lib.rs +++ b/crates/bevy_scene/src/lib.rs @@ -25,7 +25,7 @@ pub mod serde; /// Rusty Object Notation, a crate used to serialize and deserialize bevy scenes. pub use bevy_asset::ron; -use bevy_ecs::{schedule::IntoSystemConfigs, world::World}; +use bevy_ecs::schedule::IntoSystemConfigs; pub use bundle::*; pub use dynamic_scene::*; pub use dynamic_scene_builder::*; From 4bb2ecc456360b3fa7f168c122bfe594eaa3e7b0 Mon Sep 17 00:00:00 2001 From: FpguDhk <3411015214@qq.com> Date: Sat, 30 Mar 2024 02:07:09 +0800 Subject: [PATCH 7/9] Replace `resource_mut` with `get_resource_mut` --- crates/bevy_scene/src/lib.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/bevy_scene/src/lib.rs b/crates/bevy_scene/src/lib.rs index ddce38970c5e9..52862ff4e77b3 100644 --- a/crates/bevy_scene/src/lib.rs +++ b/crates/bevy_scene/src/lib.rs @@ -64,9 +64,14 @@ impl Plugin for ScenePlugin { app.world .register_component_hooks::>() .on_remove(|mut world, entity, _| { - let id = world.get::>(entity).unwrap().id(); + let Some(handle) = world.get::>(entity) else { + return; + }; + let id = handle.id(); if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { - let mut scene_spawner = world.resource_mut::(); + let Some(mut scene_spawner) = world.get_resource_mut::() else { + return; + } if let Some(instance_ids) = scene_spawner.spawned_dynamic_scenes.get_mut(&id) { instance_ids.remove(&scene_instance); } @@ -79,7 +84,9 @@ impl Plugin for ScenePlugin { .register_component_hooks::>() .on_remove(|mut world, entity, _| { if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { - let mut scene_spawner = world.resource_mut::(); + let Some(mut scene_spawner) = world.get_resource_mut::() else { + return; + } scene_spawner.despawn_instance(scene_instance); } }); From 9029b353e378f1e68d9d259cda1e667fb192e591 Mon Sep 17 00:00:00 2001 From: FpguDhk <3411015214@qq.com> Date: Sat, 30 Mar 2024 02:09:38 +0800 Subject: [PATCH 8/9] Fix CI --- crates/bevy_scene/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_scene/src/lib.rs b/crates/bevy_scene/src/lib.rs index 52862ff4e77b3..a05e9a5f8619e 100644 --- a/crates/bevy_scene/src/lib.rs +++ b/crates/bevy_scene/src/lib.rs @@ -71,7 +71,7 @@ impl Plugin for ScenePlugin { if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { let Some(mut scene_spawner) = world.get_resource_mut::() else { return; - } + }; if let Some(instance_ids) = scene_spawner.spawned_dynamic_scenes.get_mut(&id) { instance_ids.remove(&scene_instance); } @@ -86,7 +86,7 @@ impl Plugin for ScenePlugin { if let Some(&SceneInstance(scene_instance)) = world.get::(entity) { let Some(mut scene_spawner) = world.get_resource_mut::() else { return; - } + }; scene_spawner.despawn_instance(scene_instance); } }); From 346dfd6562b3ebc43c0de920a1a2ff2ca11fd7fb Mon Sep 17 00:00:00 2001 From: FpguDhk <3411015214@qq.com> Date: Sat, 30 Mar 2024 19:54:15 +0800 Subject: [PATCH 9/9] Add despawn scene test --- crates/bevy_scene/src/scene_spawner.rs | 50 +++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index c93aae2c2908f..5aa918e2c0ea0 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -440,17 +440,14 @@ pub fn scene_spawner_system(world: &mut World) { mod tests { use bevy_app::App; use bevy_asset::{AssetPlugin, AssetServer}; - use bevy_ecs::component::Component; - use bevy_ecs::entity::Entity; use bevy_ecs::event::EventReader; use bevy_ecs::prelude::ReflectComponent; use bevy_ecs::query::With; - use bevy_ecs::reflect::AppTypeRegistry; use bevy_ecs::system::{Commands, Res, ResMut, RunSystemOnce}; - use bevy_ecs::world::World; + use bevy_ecs::{component::Component, system::Query}; use bevy_reflect::Reflect; - use crate::{DynamicScene, DynamicSceneBuilder, SceneInstanceReady, ScenePlugin, SceneSpawner}; + use crate::{DynamicSceneBuilder, ScenePlugin}; use super::*; @@ -551,4 +548,47 @@ mod tests { }, ); } + + #[test] + fn despawn_scene() { + let mut app = App::new(); + app.add_plugins((AssetPlugin::default(), ScenePlugin)); + app.register_type::(); + + let asset_server = app.world.resource::(); + + // Build scene. + let scene = asset_server.add(DynamicScene::default()); + let count = 10; + + // Checks the number of scene instances stored in `SceneSpawner`. + let check = |world: &mut World, expected_count: usize| { + let scene_spawner = world.resource::(); + assert_eq!( + scene_spawner.spawned_dynamic_scenes[&scene.id()].len(), + expected_count + ); + assert_eq!(scene_spawner.spawned_instances.len(), expected_count); + }; + + // Spawn scene. + for _ in 0..count { + app.world.spawn((ComponentA, scene.clone())); + } + + app.update(); + check(&mut app.world, count); + + // Despawn scene. + app.world.run_system_once( + |mut commands: Commands, query: Query>| { + for entity in query.iter() { + commands.entity(entity).despawn_recursive(); + } + }, + ); + + app.update(); + check(&mut app.world, 0); + } }