Skip to content

Commit

Permalink
fix insert_reflect panic caused by clone_value (#10627)
Browse files Browse the repository at this point in the history
# Objective

- `insert_reflect` relies on `reflect_type_path`, which doesn't gives
the actual type path for object created by `clone_value`, leading to an
unexpected panic. This is a workaround for it.
- Fix #10590 

## Solution

- Tries to get type path from `get_represented_type_info` if get failed
from `reflect_type_path`.

---

## Defect remaining

- `get_represented_type_info` implies a shortage on performance than
using `TypeRegistry`.
  • Loading branch information
hxYuki authored and cart committed Nov 30, 2023
1 parent 6d29503 commit d616608
Showing 1 changed file with 14 additions and 6 deletions.
20 changes: 14 additions & 6 deletions crates/bevy_ecs/src/reflect/entity_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,15 +189,18 @@ fn insert_reflect(
type_registry: &TypeRegistry,
component: Box<dyn Reflect>,
) {
let type_info = component.reflect_type_path();
let type_info = component
.get_represented_type_info()
.expect("component should represent a type.");
let type_path = type_info.type_path();
let Some(mut entity) = world.get_entity_mut(entity) else {
panic!("error[B0003]: Could not insert a reflected component (of type {}) for entity {entity:?} because it doesn't exist in this World.", component.reflect_type_path());
panic!("error[B0003]: Could not insert a reflected component (of type {type_path}) for entity {entity:?} because it doesn't exist in this World.");
};
let Some(type_registration) = type_registry.get_with_type_path(type_info) else {
panic!("Could not get type registration (for component type {}) because it doesn't exist in the TypeRegistry.", component.reflect_type_path());
let Some(type_registration) = type_registry.get_with_type_path(type_path) else {
panic!("Could not get type registration (for component type {type_path}) because it doesn't exist in the TypeRegistry.");
};
let Some(reflect_component) = type_registration.data::<ReflectComponent>() else {
panic!("Could not get ReflectComponent data (for component type {}) because it doesn't exist in this TypeRegistration.", component.reflect_type_path());
panic!("Could not get ReflectComponent data (for component type {type_path}) because it doesn't exist in this TypeRegistration.");
};
reflect_component.insert(&mut entity, &*component);
}
Expand Down Expand Up @@ -346,17 +349,22 @@ mod tests {
let mut commands = system_state.get_mut(&mut world);

let entity = commands.spawn_empty().id();
let entity2 = commands.spawn_empty().id();

let boxed_reflect_component_a = Box::new(ComponentA(916)) as Box<dyn Reflect>;
let boxed_reflect_component_a_clone = boxed_reflect_component_a.clone_value();

commands
.entity(entity)
.insert_reflect(boxed_reflect_component_a);
commands
.entity(entity2)
.insert_reflect(boxed_reflect_component_a_clone);
system_state.apply(&mut world);

assert_eq!(
world.entity(entity).get::<ComponentA>(),
Some(&ComponentA(916))
world.entity(entity2).get::<ComponentA>()
);
}

Expand Down

0 comments on commit d616608

Please sign in to comment.