Skip to content

Commit 1f61c26

Browse files
authored
Align Scene::write_to_world_with to match DynamicScene::write_to_world_with (#13714)
# Objective `Scene` and `DynamicScene` work with `InstanceInfo` at different levels of abstraction - `Scene::write_to_world_with` returns an `InstanceInfo` whereas `DynamicScene::write_to_world_with` returns `()`. Instances are created one level higher at the `SceneSpawner` API level. - `DynamicScene::write_to_world_with` takes the `entity_map` as an argument whereas the `Scene` version is less flexible and creates a new one for you. No reason this needs to be the case. ## Solution I propose changing `Scene::write_to_world_with` to match the API we have for `DynamicScene`. Returning the `InstanceInfo` as we do today just seems like a leaky abstraction - it's only used in `spawn_sync_internal`. Being able to pass in an entity_map gives you more flexibility with how you write entities to a world. This also moves `InstanceInfo` out of `Scene` which is cleaner conceptually. If someone wants to work with instances then they should work with `SceneSpawner` - I see `write_to_world_with` as a lower-level API to be used with exclusive world access. ## Testing Code is just shifting things around. ## Changelog Changed `Scene::write_to_world_with` to take `entity_map` as an argument and no longer return an `InstanceInfo` ## Migration Guide `Scene::write_to_world_with` no longer returns an `InstanceInfo`. Before ```rust scene.write_to_world_with(world, &registry) ``` After ```rust let mut entity_map = EntityHashMap::default(); scene.write_to_world_with(world, &mut entity_map, &registry) ```
1 parent 33dff0d commit 1f61c26

File tree

2 files changed

+30
-24
lines changed

2 files changed

+30
-24
lines changed

crates/bevy_scene/src/scene.rs

+10-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
use crate::{DynamicScene, InstanceInfo, SceneSpawnError};
1+
use crate::{DynamicScene, SceneSpawnError};
22
use bevy_asset::Asset;
3-
use bevy_ecs::entity::EntityHashMap;
3+
use bevy_ecs::entity::{Entity, EntityHashMap};
44
use bevy_ecs::{
55
reflect::{AppTypeRegistry, ReflectComponent, ReflectMapEntities, ReflectResource},
66
world::World,
@@ -43,7 +43,8 @@ impl Scene {
4343
/// provided [`AppTypeRegistry`] or doesn't reflect the [`Component`](bevy_ecs::component::Component) trait.
4444
pub fn clone_with(&self, type_registry: &AppTypeRegistry) -> Result<Scene, SceneSpawnError> {
4545
let mut new_world = World::new();
46-
self.write_to_world_with(&mut new_world, type_registry)?;
46+
let mut entity_map = EntityHashMap::default();
47+
self.write_to_world_with(&mut new_world, &mut entity_map, type_registry)?;
4748
Ok(Self { world: new_world })
4849
}
4950

@@ -54,12 +55,9 @@ impl Scene {
5455
pub fn write_to_world_with(
5556
&self,
5657
world: &mut World,
58+
entity_map: &mut EntityHashMap<Entity>,
5759
type_registry: &AppTypeRegistry,
58-
) -> Result<InstanceInfo, SceneSpawnError> {
59-
let mut instance_info = InstanceInfo {
60-
entity_map: EntityHashMap::default(),
61-
};
62-
60+
) -> Result<(), SceneSpawnError> {
6361
let type_registry = type_registry.read();
6462

6563
// Resources archetype
@@ -94,8 +92,7 @@ impl Scene {
9492

9593
for archetype in self.world.archetypes().iter() {
9694
for scene_entity in archetype.entities() {
97-
let entity = *instance_info
98-
.entity_map
95+
let entity = entity_map
9996
.entry(scene_entity.id())
10097
.or_insert_with(|| world.spawn_empty().id());
10198
for component_id in archetype.components() {
@@ -121,7 +118,7 @@ impl Scene {
121118
&self.world,
122119
world,
123120
scene_entity.id(),
124-
entity,
121+
*entity,
125122
&type_registry,
126123
);
127124
}
@@ -130,10 +127,10 @@ impl Scene {
130127

131128
for registration in type_registry.iter() {
132129
if let Some(map_entities_reflect) = registration.data::<ReflectMapEntities>() {
133-
map_entities_reflect.map_all_entities(world, &mut instance_info.entity_map);
130+
map_entities_reflect.map_all_entities(world, entity_map);
134131
}
135132
}
136133

137-
Ok(instance_info)
134+
Ok(())
138135
}
139136
}

crates/bevy_scene/src/scene_spawner.rs

+20-11
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ impl SceneSpawner {
221221
let scene = scenes
222222
.get(id)
223223
.ok_or(SceneSpawnError::NonExistentScene { id })?;
224+
224225
scene.write_to_world(world, entity_map)
225226
})
226227
}
@@ -229,27 +230,33 @@ impl SceneSpawner {
229230
pub fn spawn_sync(
230231
&mut self,
231232
world: &mut World,
232-
id: AssetId<Scene>,
233+
id: impl Into<AssetId<Scene>>,
233234
) -> Result<InstanceId, SceneSpawnError> {
234-
self.spawn_sync_internal(world, id, InstanceId::new())
235+
let mut entity_map = EntityHashMap::default();
236+
let id = id.into();
237+
Self::spawn_sync_internal(world, id, &mut entity_map)?;
238+
let instance_id = InstanceId::new();
239+
self.spawned_instances
240+
.insert(instance_id, InstanceInfo { entity_map });
241+
Ok(instance_id)
235242
}
236243

237244
fn spawn_sync_internal(
238-
&mut self,
245+
// &mut self,
239246
world: &mut World,
240247
id: AssetId<Scene>,
241-
instance_id: InstanceId,
242-
) -> Result<InstanceId, SceneSpawnError> {
248+
entity_map: &mut EntityHashMap<Entity>,
249+
) -> Result<(), SceneSpawnError> {
243250
world.resource_scope(|world, scenes: Mut<Assets<Scene>>| {
244251
let scene = scenes
245252
.get(id)
246253
.ok_or(SceneSpawnError::NonExistentRealScene { id })?;
247254

248-
let instance_info =
249-
scene.write_to_world_with(world, &world.resource::<AppTypeRegistry>().clone())?;
250-
251-
self.spawned_instances.insert(instance_id, instance_info);
252-
Ok(instance_id)
255+
scene.write_to_world_with(
256+
world,
257+
entity_map,
258+
&world.resource::<AppTypeRegistry>().clone(),
259+
)
253260
})
254261
}
255262

@@ -319,7 +326,9 @@ impl SceneSpawner {
319326
let scenes_to_spawn = std::mem::take(&mut self.scenes_to_spawn);
320327

321328
for (scene_handle, instance_id) in scenes_to_spawn {
322-
match self.spawn_sync_internal(world, scene_handle.id(), instance_id) {
329+
let mut entity_map = EntityHashMap::default();
330+
331+
match Self::spawn_sync_internal(world, scene_handle.id(), &mut entity_map) {
323332
Ok(_) => {}
324333
Err(SceneSpawnError::NonExistentRealScene { .. }) => {
325334
self.scenes_to_spawn.push((scene_handle, instance_id));

0 commit comments

Comments
 (0)