Replace FromWorld requirement on ReflectResource and reflect `Res…

…ource` for `State<S>` (bevyengine#12136)

# Objective

- In bevyengine#9623 I forgot to change the `FromWorld` requirement for
`ReflectResource`, fix that;
- Fix bevyengine#12129

## Solution

- Use the same approach as in bevyengine#9623 to try using `FromReflect` and
falling back to the `ReflectFromWorld` contained in the `TypeRegistry`
provided
- Just reflect `Resource` on `State<S>` since now that's possible
without introducing new bounds.

---

## Changelog

- `ReflectResource`'s `FromType<T>` implementation no longer requires
`T: FromWorld`, but instead now requires `FromReflect`.
- `ReflectResource::insert`, `ReflectResource::apply_or_insert` and
`ReflectResource::copy` now take an extra `&TypeRegistry` parameter.

## Migration Guide

- Users of `#[reflect(Resource)]` will need to also implement/derive
`FromReflect` (should already be the default).
- Users of `#[reflect(Resource)]` may now want to also add `FromWorld`
to the list of reflected traits in case their `FromReflect`
implementation may fail.
- Users of `ReflectResource` will now need to pass a `&TypeRegistry` to
its `insert`, `apply_or_insert` and `copy` methods.

crates/bevy_ecs/src/reflect/component.rs

@@ -57,9 +57,7 @@
 //!
 //! [`get_type_registration`]: bevy_reflect::GetTypeRegistration::get_type_registration
 
-use std::any::TypeId;
-
-use super::ReflectFromWorld;
+use super::from_reflect_or_world;
 use crate::{
     change_detection::Mut,
     component::Component,
@@ -310,47 +308,3 @@ impl<C: Component + Reflect + FromReflect> FromType<C> for ReflectComponent {
         })
     }
 }
-
-/// 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.
-///
-/// Panics if both approaches fail.
-fn from_reflect_or_world<T: FromReflect>(
-    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 {
-        panic!(
-            "`FromReflect` failed and no `ReflectFromWorld` registration found for `{}`",
-            // FIXME: once we have unique reflect, use `TypePath`.
-            std::any::type_name::<T>(),
-        );
-    };
-
-    let Ok(mut value) = reflect_from_world
-        .from_world(world)
-        .into_any()
-        .downcast::<T>()
-    else {
-        panic!(
-            "the `ReflectFromWorld` registration for `{}` produced a value of a different type",
-            // FIXME: once we have unique reflect, use `TypePath`.
-            std::any::type_name::<T>(),
-        );
-    };
-
-    value.apply(reflected);
-    *value
-}

crates/bevy_ecs/src/reflect/mod.rs

@@ -1,10 +1,11 @@
 //! Types that enable reflection support.
 
+use std::any::TypeId;
 use std::ops::{Deref, DerefMut};
 
 use crate as bevy_ecs;
-use crate::system::Resource;
-use bevy_reflect::TypeRegistryArc;
+use crate::{system::Resource, world::World};
+use bevy_reflect::{FromReflect, Reflect, TypeRegistry, TypeRegistryArc};
 
 mod bundle;
 mod component;
@@ -20,7 +21,7 @@ pub use from_world::{ReflectFromWorld, ReflectFromWorldFns};
 pub use map_entities::ReflectMapEntities;
 pub use resource::{ReflectResource, ReflectResourceFns};
 
-/// A [`Resource`] storing [`TypeRegistry`](bevy_reflect::TypeRegistry) for
+/// A [`Resource`] storing [`TypeRegistry`] for
 /// type registrations relevant to a whole app.
 #[derive(Resource, Clone, Default)]
 pub struct AppTypeRegistry(pub TypeRegistryArc);
@@ -40,3 +41,43 @@ impl DerefMut for AppTypeRegistry {
         &mut self.0
     }
 }
+
+/// 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.
+///
+/// Panics if both approaches fail.
+fn from_reflect_or_world<T: FromReflect>(
+    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 {
+        panic!(
+            "`FromReflect` failed and no `ReflectFromWorld` registration found for `{}`",
+            // 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 {
+        panic!(
+            "the `ReflectFromWorld` registration for `{}` produced a value of a different type",
+            // FIXME: once we have unique reflect, use `TypePath`.
+            std::any::type_name::<T>(),
+        );
+    };
+
+    value.apply(reflected);
+    value
+}

crates/bevy_ecs/src/reflect/resource.rs

@@ -7,9 +7,11 @@
 use crate::{
     change_detection::Mut,
     system::Resource,
-    world::{unsafe_world_cell::UnsafeWorldCell, FromWorld, World},
+    world::{unsafe_world_cell::UnsafeWorldCell, World},
 };
-use bevy_reflect::{FromType, Reflect};
+use bevy_reflect::{FromReflect, FromType, Reflect, TypeRegistry};
+
+use super::from_reflect_or_world;
 
 /// A struct used to operate on reflected [`Resource`] of a type.
 ///
@@ -28,7 +30,7 @@ pub struct ReflectResource(ReflectResourceFns);
 /// > will not need.
 /// > Usually a [`ReflectResource`] is created for a type by deriving [`Reflect`]
 /// > and adding the `#[reflect(Resource)]` attribute.
-/// > After adding the component to the [`TypeRegistry`][bevy_reflect::TypeRegistry],
+/// > After adding the component to the [`TypeRegistry`],
 /// > its [`ReflectResource`] can then be retrieved when needed.
 ///
 /// Creating a custom [`ReflectResource`] may be useful if you need to create new resource types at
@@ -41,11 +43,11 @@ pub struct ReflectResource(ReflectResourceFns);
 #[derive(Clone)]
 pub struct ReflectResourceFns {
     /// Function pointer implementing [`ReflectResource::insert()`].
-    pub insert: fn(&mut World, &dyn Reflect),
+    pub insert: fn(&mut World, &dyn Reflect, &TypeRegistry),
     /// Function pointer implementing [`ReflectResource::apply()`].
     pub apply: fn(&mut World, &dyn Reflect),
     /// Function pointer implementing [`ReflectResource::apply_or_insert()`].
-    pub apply_or_insert: fn(&mut World, &dyn Reflect),
+    pub apply_or_insert: fn(&mut World, &dyn Reflect, &TypeRegistry),
     /// Function pointer implementing [`ReflectResource::remove()`].
     pub remove: fn(&mut World),
     /// Function pointer implementing [`ReflectResource::reflect()`].
@@ -56,7 +58,7 @@ pub struct ReflectResourceFns {
     /// The function may only be called with an [`UnsafeWorldCell`] that can be used to mutably access the relevant resource.
     pub reflect_unchecked_mut: unsafe fn(UnsafeWorldCell<'_>) -> Option<Mut<'_, dyn Reflect>>,
     /// Function pointer implementing [`ReflectResource::copy()`].
-    pub copy: fn(&World, &mut World),
+    pub copy: fn(&World, &mut World, &TypeRegistry),
 }
 
 impl ReflectResourceFns {
@@ -65,15 +67,15 @@ impl ReflectResourceFns {
     ///
     /// 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: Resource + Reflect + FromWorld>() -> Self {
+    pub fn new<T: Resource + FromReflect>() -> Self {
         <ReflectResource as FromType<T>>::from_type().0
     }
 }
 
 impl ReflectResource {
     /// Insert a reflected [`Resource`] into the world like [`insert()`](World::insert_resource).
-    pub fn insert(&self, world: &mut World, resource: &dyn Reflect) {
-        (self.0.insert)(world, resource);
+    pub fn insert(&self, world: &mut World, resource: &dyn Reflect, registry: &TypeRegistry) {
+        (self.0.insert)(world, resource, registry);
     }
 
     /// Uses reflection to set the value of this [`Resource`] type in the world to the given value.
@@ -86,8 +88,13 @@ impl ReflectResource {
     }
 
     /// Uses reflection to set the value of this [`Resource`] type in the world to the given value or insert a new one if it does not exist.
-    pub fn apply_or_insert(&self, world: &mut World, resource: &dyn Reflect) {
-        (self.0.apply_or_insert)(world, resource);
+    pub fn apply_or_insert(
+        &self,
+        world: &mut World,
+        resource: &dyn Reflect,
+        registry: &TypeRegistry,
+    ) {
+        (self.0.apply_or_insert)(world, resource, registry);
     }
 
     /// Removes this [`Resource`] type from the world. Does nothing if it doesn't exist.
@@ -124,8 +131,13 @@ impl ReflectResource {
     /// # Panics
     ///
     /// Panics if there is no [`Resource`] of the given type.
-    pub fn copy(&self, source_world: &World, destination_world: &mut World) {
-        (self.0.copy)(source_world, destination_world);
+    pub fn copy(
+        &self,
+        source_world: &World,
+        destination_world: &mut World,
+        registry: &TypeRegistry,
+    ) {
+        (self.0.copy)(source_world, destination_world, registry);
     }
 
     /// Create a custom implementation of [`ReflectResource`].
@@ -164,45 +176,43 @@ impl ReflectResource {
     }
 }
 
-impl<C: Resource + Reflect + FromWorld> FromType<C> for ReflectResource {
+impl<R: Resource + FromReflect> FromType<R> for ReflectResource {
     fn from_type() -> Self {
         ReflectResource(ReflectResourceFns {
-            insert: |world, reflected_resource| {
-                let mut resource = C::from_world(world);
-                resource.apply(reflected_resource);
+            insert: |world, reflected_resource, registry| {
+                let resource = from_reflect_or_world::<R>(reflected_resource, world, registry);
                 world.insert_resource(resource);
             },
             apply: |world, reflected_resource| {
-                let mut resource = world.resource_mut::<C>();
+                let mut resource = world.resource_mut::<R>();
                 resource.apply(reflected_resource);
             },
-            apply_or_insert: |world, reflected_resource| {
-                if let Some(mut resource) = world.get_resource_mut::<C>() {
+            apply_or_insert: |world, reflected_resource, registry| {
+                if let Some(mut resource) = world.get_resource_mut::<R>() {
                     resource.apply(reflected_resource);
                 } else {
-                    let mut resource = C::from_world(world);
-                    resource.apply(reflected_resource);
+                    let resource = from_reflect_or_world::<R>(reflected_resource, world, registry);
                     world.insert_resource(resource);
                 }
             },
             remove: |world| {
-                world.remove_resource::<C>();
+                world.remove_resource::<R>();
             },
-            reflect: |world| world.get_resource::<C>().map(|res| res as &dyn Reflect),
+            reflect: |world| world.get_resource::<R>().map(|res| res as &dyn Reflect),
             reflect_unchecked_mut: |world| {
                 // SAFETY: all usages of `reflect_unchecked_mut` guarantee that there is either a single mutable
                 // reference or multiple immutable ones alive at any given point
                 unsafe {
-                    world.get_resource_mut::<C>().map(|res| Mut {
+                    world.get_resource_mut::<R>().map(|res| Mut {
                         value: res.value as &mut dyn Reflect,
                         ticks: res.ticks,
                     })
                 }
             },
-            copy: |source_world, destination_world| {
-                let source_resource = source_world.resource::<C>();
-                let mut destination_resource = C::from_world(destination_world);
-                destination_resource.apply(source_resource);
+            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);
                 destination_world.insert_resource(destination_resource);
             },
         })

crates/bevy_ecs/src/schedule/state.rs

@@ -96,7 +96,11 @@ pub struct OnTransition<S: States> {
 /// }
 /// ```
 #[derive(Resource, Debug)]
-#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))]
+#[cfg_attr(
+    feature = "bevy_reflect",
+    derive(bevy_reflect::Reflect),
+    reflect(Resource)
+)]
 pub struct State<S: States>(S);
 
 impl<S: States> State<S> {

crates/bevy_scene/src/dynamic_scene.rs

@@ -90,7 +90,7 @@ impl DynamicScene {
 
             // If the world already contains an instance of the given resource
             // just apply the (possibly) new value, otherwise insert the resource
-            reflect_resource.apply_or_insert(world, &**resource);
+            reflect_resource.apply_or_insert(world, &**resource, &type_registry);
         }
 
         // For each component types that reference other entities, we keep track

crates/bevy_scene/src/scene.rs

@@ -89,7 +89,7 @@ impl Scene {
                     type_path: registration.type_info().type_path().to_string(),
                 }
             })?;
-            reflect_resource.copy(&self.world, world);
+            reflect_resource.copy(&self.world, world, &type_registry);
         }
 
         for archetype in self.world.archetypes().iter() {