From cf9b28793141f30a0306c0b02eaa42840768b3aa Mon Sep 17 00:00:00 2001 From: makspll Date: Thu, 7 Jul 2022 15:28:54 +0100 Subject: [PATCH 01/20] add `skip_serializing` flag to reflect macros --- .../bevy_reflect_derive/src/derive_data.rs | 49 +++++++--- .../src/field_attributes.rs | 32 +++++-- .../bevy_reflect_derive/src/from_reflect.rs | 2 + .../bevy_reflect_derive/src/impls.rs | 8 +- .../bevy_reflect_derive/src/lib.rs | 1 + .../bevy_reflect_derive/src/registration.rs | 3 + crates/bevy_reflect/src/serde/mod.rs | 90 +++++++++++++++++++ crates/bevy_reflect/src/serde/ser.rs | 25 +++++- crates/bevy_reflect/src/struct_trait.rs | 2 +- crates/bevy_reflect/src/type_registry.rs | 66 ++++++++++++++ examples/scene/scene.rs | 4 +- 11 files changed, 257 insertions(+), 25 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 219fe8158bcac..30e1e59d2173f 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -1,5 +1,5 @@ use crate::container_attributes::ReflectTraits; -use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr}; +use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr, ReflectFieldIgnore}; use crate::utility::get_bevy_reflect_path; use crate::{REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME}; use syn::{Data, DataStruct, DeriveInput, Field, Fields, Generics, Ident, Meta, Path}; @@ -137,21 +137,34 @@ impl<'a> ReflectDeriveData<'a> { Ok(output) } - /// Get an iterator over the active fields - pub fn active_fields(&self) -> impl Iterator> { - self.fields.iter().filter(|field| !field.attrs.ignore) + /// Get an iterator over the fields satisfying the given predicate + fn fields_with bool>( + &self, + predicate: F, + ) -> impl Iterator> { + self.fields.iter().filter(predicate) } - /// Get an iterator over the ignored fields - pub fn ignored_fields(&self) -> impl Iterator> { - self.fields.iter().filter(|field| field.attrs.ignore) + /// Get a collection of all field types satisfying the given predicate + fn types_with bool>(&self, predicate: F) -> Vec { + self.fields_with(predicate) + .map(|field| field.data.ty.clone()) + .collect::>() } - /// Get a collection of all active types + /// Get a collection of types which are exposed to either the scene serialization or reflection API pub fn active_types(&self) -> Vec { - self.active_fields() - .map(|field| field.data.ty.clone()) - .collect::>() + self.types_with(|field| field.attrs.ignore != ReflectFieldIgnore::IgnoreAlways) + } + + /// Get a collection of fields which are exposed to the reflection API + pub fn active_fields(&self) -> impl Iterator> { + self.fields_with(|field| field.attrs.ignore != ReflectFieldIgnore::IgnoreAlways) + } + + /// Get a collection of fields which are ignored from the reflection API + pub fn ignored_fields(&self) -> impl Iterator> { + self.fields_with(|field| field.attrs.ignore != ReflectFieldIgnore::None) } /// The [`DeriveType`] of this struct. @@ -187,11 +200,25 @@ impl<'a> ReflectDeriveData<'a> { /// Returns the `GetTypeRegistration` impl as a `TokenStream`. pub fn get_type_registration(&self) -> proc_macro2::TokenStream { + let mut idxs = Vec::default(); + self.fields.iter().fold(0, |next_idx, field| { + if field.attrs.ignore == ReflectFieldIgnore::IgnoreAlways { + idxs.push(next_idx); + next_idx + } else if field.attrs.ignore == ReflectFieldIgnore::IgnoreSerialization { + idxs.push(next_idx); + next_idx + 1 + } else { + next_idx + 1 + } + }); + crate::registration::impl_get_type_registration( self.type_name, &self.bevy_reflect_path, self.traits.idents(), self.generics, + &idxs, ) } } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs index a1f7c0e8999cb..2d2f87f6cc2b2 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs @@ -9,14 +9,28 @@ use quote::ToTokens; use syn::spanned::Spanned; use syn::{Attribute, Lit, Meta, NestedMeta}; -pub(crate) static IGNORE_ATTR: &str = "ignore"; +pub(crate) static IGNORE_SERIALIZATION_ATTR: &str = "skip_serializing"; +pub(crate) static IGNORE_ALL_ATTR: &str = "ignore"; + pub(crate) static DEFAULT_ATTR: &str = "default"; +/// Stores data about if the field should be visible via the Reflect and serialization interfaces +#[derive(Default, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ReflectFieldIgnore { + /// Don't ignore, appear to all systems + #[default] + None, + /// Ignore when serializing but not when reflecting + IgnoreSerialization, + /// Ignore for all systems + IgnoreAlways, +} + /// A container for attributes defined on a reflected type's field. #[derive(Default)] pub(crate) struct ReflectFieldAttr { - /// Determines if this field should be ignored. - pub ignore: bool, + /// Determines how this field should be ignored if at all. + pub ignore: ReflectFieldIgnore, /// Sets the default behavior of this field. pub default: DefaultBehavior, } @@ -65,9 +79,15 @@ pub(crate) fn parse_field_attrs(attrs: &[Attribute]) -> Result Result<(), syn::Error> { match meta { - Meta::Path(path) if path.is_ident(IGNORE_ATTR) => { - args.ignore = true; - Ok(()) + Meta::Path(path) if path.is_ident(IGNORE_SERIALIZATION_ATTR) => { + (args.ignore == ReflectFieldIgnore::None) + .then(|| args.ignore = ReflectFieldIgnore::IgnoreSerialization) + .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) + } + Meta::Path(path) if path.is_ident(IGNORE_ALL_ATTR) => { + (args.ignore == ReflectFieldIgnore::None) + .then(|| args.ignore = ReflectFieldIgnore::IgnoreAlways) + .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) } Meta::Path(path) if path.is_ident(DEFAULT_ATTR) => { args.default = DefaultBehavior::Default; diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs index b4c9c345f26eb..c1597f65b37d9 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs @@ -54,7 +54,9 @@ fn impl_struct_internal(derive_data: &ReflectDeriveData, is_tuple: bool) -> Toke Ident::new("Struct", Span::call_site()) }; + // types which at some point may be reachable via reflection or serialization let field_types = derive_data.active_types(); + let MemberValuePair(active_members, active_values) = get_active_fields(derive_data, &ref_struct, &ref_struct_type, is_tuple); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls.rs index 38680a35adf38..f85b3f03586bc 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls.rs @@ -32,10 +32,8 @@ pub(crate) fn impl_struct(derive_data: &ReflectDeriveData) -> TokenStream { .unwrap_or_else(|| Member::Unnamed(Index::from(field.index))) }) .collect::>(); - let field_types = derive_data - .active_fields() - .map(|field| field.data.ty.clone()) - .collect::>(); + let field_types = derive_data.active_types(); + let field_count = field_idents.len(); let field_indices = (0..field_count).collect::>(); @@ -210,10 +208,12 @@ pub(crate) fn impl_tuple_struct(derive_data: &ReflectDeriveData) -> TokenStream .active_fields() .map(|field| Member::Unnamed(Index::from(field.index))) .collect::>(); + let field_types = derive_data .active_fields() .map(|field| field.data.ty.clone()) .collect::>(); + let field_count = field_idents.len(); let field_indices = (0..field_count).collect::>(); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs index 7cf9fe560b004..4bef19ae876bb 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs @@ -108,6 +108,7 @@ pub fn impl_reflect_value(input: TokenStream) -> TokenStream { &bevy_reflect_path, registration_data, &reflect_value_def.generics, + &[], ); impls::impl_value( ty, diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index ea3157242211e..dfa8252cd9acd 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -10,6 +10,7 @@ pub(crate) fn impl_get_type_registration( bevy_reflect_path: &Path, registration_data: &[Ident], generics: &Generics, + scene_ignored_field_indices: &[usize], ) -> proc_macro2::TokenStream { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); quote! { @@ -18,6 +19,8 @@ pub(crate) fn impl_get_type_registration( fn get_type_registration() -> #bevy_reflect_path::TypeRegistration { let mut registration = #bevy_reflect_path::TypeRegistration::of::<#type_name #ty_generics>(); #(registration.insert::<#registration_data>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type());)* + let serialization_data = #bevy_reflect_path::SerializationData::new([#(#scene_ignored_field_indices),*].into_iter()); + registration.set_serialization_data(serialization_data); registration } } diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index 5e664112a8860..376d7ecdfb789 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -14,3 +14,93 @@ pub(crate) mod type_fields { pub const ARRAY: &str = "array"; pub const VALUE: &str = "value"; } + +#[cfg(test)] +mod tests { + use crate::{self as bevy_reflect, DynamicTupleStruct}; + use crate::{ + serde::{ReflectDeserializer, ReflectSerializer}, + type_registry::{GetTypeRegistration, TypeRegistry}, + DynamicStruct, Reflect, + }; + use serde::de::DeserializeSeed; + + #[test] + fn test_serialization_struct() { + #[derive(Debug, Reflect, PartialEq)] + #[reflect(PartialEq)] + struct TestStruct { + a: i32, + #[reflect(ignore)] + b: i32, + #[reflect(skip_serializing)] + c: i32, + d: i32, + } + + let mut registry = TypeRegistry::default(); + let registration = TestStruct::get_type_registration(); + registry.add_registration(registration); + + let test_struct = TestStruct { + a: 3, + b: 4, + c: 5, + d: 6, + }; + + let serializer = ReflectSerializer::new(&test_struct, ®istry); + let serialized = + ron::ser::to_string_pretty(&serializer, ron::ser::PrettyConfig::default()).unwrap(); + + let mut expected = DynamicStruct::default(); + expected.insert("a", 3); + expected.insert("d", 6); + + let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap(); + let reflect_deserializer = ReflectDeserializer::new(®istry); + let value = reflect_deserializer.deserialize(&mut deserializer).unwrap(); + let deserialized = value.take::().unwrap(); + + assert!( + expected.reflect_partial_eq(&deserialized).unwrap(), + "Expected {expected:?} found {deserialized:?}" + ); + } + + #[test] + fn test_serialization_tuple_struct() { + #[derive(Debug, Reflect, PartialEq)] + #[reflect(PartialEq)] + struct TestStruct( + i32, + #[reflect(ignore)] i32, + #[reflect(skip_serializing)] i32, + i32, + ); + + let mut registry = TypeRegistry::default(); + let registration = TestStruct::get_type_registration(); + registry.add_registration(registration); + + let test_struct = TestStruct(3, 4, 5, 6); + + let serializer = ReflectSerializer::new(&test_struct, ®istry); + let serialized = + ron::ser::to_string_pretty(&serializer, ron::ser::PrettyConfig::default()).unwrap(); + + let mut expected = DynamicTupleStruct::default(); + expected.insert(3); + expected.insert(6); + + let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap(); + let reflect_deserializer = ReflectDeserializer::new(®istry); + let value = reflect_deserializer.deserialize(&mut deserializer).unwrap(); + let deserialized = value.take::().unwrap(); + + assert!( + expected.reflect_partial_eq(&deserialized).unwrap(), + "Expected {expected:?} found {deserialized:?}" + ); + } +} diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index de0ad7760a6fa..8b2fe11e53236 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -148,7 +148,18 @@ impl<'a> Serialize for StructValueSerializer<'a> { S: serde::Serializer, { let mut state = serializer.serialize_map(Some(self.struct_value.field_len()))?; + let data = self + .registry + .get_with_name(self.struct_value.type_name()) + .and_then(|registration| registration.serialization_data()); + for (index, value) in self.struct_value.iter_fields().enumerate() { + if data + .map(|data| data.is_ignored_index(index)) + .unwrap_or(false) + { + continue; + } let key = self.struct_value.name_at(index).unwrap(); state.serialize_entry(key, &ReflectSerializer::new(value, self.registry))?; } @@ -191,7 +202,18 @@ impl<'a> Serialize for TupleStructValueSerializer<'a> { S: serde::Serializer, { let mut state = serializer.serialize_seq(Some(self.tuple_struct.field_len()))?; - for value in self.tuple_struct.iter_fields() { + let data = self + .registry + .get_with_name(self.tuple_struct.type_name()) + .and_then(|registration| registration.serialization_data()); + + for (index, value) in self.tuple_struct.iter_fields().enumerate() { + if data + .map(|data| data.is_ignored_index(index)) + .unwrap_or(false) + { + continue; + } state.serialize_element(&ReflectSerializer::new(value, self.registry))?; } state.end() @@ -233,6 +255,7 @@ impl<'a> Serialize for TupleValueSerializer<'a> { S: serde::Serializer, { let mut state = serializer.serialize_seq(Some(self.tuple.field_len()))?; + for value in self.tuple.iter_fields() { state.serialize_element(&ReflectSerializer::new(value, self.registry))?; } diff --git a/crates/bevy_reflect/src/struct_trait.rs b/crates/bevy_reflect/src/struct_trait.rs index 60a9ba9e6125e..5c6c3dfd959e4 100644 --- a/crates/bevy_reflect/src/struct_trait.rs +++ b/crates/bevy_reflect/src/struct_trait.rs @@ -59,7 +59,7 @@ pub trait Struct: Reflect { /// Returns the number of fields in the struct. fn field_len(&self) -> usize; - /// Returns an iterator over the values of the struct's fields. + /// Returns an iterator over the values of the reflectable fields for this struct. fn iter_fields(&self) -> FieldIter; /// Clones the struct into a [`DynamicStruct`]. diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 97e84fef3a56f..80d04882e4c1a 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -189,6 +189,25 @@ impl TypeRegistry { .and_then(|registration| registration.data_mut::()) } + /// Returns a reference to the [`SerializationData`] for the type associated with the given `TypeId`. + /// + /// If the specified type has not been registered, or if it has no associated [`SerializationData`], returns `None`. + pub fn get_serialization_data(&self, type_id: TypeId) -> Option<&SerializationData> { + self.get(type_id) + .and_then(|registration| registration.serialization_data()) + } + + /// Returns a mutable reference to the [`SerializationData`] for the type associated with the given `TypeId`. + /// + /// If the specified type has not been registered, or if it has no associated [`SerializationData`], returns `None`. + pub fn get_serialization_data_mut( + &mut self, + type_id: TypeId, + ) -> Option<&mut SerializationData> { + self.get_mut(type_id) + .and_then(|registration| registration.serialization_data_mut()) + } + /// Returns the [`TypeInfo`] associated with the given `TypeId`. /// /// If the specified type has not been registered, returns `None`. @@ -237,10 +256,21 @@ impl TypeRegistryArc { /// [1]: crate::Reflect pub struct TypeRegistration { short_name: String, + serialization_data: Option, data: HashMap>, type_info: &'static TypeInfo, } +impl Debug for TypeRegistration { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TypeRegistration") + .field("short_name", &self.short_name) + .field("serialization_data", &self.serialization_data) + .field("type_info", &self.type_info) + .finish() + } +} + impl TypeRegistration { /// Returns the [`TypeId`] of the type. /// @@ -270,6 +300,14 @@ impl TypeRegistration { .and_then(|value| value.downcast_mut()) } + pub fn serialization_data(&self) -> Option<&SerializationData> { + self.serialization_data.as_ref() + } + + pub fn serialization_data_mut(&mut self) -> Option<&mut SerializationData> { + self.serialization_data.as_mut() + } + /// Returns a reference to the registration's [`TypeInfo`] pub fn type_info(&self) -> &'static TypeInfo { self.type_info @@ -282,11 +320,19 @@ impl TypeRegistration { self.data.insert(TypeId::of::(), Box::new(data)); } + /// Set the scene serialization data of this registration. + /// + /// If another instance was previously set, it is replaced. + pub fn set_serialization_data(&mut self, data: SerializationData) { + self.serialization_data = Some(data); + } + /// Creates type registration information for `T`. pub fn of() -> Self { let type_name = std::any::type_name::(); Self { data: HashMap::default(), + serialization_data: None, short_name: bevy_utils::get_short_name(type_name), type_info: T::type_info(), } @@ -316,12 +362,32 @@ impl Clone for TypeRegistration { TypeRegistration { data, + serialization_data: self.serialization_data.clone(), short_name: self.short_name.clone(), type_info: self.type_info, } } } +/// Contains data relevant to the serialization of a type into the scene. +#[derive(Default, Debug, Clone)] +pub struct SerializationData { + ignored_field_indices: HashSet, +} + +impl SerializationData { + pub fn new>(ignored_field_indices: I) -> Self { + Self { + ignored_field_indices: ignored_field_indices.collect(), + } + } + + /// Returns true if the given index corresponds to a field meant to be ignored in serialization. + pub fn is_ignored_index(&self, index: usize) -> bool { + self.ignored_field_indices.contains(&index) + } +} + /// A trait for types generated by the [`#[reflect_trait]`][0] attribute macro. /// /// [0]: crate::reflect_trait diff --git a/examples/scene/scene.rs b/examples/scene/scene.rs index c6886e2244327..14dea33070cb5 100644 --- a/examples/scene/scene.rs +++ b/examples/scene/scene.rs @@ -28,14 +28,14 @@ struct ComponentA { } // Some components have fields that cannot (or should not) be written to scene files. These can be -// ignored with the #[reflect(ignore)] attribute. This is also generally where the `FromWorld` +// ignored with the #[reflect(skip_serializing)] attribute. This is also generally where the `FromWorld` // trait comes into play. `FromWorld` gives you access to your App's current ECS `Resources` // when you construct your component. #[derive(Component, Reflect)] #[reflect(Component)] struct ComponentB { pub value: String, - #[reflect(ignore)] + #[reflect(skip_serializing)] pub _time_since_startup: Duration, } From d6142523a35657df121d7ee99b8876a7e8a3f1c9 Mon Sep 17 00:00:00 2001 From: makspll Date: Thu, 7 Jul 2022 15:28:54 +0100 Subject: [PATCH 02/20] add `skip_serializing` flag to reflect macros --- .../bevy_reflect_derive/src/derive_data.rs | 49 +++++++--- .../src/field_attributes.rs | 32 +++++-- .../bevy_reflect_derive/src/registration.rs | 3 + crates/bevy_reflect/src/serde/mod.rs | 90 +++++++++++++++++++ crates/bevy_reflect/src/serde/ser.rs | 25 +++++- crates/bevy_reflect/src/struct_trait.rs | 2 +- crates/bevy_reflect/src/type_registry.rs | 66 ++++++++++++++ examples/scene/scene.rs | 4 +- 8 files changed, 251 insertions(+), 20 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 236c0e627ae3f..2c4c791025669 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -1,5 +1,5 @@ use crate::container_attributes::ReflectTraits; -use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr}; +use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr, ReflectFieldIgnore}; use quote::quote; use crate::{utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME}; @@ -241,11 +241,25 @@ impl<'a> ReflectMeta<'a> { /// Returns the `GetTypeRegistration` impl as a `TokenStream`. pub fn get_type_registration(&self) -> proc_macro2::TokenStream { + let mut idxs = Vec::default(); + self.fields.iter().fold(0, |next_idx, field| { + if field.attrs.ignore == ReflectFieldIgnore::IgnoreAlways { + idxs.push(next_idx); + next_idx + } else if field.attrs.ignore == ReflectFieldIgnore::IgnoreSerialization { + idxs.push(next_idx); + next_idx + 1 + } else { + next_idx + 1 + } + }); + crate::registration::impl_get_type_registration( self.type_name, &self.bevy_reflect_path, self.traits.idents(), self.generics, + &idxs, ) } } @@ -256,19 +270,34 @@ impl<'a> ReflectStruct<'a> { &self.meta } - /// Get an iterator over the active fields. - pub fn active_fields(&self) -> impl Iterator> { - self.fields.iter().filter(|field| !field.attrs.ignore) + /// Get an iterator over the fields satisfying the given predicate + fn fields_with bool>( + &self, + predicate: F, + ) -> impl Iterator> { + self.fields.iter().filter(predicate) } - /// Get an iterator over the ignored fields. - pub fn ignored_fields(&self) -> impl Iterator> { - self.fields.iter().filter(|field| field.attrs.ignore) + /// Get a collection of all field types satisfying the given predicate + fn types_with bool>(&self, predicate: F) -> Vec { + self.fields_with(predicate) + .map(|field| field.data.ty.clone()) + .collect::>() + } + + /// Get a collection of types which are exposed to either the scene serialization or reflection API + pub fn active_types(&self) -> Vec { + self.types_with(|field| field.attrs.ignore != ReflectFieldIgnore::IgnoreAlways) } - /// Get a collection of all active types. - pub fn active_types(&self) -> impl Iterator { - self.active_fields().map(|field| &field.data.ty) + /// Get a collection of fields which are exposed to the reflection API + pub fn active_fields(&self) -> impl Iterator> { + self.fields_with(|field| field.attrs.ignore != ReflectFieldIgnore::IgnoreAlways) + } + + /// Get a collection of fields which are ignored from the reflection API + pub fn ignored_fields(&self) -> impl Iterator> { + self.fields_with(|field| field.attrs.ignore != ReflectFieldIgnore::None) } /// The complete set of fields in this struct. diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs index 362fd93f54912..996f59f9982df 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs @@ -9,14 +9,28 @@ use quote::ToTokens; use syn::spanned::Spanned; use syn::{Attribute, Lit, Meta, NestedMeta}; -pub(crate) static IGNORE_ATTR: &str = "ignore"; +pub(crate) static IGNORE_SERIALIZATION_ATTR: &str = "skip_serializing"; +pub(crate) static IGNORE_ALL_ATTR: &str = "ignore"; + pub(crate) static DEFAULT_ATTR: &str = "default"; +/// Stores data about if the field should be visible via the Reflect and serialization interfaces +#[derive(Default, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ReflectFieldIgnore { + /// Don't ignore, appear to all systems + #[default] + None, + /// Ignore when serializing but not when reflecting + IgnoreSerialization, + /// Ignore for all systems + IgnoreAlways, +} + /// A container for attributes defined on a reflected type's field. #[derive(Default)] pub(crate) struct ReflectFieldAttr { - /// Determines if this field should be ignored. - pub ignore: bool, + /// Determines how this field should be ignored if at all. + pub ignore: ReflectFieldIgnore, /// Sets the default behavior of this field. pub default: DefaultBehavior, } @@ -65,9 +79,15 @@ pub(crate) fn parse_field_attrs(attrs: &[Attribute]) -> Result Result<(), syn::Error> { match meta { - Meta::Path(path) if path.is_ident(IGNORE_ATTR) => { - args.ignore = true; - Ok(()) + Meta::Path(path) if path.is_ident(IGNORE_SERIALIZATION_ATTR) => { + (args.ignore == ReflectFieldIgnore::None) + .then(|| args.ignore = ReflectFieldIgnore::IgnoreSerialization) + .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) + } + Meta::Path(path) if path.is_ident(IGNORE_ALL_ATTR) => { + (args.ignore == ReflectFieldIgnore::None) + .then(|| args.ignore = ReflectFieldIgnore::IgnoreAlways) + .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) } Meta::Path(path) if path.is_ident(DEFAULT_ATTR) => { args.default = DefaultBehavior::Default; diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index 7b13f7d352ef8..0d03bf2413156 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -10,6 +10,7 @@ pub(crate) fn impl_get_type_registration( bevy_reflect_path: &Path, registration_data: &[Ident], generics: &Generics, + scene_ignored_field_indices: &[usize], ) -> proc_macro2::TokenStream { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); quote! { @@ -19,6 +20,8 @@ pub(crate) fn impl_get_type_registration( let mut registration = #bevy_reflect_path::TypeRegistration::of::<#type_name #ty_generics>(); registration.insert::<#bevy_reflect_path::ReflectFromPtr>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type()); #(registration.insert::<#registration_data>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type());)* + let serialization_data = #bevy_reflect_path::SerializationData::new([#(#scene_ignored_field_indices),*].into_iter()); + registration.set_serialization_data(serialization_data); registration } } diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index 7c7c18f4ea954..855f30ebecea2 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -16,3 +16,93 @@ pub(crate) mod type_fields { pub const ARRAY: &str = "array"; pub const VALUE: &str = "value"; } + +#[cfg(test)] +mod tests { + use crate::{self as bevy_reflect, DynamicTupleStruct}; + use crate::{ + serde::{ReflectDeserializer, ReflectSerializer}, + type_registry::{GetTypeRegistration, TypeRegistry}, + DynamicStruct, Reflect, + }; + use serde::de::DeserializeSeed; + + #[test] + fn test_serialization_struct() { + #[derive(Debug, Reflect, PartialEq)] + #[reflect(PartialEq)] + struct TestStruct { + a: i32, + #[reflect(ignore)] + b: i32, + #[reflect(skip_serializing)] + c: i32, + d: i32, + } + + let mut registry = TypeRegistry::default(); + let registration = TestStruct::get_type_registration(); + registry.add_registration(registration); + + let test_struct = TestStruct { + a: 3, + b: 4, + c: 5, + d: 6, + }; + + let serializer = ReflectSerializer::new(&test_struct, ®istry); + let serialized = + ron::ser::to_string_pretty(&serializer, ron::ser::PrettyConfig::default()).unwrap(); + + let mut expected = DynamicStruct::default(); + expected.insert("a", 3); + expected.insert("d", 6); + + let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap(); + let reflect_deserializer = ReflectDeserializer::new(®istry); + let value = reflect_deserializer.deserialize(&mut deserializer).unwrap(); + let deserialized = value.take::().unwrap(); + + assert!( + expected.reflect_partial_eq(&deserialized).unwrap(), + "Expected {expected:?} found {deserialized:?}" + ); + } + + #[test] + fn test_serialization_tuple_struct() { + #[derive(Debug, Reflect, PartialEq)] + #[reflect(PartialEq)] + struct TestStruct( + i32, + #[reflect(ignore)] i32, + #[reflect(skip_serializing)] i32, + i32, + ); + + let mut registry = TypeRegistry::default(); + let registration = TestStruct::get_type_registration(); + registry.add_registration(registration); + + let test_struct = TestStruct(3, 4, 5, 6); + + let serializer = ReflectSerializer::new(&test_struct, ®istry); + let serialized = + ron::ser::to_string_pretty(&serializer, ron::ser::PrettyConfig::default()).unwrap(); + + let mut expected = DynamicTupleStruct::default(); + expected.insert(3); + expected.insert(6); + + let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap(); + let reflect_deserializer = ReflectDeserializer::new(®istry); + let value = reflect_deserializer.deserialize(&mut deserializer).unwrap(); + let deserialized = value.take::().unwrap(); + + assert!( + expected.reflect_partial_eq(&deserialized).unwrap(), + "Expected {expected:?} found {deserialized:?}" + ); + } +} diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index 0dd239d6305a7..8dd0ad063f332 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -154,7 +154,18 @@ impl<'a> Serialize for StructValueSerializer<'a> { S: serde::Serializer, { let mut state = serializer.serialize_map(Some(self.struct_value.field_len()))?; + let data = self + .registry + .get_with_name(self.struct_value.type_name()) + .and_then(|registration| registration.serialization_data()); + for (index, value) in self.struct_value.iter_fields().enumerate() { + if data + .map(|data| data.is_ignored_index(index)) + .unwrap_or(false) + { + continue; + } let key = self.struct_value.name_at(index).unwrap(); state.serialize_entry(key, &ReflectSerializer::new(value, self.registry))?; } @@ -197,7 +208,18 @@ impl<'a> Serialize for TupleStructValueSerializer<'a> { S: serde::Serializer, { let mut state = serializer.serialize_seq(Some(self.tuple_struct.field_len()))?; - for value in self.tuple_struct.iter_fields() { + let data = self + .registry + .get_with_name(self.tuple_struct.type_name()) + .and_then(|registration| registration.serialization_data()); + + for (index, value) in self.tuple_struct.iter_fields().enumerate() { + if data + .map(|data| data.is_ignored_index(index)) + .unwrap_or(false) + { + continue; + } state.serialize_element(&ReflectSerializer::new(value, self.registry))?; } state.end() @@ -350,6 +372,7 @@ impl<'a> Serialize for TupleValueSerializer<'a> { S: serde::Serializer, { let mut state = serializer.serialize_seq(Some(self.tuple.field_len()))?; + for value in self.tuple.iter_fields() { state.serialize_element(&ReflectSerializer::new(value, self.registry))?; } diff --git a/crates/bevy_reflect/src/struct_trait.rs b/crates/bevy_reflect/src/struct_trait.rs index 4f0b7819f258d..a1dc934b9efaf 100644 --- a/crates/bevy_reflect/src/struct_trait.rs +++ b/crates/bevy_reflect/src/struct_trait.rs @@ -59,7 +59,7 @@ pub trait Struct: Reflect { /// Returns the number of fields in the struct. fn field_len(&self) -> usize; - /// Returns an iterator over the values of the struct's fields. + /// Returns an iterator over the values of the reflectable fields for this struct. fn iter_fields(&self) -> FieldIter; /// Clones the struct into a [`DynamicStruct`]. diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 74865abeb0c91..3e99b5e42204d 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -220,6 +220,25 @@ impl TypeRegistry { .and_then(|registration| registration.data_mut::()) } + /// Returns a reference to the [`SerializationData`] for the type associated with the given `TypeId`. + /// + /// If the specified type has not been registered, or if it has no associated [`SerializationData`], returns `None`. + pub fn get_serialization_data(&self, type_id: TypeId) -> Option<&SerializationData> { + self.get(type_id) + .and_then(|registration| registration.serialization_data()) + } + + /// Returns a mutable reference to the [`SerializationData`] for the type associated with the given `TypeId`. + /// + /// If the specified type has not been registered, or if it has no associated [`SerializationData`], returns `None`. + pub fn get_serialization_data_mut( + &mut self, + type_id: TypeId, + ) -> Option<&mut SerializationData> { + self.get_mut(type_id) + .and_then(|registration| registration.serialization_data_mut()) + } + /// Returns the [`TypeInfo`] associated with the given `TypeId`. /// /// If the specified type has not been registered, returns `None`. @@ -268,10 +287,21 @@ impl TypeRegistryArc { /// [1]: crate::Reflect pub struct TypeRegistration { short_name: String, + serialization_data: Option, data: HashMap>, type_info: &'static TypeInfo, } +impl Debug for TypeRegistration { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TypeRegistration") + .field("short_name", &self.short_name) + .field("serialization_data", &self.serialization_data) + .field("type_info", &self.type_info) + .finish() + } +} + impl TypeRegistration { /// Returns the [`TypeId`] of the type. /// @@ -301,6 +331,14 @@ impl TypeRegistration { .and_then(|value| value.downcast_mut()) } + pub fn serialization_data(&self) -> Option<&SerializationData> { + self.serialization_data.as_ref() + } + + pub fn serialization_data_mut(&mut self) -> Option<&mut SerializationData> { + self.serialization_data.as_mut() + } + /// Returns a reference to the registration's [`TypeInfo`] pub fn type_info(&self) -> &'static TypeInfo { self.type_info @@ -313,11 +351,19 @@ impl TypeRegistration { self.data.insert(TypeId::of::(), Box::new(data)); } + /// Set the scene serialization data of this registration. + /// + /// If another instance was previously set, it is replaced. + pub fn set_serialization_data(&mut self, data: SerializationData) { + self.serialization_data = Some(data); + } + /// Creates type registration information for `T`. pub fn of() -> Self { let type_name = std::any::type_name::(); Self { data: HashMap::default(), + serialization_data: None, short_name: bevy_utils::get_short_name(type_name), type_info: T::type_info(), } @@ -347,12 +393,32 @@ impl Clone for TypeRegistration { TypeRegistration { data, + serialization_data: self.serialization_data.clone(), short_name: self.short_name.clone(), type_info: self.type_info, } } } +/// Contains data relevant to the serialization of a type into the scene. +#[derive(Default, Debug, Clone)] +pub struct SerializationData { + ignored_field_indices: HashSet, +} + +impl SerializationData { + pub fn new>(ignored_field_indices: I) -> Self { + Self { + ignored_field_indices: ignored_field_indices.collect(), + } + } + + /// Returns true if the given index corresponds to a field meant to be ignored in serialization. + pub fn is_ignored_index(&self, index: usize) -> bool { + self.ignored_field_indices.contains(&index) + } +} + /// A trait for types generated by the [`#[reflect_trait]`][0] attribute macro. /// /// [0]: crate::reflect_trait diff --git a/examples/scene/scene.rs b/examples/scene/scene.rs index 86be502230196..2515a84df5437 100644 --- a/examples/scene/scene.rs +++ b/examples/scene/scene.rs @@ -28,14 +28,14 @@ struct ComponentA { } // Some components have fields that cannot (or should not) be written to scene files. These can be -// ignored with the #[reflect(ignore)] attribute. This is also generally where the `FromWorld` +// ignored with the #[reflect(skip_serializing)] attribute. This is also generally where the `FromWorld` // trait comes into play. `FromWorld` gives you access to your App's current ECS `Resources` // when you construct your component. #[derive(Component, Reflect)] #[reflect(Component)] struct ComponentB { pub value: String, - #[reflect(ignore)] + #[reflect(skip_serializing)] pub _time_since_startup: Duration, } From e249d97fe4aafb95517a8cb235b48de0350ce9ee Mon Sep 17 00:00:00 2001 From: makspll Date: Thu, 15 Sep 2022 12:48:34 +0100 Subject: [PATCH 03/20] rebase changes --- .../bevy_reflect_derive/Cargo.toml | 1 + .../bevy_reflect_derive/src/derive_data.rs | 87 +++++++++---------- .../bevy_reflect_derive/src/enum_utility.rs | 4 +- .../src/field_attributes.rs | 34 ++++++-- .../bevy_reflect_derive/src/impls/enums.rs | 4 +- .../bevy_reflect_derive/src/lib.rs | 3 + .../bevy_reflect_derive/src/registration.rs | 6 +- .../bevy_reflect_derive/src/utility.rs | 41 +++++++++ crates/bevy_reflect/src/enums/dynamic_enum.rs | 3 +- crates/bevy_reflect/src/serde/ser.rs | 4 +- crates/bevy_reflect/src/tuple.rs | 2 +- crates/bevy_reflect/src/type_registry.rs | 22 +++-- 12 files changed, 142 insertions(+), 69 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml b/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml index ef6ba3a7733c1..c7c947537fec7 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml +++ b/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml @@ -18,3 +18,4 @@ syn = { version = "1.0", features = ["full"] } proc-macro2 = "1.0" quote = "1.0" uuid = { version = "1.1", features = ["v4"] } +bit-set = "0.5.2" \ No newline at end of file diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 2c4c791025669..be1c7cd15f590 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -1,11 +1,15 @@ +use std::iter::empty; + use crate::container_attributes::ReflectTraits; -use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr, ReflectFieldIgnore}; +use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr, ReflectIgnoreBehaviour}; +use crate::utility::members_to_serialization_blacklist; +use bit_set::BitSet; use quote::quote; use crate::{utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME}; use syn::punctuated::Punctuated; use syn::spanned::Spanned; -use syn::{Data, DeriveInput, Field, Fields, Generics, Ident, Meta, Path, Token, Type, Variant}; +use syn::{Data, DeriveInput, Field, Fields, Generics, Ident, Meta, Path, Token, Variant}; pub(crate) enum ReflectDerive<'a> { Struct(ReflectStruct<'a>), @@ -37,6 +41,8 @@ pub(crate) struct ReflectMeta<'a> { generics: &'a Generics, /// A cached instance of the path to the `bevy_reflect` crate. bevy_reflect_path: Path, + /// A collection corresponding to `ignored` fields' indices during serialization. + serialization_blacklist: BitSet, } /// Struct data used by derive macros for `Reflect` and `FromReflect`. @@ -92,6 +98,7 @@ pub(crate) struct EnumVariant<'a> { /// The fields within this variant. pub fields: EnumVariantFields<'a>, /// The reflection-based attributes on the variant. + #[allow(dead_code)] pub attrs: ReflectFieldAttr, /// The index of this variant within the enum. #[allow(dead_code)] @@ -125,19 +132,25 @@ impl<'a> ReflectDerive<'a> { _ => continue, } } - - let meta = ReflectMeta::new(&input.ident, &input.generics, traits); - if force_reflect_value { - return Ok(Self::Value(meta)); + return Ok(Self::Value(ReflectMeta::new( + &input.ident, + &input.generics, + traits, + empty(), + ))); } return match &input.data { Data::Struct(data) => { - let reflect_struct = ReflectStruct { - meta, - fields: Self::collect_struct_fields(&data.fields)?, - }; + let fields = Self::collect_struct_fields(&data.fields)?; + let meta = ReflectMeta::new( + &input.ident, + &input.generics, + traits, + fields.iter().map(|f| f.attrs.ignore), + ); + let reflect_struct = ReflectStruct { meta, fields }; match data.fields { Fields::Named(..) => Ok(Self::Struct(reflect_struct)), @@ -146,10 +159,10 @@ impl<'a> ReflectDerive<'a> { } } Data::Enum(data) => { - let reflect_enum = ReflectEnum { - meta, - variants: Self::collect_enum_variants(&data.variants)?, - }; + let variants = Self::collect_enum_variants(&data.variants)?; + let meta = ReflectMeta::new(&input.ident, &input.generics, traits, empty()); + + let reflect_enum = ReflectEnum { meta, variants }; Ok(Self::Enum(reflect_enum)) } Data::Union(..) => Err(syn::Error::new( @@ -210,12 +223,18 @@ impl<'a> ReflectDerive<'a> { } impl<'a> ReflectMeta<'a> { - pub fn new(type_name: &'a Ident, generics: &'a Generics, traits: ReflectTraits) -> Self { + pub fn new>( + type_name: &'a Ident, + generics: &'a Generics, + traits: ReflectTraits, + member_meta_iter: I, + ) -> Self { Self { traits, type_name, generics, bevy_reflect_path: utility::get_bevy_reflect_path(), + serialization_blacklist: members_to_serialization_blacklist(member_meta_iter), } } @@ -241,25 +260,12 @@ impl<'a> ReflectMeta<'a> { /// Returns the `GetTypeRegistration` impl as a `TokenStream`. pub fn get_type_registration(&self) -> proc_macro2::TokenStream { - let mut idxs = Vec::default(); - self.fields.iter().fold(0, |next_idx, field| { - if field.attrs.ignore == ReflectFieldIgnore::IgnoreAlways { - idxs.push(next_idx); - next_idx - } else if field.attrs.ignore == ReflectFieldIgnore::IgnoreSerialization { - idxs.push(next_idx); - next_idx + 1 - } else { - next_idx + 1 - } - }); - crate::registration::impl_get_type_registration( self.type_name, &self.bevy_reflect_path, self.traits.idents(), self.generics, - &idxs, + &self.serialization_blacklist, ) } } @@ -285,19 +291,19 @@ impl<'a> ReflectStruct<'a> { .collect::>() } - /// Get a collection of types which are exposed to either the scene serialization or reflection API + /// Get a collection of types which are exposed to either the serialization or reflection API pub fn active_types(&self) -> Vec { - self.types_with(|field| field.attrs.ignore != ReflectFieldIgnore::IgnoreAlways) + self.types_with(|field| field.attrs.ignore.is_active()) } - /// Get a collection of fields which are exposed to the reflection API + /// Get an iterator of fields which are exposed to the serialization or reflection API pub fn active_fields(&self) -> impl Iterator> { - self.fields_with(|field| field.attrs.ignore != ReflectFieldIgnore::IgnoreAlways) + self.fields_with(|field| field.attrs.ignore.is_active()) } - /// Get a collection of fields which are ignored from the reflection API + /// Get an iterator of fields which are ignored by the reflection and serialization API pub fn ignored_fields(&self) -> impl Iterator> { - self.fields_with(|field| field.attrs.ignore != ReflectFieldIgnore::None) + self.fields_with(|field| field.attrs.ignore.is_ignored()) } /// The complete set of fields in this struct. @@ -313,17 +319,6 @@ impl<'a> ReflectEnum<'a> { &self.meta } - /// Get an iterator over the active variants. - pub fn active_variants(&self) -> impl Iterator> { - self.variants.iter().filter(|variant| !variant.attrs.ignore) - } - - /// Get an iterator over the ignored variants. - #[allow(dead_code)] - pub fn ignored_variants(&self) -> impl Iterator> { - self.variants.iter().filter(|variant| variant.attrs.ignore) - } - /// Returns the given ident as a qualified unit variant of this enum. pub fn get_unit(&self, variant: &Ident) -> proc_macro2::TokenStream { let name = self.meta.type_name; diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs index 427db8a377e57..8b1140bb6ac71 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs @@ -24,7 +24,7 @@ pub(crate) fn get_variant_constructors( let mut variant_names = Vec::with_capacity(variant_count); let mut variant_constructors = Vec::with_capacity(variant_count); - for variant in reflect_enum.active_variants() { + for variant in reflect_enum.variants() { let ident = &variant.data.ident; let name = ident.to_string(); let variant_constructor = reflect_enum.get_unit(ident); @@ -38,7 +38,7 @@ pub(crate) fn get_variant_constructors( let mut reflect_index: usize = 0; let constructor_fields = fields.iter().enumerate().map(|(declar_index, field)| { let field_ident = ident_or_index(field.data.ident.as_ref(), declar_index); - let field_value = if field.attrs.ignore { + let field_value = if field.attrs.ignore.is_ignored() { quote! { Default::default() } } else { let error_repr = field.data.ident.as_ref().map_or_else( diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs index 996f59f9982df..51cb4ff814c60 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs @@ -15,22 +15,42 @@ pub(crate) static IGNORE_ALL_ATTR: &str = "ignore"; pub(crate) static DEFAULT_ATTR: &str = "default"; /// Stores data about if the field should be visible via the Reflect and serialization interfaces +/// +/// Note the relationship between serialization and reflection is such that a member must be reflected in order to be serialized. +/// In boolean logic this is described as: `is_serialized -> is_reflected`, this means we can reflect something without serializing it but not the other way round. +/// The `is_reflected` predicate is provided as `self.is_active()` #[derive(Default, Clone, Copy, PartialEq, Eq)] -pub(crate) enum ReflectFieldIgnore { +pub(crate) enum ReflectIgnoreBehaviour { /// Don't ignore, appear to all systems #[default] None, /// Ignore when serializing but not when reflecting IgnoreSerialization, - /// Ignore for all systems + /// Ignore both when serializing and reflecting IgnoreAlways, } +impl ReflectIgnoreBehaviour { + /// Returns `true` if the ignoring behaviour implies member is included in the reflection API, and false otherwise. + pub fn is_active(self) -> bool { + match self { + ReflectIgnoreBehaviour::None => true, + ReflectIgnoreBehaviour::IgnoreSerialization => true, + ReflectIgnoreBehaviour::IgnoreAlways => false, + } + } + + /// The exact logical opposite of `self.is_active()` returns true iff this member is not part of the reflection API whatsover (neither serialized nor reflected) + pub fn is_ignored(self) -> bool { + !self.is_active() + } +} + /// A container for attributes defined on a reflected type's field. #[derive(Default)] pub(crate) struct ReflectFieldAttr { /// Determines how this field should be ignored if at all. - pub ignore: ReflectFieldIgnore, + pub ignore: ReflectIgnoreBehaviour, /// Sets the default behavior of this field. pub default: DefaultBehavior, } @@ -80,13 +100,13 @@ pub(crate) fn parse_field_attrs(attrs: &[Attribute]) -> Result Result<(), syn::Error> { match meta { Meta::Path(path) if path.is_ident(IGNORE_SERIALIZATION_ATTR) => { - (args.ignore == ReflectFieldIgnore::None) - .then(|| args.ignore = ReflectFieldIgnore::IgnoreSerialization) + (args.ignore == ReflectIgnoreBehaviour::None) + .then(|| args.ignore = ReflectIgnoreBehaviour::IgnoreSerialization) .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) } Meta::Path(path) if path.is_ident(IGNORE_ALL_ATTR) => { - (args.ignore == ReflectFieldIgnore::None) - .then(|| args.ignore = ReflectFieldIgnore::IgnoreAlways) + (args.ignore == ReflectIgnoreBehaviour::None) + .then(|| args.ignore = ReflectIgnoreBehaviour::IgnoreAlways) .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) } Meta::Path(path) if path.is_ident(DEFAULT_ATTR) => { diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs index 6140f853eee69..83fceda4e7404 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs @@ -269,7 +269,7 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden let mut enum_variant_name = Vec::new(); let mut enum_variant_type = Vec::new(); - for variant in reflect_enum.active_variants() { + for variant in reflect_enum.variants() { let ident = &variant.data.ident; let name = ident.to_string(); let unit = reflect_enum.get_unit(ident); @@ -281,7 +281,7 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden let mut constructor_argument = Vec::new(); let mut reflect_idx = 0; for field in fields.iter() { - if field.attrs.ignore { + if field.attrs.ignore.is_ignored() { // Ignored field continue; } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs index d7a7515683a11..affdd100c8449 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs @@ -30,6 +30,7 @@ use crate::derive_data::{ReflectDerive, ReflectMeta, ReflectStruct}; use proc_macro::TokenStream; use quote::quote; use reflect_value::ReflectValueDef; +use std::iter::empty; use syn::spanned::Spanned; use syn::{parse_macro_input, DeriveInput}; @@ -99,6 +100,7 @@ pub fn impl_reflect_value(input: TokenStream) -> TokenStream { &def.type_name, &def.generics, def.traits.unwrap_or_default(), + empty(), )) } @@ -175,5 +177,6 @@ pub fn impl_from_reflect_value(input: TokenStream) -> TokenStream { &def.type_name, &def.generics, def.traits.unwrap_or_default(), + empty(), )) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index 0d03bf2413156..4701e2e3923ee 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -1,5 +1,6 @@ //! Contains code related specifically to Bevy's type registration. +use bit_set::BitSet; use proc_macro2::Ident; use quote::quote; use syn::{Generics, Path}; @@ -10,9 +11,10 @@ pub(crate) fn impl_get_type_registration( bevy_reflect_path: &Path, registration_data: &[Ident], generics: &Generics, - scene_ignored_field_indices: &[usize], + serialization_blacklist: &BitSet, ) -> proc_macro2::TokenStream { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + let serialization_blacklist_indices = serialization_blacklist.iter().map(|v| v as usize); quote! { #[allow(unused_mut)] impl #impl_generics #bevy_reflect_path::GetTypeRegistration for #type_name #ty_generics #where_clause { @@ -20,7 +22,7 @@ pub(crate) fn impl_get_type_registration( let mut registration = #bevy_reflect_path::TypeRegistration::of::<#type_name #ty_generics>(); registration.insert::<#bevy_reflect_path::ReflectFromPtr>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type()); #(registration.insert::<#registration_data>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type());)* - let serialization_data = #bevy_reflect_path::SerializationData::new([#(#scene_ignored_field_indices),*].into_iter()); + let serialization_data = #bevy_reflect_path::SerializationData::new([#(#serialization_blacklist_indices),*].into_iter()); registration.set_serialization_data(serialization_data); registration } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index f8894689a3858..e1a341f44100b 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -1,6 +1,8 @@ //! General-purpose utility functions for internal usage within this crate. +use crate::field_attributes::ReflectIgnoreBehaviour; use bevy_macro_utils::BevyManifest; +use bit_set::BitSet; use proc_macro2::{Ident, Span}; use syn::{Member, Path}; @@ -96,3 +98,42 @@ impl ResultSifter { } } } + +/// Converts an iterator over ignore behaviour of members to a bitset of ignored members. +/// +/// Takes into account the fact that always ignored (non-reflected) members do not count as members. +/// +/// # Example +/// ```rust,ignore +/// pub struct HelloWorld { +/// reflected_field: u32 // index: 0 +/// +/// #[reflect(ignore)] +/// non_reflected_field: u32 // index: N/A (not 1!) +/// +/// #[reflect(skip_serializing)] +/// non_serialized_field: u32 // index: 1 +/// } +/// ``` +/// Would convert to the `0b01` bitset (i.e second field is NOT serialized) +/// +pub(crate) fn members_to_serialization_blacklist(member_iter: T) -> BitSet +where + T: Iterator, +{ + let mut bitset = BitSet::default(); + + member_iter.fold(0, |next_idx, member| { + if member == ReflectIgnoreBehaviour::IgnoreAlways { + bitset.insert(next_idx); + next_idx + } else if member == ReflectIgnoreBehaviour::IgnoreSerialization { + bitset.insert(next_idx); + next_idx + 1 + } else { + next_idx + 1 + } + }); + + bitset +} diff --git a/crates/bevy_reflect/src/enums/dynamic_enum.rs b/crates/bevy_reflect/src/enums/dynamic_enum.rs index d0f097e977b1e..cc07b5c5282d9 100644 --- a/crates/bevy_reflect/src/enums/dynamic_enum.rs +++ b/crates/bevy_reflect/src/enums/dynamic_enum.rs @@ -7,6 +7,7 @@ use std::any::Any; use std::fmt::Formatter; /// A dynamic representation of an enum variant. +#[derive(Debug)] pub enum DynamicVariant { Unit, Tuple(DynamicTuple), @@ -72,7 +73,7 @@ impl From<()> for DynamicVariant { /// // Tada! /// assert_eq!(None, value); /// ``` -#[derive(Default)] +#[derive(Default, Debug)] pub struct DynamicEnum { name: String, variant_name: String, diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index 8dd0ad063f332..42c46f842b9c9 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -161,7 +161,7 @@ impl<'a> Serialize for StructValueSerializer<'a> { for (index, value) in self.struct_value.iter_fields().enumerate() { if data - .map(|data| data.is_ignored_index(index)) + .map(|data| data.is_ignored_field(index)) .unwrap_or(false) { continue; @@ -215,7 +215,7 @@ impl<'a> Serialize for TupleStructValueSerializer<'a> { for (index, value) in self.tuple_struct.iter_fields().enumerate() { if data - .map(|data| data.is_ignored_index(index)) + .map(|data| data.is_ignored_field(index)) .unwrap_or(false) { continue; diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index 13469c0be3fcb..b31b5f78e8da7 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -185,7 +185,7 @@ impl TupleInfo { } /// A tuple which allows fields to be added at runtime. -#[derive(Default)] +#[derive(Default, Debug)] pub struct DynamicTuple { name: String, fields: Vec>, diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 3e99b5e42204d..9e491a814c49d 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -400,21 +400,31 @@ impl Clone for TypeRegistration { } } -/// Contains data relevant to the serialization of a type into the scene. -#[derive(Default, Debug, Clone)] +/// Contains data relevant to the automatic reflect powered serialization of a type +#[derive(Debug, Clone)] pub struct SerializationData { ignored_field_indices: HashSet, } impl SerializationData { - pub fn new>(ignored_field_indices: I) -> Self { + pub fn new>(ignored_iter: I) -> Self { Self { - ignored_field_indices: ignored_field_indices.collect(), + ignored_field_indices: ignored_iter.collect(), } } - /// Returns true if the given index corresponds to a field meant to be ignored in serialization. - pub fn is_ignored_index(&self, index: usize) -> bool { + /// Indices start from 0 and ignored fields do not count as fields. + /// + /// # Example + /// + /// ```rust,ignore + /// for (idx,field) in my_struct.iter_fields().enumerate(){ + /// if serialization_data.is_ignored_field(idx){ + /// // serialize ... + /// } + /// } + /// ``` + pub fn is_ignored_field(&self, index: usize) -> bool { self.ignored_field_indices.contains(&index) } } From 6f12dad820a8a280211c6fc87aa72cba35f4feb2 Mon Sep 17 00:00:00 2001 From: makspll Date: Thu, 15 Sep 2022 13:04:30 +0100 Subject: [PATCH 04/20] fixes for clippy --- .../bevy_reflect/bevy_reflect_derive/src/field_attributes.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs index 51cb4ff814c60..7b40f3f219e3f 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs @@ -34,8 +34,7 @@ impl ReflectIgnoreBehaviour { /// Returns `true` if the ignoring behaviour implies member is included in the reflection API, and false otherwise. pub fn is_active(self) -> bool { match self { - ReflectIgnoreBehaviour::None => true, - ReflectIgnoreBehaviour::IgnoreSerialization => true, + ReflectIgnoreBehaviour::None | ReflectIgnoreBehaviour::IgnoreSerialization => true, ReflectIgnoreBehaviour::IgnoreAlways => false, } } From d7ef6d8cd0393093aaad64781abdaf639fd4c71a Mon Sep 17 00:00:00 2001 From: makspll Date: Thu, 15 Sep 2022 13:11:45 +0100 Subject: [PATCH 05/20] remove needless test --- crates/bevy_reflect/src/enums/mod.rs | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/crates/bevy_reflect/src/enums/mod.rs b/crates/bevy_reflect/src/enums/mod.rs index 6a1f80fd38cf3..e8c32e73ef9c5 100644 --- a/crates/bevy_reflect/src/enums/mod.rs +++ b/crates/bevy_reflect/src/enums/mod.rs @@ -283,30 +283,6 @@ mod tests { value.apply(&dyn_tuple); } - #[test] - #[allow(dead_code)] - fn should_skip_ignored_variants() { - #[derive(Reflect, Debug, PartialEq)] - enum TestEnum { - A, - #[reflect(ignore)] - B, - C, - } - - if let TypeInfo::Enum(info) = TestEnum::type_info() { - assert_eq!( - 2, - info.variant_len(), - "expected one of the variants to be ignored" - ); - assert_eq!("A", info.variant_at(0).unwrap().name()); - assert_eq!("C", info.variant_at(1).unwrap().name()); - } else { - panic!("expected `TypeInfo::Enum`"); - } - } - #[test] fn should_skip_ignored_fields() { #[derive(Reflect, Debug, PartialEq)] From c83f3a113413b9ad973a0bb20e9e46bab6eebfe9 Mon Sep 17 00:00:00 2001 From: Maksymilian Mozolewski Date: Sun, 18 Sep 2022 11:57:30 +0100 Subject: [PATCH 06/20] Update crates/bevy_reflect/src/type_registry.rs Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_reflect/src/type_registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 9e491a814c49d..c37b1d2ba6cc5 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -413,7 +413,7 @@ impl SerializationData { } } /// Returns true if the given index corresponds to a field meant to be ignored in serialization. - /// Indices start from 0 and ignored fields do not count as fields. + /// Indices start from 0 and ignored fields are skipped. /// /// # Example /// From 3ff2831000ce905b5a2392fc6fcf595daf473578 Mon Sep 17 00:00:00 2001 From: Maksymilian Mozolewski Date: Sun, 18 Sep 2022 11:57:40 +0100 Subject: [PATCH 07/20] Update crates/bevy_reflect/src/type_registry.rs Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_reflect/src/type_registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index c37b1d2ba6cc5..019e11c4faf99 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -418,7 +418,7 @@ impl SerializationData { /// # Example /// /// ```rust,ignore - /// for (idx,field) in my_struct.iter_fields().enumerate(){ + /// for (idx, field) in my_struct.iter_fields().enumerate() { /// if serialization_data.is_ignored_field(idx){ /// // serialize ... /// } From a9fd8241f0b671561a086951bb36f52aa94ccc47 Mon Sep 17 00:00:00 2001 From: Maksymilian Mozolewski Date: Sun, 18 Sep 2022 11:58:35 +0100 Subject: [PATCH 08/20] Update crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index be1c7cd15f590..9f0507beb48ea 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -296,7 +296,7 @@ impl<'a> ReflectStruct<'a> { self.types_with(|field| field.attrs.ignore.is_active()) } - /// Get an iterator of fields which are exposed to the serialization or reflection API + /// Get an iterator of fields which are exposed to the reflection API pub fn active_fields(&self) -> impl Iterator> { self.fields_with(|field| field.attrs.ignore.is_active()) } From 7f097d992b0ad9f9dfaef36d47fca4044f052dbc Mon Sep 17 00:00:00 2001 From: makspll Date: Sun, 18 Sep 2022 15:31:40 +0100 Subject: [PATCH 09/20] implement review changes --- .../bevy_reflect_derive/src/derive_data.rs | 73 +++++++++++-------- .../bevy_reflect_derive/src/impls/structs.rs | 13 +++- .../src/impls/tuple_structs.rs | 13 +++- .../bevy_reflect_derive/src/lib.rs | 3 - .../bevy_reflect_derive/src/registration.rs | 10 +-- .../bevy_reflect_derive/src/utility.rs | 12 +-- crates/bevy_reflect/src/serde/mod.rs | 2 + crates/bevy_reflect/src/serde/ser.rs | 14 ++-- crates/bevy_reflect/src/serde/type_data.rs | 54 ++++++++++++++ crates/bevy_reflect/src/type_registry.rs | 68 ----------------- 10 files changed, 142 insertions(+), 120 deletions(-) create mode 100644 crates/bevy_reflect/src/serde/type_data.rs diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index be1c7cd15f590..0b743878a4a97 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -1,7 +1,5 @@ -use std::iter::empty; - use crate::container_attributes::ReflectTraits; -use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr, ReflectIgnoreBehaviour}; +use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr}; use crate::utility::members_to_serialization_blacklist; use bit_set::BitSet; use quote::quote; @@ -41,8 +39,6 @@ pub(crate) struct ReflectMeta<'a> { generics: &'a Generics, /// A cached instance of the path to the `bevy_reflect` crate. bevy_reflect_path: Path, - /// A collection corresponding to `ignored` fields' indices during serialization. - serialization_blacklist: BitSet, } /// Struct data used by derive macros for `Reflect` and `FromReflect`. @@ -60,6 +56,7 @@ pub(crate) struct ReflectMeta<'a> { /// ``` pub(crate) struct ReflectStruct<'a> { meta: ReflectMeta<'a>, + serialization_blacklist: BitSet, fields: Vec>, } @@ -137,20 +134,20 @@ impl<'a> ReflectDerive<'a> { &input.ident, &input.generics, traits, - empty(), ))); } return match &input.data { Data::Struct(data) => { let fields = Self::collect_struct_fields(&data.fields)?; - let meta = ReflectMeta::new( - &input.ident, - &input.generics, - traits, - fields.iter().map(|f| f.attrs.ignore), - ); - let reflect_struct = ReflectStruct { meta, fields }; + let meta = ReflectMeta::new(&input.ident, &input.generics, traits); + let reflect_struct = ReflectStruct { + meta, + serialization_blacklist: members_to_serialization_blacklist( + fields.iter().map(|v| v.attrs.ignore), + ), + fields, + }; match data.fields { Fields::Named(..) => Ok(Self::Struct(reflect_struct)), @@ -160,7 +157,7 @@ impl<'a> ReflectDerive<'a> { } Data::Enum(data) => { let variants = Self::collect_enum_variants(&data.variants)?; - let meta = ReflectMeta::new(&input.ident, &input.generics, traits, empty()); + let meta = ReflectMeta::new(&input.ident, &input.generics, traits); let reflect_enum = ReflectEnum { meta, variants }; Ok(Self::Enum(reflect_enum)) @@ -223,18 +220,12 @@ impl<'a> ReflectDerive<'a> { } impl<'a> ReflectMeta<'a> { - pub fn new>( - type_name: &'a Ident, - generics: &'a Generics, - traits: ReflectTraits, - member_meta_iter: I, - ) -> Self { + pub fn new(type_name: &'a Ident, generics: &'a Generics, traits: ReflectTraits) -> Self { Self { traits, type_name, generics, bevy_reflect_path: utility::get_bevy_reflect_path(), - serialization_blacklist: members_to_serialization_blacklist(member_meta_iter), } } @@ -265,7 +256,7 @@ impl<'a> ReflectMeta<'a> { &self.bevy_reflect_path, self.traits.idents(), self.generics, - &self.serialization_blacklist, + false, ) } } @@ -276,8 +267,30 @@ impl<'a> ReflectStruct<'a> { &self.meta } + /// Access the data about which fields should be ignored during serialization. + /// + /// The returned bitset is a collection of indices obtained from the [members_to_serialization_blacklist](crate::utility::members_to_serialization_blacklist) function. + pub fn serialization_blacklist(&self) -> &BitSet { + &self.serialization_blacklist + } + + /// Returns the `GetTypeRegistration` impl as a `TokenStream`. + /// + /// Returns a specific implementation for structs and this method should be preffered over the generic [get_type_registration](crate::ReflectMeta) method + pub fn get_type_registration(&self) -> proc_macro2::TokenStream { + let reflect_path = self.meta.bevy_reflect_path(); + + crate::registration::impl_get_type_registration( + self.meta.type_name(), + reflect_path, + self.meta.traits().idents(), + self.meta.generics(), + true, + ) + } + /// Get an iterator over the fields satisfying the given predicate - fn fields_with bool>( + fn fields_by bool>( &self, predicate: F, ) -> impl Iterator> { @@ -285,25 +298,25 @@ impl<'a> ReflectStruct<'a> { } /// Get a collection of all field types satisfying the given predicate - fn types_with bool>(&self, predicate: F) -> Vec { - self.fields_with(predicate) + fn types_by bool>(&self, predicate: F) -> Vec { + self.fields_by(predicate) .map(|field| field.data.ty.clone()) .collect::>() } - /// Get a collection of types which are exposed to either the serialization or reflection API + /// Get a collection of types which are exposed to either the reflection API pub fn active_types(&self) -> Vec { - self.types_with(|field| field.attrs.ignore.is_active()) + self.types_by(|field| field.attrs.ignore.is_active()) } - /// Get an iterator of fields which are exposed to the serialization or reflection API + /// Get an iterator of fields which are exposed to the reflection API pub fn active_fields(&self) -> impl Iterator> { - self.fields_with(|field| field.attrs.ignore.is_active()) + self.fields_by(|field| field.attrs.ignore.is_active()) } /// Get an iterator of fields which are ignored by the reflection and serialization API pub fn ignored_fields(&self) -> impl Iterator> { - self.fields_with(|field| field.attrs.ignore.is_ignored()) + self.fields_by(|field| field.attrs.ignore.is_ignored()) } /// The complete set of fields in this struct. diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs index 4dca3a4feea16..37f3242ed3f81 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs @@ -64,10 +64,15 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream { bevy_reflect_path, ); - let get_type_registration_impl = reflect_struct.meta().get_type_registration(); + let get_type_registration_impl = reflect_struct.get_type_registration(); let (impl_generics, ty_generics, where_clause) = reflect_struct.meta().generics().split_for_impl(); + let serialization_blacklist = reflect_struct + .serialization_blacklist() + .iter() + .map(|v| v as usize); + TokenStream::from(quote! { #get_type_registration_impl @@ -197,5 +202,11 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream { #debug_fn } + + impl #impl_generics #bevy_reflect_path::serde::ReflectSerializableWithData for #struct_name #ty_generics #where_clause{ + fn get_serialization_data() -> #bevy_reflect_path::serde::SerializationData { + #bevy_reflect_path::serde::SerializationData::new([#(#serialization_blacklist),*].into_iter()) + } + } }) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs index 0ad33ba4bb8ce..72df8f6631e66 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs @@ -8,7 +8,7 @@ use syn::{Index, Member}; pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); let struct_name = reflect_struct.meta().type_name(); - let get_type_registration_impl = reflect_struct.meta().get_type_registration(); + let get_type_registration_impl = reflect_struct.get_type_registration(); let field_idents = reflect_struct .active_fields() @@ -51,6 +51,11 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { let (impl_generics, ty_generics, where_clause) = reflect_struct.meta().generics().split_for_impl(); + let serialization_blacklist = reflect_struct + .serialization_blacklist() + .iter() + .map(|v| v as usize); + TokenStream::from(quote! { #get_type_registration_impl @@ -158,5 +163,11 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { #debug_fn } + + impl #impl_generics #bevy_reflect_path::serde::ReflectSerializableWithData for #struct_name #ty_generics #where_clause{ + fn get_serialization_data() -> #bevy_reflect_path::serde::SerializationData { + #bevy_reflect_path::serde::SerializationData::new([#(#serialization_blacklist),*].into_iter()) + } + } }) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs index affdd100c8449..d7a7515683a11 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs @@ -30,7 +30,6 @@ use crate::derive_data::{ReflectDerive, ReflectMeta, ReflectStruct}; use proc_macro::TokenStream; use quote::quote; use reflect_value::ReflectValueDef; -use std::iter::empty; use syn::spanned::Spanned; use syn::{parse_macro_input, DeriveInput}; @@ -100,7 +99,6 @@ pub fn impl_reflect_value(input: TokenStream) -> TokenStream { &def.type_name, &def.generics, def.traits.unwrap_or_default(), - empty(), )) } @@ -177,6 +175,5 @@ pub fn impl_from_reflect_value(input: TokenStream) -> TokenStream { &def.type_name, &def.generics, def.traits.unwrap_or_default(), - empty(), )) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index 4701e2e3923ee..ed2d491724999 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -1,6 +1,5 @@ //! Contains code related specifically to Bevy's type registration. -use bit_set::BitSet; use proc_macro2::Ident; use quote::quote; use syn::{Generics, Path}; @@ -11,19 +10,20 @@ pub(crate) fn impl_get_type_registration( bevy_reflect_path: &Path, registration_data: &[Ident], generics: &Generics, - serialization_blacklist: &BitSet, + insert_serialization_data: bool, ) -> proc_macro2::TokenStream { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let serialization_blacklist_indices = serialization_blacklist.iter().map(|v| v as usize); + let serialization_data = insert_serialization_data.then_some(quote! { + registration.insert::<#bevy_reflect_path::serde::SerializationData>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type()); + }); quote! { #[allow(unused_mut)] impl #impl_generics #bevy_reflect_path::GetTypeRegistration for #type_name #ty_generics #where_clause { fn get_type_registration() -> #bevy_reflect_path::TypeRegistration { let mut registration = #bevy_reflect_path::TypeRegistration::of::<#type_name #ty_generics>(); registration.insert::<#bevy_reflect_path::ReflectFromPtr>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type()); + #serialization_data #(registration.insert::<#registration_data>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type());)* - let serialization_data = #bevy_reflect_path::SerializationData::new([#(#serialization_blacklist_indices),*].into_iter()); - registration.set_serialization_data(serialization_data); registration } } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index e1a341f44100b..4511c418cae49 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -101,7 +101,7 @@ impl ResultSifter { /// Converts an iterator over ignore behaviour of members to a bitset of ignored members. /// -/// Takes into account the fact that always ignored (non-reflected) members do not count as members. +/// Takes into account the fact that always ignored (non-reflected) members are skipped. /// /// # Example /// ```rust,ignore @@ -123,16 +123,16 @@ where { let mut bitset = BitSet::default(); - member_iter.fold(0, |next_idx, member| { - if member == ReflectIgnoreBehaviour::IgnoreAlways { + member_iter.fold(0, |next_idx, member| match member { + ReflectIgnoreBehaviour::IgnoreAlways => { bitset.insert(next_idx); next_idx - } else if member == ReflectIgnoreBehaviour::IgnoreSerialization { + } + ReflectIgnoreBehaviour::IgnoreSerialization => { bitset.insert(next_idx); next_idx + 1 - } else { - next_idx + 1 } + ReflectIgnoreBehaviour::None => next_idx + 1, }); bitset diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index 855f30ebecea2..58e57c7beac75 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -1,8 +1,10 @@ mod de; mod ser; +mod type_data; pub use de::*; pub use ser::*; +pub use type_data::*; pub(crate) mod type_fields { pub const TYPE: &str = "type"; diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index 42c46f842b9c9..b086a6a942085 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -8,6 +8,8 @@ use serde::{ Serialize, Serializer, }; +use super::SerializationData; + pub enum Serializable<'a> { Owned(Box), Borrowed(&'a dyn erased_serde::Serialize), @@ -154,13 +156,13 @@ impl<'a> Serialize for StructValueSerializer<'a> { S: serde::Serializer, { let mut state = serializer.serialize_map(Some(self.struct_value.field_len()))?; - let data = self + let serialization_data = self .registry .get_with_name(self.struct_value.type_name()) - .and_then(|registration| registration.serialization_data()); + .and_then(|registration| registration.data::()); for (index, value) in self.struct_value.iter_fields().enumerate() { - if data + if serialization_data .map(|data| data.is_ignored_field(index)) .unwrap_or(false) { @@ -208,13 +210,13 @@ impl<'a> Serialize for TupleStructValueSerializer<'a> { S: serde::Serializer, { let mut state = serializer.serialize_seq(Some(self.tuple_struct.field_len()))?; - let data = self + let serialization_data = self .registry .get_with_name(self.tuple_struct.type_name()) - .and_then(|registration| registration.serialization_data()); + .and_then(|registration| registration.data::()); for (index, value) in self.tuple_struct.iter_fields().enumerate() { - if data + if serialization_data .map(|data| data.is_ignored_field(index)) .unwrap_or(false) { diff --git a/crates/bevy_reflect/src/serde/type_data.rs b/crates/bevy_reflect/src/serde/type_data.rs new file mode 100644 index 0000000000000..cc739b0c1df6b --- /dev/null +++ b/crates/bevy_reflect/src/serde/type_data.rs @@ -0,0 +1,54 @@ +use std::collections::HashSet; + +use crate::{FromType, Reflect}; + +/// Trait implemented by types which require additional data to be serialized as expected via the reflection API. +/// +/// For example, structs may contain fields which are reflected but not to be serialized: +/// ```rust, ignore +/// pub struct MyStruct { +/// serialized_and_reflected : u32, +/// #[reflect(skip_serializing)] +/// reflected_only: u32, +/// } +/// ``` +pub trait ReflectSerializableWithData { + fn get_serialization_data() -> SerializationData; +} + +/// Contains data relevant to the automatic reflect powered serialization of a type +#[derive(Debug, Clone)] +pub struct SerializationData { + ignored_field_indices: HashSet, +} + +impl SerializationData { + /// Creates a new SerializationData instance given: + /// - ignored_iter: the iterator of member indices to be ignored during serialization. Indices are assigned only to reflected members, those which are not reflected are skipped. + pub fn new>(ignored_iter: I) -> Self { + Self { + ignored_field_indices: ignored_iter.collect(), + } + } + /// Returns true if the given index corresponds to a field meant to be ignored in serialization. + /// Indices start from 0 and ignored fields do not count as fields. + /// + /// # Example + /// + /// ```rust,ignore + /// for (idx,field) in my_struct.iter_fields().enumerate(){ + /// if serialization_data.is_ignored_field(idx){ + /// // serialize ... + /// } + /// } + /// ``` + pub fn is_ignored_field(&self, index: usize) -> bool { + self.ignored_field_indices.contains(&index) + } +} + +impl FromType for SerializationData { + fn from_type() -> Self { + T::get_serialization_data() + } +} diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 9e491a814c49d..fe3302774b761 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -220,25 +220,6 @@ impl TypeRegistry { .and_then(|registration| registration.data_mut::()) } - /// Returns a reference to the [`SerializationData`] for the type associated with the given `TypeId`. - /// - /// If the specified type has not been registered, or if it has no associated [`SerializationData`], returns `None`. - pub fn get_serialization_data(&self, type_id: TypeId) -> Option<&SerializationData> { - self.get(type_id) - .and_then(|registration| registration.serialization_data()) - } - - /// Returns a mutable reference to the [`SerializationData`] for the type associated with the given `TypeId`. - /// - /// If the specified type has not been registered, or if it has no associated [`SerializationData`], returns `None`. - pub fn get_serialization_data_mut( - &mut self, - type_id: TypeId, - ) -> Option<&mut SerializationData> { - self.get_mut(type_id) - .and_then(|registration| registration.serialization_data_mut()) - } - /// Returns the [`TypeInfo`] associated with the given `TypeId`. /// /// If the specified type has not been registered, returns `None`. @@ -287,7 +268,6 @@ impl TypeRegistryArc { /// [1]: crate::Reflect pub struct TypeRegistration { short_name: String, - serialization_data: Option, data: HashMap>, type_info: &'static TypeInfo, } @@ -296,7 +276,6 @@ impl Debug for TypeRegistration { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("TypeRegistration") .field("short_name", &self.short_name) - .field("serialization_data", &self.serialization_data) .field("type_info", &self.type_info) .finish() } @@ -331,14 +310,6 @@ impl TypeRegistration { .and_then(|value| value.downcast_mut()) } - pub fn serialization_data(&self) -> Option<&SerializationData> { - self.serialization_data.as_ref() - } - - pub fn serialization_data_mut(&mut self) -> Option<&mut SerializationData> { - self.serialization_data.as_mut() - } - /// Returns a reference to the registration's [`TypeInfo`] pub fn type_info(&self) -> &'static TypeInfo { self.type_info @@ -351,19 +322,11 @@ impl TypeRegistration { self.data.insert(TypeId::of::(), Box::new(data)); } - /// Set the scene serialization data of this registration. - /// - /// If another instance was previously set, it is replaced. - pub fn set_serialization_data(&mut self, data: SerializationData) { - self.serialization_data = Some(data); - } - /// Creates type registration information for `T`. pub fn of() -> Self { let type_name = std::any::type_name::(); Self { data: HashMap::default(), - serialization_data: None, short_name: bevy_utils::get_short_name(type_name), type_info: T::type_info(), } @@ -393,42 +356,11 @@ impl Clone for TypeRegistration { TypeRegistration { data, - serialization_data: self.serialization_data.clone(), short_name: self.short_name.clone(), type_info: self.type_info, } } } - -/// Contains data relevant to the automatic reflect powered serialization of a type -#[derive(Debug, Clone)] -pub struct SerializationData { - ignored_field_indices: HashSet, -} - -impl SerializationData { - pub fn new>(ignored_iter: I) -> Self { - Self { - ignored_field_indices: ignored_iter.collect(), - } - } - /// Returns true if the given index corresponds to a field meant to be ignored in serialization. - /// Indices start from 0 and ignored fields do not count as fields. - /// - /// # Example - /// - /// ```rust,ignore - /// for (idx,field) in my_struct.iter_fields().enumerate(){ - /// if serialization_data.is_ignored_field(idx){ - /// // serialize ... - /// } - /// } - /// ``` - pub fn is_ignored_field(&self, index: usize) -> bool { - self.ignored_field_indices.contains(&index) - } -} - /// A trait for types generated by the [`#[reflect_trait]`][0] attribute macro. /// /// [0]: crate::reflect_trait From 04c0a770213cc1db328cb835dfc2a80606400676 Mon Sep 17 00:00:00 2001 From: makspll Date: Sun, 18 Sep 2022 16:00:20 +0100 Subject: [PATCH 10/20] more review changes --- .../bevy_reflect_derive/src/derive_data.rs | 4 ++-- .../src/field_attributes.rs | 18 +++++++++--------- .../bevy_reflect_derive/src/utility.rs | 10 +++++----- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 0b743878a4a97..d8119daa5af6e 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -269,14 +269,14 @@ impl<'a> ReflectStruct<'a> { /// Access the data about which fields should be ignored during serialization. /// - /// The returned bitset is a collection of indices obtained from the [members_to_serialization_blacklist](crate::utility::members_to_serialization_blacklist) function. + /// The returned bitset is a collection of indices obtained from the [`members_to_serialization_blacklist`](crate::utility::members_to_serialization_blacklist) function. pub fn serialization_blacklist(&self) -> &BitSet { &self.serialization_blacklist } /// Returns the `GetTypeRegistration` impl as a `TokenStream`. /// - /// Returns a specific implementation for structs and this method should be preffered over the generic [get_type_registration](crate::ReflectMeta) method + /// Returns a specific implementation for structs and this method should be preffered over the generic [`get_type_registration`](crate::ReflectMeta) method pub fn get_type_registration(&self) -> proc_macro2::TokenStream { let reflect_path = self.meta.bevy_reflect_path(); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs index 7b40f3f219e3f..0e19b8429e581 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/field_attributes.rs @@ -20,7 +20,7 @@ pub(crate) static DEFAULT_ATTR: &str = "default"; /// In boolean logic this is described as: `is_serialized -> is_reflected`, this means we can reflect something without serializing it but not the other way round. /// The `is_reflected` predicate is provided as `self.is_active()` #[derive(Default, Clone, Copy, PartialEq, Eq)] -pub(crate) enum ReflectIgnoreBehaviour { +pub(crate) enum ReflectIgnoreBehavior { /// Don't ignore, appear to all systems #[default] None, @@ -30,12 +30,12 @@ pub(crate) enum ReflectIgnoreBehaviour { IgnoreAlways, } -impl ReflectIgnoreBehaviour { +impl ReflectIgnoreBehavior { /// Returns `true` if the ignoring behaviour implies member is included in the reflection API, and false otherwise. pub fn is_active(self) -> bool { match self { - ReflectIgnoreBehaviour::None | ReflectIgnoreBehaviour::IgnoreSerialization => true, - ReflectIgnoreBehaviour::IgnoreAlways => false, + ReflectIgnoreBehavior::None | ReflectIgnoreBehavior::IgnoreSerialization => true, + ReflectIgnoreBehavior::IgnoreAlways => false, } } @@ -49,7 +49,7 @@ impl ReflectIgnoreBehaviour { #[derive(Default)] pub(crate) struct ReflectFieldAttr { /// Determines how this field should be ignored if at all. - pub ignore: ReflectIgnoreBehaviour, + pub ignore: ReflectIgnoreBehavior, /// Sets the default behavior of this field. pub default: DefaultBehavior, } @@ -99,13 +99,13 @@ pub(crate) fn parse_field_attrs(attrs: &[Attribute]) -> Result Result<(), syn::Error> { match meta { Meta::Path(path) if path.is_ident(IGNORE_SERIALIZATION_ATTR) => { - (args.ignore == ReflectIgnoreBehaviour::None) - .then(|| args.ignore = ReflectIgnoreBehaviour::IgnoreSerialization) + (args.ignore == ReflectIgnoreBehavior::None) + .then(|| args.ignore = ReflectIgnoreBehavior::IgnoreSerialization) .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) } Meta::Path(path) if path.is_ident(IGNORE_ALL_ATTR) => { - (args.ignore == ReflectIgnoreBehaviour::None) - .then(|| args.ignore = ReflectIgnoreBehaviour::IgnoreAlways) + (args.ignore == ReflectIgnoreBehavior::None) + .then(|| args.ignore = ReflectIgnoreBehavior::IgnoreAlways) .ok_or_else(|| syn::Error::new_spanned(path, format!("Only one of ['{IGNORE_SERIALIZATION_ATTR}','{IGNORE_ALL_ATTR}'] is allowed"))) } Meta::Path(path) if path.is_ident(DEFAULT_ATTR) => { diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index 4511c418cae49..407ef8ba59b32 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -1,6 +1,6 @@ //! General-purpose utility functions for internal usage within this crate. -use crate::field_attributes::ReflectIgnoreBehaviour; +use crate::field_attributes::ReflectIgnoreBehavior; use bevy_macro_utils::BevyManifest; use bit_set::BitSet; use proc_macro2::{Ident, Span}; @@ -119,20 +119,20 @@ impl ResultSifter { /// pub(crate) fn members_to_serialization_blacklist(member_iter: T) -> BitSet where - T: Iterator, + T: Iterator, { let mut bitset = BitSet::default(); member_iter.fold(0, |next_idx, member| match member { - ReflectIgnoreBehaviour::IgnoreAlways => { + ReflectIgnoreBehavior::IgnoreAlways => { bitset.insert(next_idx); next_idx } - ReflectIgnoreBehaviour::IgnoreSerialization => { + ReflectIgnoreBehavior::IgnoreSerialization => { bitset.insert(next_idx); next_idx + 1 } - ReflectIgnoreBehaviour::None => next_idx + 1, + ReflectIgnoreBehavior::None => next_idx + 1, }); bitset From 4ffdaf1ebde36fe7d3ac0a6c9993a9fe6c44bebd Mon Sep 17 00:00:00 2001 From: makspll Date: Sun, 18 Sep 2022 16:09:35 +0100 Subject: [PATCH 11/20] remove double reference from public API --- .../bevy_reflect/bevy_reflect_derive/src/derive_data.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index d8119daa5af6e..839abd2f5edf0 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -290,15 +290,15 @@ impl<'a> ReflectStruct<'a> { } /// Get an iterator over the fields satisfying the given predicate - fn fields_by bool>( + fn fields_by bool>( &self, - predicate: F, + mut predicate: F, ) -> impl Iterator> { - self.fields.iter().filter(predicate) + self.fields.iter().filter(move |e| predicate(e)) } /// Get a collection of all field types satisfying the given predicate - fn types_by bool>(&self, predicate: F) -> Vec { + fn types_by bool>(&self, predicate: F) -> Vec { self.fields_by(predicate) .map(|field| field.data.ty.clone()) .collect::>() From 9d49b37314f0dd8dc308adcf8e7b3e307b69f6bc Mon Sep 17 00:00:00 2001 From: makspll Date: Sun, 18 Sep 2022 16:11:53 +0100 Subject: [PATCH 12/20] change doc comment --- crates/bevy_reflect/src/serde/type_data.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/serde/type_data.rs b/crates/bevy_reflect/src/serde/type_data.rs index cc739b0c1df6b..f188526e07e4d 100644 --- a/crates/bevy_reflect/src/serde/type_data.rs +++ b/crates/bevy_reflect/src/serde/type_data.rs @@ -31,7 +31,7 @@ impl SerializationData { } } /// Returns true if the given index corresponds to a field meant to be ignored in serialization. - /// Indices start from 0 and ignored fields do not count as fields. + /// Indices start from 0 and ignored fields are skipped. /// /// # Example /// From dd8a10fd28d35706530600df54452295a9c68c6f Mon Sep 17 00:00:00 2001 From: makspll Date: Sun, 18 Sep 2022 16:12:44 +0100 Subject: [PATCH 13/20] change spacing --- crates/bevy_reflect/src/serde/type_data.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/serde/type_data.rs b/crates/bevy_reflect/src/serde/type_data.rs index f188526e07e4d..4a49fc3b55c44 100644 --- a/crates/bevy_reflect/src/serde/type_data.rs +++ b/crates/bevy_reflect/src/serde/type_data.rs @@ -36,7 +36,7 @@ impl SerializationData { /// # Example /// /// ```rust,ignore - /// for (idx,field) in my_struct.iter_fields().enumerate(){ + /// for (idx, field) in my_struct.iter_fields().enumerate(){ /// if serialization_data.is_ignored_field(idx){ /// // serialize ... /// } From e3d16391680f4b4c0df5d3c57ec7d9ae0169cdd8 Mon Sep 17 00:00:00 2001 From: makspll Date: Sun, 18 Sep 2022 16:15:59 +0100 Subject: [PATCH 14/20] clippy fixes --- crates/bevy_reflect/src/serde/type_data.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/serde/type_data.rs b/crates/bevy_reflect/src/serde/type_data.rs index 4a49fc3b55c44..f6c58b9dd88a7 100644 --- a/crates/bevy_reflect/src/serde/type_data.rs +++ b/crates/bevy_reflect/src/serde/type_data.rs @@ -23,8 +23,8 @@ pub struct SerializationData { } impl SerializationData { - /// Creates a new SerializationData instance given: - /// - ignored_iter: the iterator of member indices to be ignored during serialization. Indices are assigned only to reflected members, those which are not reflected are skipped. + /// Creates a new `SerializationData` instance given: + /// - `ignored_iter`: the iterator of member indices to be ignored during serialization. Indices are assigned only to reflected members, those which are not reflected are skipped. pub fn new>(ignored_iter: I) -> Self { Self { ignored_field_indices: ignored_iter.collect(), From 0d61a9747545df160302bc9f38c053ee1bc97c4a Mon Sep 17 00:00:00 2001 From: Maksymilian Mozolewski Date: Sun, 18 Sep 2022 17:21:19 +0100 Subject: [PATCH 15/20] Update crates/bevy_reflect/src/serde/type_data.rs Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_reflect/src/serde/type_data.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_reflect/src/serde/type_data.rs b/crates/bevy_reflect/src/serde/type_data.rs index f6c58b9dd88a7..48bed9a416b89 100644 --- a/crates/bevy_reflect/src/serde/type_data.rs +++ b/crates/bevy_reflect/src/serde/type_data.rs @@ -24,6 +24,7 @@ pub struct SerializationData { impl SerializationData { /// Creates a new `SerializationData` instance given: + /// /// - `ignored_iter`: the iterator of member indices to be ignored during serialization. Indices are assigned only to reflected members, those which are not reflected are skipped. pub fn new>(ignored_iter: I) -> Self { Self { From ad1521c1fe25d2f7bf758c6d41dff6a29962a6d1 Mon Sep 17 00:00:00 2001 From: Maksymilian Mozolewski Date: Sun, 18 Sep 2022 17:21:25 +0100 Subject: [PATCH 16/20] Update crates/bevy_reflect/src/serde/type_data.rs Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_reflect/src/serde/type_data.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_reflect/src/serde/type_data.rs b/crates/bevy_reflect/src/serde/type_data.rs index 48bed9a416b89..afe2dd77bea3a 100644 --- a/crates/bevy_reflect/src/serde/type_data.rs +++ b/crates/bevy_reflect/src/serde/type_data.rs @@ -32,6 +32,7 @@ impl SerializationData { } } /// Returns true if the given index corresponds to a field meant to be ignored in serialization. + /// /// Indices start from 0 and ignored fields are skipped. /// /// # Example From 980daa3f982de7d72a2f45fe0936f34ea5f22891 Mon Sep 17 00:00:00 2001 From: makspll Date: Sun, 18 Sep 2022 17:34:06 +0100 Subject: [PATCH 17/20] implement further review changes --- .../bevy_reflect_derive/src/derive_data.rs | 5 +++-- .../bevy_reflect_derive/src/impls/structs.rs | 11 ---------- .../src/impls/tuple_structs.rs | 11 ---------- .../bevy_reflect_derive/src/registration.rs | 12 +++++++--- crates/bevy_reflect/src/serde/mod.rs | 8 +++---- crates/bevy_reflect/src/serde/type_data.rs | 22 ------------------- 6 files changed, 15 insertions(+), 54 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 839abd2f5edf0..73e678761defa 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -256,7 +256,7 @@ impl<'a> ReflectMeta<'a> { &self.bevy_reflect_path, self.traits.idents(), self.generics, - false, + None, ) } } @@ -270,6 +270,7 @@ impl<'a> ReflectStruct<'a> { /// Access the data about which fields should be ignored during serialization. /// /// The returned bitset is a collection of indices obtained from the [`members_to_serialization_blacklist`](crate::utility::members_to_serialization_blacklist) function. + #[allow(dead_code)] pub fn serialization_blacklist(&self) -> &BitSet { &self.serialization_blacklist } @@ -285,7 +286,7 @@ impl<'a> ReflectStruct<'a> { reflect_path, self.meta.traits().idents(), self.meta.generics(), - true, + Some(&self.serialization_blacklist), ) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs index 37f3242ed3f81..f47dddf6536b0 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs @@ -68,11 +68,6 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream { let (impl_generics, ty_generics, where_clause) = reflect_struct.meta().generics().split_for_impl(); - let serialization_blacklist = reflect_struct - .serialization_blacklist() - .iter() - .map(|v| v as usize); - TokenStream::from(quote! { #get_type_registration_impl @@ -202,11 +197,5 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream { #debug_fn } - - impl #impl_generics #bevy_reflect_path::serde::ReflectSerializableWithData for #struct_name #ty_generics #where_clause{ - fn get_serialization_data() -> #bevy_reflect_path::serde::SerializationData { - #bevy_reflect_path::serde::SerializationData::new([#(#serialization_blacklist),*].into_iter()) - } - } }) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs index 72df8f6631e66..ecd829f94f034 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs @@ -51,11 +51,6 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { let (impl_generics, ty_generics, where_clause) = reflect_struct.meta().generics().split_for_impl(); - let serialization_blacklist = reflect_struct - .serialization_blacklist() - .iter() - .map(|v| v as usize); - TokenStream::from(quote! { #get_type_registration_impl @@ -163,11 +158,5 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { #debug_fn } - - impl #impl_generics #bevy_reflect_path::serde::ReflectSerializableWithData for #struct_name #ty_generics #where_clause{ - fn get_serialization_data() -> #bevy_reflect_path::serde::SerializationData { - #bevy_reflect_path::serde::SerializationData::new([#(#serialization_blacklist),*].into_iter()) - } - } }) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index ed2d491724999..6beeb9ed6ee1c 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -1,5 +1,6 @@ //! Contains code related specifically to Bevy's type registration. +use bit_set::BitSet; use proc_macro2::Ident; use quote::quote; use syn::{Generics, Path}; @@ -10,12 +11,17 @@ pub(crate) fn impl_get_type_registration( bevy_reflect_path: &Path, registration_data: &[Ident], generics: &Generics, - insert_serialization_data: bool, + serialization_blacklist: Option<&BitSet>, ) -> proc_macro2::TokenStream { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let serialization_data = insert_serialization_data.then_some(quote! { - registration.insert::<#bevy_reflect_path::serde::SerializationData>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type()); + let serialization_data = serialization_blacklist.map(|blacklist| { + let blacklist = blacklist.into_iter().map(|v| v as usize); + quote! { + let ignored_indices = [#(#blacklist),*].into_iter(); + registration.insert::<#bevy_reflect_path::serde::SerializationData>(#bevy_reflect_path::serde::SerializationData::new(ignored_indices)); + } }); + quote! { #[allow(unused_mut)] impl #impl_generics #bevy_reflect_path::GetTypeRegistration for #type_name #ty_generics #where_clause { diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index 58e57c7beac75..1ff61a4e70b4e 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -24,7 +24,7 @@ mod tests { use crate::{self as bevy_reflect, DynamicTupleStruct}; use crate::{ serde::{ReflectDeserializer, ReflectSerializer}, - type_registry::{GetTypeRegistration, TypeRegistry}, + type_registry::TypeRegistry, DynamicStruct, Reflect, }; use serde::de::DeserializeSeed; @@ -43,8 +43,7 @@ mod tests { } let mut registry = TypeRegistry::default(); - let registration = TestStruct::get_type_registration(); - registry.add_registration(registration); + registry.register::(); let test_struct = TestStruct { a: 3, @@ -84,8 +83,7 @@ mod tests { ); let mut registry = TypeRegistry::default(); - let registration = TestStruct::get_type_registration(); - registry.add_registration(registration); + registry.register::(); let test_struct = TestStruct(3, 4, 5, 6); diff --git a/crates/bevy_reflect/src/serde/type_data.rs b/crates/bevy_reflect/src/serde/type_data.rs index f6c58b9dd88a7..f77ebcccc1e5d 100644 --- a/crates/bevy_reflect/src/serde/type_data.rs +++ b/crates/bevy_reflect/src/serde/type_data.rs @@ -1,21 +1,5 @@ use std::collections::HashSet; -use crate::{FromType, Reflect}; - -/// Trait implemented by types which require additional data to be serialized as expected via the reflection API. -/// -/// For example, structs may contain fields which are reflected but not to be serialized: -/// ```rust, ignore -/// pub struct MyStruct { -/// serialized_and_reflected : u32, -/// #[reflect(skip_serializing)] -/// reflected_only: u32, -/// } -/// ``` -pub trait ReflectSerializableWithData { - fn get_serialization_data() -> SerializationData; -} - /// Contains data relevant to the automatic reflect powered serialization of a type #[derive(Debug, Clone)] pub struct SerializationData { @@ -46,9 +30,3 @@ impl SerializationData { self.ignored_field_indices.contains(&index) } } - -impl FromType for SerializationData { - fn from_type() -> Self { - T::get_serialization_data() - } -} From 610774988cc84da339b82a9835efdac4c41c6a62 Mon Sep 17 00:00:00 2001 From: makspll Date: Mon, 19 Sep 2022 11:19:36 +0100 Subject: [PATCH 18/20] further review implementation --- .../bevy_reflect_derive/src/derive_data.rs | 48 +++++++------------ .../bevy_reflect_derive/src/registration.rs | 8 ++-- .../bevy_reflect_derive/src/utility.rs | 2 +- 3 files changed, 23 insertions(+), 35 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 73e678761defa..7e531e0fe87c0 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -1,6 +1,6 @@ use crate::container_attributes::ReflectTraits; use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr}; -use crate::utility::members_to_serialization_blacklist; +use crate::utility::members_to_serialization_denylist; use bit_set::BitSet; use quote::quote; @@ -56,7 +56,7 @@ pub(crate) struct ReflectMeta<'a> { /// ``` pub(crate) struct ReflectStruct<'a> { meta: ReflectMeta<'a>, - serialization_blacklist: BitSet, + serialization_denylist: BitSet, fields: Vec>, } @@ -95,10 +95,8 @@ pub(crate) struct EnumVariant<'a> { /// The fields within this variant. pub fields: EnumVariantFields<'a>, /// The reflection-based attributes on the variant. - #[allow(dead_code)] pub attrs: ReflectFieldAttr, /// The index of this variant within the enum. - #[allow(dead_code)] pub index: usize, } @@ -143,7 +141,7 @@ impl<'a> ReflectDerive<'a> { let meta = ReflectMeta::new(&input.ident, &input.generics, traits); let reflect_struct = ReflectStruct { meta, - serialization_blacklist: members_to_serialization_blacklist( + serialization_denylist: members_to_serialization_denylist( fields.iter().map(|v| v.attrs.ignore), ), fields, @@ -269,10 +267,9 @@ impl<'a> ReflectStruct<'a> { /// Access the data about which fields should be ignored during serialization. /// - /// The returned bitset is a collection of indices obtained from the [`members_to_serialization_blacklist`](crate::utility::members_to_serialization_blacklist) function. - #[allow(dead_code)] - pub fn serialization_blacklist(&self) -> &BitSet { - &self.serialization_blacklist + /// The returned bitset is a collection of indices obtained from the [`members_to_serialization_denylist`](crate::utility::members_to_serialization_denylist) function. + pub fn serialization_denylist(&self) -> &BitSet { + &self.serialization_denylist } /// Returns the `GetTypeRegistration` impl as a `TokenStream`. @@ -286,42 +283,34 @@ impl<'a> ReflectStruct<'a> { reflect_path, self.meta.traits().idents(), self.meta.generics(), - Some(&self.serialization_blacklist), + Some(&self.serialization_denylist), ) } - /// Get an iterator over the fields satisfying the given predicate - fn fields_by bool>( - &self, - mut predicate: F, - ) -> impl Iterator> { - self.fields.iter().filter(move |e| predicate(e)) - } - - /// Get a collection of all field types satisfying the given predicate - fn types_by bool>(&self, predicate: F) -> Vec { - self.fields_by(predicate) - .map(|field| field.data.ty.clone()) - .collect::>() - } - /// Get a collection of types which are exposed to either the reflection API pub fn active_types(&self) -> Vec { - self.types_by(|field| field.attrs.ignore.is_active()) + self.fields + .iter() + .filter(move |field| field.attrs.ignore.is_active()) + .map(|field| field.data.ty.clone()) + .collect::>() } /// Get an iterator of fields which are exposed to the reflection API pub fn active_fields(&self) -> impl Iterator> { - self.fields_by(|field| field.attrs.ignore.is_active()) + self.fields + .iter() + .filter(move |field| field.attrs.ignore.is_active()) } /// Get an iterator of fields which are ignored by the reflection and serialization API pub fn ignored_fields(&self) -> impl Iterator> { - self.fields_by(|field| field.attrs.ignore.is_ignored()) + self.fields + .iter() + .filter(move |field| field.attrs.ignore.is_ignored()) } /// The complete set of fields in this struct. - #[allow(dead_code)] pub fn fields(&self) -> &[StructField<'a>] { &self.fields } @@ -342,7 +331,6 @@ impl<'a> ReflectEnum<'a> { } /// The complete set of variants in this enum. - #[allow(dead_code)] pub fn variants(&self) -> &[EnumVariant<'a>] { &self.variants } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index 6beeb9ed6ee1c..0f234105452fe 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -11,13 +11,13 @@ pub(crate) fn impl_get_type_registration( bevy_reflect_path: &Path, registration_data: &[Ident], generics: &Generics, - serialization_blacklist: Option<&BitSet>, + serialization_denylist: Option<&BitSet>, ) -> proc_macro2::TokenStream { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let serialization_data = serialization_blacklist.map(|blacklist| { - let blacklist = blacklist.into_iter().map(|v| v as usize); + let serialization_data = serialization_denylist.map(|denylist| { + let denylist = denylist.into_iter().map(|v| v as usize); quote! { - let ignored_indices = [#(#blacklist),*].into_iter(); + let ignored_indices = [#(#denylist),*].into_iter(); registration.insert::<#bevy_reflect_path::serde::SerializationData>(#bevy_reflect_path::serde::SerializationData::new(ignored_indices)); } }); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index 407ef8ba59b32..4f3668bd0cdb2 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -117,7 +117,7 @@ impl ResultSifter { /// ``` /// Would convert to the `0b01` bitset (i.e second field is NOT serialized) /// -pub(crate) fn members_to_serialization_blacklist(member_iter: T) -> BitSet +pub(crate) fn members_to_serialization_denylist(member_iter: T) -> BitSet where T: Iterator, { From 936fa0bd987b94b76d516c23c7a68f72cdd7303e Mon Sep 17 00:00:00 2001 From: makspll Date: Mon, 19 Sep 2022 11:24:22 +0100 Subject: [PATCH 19/20] bring back dead_code markers --- crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 7e531e0fe87c0..7bb99870f0d62 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -95,8 +95,10 @@ pub(crate) struct EnumVariant<'a> { /// The fields within this variant. pub fields: EnumVariantFields<'a>, /// The reflection-based attributes on the variant. + #[allow(dead_code)] pub attrs: ReflectFieldAttr, /// The index of this variant within the enum. + #[allow(dead_code)] pub index: usize, } @@ -268,6 +270,7 @@ impl<'a> ReflectStruct<'a> { /// Access the data about which fields should be ignored during serialization. /// /// The returned bitset is a collection of indices obtained from the [`members_to_serialization_denylist`](crate::utility::members_to_serialization_denylist) function. + #[allow(dead_code)] pub fn serialization_denylist(&self) -> &BitSet { &self.serialization_denylist } @@ -311,6 +314,7 @@ impl<'a> ReflectStruct<'a> { } /// The complete set of fields in this struct. + #[allow(dead_code)] pub fn fields(&self) -> &[StructField<'a>] { &self.fields } From dbc13d3914ca9b9cdea1488f6dd92d29123e3880 Mon Sep 17 00:00:00 2001 From: makspll Date: Mon, 19 Sep 2022 11:25:44 +0100 Subject: [PATCH 20/20] fix doc comments --- crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 7bb99870f0d62..9fb4295ea7e70 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -290,7 +290,7 @@ impl<'a> ReflectStruct<'a> { ) } - /// Get a collection of types which are exposed to either the reflection API + /// Get a collection of types which are exposed to the reflection API pub fn active_types(&self) -> Vec { self.fields .iter() @@ -306,7 +306,7 @@ impl<'a> ReflectStruct<'a> { .filter(move |field| field.attrs.ignore.is_active()) } - /// Get an iterator of fields which are ignored by the reflection and serialization API + /// Get an iterator of fields which are ignored by the reflection API pub fn ignored_fields(&self) -> impl Iterator> { self.fields .iter()