Fix field ordering issues with #[reflect(skip_serializing)]

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" 

crates/bevy_reflect/bevy_reflect_derive/src/associated_data.rs

@@ -0,0 +1,67 @@
+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
+#[derive(Clone)]
+pub(crate) struct AssociatedData {
+ default_fn: Option<ItemFn>,
+}
+
+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<dyn #bevy_reflect_path::Reflect> {
+ #FQBox::new(<#field_ty as #FQDefault>::default())
+ }
+ }),
+ DefaultBehavior::Func(func) => Some(parse_quote! {
+ #[allow(non_snake_case)]
+ fn #ident() -> #FQBox<dyn #bevy_reflect_path::Reflect> {
+ #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);
+ }
+}

crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs

@@ -1,11 +1,12 @@
+use crate::associated_data::AssociatedData;
 use crate::container_attributes::{FromReflectAttrs, ReflectTraits};
 use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr};
 use crate::type_path::parse_path_no_leading_colon;
-use crate::utility::{members_to_serialization_denylist, StringExpr, WhereClauseOptions};
-use bit_set::BitSet;
+use crate::utility::{StringExpr, WhereClauseOptions};
 use quote::{quote, ToTokens};
 use syn::token::Comma;
 
+use crate::serialization::SerializationDataDef;
 use crate::{
  utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME, TYPE_NAME_ATTRIBUTE_NAME,
  TYPE_PATH_ATTRIBUTE_NAME,
@@ -65,7 +66,7 @@ pub(crate) struct ReflectMeta<'a> {
 /// ```
 pub(crate) struct ReflectStruct<'a> {
  meta: ReflectMeta<'a>,
- serialization_denylist: BitSet<u32>,
+ serialization_data: Option<SerializationDataDef>,
  fields: Vec<StructField<'a>>,
 }
 
@@ -94,6 +95,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.
@@ -276,12 +279,16 @@ 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_path()
+ .get_ident()
+ .expect("structs should never be anonymous"),
+ 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,
  };
 
@@ -292,7 +299,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))
@@ -314,14 +322,25 @@ impl<'a> ReflectDerive<'a> {
  }
  }
 
- fn collect_struct_fields(fields: &'a Fields) -> Result<Vec<StructField<'a>>, syn::Error> {
+ fn collect_struct_fields(
+ fields: &'a Fields,
+ qualifier: &Ident,
+ bevy_reflect_path: &Path,
+ ) -> Result<Vec<StructField<'a>>, syn::Error> {
  let mut active_index = 0;
  let sifter: utility::ResultSifter<StructField<'a>> = fields
  .iter()
  .enumerate()
  .map(
  |(declaration_index, field)| -> Result<StructField, syn::Error> {
  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
@@ -334,6 +353,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),
@@ -350,12 +370,17 @@ impl<'a> ReflectDerive<'a> {
 
  fn collect_enum_variants(
  variants: &'a Punctuated<Variant, Comma>,
+ bevy_reflect_path: &Path,
  ) -> Result<Vec<EnumVariant<'a>>, syn::Error> {
  let sifter: utility::ResultSifter<EnumVariant<'a>> = variants
  .iter()
  .enumerate()
  .map(|(index, variant)| -> Result<EnumVariant, syn::Error> {
- 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),
@@ -378,6 +403,23 @@ impl<'a> ReflectDerive<'a> {
 
  sifter.finish()
  }
+
+ /// The complete set of fields in this item.
+ pub fn fields(&self) -> Box<dyn Iterator<Item = &StructField<'a>> + '_> {
+ 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> {
@@ -439,12 +481,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<u32> {
- &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`.
@@ -457,7 +496,7 @@ impl<'a> ReflectStruct<'a> {
  crate::registration::impl_get_type_registration(
  self.meta(),
  where_clause_options,
- Some(&self.serialization_denylist),
+ self.serialization_data(),
  )
  }
 
@@ -512,6 +551,11 @@ impl<'a> ReflectEnum<'a> {
  &self.variants
  }
 
+ /// The complete set of fields in this enum.
+ pub fn fields(&self) -> impl Iterator<Item = &StructField<'a>> {
+ 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<Item = &StructField<'a>> {
  self.variants()

crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs

@@ -353,7 +353,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

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_path;
 mod type_uuid;
@@ -168,6 +170,8 @@ pub fn derive_reflect(input: TokenStream) -> TokenStream {
  Err(err) => return err.into_compile_error().into(),
  };
 
+ let associated_data = derive_data.associated_data();
+
  let (reflect_impls, from_reflect_impl) = match derive_data {
  ReflectDerive::Struct(struct_data) | ReflectDerive::UnitStruct(struct_data) => (
  impls::impl_struct(&struct_data),
@@ -204,8 +208,11 @@ pub fn derive_reflect(input: TokenStream) -> TokenStream {
  };
 
  TokenStream::from(quote! {
- #reflect_impls
- #from_reflect_impl
+ const _: () = {
+ #associated_data
+ #reflect_impls
+ #from_reflect_impl
+ };
  })
 }
 

crates/bevy_reflect/bevy_reflect_derive/src/registration.rs

@@ -1,16 +1,16 @@
 //! Contains code related specifically to Bevy's type registration.
 
 use crate::derive_data::ReflectMeta;
+use crate::serialization::SerializationDataDef;
 use crate::utility::{extend_where_clause, WhereClauseOptions};
-use bit_set::BitSet;
 use quote::quote;
 
 /// Creates the `GetTypeRegistration` impl for the given type data.
 #[allow(clippy::too_many_arguments)]
 pub(crate) fn impl_get_type_registration(
  meta: &ReflectMeta,
  where_clause_options: &WhereClauseOptions,
- serialization_denylist: Option<&BitSet<u32>>,
+ serialization_data: Option<&SerializationDataDef>,
 ) -> proc_macro2::TokenStream {
  let type_path = meta.type_path();
  let bevy_reflect_path = meta.bevy_reflect_path();
@@ -20,17 +20,16 @@ pub(crate) fn impl_get_type_registration(
 
  let from_reflect_data = if meta.from_reflect().should_auto_derive() {
  Some(quote! {
- registration.insert::<#bevy_reflect_path::ReflectFromReflect>(#bevy_reflect_path::FromType::<Self>::from_type());
+  registration.insert::<#bevy_reflect_path::ReflectFromReflect>(#bevy_reflect_path::FromType::<Self>::from_type());
  })
  } else {
  None
  };
 
- let serialization_data = serialization_denylist.map(|denylist| {
- let denylist = denylist.into_iter();
+ let 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);
  }
  });