Add ReflectFromWorld and replace the FromWorld requirement on ReflectComponent and ReflectBundle with FromReflect (#9623)

# Objective

- `FromType<T>` for `ReflectComponent` and `ReflectBundle` currently
require `T: FromWorld` for two reasons:
    - they include a `from_world` method;
- they create dummy `T`s using `FromWorld` and then `apply` a `&dyn
Reflect` to it to simulate `FromReflect`.
- However `FromWorld`/`Default` may be difficult/weird/impractical to
implement, while `FromReflect` is easier and also more natural for the
job.
- See also
https://discord.com/channels/691052431525675048/1146022009554337792

## Solution

- Split `from_world` from `ReflectComponent` and `ReflectBundle` into
its own `ReflectFromWorld` struct.
- Replace the requirement on `FromWorld` in `ReflectComponent` and
`ReflectBundle` with `FromReflect`

---

## Changelog

- `ReflectComponent` and `ReflectBundle` no longer offer a `from_world`
method.
- `ReflectComponent` and `ReflectBundle`'s `FromType<T>` implementation
no longer requires `T: FromWorld`, but now requires `FromReflect`.
- `ReflectComponent::insert`, `ReflectComponent::apply_or_insert` and
`ReflectComponent::copy` now take an extra `&TypeRegistry` parameter.
- There is now a new `ReflectFromWorld` struct.

## Migration Guide

- Existing uses of `ReflectComponent::from_world` and
`ReflectBundle::from_world` will have to be changed to
`ReflectFromWorld::from_world`.
- Users of `#[reflect(Component)]` and `#[reflect(Bundle)]` will need to
also implement/derive `FromReflect`.
- Users of `#[reflect(Component)]` and `#[reflect(Bundle)]` may now want
to also add `FromWorld` to the list of reflected traits in case their
`FromReflect` implementation may fail.
- Users of `ReflectComponent` will now need to pass a `&TypeRegistry` to
its `insert`, `apply_or_insert` and `copy` methods.

Author: SkiFire13

crates/bevy_ecs/src/lib.rs

@@ -28,7 +28,9 @@ pub use bevy_ptr as ptr;
 pub mod prelude {
     #[doc(hidden)]
     #[cfg(feature = "bevy_reflect")]
-    pub use crate::reflect::{AppTypeRegistry, ReflectComponent, ReflectResource};
+    pub use crate::reflect::{
+        AppTypeRegistry, ReflectComponent, ReflectFromWorld, ReflectResource,
+    };
     #[doc(hidden)]
     pub use crate::{
         bundle::Bundle,

crates/bevy_ecs/src/reflect/bundle.rs

@@ -1,19 +1,17 @@
 //! Definitions for [`Bundle`] reflection.
+//! This allows inserting, updating and/or removing bundles whose type is only known at runtime.
 //!
 //! This module exports two types: [`ReflectBundleFns`] and [`ReflectBundle`].
 //!
 //! Same as [`super::component`], but for bundles.
 use std::any::TypeId;
 
-use crate::{
-    prelude::Bundle,
-    world::{EntityWorldMut, FromWorld, World},
-};
-use bevy_reflect::{FromType, Reflect, ReflectRef, TypeRegistry};
+use crate::{prelude::Bundle, world::EntityWorldMut};
+use bevy_reflect::{FromReflect, FromType, Reflect, ReflectRef, TypeRegistry};
 
 use super::ReflectComponent;
 
-/// A struct used to operate on reflected [`Bundle`] of a type.
+/// A struct used to operate on reflected [`Bundle`] trait of a type.
 ///
 /// A [`ReflectBundle`] for type `T` can be obtained via
 /// [`bevy_reflect::TypeRegistration::data`].
@@ -25,8 +23,6 @@ pub struct ReflectBundle(ReflectBundleFns);
 /// The also [`super::component::ReflectComponentFns`].
 #[derive(Clone)]
 pub struct ReflectBundleFns {
-    /// Function pointer implementing [`ReflectBundle::from_world()`].
-    pub from_world: fn(&mut World) -> Box<dyn Reflect>,
     /// Function pointer implementing [`ReflectBundle::insert()`].
     pub insert: fn(&mut EntityWorldMut, &dyn Reflect),
     /// Function pointer implementing [`ReflectBundle::apply()`].
@@ -43,17 +39,12 @@ 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 + FromWorld>() -> Self {
+    pub fn new<T: Bundle + Reflect + FromReflect>() -> Self {
         <ReflectBundle as FromType<T>>::from_type().0
     }
 }
 
 impl ReflectBundle {
-    /// Constructs default reflected [`Bundle`] from world using [`from_world()`](FromWorld::from_world).
-    pub fn from_world(&self, world: &mut World) -> Box<dyn Reflect> {
-        (self.0.from_world)(world)
-    }
-
     /// 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);
@@ -123,49 +114,53 @@ impl ReflectBundle {
     }
 }
 
-impl<B: Bundle + Reflect + FromWorld> FromType<B> for ReflectBundle {
+impl<B: Bundle + Reflect + FromReflect> FromType<B> for ReflectBundle {
     fn from_type() -> Self {
         ReflectBundle(ReflectBundleFns {
-            from_world: |world| Box::new(B::from_world(world)),
             insert: |entity, reflected_bundle| {
-                let mut bundle = entity.world_scope(|world| B::from_world(world));
-                bundle.apply(reflected_bundle);
+                let bundle = B::from_reflect(reflected_bundle).unwrap();
                 entity.insert(bundle);
             },
             apply: |entity, reflected_bundle, registry| {
-                let mut bundle = entity.world_scope(|world| B::from_world(world));
-                bundle.apply(reflected_bundle);
-
-                match bundle.reflect_ref() {
-                    ReflectRef::Struct(bundle) => bundle
-                        .iter_fields()
-                        .for_each(|field| insert_field::<B>(entity, field, registry)),
-                    ReflectRef::Tuple(bundle) => bundle
-                        .iter_fields()
-                        .for_each(|field| insert_field::<B>(entity, field, registry)),
-                    _ => panic!(
-                        "expected bundle `{}` to be named struct or tuple",
-                        // FIXME: once we have unique reflect, use `TypePath`.
-                        std::any::type_name::<B>(),
-                    ),
+                if let Some(reflect_component) =
+                    registry.get_type_data::<ReflectComponent>(TypeId::of::<B>())
+                {
+                    reflect_component.apply(entity, reflected_bundle);
+                } else {
+                    match reflected_bundle.reflect_ref() {
+                        ReflectRef::Struct(bundle) => bundle
+                            .iter_fields()
+                            .for_each(|field| insert_field(entity, field, registry)),
+                        ReflectRef::Tuple(bundle) => bundle
+                            .iter_fields()
+                            .for_each(|field| insert_field(entity, field, registry)),
+                        _ => panic!(
+                            "expected bundle `{}` to be named struct or tuple",
+                            // FIXME: once we have unique reflect, use `TypePath`.
+                            std::any::type_name::<B>(),
+                        ),
+                    }
                 }
             },
             apply_or_insert: |entity, reflected_bundle, registry| {
-                let mut bundle = entity.world_scope(|world| B::from_world(world));
-                bundle.apply(reflected_bundle);
-
-                match bundle.reflect_ref() {
-                    ReflectRef::Struct(bundle) => bundle
-                        .iter_fields()
-                        .for_each(|field| apply_or_insert_field::<B>(entity, field, registry)),
-                    ReflectRef::Tuple(bundle) => bundle
-                        .iter_fields()
-                        .for_each(|field| apply_or_insert_field::<B>(entity, field, registry)),
-                    _ => panic!(
-                        "expected bundle `{}` to be named struct or tuple",
-                        // FIXME: once we have unique reflect, use `TypePath`.
-                        std::any::type_name::<B>(),
-                    ),
+                if let Some(reflect_component) =
+                    registry.get_type_data::<ReflectComponent>(TypeId::of::<B>())
+                {
+                    reflect_component.apply_or_insert(entity, reflected_bundle, registry);
+                } else {
+                    match reflected_bundle.reflect_ref() {
+                        ReflectRef::Struct(bundle) => bundle
+                            .iter_fields()
+                            .for_each(|field| apply_or_insert_field(entity, field, registry)),
+                        ReflectRef::Tuple(bundle) => bundle
+                            .iter_fields()
+                            .for_each(|field| apply_or_insert_field(entity, field, registry)),
+                        _ => panic!(
+                            "expected bundle `{}` to be named struct or tuple",
+                            // FIXME: once we have unique reflect, use `TypePath`.
+                            std::any::type_name::<B>(),
+                        ),
+                    }
                 }
             },
             remove: |entity| {
@@ -175,54 +170,58 @@ impl<B: Bundle + Reflect + FromWorld> FromType<B> for ReflectBundle {
     }
 }
 
-fn insert_field<B: 'static>(
-    entity: &mut EntityWorldMut,
-    field: &dyn Reflect,
-    registry: &TypeRegistry,
-) {
+fn insert_field(entity: &mut EntityWorldMut, field: &dyn Reflect, registry: &TypeRegistry) {
     if let Some(reflect_component) = registry.get_type_data::<ReflectComponent>(field.type_id()) {
         reflect_component.apply(entity, field);
     } else if let Some(reflect_bundle) = registry.get_type_data::<ReflectBundle>(field.type_id()) {
         reflect_bundle.apply(entity, field, registry);
     } else {
-        entity.world_scope(|world| {
-            if world.components().get_id(TypeId::of::<B>()).is_some() {
-                panic!(
-                    "no `ReflectComponent` registration found for `{}`",
-                    field.reflect_type_path(),
-                );
-            };
-        });
-
-        panic!(
-            "no `ReflectBundle` registration found for `{}`",
-            field.reflect_type_path(),
-        )
+        let is_component = entity
+            .world()
+            .components()
+            .get_id(field.type_id())
+            .is_some();
+
+        if is_component {
+            panic!(
+                "no `ReflectComponent` registration found for `{}`",
+                field.reflect_type_path(),
+            );
+        } else {
+            panic!(
+                "no `ReflectBundle` registration found for `{}`",
+                field.reflect_type_path(),
+            )
+        }
     }
 }
 
-fn apply_or_insert_field<B: 'static>(
+fn apply_or_insert_field(
     entity: &mut EntityWorldMut,
     field: &dyn Reflect,
     registry: &TypeRegistry,
 ) {
     if let Some(reflect_component) = registry.get_type_data::<ReflectComponent>(field.type_id()) {
-        reflect_component.apply_or_insert(entity, field);
+        reflect_component.apply_or_insert(entity, field, registry);
     } else if let Some(reflect_bundle) = registry.get_type_data::<ReflectBundle>(field.type_id()) {
         reflect_bundle.apply_or_insert(entity, field, registry);
     } else {
-        entity.world_scope(|world| {
-            if world.components().get_id(TypeId::of::<B>()).is_some() {
-                panic!(
-                    "no `ReflectComponent` registration found for `{}`",
-                    field.reflect_type_path(),
-                );
-            };
-        });
-
-        panic!(
-            "no `ReflectBundle` registration found for `{}`",
-            field.reflect_type_path(),
-        )
+        let is_component = entity
+            .world()
+            .components()
+            .get_id(field.type_id())
+            .is_some();
+
+        if is_component {
+            panic!(
+                "no `ReflectComponent` registration found for `{}`",
+                field.reflect_type_path(),
+            );
+        } else {
+            panic!(
+                "no `ReflectBundle` registration found for `{}`",
+                field.reflect_type_path(),
+            )
+        }
     }
 }