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)
```
This commit is contained in:
Dmytro Banin 2024-06-10 05:39:35 -07:00 committed by GitHub
parent 33dff0d3f7
commit 1f61c26d2e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 30 additions and 24 deletions

View File

@ -1,6 +1,6 @@
use crate::{DynamicScene, InstanceInfo, SceneSpawnError}; use crate::{DynamicScene, SceneSpawnError};
use bevy_asset::Asset; use bevy_asset::Asset;
use bevy_ecs::entity::EntityHashMap; use bevy_ecs::entity::{Entity, EntityHashMap};
use bevy_ecs::{ use bevy_ecs::{
reflect::{AppTypeRegistry, ReflectComponent, ReflectMapEntities, ReflectResource}, reflect::{AppTypeRegistry, ReflectComponent, ReflectMapEntities, ReflectResource},
world::World, world::World,
@ -43,7 +43,8 @@ impl Scene {
/// provided [`AppTypeRegistry`] or doesn't reflect the [`Component`](bevy_ecs::component::Component) trait. /// provided [`AppTypeRegistry`] or doesn't reflect the [`Component`](bevy_ecs::component::Component) trait.
pub fn clone_with(&self, type_registry: &AppTypeRegistry) -> Result<Scene, SceneSpawnError> { pub fn clone_with(&self, type_registry: &AppTypeRegistry) -> Result<Scene, SceneSpawnError> {
let mut new_world = World::new(); let mut new_world = World::new();
self.write_to_world_with(&mut new_world, type_registry)?; let mut entity_map = EntityHashMap::default();
self.write_to_world_with(&mut new_world, &mut entity_map, type_registry)?;
Ok(Self { world: new_world }) Ok(Self { world: new_world })
} }
@ -54,12 +55,9 @@ impl Scene {
pub fn write_to_world_with( pub fn write_to_world_with(
&self, &self,
world: &mut World, world: &mut World,
entity_map: &mut EntityHashMap<Entity>,
type_registry: &AppTypeRegistry, type_registry: &AppTypeRegistry,
) -> Result<InstanceInfo, SceneSpawnError> { ) -> Result<(), SceneSpawnError> {
let mut instance_info = InstanceInfo {
entity_map: EntityHashMap::default(),
};
let type_registry = type_registry.read(); let type_registry = type_registry.read();
// Resources archetype // Resources archetype
@ -94,8 +92,7 @@ impl Scene {
for archetype in self.world.archetypes().iter() { for archetype in self.world.archetypes().iter() {
for scene_entity in archetype.entities() { for scene_entity in archetype.entities() {
let entity = *instance_info let entity = entity_map
.entity_map
.entry(scene_entity.id()) .entry(scene_entity.id())
.or_insert_with(|| world.spawn_empty().id()); .or_insert_with(|| world.spawn_empty().id());
for component_id in archetype.components() { for component_id in archetype.components() {
@ -121,7 +118,7 @@ impl Scene {
&self.world, &self.world,
world, world,
scene_entity.id(), scene_entity.id(),
entity, *entity,
&type_registry, &type_registry,
); );
} }
@ -130,10 +127,10 @@ impl Scene {
for registration in type_registry.iter() { for registration in type_registry.iter() {
if let Some(map_entities_reflect) = registration.data::<ReflectMapEntities>() { if let Some(map_entities_reflect) = registration.data::<ReflectMapEntities>() {
map_entities_reflect.map_all_entities(world, &mut instance_info.entity_map); map_entities_reflect.map_all_entities(world, entity_map);
} }
} }
Ok(instance_info) Ok(())
} }
} }

View File

@ -221,6 +221,7 @@ impl SceneSpawner {
let scene = scenes let scene = scenes
.get(id) .get(id)
.ok_or(SceneSpawnError::NonExistentScene { id })?; .ok_or(SceneSpawnError::NonExistentScene { id })?;
scene.write_to_world(world, entity_map) scene.write_to_world(world, entity_map)
}) })
} }
@ -229,27 +230,33 @@ impl SceneSpawner {
pub fn spawn_sync( pub fn spawn_sync(
&mut self, &mut self,
world: &mut World, world: &mut World,
id: AssetId<Scene>, id: impl Into<AssetId<Scene>>,
) -> Result<InstanceId, SceneSpawnError> { ) -> Result<InstanceId, SceneSpawnError> {
self.spawn_sync_internal(world, id, InstanceId::new()) let mut entity_map = EntityHashMap::default();
let id = id.into();
Self::spawn_sync_internal(world, id, &mut entity_map)?;
let instance_id = InstanceId::new();
self.spawned_instances
.insert(instance_id, InstanceInfo { entity_map });
Ok(instance_id)
} }
fn spawn_sync_internal( fn spawn_sync_internal(
&mut self, // &mut self,
world: &mut World, world: &mut World,
id: AssetId<Scene>, id: AssetId<Scene>,
instance_id: InstanceId, entity_map: &mut EntityHashMap<Entity>,
) -> Result<InstanceId, SceneSpawnError> { ) -> Result<(), SceneSpawnError> {
world.resource_scope(|world, scenes: Mut<Assets<Scene>>| { world.resource_scope(|world, scenes: Mut<Assets<Scene>>| {
let scene = scenes let scene = scenes
.get(id) .get(id)
.ok_or(SceneSpawnError::NonExistentRealScene { id })?; .ok_or(SceneSpawnError::NonExistentRealScene { id })?;
let instance_info = scene.write_to_world_with(
scene.write_to_world_with(world, &world.resource::<AppTypeRegistry>().clone())?; world,
entity_map,
self.spawned_instances.insert(instance_id, instance_info); &world.resource::<AppTypeRegistry>().clone(),
Ok(instance_id) )
}) })
} }
@ -319,7 +326,9 @@ impl SceneSpawner {
let scenes_to_spawn = std::mem::take(&mut self.scenes_to_spawn); 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) in scenes_to_spawn {
match self.spawn_sync_internal(world, scene_handle.id(), instance_id) { let mut entity_map = EntityHashMap::default();
match Self::spawn_sync_internal(world, scene_handle.id(), &mut entity_map) {
Ok(_) => {} Ok(_) => {}
Err(SceneSpawnError::NonExistentRealScene { .. }) => { Err(SceneSpawnError::NonExistentRealScene { .. }) => {
self.scenes_to_spawn.push((scene_handle, instance_id)); self.scenes_to_spawn.push((scene_handle, instance_id));