From 48ab92742ea03b556011f078afc2fda8a02622fb Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Wed, 8 Feb 2023 18:36:11 -0700 Subject: [PATCH] Fix field ordering issues with #[reflect(skip_serializing)] --- .../bevy_reflect_derive/Cargo.toml | 1 - .../src/associated_data.rs | 66 ++++ .../bevy_reflect_derive/src/derive_data.rs | 81 +++-- .../bevy_reflect_derive/src/impls/enums.rs | 13 +- .../bevy_reflect_derive/src/impls/structs.rs | 7 +- .../src/impls/tuple_structs.rs | 7 +- .../bevy_reflect_derive/src/impls/values.rs | 7 +- .../bevy_reflect_derive/src/lib.rs | 19 +- .../bevy_reflect_derive/src/registration.rs | 13 +- .../bevy_reflect_derive/src/serialization.rs | 96 +++++ .../bevy_reflect_derive/src/utility.rs | 38 -- crates/bevy_reflect/src/serde/de.rs | 331 +++++++++--------- crates/bevy_reflect/src/serde/mod.rs | 3 +- crates/bevy_reflect/src/serde/ser.rs | 4 +- crates/bevy_reflect/src/serde/type_data.rs | 132 +++++-- crates/bevy_reflect/src/tuple_struct.rs | 13 +- 16 files changed, 562 insertions(+), 269 deletions(-) create mode 100644 crates/bevy_reflect/bevy_reflect_derive/src/associated_data.rs create mode 100644 crates/bevy_reflect/bevy_reflect_derive/src/serialization.rs diff --git a/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml b/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml index fa6b7259dce536..021a12427edf17 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml +++ b/crates/bevy_reflect/bevy_reflect_derive/Cargo.toml @@ -23,4 +23,3 @@ syn = { version = "2.0", features = ["full"] } proc-macro2 = "1.0" quote = "1.0" uuid = { version = "1.1", features = ["v4"] } -bit-set = "0.5.2" diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/associated_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/associated_data.rs new file mode 100644 index 00000000000000..aab4ed25b51123 --- /dev/null +++ b/crates/bevy_reflect/bevy_reflect_derive/src/associated_data.rs @@ -0,0 +1,66 @@ +use crate::field_attributes::{DefaultBehavior, ReflectFieldAttr, ReflectIgnoreBehavior}; +use crate::fq_std::{FQBox, FQDefault}; +use crate::utility::ident_or_index; +use proc_macro2::{Ident, TokenStream}; +use quote::{format_ident, ToTokens}; +use syn::{parse_quote, Field, ItemFn, Path}; + +/// Associated data to generate alongside derived trait implementations. +/// +/// It's important these are generated within the context of an [_unnamed const_] +/// in order to avoid conflicts and keep the macro hygenic. +/// +/// [_unnamed const_]: https://doc.rust-lang.org/stable/reference/items/constant-items.html#unnamed-constant +pub(crate) struct AssociatedData { + default_fn: Option, +} + +impl AssociatedData { + /// Generates a new `AssociatedData` for a given field. + pub fn new( + field: &Field, + index: usize, + attrs: &ReflectFieldAttr, + qualifier: &Ident, + bevy_reflect_path: &Path, + ) -> Self { + let field_ident = ident_or_index(field.ident.as_ref(), index); + let field_ty = &field.ty; + + let default_fn = match attrs.ignore { + ReflectIgnoreBehavior::IgnoreSerialization => { + let ident = format_ident!("get_default__{}__{}", qualifier, field_ident); + match &attrs.default { + DefaultBehavior::Required | DefaultBehavior::Default => Some(parse_quote! { + #[allow(non_snake_case)] + fn #ident() -> #FQBox { + #FQBox::new(<#field_ty as #FQDefault>::default()) + } + }), + DefaultBehavior::Func(func) => Some(parse_quote! { + #[allow(non_snake_case)] + fn #ident() -> #FQBox { + #FQBox::new(#func() as #field_ty) + } + }), + } + } + _ => None, + }; + + Self { default_fn } + } + + /// Returns the function used to generate a default instance of a field. + /// + /// Returns `None` if the field does not have or need such a function. + pub fn default_fn(&self) -> Option<&ItemFn> { + self.default_fn.as_ref() + } +} + +impl ToTokens for AssociatedData { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.default_fn.to_tokens(tokens); + } +} 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 a3e8082211b7d0..57b448fecdf7bf 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -1,14 +1,14 @@ +use crate::associated_data::AssociatedData; use crate::container_attributes::ReflectTraits; use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr}; use crate::fq_std::{FQAny, FQDefault, FQSend, FQSync}; -use crate::utility::{members_to_serialization_denylist, WhereClauseOptions}; -use bit_set::BitSet; -use quote::quote; -use syn::token::Comma; - +use crate::serialization::SerializationDataDef; +use crate::utility::WhereClauseOptions; use crate::{utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME}; +use quote::quote; use syn::punctuated::Punctuated; use syn::spanned::Spanned; +use syn::token::Comma; use syn::{Data, DeriveInput, Field, Fields, Generics, Ident, Meta, Path, Variant}; pub(crate) enum ReflectDerive<'a> { @@ -61,7 +61,7 @@ pub(crate) struct ReflectMeta<'a> { /// ``` pub(crate) struct ReflectStruct<'a> { meta: ReflectMeta<'a>, - serialization_denylist: BitSet, + serialization_data: Option, fields: Vec>, } @@ -89,6 +89,8 @@ pub(crate) struct StructField<'a> { pub data: &'a Field, /// The reflection-based attributes on the field. pub attrs: ReflectFieldAttr, + /// The associated data to be generated on behalf of this field. + pub associated_data: AssociatedData, /// The index of this field within the struct. pub declaration_index: usize, /// The index of this field as seen by the reflection API. @@ -212,12 +214,14 @@ impl<'a> ReflectDerive<'a> { return match &input.data { Data::Struct(data) => { - let fields = Self::collect_struct_fields(&data.fields)?; + let fields = Self::collect_struct_fields( + &data.fields, + meta.type_name, + meta.bevy_reflect_path(), + )?; let reflect_struct = ReflectStruct { meta, - serialization_denylist: members_to_serialization_denylist( - fields.iter().map(|v| v.attrs.ignore), - ), + serialization_data: SerializationDataDef::new(&fields)?, fields, }; @@ -228,7 +232,8 @@ impl<'a> ReflectDerive<'a> { } } Data::Enum(data) => { - let variants = Self::collect_enum_variants(&data.variants)?; + let variants = + Self::collect_enum_variants(&data.variants, meta.bevy_reflect_path())?; let reflect_enum = ReflectEnum { meta, variants }; Ok(Self::Enum(reflect_enum)) @@ -240,7 +245,11 @@ impl<'a> ReflectDerive<'a> { }; } - fn collect_struct_fields(fields: &'a Fields) -> Result>, syn::Error> { + fn collect_struct_fields( + fields: &'a Fields, + qualifier: &Ident, + bevy_reflect_path: &Path, + ) -> Result>, syn::Error> { let mut active_index = 0; let sifter: utility::ResultSifter> = fields .iter() @@ -248,6 +257,13 @@ impl<'a> ReflectDerive<'a> { .map( |(declaration_index, field)| -> Result { let attrs = parse_field_attrs(&field.attrs)?; + let associated_data = AssociatedData::new( + field, + declaration_index, + &attrs, + qualifier, + bevy_reflect_path, + ); let reflection_index = if attrs.ignore.is_ignored() { None @@ -260,6 +276,7 @@ impl<'a> ReflectDerive<'a> { declaration_index, reflection_index, attrs, + associated_data, data: field, #[cfg(feature = "documentation")] doc: crate::documentation::Documentation::from_attributes(&field.attrs), @@ -276,12 +293,17 @@ impl<'a> ReflectDerive<'a> { fn collect_enum_variants( variants: &'a Punctuated, + bevy_reflect_path: &Path, ) -> Result>, syn::Error> { let sifter: utility::ResultSifter> = variants .iter() .enumerate() .map(|(index, variant)| -> Result { - let fields = Self::collect_struct_fields(&variant.fields)?; + let fields = Self::collect_struct_fields( + &variant.fields, + &variant.ident, + bevy_reflect_path, + )?; let fields = match variant.fields { Fields::Named(..) => EnumVariantFields::Named(fields), @@ -304,6 +326,23 @@ impl<'a> ReflectDerive<'a> { sifter.finish() } + + /// The complete set of fields in this item. + pub fn fields(&self) -> Box> + '_> { + match self { + ReflectDerive::Struct(reflect_struct) + | ReflectDerive::TupleStruct(reflect_struct) + | ReflectDerive::UnitStruct(reflect_struct) => Box::new(reflect_struct.fields.iter()), + ReflectDerive::Enum(reflect_enum) => Box::new(reflect_enum.fields()), + ReflectDerive::Value(_) => Box::new(core::iter::empty()), + } + } + + /// Generate all [associated data](AssociatedData) into a `TokenStream`. + pub fn associated_data(&self) -> proc_macro2::TokenStream { + let associated_data = self.fields().map(|field| &field.associated_data); + quote!(#(#associated_data)*) + } } impl<'a> ReflectMeta<'a> { @@ -372,12 +411,9 @@ 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_denylist`](crate::utility::members_to_serialization_denylist) function. - #[allow(dead_code)] - pub fn serialization_denylist(&self) -> &BitSet { - &self.serialization_denylist + /// Returns the [`SerializationDataDef`] for this struct. + pub fn serialization_data(&self) -> Option<&SerializationDataDef> { + self.serialization_data.as_ref() } /// Returns the `GetTypeRegistration` impl as a `TokenStream`. @@ -395,7 +431,7 @@ impl<'a> ReflectStruct<'a> { self.meta.traits().idents(), self.meta.generics(), where_clause_options, - Some(&self.serialization_denylist), + self.serialization_data(), ) } @@ -463,6 +499,11 @@ impl<'a> ReflectEnum<'a> { &self.variants } + /// The complete set of fields in this enum. + pub fn fields(&self) -> impl Iterator> { + self.variants().iter().flat_map(|variant| variant.fields()) + } + /// Get an iterator of fields which are exposed to the reflection API pub fn active_fields(&self) -> impl Iterator> { self.variants() 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 1982870a711889..5617423cb8ad98 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs @@ -3,12 +3,11 @@ use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors}; use crate::fq_std::{FQAny, FQBox, FQOption, FQResult}; use crate::impls::impl_typed; use crate::utility::extend_where_clause; -use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; use quote::quote; use syn::Fields; -pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { +pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream { let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); let enum_name = reflect_enum.meta().type_name(); @@ -96,7 +95,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { let where_reflect_clause = extend_where_clause(where_clause, &where_clause_options); - TokenStream::from(quote! { + quote! { #get_type_registration_impl #typed_impl @@ -288,7 +287,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { #debug_fn } - }) + } } struct EnumImpls { @@ -351,7 +350,11 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden // Ignored field continue; } - constructor_argument.push(generate_for_field(reflect_idx, field.declaration_index, field)); + constructor_argument.push(generate_for_field( + reflect_idx, + field.declaration_index, + field, + )); reflect_idx += 1; } constructor_argument 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 d17cf2991bf7b1..6c94dc1ca4d6a6 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs @@ -2,11 +2,10 @@ use crate::fq_std::{FQAny, FQBox, FQDefault, FQOption, FQResult}; use crate::impls::impl_typed; use crate::utility::{extend_where_clause, ident_or_index}; use crate::ReflectStruct; -use proc_macro::TokenStream; use quote::{quote, ToTokens}; /// Implements `Struct`, `GetTypeRegistration`, and `Reflect` for the given derive data. -pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream { +pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenStream { let fqoption = FQOption.into_token_stream(); let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); @@ -100,7 +99,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream { let where_reflect_clause = extend_where_clause(where_clause, &where_clause_options); - TokenStream::from(quote! { + quote! { #get_type_registration_impl #typed_impl @@ -239,5 +238,5 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream { #debug_fn } - }) + } } 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 af7c2eb136e377..9b1294a748f0e8 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 @@ -2,12 +2,11 @@ use crate::fq_std::{FQAny, FQBox, FQDefault, FQOption, FQResult}; use crate::impls::impl_typed; use crate::utility::extend_where_clause; use crate::ReflectStruct; -use proc_macro::TokenStream; use quote::{quote, ToTokens}; use syn::{Index, Member}; /// Implements `TupleStruct`, `GetTypeRegistration`, and `Reflect` for the given derive data. -pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { +pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenStream { let fqoption = FQOption.into_token_stream(); let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); @@ -92,7 +91,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { let where_reflect_clause = extend_where_clause(where_clause, &where_clause_options); - TokenStream::from(quote! { + quote! { #get_type_registration_impl #typed_impl @@ -209,5 +208,5 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { #debug_fn } - }) + } } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs index 964ef1f2ce8a83..b1853eafd776de 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs @@ -2,11 +2,10 @@ use crate::fq_std::{FQAny, FQBox, FQClone, FQOption, FQResult}; use crate::impls::impl_typed; use crate::utility::WhereClauseOptions; use crate::ReflectMeta; -use proc_macro::TokenStream; use quote::quote; /// Implements `GetTypeRegistration` and `Reflect` for the given type data. -pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream { +pub(crate) fn impl_value(meta: &ReflectMeta) -> proc_macro2::TokenStream { let bevy_reflect_path = meta.bevy_reflect_path(); let type_name = meta.type_name(); @@ -37,7 +36,7 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream { let (impl_generics, ty_generics, where_clause) = meta.generics().split_for_impl(); let get_type_registration_impl = meta.get_type_registration(&where_clause_options); - TokenStream::from(quote! { + quote! { #get_type_registration_impl #typed_impl @@ -122,5 +121,5 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream { #debug_fn } - }) + } } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs index f7f2859d10734b..ce9dd4d39818ba 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs @@ -14,6 +14,7 @@ extern crate proc_macro; +mod associated_data; mod container_attributes; mod derive_data; #[cfg(feature = "documentation")] @@ -25,6 +26,7 @@ mod from_reflect; mod impls; mod reflect_value; mod registration; +mod serialization; mod trait_reflection; mod type_uuid; mod utility; @@ -133,14 +135,23 @@ pub fn derive_reflect(input: TokenStream) -> TokenStream { Err(err) => return err.into_compile_error().into(), }; - match derive_data { + let associated_data = derive_data.associated_data(); + + let output = match derive_data { ReflectDerive::Struct(struct_data) | ReflectDerive::UnitStruct(struct_data) => { impls::impl_struct(&struct_data) } ReflectDerive::TupleStruct(struct_data) => impls::impl_tuple_struct(&struct_data), ReflectDerive::Enum(meta) => impls::impl_enum(&meta), ReflectDerive::Value(meta) => impls::impl_value(&meta), - } + }; + + TokenStream::from(quote! { + const _: () = { + #associated_data + #output + }; + }) } /// Derives the `FromReflect` trait. @@ -282,7 +293,7 @@ pub fn impl_reflect_value(input: TokenStream) -> TokenStream { #[cfg(feature = "documentation")] let meta = meta.with_docs(documentation::Documentation::from_attributes(&def.attrs)); - impls::impl_value(&meta) + TokenStream::from(impls::impl_value(&meta)) } /// A replacement for `#[derive(Reflect)]` to be used with foreign types which @@ -323,7 +334,7 @@ pub fn impl_reflect_struct(input: TokenStream) -> TokenStream { match derive_data { ReflectDerive::Struct(struct_data) => { - let impl_struct: proc_macro2::TokenStream = impls::impl_struct(&struct_data).into(); + let impl_struct = impls::impl_struct(&struct_data); let impl_from_struct: proc_macro2::TokenStream = from_reflect::impl_struct(&struct_data).into(); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index 256fd82c558862..1171d59f39c7a1 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -1,7 +1,7 @@ //! Contains code related specifically to Bevy's type registration. +use crate::serialization::SerializationDataDef; use crate::utility::{extend_where_clause, WhereClauseOptions}; -use bit_set::BitSet; use proc_macro2::Ident; use quote::quote; use syn::{Generics, Path}; @@ -14,14 +14,13 @@ pub(crate) fn impl_get_type_registration( registration_data: &[Ident], generics: &Generics, where_clause_options: &WhereClauseOptions, - serialization_denylist: Option<&BitSet>, + serialization_data: Option<&SerializationDataDef>, ) -> proc_macro2::TokenStream { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let serialization_data = serialization_denylist.map(|denylist| { - let denylist = denylist.into_iter(); + let register_serialization_data = serialization_data.map(|data| { + let serialization_data = data.as_serialization_data(bevy_reflect_path); quote! { - let ignored_indices = ::core::iter::IntoIterator::into_iter([#(#denylist),*]); - registration.insert::<#bevy_reflect_path::serde::SerializationData>(#bevy_reflect_path::serde::SerializationData::new(ignored_indices)); + registration.insert::<#bevy_reflect_path::serde::SerializationData>(#serialization_data); } }); @@ -33,7 +32,7 @@ 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::<#bevy_reflect_path::ReflectFromPtr>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type()); - #serialization_data + #register_serialization_data #(registration.insert::<#registration_data>(#bevy_reflect_path::FromType::<#type_name #ty_generics>::from_type());)* registration } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/serialization.rs b/crates/bevy_reflect/bevy_reflect_derive/src/serialization.rs new file mode 100644 index 00000000000000..c12fe5715bfc19 --- /dev/null +++ b/crates/bevy_reflect/bevy_reflect_derive/src/serialization.rs @@ -0,0 +1,96 @@ +use crate::derive_data::StructField; +use crate::field_attributes::ReflectIgnoreBehavior; +use proc_macro2::Ident; +use quote::quote; +use std::collections::HashMap; +use syn::spanned::Spanned; +use syn::Path; + +type ReflectionIndex = usize; + +/// Collected serialization data used to generate a `SerializationData` type. +pub(crate) struct SerializationDataDef { + /// Maps a field's _reflection_ index to its [`SkippedFieldDef`] if marked as `#[reflect(skip_serializing)]`. + skipped: HashMap, +} + +impl SerializationDataDef { + /// Attempts to create a new `SerializationDataDef` from the given collection of fields. + /// + /// Returns `Ok(Some(data))` if there are any fields needing to be skipped during serialization. + /// Otherwise, returns `Ok(None)`. + pub fn new(fields: &[StructField<'_>]) -> Result, syn::Error> { + let mut skipped = HashMap::default(); + + for field in fields { + match field.attrs.ignore { + ReflectIgnoreBehavior::IgnoreSerialization => { + skipped.insert( + field.reflection_index.ok_or_else(|| { + syn::Error::new( + field.data.span(), + "internal error: field is missing a reflection index", + ) + })?, + SkippedFieldDef::new(field)?, + ); + } + _ => continue, + } + } + + if skipped.is_empty() { + Ok(None) + } else { + Ok(Some(Self { skipped })) + } + } + + /// Returns a `TokenStream` containing an initialized `SerializationData` type. + pub fn as_serialization_data(&self, bevy_reflect_path: &Path) -> proc_macro2::TokenStream { + let fields = self.skipped.iter().map(|(reflection_index, data)| { + let SkippedFieldDef { + associated_default_fn: default_fn, + } = data; + quote! {( + #reflection_index, + #bevy_reflect_path::serde::SkippedField::new(#default_fn) + )} + }); + quote! { + #bevy_reflect_path::serde::SerializationData::new( + ::core::iter::IntoIterator::into_iter([#(#fields),*]) + ) + } + } +} + +/// Collected field data used to generate a `SkippedField` type. +pub(crate) struct SkippedFieldDef { + /// The identifier of the [default function] generated by [`AssociatedData`]. + /// + /// [default function]: crate::associated_data::AssociatedData::default_fn + /// [`AssociatedData`]: crate::associated_data::AssociatedData + associated_default_fn: Ident, +} + +impl SkippedFieldDef { + pub fn new(field: &StructField<'_>) -> Result { + let associated_default_fn = field + .associated_data + .default_fn() + .ok_or_else(|| { + syn::Error::new( + field.data.span(), + "internal error: field is missing an associated default function", + ) + })? + .sig + .ident + .clone(); + + Ok(Self { + associated_default_fn, + }) + } +} diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index 76d45f39e24a59..5fd6d1b155b2f1 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -1,8 +1,6 @@ //! General-purpose utility functions for internal usage within this crate. -use crate::field_attributes::ReflectIgnoreBehavior; use bevy_macro_utils::BevyManifest; -use bit_set::BitSet; use proc_macro2::{Ident, Span}; use quote::quote; use syn::{Member, Path, Type, WhereClause}; @@ -176,39 +174,3 @@ impl ResultSifter { } } } - -/// Converts an iterator over ignore behavior of members to a bitset of ignored members. -/// -/// Takes into account the fact that always ignored (non-reflected) members are skipped. -/// -/// # 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_denylist(member_iter: T) -> BitSet -where - T: Iterator, -{ - let mut bitset = BitSet::default(); - - member_iter.fold(0, |next_idx, member| match member { - ReflectIgnoreBehavior::IgnoreAlways => next_idx, - ReflectIgnoreBehavior::IgnoreSerialization => { - bitset.insert(next_idx); - next_idx + 1 - } - ReflectIgnoreBehavior::None => next_idx + 1, - }); - - bitset -} diff --git a/crates/bevy_reflect/src/serde/de.rs b/crates/bevy_reflect/src/serde/de.rs index de7c47438104c6..7568b369ed3a46 100644 --- a/crates/bevy_reflect/src/serde/de.rs +++ b/crates/bevy_reflect/src/serde/de.rs @@ -2,9 +2,8 @@ use crate::serde::SerializationData; use crate::{ ArrayInfo, DynamicArray, DynamicEnum, DynamicList, DynamicMap, DynamicStruct, DynamicTuple, DynamicTupleStruct, DynamicVariant, EnumInfo, ListInfo, Map, MapInfo, NamedField, Reflect, - ReflectDeserialize, StructInfo, StructVariantInfo, Tuple, TupleInfo, TupleStruct, - TupleStructInfo, TupleVariantInfo, TypeInfo, TypeRegistration, TypeRegistry, UnnamedField, - VariantInfo, + ReflectDeserialize, StructInfo, StructVariantInfo, TupleInfo, TupleStructInfo, + TupleVariantInfo, TypeInfo, TypeRegistration, TypeRegistry, UnnamedField, VariantInfo, }; use erased_serde::Deserializer; use serde::de::{ @@ -26,6 +25,8 @@ pub trait DeserializeValue { trait StructLikeInfo { fn get_name(&self) -> &str; fn get_field(&self, name: &str) -> Option<&NamedField>; + fn field_at(&self, index: usize) -> Option<&NamedField>; + fn get_field_len(&self) -> usize; fn iter_fields(&self) -> Iter<'_, NamedField>; } @@ -48,10 +49,18 @@ impl StructLikeInfo for StructInfo { self.type_name() } + fn field_at(&self, index: usize) -> Option<&NamedField> { + self.field_at(index) + } + fn get_field(&self, name: &str) -> Option<&NamedField> { self.field(name) } + fn get_field_len(&self) -> usize { + self.field_len() + } + fn iter_fields(&self) -> Iter<'_, NamedField> { self.iter() } @@ -79,10 +88,18 @@ impl StructLikeInfo for StructVariantInfo { self.name() } + fn field_at(&self, index: usize) -> Option<&NamedField> { + self.field_at(index) + } + fn get_field(&self, name: &str) -> Option<&NamedField> { self.field(name) } + fn get_field_len(&self) -> usize { + self.field_len() + } + fn iter_fields(&self) -> Iter<'_, NamedField> { self.iter() } @@ -119,6 +136,54 @@ impl TupleLikeInfo for TupleInfo { } } +impl Container for TupleInfo { + fn get_field_registration<'a, E: Error>( + &self, + index: usize, + registry: &'a TypeRegistry, + ) -> Result<&'a TypeRegistration, E> { + let field = self.field_at(index).ok_or_else(|| { + de::Error::custom(format_args!( + "no field at index {} on tuple {}", + index, + self.type_name(), + )) + })?; + get_registration(field.type_id(), field.type_name(), registry) + } +} + +impl TupleLikeInfo for TupleStructInfo { + fn get_name(&self) -> &str { + self.type_name() + } + + fn get_field(&self, index: usize) -> Option<&UnnamedField> { + self.field_at(index) + } + + fn get_field_len(&self) -> usize { + self.field_len() + } +} + +impl Container for TupleStructInfo { + fn get_field_registration<'a, E: Error>( + &self, + index: usize, + registry: &'a TypeRegistry, + ) -> Result<&'a TypeRegistration, E> { + let field = self.field_at(index).ok_or_else(|| { + de::Error::custom(format_args!( + "no field at index {} on tuple struct {}", + index, + self.type_name(), + )) + })?; + get_registration(field.type_id(), field.type_name(), registry) + } +} + impl TupleLikeInfo for TupleVariantInfo { fn get_name(&self) -> &str { self.name() @@ -133,6 +198,23 @@ impl TupleLikeInfo for TupleVariantInfo { } } +impl Container for TupleVariantInfo { + fn get_field_registration<'a, E: Error>( + &self, + index: usize, + registry: &'a TypeRegistry, + ) -> Result<&'a TypeRegistration, E> { + let field = self.field_at(index).ok_or_else(|| { + de::Error::custom(format_args!( + "no field at index {} on tuple variant {}", + index, + self.name(), + )) + })?; + get_registration(field.type_id(), field.type_name(), registry) + } +} + /// A debug struct used for error messages that displays a list of expected values. /// /// # Example @@ -437,6 +519,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { tuple_info.field_len(), TupleVisitor { tuple_info, + registration: self.registration, registry: self.registry, }, )?; @@ -491,43 +574,14 @@ impl<'a, 'de> Visitor<'de> for StructVisitor<'a> { where V: MapAccess<'de>, { - visit_struct(&mut map, self.struct_info, self.registry) + visit_struct(&mut map, self.struct_info, self.registration, self.registry) } fn visit_seq(self, mut seq: A) -> Result where A: SeqAccess<'de>, { - let mut index = 0usize; - let mut output = DynamicStruct::default(); - - let ignored_len = self - .registration - .data::() - .map(|data| data.len()) - .unwrap_or(0); - let field_len = self.struct_info.field_len().saturating_sub(ignored_len); - - if field_len == 0 { - // Handle unit structs and ignored fields - return Ok(output); - } - - while let Some(value) = seq.next_element_seed(TypedReflectDeserializer { - registration: self - .struct_info - .get_field_registration(index, self.registry)?, - registry: self.registry, - })? { - let name = self.struct_info.field_at(index).unwrap().name(); - output.insert_boxed(name, value); - index += 1; - if index >= self.struct_info.field_len() { - break; - } - } - - Ok(output) + visit_struct_seq(&mut seq, self.struct_info, self.registration, self.registry) } } @@ -548,64 +602,19 @@ impl<'a, 'de> Visitor<'de> for TupleStructVisitor<'a> { where V: SeqAccess<'de>, { - let mut index = 0usize; - let mut tuple_struct = DynamicTupleStruct::default(); - - let ignored_len = self - .registration - .data::() - .map(|data| data.len()) - .unwrap_or(0); - let field_len = self - .tuple_struct_info - .field_len() - .saturating_sub(ignored_len); - - if field_len == 0 { - // Handle unit structs and ignored fields - return Ok(tuple_struct); - } - - let get_field_registration = |index: usize| -> Result<&'a TypeRegistration, V::Error> { - let field = self.tuple_struct_info.field_at(index).ok_or_else(|| { - de::Error::custom(format_args!( - "no field at index {} on tuple {}", - index, - self.tuple_struct_info.type_name(), - )) - })?; - get_registration(field.type_id(), field.type_name(), self.registry) - }; - - while let Some(value) = seq.next_element_seed(TypedReflectDeserializer { - registration: get_field_registration(index)?, - registry: self.registry, - })? { - tuple_struct.insert_boxed(value); - index += 1; - if index >= self.tuple_struct_info.field_len() { - break; - } - } - - let ignored_len = self - .registration - .data::() - .map(|data| data.len()) - .unwrap_or(0); - if tuple_struct.field_len() != self.tuple_struct_info.field_len() - ignored_len { - return Err(Error::invalid_length( - tuple_struct.field_len(), - &self.tuple_struct_info.field_len().to_string().as_str(), - )); - } - - Ok(tuple_struct) + visit_tuple( + &mut seq, + self.tuple_struct_info, + self.registration, + self.registry, + ) + .map(DynamicTupleStruct::from) } } struct TupleVisitor<'a> { tuple_info: &'static TupleInfo, + registration: &'a TypeRegistration, registry: &'a TypeRegistry, } @@ -620,7 +629,7 @@ impl<'a, 'de> Visitor<'de> for TupleVisitor<'a> { where V: SeqAccess<'de>, { - visit_tuple(&mut seq, self.tuple_info, self.registry) + visit_tuple(&mut seq, self.tuple_info, self.registration, self.registry) } } @@ -773,9 +782,7 @@ impl<'a, 'de> Visitor<'de> for EnumVisitor<'a> { )? .into(), VariantInfo::Tuple(tuple_info) if tuple_info.field_len() == 1 => { - let field = tuple_info.field_at(0).unwrap(); - let registration = - get_registration(field.type_id(), field.type_name(), self.registry)?; + let registration = tuple_info.get_field_registration(0, self.registry)?; let value = variant.newtype_variant_seed(TypedReflectDeserializer { registration, registry: self.registry, @@ -870,43 +877,14 @@ impl<'a, 'de> Visitor<'de> for StructVariantVisitor<'a> { where V: MapAccess<'de>, { - visit_struct(&mut map, self.struct_info, self.registry) + visit_struct(&mut map, self.struct_info, self.registration, self.registry) } fn visit_seq(self, mut seq: A) -> Result where A: SeqAccess<'de>, { - let mut index = 0usize; - let mut output = DynamicStruct::default(); - - let ignored_len = self - .registration - .data::() - .map(|data| data.len()) - .unwrap_or(0); - let field_len = self.struct_info.field_len().saturating_sub(ignored_len); - - if field_len == 0 { - // Handle all fields being ignored - return Ok(output); - } - - while let Some(value) = seq.next_element_seed(TypedReflectDeserializer { - registration: self - .struct_info - .get_field_registration(index, self.registry)?, - registry: self.registry, - })? { - let name = self.struct_info.field_at(index).unwrap().name(); - output.insert_boxed(name, value); - index += 1; - if index >= self.struct_info.field_len() { - break; - } - } - - Ok(output) + visit_struct_seq(&mut seq, self.struct_info, self.registration, self.registry) } } @@ -927,19 +905,7 @@ impl<'a, 'de> Visitor<'de> for TupleVariantVisitor<'a> { where V: SeqAccess<'de>, { - let ignored_len = self - .registration - .data::() - .map(|data| data.len()) - .unwrap_or(0); - let field_len = self.tuple_info.field_len().saturating_sub(ignored_len); - - if field_len == 0 { - // Handle all fields being ignored - return Ok(DynamicTuple::default()); - } - - visit_tuple(&mut seq, self.tuple_info, self.registry) + visit_tuple(&mut seq, self.tuple_info, self.registration, self.registry) } } @@ -996,6 +962,7 @@ impl<'a, 'de> Visitor<'de> for OptionVisitor<'a> { fn visit_struct<'de, T, V>( map: &mut V, info: &'static T, + registration: &TypeRegistration, registry: &TypeRegistry, ) -> Result where @@ -1020,49 +987,99 @@ where dynamic_struct.insert_boxed(&key, value); } + if let Some(serialization_data) = registration.data::() { + for (skipped_index, skipped_field) in serialization_data.iter_skipped() { + let Some(field) = info.field_at(*skipped_index) else { continue }; + dynamic_struct.insert_boxed(field.name(), skipped_field.generate_default()); + } + } + Ok(dynamic_struct) } fn visit_tuple<'de, T, V>( seq: &mut V, info: &T, + registration: &TypeRegistration, registry: &TypeRegistry, ) -> Result where - T: TupleLikeInfo, + T: TupleLikeInfo + Container, V: SeqAccess<'de>, { let mut tuple = DynamicTuple::default(); - let mut index = 0usize; - let get_field_registration = |index: usize| -> Result<&TypeRegistration, V::Error> { - let field = info.get_field(index).ok_or_else(|| { - Error::invalid_length(index, &info.get_field_len().to_string().as_str()) - })?; - get_registration(field.type_id(), field.type_name(), registry) - }; + let len = info.get_field_len(); - while let Some(value) = seq.next_element_seed(TypedReflectDeserializer { - registration: get_field_registration(index)?, - registry, - })? { - tuple.insert_boxed(value); - index += 1; - if index >= info.get_field_len() { - break; + if len == 0 { + // Handle empty tuple/tuple struct + return Ok(tuple); + } + + let serialization_data = registration.data::(); + + for index in 0..len { + if let Some(value) = serialization_data.and_then(|data| data.generate_default(index)) { + tuple.insert_boxed(value); + continue; } + + let value = seq + .next_element_seed(TypedReflectDeserializer { + registration: info.get_field_registration(index, registry)?, + registry, + })? + .ok_or_else(|| Error::invalid_length(index, &len.to_string().as_str()))?; + tuple.insert_boxed(value); } + Ok(tuple) +} + +fn visit_struct_seq<'de, T, V>( + seq: &mut V, + info: &T, + registration: &TypeRegistration, + registry: &TypeRegistry, +) -> Result +where + T: StructLikeInfo + Container, + V: SeqAccess<'de>, +{ + let mut dynamic_struct = DynamicStruct::default(); + let len = info.get_field_len(); - if tuple.field_len() != len { - return Err(Error::invalid_length( - tuple.field_len(), - &len.to_string().as_str(), - )); + if len == 0 { + // Handle unit structs + return Ok(dynamic_struct); } - Ok(tuple) + let serialization_data = registration.data::(); + + for index in 0..len { + let name = info.field_at(index).unwrap().name(); + + if serialization_data + .map(|data| data.is_field_skipped(index)) + .unwrap_or_default() + { + if let Some(value) = serialization_data.unwrap().generate_default(index) { + dynamic_struct.insert_boxed(name, value); + } + continue; + } + + let value = seq + .next_element_seed(TypedReflectDeserializer { + registration: info.get_field_registration(index, registry)?, + registry, + })? + .ok_or_else(|| Error::invalid_length(index, &len.to_string().as_str()))?; + dynamic_struct.insert_boxed(name, value); + } + + Ok(dynamic_struct) } fn get_registration<'a, E: Error>( diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index e496775480167a..e5663df7d76127 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -46,6 +46,7 @@ mod tests { let mut expected = DynamicStruct::default(); expected.insert("a", 3); expected.insert("d", 6); + expected.insert("c", 0); let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap(); let reflect_deserializer = UntypedReflectDeserializer::new(®istry); @@ -87,8 +88,6 @@ mod tests { let value = reflect_deserializer.deserialize(&mut deserializer).unwrap(); let deserialized = value.take::().unwrap(); - println!("{:?}", deserialized); - let expected = TestStruct(3, 0, 0, 6); let received = ::from_reflect(&deserialized).unwrap(); diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index 38d4529a3a40b1..9767847c63d102 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -197,7 +197,7 @@ impl<'a> Serialize for StructSerializer<'a> { for (index, value) in self.struct_value.iter_fields().enumerate() { if serialization_data - .map(|data| data.is_ignored_field(index)) + .map(|data| data.is_field_skipped(index)) .unwrap_or(false) { continue; @@ -250,7 +250,7 @@ impl<'a> Serialize for TupleStructSerializer<'a> { for (index, value) in self.tuple_struct.iter_fields().enumerate() { if serialization_data - .map(|data| data.is_ignored_field(index)) + .map(|data| data.is_field_skipped(index)) .unwrap_or(false) { continue; diff --git a/crates/bevy_reflect/src/serde/type_data.rs b/crates/bevy_reflect/src/serde/type_data.rs index ee69a390d09cb8..d82f3b45790955 100644 --- a/crates/bevy_reflect/src/serde/type_data.rs +++ b/crates/bevy_reflect/src/serde/type_data.rs @@ -1,44 +1,136 @@ -use std::collections::HashSet; +use crate::Reflect; +use bevy_utils::hashbrown::hash_map::Iter; +use bevy_utils::HashMap; -/// Contains data relevant to the automatic reflect powered serialization of a type +/// Contains data relevant to the automatic reflect powered (de)serialization of a type. #[derive(Debug, Clone)] pub struct SerializationData { - ignored_field_indices: HashSet, + skipped_fields: HashMap, } impl SerializationData { - /// Creates a new `SerializationData` instance given: + /// Creates a new `SerializationData` instance with the given skipped fields. /// - /// - `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 { + /// # Arguments + /// + /// * `skipped_iter`: The iterator of field indices to be skipped during (de)serialization. + /// Indices are assigned only to reflected fields. + /// Ignored fields (i.e. those marked `#[reflect(ignore)]`) are implicitly skipped + /// and do not need to be included in this iterator. + pub fn new>(skipped_iter: I) -> Self { Self { - ignored_field_indices: ignored_iter.collect(), + skipped_fields: skipped_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 are skipped. + /// Returns true if the given index corresponds to a field meant to be skipped during (de)serialization. /// /// # Example /// - /// ```rust,ignore + /// ``` + /// # use std::any::TypeId; + /// # use bevy_reflect::{Reflect, Struct, TypeRegistry, serde::SerializationData}; + /// #[derive(Reflect)] + /// struct MyStruct { + /// serialize_me: i32, + /// #[reflect(skip_serializing)] + /// skip_me: i32 + /// } + /// + /// let mut registry = TypeRegistry::new(); + /// registry.register::(); + /// + /// let my_struct = MyStruct { + /// serialize_me: 123, + /// skip_me: 321, + /// }; + /// + /// let serialization_data = registry.get_type_data::(TypeId::of::()).unwrap(); + /// /// for (idx, field) in my_struct.iter_fields().enumerate(){ - /// if serialization_data.is_ignored_field(idx){ - /// // serialize ... - /// } + /// if serialization_data.is_field_skipped(idx) { + /// // Skipped! + /// assert_eq!(1, idx); + /// } else { + /// // Not Skipped! + /// assert_eq!(0, idx); + /// } + /// } + /// ``` + pub fn is_field_skipped(&self, index: usize) -> bool { + self.skipped_fields.contains_key(&index) + } + + /// Generates a default instance of the skipped field at the given index. + /// + /// Returns `None` if the field is not skipped. + /// + /// # Example + /// + /// ``` + /// # use std::any::TypeId; + /// # use bevy_reflect::{Reflect, Struct, TypeRegistry, serde::SerializationData}; + /// #[derive(Reflect)] + /// struct MyStruct { + /// serialize_me: i32, + /// #[reflect(skip_serializing)] + /// #[reflect(default = "skip_me_default")] + /// skip_me: i32 /// } + /// + /// fn skip_me_default() -> i32 { + /// 789 + /// } + /// + /// let mut registry = TypeRegistry::new(); + /// registry.register::(); + /// + /// let serialization_data = registry.get_type_data::(TypeId::of::()).unwrap(); + /// assert_eq!(789, serialization_data.generate_default(1).unwrap().take::().unwrap()); /// ``` - pub fn is_ignored_field(&self, index: usize) -> bool { - self.ignored_field_indices.contains(&index) + pub fn generate_default(&self, index: usize) -> Option> { + self.skipped_fields + .get(&index) + .map(|field| field.generate_default()) } - /// Returns the number of ignored fields. + /// Returns the number of skipped fields. pub fn len(&self) -> usize { - self.ignored_field_indices.len() + self.skipped_fields.len() } - /// Returns true if there are no ignored fields. + /// Returns true if there are no skipped fields. pub fn is_empty(&self) -> bool { - self.ignored_field_indices.is_empty() + self.skipped_fields.is_empty() + } + + /// Returns an iterator over the skipped fields. + /// + /// Each item in the iterator is a tuple containing: + /// 1. The reflected index of the field + /// 2. The (de)serialization metadata of the field + pub fn iter_skipped(&self) -> Iter<'_, usize, SkippedField> { + self.skipped_fields.iter() + } +} + +/// Data needed for (de)serialization of a skipped field. +#[derive(Debug, Clone)] +pub struct SkippedField { + default_fn: fn() -> Box, +} + +impl SkippedField { + /// Create a new `SkippedField`. + /// + /// # Arguments + /// + /// * `default_fn`: A function pointer used to generate a default instance of the field. + pub fn new(default_fn: fn() -> Box) -> Self { + Self { default_fn } + } + + /// Generates a default instance of the field. + pub fn generate_default(&self) -> Box { + (self.default_fn)() } } diff --git a/crates/bevy_reflect/src/tuple_struct.rs b/crates/bevy_reflect/src/tuple_struct.rs index 9b7466d5757ad1..ebf7dbfcce1f27 100644 --- a/crates/bevy_reflect/src/tuple_struct.rs +++ b/crates/bevy_reflect/src/tuple_struct.rs @@ -1,4 +1,6 @@ -use crate::{Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, UnnamedField}; +use crate::{ + DynamicTuple, Reflect, ReflectMut, ReflectOwned, ReflectRef, Tuple, TypeInfo, UnnamedField, +}; use std::any::{Any, TypeId}; use std::fmt::{Debug, Formatter}; use std::slice::Iter; @@ -392,6 +394,15 @@ impl Debug for DynamicTupleStruct { } } +impl From for DynamicTupleStruct { + fn from(value: DynamicTuple) -> Self { + Self { + represented_type: None, + fields: Box::new(value).drain(), + } + } +} + /// Compares a [`TupleStruct`] with a [`Reflect`] value. /// /// Returns true if and only if all of the following are true: