Use EntityHashMap for EntityMapper (#10415)

# Objective

- There is a specialized hasher for entities:
[`EntityHashMap`](https://docs.rs/bevy/latest/bevy/utils/type.EntityHashMap.html)
- [`EntityMapper`] currently uses a normal `HashMap<Entity, Entity>`
- Fixes #10391

## Solution

- Replace the normal `HashMap` with the more performant `EntityHashMap`

## Questions

- This does change public API. Should a system be implemented to help
migrate code?
  - Perhaps an `impl From<HashMap<K, V, S>> for EntityHashMap<K, V>`
- I updated to docs for each function that I changed, but I may have
missed something

---

## Changelog

- Changed `EntityMapper` to use `EntityHashMap` instead of normal
`HashMap`

## Migration Guide

If you are using the following types, update their listed methods to use
the new `EntityHashMap`. `EntityHashMap` has the same methods as the
normal `HashMap`, so you just need to replace the name.

### `EntityMapper`

- `get_map`
- `get_mut_map`
- `new`
- `world_scope`

### `ReflectMapEntities`

- `map_all_entities`
- `map_entities`
- `write_to_world`

### `InstanceInfo`

- `entity_map`
  - This is a property, not a method.

---

This is my first time contributing in a while, and I'm not familiar with
the usage of `EntityMapper`. I changed the type definition and fixed all
errors, but there may have been things I've missed. Please keep an eye
out for me!
This commit is contained in:
BD103 2023-11-07 03:23:04 -05:00 committed by GitHub
parent 6ce33d0267
commit 04ceb46fe0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 43 additions and 39 deletions

View file

@ -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.
@ -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
@ -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.
@ -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`
@ -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 {
@ -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},
@ -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);
@ -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| {

View file

@ -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
@ -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| {

View file

@ -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")]
@ -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();
@ -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)
@ -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;
@ -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();

View file

@ -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)
@ -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 })
@ -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();

View file

@ -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;
@ -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.
@ -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();
@ -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
@ -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(_) => {

View file

@ -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;
@ -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();
@ -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)