Skip to content

Commit

Permalink
Change SceneInstanceReady to trigger an observer. (#13859)
Browse files Browse the repository at this point in the history
# Objective

The `SceneInstanceReady` event would be more ergonomic (and potentially
efficient) if it could be delivered to listeners attached to the scene
entities becoming ready rather than into a World-global queue.

This is an evolution of @Shatur's work in #9313.

## Solution

The scene spawner is changed to trigger observers on the scene entity
when it is ready rather than enqueue an event with `EventWriter`.

This addresses the two outstanding feature requests mentioned on #2218,
that i) the events should be "scoped" in some way and ii) that the
`InstanceId` should be included in the event.

## Testing

Modified the `scene_spawner::tests::event` test to use the new
mechanism.

---

## Changelog

- Changed `SceneInstanceReady` to trigger an entity observer rather than
be written to an event queue.
- Changed `SceneInstanceReady` to carry the `InstanceId` of the scene.

## Migration Guide

If you have a system which read `SceneInstanceReady` events:

> ```fn ready_system(ready_events: EventReader<'_, '_,
SceneInstanceReady>) {```

It must be rewritten as an observer:

> ```commands.observe(|trigger: Trigger<SceneInstanceReady>| {```

Or, if you were expecting the event in relation to a specific entity or
entities, as an entity observer:

> ```commands.entity(entity).observe(|trigger:
Trigger<SceneInstanceReady>| {```
  • Loading branch information
komadori committed Jul 30, 2024
1 parent ba09f35 commit 3d1c9ca
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 112 deletions.
1 change: 0 additions & 1 deletion crates/bevy_scene/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ impl Plugin for ScenePlugin {
app.init_asset::<DynamicScene>()
.init_asset::<Scene>()
.init_asset_loader::<SceneLoader>()
.add_event::<SceneInstanceReady>()
.init_resource::<SceneSpawner>()
.add_systems(SpawnScene, (scene_spawner, scene_spawner_system).chain());

Expand Down
195 changes: 84 additions & 111 deletions crates/bevy_scene/src/scene_spawner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ use bevy_utils::{tracing::error, HashMap, HashSet};
use thiserror::Error;
use uuid::Uuid;

/// Emitted when [`crate::SceneInstance`] becomes ready to use.
/// Triggered on a scene's parent entity when [`crate::SceneInstance`] becomes ready to use.
///
/// See also [`SceneSpawner::instance_is_ready`].
/// See also [`Trigger`], [`SceneSpawner::instance_is_ready`].
///
/// [`Trigger`]: bevy_ecs::observer::Trigger
#[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: Option<Entity>,
/// Instance which has been spawned.
pub instance_id: InstanceId,
}

/// Information about a scene instance.
Expand Down Expand Up @@ -323,10 +323,8 @@ impl SceneSpawner {
// 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,
});
// Defer via commands otherwise SceneSpawner is not available in the observer.
world.commands().trigger(SceneInstanceReady { instance_id });
}
}
Err(SceneSpawnError::NonExistentScene { .. }) => {
Expand All @@ -350,10 +348,8 @@ impl SceneSpawner {
// 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,
});
// Defer via commands otherwise SceneSpawner is not available in the observer.
world.commands().trigger(SceneInstanceReady { instance_id });
}
}
Err(SceneSpawnError::NonExistentRealScene { .. }) => {
Expand Down Expand Up @@ -392,10 +388,10 @@ impl SceneSpawner {
}
}

world.send_event(SceneInstanceReady {
id: instance_id,
parent: Some(parent),
});
// Defer via commands otherwise SceneSpawner is not available in the observer.
world
.commands()
.trigger_targets(SceneInstanceReady { instance_id }, parent);
} else {
self.scenes_with_parent.push((instance_id, parent));
}
Expand Down Expand Up @@ -479,7 +475,7 @@ mod tests {
use bevy_app::App;
use bevy_asset::Handle;
use bevy_asset::{AssetPlugin, AssetServer};
use bevy_ecs::event::EventReader;
use bevy_ecs::observer::Trigger;
use bevy_ecs::prelude::ReflectComponent;
use bevy_ecs::query::With;
use bevy_ecs::system::{Commands, Res, ResMut, RunSystemOnce};
Expand Down Expand Up @@ -545,9 +541,13 @@ mod tests {
#[reflect(Component)]
struct ComponentA;

#[derive(Resource, Default)]
struct TriggerCount(u32);

fn setup() -> App {
let mut app = App::new();
app.add_plugins((AssetPlugin::default(), ScenePlugin));
app.init_resource::<TriggerCount>();

app.register_type::<ComponentA>();
app.world_mut().spawn(ComponentA);
Expand All @@ -569,84 +569,68 @@ mod tests {
)
}

#[test]
fn event_scene() {
let mut app = setup();

// Build scene.
let scene = build_scene(&mut app);

// Spawn scene.
let scene_id =
app.world_mut()
.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();
fn build_dynamic_scene(app: &mut App) -> Handle<DynamicScene> {
app.world_mut()
.run_system_once(|world: &World, asset_server: Res<'_, AssetServer>| {
asset_server.add(DynamicScene::from_world(world))
})
}

let event = events.next().expect("found no `SceneInstanceReady` event");
fn observe_trigger(app: &mut App, scene_id: InstanceId, scene_entity: Entity) {
// Add observer
app.world_mut().observe(
move |trigger: Trigger<SceneInstanceReady>,
scene_spawner: Res<SceneSpawner>,
mut trigger_count: ResMut<TriggerCount>| {
assert_eq!(
event.id, scene_id,
trigger.event().instance_id,
scene_id,
"`SceneInstanceReady` contains the wrong `InstanceId`"
);

assert!(events.next().is_none(), "found more than one event");
assert_eq!(
trigger.entity(),
scene_entity,
"`SceneInstanceReady` triggered on the wrong parent entity"
);
assert!(
scene_spawner.instance_is_ready(trigger.event().instance_id),
"`InstanceId` is not ready"
);
trigger_count.0 += 1;
},
);

// Check observer is triggered once.
app.update();
app.world_mut()
.run_system_once(|trigger_count: Res<TriggerCount>| {
assert_eq!(
trigger_count.0, 1,
"wrong number of `SceneInstanceReady` triggers"
);
});
}

#[test]
fn event_scene_as_child() {
fn observe_scene() {
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");
},
);
}
// Spawn scene.
let scene_id =
app.world_mut()
.run_system_once(move |mut scene_spawner: ResMut<'_, SceneSpawner>| {
scene_spawner.spawn(scene.clone())
});

fn build_dynamic_scene(app: &mut App) -> Handle<DynamicScene> {
app.world_mut()
.run_system_once(|world: &World, asset_server: Res<'_, AssetServer>| {
asset_server.add(DynamicScene::from_world(world))
})
// Check trigger.
observe_trigger(&mut app, scene_id, Entity::PLACEHOLDER);
}

#[test]
fn event_dynamic_scene() {
fn observe_dynamic_scene() {
let mut app = setup();

// Build scene.
Expand All @@ -659,25 +643,32 @@ mod tests {
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();
// Check trigger.
observe_trigger(&mut app, scene_id, Entity::PLACEHOLDER);
}

let event = events.next().expect("found no `SceneInstanceReady` event");
assert_eq!(
event.id, scene_id,
"`SceneInstanceReady` contains the wrong `InstanceId`"
);
#[test]
fn observe_scene_as_child() {
let mut app = setup();

// Build scene.
let scene = build_scene(&mut app);

assert!(events.next().is_none(), "found more than one event");
// 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 trigger.
observe_trigger(&mut app, scene_id, scene_entity);
}

#[test]
fn event_dynamic_scene_as_child() {
fn observe_dynamic_scene_as_child() {
let mut app = setup();

// Build scene.
Expand All @@ -692,26 +683,8 @@ mod tests {
},
);

// 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");
},
);
// Check trigger.
observe_trigger(&mut app, scene_id, scene_entity);
}

#[test]
Expand Down

0 comments on commit 3d1c9ca

Please sign in to comment.