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

bevy_reflect: Small refactor and default Reflect methods (#4739)

Browse Source 
# Objective

Quick followup to #4712.

While updating some [other PRs](https://github.com/bevyengine/bevy/pull/4218), I realized the `ReflectTraits` struct could be improved. The issue with the current implementation is that `ReflectTraits::get_xxx_impl(...)` returns just the _logic_ to the corresponding `Reflect` trait method, rather than the entire function.

This makes it slightly more annoying to manage since the variable names need to be consistent across files. For example, `get_partial_eq_impl` uses a `value` variable. But the name "value" isn't defined in the `get_partial_eq_impl` method, it's defined in three other methods in a completely separate file.

It's not likely to cause any bugs if we keep it as it is since differing variable names will probably just result in a compile error (except in very particular cases). But it would be useful to someone who wanted to edit/add/remove a method.

## Solution

Made `get_hash_impl`, `get_partial_eq_impl` and `get_serialize_impl` return the entire method implementation for `reflect_hash`, `reflect_partial_eq`, and `serializable`, respectively.

As a result of this, those three `Reflect` methods were also given default implementations. This was fairly simple to do since all three could just be made to return `None`.

---

## Changelog

* Small cleanup/refactor to `ReflectTraits` in `bevy_reflect_derive`
* Gave `Reflect::reflect_hash`, `Reflect::reflect_partial_eq`, and `Reflect::serializable` default implementations
This commit is contained in:
MrGVSV 2022-05-18 12:26:11 +00:00
parent de2b1a4e94
commit 15acd6f45d
9 changed files with 88 additions and 157 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs
View File

@ -171,55 +171,70 @@ impl ReflectTraits {
&self.idents
}
/// Returns the logic for `Reflect::reflect_hash` as a `TokenStream`.
/// Returns the implementation of `Reflect::reflect_hash` as a `TokenStream`.
///
/// If `Hash` was not registered, returns `None`.
pub fn get_hash_impl(&self, path: &Path) -> Option<proc_macro2::TokenStream> {
pub fn get_hash_impl(&self, bevy_reflect_path: &Path) -> Option<proc_macro2::TokenStream> {
match &self.hash {
TraitImpl::Implemented => Some(quote! {
use std::hash::{Hash, Hasher};
let mut hasher = #path::ReflectHasher::default();
Hash::hash(&std::any::Any::type_id(self), &mut hasher);
Hash::hash(self, &mut hasher);
Some(hasher.finish())
}),
TraitImpl::Custom(impl_fn) => Some(quote! {
Some(#impl_fn(self))
}),
TraitImpl::NotImplemented => None,
}
}
/// Returns the logic for `Reflect::reflect_partial_eq` as a `TokenStream`.
///
/// If `PartialEq` was not registered, returns `None`.
pub fn get_partial_eq_impl(&self) -> Option<proc_macro2::TokenStream> {
match &self.partial_eq {
TraitImpl::Implemented => Some(quote! {
let value = value.any();
if let Some(value) = value.downcast_ref::<Self>() {
Some(std::cmp::PartialEq::eq(self, value))
} else {
Some(false)
fn reflect_hash(&self) -> Option<u64> {
use std::hash::{Hash, Hasher};
let mut hasher = #bevy_reflect_path::ReflectHasher::default();
Hash::hash(&std::any::Any::type_id(self), &mut hasher);
Hash::hash(self, &mut hasher);
Some(hasher.finish())
}
}),
TraitImpl::Custom(impl_fn) => Some(quote! {
Some(#impl_fn(self, value))
fn reflect_hash(&self) -> Option<u64> {
Some(#impl_fn(self))
}
}),
TraitImpl::NotImplemented => None,
}
}
/// Returns the logic for `Reflect::serializable` as a `TokenStream`.
/// Returns the implementation of `Reflect::reflect_partial_eq` as a `TokenStream`.
///
/// If `Serialize` was not registered, returns `None`.
pub fn get_serialize_impl(&self, path: &Path) -> Option<proc_macro2::TokenStream> {
match &self.serialize {
/// If `PartialEq` was not registered, returns `None`.
pub fn get_partial_eq_impl(
&self,
bevy_reflect_path: &Path,
) -> Option<proc_macro2::TokenStream> {
match &self.partial_eq {
TraitImpl::Implemented => Some(quote! {
Some(#path::serde::Serializable::Borrowed(self))
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option<bool> {
let value = value.any();
if let Some(value) = value.downcast_ref::<Self>() {
Some(std::cmp::PartialEq::eq(self, value))
} else {
Some(false)
}
}
}),
TraitImpl::Custom(impl_fn) => Some(quote! {
Some(#impl_fn(self))
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option<bool> {
Some(#impl_fn(self, value))
}
}),
TraitImpl::NotImplemented => None,
}
}
/// Returns the implementation of `Reflect::serializable` as a `TokenStream`.
///
/// If `Serialize` was not registered, returns `None`.
pub fn get_serialize_impl(&self, bevy_reflect_path: &Path) -> Option<proc_macro2::TokenStream> {
match &self.serialize {
TraitImpl::Implemented => Some(quote! {
fn serializable(&self) -> Option<#bevy_reflect_path::serde::Serializable> {
Some(#bevy_reflect_path::serde::Serializable::Borrowed(self))
}
}),
TraitImpl::Custom(impl_fn) => Some(quote! {
fn serializable(&self) -> Option<#bevy_reflect_path::serde::Serializable> {
Some(#impl_fn(self))
}
}),
TraitImpl::NotImplemented => None,
}

82
crates/bevy_reflect/bevy_reflect_derive/src/impls.rs
View File

@ -35,20 +35,16 @@ pub(crate) fn impl_struct(derive_data: &ReflectDeriveData) -> TokenStream {
let field_count = field_idents.len();
let field_indices = (0..field_count).collect::<Vec<usize>>();
let hash_fn = derive_data
.traits()
.get_hash_impl(bevy_reflect_path)
.unwrap_or_else(|| quote!(None));
let serialize_fn = derive_data
.traits()
.get_serialize_impl(bevy_reflect_path)
.unwrap_or_else(|| quote!(None));
let hash_fn = derive_data.traits().get_hash_impl(bevy_reflect_path);
let serialize_fn = derive_data.traits().get_serialize_impl(bevy_reflect_path);
let partial_eq_fn = derive_data
.traits()
.get_partial_eq_impl()
.get_partial_eq_impl(bevy_reflect_path)
.unwrap_or_else(|| {
quote! {
#bevy_reflect_path::struct_partial_eq(self, value)
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option<bool> {
#bevy_reflect_path::struct_partial_eq(self, value)
}
}
});
@ -166,17 +162,11 @@ pub(crate) fn impl_struct(derive_data: &ReflectDeriveData) -> TokenStream {
#bevy_reflect_path::ReflectMut::Struct(self)
}
fn serializable(&self) -> Option<#bevy_reflect_path::serde::Serializable> {
#serialize_fn
}
#hash_fn
fn reflect_hash(&self) -> Option<u64> {
#hash_fn
}
#partial_eq_fn
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option<bool> {
#partial_eq_fn
}
#serialize_fn
}
})
}
@ -194,20 +184,16 @@ pub(crate) fn impl_tuple_struct(derive_data: &ReflectDeriveData) -> TokenStream
let field_count = field_idents.len();
let field_indices = (0..field_count).collect::<Vec<usize>>();
let hash_fn = derive_data
.traits()
.get_hash_impl(bevy_reflect_path)
.unwrap_or_else(|| quote!(None));
let serialize_fn = derive_data
.traits()
.get_serialize_impl(bevy_reflect_path)
.unwrap_or_else(|| quote!(None));
let hash_fn = derive_data.traits().get_hash_impl(bevy_reflect_path);
let serialize_fn = derive_data.traits().get_serialize_impl(bevy_reflect_path);
let partial_eq_fn = derive_data
.traits()
.get_partial_eq_impl()
.get_partial_eq_impl(bevy_reflect_path)
.unwrap_or_else(|| {
quote! {
#bevy_reflect_path::tuple_struct_partial_eq(self, value)
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option<bool> {
#bevy_reflect_path::tuple_struct_partial_eq(self, value)
}
}
});
@ -301,17 +287,11 @@ pub(crate) fn impl_tuple_struct(derive_data: &ReflectDeriveData) -> TokenStream
#bevy_reflect_path::ReflectMut::TupleStruct(self)
}
fn serializable(&self) -> Option<#bevy_reflect_path::serde::Serializable> {
#serialize_fn
}
#hash_fn
fn reflect_hash(&self) -> Option<u64> {
#hash_fn
}
#partial_eq_fn
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option<bool> {
#partial_eq_fn
}
#serialize_fn
}
})
}
@ -322,17 +302,11 @@ pub(crate) fn impl_value(
generics: &Generics,
get_type_registration_impl: proc_macro2::TokenStream,
bevy_reflect_path: &Path,
reflect_attrs: &ReflectTraits,
reflect_traits: &ReflectTraits,
) -> TokenStream {
let hash_fn = reflect_attrs
.get_hash_impl(bevy_reflect_path)
.unwrap_or_else(|| quote!(None));
let partial_eq_fn = reflect_attrs
.get_partial_eq_impl()
.unwrap_or_else(|| quote!(None));
let serialize_fn = reflect_attrs
.get_serialize_impl(bevy_reflect_path)
.unwrap_or_else(|| quote!(None));
let hash_fn = reflect_traits.get_hash_impl(bevy_reflect_path);
let serialize_fn = reflect_traits.get_serialize_impl(bevy_reflect_path);
let partial_eq_fn = reflect_traits.get_partial_eq_impl(bevy_reflect_path);
let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
TokenStream::from(quote! {
@ -394,17 +368,11 @@ pub(crate) fn impl_value(
#bevy_reflect_path::ReflectMut::Value(self)
}
fn reflect_hash(&self) -> Option<u64> {
#hash_fn
}
#hash_fn
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option<bool> {
#partial_eq_fn
}
#partial_eq_fn
fn serializable(&self) -> Option<#bevy_reflect_path::serde::Serializable> {
#serialize_fn
}
#serialize_fn
}
})
}

12
crates/bevy_reflect/src/impls/smallvec.rs
View File

@ -1,9 +1,7 @@
use smallvec::SmallVec;
use std::any::Any;
use crate::{
serde::Serializable, Array, ArrayIter, FromReflect, List, Reflect, ReflectMut, ReflectRef,
};
use crate::{Array, ArrayIter, FromReflect, List, Reflect, ReflectMut, ReflectRef};
impl<T: smallvec::Array + Send + Sync + 'static> Array for SmallVec<T>
where
@ -100,17 +98,9 @@ where
Box::new(List::clone_dynamic(self))
}
fn reflect_hash(&self) -> Option<u64> {
None
}
fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option<bool> {
crate::list_partial_eq(self, value)
}
fn serializable(&self) -> Option<Serializable> {
None
}
}
impl<T: smallvec::Array + Send + Sync + 'static> FromReflect for SmallVec<T>

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

@ -273,17 +273,9 @@ unsafe impl<K: Reflect + Eq + Hash, V: Reflect> Reflect for HashMap<K, V> {
Box::new(self.clone_dynamic())
}
fn reflect_hash(&self) -> Option<u64> {
None
}
fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option<bool> {
map_partial_eq(self, value)
}
fn serializable(&self) -> Option<Serializable> {
None
}
}
impl<K, V> GetTypeRegistration for HashMap<K, V>

10
crates/bevy_reflect/src/map.rs
View File

@ -2,7 +2,7 @@ use std::any::Any;
use bevy_utils::{Entry, HashMap};
use crate::{serde::Serializable, Reflect, ReflectMut, ReflectRef};
use crate::{Reflect, ReflectMut, ReflectRef};
/// An ordered mapping between [`Reflect`] values.
///
@ -186,17 +186,9 @@ unsafe impl Reflect for DynamicMap {
Box::new(self.clone_dynamic())
}
fn reflect_hash(&self) -> Option<u64> {
None
}
fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option<bool> {
map_partial_eq(self, value)
}
fn serializable(&self) -> Option<Serializable> {
None
}
}
/// An iterator over the key-value pairs of a [`Map`].

12
crates/bevy_reflect/src/reflect.rs
View File

@ -130,17 +130,23 @@ pub unsafe trait Reflect: Any + Send + Sync {
/// Returns a hash of the value (which includes the type).
///
/// If the underlying type does not support hashing, returns `None`.
fn reflect_hash(&self) -> Option<u64>;
fn reflect_hash(&self) -> Option<u64> {
None
}
/// Returns a "partial equality" comparison result.
///
/// If the underlying type does not support equality testing, returns `None`.
fn reflect_partial_eq(&self, _value: &dyn Reflect) -> Option<bool>;
fn reflect_partial_eq(&self, _value: &dyn Reflect) -> Option<bool> {
None
}
/// Returns a serializable version of the value.
///
/// If the underlying type does not support serialization, returns `None`.
fn serializable(&self) -> Option<Serializable>;
fn serializable(&self) -> Option<Serializable> {
None
}
}
/// A trait for types which can be constructed from a reflected type.

10
crates/bevy_reflect/src/struct_trait.rs
View File

@ -1,4 +1,4 @@
use crate::{serde::Serializable, Reflect, ReflectMut, ReflectRef};
use crate::{Reflect, ReflectMut, ReflectRef};
use bevy_utils::{Entry, HashMap};
use std::{any::Any, borrow::Cow};
@ -312,17 +312,9 @@ unsafe impl Reflect for DynamicStruct {
Ok(())
}
fn reflect_hash(&self) -> Option<u64> {
None
}
fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option<bool> {
struct_partial_eq(self, value)
}
fn serializable(&self) -> Option<Serializable> {
None
}
}
/// Compares a [`Struct`] with a [`Reflect`] value.

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

@ -1,6 +1,6 @@
use crate::{
serde::Serializable, FromReflect, FromType, GetTypeRegistration, Reflect, ReflectDeserialize,
ReflectMut, ReflectRef, TypeRegistration,
FromReflect, FromType, GetTypeRegistration, Reflect, ReflectDeserialize, ReflectMut,
ReflectRef, TypeRegistration,
};
use serde::Deserialize;
use std::any::Any;
@ -259,17 +259,9 @@ unsafe impl Reflect for DynamicTuple {
Ok(())
}
fn reflect_hash(&self) -> Option<u64> {
None
}
fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option<bool> {
tuple_partial_eq(self, value)
}
fn serializable(&self) -> Option<Serializable> {
None
}
}
/// Applies the elements of `b` to the corresponding elements of `a`.
@ -408,17 +400,9 @@ macro_rules! impl_reflect_tuple {
Box::new(self.clone_dynamic())
}
fn reflect_hash(&self) -> Option<u64> {
None
}
fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option<bool> {
crate::tuple_partial_eq(self, value)
}
fn serializable(&self) -> Option<Serializable> {
None
}
}
impl<$($name: Reflect + for<'de> Deserialize<'de>),*> GetTypeRegistration for ($($name,)*) {

10
crates/bevy_reflect/src/tuple_struct.rs
View File

@ -1,4 +1,4 @@
use crate::{serde::Serializable, Reflect, ReflectMut, ReflectRef};
use crate::{Reflect, ReflectMut, ReflectRef};
use std::any::Any;
/// A reflected Rust tuple struct.
@ -251,17 +251,9 @@ unsafe impl Reflect for DynamicTupleStruct {
Ok(())
}
fn reflect_hash(&self) -> Option<u64> {
None
}
fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option<bool> {
tuple_struct_partial_eq(self, value)
}
fn serializable(&self) -> Option<Serializable> {
None
}
}
/// Compares a [`TupleStruct`] with a [`Reflect`] value.