Make scene handling of entity references robust (#7335)

# Objective

- Handle dangling entity references inside scenes
- Handle references to entities with generation > 0 inside scenes
- Fix a latent bug in `Parent`'s `MapEntities` implementation, which
would, if the parent was outside the scene, cause the scene to be loaded
into the new world with a parent reference potentially pointing to some
random entity in that new world.
- Fixes #4793 and addresses #7235 

## Solution

- DynamicScenes now identify entities with a `Entity` instead of a u32,
therefore including generation
- `World` exposes a new `reserve_generations` function that despawns an
entity and advances its generation by some extra amount.
- `MapEntities` implementations have a new `get_or_reserve` function
available that will always return an `Entity`, establishing a new
mapping to a dead entity when the entity they are called with is not in
the `EntityMap`. Subsequent calls with that same `Entity` will return
the same newly created dead entity reference, preserving equality
semantics.
- As a result, after loading a scene containing references to dead
entities (or entities otherwise outside the scene), those references
will all point to different generations on a single entity id in the new
world.

---

## Changelog

### Changed
- In serialized scenes, entities are now identified by a u64 instead of
a u32.
- In serialized scenes, components with entity references now have those
references serialize as u64s instead of structs.
### Fixed
- Scenes containing components with entity references will now
deserialize and add to a world reliably.

## Migration Guide

- `MapEntities` implementations must change from a `&EntityMap`
parameter to a `&mut EntityMapper` parameter and can no longer return a
`Result`. Finally, they should switch from calling `EntityMap::get` to
calling `EntityMapper::get_or_reserve`.

---------

Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
This commit is contained in:
Illiux 2023-05-01 08:49:27 -07:00 committed by GitHub
parent ba532e4a37
commit eebc92a7d4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 360 additions and 125 deletions

View File

@ -26,7 +26,7 @@ thread_local = "1.1.4"
fixedbitset = "0.4.2"
rustc-hash = "1.1"
downcast-rs = "1.2"
serde = { version = "1", features = ["derive"] }
serde = "1"
thiserror = "1.0"
[dev-dependencies]

View File

@ -1,24 +1,5 @@
use crate::entity::Entity;
use crate::{entity::Entity, world::World};
use bevy_utils::{Entry, HashMap};
use std::fmt;
/// The errors that might be returned while using [`MapEntities::map_entities`].
#[derive(Debug)]
pub enum MapEntitiesError {
EntityNotFound(Entity),
}
impl std::error::Error for MapEntitiesError {}
impl fmt::Display for MapEntitiesError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
MapEntitiesError::EntityNotFound(_) => {
write!(f, "the given entity does not exist in the map")
}
}
}
}
/// Operation to map all contained [`Entity`] fields in a type to new values.
///
@ -33,7 +14,7 @@ impl fmt::Display for MapEntitiesError {
///
/// ```rust
/// use bevy_ecs::prelude::*;
/// use bevy_ecs::entity::{EntityMap, MapEntities, MapEntitiesError};
/// use bevy_ecs::entity::{EntityMapper, MapEntities};
///
/// #[derive(Component)]
/// struct Spring {
@ -42,10 +23,9 @@ impl fmt::Display for MapEntitiesError {
/// }
///
/// impl MapEntities for Spring {
/// fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError> {
/// self.a = entity_map.get(self.a)?;
/// self.b = entity_map.get(self.b)?;
/// Ok(())
/// fn map_entities(&mut self, entity_mapper: &mut EntityMapper) {
/// self.a = entity_mapper.get_or_reserve(self.a);
/// self.b = entity_mapper.get_or_reserve(self.b);
/// }
/// }
/// ```
@ -55,16 +35,22 @@ pub trait MapEntities {
/// Updates all [`Entity`] references stored inside using `entity_map`.
///
/// Implementors should look up any and all [`Entity`] values stored within and
/// update them to the mapped values via `entity_map`.
fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError>;
/// update them to the mapped values via `entity_mapper`.
fn map_entities(&mut self, entity_mapper: &mut EntityMapper);
}
/// A mapping from one set of entities to another.
///
/// The API generally follows [`HashMap`], but each [`Entity`] is returned by value, as they are [`Copy`].
///
/// This is typically used to coordinate data transfer between sets of entities, such as between a scene and the world or over the network.
/// This is required as [`Entity`] identifiers are opaque; you cannot and do not want to reuse identifiers directly.
/// This is typically used to coordinate data transfer between sets of entities, such as between a scene and the world
/// or over the network. This is required as [`Entity`] identifiers are opaque; you cannot and do not want to reuse
/// identifiers directly.
///
/// On its own, an `EntityMap` is not capable of allocating new entity identifiers, which is needed to map references
/// to entities that lie outside the source entity set. To do this, an `EntityMap` can be wrapped in an
/// [`EntityMapper`] which scopes it to a particular destination [`World`] and allows new identifiers to be allocated.
/// This functionality can be accessed through [`Self::world_scope()`].
#[derive(Default, Debug)]
pub struct EntityMap {
map: HashMap<Entity, Entity>,
@ -91,11 +77,8 @@ impl EntityMap {
}
/// Returns the corresponding mapped entity.
pub fn get(&self, entity: Entity) -> Result<Entity, MapEntitiesError> {
self.map
.get(&entity)
.cloned()
.ok_or(MapEntitiesError::EntityNotFound(entity))
pub fn get(&self, entity: Entity) -> Option<Entity> {
self.map.get(&entity).copied()
}
/// An iterator visiting all keys in arbitrary order.
@ -122,4 +105,138 @@ impl EntityMap {
pub fn iter(&self) -> impl Iterator<Item = (Entity, Entity)> + '_ {
self.map.iter().map(|(from, to)| (*from, *to))
}
/// Creates an [`EntityMapper`] from this [`EntityMap`] and scoped to the provided [`World`], then calls the
/// provided function with it. This allows one to allocate new entity references in the provided `World` that are
/// guaranteed to never point at a living entity now or in the future. This functionality is useful for safely
/// mapping entity identifiers that point at entities outside the source world. The passed function, `f`, is called
/// within the scope of the passed world. Its return value is then returned from `world_scope` as the generic type
/// parameter `R`.
pub fn world_scope<R>(
&mut self,
world: &mut World,
f: impl FnOnce(&mut World, &mut EntityMapper) -> R,
) -> R {
let mut mapper = EntityMapper::new(self, world);
let result = f(world, &mut mapper);
mapper.finish(world);
result
}
}
/// A wrapper for [`EntityMap`], augmenting it with the ability to allocate new [`Entity`] references in a destination
/// world. These newly allocated references are guaranteed to never point to any living entity in that world.
///
/// References are allocated by returning increasing generations starting from an internally initialized base
/// [`Entity`]. After it is finished being used by [`MapEntities`] implementations, this entity is despawned and the
/// requisite number of generations reserved.
pub struct EntityMapper<'m> {
/// The wrapped [`EntityMap`].
map: &'m mut EntityMap,
/// A base [`Entity`] used to allocate new references.
dead_start: Entity,
/// The number of generations this mapper has allocated thus far.
generations: u32,
}
impl<'m> EntityMapper<'m> {
/// Returns the corresponding mapped entity or reserves a new dead entity ID if it is absent.
pub fn get_or_reserve(&mut self, entity: Entity) -> Entity {
if let Some(mapped) = self.map.get(entity) {
return mapped;
}
// this new entity reference is specifically designed to never represent any living entity
let new = Entity {
generation: self.dead_start.generation + self.generations,
index: self.dead_start.index,
};
self.generations += 1;
self.map.insert(entity, new);
new
}
/// Gets a reference to the underlying [`EntityMap`].
pub fn get_map(&'m self) -> &'m EntityMap {
self.map
}
/// Gets a mutable reference to the underlying [`EntityMap`]
pub fn get_map_mut(&'m mut self) -> &'m mut EntityMap {
self.map
}
/// Creates a new [`EntityMapper`], spawning a temporary base [`Entity`] in the provided [`World`]
fn new(map: &'m mut EntityMap, world: &mut World) -> Self {
Self {
map,
// SAFETY: Entities data is kept in a valid state via `EntityMap::world_scope`
dead_start: unsafe { world.entities_mut().alloc() },
generations: 0,
}
}
/// Reserves the allocated references to dead entities within the world. This frees the temporary base
/// [`Entity`] while reserving extra generations via [`crate::entity::Entities::reserve_generations`]. Because this
/// renders the [`EntityMapper`] unable to safely allocate any more references, this method takes ownership of
/// `self` in order to render it unusable.
fn finish(self, world: &mut World) {
// SAFETY: Entities data is kept in a valid state via `EntityMap::world_scope`
let entities = unsafe { world.entities_mut() };
assert!(entities.free(self.dead_start).is_some());
assert!(entities.reserve_generations(self.dead_start.index, self.generations));
}
}
#[cfg(test)]
mod tests {
use super::{EntityMap, EntityMapper};
use crate::{entity::Entity, world::World};
#[test]
fn entity_mapper() {
const FIRST_IDX: u32 = 1;
const SECOND_IDX: u32 = 2;
let mut map = EntityMap::default();
let mut world = World::new();
let mut mapper = EntityMapper::new(&mut map, &mut world);
let mapped_ent = Entity::new(FIRST_IDX, 0);
let dead_ref = mapper.get_or_reserve(mapped_ent);
assert_eq!(
dead_ref,
mapper.get_or_reserve(mapped_ent),
"should persist the allocated mapping from the previous line"
);
assert_eq!(
mapper.get_or_reserve(Entity::new(SECOND_IDX, 0)).index(),
dead_ref.index(),
"should re-use the same index for further dead refs"
);
mapper.finish(&mut world);
// Next allocated entity should be a further generation on the same index
let entity = world.spawn_empty().id();
assert_eq!(entity.index(), dead_ref.index());
assert!(entity.generation() > dead_ref.generation());
}
#[test]
fn world_scope_reserves_generations() {
let mut map = EntityMap::default();
let mut world = World::new();
let dead_ref = map.world_scope(&mut world, |_, mapper| {
mapper.get_or_reserve(Entity::new(0, 0))
});
// Next allocated entity should be a further generation on the same index
let entity = world.spawn_empty().id();
assert_eq!(entity.index(), dead_ref.index());
assert!(entity.generation() > dead_ref.generation());
}
}

View File

@ -115,7 +115,7 @@ type IdCursor = isize;
/// [`EntityCommands`]: crate::system::EntityCommands
/// [`Query::get`]: crate::system::Query::get
/// [`World`]: crate::world::World
#[derive(Clone, Copy, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
#[derive(Clone, Copy, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct Entity {
generation: u32,
index: u32,
@ -227,6 +227,25 @@ impl Entity {
}
}
impl Serialize for Entity {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
serializer.serialize_u64(self.to_bits())
}
}
impl<'de> Deserialize<'de> for Entity {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
let id: u64 = serde::de::Deserialize::deserialize(deserializer)?;
Ok(Entity::from_bits(id))
}
}
impl fmt::Debug for Entity {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}v{}", self.index, self.generation)
@ -604,6 +623,25 @@ impl Entities {
self.meta.get_unchecked_mut(index as usize).location = location;
}
/// Increments the `generation` of a freed [`Entity`]. The next entity ID allocated with this
/// `index` will count `generation` starting from the prior `generation` + the specified
/// value + 1.
///
/// Does nothing if no entity with this `index` has been allocated yet.
pub(crate) fn reserve_generations(&mut self, index: u32, generations: u32) -> bool {
if (index as usize) >= self.meta.len() {
return false;
}
let meta = &mut self.meta[index as usize];
if meta.location.archetype_id == ArchetypeId::INVALID {
meta.generation += generations;
true
} else {
false
}
}
/// Get the [`Entity`] with a given id, if it exists in this [`Entities`] collection
/// Returns `None` if this [`Entity`] is outside of the range of currently reserved Entities
///
@ -847,4 +885,29 @@ mod tests {
const C4: u32 = Entity::from_bits(0x00dd_00ff_0000_0000).generation();
assert_eq!(0x00dd_00ff, C4);
}
#[test]
fn reserve_generations() {
let mut entities = Entities::new();
let entity = entities.alloc();
entities.free(entity);
assert!(entities.reserve_generations(entity.index, 1));
}
#[test]
fn reserve_generations_and_alloc() {
const GENERATIONS: u32 = 10;
let mut entities = Entities::new();
let entity = entities.alloc();
entities.free(entity);
assert!(entities.reserve_generations(entity.index, GENERATIONS));
// The very next entitiy allocated should be a further generation on the same index
let next_entity = entities.alloc();
assert_eq!(next_entity.index(), entity.index());
assert!(next_entity.generation > entity.generation + GENERATIONS);
}
}

View File

@ -3,7 +3,7 @@
use crate::{
change_detection::Mut,
component::Component,
entity::{Entity, EntityMap, MapEntities, MapEntitiesError},
entity::{Entity, EntityMap, EntityMapper, MapEntities},
system::Resource,
world::{
unsafe_world_cell::{UnsafeEntityCell, UnsafeWorldCell},
@ -412,8 +412,8 @@ impl_from_reflect_value!(Entity);
#[derive(Clone)]
pub struct ReflectMapEntities {
map_entities: fn(&mut World, &EntityMap) -> Result<(), MapEntitiesError>,
map_specific_entities: fn(&mut World, &EntityMap, &[Entity]) -> Result<(), MapEntitiesError>,
map_entities: fn(&mut World, &mut EntityMapper),
map_specific_entities: fn(&mut World, &mut EntityMapper, &[Entity]),
}
impl ReflectMapEntities {
@ -426,12 +426,8 @@ impl ReflectMapEntities {
/// An example of this: A scene can be loaded with `Parent` components, but then a `Parent` component can be added
/// to these entities after they have been loaded. If you reload the scene using [`map_entities`](Self::map_entities), those `Parent`
/// components with already valid entity references could be updated to point at something else entirely.
pub fn map_entities(
&self,
world: &mut World,
entity_map: &EntityMap,
) -> Result<(), MapEntitiesError> {
(self.map_entities)(world, entity_map)
pub fn map_entities(&self, world: &mut World, entity_map: &mut EntityMap) {
entity_map.world_scope(world, self.map_entities);
}
/// This is like [`map_entities`](Self::map_entities), but only applied to specific entities, not all values
@ -442,31 +438,32 @@ impl ReflectMapEntities {
pub fn map_specific_entities(
&self,
world: &mut World,
entity_map: &EntityMap,
entity_map: &mut EntityMap,
entities: &[Entity],
) -> Result<(), MapEntitiesError> {
(self.map_specific_entities)(world, entity_map, entities)
) {
entity_map.world_scope(world, |world, mapper| {
(self.map_specific_entities)(world, mapper, entities);
});
}
}
impl<C: Component + MapEntities> FromType<C> for ReflectMapEntities {
fn from_type() -> Self {
ReflectMapEntities {
map_specific_entities: |world, entity_map, entities| {
map_specific_entities: |world, entity_mapper, entities| {
for &entity in entities {
if let Some(mut component) = world.get_mut::<C>(entity) {
component.map_entities(entity_map)?;
component.map_entities(entity_mapper);
}
}
Ok(())
},
map_entities: |world, entity_map| {
for entity in entity_map.values() {
if let Some(mut component) = world.get_mut::<C>(entity) {
component.map_entities(entity_map)?;
map_entities: |world, entity_mapper| {
let entities = entity_mapper.get_map().values().collect::<Vec<Entity>>();
for entity in &entities {
if let Some(mut component) = world.get_mut::<C>(*entity) {
component.map_entities(entity_mapper);
}
}
Ok(())
},
}
}

View File

@ -1,6 +1,6 @@
use bevy_ecs::{
component::Component,
entity::{Entity, EntityMap, MapEntities, MapEntitiesError},
entity::{Entity, EntityMapper, MapEntities},
prelude::FromWorld,
reflect::{ReflectComponent, ReflectMapEntities},
world::World,
@ -21,12 +21,10 @@ use std::ops::Deref;
pub struct Children(pub(crate) SmallVec<[Entity; 8]>);
impl MapEntities for Children {
fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError> {
fn map_entities(&mut self, entity_mapper: &mut EntityMapper) {
for entity in &mut self.0 {
*entity = entity_map.get(*entity)?;
*entity = entity_mapper.get_or_reserve(*entity);
}
Ok(())
}
}

View File

@ -1,6 +1,6 @@
use bevy_ecs::{
component::Component,
entity::{Entity, EntityMap, MapEntities, MapEntitiesError},
entity::{Entity, EntityMapper, MapEntities},
reflect::{ReflectComponent, ReflectMapEntities},
world::{FromWorld, World},
};
@ -36,13 +36,8 @@ impl FromWorld for Parent {
}
impl MapEntities for Parent {
fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError> {
// Parent of an entity in the new world can be in outside world, in which case it
// should not be mapped.
if let Ok(mapped_entity) = entity_map.get(self.0) {
self.0 = mapped_entity;
}
Ok(())
fn map_entities(&mut self, entity_mapper: &mut EntityMapper) {
self.0 = entity_mapper.get_or_reserve(self.0);
}
}

View File

@ -1,7 +1,7 @@
use bevy_asset::Handle;
use bevy_ecs::{
component::Component,
entity::{Entity, EntityMap, MapEntities, MapEntitiesError},
entity::{Entity, EntityMapper, MapEntities},
prelude::ReflectComponent,
reflect::ReflectMapEntities,
};
@ -17,12 +17,10 @@ pub struct SkinnedMesh {
}
impl MapEntities for SkinnedMesh {
fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError> {
fn map_entities(&mut self, entity_mapper: &mut EntityMapper) {
for joint in &mut self.joints {
*joint = entity_map.get(*joint)?;
*joint = entity_mapper.get_or_reserve(*joint);
}
Ok(())
}
}

View File

@ -4,8 +4,7 @@ use crate::{DynamicSceneBuilder, Scene, SceneSpawnError};
use anyhow::Result;
use bevy_app::AppTypeRegistry;
use bevy_ecs::{
entity::EntityMap,
prelude::Entity,
entity::{Entity, EntityMap},
reflect::{ReflectComponent, ReflectMapEntities},
world::World,
};
@ -36,8 +35,10 @@ pub struct DynamicScene {
/// A reflection-powered serializable representation of an entity and its components.
pub struct DynamicEntity {
/// The transiently unique identifier of a corresponding [`Entity`](bevy_ecs::entity::Entity).
pub entity: u32,
/// The identifier of the entity, unique within a scene (and the world it may have been generated from).
///
/// Components that reference this entity must consistently use this identifier.
pub entity: Entity,
/// A vector of boxed components that belong to the given entity and
/// implement the [`Reflect`] trait.
pub components: Vec<Box<dyn Reflect>>,
@ -101,7 +102,7 @@ impl DynamicScene {
// or spawn a new entity with a transiently unique id if there is
// no corresponding entry.
let entity = *entity_map
.entry(bevy_ecs::entity::Entity::from_raw(scene_entity.entity))
.entry(scene_entity.entity)
.or_insert_with(|| world.spawn_empty().id());
let entity_mut = &mut world.entity_mut(entity);
@ -141,9 +142,7 @@ impl DynamicScene {
"we should be getting TypeId from this TypeRegistration in the first place",
);
if let Some(map_entities_reflect) = registration.data::<ReflectMapEntities>() {
map_entities_reflect
.map_specific_entities(world, entity_map, &entities)
.unwrap();
map_entities_reflect.map_specific_entities(world, entity_map, &entities);
}
}

View File

@ -38,7 +38,7 @@ use std::collections::BTreeMap;
/// ```
pub struct DynamicSceneBuilder<'w> {
extracted_resources: BTreeMap<ComponentId, Box<dyn Reflect>>,
extracted_scene: BTreeMap<u32, DynamicEntity>,
extracted_scene: BTreeMap<Entity, DynamicEntity>,
type_registry: AppTypeRegistry,
original_world: &'w World,
}
@ -123,19 +123,17 @@ impl<'w> DynamicSceneBuilder<'w> {
let type_registry = self.type_registry.read();
for entity in entities {
let index = entity.index();
if self.extracted_scene.contains_key(&index) {
if self.extracted_scene.contains_key(&entity) {
continue;
}
let mut entry = DynamicEntity {
entity: index,
entity,
components: Vec::new(),
};
let entity = self.original_world.entity(entity);
for component_id in entity.archetype().components() {
let original_entity = self.original_world.entity(entity);
for component_id in original_entity.archetype().components() {
let mut extract_and_push = || {
let type_id = self
.original_world
@ -145,13 +143,13 @@ impl<'w> DynamicSceneBuilder<'w> {
let component = type_registry
.get(type_id)?
.data::<ReflectComponent>()?
.reflect(entity)?;
.reflect(original_entity)?;
entry.components.push(component.clone_value());
Some(())
};
extract_and_push();
}
self.extracted_scene.insert(index, entry);
self.extracted_scene.insert(entity, entry);
}
drop(type_registry);
@ -243,7 +241,7 @@ mod tests {
let scene = builder.build();
assert_eq!(scene.entities.len(), 1);
assert_eq!(scene.entities[0].entity, entity.index());
assert_eq!(scene.entities[0].entity, entity);
assert_eq!(scene.entities[0].components.len(), 1);
assert!(scene.entities[0].components[0].represents::<ComponentA>());
}
@ -264,7 +262,7 @@ mod tests {
let scene = builder.build();
assert_eq!(scene.entities.len(), 1);
assert_eq!(scene.entities[0].entity, entity.index());
assert_eq!(scene.entities[0].entity, entity);
assert_eq!(scene.entities[0].components.len(), 1);
assert!(scene.entities[0].components[0].represents::<ComponentA>());
}
@ -288,7 +286,7 @@ mod tests {
let scene = builder.build();
assert_eq!(scene.entities.len(), 1);
assert_eq!(scene.entities[0].entity, entity.index());
assert_eq!(scene.entities[0].entity, entity);
assert_eq!(scene.entities[0].components.len(), 2);
assert!(scene.entities[0].components[0].represents::<ComponentA>());
assert!(scene.entities[0].components[1].represents::<ComponentB>());
@ -315,10 +313,10 @@ mod tests {
let mut entities = builder.build().entities.into_iter();
// Assert entities are ordered
assert_eq!(entity_a.index(), entities.next().map(|e| e.entity).unwrap());
assert_eq!(entity_b.index(), entities.next().map(|e| e.entity).unwrap());
assert_eq!(entity_c.index(), entities.next().map(|e| e.entity).unwrap());
assert_eq!(entity_d.index(), entities.next().map(|e| e.entity).unwrap());
assert_eq!(entity_a, entities.next().map(|e| e.entity).unwrap());
assert_eq!(entity_b, entities.next().map(|e| e.entity).unwrap());
assert_eq!(entity_c, entities.next().map(|e| e.entity).unwrap());
assert_eq!(entity_d, entities.next().map(|e| e.entity).unwrap());
}
#[test]
@ -345,7 +343,7 @@ mod tests {
assert_eq!(scene.entities.len(), 2);
let mut scene_entities = vec![scene.entities[0].entity, scene.entities[1].entity];
scene_entities.sort();
assert_eq!(scene_entities, [entity_a_b.index(), entity_a.index()]);
assert_eq!(scene_entities, [entity_a_b, entity_a]);
}
#[test]
@ -365,7 +363,7 @@ mod tests {
let scene = builder.build();
assert_eq!(scene.entities.len(), 1);
assert_eq!(scene.entities[0].entity, entity_a.index());
assert_eq!(scene.entities[0].entity, entity_a);
}
#[test]

View File

@ -120,9 +120,7 @@ impl Scene {
for registration in type_registry.iter() {
if let Some(map_entities_reflect) = registration.data::<ReflectMapEntities>() {
map_entities_reflect
.map_entities(world, &instance_info.entity_map)
.unwrap();
map_entities_reflect.map_entities(world, &mut instance_info.entity_map);
}
}

View File

@ -1,5 +1,6 @@
use crate::{DynamicEntity, DynamicScene};
use anyhow::Result;
use bevy_ecs::entity::Entity;
use bevy_reflect::serde::{TypedReflectDeserializer, TypedReflectSerializer};
use bevy_reflect::{
serde::{TypeRegistrationDeserializer, UntypedReflectDeserializer},
@ -260,9 +261,9 @@ impl<'a, 'de> Visitor<'de> for SceneEntitiesVisitor<'a> {
A: MapAccess<'de>,
{
let mut entities = Vec::new();
while let Some(id) = map.next_key::<u32>()? {
while let Some(entity) = map.next_key::<Entity>()? {
let entity = map.next_value_seed(SceneEntityDeserializer {
id,
entity,
type_registry: self.type_registry,
})?;
entities.push(entity);
@ -273,7 +274,7 @@ impl<'a, 'de> Visitor<'de> for SceneEntitiesVisitor<'a> {
}
pub struct SceneEntityDeserializer<'a> {
pub id: u32,
pub entity: Entity,
pub type_registry: &'a TypeRegistry,
}
@ -288,7 +289,7 @@ impl<'a, 'de> DeserializeSeed<'de> for SceneEntityDeserializer<'a> {
ENTITY_STRUCT,
&[ENTITY_FIELD_COMPONENTS],
SceneEntityVisitor {
id: self.id,
entity: self.entity,
registry: self.type_registry,
},
)
@ -296,7 +297,7 @@ impl<'a, 'de> DeserializeSeed<'de> for SceneEntityDeserializer<'a> {
}
struct SceneEntityVisitor<'a> {
pub id: u32,
pub entity: Entity,
pub registry: &'a TypeRegistry,
}
@ -318,7 +319,7 @@ impl<'a, 'de> Visitor<'de> for SceneEntityVisitor<'a> {
.ok_or_else(|| Error::missing_field(ENTITY_FIELD_COMPONENTS))?;
Ok(DynamicEntity {
entity: self.id,
entity: self.entity,
components,
})
}
@ -346,7 +347,7 @@ impl<'a, 'de> Visitor<'de> for SceneEntityVisitor<'a> {
.take()
.ok_or_else(|| Error::missing_field(ENTITY_FIELD_COMPONENTS))?;
Ok(DynamicEntity {
entity: self.id,
entity: self.entity,
components,
})
}
@ -424,8 +425,11 @@ mod tests {
use crate::serde::{SceneDeserializer, SceneSerializer};
use crate::{DynamicScene, DynamicSceneBuilder};
use bevy_app::AppTypeRegistry;
use bevy_ecs::entity::EntityMap;
use bevy_ecs::entity::{Entity, EntityMap, EntityMapper, MapEntities};
use bevy_ecs::prelude::{Component, ReflectComponent, ReflectResource, Resource, World};
use bevy_ecs::query::{With, Without};
use bevy_ecs::reflect::ReflectMapEntities;
use bevy_ecs::world::FromWorld;
use bevy_reflect::{FromReflect, Reflect, ReflectSerialize};
use bincode::Options;
use serde::de::DeserializeSeed;
@ -466,6 +470,22 @@ mod tests {
foo: i32,
}
#[derive(Clone, Component, Reflect, PartialEq)]
#[reflect(Component, MapEntities, PartialEq)]
struct MyEntityRef(Entity);
impl MapEntities for MyEntityRef {
fn map_entities(&mut self, entity_mapper: &mut EntityMapper) {
self.0 = entity_mapper.get_or_reserve(self.0);
}
}
impl FromWorld for MyEntityRef {
fn from_world(_world: &mut World) -> Self {
Self(Entity::PLACEHOLDER)
}
}
fn create_world() -> World {
let mut world = World::new();
let registry = AppTypeRegistry::default();
@ -480,6 +500,8 @@ mod tests {
registry.register_type_data::<String, ReflectSerialize>();
registry.register::<[usize; 3]>();
registry.register::<(f32, f32)>();
registry.register::<MyEntityRef>();
registry.register::<Entity>();
registry.register::<MyResource>();
}
world.insert_resource(registry);
@ -596,6 +618,57 @@ mod tests {
assert_eq!(1, dst_world.query::<&Baz>().iter(&dst_world).count());
}
#[test]
fn should_roundtrip_with_later_generations_and_obsolete_references() {
let mut world = create_world();
world.spawn_empty().despawn();
let a = world.spawn_empty().id();
let foo = world.spawn(MyEntityRef(a)).insert(Foo(123)).id();
world.despawn(a);
world.spawn(MyEntityRef(foo)).insert(Bar(123));
let registry = world.resource::<AppTypeRegistry>();
let scene = DynamicScene::from_world(&world, registry);
let serialized = scene
.serialize_ron(&world.resource::<AppTypeRegistry>().0)
.unwrap();
let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap();
let scene_deserializer = SceneDeserializer {
type_registry: &registry.0.read(),
};
let deserialized_scene = scene_deserializer.deserialize(&mut deserializer).unwrap();
let mut map = EntityMap::default();
let mut dst_world = create_world();
deserialized_scene
.write_to_world(&mut dst_world, &mut map)
.unwrap();
assert_eq!(2, deserialized_scene.entities.len());
assert_scene_eq(&scene, &deserialized_scene);
let bar_to_foo = dst_world
.query_filtered::<&MyEntityRef, Without<Foo>>()
.get_single(&dst_world)
.cloned()
.unwrap();
let foo = dst_world
.query_filtered::<Entity, With<Foo>>()
.get_single(&dst_world)
.unwrap();
assert_eq!(foo, bar_to_foo.0);
assert!(dst_world
.query_filtered::<&MyEntityRef, With<Foo>>()
.iter(&dst_world)
.all(|r| world.get_entity(r.0).is_none()));
}
#[test]
fn should_roundtrip_postcard() {
let mut world = create_world();
@ -696,12 +769,12 @@ mod tests {
assert_eq!(
vec![
0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0,
37, 0, 0, 0, 0, 0, 0, 0, 98, 101, 118, 121, 95, 115, 99, 101, 110, 101, 58, 58,
115, 101, 114, 100, 101, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77, 121, 67, 111,
109, 112, 111, 110, 101, 110, 116, 1, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0,
3, 0, 0, 0, 0, 0, 0, 0, 102, 102, 166, 63, 205, 204, 108, 64, 1, 0, 0, 0, 12, 0, 0,
0, 0, 0, 0, 0, 72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100, 33
0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0,
0, 0, 0, 0, 37, 0, 0, 0, 0, 0, 0, 0, 98, 101, 118, 121, 95, 115, 99, 101, 110, 101,
58, 58, 115, 101, 114, 100, 101, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77, 121,
67, 111, 109, 112, 111, 110, 101, 110, 116, 1, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0,
0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 102, 102, 166, 63, 205, 204, 108, 64, 1, 0, 0, 0,
12, 0, 0, 0, 0, 0, 0, 0, 72, 101, 108, 108, 111, 32, 87, 111, 114, 108, 100, 33
],
serialized_scene
);
@ -732,7 +805,7 @@ mod tests {
.entities
.iter()
.find(|dynamic_entity| dynamic_entity.entity == expected.entity)
.unwrap_or_else(|| panic!("missing entity (expected: `{}`)", expected.entity));
.unwrap_or_else(|| panic!("missing entity (expected: `{:?}`)", expected.entity));
assert_eq!(expected.entity, received.entity, "entities did not match",);

View File

@ -1,5 +1,5 @@
use bevy_ecs::{
entity::{Entity, EntityMap, MapEntities, MapEntitiesError},
entity::{Entity, EntityMapper, MapEntities},
prelude::{Component, ReflectComponent},
};
use bevy_math::{DVec2, IVec2, Vec2};
@ -55,14 +55,13 @@ impl WindowRef {
}
impl MapEntities for WindowRef {
fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError> {
fn map_entities(&mut self, entity_mapper: &mut EntityMapper) {
match self {
Self::Entity(entity) => {
*entity = entity_map.get(*entity)?;
Ok(())
*entity = entity_mapper.get_or_reserve(*entity);
}
Self::Primary => Ok(()),
}
Self::Primary => {}
};
}
}
@ -145,7 +144,7 @@ pub struct Window {
/// The "html canvas" element selector.
///
/// If set, this selector will be used to find a matching html canvas element,
/// rather than creating a new one.
/// rather than creating a new one.
/// Uses the [CSS selector format](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector).
///
/// This value has no effect on non-web platforms.