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

Make SceneSpawner::spawn_dynamic return the spawned scene's InstanceId #6661

Closed
Carnagion opened this issue Nov 17, 2022 · 0 comments
Closed
Labels
A-Scenes Serialized ECS data stored on the disk C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@Carnagion
Copy link

Carnagion commented Nov 17, 2022

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

Currently, SceneSpawner::spawn_dynamic returns unit (()) rather than the spawned scene's InstanceId.

It is difficult to work with any DynamicScene that is spawned this way, because the lack of an InstanceId makes it impossible to do things such as iterating over the scene's entities using SceneSpawner::iter_instance_entities or checking if it's ready using SceneSpawner::is_instance_ready:

scene_spawner.spawn_dynamic(dynamic_scene_handle); // no instance id returned
// cannot use with SceneSpawner::iter_instance_entities or SceneSpawner::is_instance_ready

It also makes the API inconsistent, since every other method in SceneSpawner returns an InstanceId in some form, including other DynamicScene-spawning methods such as SceneSpawner::spawn_dynamic_as_child:

// this works
let id = scene_spawner.spawn_dynamic_as_child(dynamic_scene_handle);
for entity in scene_spawner.iter_instance_entities(id) {
}

// so does this - the Scene counterpart to SceneSpawner::spawn_dynamic
let id = scene_spawner.spawn(scene_handle);
for entity in scene_spawner.iter_instance_entities(id) {
}

What solution would you like?

Change spawn_dynamic's return type to InstanceId and return the spawned scene's InstanceId:

let id = scene_spawner.spawn_dynamic(dynamic_scene_handle);
for entity in scene_spawner.iter_instance_entities(id) {
}

What alternative(s) have you considered?

A possible workaround would be to go through multiple steps to transform a Handle<DynamicScene> to a Handle<Scene>, with the help of Res<Assets<DynamicScene>>, Scene::from_dynamic_scene, and ResMut<Assets<Scene>>.

This adds unnecessary complexity and is not very elegant, but is unfortunately the only solution for those who do not want to spawn the DynamicScene as a child of another entity, and want to do something with the scene's InstanceId.

@Carnagion Carnagion added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Nov 17, 2022
@mockersf mockersf added A-Scenes Serialized ECS data stored on the disk and removed S-Needs-Triage This issue needs to be labelled labels Nov 17, 2022
@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Feature A new feature, making something new possible labels Nov 17, 2022
@bors bors bot closed this as completed in 4209fca Nov 18, 2022
taiyoungjang pushed a commit to taiyoungjang/bevy that referenced this issue Dec 15, 2022
# Objective

Fixes bevyengine#6661 

## Solution

Make `SceneSpawner::spawn_dynamic` return `InstanceId` like other functions there.

---

## Changelog

Make `SceneSpawner::spawn_dynamic` return `InstanceId` instead of `()`.
alradish pushed a commit to alradish/bevy that referenced this issue Jan 22, 2023
# Objective

Fixes bevyengine#6661 

## Solution

Make `SceneSpawner::spawn_dynamic` return `InstanceId` like other functions there.

---

## Changelog

Make `SceneSpawner::spawn_dynamic` return `InstanceId` instead of `()`.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

Fixes bevyengine#6661 

## Solution

Make `SceneSpawner::spawn_dynamic` return `InstanceId` like other functions there.

---

## Changelog

Make `SceneSpawner::spawn_dynamic` return `InstanceId` instead of `()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Scenes Serialized ECS data stored on the disk C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants