From 69541462c5ef70914fd719fcd1f503045c76a453 Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Tue, 17 Sep 2024 17:36:41 -0700 Subject: [PATCH] bevy_reflect: Add `Reflectable` trait (#5772) # Objective When deriving `Reflect`, users will notice that their generic arguments also need to implement `Reflect`: ```rust #[derive(Reflect)] struct Foo { value: T } ``` This works well for now. However, as we want to do more with `Reflect`, these bounds might need to change. For example, to get #4154 working, we likely need to enforce the `GetTypeRegistration` trait. So now we have: ```rust #[derive(Reflect)] struct Foo { value: T } ``` Not great, but not horrible. However, we might then want to do something as suggested in [this](https://github.com/bevyengine/bevy/issues/5745#issuecomment-1221389131) comment and add a `ReflectTypeName` trait for stable type name support. Well now we have: ```rust #[derive(Reflect)] struct Foo { value: T } ``` Now imagine that for even two or three generic types. Yikes! As the API changes it would be nice if users didn't need to manually migrate their generic type bounds like this. A lot of these traits are (or will/might be) core to the entire reflection API. And although `Reflect` can't add them as supertraits for object-safety reasons, they are still indirectly required for things to function properly (manual implementors will know how easy it is to forget to implement `GetTypeRegistration`). And they should all be automatically implemented for user types anyways as long they're using `#[derive(Reflect)]`. ## Solution Add a "catch-all" trait called `Reflectable` whose supertraits are a select handful of core reflection traits. This allows us to consolidate all the examples above into this: ```rust #[derive(Reflect)] struct Foo { value: T } ``` And as we experiment with the API, users can rest easy knowing they don't need to migrate dozens upon dozens of types. It should all be automatic! ## Discussion 1. Thoughts on the name `Reflectable`? Is it too easily confused with `Reflect`? Or does it at least accurately describe that this contains the core traits? If not, maybe `BaseReflect`? --- ## Changelog - Added the `Reflectable` trait --------- Co-authored-by: Alice Cecile --- crates/bevy_reflect/src/lib.rs | 8 +++--- crates/bevy_reflect/src/reflect.rs | 4 +++ crates/bevy_reflect/src/reflectable.rs | 33 ++++++++++++++++++++++++ crates/bevy_reflect/src/type_info.rs | 4 +++ crates/bevy_reflect/src/type_registry.rs | 4 +++ 5 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 crates/bevy_reflect/src/reflectable.rs diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 61479625e3..5505bad824 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -546,6 +546,7 @@ mod list; mod map; mod path; mod reflect; +mod reflectable; mod remote; mod set; mod struct_trait; @@ -602,6 +603,7 @@ pub use list::*; pub use map::*; pub use path::*; pub use reflect::*; +pub use reflectable::*; pub use remote::*; pub use set::*; pub use struct_trait::*; @@ -2756,7 +2758,7 @@ bevy_reflect::tests::Test { } #[reflect_remote(external_crate::TheirOuter)] - struct MyOuter { + struct MyOuter { #[reflect(remote = MyInner)] pub a: external_crate::TheirInner, #[reflect(remote = MyInner)] @@ -2804,7 +2806,7 @@ bevy_reflect::tests::Test { #[reflect_remote(external_crate::TheirOuter)] #[derive(Debug)] - enum MyOuter { + enum MyOuter { Unit, Tuple(#[reflect(remote = MyInner)] external_crate::TheirInner), Struct { @@ -2917,7 +2919,7 @@ bevy_reflect::tests::Test { } #[reflect_remote(external_crate::TheirOuter)] - struct MyOuter { + struct MyOuter { #[reflect(remote = MyInner)] pub inner: external_crate::TheirInner, } diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 9efa868e6c..6538d55c29 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -399,10 +399,14 @@ where /// Doing so will automatically implement this trait, [`PartialReflect`], and many other useful traits for reflection, /// including one of the appropriate subtraits: [`Struct`], [`TupleStruct`] or [`Enum`]. /// +/// If you need to use this trait as a generic bound along with other reflection traits, +/// for your convenience, consider using [`Reflectable`] instead. +/// /// See the [crate-level documentation] to see how this trait can be used. /// /// [`bevy_reflect`]: crate /// [the derive macro]: bevy_reflect_derive::Reflect +/// [`Reflectable`]: crate::Reflectable /// [crate-level documentation]: crate #[diagnostic::on_unimplemented( message = "`{Self}` does not implement `Reflect` so cannot be fully reflected", diff --git a/crates/bevy_reflect/src/reflectable.rs b/crates/bevy_reflect/src/reflectable.rs new file mode 100644 index 0000000000..08226bc214 --- /dev/null +++ b/crates/bevy_reflect/src/reflectable.rs @@ -0,0 +1,33 @@ +use crate::{GetTypeRegistration, Reflect, TypePath, Typed}; + +/// A catch-all trait that is bound by the core reflection traits, +/// useful to simplify reflection-based generic type bounds. +/// +/// You do _not_ need to implement this trait manually. +/// It is automatically implemented for all types that implement its supertraits. +/// And these supertraits are all automatically derived with the [`Reflect` derive macro]. +/// +/// This should namely be used to bound generic arguments to the necessary traits for reflection. +/// Doing this has the added benefit of reducing migration costs, as a change to the required traits +/// is automatically handled by this trait. +/// +/// For now, the supertraits of this trait includes: +/// * [`Reflect`] +/// * [`GetTypeRegistration`] +/// * [`Typed`] +/// * [`TypePath`] +/// +/// ## Example +/// +/// ``` +/// # use bevy_reflect::{Reflect, Reflectable}; +/// #[derive(Reflect)] +/// struct MyStruct { +/// value: T +/// } +/// ``` +/// +/// [`Reflect` derive macro]: bevy_reflect_derive::Reflect +pub trait Reflectable: Reflect + GetTypeRegistration + Typed + TypePath {} + +impl Reflectable for T {} diff --git a/crates/bevy_reflect/src/type_info.rs b/crates/bevy_reflect/src/type_info.rs index bc1ef1ef12..fdd59396da 100644 --- a/crates/bevy_reflect/src/type_info.rs +++ b/crates/bevy_reflect/src/type_info.rs @@ -14,6 +14,9 @@ use thiserror::Error; /// This trait is automatically implemented by the [`#[derive(Reflect)]`](derive@crate::Reflect) macro /// and allows type information to be processed without an instance of that type. /// +/// If you need to use this trait as a generic bound along with other reflection traits, +/// for your convenience, consider using [`Reflectable`] instead. +/// /// # Implementing /// /// While it is recommended to leave implementing this trait to the `#[derive(Reflect)]` macro, @@ -81,6 +84,7 @@ use thiserror::Error; /// # } /// ``` /// +/// [`Reflectable`]: crate::Reflectable /// [utility]: crate::utility #[diagnostic::on_unimplemented( message = "`{Self}` does not implement `Typed` so cannot provide static type information", diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 1fe7208956..fa74e51950 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -53,8 +53,12 @@ impl Debug for TypeRegistryArc { /// This trait is automatically implemented for items using [`#[derive(Reflect)]`](derive@crate::Reflect). /// The macro also allows [`TypeData`] to be more easily registered. /// +/// If you need to use this trait as a generic bound along with other reflection traits, +/// for your convenience, consider using [`Reflectable`] instead. +/// /// See the [crate-level documentation] for more information on type registration. /// +/// [`Reflectable`]: crate::Reflectable /// [crate-level documentation]: crate #[diagnostic::on_unimplemented( message = "`{Self}` does not implement `GetTypeRegistration` so cannot provide type registration information",