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

Emit an event when a scene has finished being spawned #2218

Closed
szunami opened this issue May 19, 2021 · 4 comments · Fixed by #11741
Closed

Emit an event when a scene has finished being spawned #2218

szunami opened this issue May 19, 2021 · 4 comments · Fixed by #11741
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@szunami
Copy link
Contributor

szunami commented May 19, 2021

What problem does this solve or what need does it fill?

Spawning a scene happens asynchronously and there isn't a pattern I know of to perform a callback when a scene finishes spawning. In my case, my scene contains a bunch of entities with Ground components; when the scene is done spawning, I want to add SpriteBundles to them.

What solution would you like?

Emit an event when a scene has finished spawning, roughly here: https://github.com/bevyengine/bevy/blob/main/crates/bevy_scene/src/scene_spawner.rs#L242

It is desirable that this event includes the InstanceId

What alternative(s) have you considered?

I currently have a workaround where I use a Query<&Ground, Without<Sprite>> and insert sprites whenever the scene finishes loading. This is no-op after the first time.

Additional context

I have talked this over with @TheRawMeatball & @alice-i-cecile in discord a bit.

@szunami szunami added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels May 19, 2021
@alice-i-cecile alice-i-cecile added A-Assets Load files from disk to use for things like images, models, and sounds C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels May 19, 2021
@cart
Copy link
Member

cart commented Feb 8, 2022

Before adding this, I think its worth discussing alternatives. As it stands, this encourages the following pattern:

struct SpawningPlayers(HashMap<InstanceId, Entity>)

fn spawn_players(mut commands: Commands, asset_server: Res<AssetServer>, mut scene_spawner: ResMut<SpawnScene>, mut spawning_players: ResMut<SpawningPlayers>) {
    let scene = asset_server.load("player_scene.gltf");
    let player = commands.spawn_bundle(PlayerBundle::default()).id();
    let instance = scene_spawner.spawn_as_child(scene, player);
    spawning_players.0.insert(instance, player);
}

fn do_thing_on_spawn(mut events: EventReader<SceneSpawn>, players: Query<&Player>, spawning_players: ResMut<SpawningPlayers>) {
    // must handle every scene spawn event, just to process "player scene spawn" events
    for event in events.iter() {
        if let Some(entity) = spawning_players.0.remove(event.instance_id) {
          // do "spawned player" action
        }
    }
}

This is suboptimal for a number of reasons:

  • Large amounts of boilerplate for what will probably be a very common action
  • Inefficient: we need to iterate over every scene spawn event and filter down to "player spawn events". We also need to allocate a list of "scene spawn events". For an app with thousands (or hundreds of thousands) of spawned scenes, this is non-trivial memory / cpu overhead.

I think we should consider other patterns instead:

fn spawn_players(mut commands: Commands, asset_server: Res<AssetServer>) {
    let scene = asset_server.load("player_scene.gltf");
    commands
        .spawn_scene(scene)
        .insert_bundle(PlayerBundle::default());
}

fn do_thing_on_spawn(new_players: Query<&Player, Added<SceneRoot>>) {
    for new_players in new_players.iter() {
          // do "spawned player" action
    }
}

This is significantly simpler, and it could also be significantly more efficient, because we have filtered down to the "player scope". Note that I say "could" because we need to iterate every player to check for the Added component state. For a large number of players (and a small number of scene instances) this could end up being worse.

Alternatively, we could do "typed scene spawn events" to help filter down the "scene spawn event scope" (ex: EventReader<SpawnedScene<Player>>), but that seems like its own flavor of "complex".

I'm not pushing any particular solution at this point, but I think we should have more discussion about what our ideal implementation looks like, because I don't think the UX in #2425 is ideal yet.

@szunami
Copy link
Contributor Author

szunami commented Feb 8, 2022

I appreciate the feedback, thanks @cart . I'll give this some more thought.

@marko-lazic
Copy link

Here is my use.

I am using blender_bevy_toolkit for level editing. My project has colonies, portal gates and portal nodes to whom position player can teleport. The toolkit parses a colony blender scene into .scn file which can be later spawned by SceneSpawner. Because scripting each bevy component into the toolkit is slow I just added one component representing portal gate and one representing portal node and then I add their other components such as rigid body programmatically.

Here is the problem.

For player to spawn to world, portal node component has to exist, so that it can know spawn position.
For player to leave colony a portal gate component has to have rigid body component so that it can detect player crossing that gate.
Queries are empty, probably because they got called before scene finished spawning.

It feels unnatural to have systems listen for changes. But firing events would not help much either. The best would be if I had an editor to add all components and save it into .scn from the beginning.

That are my thoughts for now.

@daxpedda
Copy link
Contributor

daxpedda commented Dec 16, 2023

I believe that this was already covered by SceneInstanceReady in #9313, but instead of including the InstanceId it includes the parent Entity.

It also emits an event for each entity it inserts, which might have been unintentional.

EDIT: SceneInstanceReady only gets emitted when spawning a scene into an entity.

github-merge-queue bot pushed a commit that referenced this issue Jul 6, 2024
# 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<Entity> }`.
github-merge-queue bot pushed a commit that referenced this issue Jul 30, 2024
# 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>| {```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
Status: Assets-wide Usability
Development

Successfully merging a pull request may close this issue.

5 participants