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

Use EntityHashMap for EntityMapper #10415

Merged
merged 4 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions crates/bevy_ecs/src/entity/map_entities.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use crate::{entity::Entity, world::World};
use bevy_utils::HashMap;
use bevy_utils::EntityHashMap;

/// Operation to map all contained [`Entity`] fields in a type to new values.
///
/// As entity IDs are valid only for the [`World`] they're sourced from, using [`Entity`]
/// as references in components copied from another world will be invalid. This trait
/// allows defining custom mappings for these references via [`HashMap<Entity, Entity>`]
/// allows defining custom mappings for these references via [`EntityHashMap<Entity, Entity>`]
///
/// Implementing this trait correctly is required for properly loading components
/// with entity references from scenes.
Expand Down Expand Up @@ -39,7 +39,7 @@ pub trait MapEntities {
fn map_entities(&mut self, entity_mapper: &mut EntityMapper);
}

/// A wrapper for [`HashMap<Entity, Entity>`], augmenting it with the ability to allocate new [`Entity`] references in a destination
/// A wrapper for [`EntityHashMap<Entity, Entity>`], 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
Expand All @@ -52,9 +52,9 @@ pub struct EntityMapper<'m> {
/// 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, a [`HashMap<Entity, Entity>`] is not capable of allocating new entity identifiers, which is needed to map references
/// On its own, a [`EntityHashMap<Entity, Entity>`] is not capable of allocating new entity identifiers, which is needed to map references
/// to entities that lie outside the source entity set. This functionality can be accessed through [`EntityMapper::world_scope()`].
map: &'m mut HashMap<Entity, Entity>,
map: &'m mut EntityHashMap<Entity, Entity>,
/// A base [`Entity`] used to allocate new references.
dead_start: Entity,
/// The number of generations this mapper has allocated thus far.
Expand All @@ -80,18 +80,18 @@ impl<'m> EntityMapper<'m> {
new
}

/// Gets a reference to the underlying [`HashMap<Entity, Entity>`].
pub fn get_map(&'m self) -> &'m HashMap<Entity, Entity> {
/// Gets a reference to the underlying [`EntityHashMap<Entity, Entity>`].
pub fn get_map(&'m self) -> &'m EntityHashMap<Entity, Entity> {
self.map
}

/// Gets a mutable reference to the underlying [`HashMap<Entity, Entity>`].
pub fn get_map_mut(&'m mut self) -> &'m mut HashMap<Entity, Entity> {
/// Gets a mutable reference to the underlying [`EntityHashMap<Entity, Entity>`].
pub fn get_map_mut(&'m mut self) -> &'m mut EntityHashMap<Entity, Entity> {
self.map
}

/// Creates a new [`EntityMapper`], spawning a temporary base [`Entity`] in the provided [`World`]
fn new(map: &'m mut HashMap<Entity, Entity>, world: &mut World) -> Self {
fn new(map: &'m mut EntityHashMap<Entity, Entity>, world: &mut World) -> Self {
Self {
map,
// SAFETY: Entities data is kept in a valid state via `EntityMapper::world_scope`
Expand All @@ -111,14 +111,14 @@ impl<'m> EntityMapper<'m> {
assert!(entities.reserve_generations(self.dead_start.index, self.generations));
}

/// Creates an [`EntityMapper`] from a provided [`World`] and [`HashMap<Entity, Entity>`], then calls the
/// Creates an [`EntityMapper`] from a provided [`World`] and [`EntityHashMap<Entity, Entity>`], then calls the
/// provided function with it. This allows one to allocate new entity references in this [`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 this world. Its return value is then returned from `world_scope` as the generic type
/// parameter `R`.
pub fn world_scope<R>(
entity_map: &'m mut HashMap<Entity, Entity>,
entity_map: &'m mut EntityHashMap<Entity, Entity>,
world: &mut World,
f: impl FnOnce(&mut World, &mut Self) -> R,
) -> R {
Expand All @@ -131,7 +131,7 @@ impl<'m> EntityMapper<'m> {

#[cfg(test)]
mod tests {
use bevy_utils::HashMap;
use bevy_utils::EntityHashMap;

use crate::{
entity::{Entity, EntityMapper},
Expand All @@ -143,7 +143,7 @@ mod tests {
const FIRST_IDX: u32 = 1;
const SECOND_IDX: u32 = 2;

let mut map = HashMap::default();
let mut map = EntityHashMap::default();
let mut world = World::new();
let mut mapper = EntityMapper::new(&mut map, &mut world);

Expand All @@ -170,7 +170,7 @@ mod tests {

#[test]
fn world_scope_reserves_generations() {
let mut map = HashMap::default();
let mut map = EntityHashMap::default();
let mut world = World::new();

let dead_ref = EntityMapper::world_scope(&mut map, &mut world, |_, mapper| {
Expand Down
20 changes: 12 additions & 8 deletions crates/bevy_ecs/src/reflect/map_entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
world::World,
};
use bevy_reflect::FromType;
use bevy_utils::HashMap;
use bevy_utils::EntityHashMap;

/// For a specific type of component, this maps any fields with values of type [`Entity`] to a new world.
/// Since a given `Entity` ID is only valid for the world it came from, when performing deserialization
Expand All @@ -18,29 +18,33 @@ pub struct ReflectMapEntities {
}

impl ReflectMapEntities {
/// A general method for applying [`MapEntities`] behavior to all elements in an [`HashMap<Entity, Entity>`].
/// A general method for applying [`MapEntities`] behavior to all elements in an [`EntityHashMap<Entity, Entity>`].
///
/// Be mindful in its usage: Works best in situations where the entities in the [`HashMap<Entity, Entity>`] are newly
/// Be mindful in its usage: Works best in situations where the entities in the [`EntityHashMap<Entity, Entity>`] are newly
/// created, before systems have a chance to add new components. If some of the entities referred to
/// by the [`HashMap<Entity, Entity>`] might already contain valid entity references, you should use [`map_entities`](Self::map_entities).
/// by the [`EntityHashMap<Entity, Entity>`] might already contain valid entity references, you should use [`map_entities`](Self::map_entities).
///
/// 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_all_entities`](Self::map_all_entities), those `Parent`
/// components with already valid entity references could be updated to point at something else entirely.
pub fn map_all_entities(&self, world: &mut World, entity_map: &mut HashMap<Entity, Entity>) {
pub fn map_all_entities(
&self,
world: &mut World,
entity_map: &mut EntityHashMap<Entity, Entity>,
) {
EntityMapper::world_scope(entity_map, world, self.map_all_entities);
}

/// A general method for applying [`MapEntities`] behavior to elements in an [`HashMap<Entity, Entity>`]. Unlike
/// A general method for applying [`MapEntities`] behavior to elements in an [`EntityHashMap<Entity, Entity>`]. Unlike
/// [`map_all_entities`](Self::map_all_entities), this is applied to specific entities, not all values
/// in the [`HashMap<Entity, Entity>`].
/// in the [`EntityHashMap<Entity, Entity>`].
///
/// This is useful mostly for when you need to be careful not to update components that already contain valid entity
/// values. See [`map_all_entities`](Self::map_all_entities) for more details.
pub fn map_entities(
&self,
world: &mut World,
entity_map: &mut HashMap<Entity, Entity>,
entity_map: &mut EntityHashMap<Entity, Entity>,
entities: &[Entity],
) {
EntityMapper::world_scope(entity_map, world, |world, mapper| {
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_scene/src/dynamic_scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use bevy_ecs::{
world::World,
};
use bevy_reflect::{Reflect, TypePath, TypeRegistryArc};
use bevy_utils::HashMap;
use bevy_utils::{EntityHashMap, HashMap};
use std::any::TypeId;

#[cfg(feature = "serialize")]
Expand Down Expand Up @@ -65,7 +65,7 @@ impl DynamicScene {
pub fn write_to_world_with(
&self,
world: &mut World,
entity_map: &mut HashMap<Entity, Entity>,
entity_map: &mut EntityHashMap<Entity, Entity>,
type_registry: &AppTypeRegistry,
) -> Result<(), SceneSpawnError> {
let type_registry = type_registry.read();
Expand Down Expand Up @@ -163,7 +163,7 @@ impl DynamicScene {
pub fn write_to_world(
&self,
world: &mut World,
entity_map: &mut HashMap<Entity, Entity>,
entity_map: &mut EntityHashMap<Entity, Entity>,
) -> Result<(), SceneSpawnError> {
let registry = world.resource::<AppTypeRegistry>().clone();
self.write_to_world_with(world, entity_map, &registry)
Expand Down Expand Up @@ -193,7 +193,7 @@ where
mod tests {
use bevy_ecs::{reflect::AppTypeRegistry, system::Command, world::World};
use bevy_hierarchy::{AddChild, Parent};
use bevy_utils::HashMap;
use bevy_utils::EntityHashMap;

use crate::dynamic_scene_builder::DynamicSceneBuilder;

Expand Down Expand Up @@ -222,7 +222,7 @@ mod tests {
.extract_entity(original_parent_entity)
.extract_entity(original_child_entity)
.build();
let mut entity_map = HashMap::default();
let mut entity_map = EntityHashMap::default();
scene.write_to_world(&mut world, &mut entity_map).unwrap();

let &from_scene_parent_entity = entity_map.get(&original_parent_entity).unwrap();
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_scene/src/scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use bevy_ecs::{
world::World,
};
use bevy_reflect::TypePath;
use bevy_utils::HashMap;
use bevy_utils::EntityHashMap;

/// To spawn a scene, you can use either:
/// * [`SceneSpawner::spawn`](crate::SceneSpawner::spawn)
Expand All @@ -31,7 +31,7 @@ impl Scene {
type_registry: &AppTypeRegistry,
) -> Result<Scene, SceneSpawnError> {
let mut world = World::new();
let mut entity_map = HashMap::default();
let mut entity_map = EntityHashMap::default();
dynamic_scene.write_to_world_with(&mut world, &mut entity_map, type_registry)?;

Ok(Self { world })
Expand All @@ -57,7 +57,7 @@ impl Scene {
type_registry: &AppTypeRegistry,
) -> Result<InstanceInfo, SceneSpawnError> {
let mut instance_info = InstanceInfo {
entity_map: HashMap::default(),
entity_map: EntityHashMap::default(),
};

let type_registry = type_registry.read();
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_scene/src/scene_spawner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use bevy_ecs::{
world::{Mut, World},
};
use bevy_hierarchy::{AddChild, Parent};
use bevy_utils::{tracing::error, HashMap, HashSet};
use bevy_utils::{tracing::error, EntityHashMap, HashMap, HashSet};
use thiserror::Error;
use uuid::Uuid;

Expand All @@ -25,7 +25,7 @@ pub struct SceneInstanceReady {
#[derive(Debug)]
pub struct InstanceInfo {
/// Mapping of entities from the scene world to the instance world.
pub entity_map: HashMap<Entity, Entity>,
pub entity_map: EntityHashMap<Entity, Entity>,
}

/// Unique id identifying a scene instance.
Expand Down Expand Up @@ -199,7 +199,7 @@ impl SceneSpawner {
world: &mut World,
id: impl Into<AssetId<DynamicScene>>,
) -> Result<(), SceneSpawnError> {
let mut entity_map = HashMap::default();
let mut entity_map = EntityHashMap::default();
let id = id.into();
Self::spawn_dynamic_internal(world, id, &mut entity_map)?;
let instance_id = InstanceId::new();
Expand All @@ -213,7 +213,7 @@ impl SceneSpawner {
fn spawn_dynamic_internal(
world: &mut World,
id: AssetId<DynamicScene>,
entity_map: &mut HashMap<Entity, Entity>,
entity_map: &mut EntityHashMap<Entity, Entity>,
) -> Result<(), SceneSpawnError> {
world.resource_scope(|world, scenes: Mut<Assets<DynamicScene>>| {
let scene = scenes
Expand Down Expand Up @@ -297,7 +297,7 @@ impl SceneSpawner {
let scenes_to_spawn = std::mem::take(&mut self.dynamic_scenes_to_spawn);

for (id, instance_id) in scenes_to_spawn {
let mut entity_map = HashMap::default();
let mut entity_map = EntityHashMap::default();

match Self::spawn_dynamic_internal(world, id, &mut entity_map) {
Ok(_) => {
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_scene/src/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ mod tests {
use bevy_ecs::reflect::{AppTypeRegistry, ReflectMapEntities};
use bevy_ecs::world::FromWorld;
use bevy_reflect::{Reflect, ReflectSerialize};
use bevy_utils::HashMap;
use bevy_utils::EntityHashMap;
use bincode::Options;
use serde::de::DeserializeSeed;
use serde::Serialize;
Expand Down Expand Up @@ -678,7 +678,7 @@ mod tests {
"expected `entities` to contain 3 entities"
);

let mut map = HashMap::default();
let mut map = EntityHashMap::default();
let mut dst_world = create_world();
scene.write_to_world(&mut dst_world, &mut map).unwrap();

Expand Down Expand Up @@ -717,7 +717,7 @@ mod tests {

let deserialized_scene = scene_deserializer.deserialize(&mut deserializer).unwrap();

let mut map = HashMap::default();
let mut map = EntityHashMap::default();
let mut dst_world = create_world();
deserialized_scene
.write_to_world(&mut dst_world, &mut map)
Expand Down