forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

remove blanket Serialize + Deserialize requirement for Reflect on generic types (#5197)

Browse Source 
# Objective

Some generic types like `Option<T>`, `Vec<T>` and `HashMap<K, V>` 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<NonSerdeStruct>` 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::<Option<DoesImplSerde>>()`
- add `register_type_data<T, D>` shorthand for `registry.get_mut(T).insert(D::from_type<T>())`
- require users to register their specific generic types **and the serde types** separately like
```rust
        .register_type::<Option<String>>()
        .register_type_data::<Option<String>, ReflectSerialize>()
        .register_type_data::<Option<String>, ReflectDeserialize>()

```
I believe this is the best we can do for extensibility and convenience without specialization.


## Changelog

- `.register_type` for generic types like `Option<T>`, `Vec<T>`, `HashMap<K, V>` 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::<Option<String>>()
        .register_type_data::<Option<String>, ReflectSerialize>()
        .register_type_data::<Option<String>, ReflectDeserialize>()
```

TODO: more docs and tweaks to the scene example to demonstrate registering generic types.
This commit is contained in:
Jakob Hellermann 2022-07-21 14:57:37 +00:00
parent a96b3b2e2f
commit 4b191d968d
5 changed files with 95 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
crates/bevy_app/src/app.rs
View File

@ -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<T: bevy_reflect::GetTypeRegistration>(&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::<Option<String>>()
/// .register_type_data::<Option<String>, ReflectSerialize>()
/// .register_type_data::<Option<String>, 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<T>,
>(
&mut self,
) -> &mut Self {
{
let registry = self.world.resource_mut::<bevy_reflect::TypeRegistryArc>();
registry.write().register_type_data::<T, D>();
}
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`]

37
crates/bevy_reflect/src/impls/std.rs
View File

@ -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<T: Serialize + Clone + for<'de> Deserialize<'de> + Reflect + 'static>(Serialize, Deserialize));
impl_reflect_value!(HashSet<T: Serialize + Hash + Eq + Clone + for<'de> Deserialize<'de> + Send + Sync + 'static>(Serialize, Deserialize));
impl_reflect_value!(Range<T: Serialize + Clone + for<'de> Deserialize<'de> + Send + Sync + 'static>(Serialize, Deserialize));
impl_reflect_value!(Option<T: Clone + Reflect + 'static>());
impl_reflect_value!(Result<T: Clone + Reflect + 'static, E: Clone + Reflect + 'static>());
impl_reflect_value!(HashSet<T: Hash + Eq + Clone + Send + Sync + 'static>());
impl_reflect_value!(Range<T: Clone + Send + Sync + 'static>());
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<T: Serialize + Clone + for<'de> Deserialize<'de> + Reflect + 'static>
);
impl_from_reflect_value!(
HashSet<T: Serialize + Hash + Eq + Clone + for<'de> Deserialize<'de> + Send + Sync + 'static>
);
impl_from_reflect_value!(
Range<T: Serialize + Clone + for<'de> Deserialize<'de> + Send + Sync + 'static>
);
impl_from_reflect_value!(Option<T: Clone + Reflect + 'static>);
impl_from_reflect_value!(HashSet<T: Hash + Eq + Clone + Send + Sync + 'static>);
impl_from_reflect_value!(Range<T: Clone + Send + Sync + 'static>);
impl_from_reflect_value!(Duration);
impl<T: FromReflect> Array for Vec<T> {
@ -171,10 +166,9 @@ impl<T: FromReflect> Typed for Vec<T> {
}
}
impl<T: FromReflect + for<'de> Deserialize<'de>> GetTypeRegistration for Vec<T> {
impl<T: FromReflect> GetTypeRegistration for Vec<T> {
fn get_type_registration() -> TypeRegistration {
let mut registration = TypeRegistration::of::<Vec<T>>();
registration.insert::<ReflectDeserialize>(FromType::<Vec<T>>::from_type());
registration.insert::<ReflectFromPtr>(FromType::<Vec<T>>::from_type());
registration
}
@ -323,12 +317,11 @@ impl<K: FromReflect + Eq + Hash, V: FromReflect> Typed for HashMap<K, V> {
impl<K, V> GetTypeRegistration for HashMap<K, V>
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::<HashMap<K, V>>();
registration.insert::<ReflectDeserialize>(FromType::<HashMap<K, V>>::from_type());
registration.insert::<ReflectFromPtr>(FromType::<HashMap<K, V>>::from_type());
registration
}
@ -476,11 +469,9 @@ impl<T: Reflect, const N: usize> Typed for [T; N] {
macro_rules! impl_array_get_type_registration {
($($N:expr)+) => {
$(
impl<T: Reflect + for<'de> Deserialize<'de>> GetTypeRegistration for [T; $N] {
impl<T: Reflect > GetTypeRegistration for [T; $N] {
fn get_type_registration() -> TypeRegistration {
let mut registration = TypeRegistration::of::<[T; $N]>();
registration.insert::<ReflectDeserialize>(FromType::<[T; $N]>::from_type());
registration
TypeRegistration::of::<[T; $N]>()
}
}
)+

11
crates/bevy_reflect/src/tuple.rs
View File

@ -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::<ReflectDeserialize>(FromType::<($($name,)*)>::from_type());
registration
TypeRegistration::of::<($($name,)*)>()
}
}

33
crates/bevy_reflect/src/type_registry.rs
View File

@ -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<T>(&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::<Option<String>>();
/// type_registry.register_type_data::<Option<String>, ReflectSerialize>();
/// type_registry.register_type_data::<Option<String>, ReflectDeserialize>();
/// ```
pub fn register_type_data<T: Reflect + 'static, D: TypeData + FromType<T>>(&mut self) {
let data = self.get_mut(TypeId::of::<T>()).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::<T>(),
D = std::any::type_name::<D>(),
)
});
data.insert(D::from_type());
}
/// Returns a reference to the [`TypeRegistration`] of the type with the
/// given [`TypeId`].
///

5
crates/bevy_time/src/time.rs
View File

@ -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<Instant>,