60773e6787
# Objective Fixes #5101 Alternative to #6511 ## Solution Corrected the behavior for ignored fields in `FromReflect`, which was previously using the incorrect field indexes. Similarly, fields marked with `#[reflect(skip_serializing)]` no longer break when using `FromReflect` after deserialization. This was done by modifying `SerializationData` to store a function pointer that can later be used to generate a default instance of the skipped field during deserialization. The function pointer points to a function generated by the derive macro using the behavior designated by `#[reflect(default)]` (or just `Default` if none provided). The entire output of the macro is now wrapped in an [unnamed constant](https://doc.rust-lang.org/stable/reference/items/constant-items.html#unnamed-constant) which keeps this behavior hygienic. #### Rationale The biggest downside to this approach is that it requires fields marked `#[reflect(skip_serializing)]` to provide the ability to create a default instance— either via a `Default` impl or by specifying a custom one. While this isn't great, I think it might be justified by the fact that we really need to create this value when using `FromReflect` on a deserialized object. And we need to do this _during_ deserialization because after that (at least for tuples and tuple structs) we lose information about which field is which: _"is the value at index 1 in this `DynamicTupleStruct` the actual value for index 1 or is it really the value for index 2 since index 1 is skippable...?"_ #### Alternatives An alternative would be to store `Option<Box<dyn Reflect>>` within `DynamicTuple` and `DynamicTupleStruct` instead of just `Box<dyn Reflect>`. This would allow us to insert "empty"/"missing" fields during deserialization, thus saving the positional information of the skipped fields. However, this may require changing the API of `Tuple` and `TupleStruct` such that they can account for their dynamic counterparts returning `None` for a skipped field. In practice this would probably mean exposing the `Option`-ness of the dynamics onto implementors via methods like `Tuple::drain` or `TupleStruct::field`. Personally, I think requiring `Default` would be better than muddying up the API to account for these special cases. But I'm open to trying out this other approach if the community feels that it's better. --- ## Changelog ### Public Changes #### Fixed - The behaviors of `#[reflect(ignore)]` and `#[reflect(skip_serializing)]` are no longer dependent on field order #### Changed - Fields marked with `#[reflect(skip_serializing)]` now need to either implement `Default` or specify a custom default function using `#[reflect(default = "path::to::some_func")]` - Deserializing a type with fields marked `#[reflect(skip_serializing)]` will now include that field initialized to its specified default value - `SerializationData::new` now takes the new `SkippedField` struct along with the skipped field index - Renamed `SerializationData::is_ignored_field` to `SerializationData::is_field_skipped` #### Added - Added `SkippedField` struct - Added methods `SerializationData::generate_default` and `SerializationData::iter_skipped` ### Internal Changes #### Changed - Replaced `members_to_serialization_denylist` and `BitSet<u32>` with `SerializationDataDef` - The `Reflect` derive is more hygienic as it now outputs within an [unnamed constant](https://doc.rust-lang.org/stable/reference/items/constant-items.html#unnamed-constant) - `StructField::index` has been split up into `StructField::declaration_index` and `StructField::reflection_index` #### Removed - Removed `bitset` dependency ## Migration Guide * Fields marked `#[reflect(skip_serializing)]` now must implement `Default` or specify a custom default function with `#[reflect(default = "path::to::some_func")]` ```rust #[derive(Reflect)] struct MyStruct { #[reflect(skip_serializing)] #[reflect(default = "get_foo_default")] foo: Foo, // <- `Foo` does not impl `Default` so requires a custom function #[reflect(skip_serializing)] bar: Bar, // <- `Bar` impls `Default` } #[derive(Reflect)] struct Foo(i32); #[derive(Reflect, Default)] struct Bar(i32); fn get_foo_default() -> Foo { Foo(123) } ``` * `SerializationData::new` has been changed to expect an iterator of `(usize, SkippedField)` rather than one of just `usize` ```rust // BEFORE SerializationData::new([0, 3].into_iter()); // AFTER SerializationData::new([ (0, SkippedField::new(field_0_default_fn)), (3, SkippedField::new(field_3_default_fn)), ].into_iter()); ``` * `Serialization::is_ignored_field` has been renamed to `Serialization::is_field_skipped` * Fields marked `#[reflect(skip_serializing)]` are now included in deserialization output. This may affect logic that expected those fields to be absent.
92 lines
3.1 KiB
Rust
92 lines
3.1 KiB
Rust
use crate::derive_data::StructField;
|
|
use crate::field_attributes::{DefaultBehavior, ReflectIgnoreBehavior};
|
|
use bevy_macro_utils::fq_std::{FQBox, FQDefault};
|
|
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<ReflectionIndex, SkippedFieldDef>,
|
|
}
|
|
|
|
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<Option<Self>, 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, SkippedFieldDef { default_fn })| {
|
|
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 default function for this field.
|
|
///
|
|
/// This is of type `fn() -> Box<dyn Reflect>`.
|
|
default_fn: proc_macro2::TokenStream,
|
|
}
|
|
|
|
impl SkippedFieldDef {
|
|
pub fn new(field: &StructField<'_>) -> Result<Self, syn::Error> {
|
|
let ty = &field.data.ty;
|
|
|
|
let default_fn = match &field.attrs.default {
|
|
DefaultBehavior::Func(func) => quote! {
|
|
|| { #FQBox::new(#func()) }
|
|
},
|
|
_ => quote! {
|
|
|| { #FQBox::new(<#ty as #FQDefault>::default()) }
|
|
},
|
|
};
|
|
|
|
Ok(Self { default_fn })
|
|
}
|
|
}
|