bevy_reflect: Small refactor and default Reflect methods (bevyengin…

…e#4739)

# Objective

Quick followup to bevyengine#4712.

While updating some [other PRs](bevyengine#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

crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs

@@ -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())
+                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))
+                fn reflect_hash(&self) -> Option<u64> {
+                    Some(#impl_fn(self))
+                }
             }),
             TraitImpl::NotImplemented => None,
         }
     }
 
-    /// Returns the logic for `Reflect::reflect_partial_eq` as a `TokenStream`.
+    /// Returns the implementation of `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> {
+    pub fn get_partial_eq_impl(
+        &self,
+        bevy_reflect_path: &Path,
+    ) -> 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_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, value))
+                fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option<bool> {
+                    Some(#impl_fn(self, value))
+                }
             }),
             TraitImpl::NotImplemented => None,
         }
     }
 
-    /// Returns the logic for `Reflect::serializable` as a `TokenStream`.
+    /// Returns the implementation of `Reflect::serializable` as a `TokenStream`.
     ///
     /// If `Serialize` was not registered, returns `None`.
-    pub fn get_serialize_impl(&self, path: &Path) -> Option<proc_macro2::TokenStream> {
+    pub fn get_serialize_impl(&self, bevy_reflect_path: &Path) -> Option<proc_macro2::TokenStream> {
         match &self.serialize {
             TraitImpl::Implemented => Some(quote! {
-                Some(#path::serde::Serializable::Borrowed(self))
+                fn serializable(&self) -> Option<#bevy_reflect_path::serde::Serializable> {
+                    Some(#bevy_reflect_path::serde::Serializable::Borrowed(self))
+                }
             }),
             TraitImpl::Custom(impl_fn) => Some(quote! {
-                Some(#impl_fn(self))
+                fn serializable(&self) -> Option<#bevy_reflect_path::serde::Serializable> {
+                    Some(#impl_fn(self))
+                }
             }),
             TraitImpl::NotImplemented => None,
         }

crates/bevy_reflect/bevy_reflect_derive/src/impls.rs

@@ -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
         }
     })
 }

crates/bevy_reflect/src/impls/smallvec.rs

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

crates/bevy_reflect/src/impls/std.rs

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

crates/bevy_reflect/src/map.rs

@@ -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`].

crates/bevy_reflect/src/reflect.rs

@@ -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.

crates/bevy_reflect/src/struct_trait.rs

@@ -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.

crates/bevy_reflect/src/tuple.rs

@@ -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,)*) {