Skip to content

Commit

Permalink
Rename map_entities and map_specific_entities (#7570)
Browse files Browse the repository at this point in the history
# Objective

After fixing dynamic scene to only map specific entities, we want
map_entities to default to the less error prone behavior and have the
previous behavior renamed to "map_all_entities." As this is a breaking
change, it could not be pushed out with the bug fix.

## Solution

Simple rename and refactor.

## Changelog

### Changed
- `map_entities` now accepts a list of entities to apply to, with
`map_all_entities` retaining previous behavior of applying to all
entities in the map.

## Migration Guide 

- In `bevy_ecs`, `ReflectMapEntities::map_entites` now requires an
additional `entities` parameter to specify which entities it applies to.
To keep the old behavior, use the new
`ReflectMapEntities::map_all_entities`, but consider if passing the
entities in specifically might be better for your use case to avoid
bugs.
  • Loading branch information
Testare authored May 1, 2023
1 parent 8070c29 commit a29d328
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 18 deletions.
28 changes: 12 additions & 16 deletions crates/bevy_ecs/src/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,8 @@ impl_from_reflect_value!(Entity);

#[derive(Clone)]
pub struct ReflectMapEntities {
map_entities: fn(&mut World, &mut EntityMapper),
map_specific_entities: fn(&mut World, &mut EntityMapper, &[Entity]),
map_all_entities: fn(&mut World, &mut EntityMapper),
map_entities: fn(&mut World, &mut EntityMapper, &[Entity]),
}

impl ReflectMapEntities {
Expand All @@ -424,40 +424,36 @@ impl ReflectMapEntities {
/// by the [`EntityMap`] 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_entities`](Self::map_entities), those `Parent`
/// 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_entities(&self, world: &mut World, entity_map: &mut EntityMap) {
entity_map.world_scope(world, self.map_entities);
pub fn map_all_entities(&self, world: &mut World, entity_map: &mut EntityMap) {
entity_map.world_scope(world, self.map_all_entities);
}

/// This is like [`map_entities`](Self::map_entities), but only applied to specific entities, not all values
/// A general method for applying [`MapEntities`] behavior to elements in an [`EntityMap`]. Unlike
/// [`map_all_entities`](Self::map_all_entities), this is applied to specific entities, not all values
/// in the [`EntityMap`].
///
/// This is useful mostly for when you need to be careful not to update components that already contain valid entity
/// values. See [`map_entities`](Self::map_entities) for more details.
pub fn map_specific_entities(
&self,
world: &mut World,
entity_map: &mut EntityMap,
entities: &[Entity],
) {
/// values. See [`map_all_entities`](Self::map_all_entities) for more details.
pub fn map_entities(&self, world: &mut World, entity_map: &mut EntityMap, entities: &[Entity]) {
entity_map.world_scope(world, |world, mapper| {
(self.map_specific_entities)(world, mapper, entities);
(self.map_entities)(world, mapper, entities);
});
}
}

impl<C: Component + MapEntities> FromType<C> for ReflectMapEntities {
fn from_type() -> Self {
ReflectMapEntities {
map_specific_entities: |world, entity_mapper, entities| {
map_entities: |world, entity_mapper, entities| {
for &entity in entities {
if let Some(mut component) = world.get_mut::<C>(entity) {
component.map_entities(entity_mapper);
}
}
},
map_entities: |world, entity_mapper| {
map_all_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) {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_scene/src/dynamic_scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +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);
map_entities_reflect.map_entities(world, entity_map, &entities);
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_scene/src/scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +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, &mut instance_info.entity_map);
map_entities_reflect.map_all_entities(world, &mut instance_info.entity_map);
}
}

Expand Down

0 comments on commit a29d328

Please sign in to comment.