Make from_reflect_or_world also try ReflectDefault and improve some comments and panic messages (#12499)

# Objective

- `from_reflect_or_world` is an internal utilty used in the
implementations of `ReflectComponent` and `ReflectBundle` to create a
`T` given a `&dyn Reflect` by trying to use `FromReflect`, and if that
fails it falls back to `ReflectFromWorld`
- reflecting `FromWorld` is not intuitive though: often it is implicitly
implemented by deriving `Default` so people might not even be aware of
it.
- the panic messages mentioning `ReflectFromWorld` are not directly
correlated to what the user would have to do (reflect `FromWorld`)

## Solution

- Also check for `ReflectDefault` in addition to `ReflectFromWorld`.
- Change the panic messages to mention the reflected trait rather than
the `Reflect*` types.

---

## Changelog

- `ReflectComponent` and `ReflectBundle` no longer require `T:
FromReflect` but instead only `T: Reflect`.
- `ReflectComponent` and `ReflectBundle` will also work with types that
only reflected `Default` and not `FromWorld`.

## Migration Guide

- `ReflectBundle::insert` now requires an additional `&TypeRegistry`
parameter.
This commit is contained in:
Giacomo Stevanato 2024-04-30 02:48:46 +02:00 committed by GitHub
parent 6c57a16b5e
commit d9b69731de
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 77 additions and 35 deletions

View file

@ -12,7 +12,7 @@ use crate::{
};
use bevy_reflect::{FromReflect, FromType, Reflect, ReflectRef, TypeRegistry};
use super::ReflectComponent;
use super::{from_reflect_with_fallback, ReflectComponent};
/// A struct used to operate on reflected [`Bundle`] trait of a type.
///
@ -27,7 +27,7 @@ pub struct ReflectBundle(ReflectBundleFns);
#[derive(Clone)]
pub struct ReflectBundleFns {
/// Function pointer implementing [`ReflectBundle::insert()`].
pub insert: fn(&mut EntityWorldMut, &dyn Reflect),
pub insert: fn(&mut EntityWorldMut, &dyn Reflect, &TypeRegistry),
/// Function pointer implementing [`ReflectBundle::apply()`].
pub apply: fn(EntityMut, &dyn Reflect, &TypeRegistry),
/// Function pointer implementing [`ReflectBundle::apply_or_insert()`].
@ -49,8 +49,13 @@ impl ReflectBundleFns {
impl ReflectBundle {
/// Insert a reflected [`Bundle`] into the entity like [`insert()`](EntityWorldMut::insert).
pub fn insert(&self, entity: &mut EntityWorldMut, bundle: &dyn Reflect) {
(self.0.insert)(entity, bundle);
pub fn insert(
&self,
entity: &mut EntityWorldMut,
bundle: &dyn Reflect,
registry: &TypeRegistry,
) {
(self.0.insert)(entity, bundle, registry);
}
/// Uses reflection to set the value of this [`Bundle`] type in the entity to the given value.
@ -117,11 +122,13 @@ impl ReflectBundle {
}
}
impl<B: Bundle + Reflect + FromReflect> FromType<B> for ReflectBundle {
impl<B: Bundle + Reflect> FromType<B> for ReflectBundle {
fn from_type() -> Self {
ReflectBundle(ReflectBundleFns {
insert: |entity, reflected_bundle| {
let bundle = B::from_reflect(reflected_bundle).unwrap();
insert: |entity, reflected_bundle, registry| {
let bundle = entity.world_scope(|world| {
from_reflect_with_fallback::<B>(reflected_bundle, world, registry)
});
entity.insert(bundle);
},
apply: |mut entity, reflected_bundle, registry| {

View file

@ -57,7 +57,7 @@
//!
//! [`get_type_registration`]: bevy_reflect::GetTypeRegistration::get_type_registration
use super::from_reflect_or_world;
use super::from_reflect_with_fallback;
use crate::{
change_detection::Mut,
component::Component,
@ -253,12 +253,12 @@ impl ReflectComponent {
}
}
impl<C: Component + Reflect + FromReflect> FromType<C> for ReflectComponent {
impl<C: Component + Reflect> FromType<C> for ReflectComponent {
fn from_type() -> Self {
ReflectComponent(ReflectComponentFns {
insert: |entity, reflected_component, registry| {
let component = entity.world_scope(|world| {
from_reflect_or_world::<C>(reflected_component, world, registry)
from_reflect_with_fallback::<C>(reflected_component, world, registry)
});
entity.insert(component);
},
@ -271,7 +271,7 @@ impl<C: Component + Reflect + FromReflect> FromType<C> for ReflectComponent {
component.apply(reflected_component);
} else {
let component = entity.world_scope(|world| {
from_reflect_or_world::<C>(reflected_component, world, registry)
from_reflect_with_fallback::<C>(reflected_component, world, registry)
});
entity.insert(component);
}
@ -283,7 +283,7 @@ impl<C: Component + Reflect + FromReflect> FromType<C> for ReflectComponent {
copy: |source_world, destination_world, source_entity, destination_entity, registry| {
let source_component = source_world.get::<C>(source_entity).unwrap();
let destination_component =
from_reflect_or_world::<C>(source_component, destination_world, registry);
from_reflect_with_fallback::<C>(source_component, destination_world, registry);
destination_world
.entity_mut(destination_entity)
.insert(destination_component);

View file

@ -5,7 +5,8 @@ use std::ops::{Deref, DerefMut};
use crate as bevy_ecs;
use crate::{system::Resource, world::World};
use bevy_reflect::{FromReflect, Reflect, TypeRegistry, TypeRegistryArc};
use bevy_reflect::std_traits::ReflectDefault;
use bevy_reflect::{Reflect, ReflectFromReflect, TypeRegistry, TypeRegistryArc};
mod bundle;
mod component;
@ -44,35 +45,68 @@ impl DerefMut for AppTypeRegistry {
/// Creates a `T` from a `&dyn Reflect`.
///
/// The first approach uses `T`'s implementation of `FromReflect`.
/// If this fails, it falls back to default-initializing a new instance of `T` using its
/// `ReflectFromWorld` data from the `world`'s `AppTypeRegistry` and `apply`ing the
/// `&dyn Reflect` on it.
/// This will try the following strategies, in this order:
///
/// Panics if both approaches fail.
fn from_reflect_or_world<T: FromReflect>(
/// - use the reflected `FromReflect`, if it's present and doesn't fail;
/// - use the reflected `Default`, if it's present, and then call `apply` on the result;
/// - use the reflected `FromWorld`, just like the `Default`.
///
/// The first one that is present and doesn't fail will be used.
///
/// # Panics
///
/// If any strategy produces a `Box<dyn Reflect>` that doesn't store a value of type `T`
/// this method will panic.
///
/// If none of the strategies succeed, this method will panic.
fn from_reflect_with_fallback<T: Reflect>(
reflected: &dyn Reflect,
world: &mut World,
registry: &TypeRegistry,
) -> T {
if let Some(value) = T::from_reflect(reflected) {
return value;
}
// Clone the `ReflectFromWorld` because it's cheap and "frees"
// the borrow of `world` so that it can be passed to `from_world`.
let Some(reflect_from_world) = registry.get_type_data::<ReflectFromWorld>(TypeId::of::<T>())
else {
fn different_type_error<T>(reflected: &str) -> ! {
panic!(
"`FromReflect` failed and no `ReflectFromWorld` registration found for `{}`",
"The registration for the reflected `{}` trait for the type `{}` produced \
a value of a different type",
reflected,
// FIXME: once we have unique reflect, use `TypePath`.
std::any::type_name::<T>(),
);
};
}
let Ok(mut value) = reflect_from_world.from_world(world).take::<T>() else {
// First, try `FromReflect`. This is handled differently from the others because
// it doesn't need a subsequent `apply` and may fail.
if let Some(reflect_from_reflect) =
registry.get_type_data::<ReflectFromReflect>(TypeId::of::<T>())
{
// If it fails it's ok, we can continue checking `Default` and `FromWorld`.
if let Some(value) = reflect_from_reflect.from_reflect(reflected) {
return value
.take::<T>()
.unwrap_or_else(|_| different_type_error::<T>("FromReflect"));
}
}
// Create an instance of `T` using either the reflected `Default` or `FromWorld`.
let mut value = if let Some(reflect_default) =
registry.get_type_data::<ReflectDefault>(TypeId::of::<T>())
{
reflect_default
.default()
.take::<T>()
.unwrap_or_else(|_| different_type_error::<T>("Default"))
} else if let Some(reflect_from_world) =
registry.get_type_data::<ReflectFromWorld>(TypeId::of::<T>())
{
reflect_from_world
.from_world(world)
.take::<T>()
.unwrap_or_else(|_| different_type_error::<T>("FromWorld"))
} else {
panic!(
"the `ReflectFromWorld` registration for `{}` produced a value of a different type",
"Couldn't create an instance of `{}` using the reflected `FromReflect`, \
`Default` or `FromWorld` traits. Are you perhaps missing a `#[reflect(Default)]` \
or `#[reflect(FromWorld)]`?",
// FIXME: once we have unique reflect, use `TypePath`.
std::any::type_name::<T>(),
);

View file

@ -11,7 +11,7 @@ use crate::{
};
use bevy_reflect::{FromReflect, FromType, Reflect, TypeRegistry};
use super::from_reflect_or_world;
use super::from_reflect_with_fallback;
/// A struct used to operate on reflected [`Resource`] of a type.
///
@ -180,7 +180,7 @@ impl<R: Resource + FromReflect> FromType<R> for ReflectResource {
fn from_type() -> Self {
ReflectResource(ReflectResourceFns {
insert: |world, reflected_resource, registry| {
let resource = from_reflect_or_world::<R>(reflected_resource, world, registry);
let resource = from_reflect_with_fallback::<R>(reflected_resource, world, registry);
world.insert_resource(resource);
},
apply: |world, reflected_resource| {
@ -191,7 +191,8 @@ impl<R: Resource + FromReflect> FromType<R> for ReflectResource {
if let Some(mut resource) = world.get_resource_mut::<R>() {
resource.apply(reflected_resource);
} else {
let resource = from_reflect_or_world::<R>(reflected_resource, world, registry);
let resource =
from_reflect_with_fallback::<R>(reflected_resource, world, registry);
world.insert_resource(resource);
}
},
@ -212,7 +213,7 @@ impl<R: Resource + FromReflect> FromType<R> for ReflectResource {
copy: |source_world, destination_world, registry| {
let source_resource = source_world.resource::<R>();
let destination_resource =
from_reflect_or_world::<R>(source_resource, destination_world, registry);
from_reflect_with_fallback::<R>(source_resource, destination_world, registry);
destination_world.insert_resource(destination_resource);
},
})