Skip to content

Commit

Permalink
reflect: implement the unique reflect rfc (#7207)
Browse files Browse the repository at this point in the history
# Objective

- Implements the [Unique Reflect
RFC](https://github.com/nicopap/rfcs/blob/bevy-reflect-api/rfcs/56-better-reflect.md).

## Solution

- Implements the RFC.
- This implementation differs in some ways from the RFC:
- In the RFC, it was suggested `Reflect: Any` but `PartialReflect:
?Any`. During initial implementation I tried this, but we assume the
`PartialReflect: 'static` in a lot of places and the changes required
crept out of the scope of this PR.
- `PartialReflect::try_into_reflect` originally returned `Option<Box<dyn
Reflect>>` but i changed this to `Result<Box<dyn Reflect>, Box<dyn
PartialReflect>>` since the method takes by value and otherwise there
would be no way to recover the type. `as_full` and `as_full_mut` both
still return `Option<&(mut) dyn Reflect>`.

---

## Changelog

- Added `PartialReflect`.
- `Reflect` is now a subtrait of `PartialReflect`.
- Moved most methods on `Reflect` to the new `PartialReflect`.
- Added `PartialReflect::{as_partial_reflect, as_partial_reflect_mut,
into_partial_reflect}`.
- Added `PartialReflect::{try_as_reflect, try_as_reflect_mut,
try_into_reflect}`.
- Added `<dyn PartialReflect>::{try_downcast_ref, try_downcast_mut,
try_downcast, try_take}` supplementing the methods on `dyn Reflect`.

## Migration Guide

- Most instances of `dyn Reflect` should be changed to `dyn
PartialReflect` which is less restrictive, however trait bounds should
generally stay as `T: Reflect`.
- The new `PartialReflect::{as_partial_reflect, as_partial_reflect_mut,
into_partial_reflect, try_as_reflect, try_as_reflect_mut,
try_into_reflect}` methods as well as `Reflect::{as_reflect,
as_reflect_mut, into_reflect}` will need to be implemented for manual
implementors of `Reflect`.

## Future Work

- This PR is designed to be followed up by another "Unique Reflect Phase
2" that addresses the following points:
- Investigate making serialization revolve around `Reflect` instead of
`PartialReflect`.
- [Remove the `try_*` methods on `dyn PartialReflect` since they are
stop
gaps](#7207 (comment)).
- Investigate usages like `ReflectComponent`. In the places they
currently use `PartialReflect`, should they be changed to use `Reflect`?
- Merging this opens the door to lots of reflection features we haven't
been able to implement.
- We could re-add [the `Reflectable`
trait](https://github.com/bevyengine/bevy/blob/8e3488c88065a94a4f72199587e59341c9b6553d/crates/bevy_reflect/src/reflect.rs#L337-L342)
and make `FromReflect` a requirement to improve [`FromReflect`
ergonomics](bevyengine/rfcs#59). This is
currently not possible because dynamic types cannot sensibly be
`FromReflect`.
  - Since this is an alternative to #5772, #5781 would be made cleaner.

---------

Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: Gino Valente <[email protected]>
  • Loading branch information
3 people authored Aug 12, 2024
1 parent 7b81ae7 commit 6ab8767
Show file tree
Hide file tree
Showing 65 changed files with 2,163 additions and 1,747 deletions.
4 changes: 3 additions & 1 deletion crates/bevy_asset/src/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,8 @@ pub enum UntypedAssetConversionError {

#[cfg(test)]
mod tests {
use bevy_reflect::PartialReflect;

use super::*;

type TestAsset = ();
Expand Down Expand Up @@ -651,7 +653,7 @@ mod tests {
);

let reflected: &dyn Reflect = &handle;
let cloned_handle: Box<dyn Reflect> = reflected.clone_value();
let cloned_handle: Box<dyn PartialReflect> = reflected.clone_value();

assert_eq!(
Arc::strong_count(strong),
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_asset/src/reflect.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::any::{Any, TypeId};

use bevy_ecs::world::{unsafe_world_cell::UnsafeWorldCell, World};
use bevy_reflect::{FromReflect, FromType, Reflect};
use bevy_reflect::{FromReflect, FromType, PartialReflect, Reflect};

use crate::{Asset, AssetId, Assets, Handle, UntypedAssetId, UntypedHandle};

Expand All @@ -22,8 +22,8 @@ pub struct ReflectAsset {
// - may only be called with an [`UnsafeWorldCell`] which can be used to access the corresponding `Assets<T>` resource mutably
// - may only be used to access **at most one** access at once
get_unchecked_mut: unsafe fn(UnsafeWorldCell<'_>, UntypedHandle) -> Option<&mut dyn Reflect>,
add: fn(&mut World, &dyn Reflect) -> UntypedHandle,
insert: fn(&mut World, UntypedHandle, &dyn Reflect),
add: fn(&mut World, &dyn PartialReflect) -> UntypedHandle,
insert: fn(&mut World, UntypedHandle, &dyn PartialReflect),
len: fn(&World) -> usize,
ids: for<'w> fn(&'w World) -> Box<dyn Iterator<Item = UntypedAssetId> + 'w>,
remove: fn(&mut World, UntypedHandle) -> Option<Box<dyn Reflect>>,
Expand Down Expand Up @@ -94,11 +94,11 @@ impl ReflectAsset {
}

/// Equivalent of [`Assets::add`]
pub fn add(&self, world: &mut World, value: &dyn Reflect) -> UntypedHandle {
pub fn add(&self, world: &mut World, value: &dyn PartialReflect) -> UntypedHandle {
(self.add)(world, value)
}
/// Equivalent of [`Assets::insert`]
pub fn insert(&self, world: &mut World, handle: UntypedHandle, value: &dyn Reflect) {
pub fn insert(&self, world: &mut World, handle: UntypedHandle, value: &dyn PartialReflect) {
(self.insert)(world, handle, value);
}

Expand Down
53 changes: 32 additions & 21 deletions crates/bevy_ecs/src/reflect/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
//! This module exports two types: [`ReflectBundleFns`] and [`ReflectBundle`].
//!
//! Same as [`super::component`], but for bundles.
use std::any::TypeId;
use std::any::{Any, TypeId};

use crate::{
prelude::Bundle,
world::{EntityMut, EntityWorldMut},
};
use bevy_reflect::{FromReflect, FromType, Reflect, ReflectRef, TypeRegistry};
use bevy_reflect::{
FromReflect, FromType, PartialReflect, Reflect, ReflectRef, TypePath, TypeRegistry,
};

use super::{from_reflect_with_fallback, ReflectComponent};

Expand All @@ -27,11 +29,11 @@ pub struct ReflectBundle(ReflectBundleFns);
#[derive(Clone)]
pub struct ReflectBundleFns {
/// Function pointer implementing [`ReflectBundle::insert()`].
pub insert: fn(&mut EntityWorldMut, &dyn Reflect, &TypeRegistry),
pub insert: fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry),
/// Function pointer implementing [`ReflectBundle::apply()`].
pub apply: fn(EntityMut, &dyn Reflect, &TypeRegistry),
pub apply: fn(EntityMut, &dyn PartialReflect, &TypeRegistry),
/// Function pointer implementing [`ReflectBundle::apply_or_insert()`].
pub apply_or_insert: fn(&mut EntityWorldMut, &dyn Reflect, &TypeRegistry),
pub apply_or_insert: fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry),
/// Function pointer implementing [`ReflectBundle::remove()`].
pub remove: fn(&mut EntityWorldMut),
}
Expand All @@ -42,7 +44,7 @@ impl ReflectBundleFns {
///
/// This is useful if you want to start with the default implementation before overriding some
/// of the functions to create a custom implementation.
pub fn new<T: Bundle + Reflect + FromReflect>() -> Self {
pub fn new<T: Bundle + FromReflect + TypePath>() -> Self {
<ReflectBundle as FromType<T>>::from_type().0
}
}
Expand All @@ -52,7 +54,7 @@ impl ReflectBundle {
pub fn insert(
&self,
entity: &mut EntityWorldMut,
bundle: &dyn Reflect,
bundle: &dyn PartialReflect,
registry: &TypeRegistry,
) {
(self.0.insert)(entity, bundle, registry);
Expand All @@ -66,7 +68,7 @@ impl ReflectBundle {
pub fn apply<'a>(
&self,
entity: impl Into<EntityMut<'a>>,
bundle: &dyn Reflect,
bundle: &dyn PartialReflect,
registry: &TypeRegistry,
) {
(self.0.apply)(entity.into(), bundle, registry);
Expand All @@ -76,7 +78,7 @@ impl ReflectBundle {
pub fn apply_or_insert(
&self,
entity: &mut EntityWorldMut,
bundle: &dyn Reflect,
bundle: &dyn PartialReflect,
registry: &TypeRegistry,
) {
(self.0.apply_or_insert)(entity, bundle, registry);
Expand Down Expand Up @@ -122,7 +124,7 @@ impl ReflectBundle {
}
}

impl<B: Bundle + Reflect> FromType<B> for ReflectBundle {
impl<B: Bundle + Reflect + TypePath> FromType<B> for ReflectBundle {
fn from_type() -> Self {
ReflectBundle(ReflectBundleFns {
insert: |entity, reflected_bundle, registry| {
Expand Down Expand Up @@ -180,10 +182,16 @@ impl<B: Bundle + Reflect> FromType<B> for ReflectBundle {
}
}

fn apply_field(entity: &mut EntityMut, field: &dyn Reflect, registry: &TypeRegistry) {
if let Some(reflect_component) = registry.get_type_data::<ReflectComponent>(field.type_id()) {
fn apply_field(entity: &mut EntityMut, field: &dyn PartialReflect, registry: &TypeRegistry) {
let Some(type_id) = field.try_as_reflect().map(Any::type_id) else {
panic!(
"`{}` did not implement `Reflect`",
field.reflect_type_path()
);
};
if let Some(reflect_component) = registry.get_type_data::<ReflectComponent>(type_id) {
reflect_component.apply(entity.reborrow(), field);
} else if let Some(reflect_bundle) = registry.get_type_data::<ReflectBundle>(field.type_id()) {
} else if let Some(reflect_bundle) = registry.get_type_data::<ReflectBundle>(type_id) {
reflect_bundle.apply(entity.reborrow(), field, registry);
} else {
panic!(
Expand All @@ -195,19 +203,22 @@ fn apply_field(entity: &mut EntityMut, field: &dyn Reflect, registry: &TypeRegis

fn apply_or_insert_field(
entity: &mut EntityWorldMut,
field: &dyn Reflect,
field: &dyn PartialReflect,
registry: &TypeRegistry,
) {
if let Some(reflect_component) = registry.get_type_data::<ReflectComponent>(field.type_id()) {
let Some(type_id) = field.try_as_reflect().map(Any::type_id) else {
panic!(
"`{}` did not implement `Reflect`",
field.reflect_type_path()
);
};

if let Some(reflect_component) = registry.get_type_data::<ReflectComponent>(type_id) {
reflect_component.apply_or_insert(entity, field, registry);
} else if let Some(reflect_bundle) = registry.get_type_data::<ReflectBundle>(field.type_id()) {
} else if let Some(reflect_bundle) = registry.get_type_data::<ReflectBundle>(type_id) {
reflect_bundle.apply_or_insert(entity, field, registry);
} else {
let is_component = entity
.world()
.components()
.get_id(field.type_id())
.is_some();
let is_component = entity.world().components().get_id(type_id).is_some();

if is_component {
panic!(
Expand Down
20 changes: 10 additions & 10 deletions crates/bevy_ecs/src/reflect/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ use crate::{
FilteredEntityRef, World,
},
};
use bevy_reflect::{FromReflect, FromType, Reflect, TypeRegistry};
use bevy_reflect::{FromReflect, FromType, PartialReflect, Reflect, TypePath, TypeRegistry};

/// A struct used to operate on reflected [`Component`] trait of a type.
///
Expand Down Expand Up @@ -99,11 +99,11 @@ pub struct ReflectComponent(ReflectComponentFns);
#[derive(Clone)]
pub struct ReflectComponentFns {
/// Function pointer implementing [`ReflectComponent::insert()`].
pub insert: fn(&mut EntityWorldMut, &dyn Reflect, &TypeRegistry),
pub insert: fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry),
/// Function pointer implementing [`ReflectComponent::apply()`].
pub apply: fn(EntityMut, &dyn Reflect),
pub apply: fn(EntityMut, &dyn PartialReflect),
/// Function pointer implementing [`ReflectComponent::apply_or_insert()`].
pub apply_or_insert: fn(&mut EntityWorldMut, &dyn Reflect, &TypeRegistry),
pub apply_or_insert: fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry),
/// Function pointer implementing [`ReflectComponent::remove()`].
pub remove: fn(&mut EntityWorldMut),
/// Function pointer implementing [`ReflectComponent::contains()`].
Expand All @@ -127,7 +127,7 @@ impl ReflectComponentFns {
///
/// This is useful if you want to start with the default implementation before overriding some
/// of the functions to create a custom implementation.
pub fn new<T: Component + Reflect + FromReflect>() -> Self {
pub fn new<T: Component + FromReflect + TypePath>() -> Self {
<ReflectComponent as FromType<T>>::from_type().0
}
}
Expand All @@ -137,7 +137,7 @@ impl ReflectComponent {
pub fn insert(
&self,
entity: &mut EntityWorldMut,
component: &dyn Reflect,
component: &dyn PartialReflect,
registry: &TypeRegistry,
) {
(self.0.insert)(entity, component, registry);
Expand All @@ -148,15 +148,15 @@ impl ReflectComponent {
/// # Panics
///
/// Panics if there is no [`Component`] of the given type.
pub fn apply<'a>(&self, entity: impl Into<EntityMut<'a>>, component: &dyn Reflect) {
pub fn apply<'a>(&self, entity: impl Into<EntityMut<'a>>, component: &dyn PartialReflect) {
(self.0.apply)(entity.into(), component);
}

/// Uses reflection to set the value of this [`Component`] type in the entity to the given value or insert a new one if it does not exist.
pub fn apply_or_insert(
&self,
entity: &mut EntityWorldMut,
component: &dyn Reflect,
component: &dyn PartialReflect,
registry: &TypeRegistry,
) {
(self.0.apply_or_insert)(entity, component, registry);
Expand Down Expand Up @@ -256,7 +256,7 @@ impl ReflectComponent {
}
}

impl<C: Component + Reflect> FromType<C> for ReflectComponent {
impl<C: Component + Reflect + TypePath> FromType<C> for ReflectComponent {
fn from_type() -> Self {
ReflectComponent(ReflectComponentFns {
insert: |entity, reflected_component, registry| {
Expand All @@ -271,7 +271,7 @@ impl<C: Component + Reflect> FromType<C> for ReflectComponent {
},
apply_or_insert: |entity, reflected_component, registry| {
if let Some(mut component) = entity.get_mut::<C>() {
component.apply(reflected_component);
component.apply(reflected_component.as_partial_reflect());
} else {
let component = entity.world_scope(|world| {
from_reflect_with_fallback::<C>(reflected_component, world, registry)
Expand Down
28 changes: 14 additions & 14 deletions crates/bevy_ecs/src/reflect/entity_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::reflect::AppTypeRegistry;
use crate::system::{EntityCommands, Resource};
use crate::world::Command;
use crate::{entity::Entity, reflect::ReflectComponent, world::World};
use bevy_reflect::{Reflect, TypeRegistry};
use bevy_reflect::{PartialReflect, TypeRegistry};
use std::borrow::Cow;
use std::marker::PhantomData;

Expand All @@ -18,7 +18,7 @@ pub trait ReflectCommandExt {
///
/// - If the entity doesn't exist.
/// - If [`AppTypeRegistry`] does not have the reflection data for the given [`Component`](crate::component::Component).
/// - If the component data is invalid. See [`Reflect::apply`] for further details.
/// - If the component data is invalid. See [`PartialReflect::apply`] for further details.
/// - If [`AppTypeRegistry`] is not present in the [`World`].
///
/// # Note
Expand Down Expand Up @@ -69,7 +69,7 @@ pub trait ReflectCommandExt {
/// }
///
/// ```
fn insert_reflect(&mut self, component: Box<dyn Reflect>) -> &mut Self;
fn insert_reflect(&mut self, component: Box<dyn PartialReflect>) -> &mut Self;

/// Same as [`insert_reflect`](ReflectCommandExt::insert_reflect), but using the `T` resource as type registry instead of
/// `AppTypeRegistry`.
Expand All @@ -83,7 +83,7 @@ pub trait ReflectCommandExt {
/// - The given [`Resource`] is removed from the [`World`] before the command is applied.
fn insert_reflect_with_registry<T: Resource + AsRef<TypeRegistry>>(
&mut self,
component: Box<dyn Reflect>,
component: Box<dyn PartialReflect>,
) -> &mut Self;

/// Removes from the entity the component with the given type name registered in [`AppTypeRegistry`].
Expand Down Expand Up @@ -142,7 +142,7 @@ pub trait ReflectCommandExt {
}

impl ReflectCommandExt for EntityCommands<'_> {
fn insert_reflect(&mut self, component: Box<dyn Reflect>) -> &mut Self {
fn insert_reflect(&mut self, component: Box<dyn PartialReflect>) -> &mut Self {
self.commands.add(InsertReflect {
entity: self.entity,
component,
Expand All @@ -152,7 +152,7 @@ impl ReflectCommandExt for EntityCommands<'_> {

fn insert_reflect_with_registry<T: Resource + AsRef<TypeRegistry>>(
&mut self,
component: Box<dyn Reflect>,
component: Box<dyn PartialReflect>,
) -> &mut Self {
self.commands.add(InsertReflectWithRegistry::<T> {
entity: self.entity,
Expand Down Expand Up @@ -188,7 +188,7 @@ fn insert_reflect(
world: &mut World,
entity: Entity,
type_registry: &TypeRegistry,
component: Box<dyn Reflect>,
component: Box<dyn PartialReflect>,
) {
let type_info = component
.get_represented_type_info()
Expand All @@ -197,13 +197,13 @@ fn insert_reflect(
let Some(mut entity) = world.get_entity_mut(entity) else {
panic!("error[B0003]: Could not insert a reflected component (of type {type_path}) for entity {entity:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003");
};
let Some(type_registration) = type_registry.get_with_type_path(type_path) else {
let Some(type_registration) = type_registry.get(type_info.type_id()) 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 {type_path}) because it doesn't exist in this TypeRegistration.");
};
reflect_component.insert(&mut entity, &*component, type_registry);
reflect_component.insert(&mut entity, component.as_partial_reflect(), type_registry);
}

/// A [`Command`] that adds the boxed reflect component to an entity using the data in
Expand All @@ -214,7 +214,7 @@ pub struct InsertReflect {
/// The entity on which the component will be inserted.
pub entity: Entity,
/// The reflect [`Component`](crate::component::Component) that will be added to the entity.
pub component: Box<dyn Reflect>,
pub component: Box<dyn PartialReflect>,
}

impl Command for InsertReflect {
Expand All @@ -233,7 +233,7 @@ pub struct InsertReflectWithRegistry<T: Resource + AsRef<TypeRegistry>> {
pub entity: Entity,
pub _t: PhantomData<T>,
/// The reflect [`Component`](crate::component::Component) that will be added to the entity.
pub component: Box<dyn Reflect>,
pub component: Box<dyn PartialReflect>,
}

impl<T: Resource + AsRef<TypeRegistry>> Command for InsertReflectWithRegistry<T> {
Expand Down Expand Up @@ -317,7 +317,7 @@ mod tests {
use crate::system::{Commands, SystemState};
use crate::{self as bevy_ecs, component::Component, world::World};
use bevy_ecs_macros::Resource;
use bevy_reflect::{Reflect, TypeRegistry};
use bevy_reflect::{PartialReflect, Reflect, TypeRegistry};

#[derive(Resource)]
struct TypeRegistryResource {
Expand Down Expand Up @@ -352,7 +352,7 @@ mod tests {
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 = Box::new(ComponentA(916)) as Box<dyn PartialReflect>;
let boxed_reflect_component_a_clone = boxed_reflect_component_a.clone_value();

commands
Expand Down Expand Up @@ -388,7 +388,7 @@ mod tests {

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

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

commands
.entity(entity)
Expand Down
Loading

0 comments on commit 6ab8767

Please sign in to comment.