Fix unintuitive ignored field ordering issues with FromReflect

crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs

@@ -89,7 +89,14 @@ pub(crate) struct StructField<'a> {
  /// The reflection-based attributes on the field.
  pub attrs: ReflectFieldAttr,
  /// The index of this field within the struct.
- pub index: usize,
+ pub declaration_index: usize,
+ /// The index of this field as seen by the reflection API.
+ ///
+ /// This index accounts for the removal of [ignored] fields.
+ /// It will only be `Some(index)` when the field is not ignored.
+ ///
+ /// [ignored]: crate::field_attributes::ReflectIgnoreBehavior::IgnoreAlways
+ pub reflection_index: Option<usize>,
  /// The documentation for this field, if any
  #[cfg(feature = "documentation")]
  pub doc: crate::documentation::Documentation,
@@ -225,19 +232,31 @@ impl<'a> ReflectDerive<'a> {
  }
 
  fn collect_struct_fields(fields: &'a Fields) -> Result<Vec<StructField<'a>>, syn::Error> {
+ let mut active_index = 0;
  let sifter: utility::ResultSifter<StructField<'a>> = fields
  .iter()
  .enumerate()
- .map(|(index, field)| -> Result<StructField, syn::Error> {
- let attrs = parse_field_attrs(&field.attrs)?;
- Ok(StructField {
- index,
- attrs,
- data: field,
- #[cfg(feature = "documentation")]
- doc: crate::documentation::Documentation::from_attributes(&field.attrs),
- })
- })
+ .map(
+ |(declaration_index, field)| -> Result<StructField, syn::Error> {
+ let attrs = parse_field_attrs(&field.attrs)?;
+
+ let reflection_index = if attrs.ignore.is_ignored() {
+ None
+ } else {
+ active_index += 1;
+ Some(active_index - 1)
+ };
+
+ Ok(StructField {
+ declaration_index,
+ reflection_index,
+ attrs,
+ data: field,
+ #[cfg(feature = "documentation")]
+ doc: crate::documentation::Documentation::from_attributes(&field.attrs),
+ })
+ },
+ )
  .fold(
  utility::ResultSifter::default(),
  utility::ResultSifter::fold,

crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs

@@ -169,7 +169,7 @@ fn get_ignored_fields(reflect_struct: &ReflectStruct) -> MemberValuePair {
  reflect_struct
  .ignored_fields()
  .map(|field| {
- let member = ident_or_index(field.data.ident.as_ref(), field.index);
+ let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index);
 
  let value = match &field.attrs.default {
  DefaultBehavior::Func(path) => quote! {#path()},
@@ -198,8 +198,12 @@ fn get_active_fields(
  reflect_struct
  .active_fields()
  .map(|field| {
- let member = ident_or_index(field.data.ident.as_ref(), field.index);
- let accessor = get_field_accessor(field.data, field.index, is_tuple);
+ let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index);
+ let accessor = get_field_accessor(
+ field.data,
+ field.reflection_index.expect("field should be active"),
+ is_tuple,
+ );
  let ty = field.data.ty.clone();
 
  let get_field = quote! {

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

@@ -351,7 +351,7 @@ fn generate_impls(reflect_enum: &ReflectEnum, ref_index: &Ident, ref_name: &Iden
  // Ignored field
  continue;
  }
- constructor_argument.push(generate_for_field(reflect_idx, field.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/impls/structs.rs

@@ -20,12 +20,12 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
  .ident
  .as_ref()
  .map(|i| i.to_string())
- .unwrap_or_else(|| field.index.to_string())
+ .unwrap_or_else(|| field.declaration_index.to_string())
  })
  .collect::<Vec<String>>();
  let field_idents = reflect_struct
  .active_fields()
- .map(|field| ident_or_index(field.data.ident.as_ref(), field.index))
+ .map(|field| ident_or_index(field.data.ident.as_ref(), field.declaration_index))
  .collect::<Vec<_>>();
  let field_types = reflect_struct.active_types();
  let field_count = field_idents.len();

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

@@ -15,7 +15,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {
 
  let field_idents = reflect_struct
  .active_fields()
- .map(|field| Member::Unnamed(Index::from(field.index)))
+ .map(|field| Member::Unnamed(Index::from(field.declaration_index)))
  .collect::<Vec<_>>();
  let field_types = reflect_struct.active_types();
  let field_count = field_idents.len();

crates/bevy_reflect/src/lib.rs

@@ -751,6 +751,38 @@ mod tests {
  assert_eq!(Some(expected), my_struct);
  }
 
+ #[test]
+ fn from_reflect_should_allow_ignored_unnamed_fields() {
+ #[derive(Reflect, FromReflect, Eq, PartialEq, Debug)]
+ struct MyTupleStruct(i8, #[reflect(ignore)] i16, i32);
+
+ let expected = MyTupleStruct(1, 0, 3);
+
+ let mut dyn_tuple_struct = DynamicTupleStruct::default();
+ dyn_tuple_struct.insert(1_i8);
+ dyn_tuple_struct.insert(3_i32);
+ let my_tuple_struct = <MyTupleStruct as FromReflect>::from_reflect(&dyn_tuple_struct);
+
+ assert_eq!(Some(expected), my_tuple_struct);
+
+ #[derive(Reflect, FromReflect, Eq, PartialEq, Debug)]
+ enum MyEnum {
+ Tuple(i8, #[reflect(ignore)] i16, i32),
+ }
+
+ let expected = MyEnum::Tuple(1, 0, 3);
+
+ let mut dyn_tuple = DynamicTuple::default();
+ dyn_tuple.insert(1_i8);
+ dyn_tuple.insert(3_i32);
+
+ let mut dyn_enum = DynamicEnum::default();
+ dyn_enum.set_variant("Tuple", dyn_tuple);
+ let my_enum = <MyEnum as FromReflect>::from_reflect(&dyn_enum);
+
+ assert_eq!(Some(expected), my_enum);
+ }
+
  #[test]
  fn from_reflect_should_use_default_container_attribute() {
  #[derive(Reflect, FromReflect, Eq, PartialEq, Debug)]

crates/bevy_reflect/src/serde/mod.rs

@@ -12,7 +12,7 @@ mod tests {
  use crate::{
  serde::{ReflectSerializer, UntypedReflectDeserializer},
  type_registry::TypeRegistry,
- DynamicStruct, Reflect,
+ DynamicStruct, FromReflect, Reflect,
  };
  use serde::de::DeserializeSeed;
 
@@ -60,7 +60,7 @@ mod tests {
 
  #[test]
  fn test_serialization_tuple_struct() {
- #[derive(Debug, Reflect, PartialEq)]
+ #[derive(Debug, Reflect, FromReflect, PartialEq)]
  #[reflect(PartialEq)]
  struct TestStruct(
  i32,
@@ -87,9 +87,11 @@ mod tests {
  let value = reflect_deserializer.deserialize(&mut deserializer).unwrap();
  let deserialized = value.take::<DynamicTupleStruct>().unwrap();
 
- assert!(
- expected.reflect_partial_eq(&deserialized).unwrap(),
- "Expected {expected:?} found {deserialized:?}"
- );
+ println!("{:?}", deserialized);
+
+ let expected = TestStruct(3, 0, 0, 6);
+ let received = <TestStruct as FromReflect>::from_reflect(&deserialized).unwrap();
+
+ assert_eq!(expected, received);
  }
 }