Skip to content
Closed
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
59 changes: 54 additions & 5 deletions crates/bevy_ecs/src/reflect.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Types that enable reflection support.
//! Types that enable run-time type reflection of ECS data.

pub use crate::change_detection::ReflectMut;
use crate::{
Expand All @@ -8,9 +8,23 @@ use crate::{
};
use bevy_reflect::{impl_reflect_value, FromType, Reflect, ReflectDeserialize};

/// A runtime type-reflectable component

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely correct. ReflectComponent is a type-erased collection of operations on a given component type. By itself, it doesn't contain any reference to a given component. It's meant to be used as metadata that is fetched Res<TypeRegistryArc>.

It can be used to get a dyn Reflect reference given a World reference and Entity, but by itself it does not hold nor refer to component data at all.

This isn't too intuitive at all, so an explicit example is probably best here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's a very useful explanation! I was muddling through based on my first pass examining the raw code.

///
/// Intended for use with [`bevy_reflect`],
/// a [`ReflectComponent`] is a type-erased version of a component's data,
/// can be transformed into ['dyn Reflect'](Reflect) trait objects,
/// which can be worked with generically
/// and loaded from disk in a type-safe fashion.
Comment on lines +14 to +17

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be more accurate to say something like "a ReflectComponent exposes type-erased methods for interacting with components of a certain type." IMO what you have here makes it sound like the ReflectComponent actually stores the component instance's data or something, when in reality it's essentially a virtual method table associated with the component's type. It doesn't so much "transform into" a dyn reflect as fetch the component from the world and then cast it to a dyn reflect.

///
/// [`ReflectComponent`] objects are created for a particular [`Component`] type (`C`) using the [`from_type`](FromType::from_type) method.
/// That type `C` is implicitly stored in the function pointers held within the private fields of this type;
/// it cannot be changed after creation.
///
/// Once a [`ReflectComponent`] object has been created, you can use that concrete struct
/// to use the methods on this type, which always implicitly affect only the component type originally used to create this struct.
Comment on lines +23 to +24

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could really use an example. The interaction between ReflectComponent and any given &dyn Reflect is profoundly unintuitive.

Also, nit: "always implicitly affects only..." to me sounds like it's suggesting you can use these methods without worrying about types, because they handle the checking for you. In actual fact the situation is a bit more dire; some of these methods panic if the reference you give them is of the wrong type.

#[derive(Clone)]
pub struct ReflectComponent {
add_component: fn(&mut World, Entity, &dyn Reflect),
insert_component: fn(&mut World, Entity, &dyn Reflect),
apply_component: fn(&mut World, Entity, &dyn Reflect),
remove_component: fn(&mut World, Entity),
reflect_component: fn(&World, Entity) -> Option<&dyn Reflect>,
Expand All @@ -19,18 +33,31 @@ pub struct ReflectComponent {
}

impl ReflectComponent {
pub fn add_component(&self, world: &mut World, entity: Entity, component: &dyn Reflect) {
(self.add_component)(world, entity, component);
/// Inserts the non-erased value of `component` (with type `C`) into the `entity`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if the entity already has a component of type C?

///
/// # Panics
/// `component` must have the same type `C` as the type used to create this struct

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik component only needs to be applicable to C, but need not actually be of type C. for instance, on this line, component may be a DynamicStruct.

pub fn insert_component(&self, world: &mut World, entity: Entity, component: &dyn Reflect) {
(self.insert_component)(world, entity, component);
}

/// Sets the existing value of type `C` found on `entity` to the non-erased value of `component`
///
/// # Panics
/// `component` must have the same type `C` as the type used to create this struct
/// Additionally, a component of type `C` must already exist on `entity.
pub fn apply_component(&self, world: &mut World, entity: Entity, component: &dyn Reflect) {
(self.apply_component)(world, entity, component);
}

/// Removes any component of type `C` from the `entity`
pub fn remove_component(&self, world: &mut World, entity: Entity) {
(self.remove_component)(world, entity);
}

/// Fetches an immutable reference to the component of type `C` on `entity`
///
/// If the `Entity` does not have a component of the specified type, `None` is returned instead.
pub fn reflect_component<'a>(
&self,
world: &'a World,
Expand All @@ -39,6 +66,9 @@ impl ReflectComponent {
(self.reflect_component)(world, entity)
}

/// Fetches a mutable reference to the component of type `C` on `entity`
///
/// If the [`Entity`] does not have a component of the specified type, `None` is returned instead.
pub fn reflect_component_mut<'a>(
&self,
world: &'a mut World,
Expand All @@ -48,6 +78,13 @@ impl ReflectComponent {
unsafe { (self.reflect_component_mut)(world, entity) }
}

/// Fetches a mutable reference to the component of type `C` on `entity` without guaranteeing unique mutable access to the `world`
///
/// This method does not require exclusive [`World`] access, and so multiple mutable references can be alive at once.
/// If possible, please prefer the safe version of this method, [`reflect_component_mut`](Self::reflect_component_mut).
///
/// If the [`Entity`] does not have a component of the specified type, [`None`] is returned instead.
///
/// # Safety
/// This method does not prevent you from having two mutable pointers to the same data,
/// violating Rust's aliasing rules. To avoid this:
Expand All @@ -62,6 +99,17 @@ impl ReflectComponent {
(self.reflect_component_mut)(world, entity)
}

/// Directly copies the value of the component of type `C` to a new entity
///
/// This method creates a new component of type `C` using the [`FromWorld`] trait,
/// sets its value to the value of the component of type `C` on the `source_entity`,
/// and then inserts the new component into the `destination_entity`.
///
/// **Note**: this method uses [`Reflect`] to create a shallow value-based copy of the component and will not respect `Clone` implementations.
/// This can have unexpected negative consequences if you are relying on ref-counting or the like.
///
/// # Panics
/// The `source_entity` in the `source_world` must have a component of type `C`
pub fn copy_component(
&self,
source_world: &World,
Expand All @@ -81,7 +129,7 @@ impl ReflectComponent {
impl<C: Component + Reflect + FromWorld> FromType<C> for ReflectComponent {
fn from_type() -> Self {
ReflectComponent {
add_component: |world, entity, reflected_component| {
insert_component: |world, entity, reflected_component| {
let mut component = C::from_world(world);
component.apply(reflected_component);
world.entity_mut(entity).insert(component);
Expand Down Expand Up @@ -122,6 +170,7 @@ impl<C: Component + Reflect + FromWorld> FromType<C> for ReflectComponent {

impl_reflect_value!(Entity(Hash, PartialEq, Serialize, Deserialize));

/// A reflected [EntityMap]
#[derive(Clone)]
pub struct ReflectMapEntities {
map_entities: fn(&mut World, &EntityMap) -> Result<(), MapEntitiesError>,
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 @@ -87,7 +87,7 @@ impl DynamicScene {
{
reflect_component.apply_component(world, entity, &**component);
} else {
reflect_component.add_component(world, entity, &**component);
reflect_component.insert_component(world, entity, &**component);
}
}
}
Expand Down