From 50eb902e91aa02a1ad16cc9c476119787ea297c7 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 21 Jul 2024 15:55:58 +0200 Subject: [PATCH 01/14] WIP --- crates/bevy_ecs/src/world/mod.rs | 69 ++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index f1165daade536..53a9a8fb0b0c1 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2635,6 +2635,75 @@ impl World { .get_mut_by_id(component_id) } } + + /// # Note + /// Requires the feature `bevy_reflect` (included in default features). + #[inline] + #[cfg(feature = "bevy_reflect")] + pub fn get_reflect( + &self, + entity: Entity, + type_id: TypeId, + ) -> Option<&dyn bevy_reflect::Reflect> { + use bevy_reflect::ReflectFromPtr; + + use crate::prelude::AppTypeRegistry; + + let component_id = self.components().get_id(type_id)?; + let comp_ptr = self.get_by_id(entity, component_id)?; + let type_registry = self.get_resource::()?.read(); + let reflect_from_ptr = type_registry.get_type_data::(type_id)?; + + // SAFETY: + // - `comp_ptr` is guaranteed to point to an object of type `type_id` + // - `reflect_from_ptr` was constructed for type `type_id` + // - Assertion that checks this equality is present + unsafe { + assert_eq!( + reflect_from_ptr.type_id(), + type_id, + "Mismatch between Ptr's type_id and ReflectFromPtr's type_id", + ); + + Some(reflect_from_ptr.as_reflect(comp_ptr)) + } + } + + /// # Note + /// Requires the feature `bevy_reflect` (included in default features). + #[inline] + #[cfg(feature = "bevy_reflect")] + pub fn get_reflect_mut( + &mut self, + entity: Entity, + type_id: TypeId, + ) -> Option> { + use bevy_reflect::ReflectFromPtr; + + use crate::prelude::AppTypeRegistry; + + let component_id = self.components().get_id(type_id)?; + let app_type_registry = self.get_resource::()?.clone(); + let type_registry = app_type_registry.read(); + let reflect_from_ptr = type_registry.get_type_data::(type_id)?; + let comp_mut_untyped = self.get_mut_by_id(entity, component_id)?; + + // SAFETY: + // - `comp_mut_untyped` is guaranteed to point to an object of type `type_id` + // - `reflect_from_ptr` was constructed for type `type_id` + // - Assertion that checks this equality is present + let comp_mut_typed = comp_mut_untyped.map_unchanged(|ptr_mut| unsafe { + assert_eq!( + reflect_from_ptr.type_id(), + type_id, + "Mismatch between PtrMut's type_id and ReflectFromPtr's type_id", + ); + + reflect_from_ptr.as_reflect_mut(ptr_mut) + }); + + Some(comp_mut_typed) + } } // Schedule-related methods From 9e0b4f50e45da26fa6ecf11211a9a30c4c033be7 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 21 Jul 2024 16:36:13 +0200 Subject: [PATCH 02/14] wip: Add first test --- crates/bevy_ecs/src/world/mod.rs | 61 +++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 53a9a8fb0b0c1..54172184ceec4 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2682,10 +2682,13 @@ impl World { use crate::prelude::AppTypeRegistry; - let component_id = self.components().get_id(type_id)?; + // little clone() + read() dance so we a) don't keep a borrow of `self` and b) don't drop a + // temporary (from read()) too early. let app_type_registry = self.get_resource::()?.clone(); let type_registry = app_type_registry.read(); let reflect_from_ptr = type_registry.get_type_data::(type_id)?; + + let component_id = self.components().get_id(type_id)?; let comp_mut_untyped = self.get_mut_by_id(entity, component_id)?; // SAFETY: @@ -3401,4 +3404,60 @@ mod tests { ]) .is_err()); } + + #[cfg(all(test, feature = "bevy_reflect"))] + mod reflect_tests { + use std::any::TypeId; + + use bevy_reflect::Reflect; + + use crate::{ + // For bevy_ecs_macros + self as bevy_ecs, + prelude::{AppTypeRegistry, Component, World}, + }; + + #[derive(Component, Reflect)] + struct RFoo; + + #[derive(Component)] + struct Bar; + + #[test] + fn get_component_as_reflect() { + let mut world = World::new(); + world.init_resource::(); + + let app_type_registry = world.get_resource_mut::().unwrap(); + app_type_registry.write().register::(); + + { + let entity_with_rfoo = world.spawn(RFoo).id(); + let comp_reflect = world + .get_reflect(entity_with_rfoo, TypeId::of::()) + .unwrap(); + + assert!(comp_reflect.is::()); + } + + { + let entity_without_rfoo = world.spawn_empty().id(); + let reflect_opt = world.get_reflect(entity_without_rfoo, TypeId::of::()); + + assert!(reflect_opt.is_none()); + } + + { + let entity_with_bar = world.spawn(Bar).id(); + let reflect_opt = world.get_reflect(entity_with_bar, TypeId::of::()); + + assert!(reflect_opt.is_none()); + } + } + + #[test] + fn get_component_as_mut_reflect() { + todo!(); + } + } } From 1dd75178127655a88a1c69a5b906c9dcc0e8d9ec Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 21 Jul 2024 16:43:19 +0200 Subject: [PATCH 03/14] wip: finish tests --- crates/bevy_ecs/src/world/mod.rs | 45 ++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 54172184ceec4..0facc9dbea73d 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -3414,11 +3414,11 @@ mod tests { use crate::{ // For bevy_ecs_macros self as bevy_ecs, - prelude::{AppTypeRegistry, Component, World}, + prelude::{AppTypeRegistry, Component, DetectChanges, World}, }; #[derive(Component, Reflect)] - struct RFoo; + struct RFoo(i32); #[derive(Component)] struct Bar; @@ -3432,10 +3432,10 @@ mod tests { app_type_registry.write().register::(); { - let entity_with_rfoo = world.spawn(RFoo).id(); + let entity_with_rfoo = world.spawn(RFoo(42)).id(); let comp_reflect = world .get_reflect(entity_with_rfoo, TypeId::of::()) - .unwrap(); + .expect("Reflection of RFoo-component failed"); assert!(comp_reflect.is::()); } @@ -3457,7 +3457,42 @@ mod tests { #[test] fn get_component_as_mut_reflect() { - todo!(); + let mut world = World::new(); + world.init_resource::(); + + let app_type_registry = world.get_resource_mut::().unwrap(); + app_type_registry.write().register::(); + + { + let entity_with_rfoo = world.spawn(RFoo(42)).id(); + let mut comp_reflect = world + .get_reflect_mut(entity_with_rfoo, TypeId::of::()) + .expect("Mutable reflection of RFoo-component failed"); + + let comp_rfoo_reflected = comp_reflect + .downcast_mut::() + .expect("Wrong type reflected (expected RFoo)"); + assert_eq!(comp_rfoo_reflected.0, 42); + comp_rfoo_reflected.0 = 1337; + + let rfoo_ref = world.entity(entity_with_rfoo).get_ref::().unwrap(); + assert!(rfoo_ref.is_changed()); + assert_eq!(rfoo_ref.0, 1337); + } + + { + let entity_without_rfoo = world.spawn_empty().id(); + let reflect_opt = world.get_reflect_mut(entity_without_rfoo, TypeId::of::()); + + assert!(reflect_opt.is_none()); + } + + { + let entity_with_bar = world.spawn(Bar).id(); + let reflect_opt = world.get_reflect_mut(entity_with_bar, TypeId::of::()); + + assert!(reflect_opt.is_none()); + } } } } From 4d75d596b217dd9de6c567e61857191f4256fb4a Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 21 Jul 2024 17:58:34 +0200 Subject: [PATCH 04/14] wip: first version of docs, with Example! :) --- crates/bevy_ecs/src/world/mod.rs | 52 +++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 0facc9dbea73d..03fb5e6cce348 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2636,8 +2636,58 @@ impl World { } } + /// Retrieves a reference to the given `entity`'s [`Component`] of the given `type_id` using + /// reflection. + /// + /// Requires [`#[derive(Reflect)`](derive@bevy_reflect::Reflect) on the [`Component`] and + /// `app.register_type::()` to have been called[^note-reflect-impl]. + /// + /// If you want to call this with a [`ComponentId`], see [`World::components()`] and [`Components::get_id()`] to get + /// the corresponding [`TypeId`]. + /// + /// + /// Also see the crate documentation for [`bevy_reflect`] for more information on + /// [`Reflect`] and bevy's reflection capabilities. + /// + /// TODO: Errors + descriptions + /// + /// # Example + /// + /// ``` + /// use bevy_ecs::prelude::*; + /// use bevy_reflect::Reflect; + /// use std::any::TypeId; + /// + /// // define a `Component` and derive `Reflect` for it + /// #[derive(Component, Reflect)] + /// struct MyComponent; + /// + /// // create a `World` for this example + /// let mut world = World::new(); + /// + /// // Note: This is usually handled by `App::register_type()`, but this example can not use `App`. + /// world.init_resource::(); + /// world.get_resource_mut::().unwrap().write().register::(); + /// + /// // spawn an entity with a `MyComponent` + /// let entity = world.spawn(MyComponent).id(); + /// + /// // retrieve a reflected reference to the entity's `MyComponent` + /// let comp_reflected: Option<&dyn Reflect> = world.get_reflect(entity, TypeId::of::()); + /// + /// // make sure it worked + /// assert!(comp_reflected.is_some_and(|reflect| reflect.is::())); + /// ``` + /// /// # Note - /// Requires the feature `bevy_reflect` (included in default features). + /// Requires the `bevy_reflect` feature (included in the default features). + /// + /// [`ReflectFromPtr`]: bevy_reflect::ReflectFromPtr + /// [`TypeData`]: bevy_reflect::TypeData + /// [`Reflect`]: bevy_reflect::Reflect + /// [`App::register_type`]: ../../bevy_app/struct.App.html#method.register_type + /// [^note-reflect-impl]: More specifically: Requires [`TypeData`] for [`ReflectFromPtr`] to be registered for the given `type_id`, + /// which is automatically handled when deriving [`Reflect`] and calling [`App::register_type`]. #[inline] #[cfg(feature = "bevy_reflect")] pub fn get_reflect( From bd4e607fdd6293a756d06d95276f76a66270ecc7 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 21 Jul 2024 18:05:15 +0200 Subject: [PATCH 05/14] wip: Also add docs for `World::get_reflect_mut()` --- crates/bevy_ecs/src/world/mod.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 03fb5e6cce348..b47a38c705c46 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2719,8 +2719,27 @@ impl World { } } + /// Retrieves a mutable reference to the given `entity`'s [`Component`] of the given `type_id` using + /// reflection. + /// + /// Requires [`#[derive(Reflect)`](derive@bevy_reflect::Reflect) on the [`Component`] and + /// `app.register_type::()` to have been called. + /// + /// This is the mutable version of [`World::get_reflect()`], see its docs for more information + /// and an example. + /// + /// Just calling this method does not trigger [change detection](crate::change_detection). + /// + /// # Errors + /// + /// See the documentation for [`World::get_reflect()`]. + /// + /// # Example + /// + /// See the documentation for [`World::get_reflect()`]. + /// /// # Note - /// Requires the feature `bevy_reflect` (included in default features). + /// Requires the feature `bevy_reflect` (included in the default features). #[inline] #[cfg(feature = "bevy_reflect")] pub fn get_reflect_mut( From 2cd3e2fe359d675317a0fad512ebd859b454e66e Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 21 Jul 2024 18:56:46 +0200 Subject: [PATCH 06/14] wip: Adapt World::get_reflect() to return GetComponentReflectError --- crates/bevy_ecs/src/world/error.rs | 54 ++++++++++++++++++++++++++++-- crates/bevy_ecs/src/world/mod.rs | 50 +++++++++++++++++++++------ 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index 326b0310ba15c..35774595ab147 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -1,8 +1,12 @@ -//! Contains error types returned by bevy's schedule. +//! Contains error types returned by methods on [`World`]. +//! +//! [`World`]: crate::world::World + +use std::any::TypeId; use thiserror::Error; -use crate::schedule::InternedScheduleLabel; +use crate::{component::ComponentId, prelude::*, schedule::InternedScheduleLabel}; /// The error type returned by [`World::try_run_schedule`] if the provided schedule does not exist. /// @@ -10,3 +14,49 @@ use crate::schedule::InternedScheduleLabel; #[derive(Error, Debug)] #[error("The schedule with the label {0:?} was not found.")] pub struct TryRunScheduleError(pub InternedScheduleLabel); + +/// The error type returned by [`World::get_reflect`] and [`World::get_reflect_mut`]. +/// +/// [`World::try_run_schedule`]: crate::world::World::try_run_schedule +#[cfg(feature = "bevy_reflect")] +#[derive(Error, Debug)] +pub enum GetComponentReflectError { + /// There is no [`ComponentId`] corresponding to the given [`TypeId`]. + /// + /// Did you call [`App::register_type()`]? + /// + /// [`App::register_type()`]: ../../../bevy_app/struct.App.html#method.register_type + #[error( + "No `ComponentId` corresponding to `TypeId` {0:?} found (did you call App::register_type()?)" + )] + NoCorrespondingComponentId(TypeId), + + /// The given [`Entity`] does not have a [`Component`] corresponding to the given [`TypeId`]. + #[error("The given `Entity` {entity:?} does not have a `{component_name:?}` component (`ComponentId` {component_id:?}, which corresponds to `TypeId` {type_id:?})")] + EntityDoesNotHaveComponent { + /// The given [`Entity`]. + entity: Entity, + /// The given [`TypeId`]. + type_id: TypeId, + /// The [`ComponentId`] correspondig to the given [`TypeId`]. + component_id: ComponentId, + /// The name corresponding to the `Component` with the given [`TypeId`], or `None` + /// if not available. + component_name: Option, + }, + + /// The [`World`] was missing the [`AppTypeRegistry`] resource. + #[error("The `World` was missing the `AppTypeRegistry` resource")] + MissingAppTypeRegistry, + + /// The [`World`]'s [`TypeRegistry`] did not contain [`TypeData`] for [`ReflectFromPtr`] for the given [`TypeId`]. + /// + /// Did you call [`App::register_type()`]? + /// + /// [`TypeData`]: bevy_reflect::TypeData + /// [`TypeRegistry`]: bevy_reflect::TypeRegistry + /// [`ReflectFromPtr`]: bevy_reflect::ReflectFromPtr + /// [`App::register_type()`]: ../../../bevy_app/struct.App.html#method.register_type + #[error("The `World`'s `TypeRegistry` did not contain `TypeData` for `ReflectFromPtr` for the given `TypeId` {0:?} (did you call `App::register_type()`?)")] + MissingReflectFromPtrTypeData(TypeId), +} diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index b47a38c705c46..fe7056b59670f 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2673,10 +2673,10 @@ impl World { /// let entity = world.spawn(MyComponent).id(); /// /// // retrieve a reflected reference to the entity's `MyComponent` - /// let comp_reflected: Option<&dyn Reflect> = world.get_reflect(entity, TypeId::of::()); + /// let comp_reflected: &dyn Reflect = world.get_reflect(entity, TypeId::of::()).unwrap(); /// - /// // make sure it worked - /// assert!(comp_reflected.is_some_and(|reflect| reflect.is::())); + /// // make sure we got the expected type + /// assert!(comp_reflected.is::()); /// ``` /// /// # Note @@ -2694,15 +2694,43 @@ impl World { &self, entity: Entity, type_id: TypeId, - ) -> Option<&dyn bevy_reflect::Reflect> { + ) -> Result<&dyn bevy_reflect::Reflect, error::GetComponentReflectError> { use bevy_reflect::ReflectFromPtr; use crate::prelude::AppTypeRegistry; - let component_id = self.components().get_id(type_id)?; - let comp_ptr = self.get_by_id(entity, component_id)?; - let type_registry = self.get_resource::()?.read(); - let reflect_from_ptr = type_registry.get_type_data::(type_id)?; + use error::GetComponentReflectError; + + let Some(component_id) = self.components().get_id(type_id) else { + return Err(GetComponentReflectError::NoCorrespondingComponentId( + type_id, + )); + }; + + let Some(comp_ptr) = self.get_by_id(entity, component_id) else { + let component_name = self + .components() + .get_name(component_id) + .map(ToString::to_string); + + return Err(GetComponentReflectError::EntityDoesNotHaveComponent { + entity, + type_id, + component_id, + component_name, + }); + }; + + let Some(type_registry) = self.get_resource::().map(|atr| atr.read()) + else { + return Err(GetComponentReflectError::MissingAppTypeRegistry); + }; + + let Some(reflect_from_ptr) = type_registry.get_type_data::(type_id) else { + return Err(GetComponentReflectError::MissingReflectFromPtrTypeData( + type_id, + )); + }; // SAFETY: // - `comp_ptr` is guaranteed to point to an object of type `type_id` @@ -2715,7 +2743,7 @@ impl World { "Mismatch between Ptr's type_id and ReflectFromPtr's type_id", ); - Some(reflect_from_ptr.as_reflect(comp_ptr)) + Ok(reflect_from_ptr.as_reflect(comp_ptr)) } } @@ -3513,14 +3541,14 @@ mod tests { let entity_without_rfoo = world.spawn_empty().id(); let reflect_opt = world.get_reflect(entity_without_rfoo, TypeId::of::()); - assert!(reflect_opt.is_none()); + assert!(reflect_opt.is_err()); } { let entity_with_bar = world.spawn(Bar).id(); let reflect_opt = world.get_reflect(entity_with_bar, TypeId::of::()); - assert!(reflect_opt.is_none()); + assert!(reflect_opt.is_err()); } } From 5210a30e7b28d3de2a29d2e742dd19e22f3d03d0 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 21 Jul 2024 19:19:54 +0200 Subject: [PATCH 07/14] wip: Add error-returns + error-docs to `World::get_reflect{_mut}()` --- crates/bevy_ecs/src/world/mod.rs | 57 ++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index fe7056b59670f..d17c6bc45906e 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2649,7 +2649,9 @@ impl World { /// Also see the crate documentation for [`bevy_reflect`] for more information on /// [`Reflect`] and bevy's reflection capabilities. /// - /// TODO: Errors + descriptions + /// # Errors + /// + /// See [`GetComponentReflectError`](error::GetComponentReflectError) for the possible errors and their descriptions. /// /// # Example /// @@ -2760,7 +2762,7 @@ impl World { /// /// # Errors /// - /// See the documentation for [`World::get_reflect()`]. + /// See [`GetComponentReflectError`](error::GetComponentReflectError) for the possible errors and their descriptions. /// /// # Example /// @@ -2774,19 +2776,54 @@ impl World { &mut self, entity: Entity, type_id: TypeId, - ) -> Option> { + ) -> Result, error::GetComponentReflectError> { use bevy_reflect::ReflectFromPtr; use crate::prelude::AppTypeRegistry; + use error::GetComponentReflectError; + // little clone() + read() dance so we a) don't keep a borrow of `self` and b) don't drop a // temporary (from read()) too early. - let app_type_registry = self.get_resource::()?.clone(); + let Some(app_type_registry) = self.get_resource::().cloned() else { + return Err(GetComponentReflectError::MissingAppTypeRegistry); + }; let type_registry = app_type_registry.read(); - let reflect_from_ptr = type_registry.get_type_data::(type_id)?; - let component_id = self.components().get_id(type_id)?; - let comp_mut_untyped = self.get_mut_by_id(entity, component_id)?; + let Some(reflect_from_ptr) = type_registry.get_type_data::(type_id) else { + return Err(GetComponentReflectError::MissingReflectFromPtrTypeData( + type_id, + )); + }; + + let Some(component_id) = self.components().get_id(type_id) else { + return Err(GetComponentReflectError::NoCorrespondingComponentId( + type_id, + )); + }; + + // HACK: Only required for the `None`-case/`else`-branch, but it borrows `self`, which will + // already be borrowed by `self.get_mut_by_id()`, and I didn't find a way around it. + let component_name = self + .components() + .get_name(component_id) + .map(ToString::to_string); + + let Some(comp_mut_untyped) = self.get_mut_by_id(entity, component_id) else { + return Err(GetComponentReflectError::EntityDoesNotHaveComponent { + entity, + type_id, + component_id, + component_name, + }); + }; + + // let app_type_registry = self.get_resource::()?.clone(); + // let type_registry = app_type_registry.read(); + // let reflect_from_ptr = type_registry.get_type_data::(type_id)?; + // + // let component_id = self.components().get_id(type_id)?; + // let comp_mut_untyped = self.get_mut_by_id(entity, component_id)?; // SAFETY: // - `comp_mut_untyped` is guaranteed to point to an object of type `type_id` @@ -2802,7 +2839,7 @@ impl World { reflect_from_ptr.as_reflect_mut(ptr_mut) }); - Some(comp_mut_typed) + Ok(comp_mut_typed) } } @@ -3581,14 +3618,14 @@ mod tests { let entity_without_rfoo = world.spawn_empty().id(); let reflect_opt = world.get_reflect_mut(entity_without_rfoo, TypeId::of::()); - assert!(reflect_opt.is_none()); + assert!(reflect_opt.is_err()); } { let entity_with_bar = world.spawn(Bar).id(); let reflect_opt = world.get_reflect_mut(entity_with_bar, TypeId::of::()); - assert!(reflect_opt.is_none()); + assert!(reflect_opt.is_err()); } } } From 5f8e5713554481cd5ab85e7beff6dc13fd88eace Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Sun, 21 Jul 2024 19:41:45 +0200 Subject: [PATCH 08/14] fix: Fix typo --- crates/bevy_ecs/src/world/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index 35774595ab147..bbf23d446173c 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -38,7 +38,7 @@ pub enum GetComponentReflectError { entity: Entity, /// The given [`TypeId`]. type_id: TypeId, - /// The [`ComponentId`] correspondig to the given [`TypeId`]. + /// The [`ComponentId`] corresponding to the given [`TypeId`]. component_id: ComponentId, /// The name corresponding to the `Component` with the given [`TypeId`], or `None` /// if not available. From 2d5a09b7edcb7000be64bd110d71dceaaeaf005e Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Tue, 23 Jul 2024 11:10:22 +0200 Subject: [PATCH 09/14] feedback: From PR Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_ecs/src/world/mod.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d17c6bc45906e..85d25d6ef34c1 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2667,7 +2667,7 @@ impl World { /// // create a `World` for this example /// let mut world = World::new(); /// - /// // Note: This is usually handled by `App::register_type()`, but this example can not use `App`. + /// // Note: This is usually handled by `App::register_type()`, but this example cannot use `App`. /// world.init_resource::(); /// world.get_resource_mut::().unwrap().write().register::(); /// @@ -2818,12 +2818,6 @@ impl World { }); }; - // let app_type_registry = self.get_resource::()?.clone(); - // let type_registry = app_type_registry.read(); - // let reflect_from_ptr = type_registry.get_type_data::(type_id)?; - // - // let component_id = self.components().get_id(type_id)?; - // let comp_mut_untyped = self.get_mut_by_id(entity, component_id)?; // SAFETY: // - `comp_mut_untyped` is guaranteed to point to an object of type `type_id` From 242c5ea217a036e7d591ab89d93849117a23d0b3 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Tue, 23 Jul 2024 11:32:22 +0200 Subject: [PATCH 10/14] docs: Feedback from PR --- crates/bevy_ecs/src/world/error.rs | 26 ++++++++++++++------------ crates/bevy_ecs/src/world/mod.rs | 20 ++++++++++---------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index bbf23d446173c..f1f354c73d4c8 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -16,23 +16,22 @@ use crate::{component::ComponentId, prelude::*, schedule::InternedScheduleLabel} pub struct TryRunScheduleError(pub InternedScheduleLabel); /// The error type returned by [`World::get_reflect`] and [`World::get_reflect_mut`]. -/// -/// [`World::try_run_schedule`]: crate::world::World::try_run_schedule #[cfg(feature = "bevy_reflect")] #[derive(Error, Debug)] pub enum GetComponentReflectError { /// There is no [`ComponentId`] corresponding to the given [`TypeId`]. /// - /// Did you call [`App::register_type()`]? + /// This is usually handled by calling [`App::register_type`] for the type corresponding to + /// the given [`TypeId`]. /// - /// [`App::register_type()`]: ../../../bevy_app/struct.App.html#method.register_type - #[error( - "No `ComponentId` corresponding to `TypeId` {0:?} found (did you call App::register_type()?)" - )] + /// See the documentation for [`bevy_reflect`] for more information. + /// + /// [`App::register_type`]: ../../../bevy_app/struct.App.html#method.register_type + #[error("No `ComponentId` corresponding to {0:?} found (did you call App::register_type()?)")] NoCorrespondingComponentId(TypeId), /// The given [`Entity`] does not have a [`Component`] corresponding to the given [`TypeId`]. - #[error("The given `Entity` {entity:?} does not have a `{component_name:?}` component (`ComponentId` {component_id:?}, which corresponds to `TypeId` {type_id:?})")] + #[error("The given `Entity` {entity:?} does not have a `{component_name:?}` component ({component_id:?}, which corresponds to {type_id:?})")] EntityDoesNotHaveComponent { /// The given [`Entity`]. entity: Entity, @@ -40,7 +39,7 @@ pub enum GetComponentReflectError { type_id: TypeId, /// The [`ComponentId`] corresponding to the given [`TypeId`]. component_id: ComponentId, - /// The name corresponding to the `Component` with the given [`TypeId`], or `None` + /// The name corresponding to the [`Component`] with the given [`TypeId`], or `None` /// if not available. component_name: Option, }, @@ -51,12 +50,15 @@ pub enum GetComponentReflectError { /// The [`World`]'s [`TypeRegistry`] did not contain [`TypeData`] for [`ReflectFromPtr`] for the given [`TypeId`]. /// - /// Did you call [`App::register_type()`]? + /// This is usually handled by calling [`App::register_type`] for the type corresponding to + /// the given [`TypeId`]. + /// + /// See the documentation for [`bevy_reflect`] for more information. /// /// [`TypeData`]: bevy_reflect::TypeData /// [`TypeRegistry`]: bevy_reflect::TypeRegistry /// [`ReflectFromPtr`]: bevy_reflect::ReflectFromPtr - /// [`App::register_type()`]: ../../../bevy_app/struct.App.html#method.register_type - #[error("The `World`'s `TypeRegistry` did not contain `TypeData` for `ReflectFromPtr` for the given `TypeId` {0:?} (did you call `App::register_type()`?)")] + /// [`App::register_type`]: ../../../bevy_app/struct.App.html#method.register_type + #[error("The `World`'s `TypeRegistry` did not contain `TypeData` for `ReflectFromPtr` for the given {0:?} (did you call `App::register_type()`?)")] MissingReflectFromPtrTypeData(TypeId), } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 85d25d6ef34c1..6c34b0eafd3a7 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2639,13 +2639,12 @@ impl World { /// Retrieves a reference to the given `entity`'s [`Component`] of the given `type_id` using /// reflection. /// - /// Requires [`#[derive(Reflect)`](derive@bevy_reflect::Reflect) on the [`Component`] and - /// `app.register_type::()` to have been called[^note-reflect-impl]. + /// Requires implementing [`Reflect`] for the [`Component`] (e.g., using [`#[derive(Reflect)`](derive@bevy_reflect::Reflect)) + /// and `app.register_type::()` to have been called[^note-reflect-impl]. /// - /// If you want to call this with a [`ComponentId`], see [`World::components()`] and [`Components::get_id()`] to get + /// If you want to call this with a [`ComponentId`], see [`World::components`] and [`Components::get_id`] to get /// the corresponding [`TypeId`]. /// - /// /// Also see the crate documentation for [`bevy_reflect`] for more information on /// [`Reflect`] and bevy's reflection capabilities. /// @@ -2752,10 +2751,10 @@ impl World { /// Retrieves a mutable reference to the given `entity`'s [`Component`] of the given `type_id` using /// reflection. /// - /// Requires [`#[derive(Reflect)`](derive@bevy_reflect::Reflect) on the [`Component`] and - /// `app.register_type::()` to have been called. + /// Requires implementing [`Reflect`] for the [`Component`] (e.g., using [`#[derive(Reflect)`](derive@bevy_reflect::Reflect)) + /// and `app.register_type::()` to have been called. /// - /// This is the mutable version of [`World::get_reflect()`], see its docs for more information + /// This is the mutable version of [`World::get_reflect`], see its docs for more information /// and an example. /// /// Just calling this method does not trigger [change detection](crate::change_detection). @@ -2766,10 +2765,12 @@ impl World { /// /// # Example /// - /// See the documentation for [`World::get_reflect()`]. + /// See the documentation for [`World::get_reflect`]. /// /// # Note /// Requires the feature `bevy_reflect` (included in the default features). + /// + /// [`Reflect`]: bevy_reflect::Reflect #[inline] #[cfg(feature = "bevy_reflect")] pub fn get_reflect_mut( @@ -2803,7 +2804,7 @@ impl World { }; // HACK: Only required for the `None`-case/`else`-branch, but it borrows `self`, which will - // already be borrowed by `self.get_mut_by_id()`, and I didn't find a way around it. + // already be mutablyy borrowed by `self.get_mut_by_id()`, and I didn't find a way around it. let component_name = self .components() .get_name(component_id) @@ -2818,7 +2819,6 @@ impl World { }); }; - // SAFETY: // - `comp_mut_untyped` is guaranteed to point to an object of type `type_id` // - `reflect_from_ptr` was constructed for type `type_id` From bb54563144a80ea955a8affa166584cf5b2d7988 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Tue, 23 Jul 2024 12:02:51 +0200 Subject: [PATCH 11/14] refactor: Move `World::get_reflect{_mut}` to new module `world::reflect` Feedback from PR --- crates/bevy_ecs/src/world/error.rs | 52 +----- crates/bevy_ecs/src/world/mod.rs | 203 +-------------------- crates/bevy_ecs/src/world/reflect.rs | 253 +++++++++++++++++++++++++++ 3 files changed, 257 insertions(+), 251 deletions(-) create mode 100644 crates/bevy_ecs/src/world/reflect.rs diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index f1f354c73d4c8..c53dd188719ad 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -2,11 +2,9 @@ //! //! [`World`]: crate::world::World -use std::any::TypeId; - use thiserror::Error; -use crate::{component::ComponentId, prelude::*, schedule::InternedScheduleLabel}; +use crate::schedule::InternedScheduleLabel; /// The error type returned by [`World::try_run_schedule`] if the provided schedule does not exist. /// @@ -14,51 +12,3 @@ use crate::{component::ComponentId, prelude::*, schedule::InternedScheduleLabel} #[derive(Error, Debug)] #[error("The schedule with the label {0:?} was not found.")] pub struct TryRunScheduleError(pub InternedScheduleLabel); - -/// The error type returned by [`World::get_reflect`] and [`World::get_reflect_mut`]. -#[cfg(feature = "bevy_reflect")] -#[derive(Error, Debug)] -pub enum GetComponentReflectError { - /// There is no [`ComponentId`] corresponding to the given [`TypeId`]. - /// - /// This is usually handled by calling [`App::register_type`] for the type corresponding to - /// the given [`TypeId`]. - /// - /// See the documentation for [`bevy_reflect`] for more information. - /// - /// [`App::register_type`]: ../../../bevy_app/struct.App.html#method.register_type - #[error("No `ComponentId` corresponding to {0:?} found (did you call App::register_type()?)")] - NoCorrespondingComponentId(TypeId), - - /// The given [`Entity`] does not have a [`Component`] corresponding to the given [`TypeId`]. - #[error("The given `Entity` {entity:?} does not have a `{component_name:?}` component ({component_id:?}, which corresponds to {type_id:?})")] - EntityDoesNotHaveComponent { - /// The given [`Entity`]. - entity: Entity, - /// The given [`TypeId`]. - type_id: TypeId, - /// The [`ComponentId`] corresponding to the given [`TypeId`]. - component_id: ComponentId, - /// The name corresponding to the [`Component`] with the given [`TypeId`], or `None` - /// if not available. - component_name: Option, - }, - - /// The [`World`] was missing the [`AppTypeRegistry`] resource. - #[error("The `World` was missing the `AppTypeRegistry` resource")] - MissingAppTypeRegistry, - - /// The [`World`]'s [`TypeRegistry`] did not contain [`TypeData`] for [`ReflectFromPtr`] for the given [`TypeId`]. - /// - /// This is usually handled by calling [`App::register_type`] for the type corresponding to - /// the given [`TypeId`]. - /// - /// See the documentation for [`bevy_reflect`] for more information. - /// - /// [`TypeData`]: bevy_reflect::TypeData - /// [`TypeRegistry`]: bevy_reflect::TypeRegistry - /// [`ReflectFromPtr`]: bevy_reflect::ReflectFromPtr - /// [`App::register_type`]: ../../../bevy_app/struct.App.html#method.register_type - #[error("The `World`'s `TypeRegistry` did not contain `TypeData` for `ReflectFromPtr` for the given {0:?} (did you call `App::register_type()`?)")] - MissingReflectFromPtrTypeData(TypeId), -} diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 6c34b0eafd3a7..a56129f3ecfa2 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -9,6 +9,9 @@ mod identifier; mod spawn_batch; pub mod unsafe_world_cell; +#[cfg(feature = "bevy_reflect")] +pub mod reflect; + pub use crate::{ change_detection::{Mut, Ref, CHECK_TICK_THRESHOLD}, world::command_queue::CommandQueue, @@ -2635,206 +2638,6 @@ impl World { .get_mut_by_id(component_id) } } - - /// Retrieves a reference to the given `entity`'s [`Component`] of the given `type_id` using - /// reflection. - /// - /// Requires implementing [`Reflect`] for the [`Component`] (e.g., using [`#[derive(Reflect)`](derive@bevy_reflect::Reflect)) - /// and `app.register_type::()` to have been called[^note-reflect-impl]. - /// - /// If you want to call this with a [`ComponentId`], see [`World::components`] and [`Components::get_id`] to get - /// the corresponding [`TypeId`]. - /// - /// Also see the crate documentation for [`bevy_reflect`] for more information on - /// [`Reflect`] and bevy's reflection capabilities. - /// - /// # Errors - /// - /// See [`GetComponentReflectError`](error::GetComponentReflectError) for the possible errors and their descriptions. - /// - /// # Example - /// - /// ``` - /// use bevy_ecs::prelude::*; - /// use bevy_reflect::Reflect; - /// use std::any::TypeId; - /// - /// // define a `Component` and derive `Reflect` for it - /// #[derive(Component, Reflect)] - /// struct MyComponent; - /// - /// // create a `World` for this example - /// let mut world = World::new(); - /// - /// // Note: This is usually handled by `App::register_type()`, but this example cannot use `App`. - /// world.init_resource::(); - /// world.get_resource_mut::().unwrap().write().register::(); - /// - /// // spawn an entity with a `MyComponent` - /// let entity = world.spawn(MyComponent).id(); - /// - /// // retrieve a reflected reference to the entity's `MyComponent` - /// let comp_reflected: &dyn Reflect = world.get_reflect(entity, TypeId::of::()).unwrap(); - /// - /// // make sure we got the expected type - /// assert!(comp_reflected.is::()); - /// ``` - /// - /// # Note - /// Requires the `bevy_reflect` feature (included in the default features). - /// - /// [`ReflectFromPtr`]: bevy_reflect::ReflectFromPtr - /// [`TypeData`]: bevy_reflect::TypeData - /// [`Reflect`]: bevy_reflect::Reflect - /// [`App::register_type`]: ../../bevy_app/struct.App.html#method.register_type - /// [^note-reflect-impl]: More specifically: Requires [`TypeData`] for [`ReflectFromPtr`] to be registered for the given `type_id`, - /// which is automatically handled when deriving [`Reflect`] and calling [`App::register_type`]. - #[inline] - #[cfg(feature = "bevy_reflect")] - pub fn get_reflect( - &self, - entity: Entity, - type_id: TypeId, - ) -> Result<&dyn bevy_reflect::Reflect, error::GetComponentReflectError> { - use bevy_reflect::ReflectFromPtr; - - use crate::prelude::AppTypeRegistry; - - use error::GetComponentReflectError; - - let Some(component_id) = self.components().get_id(type_id) else { - return Err(GetComponentReflectError::NoCorrespondingComponentId( - type_id, - )); - }; - - let Some(comp_ptr) = self.get_by_id(entity, component_id) else { - let component_name = self - .components() - .get_name(component_id) - .map(ToString::to_string); - - return Err(GetComponentReflectError::EntityDoesNotHaveComponent { - entity, - type_id, - component_id, - component_name, - }); - }; - - let Some(type_registry) = self.get_resource::().map(|atr| atr.read()) - else { - return Err(GetComponentReflectError::MissingAppTypeRegistry); - }; - - let Some(reflect_from_ptr) = type_registry.get_type_data::(type_id) else { - return Err(GetComponentReflectError::MissingReflectFromPtrTypeData( - type_id, - )); - }; - - // SAFETY: - // - `comp_ptr` is guaranteed to point to an object of type `type_id` - // - `reflect_from_ptr` was constructed for type `type_id` - // - Assertion that checks this equality is present - unsafe { - assert_eq!( - reflect_from_ptr.type_id(), - type_id, - "Mismatch between Ptr's type_id and ReflectFromPtr's type_id", - ); - - Ok(reflect_from_ptr.as_reflect(comp_ptr)) - } - } - - /// Retrieves a mutable reference to the given `entity`'s [`Component`] of the given `type_id` using - /// reflection. - /// - /// Requires implementing [`Reflect`] for the [`Component`] (e.g., using [`#[derive(Reflect)`](derive@bevy_reflect::Reflect)) - /// and `app.register_type::()` to have been called. - /// - /// This is the mutable version of [`World::get_reflect`], see its docs for more information - /// and an example. - /// - /// Just calling this method does not trigger [change detection](crate::change_detection). - /// - /// # Errors - /// - /// See [`GetComponentReflectError`](error::GetComponentReflectError) for the possible errors and their descriptions. - /// - /// # Example - /// - /// See the documentation for [`World::get_reflect`]. - /// - /// # Note - /// Requires the feature `bevy_reflect` (included in the default features). - /// - /// [`Reflect`]: bevy_reflect::Reflect - #[inline] - #[cfg(feature = "bevy_reflect")] - pub fn get_reflect_mut( - &mut self, - entity: Entity, - type_id: TypeId, - ) -> Result, error::GetComponentReflectError> { - use bevy_reflect::ReflectFromPtr; - - use crate::prelude::AppTypeRegistry; - - use error::GetComponentReflectError; - - // little clone() + read() dance so we a) don't keep a borrow of `self` and b) don't drop a - // temporary (from read()) too early. - let Some(app_type_registry) = self.get_resource::().cloned() else { - return Err(GetComponentReflectError::MissingAppTypeRegistry); - }; - let type_registry = app_type_registry.read(); - - let Some(reflect_from_ptr) = type_registry.get_type_data::(type_id) else { - return Err(GetComponentReflectError::MissingReflectFromPtrTypeData( - type_id, - )); - }; - - let Some(component_id) = self.components().get_id(type_id) else { - return Err(GetComponentReflectError::NoCorrespondingComponentId( - type_id, - )); - }; - - // HACK: Only required for the `None`-case/`else`-branch, but it borrows `self`, which will - // already be mutablyy borrowed by `self.get_mut_by_id()`, and I didn't find a way around it. - let component_name = self - .components() - .get_name(component_id) - .map(ToString::to_string); - - let Some(comp_mut_untyped) = self.get_mut_by_id(entity, component_id) else { - return Err(GetComponentReflectError::EntityDoesNotHaveComponent { - entity, - type_id, - component_id, - component_name, - }); - }; - - // SAFETY: - // - `comp_mut_untyped` is guaranteed to point to an object of type `type_id` - // - `reflect_from_ptr` was constructed for type `type_id` - // - Assertion that checks this equality is present - let comp_mut_typed = comp_mut_untyped.map_unchanged(|ptr_mut| unsafe { - assert_eq!( - reflect_from_ptr.type_id(), - type_id, - "Mismatch between PtrMut's type_id and ReflectFromPtr's type_id", - ); - - reflect_from_ptr.as_reflect_mut(ptr_mut) - }); - - Ok(comp_mut_typed) - } } // Schedule-related methods diff --git a/crates/bevy_ecs/src/world/reflect.rs b/crates/bevy_ecs/src/world/reflect.rs new file mode 100644 index 0000000000000..7f08173733aac --- /dev/null +++ b/crates/bevy_ecs/src/world/reflect.rs @@ -0,0 +1,253 @@ +//! Provides additional functionality for [`World`] when the `bevy_reflect` feature is enabled. + +use crate::prelude::*; +use crate::world::ComponentId; +use core::any::TypeId; + +use thiserror::Error; + +impl World { + /// Retrieves a reference to the given `entity`'s [`Component`] of the given `type_id` using + /// reflection. + /// + /// Requires implementing [`Reflect`] for the [`Component`] (e.g., using [`#[derive(Reflect)`](derive@bevy_reflect::Reflect)) + /// and `app.register_type::()` to have been called[^note-reflect-impl]. + /// + /// If you want to call this with a [`ComponentId`], see [`World::components`] and [`Components::get_id`] to get + /// the corresponding [`TypeId`]. + /// + /// Also see the crate documentation for [`bevy_reflect`] for more information on + /// [`Reflect`] and bevy's reflection capabilities. + /// + /// # Errors + /// + /// See [`GetComponentReflectError`] for the possible errors and their descriptions. + /// + /// # Example + /// + /// ``` + /// use bevy_ecs::prelude::*; + /// use bevy_reflect::Reflect; + /// use std::any::TypeId; + /// + /// // define a `Component` and derive `Reflect` for it + /// #[derive(Component, Reflect)] + /// struct MyComponent; + /// + /// // create a `World` for this example + /// let mut world = World::new(); + /// + /// // Note: This is usually handled by `App::register_type()`, but this example cannot use `App`. + /// world.init_resource::(); + /// world.get_resource_mut::().unwrap().write().register::(); + /// + /// // spawn an entity with a `MyComponent` + /// let entity = world.spawn(MyComponent).id(); + /// + /// // retrieve a reflected reference to the entity's `MyComponent` + /// let comp_reflected: &dyn Reflect = world.get_reflect(entity, TypeId::of::()).unwrap(); + /// + /// // make sure we got the expected type + /// assert!(comp_reflected.is::()); + /// ``` + /// + /// # Note + /// Requires the `bevy_reflect` feature (included in the default features). + /// + /// [`Components::get_id`]: crate::component::Components::get_id + /// [`ReflectFromPtr`]: bevy_reflect::ReflectFromPtr + /// [`TypeData`]: bevy_reflect::TypeData + /// [`Reflect`]: bevy_reflect::Reflect + /// [`App::register_type`]: ../../bevy_app/struct.App.html#method.register_type + /// [^note-reflect-impl]: More specifically: Requires [`TypeData`] for [`ReflectFromPtr`] to be registered for the given `type_id`, + /// which is automatically handled when deriving [`Reflect`] and calling [`App::register_type`]. + #[inline] + #[cfg(feature = "bevy_reflect")] + pub fn get_reflect( + &self, + entity: Entity, + type_id: TypeId, + ) -> Result<&dyn bevy_reflect::Reflect, GetComponentReflectError> { + use bevy_reflect::ReflectFromPtr; + + use crate::prelude::AppTypeRegistry; + + let Some(component_id) = self.components().get_id(type_id) else { + return Err(GetComponentReflectError::NoCorrespondingComponentId( + type_id, + )); + }; + + let Some(comp_ptr) = self.get_by_id(entity, component_id) else { + let component_name = self + .components() + .get_name(component_id) + .map(ToString::to_string); + + return Err(GetComponentReflectError::EntityDoesNotHaveComponent { + entity, + type_id, + component_id, + component_name, + }); + }; + + let Some(type_registry) = self.get_resource::().map(|atr| atr.read()) + else { + return Err(GetComponentReflectError::MissingAppTypeRegistry); + }; + + let Some(reflect_from_ptr) = type_registry.get_type_data::(type_id) else { + return Err(GetComponentReflectError::MissingReflectFromPtrTypeData( + type_id, + )); + }; + + // SAFETY: + // - `comp_ptr` is guaranteed to point to an object of type `type_id` + // - `reflect_from_ptr` was constructed for type `type_id` + // - Assertion that checks this equality is present + unsafe { + assert_eq!( + reflect_from_ptr.type_id(), + type_id, + "Mismatch between Ptr's type_id and ReflectFromPtr's type_id", + ); + + Ok(reflect_from_ptr.as_reflect(comp_ptr)) + } + } + + /// Retrieves a mutable reference to the given `entity`'s [`Component`] of the given `type_id` using + /// reflection. + /// + /// Requires implementing [`Reflect`] for the [`Component`] (e.g., using [`#[derive(Reflect)`](derive@bevy_reflect::Reflect)) + /// and `app.register_type::()` to have been called. + /// + /// This is the mutable version of [`World::get_reflect`], see its docs for more information + /// and an example. + /// + /// Just calling this method does not trigger [change detection](crate::change_detection). + /// + /// # Errors + /// + /// See [`GetComponentReflectError`] for the possible errors and their descriptions. + /// + /// # Example + /// + /// See the documentation for [`World::get_reflect`]. + /// + /// # Note + /// Requires the feature `bevy_reflect` (included in the default features). + /// + /// [`Reflect`]: bevy_reflect::Reflect + #[inline] + #[cfg(feature = "bevy_reflect")] + pub fn get_reflect_mut( + &mut self, + entity: Entity, + type_id: TypeId, + ) -> Result, GetComponentReflectError> { + use bevy_reflect::ReflectFromPtr; + + use crate::prelude::AppTypeRegistry; + + // little clone() + read() dance so we a) don't keep a borrow of `self` and b) don't drop a + // temporary (from read()) too early. + let Some(app_type_registry) = self.get_resource::().cloned() else { + return Err(GetComponentReflectError::MissingAppTypeRegistry); + }; + let type_registry = app_type_registry.read(); + + let Some(reflect_from_ptr) = type_registry.get_type_data::(type_id) else { + return Err(GetComponentReflectError::MissingReflectFromPtrTypeData( + type_id, + )); + }; + + let Some(component_id) = self.components().get_id(type_id) else { + return Err(GetComponentReflectError::NoCorrespondingComponentId( + type_id, + )); + }; + + // HACK: Only required for the `None`-case/`else`-branch, but it borrows `self`, which will + // already be mutablyy borrowed by `self.get_mut_by_id()`, and I didn't find a way around it. + let component_name = self + .components() + .get_name(component_id) + .map(ToString::to_string); + + let Some(comp_mut_untyped) = self.get_mut_by_id(entity, component_id) else { + return Err(GetComponentReflectError::EntityDoesNotHaveComponent { + entity, + type_id, + component_id, + component_name, + }); + }; + + // SAFETY: + // - `comp_mut_untyped` is guaranteed to point to an object of type `type_id` + // - `reflect_from_ptr` was constructed for type `type_id` + // - Assertion that checks this equality is present + let comp_mut_typed = comp_mut_untyped.map_unchanged(|ptr_mut| unsafe { + assert_eq!( + reflect_from_ptr.type_id(), + type_id, + "Mismatch between PtrMut's type_id and ReflectFromPtr's type_id", + ); + + reflect_from_ptr.as_reflect_mut(ptr_mut) + }); + + Ok(comp_mut_typed) + } +} + +/// The error type returned by [`World::get_reflect`] and [`World::get_reflect_mut`]. +#[derive(Error, Debug)] +pub enum GetComponentReflectError { + /// There is no [`ComponentId`] corresponding to the given [`TypeId`]. + /// + /// This is usually handled by calling [`App::register_type`] for the type corresponding to + /// the given [`TypeId`]. + /// + /// See the documentation for [`bevy_reflect`] for more information. + /// + /// [`App::register_type`]: ../../../bevy_app/struct.App.html#method.register_type + #[error("No `ComponentId` corresponding to {0:?} found (did you call App::register_type()?)")] + NoCorrespondingComponentId(TypeId), + + /// The given [`Entity`] does not have a [`Component`] corresponding to the given [`TypeId`]. + #[error("The given `Entity` {entity:?} does not have a `{component_name:?}` component ({component_id:?}, which corresponds to {type_id:?})")] + EntityDoesNotHaveComponent { + /// The given [`Entity`]. + entity: Entity, + /// The given [`TypeId`]. + type_id: TypeId, + /// The [`ComponentId`] corresponding to the given [`TypeId`]. + component_id: ComponentId, + /// The name corresponding to the [`Component`] with the given [`TypeId`], or `None` + /// if not available. + component_name: Option, + }, + + /// The [`World`] was missing the [`AppTypeRegistry`] resource. + #[error("The `World` was missing the `AppTypeRegistry` resource")] + MissingAppTypeRegistry, + + /// The [`World`]'s [`TypeRegistry`] did not contain [`TypeData`] for [`ReflectFromPtr`] for the given [`TypeId`]. + /// + /// This is usually handled by calling [`App::register_type`] for the type corresponding to + /// the given [`TypeId`]. + /// + /// See the documentation for [`bevy_reflect`] for more information. + /// + /// [`TypeData`]: bevy_reflect::TypeData + /// [`TypeRegistry`]: bevy_reflect::TypeRegistry + /// [`ReflectFromPtr`]: bevy_reflect::ReflectFromPtr + /// [`App::register_type`]: ../../../bevy_app/struct.App.html#method.register_type + #[error("The `World`'s `TypeRegistry` did not contain `TypeData` for `ReflectFromPtr` for the given {0:?} (did you call `App::register_type()`?)")] + MissingReflectFromPtrTypeData(TypeId), +} From c46f9119a09630d254a90c327947d0ccd6bb0cb7 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Tue, 23 Jul 2024 13:52:51 +0200 Subject: [PATCH 12/14] refactor: Also move tests to world::reflect --- crates/bevy_ecs/src/world/mod.rs | 91 ---------------------------- crates/bevy_ecs/src/world/reflect.rs | 91 ++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 91 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index a56129f3ecfa2..f47aa55ae573f 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -3335,95 +3335,4 @@ mod tests { ]) .is_err()); } - - #[cfg(all(test, feature = "bevy_reflect"))] - mod reflect_tests { - use std::any::TypeId; - - use bevy_reflect::Reflect; - - use crate::{ - // For bevy_ecs_macros - self as bevy_ecs, - prelude::{AppTypeRegistry, Component, DetectChanges, World}, - }; - - #[derive(Component, Reflect)] - struct RFoo(i32); - - #[derive(Component)] - struct Bar; - - #[test] - fn get_component_as_reflect() { - let mut world = World::new(); - world.init_resource::(); - - let app_type_registry = world.get_resource_mut::().unwrap(); - app_type_registry.write().register::(); - - { - let entity_with_rfoo = world.spawn(RFoo(42)).id(); - let comp_reflect = world - .get_reflect(entity_with_rfoo, TypeId::of::()) - .expect("Reflection of RFoo-component failed"); - - assert!(comp_reflect.is::()); - } - - { - let entity_without_rfoo = world.spawn_empty().id(); - let reflect_opt = world.get_reflect(entity_without_rfoo, TypeId::of::()); - - assert!(reflect_opt.is_err()); - } - - { - let entity_with_bar = world.spawn(Bar).id(); - let reflect_opt = world.get_reflect(entity_with_bar, TypeId::of::()); - - assert!(reflect_opt.is_err()); - } - } - - #[test] - fn get_component_as_mut_reflect() { - let mut world = World::new(); - world.init_resource::(); - - let app_type_registry = world.get_resource_mut::().unwrap(); - app_type_registry.write().register::(); - - { - let entity_with_rfoo = world.spawn(RFoo(42)).id(); - let mut comp_reflect = world - .get_reflect_mut(entity_with_rfoo, TypeId::of::()) - .expect("Mutable reflection of RFoo-component failed"); - - let comp_rfoo_reflected = comp_reflect - .downcast_mut::() - .expect("Wrong type reflected (expected RFoo)"); - assert_eq!(comp_rfoo_reflected.0, 42); - comp_rfoo_reflected.0 = 1337; - - let rfoo_ref = world.entity(entity_with_rfoo).get_ref::().unwrap(); - assert!(rfoo_ref.is_changed()); - assert_eq!(rfoo_ref.0, 1337); - } - - { - let entity_without_rfoo = world.spawn_empty().id(); - let reflect_opt = world.get_reflect_mut(entity_without_rfoo, TypeId::of::()); - - assert!(reflect_opt.is_err()); - } - - { - let entity_with_bar = world.spawn(Bar).id(); - let reflect_opt = world.get_reflect_mut(entity_with_bar, TypeId::of::()); - - assert!(reflect_opt.is_err()); - } - } - } } diff --git a/crates/bevy_ecs/src/world/reflect.rs b/crates/bevy_ecs/src/world/reflect.rs index 7f08173733aac..aded409073fa8 100644 --- a/crates/bevy_ecs/src/world/reflect.rs +++ b/crates/bevy_ecs/src/world/reflect.rs @@ -251,3 +251,94 @@ pub enum GetComponentReflectError { #[error("The `World`'s `TypeRegistry` did not contain `TypeData` for `ReflectFromPtr` for the given {0:?} (did you call `App::register_type()`?)")] MissingReflectFromPtrTypeData(TypeId), } + +#[cfg(test)] +mod tests { + use std::any::TypeId; + + use bevy_reflect::Reflect; + + use crate::{ + // For bevy_ecs_macros + self as bevy_ecs, + prelude::{AppTypeRegistry, Component, DetectChanges, World}, + }; + + #[derive(Component, Reflect)] + struct RFoo(i32); + + #[derive(Component)] + struct Bar; + + #[test] + fn get_component_as_reflect() { + let mut world = World::new(); + world.init_resource::(); + + let app_type_registry = world.get_resource_mut::().unwrap(); + app_type_registry.write().register::(); + + { + let entity_with_rfoo = world.spawn(RFoo(42)).id(); + let comp_reflect = world + .get_reflect(entity_with_rfoo, TypeId::of::()) + .expect("Reflection of RFoo-component failed"); + + assert!(comp_reflect.is::()); + } + + { + let entity_without_rfoo = world.spawn_empty().id(); + let reflect_opt = world.get_reflect(entity_without_rfoo, TypeId::of::()); + + assert!(reflect_opt.is_err()); + } + + { + let entity_with_bar = world.spawn(Bar).id(); + let reflect_opt = world.get_reflect(entity_with_bar, TypeId::of::()); + + assert!(reflect_opt.is_err()); + } + } + + #[test] + fn get_component_as_mut_reflect() { + let mut world = World::new(); + world.init_resource::(); + + let app_type_registry = world.get_resource_mut::().unwrap(); + app_type_registry.write().register::(); + + { + let entity_with_rfoo = world.spawn(RFoo(42)).id(); + let mut comp_reflect = world + .get_reflect_mut(entity_with_rfoo, TypeId::of::()) + .expect("Mutable reflection of RFoo-component failed"); + + let comp_rfoo_reflected = comp_reflect + .downcast_mut::() + .expect("Wrong type reflected (expected RFoo)"); + assert_eq!(comp_rfoo_reflected.0, 42); + comp_rfoo_reflected.0 = 1337; + + let rfoo_ref = world.entity(entity_with_rfoo).get_ref::().unwrap(); + assert!(rfoo_ref.is_changed()); + assert_eq!(rfoo_ref.0, 1337); + } + + { + let entity_without_rfoo = world.spawn_empty().id(); + let reflect_opt = world.get_reflect_mut(entity_without_rfoo, TypeId::of::()); + + assert!(reflect_opt.is_err()); + } + + { + let entity_with_bar = world.spawn(Bar).id(); + let reflect_opt = world.get_reflect_mut(entity_with_bar, TypeId::of::()); + + assert!(reflect_opt.is_err()); + } + } +} From 0cea40b761a7eddbb036c4b748860f32ebf9aba3 Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Tue, 23 Jul 2024 13:53:54 +0200 Subject: [PATCH 13/14] chore: Drop changes to `world/error.rs`, as we didn't need to change it --- crates/bevy_ecs/src/world/error.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/world/error.rs b/crates/bevy_ecs/src/world/error.rs index c53dd188719ad..326b0310ba15c 100644 --- a/crates/bevy_ecs/src/world/error.rs +++ b/crates/bevy_ecs/src/world/error.rs @@ -1,6 +1,4 @@ -//! Contains error types returned by methods on [`World`]. -//! -//! [`World`]: crate::world::World +//! Contains error types returned by bevy's schedule. use thiserror::Error; From f2031134aa5b2fd11c1d2c564a8b43c6b489f63d Mon Sep 17 00:00:00 2001 From: Felix Rath Date: Tue, 23 Jul 2024 18:11:00 +0200 Subject: [PATCH 14/14] chore: Cleanup imports since it's now in it's own module --- crates/bevy_ecs/src/world/reflect.rs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/world/reflect.rs b/crates/bevy_ecs/src/world/reflect.rs index aded409073fa8..2609d4e416722 100644 --- a/crates/bevy_ecs/src/world/reflect.rs +++ b/crates/bevy_ecs/src/world/reflect.rs @@ -1,11 +1,14 @@ //! Provides additional functionality for [`World`] when the `bevy_reflect` feature is enabled. -use crate::prelude::*; -use crate::world::ComponentId; use core::any::TypeId; use thiserror::Error; +use bevy_reflect::{Reflect, ReflectFromPtr}; + +use crate::prelude::*; +use crate::world::ComponentId; + impl World { /// Retrieves a reference to the given `entity`'s [`Component`] of the given `type_id` using /// reflection. @@ -62,16 +65,11 @@ impl World { /// [^note-reflect-impl]: More specifically: Requires [`TypeData`] for [`ReflectFromPtr`] to be registered for the given `type_id`, /// which is automatically handled when deriving [`Reflect`] and calling [`App::register_type`]. #[inline] - #[cfg(feature = "bevy_reflect")] pub fn get_reflect( &self, entity: Entity, type_id: TypeId, - ) -> Result<&dyn bevy_reflect::Reflect, GetComponentReflectError> { - use bevy_reflect::ReflectFromPtr; - - use crate::prelude::AppTypeRegistry; - + ) -> Result<&dyn Reflect, GetComponentReflectError> { let Some(component_id) = self.components().get_id(type_id) else { return Err(GetComponentReflectError::NoCorrespondingComponentId( type_id, @@ -142,16 +140,11 @@ impl World { /// /// [`Reflect`]: bevy_reflect::Reflect #[inline] - #[cfg(feature = "bevy_reflect")] pub fn get_reflect_mut( &mut self, entity: Entity, type_id: TypeId, - ) -> Result, GetComponentReflectError> { - use bevy_reflect::ReflectFromPtr; - - use crate::prelude::AppTypeRegistry; - + ) -> Result, GetComponentReflectError> { // little clone() + read() dance so we a) don't keep a borrow of `self` and b) don't drop a // temporary (from read()) too early. let Some(app_type_registry) = self.get_resource::().cloned() else {