From ea2a7e5552ea364ee9a12771162a4691f21891bd Mon Sep 17 00:00:00 2001 From: daxpedda Date: Sat, 6 Jul 2024 16:00:39 +0200 Subject: [PATCH] Send `SceneInstanceReady` when spawning any kind of scene (#11741) # Objective - Emit an event regardless of scene type (`Scene` and `DynamicScene`). - Also send the `InstanceId` along. Follow-up to #11002. Fixes #2218. ## Solution - Send `SceneInstanceReady` regardless of scene type. - Make `SceneInstanceReady::parent` `Option`al. - Add `SceneInstanceReady::id`. --- ## Changelog ### Changed - `SceneInstanceReady` is now sent for `Scene` as well. `SceneInstanceReady::parent` is an `Option` and `SceneInstanceReady::id`, an `InstanceId`, is added to identify the corresponding `Scene`. ## Migration Guide - `SceneInstanceReady { parent: Entity }` is now `SceneInstanceReady { id: InstanceId, parent: Option }`. --- crates/bevy_scene/src/scene_spawner.rs | 211 +++++++++++++++++++++---- 1 file changed, 183 insertions(+), 28 deletions(-) diff --git a/crates/bevy_scene/src/scene_spawner.rs b/crates/bevy_scene/src/scene_spawner.rs index e478e7b412676..9e43b8711d154 100644 --- a/crates/bevy_scene/src/scene_spawner.rs +++ b/crates/bevy_scene/src/scene_spawner.rs @@ -18,8 +18,10 @@ use uuid::Uuid; /// See also [`SceneSpawner::instance_is_ready`]. #[derive(Clone, Copy, Debug, Eq, PartialEq, Event)] pub struct SceneInstanceReady { + /// ID of the spawned instance. + pub id: InstanceId, /// Entity to which the scene was spawned as a child. - pub parent: Entity, + pub parent: Option, } /// Information about a scene instance. @@ -63,8 +65,8 @@ pub struct SceneSpawner { pub(crate) spawned_dynamic_scenes: HashMap, HashSet>, pub(crate) spawned_instances: HashMap, scene_asset_event_reader: ManualEventReader>, - dynamic_scenes_to_spawn: Vec<(Handle, InstanceId)>, - scenes_to_spawn: Vec<(Handle, InstanceId)>, + dynamic_scenes_to_spawn: Vec<(Handle, InstanceId, Option)>, + scenes_to_spawn: Vec<(Handle, InstanceId, Option)>, scenes_to_despawn: Vec>, instances_to_despawn: Vec, scenes_with_parent: Vec<(InstanceId, Entity)>, @@ -128,7 +130,8 @@ impl SceneSpawner { /// Schedule the spawn of a new instance of the provided dynamic scene. pub fn spawn_dynamic(&mut self, id: impl Into>) -> InstanceId { let instance_id = InstanceId::new(); - self.dynamic_scenes_to_spawn.push((id.into(), instance_id)); + self.dynamic_scenes_to_spawn + .push((id.into(), instance_id, None)); instance_id } @@ -139,7 +142,8 @@ impl SceneSpawner { parent: Entity, ) -> InstanceId { let instance_id = InstanceId::new(); - self.dynamic_scenes_to_spawn.push((id.into(), instance_id)); + self.dynamic_scenes_to_spawn + .push((id.into(), instance_id, Some(parent))); self.scenes_with_parent.push((instance_id, parent)); instance_id } @@ -147,14 +151,15 @@ impl SceneSpawner { /// Schedule the spawn of a new instance of the provided scene. pub fn spawn(&mut self, id: impl Into>) -> InstanceId { let instance_id = InstanceId::new(); - self.scenes_to_spawn.push((id.into(), instance_id)); + self.scenes_to_spawn.push((id.into(), instance_id, None)); instance_id } /// Schedule the spawn of a new instance of the provided scene as a child of `parent`. pub fn spawn_as_child(&mut self, id: impl Into>, parent: Entity) -> InstanceId { let instance_id = InstanceId::new(); - self.scenes_to_spawn.push((id.into(), instance_id)); + self.scenes_to_spawn + .push((id.into(), instance_id, Some(parent))); self.scenes_with_parent.push((instance_id, parent)); instance_id } @@ -296,7 +301,7 @@ impl SceneSpawner { pub fn spawn_queued_scenes(&mut self, world: &mut World) -> Result<(), SceneSpawnError> { let scenes_to_spawn = std::mem::take(&mut self.dynamic_scenes_to_spawn); - for (handle, instance_id) in scenes_to_spawn { + for (handle, instance_id, parent) in scenes_to_spawn { let mut entity_map = EntityHashMap::default(); match Self::spawn_dynamic_internal(world, handle.id(), &mut entity_map) { @@ -308,9 +313,19 @@ impl SceneSpawner { .entry(handle.id()) .or_insert_with(HashSet::new); spawned.insert(instance_id); + + // Scenes with parents need more setup before they are ready. + // See `set_scene_instance_parent_sync()`. + if parent.is_none() { + world.send_event(SceneInstanceReady { + id: instance_id, + parent: None, + }); + } } Err(SceneSpawnError::NonExistentScene { .. }) => { - self.dynamic_scenes_to_spawn.push((handle, instance_id)); + self.dynamic_scenes_to_spawn + .push((handle, instance_id, parent)); } Err(err) => return Err(err), } @@ -318,11 +333,21 @@ impl SceneSpawner { let scenes_to_spawn = std::mem::take(&mut self.scenes_to_spawn); - for (scene_handle, instance_id) in scenes_to_spawn { + for (scene_handle, instance_id, parent) in scenes_to_spawn { match self.spawn_sync_internal(world, scene_handle.id(), instance_id) { - Ok(_) => {} + Ok(_) => { + // Scenes with parents need more setup before they are ready. + // See `set_scene_instance_parent_sync()`. + if parent.is_none() { + world.send_event(SceneInstanceReady { + id: instance_id, + parent: None, + }); + } + } Err(SceneSpawnError::NonExistentRealScene { .. }) => { - self.scenes_to_spawn.push((scene_handle, instance_id)); + self.scenes_to_spawn + .push((scene_handle, instance_id, parent)); } Err(err) => return Err(err), } @@ -356,7 +381,10 @@ impl SceneSpawner { } } - world.send_event(SceneInstanceReady { parent }); + world.send_event(SceneInstanceReady { + id: instance_id, + parent: Some(parent), + }); } else { self.scenes_with_parent.push((instance_id, parent)); } @@ -403,10 +431,10 @@ pub fn scene_spawner_system(world: &mut World) { }); scene_spawner .dynamic_scenes_to_spawn - .retain(|(_, instance)| !dead_instances.contains(instance)); + .retain(|(_, instance, _)| !dead_instances.contains(instance)); scene_spawner .scenes_to_spawn - .retain(|(_, instance)| !dead_instances.contains(instance)); + .retain(|(_, instance, _)| !dead_instances.contains(instance)); let scene_asset_events = world.resource::>>(); @@ -438,6 +466,7 @@ pub fn scene_spawner_system(world: &mut World) { #[cfg(test)] mod tests { use bevy_app::App; + use bevy_asset::Handle; use bevy_asset::{AssetPlugin, AssetServer}; use bevy_ecs::event::EventReader; use bevy_ecs::prelude::ReflectComponent; @@ -505,8 +534,7 @@ mod tests { #[reflect(Component)] struct ComponentA; - #[test] - fn event() { + fn setup() -> App { let mut app = App::new(); app.add_plugins((AssetPlugin::default(), ScenePlugin)); @@ -514,19 +542,142 @@ mod tests { app.world_mut().spawn(ComponentA); app.world_mut().spawn(ComponentA); + app + } + + fn build_scene(app: &mut App) -> Handle { + app.world_mut().run_system_once( + |world: &World, + type_registry: Res<'_, AppTypeRegistry>, + asset_server: Res<'_, AssetServer>| { + asset_server.add( + Scene::from_dynamic_scene(&DynamicScene::from_world(world), &type_registry) + .unwrap(), + ) + }, + ) + } + + #[test] + fn event_scene() { + let mut app = setup(); + // Build scene. - let scene = + let scene = build_scene(&mut app); + + // Spawn scene. + let scene_id = app.world_mut() - .run_system_once(|world: &World, asset_server: Res<'_, AssetServer>| { - asset_server.add(DynamicScene::from_world(world)) + .run_system_once(move |mut scene_spawner: ResMut<'_, SceneSpawner>| { + scene_spawner.spawn(scene.clone()) }); + // Check for event arrival. + app.update(); + app.world_mut().run_system_once( + move |mut ev_scene: EventReader<'_, '_, SceneInstanceReady>| { + let mut events = ev_scene.read(); + + let event = events.next().expect("found no `SceneInstanceReady` event"); + assert_eq!( + event.id, scene_id, + "`SceneInstanceReady` contains the wrong `InstanceId`" + ); + + assert!(events.next().is_none(), "found more than one event"); + }, + ); + } + + #[test] + fn event_scene_as_child() { + let mut app = setup(); + + // Build scene. + let scene = build_scene(&mut app); + + // Spawn scene as child. + let (scene_id, scene_entity) = app.world_mut().run_system_once( + move |mut commands: Commands<'_, '_>, mut scene_spawner: ResMut<'_, SceneSpawner>| { + let entity = commands.spawn_empty().id(); + let id = scene_spawner.spawn_as_child(scene.clone(), entity); + (id, entity) + }, + ); + + // Check for event arrival. + app.update(); + app.world_mut().run_system_once( + move |mut ev_scene: EventReader<'_, '_, SceneInstanceReady>| { + let mut events = ev_scene.read(); + + let event = events.next().expect("found no `SceneInstanceReady` event"); + assert_eq!( + event.id, scene_id, + "`SceneInstanceReady` contains the wrong `InstanceId`" + ); + assert_eq!( + event.parent, + Some(scene_entity), + "`SceneInstanceReady` contains the wrong parent entity" + ); + + assert!(events.next().is_none(), "found more than one event"); + }, + ); + } + + fn build_dynamic_scene(app: &mut App) -> Handle { + app.world_mut() + .run_system_once(|world: &World, asset_server: Res<'_, AssetServer>| { + asset_server.add(DynamicScene::from_world(world)) + }) + } + + #[test] + fn event_dynamic_scene() { + let mut app = setup(); + + // Build scene. + let scene = build_dynamic_scene(&mut app); + // Spawn scene. - let scene_entity = app.world_mut().run_system_once( + let scene_id = + app.world_mut() + .run_system_once(move |mut scene_spawner: ResMut<'_, SceneSpawner>| { + scene_spawner.spawn_dynamic(scene.clone()) + }); + + // Check for event arrival. + app.update(); + app.world_mut().run_system_once( + move |mut ev_scene: EventReader<'_, '_, SceneInstanceReady>| { + let mut events = ev_scene.read(); + + let event = events.next().expect("found no `SceneInstanceReady` event"); + assert_eq!( + event.id, scene_id, + "`SceneInstanceReady` contains the wrong `InstanceId`" + ); + + assert!(events.next().is_none(), "found more than one event"); + }, + ); + } + + #[test] + fn event_dynamic_scene_as_child() { + let mut app = setup(); + + // Build scene. + let scene = build_dynamic_scene(&mut app); + + // Spawn scene as child. + let (scene_id, scene_entity) = app.world_mut().run_system_once( move |mut commands: Commands<'_, '_>, mut scene_spawner: ResMut<'_, SceneSpawner>| { - let scene_entity = commands.spawn_empty().id(); - scene_spawner.spawn_dynamic_as_child(scene.clone(), scene_entity); - scene_entity + let entity = commands.spawn_empty().id(); + let id = scene_spawner.spawn_dynamic_as_child(scene.clone(), entity); + (id, entity) }, ); @@ -536,13 +687,17 @@ mod tests { move |mut ev_scene: EventReader<'_, '_, SceneInstanceReady>| { let mut events = ev_scene.read(); + let event = events.next().expect("found no `SceneInstanceReady` event"); assert_eq!( - events.next().expect("found no `SceneInstanceReady` event"), - &SceneInstanceReady { - parent: scene_entity - }, + event.id, scene_id, + "`SceneInstanceReady` contains the wrong `InstanceId`" + ); + assert_eq!( + event.parent, + Some(scene_entity), "`SceneInstanceReady` contains the wrong parent entity" ); + assert!(events.next().is_none(), "found more than one event"); }, );