From 4b191d968d2ea011b26d9f91da65b77f09da22e2 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 21 Jul 2022 14:57:37 +0000 Subject: [PATCH] remove blanket `Serialize + Deserialize` requirement for `Reflect` on generic types (#5197) # Objective Some generic types like `Option`, `Vec` and `HashMap` implement `Reflect` when where their generic types `T`/`K`/`V` implement `Serialize + for<'de> Deserialize<'de>`. This is so that in their `GetTypeRegistration` impl they can insert the `ReflectSerialize` and `ReflectDeserialize` type data structs. This has the annoying side effect that if your struct contains a `Option` you won't be able to derive reflect (https://github.com/bevyengine/bevy/issues/4054). ## Solution - remove the `Serialize + Deserialize` bounds on wrapper types - this means that `ReflectSerialize` and `ReflectDeserialize` will no longer be inserted even for `.register::>()` - add `register_type_data` shorthand for `registry.get_mut(T).insert(D::from_type())` - require users to register their specific generic types **and the serde types** separately like ```rust .register_type::>() .register_type_data::, ReflectSerialize>() .register_type_data::, ReflectDeserialize>() ``` I believe this is the best we can do for extensibility and convenience without specialization. ## Changelog - `.register_type` for generic types like `Option`, `Vec`, `HashMap` will no longer insert `ReflectSerialize` and `ReflectDeserialize` type data. Instead you need to register it separately for concrete generic types like so: ```rust .register_type::>() .register_type_data::, ReflectSerialize>() .register_type_data::, ReflectDeserialize>() ``` TODO: more docs and tweaks to the scene example to demonstrate registering generic types. --- crates/bevy_app/src/app.rs | 42 +++++++++++++++++++++++- crates/bevy_reflect/src/impls/std.rs | 37 ++++++++------------- crates/bevy_reflect/src/tuple.rs | 11 +++---- crates/bevy_reflect/src/type_registry.rs | 33 ++++++++++++++++++- crates/bevy_time/src/time.rs | 5 ++- 5 files changed, 95 insertions(+), 33 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index e9166bc3dd573..ebdd2ee0df54a 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -862,7 +862,14 @@ impl App { self } - /// Adds the type `T` to the type registry [`Resource`]. + /// Registers the type `T` in the [`TypeRegistry`](bevy_reflect::TypeRegistry) resource, + /// adding reflect data as specified in the [`Reflect`](bevy_reflect::Reflect) derive: + /// ```rust,ignore + /// #[derive(Reflect)] + /// #[reflect(Component, Serialize, Deserialize)] // will register ReflectComponent, ReflectSerialize, ReflectDeserialize + /// ``` + /// + /// See [`bevy_reflect::TypeRegistry::register`]. #[cfg(feature = "bevy_reflect")] pub fn register_type(&mut self) -> &mut Self { { @@ -872,6 +879,39 @@ impl App { self } + /// Adds the type data `D` to type `T` in the [`TypeRegistry`](bevy_reflect::TypeRegistry) resource. + /// + /// Most of the time [`App::register_type`] can be used instead to register a type you derived [`Reflect`](bevy_reflect::Reflect) for. + /// However, in cases where you want to add a piece of type data that was not included in the list of `#[reflect(...)]` type data in the derive, + /// or where the type is generic and cannot register e.g. `ReflectSerialize` unconditionally without knowing the specific type parameters, + /// this method can be used to insert additional type data. + /// + /// # Example + /// ```rust + /// use bevy_app::App; + /// use bevy_reflect::{ReflectSerialize, ReflectDeserialize}; + /// + /// App::new() + /// .register_type::>() + /// .register_type_data::, ReflectSerialize>() + /// .register_type_data::, ReflectDeserialize>(); + /// ``` + /// + /// See [`bevy_reflect::TypeRegistry::register_type_data`]. + #[cfg(feature = "bevy_reflect")] + pub fn register_type_data< + T: bevy_reflect::Reflect + 'static, + D: bevy_reflect::TypeData + bevy_reflect::FromType, + >( + &mut self, + ) -> &mut Self { + { + let registry = self.world.resource_mut::(); + registry.write().register_type_data::(); + } + self + } + /// Adds an [`App`] as a child of the current one. /// /// The provided function `f` is called by the [`update`](Self::update) method. The [`World`] diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index e172482ed07c4..6ce789f975709 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -7,8 +7,7 @@ use crate::{ use crate::utility::{GenericTypeInfoCell, NonGenericTypeInfoCell}; use bevy_reflect_derive::{impl_from_reflect_value, impl_reflect_value}; -use bevy_utils::{Duration, HashMap, HashSet}; -use serde::{Deserialize, Serialize}; +use bevy_utils::{Duration, HashMap, HashSet, Instant}; use std::{ any::Any, borrow::Cow, @@ -33,10 +32,12 @@ impl_reflect_value!(isize(Debug, Hash, PartialEq, Serialize, Deserialize)); impl_reflect_value!(f32(Debug, PartialEq, Serialize, Deserialize)); impl_reflect_value!(f64(Debug, PartialEq, Serialize, Deserialize)); impl_reflect_value!(String(Debug, Hash, PartialEq, Serialize, Deserialize)); -impl_reflect_value!(Option Deserialize<'de> + Reflect + 'static>(Serialize, Deserialize)); -impl_reflect_value!(HashSet Deserialize<'de> + Send + Sync + 'static>(Serialize, Deserialize)); -impl_reflect_value!(Range Deserialize<'de> + Send + Sync + 'static>(Serialize, Deserialize)); +impl_reflect_value!(Option()); +impl_reflect_value!(Result()); +impl_reflect_value!(HashSet()); +impl_reflect_value!(Range()); impl_reflect_value!(Duration(Debug, Hash, PartialEq, Serialize, Deserialize)); +impl_reflect_value!(Instant(Debug, Hash, PartialEq)); impl_from_reflect_value!(bool); impl_from_reflect_value!(char); @@ -55,15 +56,9 @@ impl_from_reflect_value!(isize); impl_from_reflect_value!(f32); impl_from_reflect_value!(f64); impl_from_reflect_value!(String); -impl_from_reflect_value!( - Option Deserialize<'de> + Reflect + 'static> -); -impl_from_reflect_value!( - HashSet Deserialize<'de> + Send + Sync + 'static> -); -impl_from_reflect_value!( - Range Deserialize<'de> + Send + Sync + 'static> -); +impl_from_reflect_value!(Option); +impl_from_reflect_value!(HashSet); +impl_from_reflect_value!(Range); impl_from_reflect_value!(Duration); impl Array for Vec { @@ -171,10 +166,9 @@ impl Typed for Vec { } } -impl Deserialize<'de>> GetTypeRegistration for Vec { +impl GetTypeRegistration for Vec { fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::>(); - registration.insert::(FromType::>::from_type()); registration.insert::(FromType::>::from_type()); registration } @@ -323,12 +317,11 @@ impl Typed for HashMap { impl GetTypeRegistration for HashMap where - K: FromReflect + Clone + Eq + Hash + for<'de> Deserialize<'de>, - V: FromReflect + Clone + for<'de> Deserialize<'de>, + K: FromReflect + Clone + Eq + Hash, + V: FromReflect + Clone, { fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::>(); - registration.insert::(FromType::>::from_type()); registration.insert::(FromType::>::from_type()); registration } @@ -476,11 +469,9 @@ impl Typed for [T; N] { macro_rules! impl_array_get_type_registration { ($($N:expr)+) => { $( - impl Deserialize<'de>> GetTypeRegistration for [T; $N] { + impl GetTypeRegistration for [T; $N] { fn get_type_registration() -> TypeRegistration { - let mut registration = TypeRegistration::of::<[T; $N]>(); - registration.insert::(FromType::<[T; $N]>::from_type()); - registration + TypeRegistration::of::<[T; $N]>() } } )+ diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index 7651f7f5e32dd..96386e85d6bbc 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -1,9 +1,8 @@ use crate::utility::NonGenericTypeInfoCell; use crate::{ - DynamicInfo, FromReflect, FromType, GetTypeRegistration, Reflect, ReflectDeserialize, - ReflectMut, ReflectRef, TypeInfo, TypeRegistration, Typed, UnnamedField, + DynamicInfo, FromReflect, GetTypeRegistration, Reflect, ReflectMut, ReflectRef, TypeInfo, + TypeRegistration, Typed, UnnamedField, }; -use serde::Deserialize; use std::any::{Any, TypeId}; use std::fmt::{Debug, Formatter}; use std::slice::Iter; @@ -534,11 +533,9 @@ macro_rules! impl_reflect_tuple { } } - impl<$($name: Reflect + Typed + for<'de> Deserialize<'de>),*> GetTypeRegistration for ($($name,)*) { + impl<$($name: Reflect + Typed),*> GetTypeRegistration for ($($name,)*) { fn get_type_registration() -> TypeRegistration { - let mut registration = TypeRegistration::of::<($($name,)*)>(); - registration.insert::(FromType::<($($name,)*)>::from_type()); - registration + TypeRegistration::of::<($($name,)*)>() } } diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 3a44fe4da1499..74865abeb0c91 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -73,7 +73,11 @@ impl TypeRegistry { registry } - /// Registers the type `T`. + /// Registers the type `T`, adding reflect data as specified in the [`Reflect`] derive: + /// ```rust,ignore + /// #[derive(Reflect)] + /// #[reflect(Component, Serialize, Deserialize)] // will register ReflectComponent, ReflectSerialize, ReflectDeserialize + /// ``` pub fn register(&mut self) where T: GetTypeRegistration, @@ -100,6 +104,33 @@ impl TypeRegistry { .insert(registration.type_id(), registration); } + /// Registers the type data `D` for type `T`. + /// + /// Most of the time [`TypeRegistry::register`] can be used instead to register a type you derived [`Reflect`] for. + /// However, in cases where you want to add a piece of type data that was not included in the list of `#[reflect(...)]` type data in the derive, + /// or where the type is generic and cannot register e.g. `ReflectSerialize` unconditionally without knowing the specific type parameters, + /// this method can be used to insert additional type data. + /// + /// # Example + /// ```rust + /// use bevy_reflect::{TypeRegistry, ReflectSerialize, ReflectDeserialize}; + /// + /// let mut type_registry = TypeRegistry::default(); + /// type_registry.register::>(); + /// type_registry.register_type_data::, ReflectSerialize>(); + /// type_registry.register_type_data::, ReflectDeserialize>(); + /// ``` + pub fn register_type_data>(&mut self) { + let data = self.get_mut(TypeId::of::()).unwrap_or_else(|| { + panic!( + "attempted to call `TypeRegistry::register_type_data` for type `{T}` with data `{D}` without registering `{T}` first", + T = std::any::type_name::(), + D = std::any::type_name::(), + ) + }); + data.insert(D::from_type()); + } + /// Returns a reference to the [`TypeRegistration`] of the type with the /// given [`TypeId`]. /// diff --git a/crates/bevy_time/src/time.rs b/crates/bevy_time/src/time.rs index 179eb74073b70..cfe34316a745a 100644 --- a/crates/bevy_time/src/time.rs +++ b/crates/bevy_time/src/time.rs @@ -1,7 +1,10 @@ +use bevy_ecs::reflect::ReflectResource; +use bevy_reflect::Reflect; use bevy_utils::{Duration, Instant}; /// Tracks elapsed time since the last update and since the App has started -#[derive(Debug, Clone)] +#[derive(Reflect, Debug, Clone)] +#[reflect(Resource)] pub struct Time { delta: Duration, last_update: Option,