Skip to content

Commit 6ab8767

Browse files
soqbalice-i-cecileMrGVSV
authored
reflect: implement the unique reflect rfc (#7207)
# 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]>
1 parent 7b81ae7 commit 6ab8767

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+2163
-1747
lines changed

crates/bevy_asset/src/handle.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,8 @@ pub enum UntypedAssetConversionError {
515515

516516
#[cfg(test)]
517517
mod tests {
518+
use bevy_reflect::PartialReflect;
519+
518520
use super::*;
519521

520522
type TestAsset = ();
@@ -651,7 +653,7 @@ mod tests {
651653
);
652654

653655
let reflected: &dyn Reflect = &handle;
654-
let cloned_handle: Box<dyn Reflect> = reflected.clone_value();
656+
let cloned_handle: Box<dyn PartialReflect> = reflected.clone_value();
655657

656658
assert_eq!(
657659
Arc::strong_count(strong),

crates/bevy_asset/src/reflect.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::any::{Any, TypeId};
22

33
use bevy_ecs::world::{unsafe_world_cell::UnsafeWorldCell, World};
4-
use bevy_reflect::{FromReflect, FromType, Reflect};
4+
use bevy_reflect::{FromReflect, FromType, PartialReflect, Reflect};
55

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

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

9696
/// Equivalent of [`Assets::add`]
97-
pub fn add(&self, world: &mut World, value: &dyn Reflect) -> UntypedHandle {
97+
pub fn add(&self, world: &mut World, value: &dyn PartialReflect) -> UntypedHandle {
9898
(self.add)(world, value)
9999
}
100100
/// Equivalent of [`Assets::insert`]
101-
pub fn insert(&self, world: &mut World, handle: UntypedHandle, value: &dyn Reflect) {
101+
pub fn insert(&self, world: &mut World, handle: UntypedHandle, value: &dyn PartialReflect) {
102102
(self.insert)(world, handle, value);
103103
}
104104

crates/bevy_ecs/src/reflect/bundle.rs

+32-21
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@
44
//! This module exports two types: [`ReflectBundleFns`] and [`ReflectBundle`].
55
//!
66
//! Same as [`super::component`], but for bundles.
7-
use std::any::TypeId;
7+
use std::any::{Any, TypeId};
88

99
use crate::{
1010
prelude::Bundle,
1111
world::{EntityMut, EntityWorldMut},
1212
};
13-
use bevy_reflect::{FromReflect, FromType, Reflect, ReflectRef, TypeRegistry};
13+
use bevy_reflect::{
14+
FromReflect, FromType, PartialReflect, Reflect, ReflectRef, TypePath, TypeRegistry,
15+
};
1416

1517
use super::{from_reflect_with_fallback, ReflectComponent};
1618

@@ -27,11 +29,11 @@ pub struct ReflectBundle(ReflectBundleFns);
2729
#[derive(Clone)]
2830
pub struct ReflectBundleFns {
2931
/// Function pointer implementing [`ReflectBundle::insert()`].
30-
pub insert: fn(&mut EntityWorldMut, &dyn Reflect, &TypeRegistry),
32+
pub insert: fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry),
3133
/// Function pointer implementing [`ReflectBundle::apply()`].
32-
pub apply: fn(EntityMut, &dyn Reflect, &TypeRegistry),
34+
pub apply: fn(EntityMut, &dyn PartialReflect, &TypeRegistry),
3335
/// Function pointer implementing [`ReflectBundle::apply_or_insert()`].
34-
pub apply_or_insert: fn(&mut EntityWorldMut, &dyn Reflect, &TypeRegistry),
36+
pub apply_or_insert: fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry),
3537
/// Function pointer implementing [`ReflectBundle::remove()`].
3638
pub remove: fn(&mut EntityWorldMut),
3739
}
@@ -42,7 +44,7 @@ impl ReflectBundleFns {
4244
///
4345
/// This is useful if you want to start with the default implementation before overriding some
4446
/// of the functions to create a custom implementation.
45-
pub fn new<T: Bundle + Reflect + FromReflect>() -> Self {
47+
pub fn new<T: Bundle + FromReflect + TypePath>() -> Self {
4648
<ReflectBundle as FromType<T>>::from_type().0
4749
}
4850
}
@@ -52,7 +54,7 @@ impl ReflectBundle {
5254
pub fn insert(
5355
&self,
5456
entity: &mut EntityWorldMut,
55-
bundle: &dyn Reflect,
57+
bundle: &dyn PartialReflect,
5658
registry: &TypeRegistry,
5759
) {
5860
(self.0.insert)(entity, bundle, registry);
@@ -66,7 +68,7 @@ impl ReflectBundle {
6668
pub fn apply<'a>(
6769
&self,
6870
entity: impl Into<EntityMut<'a>>,
69-
bundle: &dyn Reflect,
71+
bundle: &dyn PartialReflect,
7072
registry: &TypeRegistry,
7173
) {
7274
(self.0.apply)(entity.into(), bundle, registry);
@@ -76,7 +78,7 @@ impl ReflectBundle {
7678
pub fn apply_or_insert(
7779
&self,
7880
entity: &mut EntityWorldMut,
79-
bundle: &dyn Reflect,
81+
bundle: &dyn PartialReflect,
8082
registry: &TypeRegistry,
8183
) {
8284
(self.0.apply_or_insert)(entity, bundle, registry);
@@ -122,7 +124,7 @@ impl ReflectBundle {
122124
}
123125
}
124126

125-
impl<B: Bundle + Reflect> FromType<B> for ReflectBundle {
127+
impl<B: Bundle + Reflect + TypePath> FromType<B> for ReflectBundle {
126128
fn from_type() -> Self {
127129
ReflectBundle(ReflectBundleFns {
128130
insert: |entity, reflected_bundle, registry| {
@@ -180,10 +182,16 @@ impl<B: Bundle + Reflect> FromType<B> for ReflectBundle {
180182
}
181183
}
182184

183-
fn apply_field(entity: &mut EntityMut, field: &dyn Reflect, registry: &TypeRegistry) {
184-
if let Some(reflect_component) = registry.get_type_data::<ReflectComponent>(field.type_id()) {
185+
fn apply_field(entity: &mut EntityMut, field: &dyn PartialReflect, registry: &TypeRegistry) {
186+
let Some(type_id) = field.try_as_reflect().map(Any::type_id) else {
187+
panic!(
188+
"`{}` did not implement `Reflect`",
189+
field.reflect_type_path()
190+
);
191+
};
192+
if let Some(reflect_component) = registry.get_type_data::<ReflectComponent>(type_id) {
185193
reflect_component.apply(entity.reborrow(), field);
186-
} else if let Some(reflect_bundle) = registry.get_type_data::<ReflectBundle>(field.type_id()) {
194+
} else if let Some(reflect_bundle) = registry.get_type_data::<ReflectBundle>(type_id) {
187195
reflect_bundle.apply(entity.reborrow(), field, registry);
188196
} else {
189197
panic!(
@@ -195,19 +203,22 @@ fn apply_field(entity: &mut EntityMut, field: &dyn Reflect, registry: &TypeRegis
195203

196204
fn apply_or_insert_field(
197205
entity: &mut EntityWorldMut,
198-
field: &dyn Reflect,
206+
field: &dyn PartialReflect,
199207
registry: &TypeRegistry,
200208
) {
201-
if let Some(reflect_component) = registry.get_type_data::<ReflectComponent>(field.type_id()) {
209+
let Some(type_id) = field.try_as_reflect().map(Any::type_id) else {
210+
panic!(
211+
"`{}` did not implement `Reflect`",
212+
field.reflect_type_path()
213+
);
214+
};
215+
216+
if let Some(reflect_component) = registry.get_type_data::<ReflectComponent>(type_id) {
202217
reflect_component.apply_or_insert(entity, field, registry);
203-
} else if let Some(reflect_bundle) = registry.get_type_data::<ReflectBundle>(field.type_id()) {
218+
} else if let Some(reflect_bundle) = registry.get_type_data::<ReflectBundle>(type_id) {
204219
reflect_bundle.apply_or_insert(entity, field, registry);
205220
} else {
206-
let is_component = entity
207-
.world()
208-
.components()
209-
.get_id(field.type_id())
210-
.is_some();
221+
let is_component = entity.world().components().get_id(type_id).is_some();
211222

212223
if is_component {
213224
panic!(

crates/bevy_ecs/src/reflect/component.rs

+10-10
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ use crate::{
6767
FilteredEntityRef, World,
6868
},
6969
};
70-
use bevy_reflect::{FromReflect, FromType, Reflect, TypeRegistry};
70+
use bevy_reflect::{FromReflect, FromType, PartialReflect, Reflect, TypePath, TypeRegistry};
7171

7272
/// A struct used to operate on reflected [`Component`] trait of a type.
7373
///
@@ -99,11 +99,11 @@ pub struct ReflectComponent(ReflectComponentFns);
9999
#[derive(Clone)]
100100
pub struct ReflectComponentFns {
101101
/// Function pointer implementing [`ReflectComponent::insert()`].
102-
pub insert: fn(&mut EntityWorldMut, &dyn Reflect, &TypeRegistry),
102+
pub insert: fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry),
103103
/// Function pointer implementing [`ReflectComponent::apply()`].
104-
pub apply: fn(EntityMut, &dyn Reflect),
104+
pub apply: fn(EntityMut, &dyn PartialReflect),
105105
/// Function pointer implementing [`ReflectComponent::apply_or_insert()`].
106-
pub apply_or_insert: fn(&mut EntityWorldMut, &dyn Reflect, &TypeRegistry),
106+
pub apply_or_insert: fn(&mut EntityWorldMut, &dyn PartialReflect, &TypeRegistry),
107107
/// Function pointer implementing [`ReflectComponent::remove()`].
108108
pub remove: fn(&mut EntityWorldMut),
109109
/// Function pointer implementing [`ReflectComponent::contains()`].
@@ -127,7 +127,7 @@ impl ReflectComponentFns {
127127
///
128128
/// This is useful if you want to start with the default implementation before overriding some
129129
/// of the functions to create a custom implementation.
130-
pub fn new<T: Component + Reflect + FromReflect>() -> Self {
130+
pub fn new<T: Component + FromReflect + TypePath>() -> Self {
131131
<ReflectComponent as FromType<T>>::from_type().0
132132
}
133133
}
@@ -137,7 +137,7 @@ impl ReflectComponent {
137137
pub fn insert(
138138
&self,
139139
entity: &mut EntityWorldMut,
140-
component: &dyn Reflect,
140+
component: &dyn PartialReflect,
141141
registry: &TypeRegistry,
142142
) {
143143
(self.0.insert)(entity, component, registry);
@@ -148,15 +148,15 @@ impl ReflectComponent {
148148
/// # Panics
149149
///
150150
/// Panics if there is no [`Component`] of the given type.
151-
pub fn apply<'a>(&self, entity: impl Into<EntityMut<'a>>, component: &dyn Reflect) {
151+
pub fn apply<'a>(&self, entity: impl Into<EntityMut<'a>>, component: &dyn PartialReflect) {
152152
(self.0.apply)(entity.into(), component);
153153
}
154154

155155
/// 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.
156156
pub fn apply_or_insert(
157157
&self,
158158
entity: &mut EntityWorldMut,
159-
component: &dyn Reflect,
159+
component: &dyn PartialReflect,
160160
registry: &TypeRegistry,
161161
) {
162162
(self.0.apply_or_insert)(entity, component, registry);
@@ -256,7 +256,7 @@ impl ReflectComponent {
256256
}
257257
}
258258

259-
impl<C: Component + Reflect> FromType<C> for ReflectComponent {
259+
impl<C: Component + Reflect + TypePath> FromType<C> for ReflectComponent {
260260
fn from_type() -> Self {
261261
ReflectComponent(ReflectComponentFns {
262262
insert: |entity, reflected_component, registry| {
@@ -271,7 +271,7 @@ impl<C: Component + Reflect> FromType<C> for ReflectComponent {
271271
},
272272
apply_or_insert: |entity, reflected_component, registry| {
273273
if let Some(mut component) = entity.get_mut::<C>() {
274-
component.apply(reflected_component);
274+
component.apply(reflected_component.as_partial_reflect());
275275
} else {
276276
let component = entity.world_scope(|world| {
277277
from_reflect_with_fallback::<C>(reflected_component, world, registry)

crates/bevy_ecs/src/reflect/entity_commands.rs

+14-14
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::reflect::AppTypeRegistry;
33
use crate::system::{EntityCommands, Resource};
44
use crate::world::Command;
55
use crate::{entity::Entity, reflect::ReflectComponent, world::World};
6-
use bevy_reflect::{Reflect, TypeRegistry};
6+
use bevy_reflect::{PartialReflect, TypeRegistry};
77
use std::borrow::Cow;
88
use std::marker::PhantomData;
99

@@ -18,7 +18,7 @@ pub trait ReflectCommandExt {
1818
///
1919
/// - If the entity doesn't exist.
2020
/// - If [`AppTypeRegistry`] does not have the reflection data for the given [`Component`](crate::component::Component).
21-
/// - If the component data is invalid. See [`Reflect::apply`] for further details.
21+
/// - If the component data is invalid. See [`PartialReflect::apply`] for further details.
2222
/// - If [`AppTypeRegistry`] is not present in the [`World`].
2323
///
2424
/// # Note
@@ -69,7 +69,7 @@ pub trait ReflectCommandExt {
6969
/// }
7070
///
7171
/// ```
72-
fn insert_reflect(&mut self, component: Box<dyn Reflect>) -> &mut Self;
72+
fn insert_reflect(&mut self, component: Box<dyn PartialReflect>) -> &mut Self;
7373

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

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

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

153153
fn insert_reflect_with_registry<T: Resource + AsRef<TypeRegistry>>(
154154
&mut self,
155-
component: Box<dyn Reflect>,
155+
component: Box<dyn PartialReflect>,
156156
) -> &mut Self {
157157
self.commands.add(InsertReflectWithRegistry::<T> {
158158
entity: self.entity,
@@ -188,7 +188,7 @@ fn insert_reflect(
188188
world: &mut World,
189189
entity: Entity,
190190
type_registry: &TypeRegistry,
191-
component: Box<dyn Reflect>,
191+
component: Box<dyn PartialReflect>,
192192
) {
193193
let type_info = component
194194
.get_represented_type_info()
@@ -197,13 +197,13 @@ fn insert_reflect(
197197
let Some(mut entity) = world.get_entity_mut(entity) else {
198198
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");
199199
};
200-
let Some(type_registration) = type_registry.get_with_type_path(type_path) else {
200+
let Some(type_registration) = type_registry.get(type_info.type_id()) else {
201201
panic!("Could not get type registration (for component type {type_path}) because it doesn't exist in the TypeRegistry.");
202202
};
203203
let Some(reflect_component) = type_registration.data::<ReflectComponent>() else {
204204
panic!("Could not get ReflectComponent data (for component type {type_path}) because it doesn't exist in this TypeRegistration.");
205205
};
206-
reflect_component.insert(&mut entity, &*component, type_registry);
206+
reflect_component.insert(&mut entity, component.as_partial_reflect(), type_registry);
207207
}
208208

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

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

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

322322
#[derive(Resource)]
323323
struct TypeRegistryResource {
@@ -352,7 +352,7 @@ mod tests {
352352
let entity = commands.spawn_empty().id();
353353
let entity2 = commands.spawn_empty().id();
354354

355-
let boxed_reflect_component_a = Box::new(ComponentA(916)) as Box<dyn Reflect>;
355+
let boxed_reflect_component_a = Box::new(ComponentA(916)) as Box<dyn PartialReflect>;
356356
let boxed_reflect_component_a_clone = boxed_reflect_component_a.clone_value();
357357

358358
commands
@@ -388,7 +388,7 @@ mod tests {
388388

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

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

393393
commands
394394
.entity(entity)

0 commit comments

Comments
 (0)