From 6b292d426361c549a8cdaab8d75b83cdff1dafa2 Mon Sep 17 00:00:00 2001
From: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Date: Mon, 29 May 2023 08:29:29 -0700
Subject: [PATCH] bevy_reflect: Allow `#[reflect(default)]` on enum variant
fields (#8514)
# Objective
When using `FromReflect`, fields can be optionally left out if they are
marked with `#[reflect(default)]`. This is very handy for working with
serialized data as giant structs only need to list a subset of defined
fields in order to be constructed.
Example
Take the following struct:
```rust
#[derive(Reflect, FromReflect)]
struct Foo {
#[reflect(default)]
a: usize,
#[reflect(default)]
b: usize,
#[reflect(default)]
c: usize,
#[reflect(default)]
d: usize,
}
```
Since all the fields are default-able, we can successfully call
`FromReflect` on deserialized data like:
```rust
(
"foo::Foo": (
// Only set `b` and default the rest
b: 123
)
)
```
Unfortunately, this does not work with fields in enum variants. Marking
a variant field as `#[reflect(default)]` does nothing when calling
`FromReflect`.
## Solution
Allow enum variant fields to define a default value using
`#[reflect(default)]`.
### `#[reflect(Default)]`
One thing that structs and tuple structs can do is use their `Default`
implementation when calling `FromReflect`. Adding `#[reflect(Default)]`
to the struct or tuple struct both registers `ReflectDefault` and alters
the `FromReflect` implementation to use `Default` to generate any
missing fields.
This works well enough for structs and tuple structs, but for enums it's
not as simple. Since the `Default` implementation for an enum only
covers a single variant, it's not as intuitive as to what the behavior
will be. And (imo) it feels weird that we would be able to specify
default values in this way for one variant but not the others.
Because of this, I chose to not implement that behavior here. However,
I'm open to adding it in if anyone feels otherwise.
---
## Changelog
- Allow enum variant fields to define a default value using
`#[reflect(default)]`
---
.../bevy_reflect_derive/src/enum_utility.rs | 69 +++++++++++++------
.../bevy_reflect_derive/src/lib.rs | 3 +-
crates/bevy_reflect/src/lib.rs | 33 +++++++++
3 files changed, 83 insertions(+), 22 deletions(-)
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 a1a8fc8eb7..2a09055e20 100644
--- a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs
+++ b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs
@@ -1,10 +1,13 @@
-use crate::fq_std::FQDefault;
+use crate::derive_data::StructField;
+use crate::field_attributes::DefaultBehavior;
+use crate::fq_std::{FQDefault, FQOption};
use crate::{
derive_data::{EnumVariantFields, ReflectEnum},
utility::ident_or_index,
};
use proc_macro2::Ident;
use quote::{quote, ToTokens};
+use syn::Member;
/// Contains all data needed to construct all variants within an enum.
pub(crate) struct EnumVariantConstructors {
@@ -30,7 +33,7 @@ pub(crate) fn get_variant_constructors(
let name = ident.to_string();
let variant_constructor = reflect_enum.get_unit(ident);
- let fields = match &variant.fields {
+ let fields: &[StructField] = match &variant.fields {
EnumVariantFields::Unit => &[],
EnumVariantFields::Named(fields) | EnumVariantFields::Unnamed(fields) => {
fields.as_slice()
@@ -39,35 +42,59 @@ pub(crate) fn get_variant_constructors(
let mut reflect_index: usize = 0;
let constructor_fields = fields.iter().enumerate().map(|(declare_index, field)| {
let field_ident = ident_or_index(field.data.ident.as_ref(), declare_index);
+ let field_ty = &field.data.ty;
+
let field_value = if field.attrs.ignore.is_ignored() {
- quote! { #FQDefault::default() }
+ match &field.attrs.default {
+ DefaultBehavior::Func(path) => quote! { #path() },
+ _ => quote! { #FQDefault::default() }
+ }
} else {
- let error_repr = field.data.ident.as_ref().map_or_else(
- || format!("at index {reflect_index}"),
- |name| format!("`{name}`"),
- );
- let unwrapper = if can_panic {
- let type_err_message = format!(
- "the field {error_repr} should be of type `{}`",
- field.data.ty.to_token_stream()
- );
- quote!(.expect(#type_err_message))
+ let (resolve_error, resolve_missing) = if can_panic {
+ let field_ref_str = match &field_ident {
+ Member::Named(ident) => format!("the field `{ident}`"),
+ Member::Unnamed(index) => format!("the field at index {}", index.index)
+ };
+ let ty = field.data.ty.to_token_stream();
+
+ let on_error = format!("{field_ref_str} should be of type `{ty}`");
+ let on_missing = format!("{field_ref_str} is required but could not be found");
+
+ (quote!(.expect(#on_error)), quote!(.expect(#on_missing)))
} else {
- quote!(?)
+ (quote!(?), quote!(?))
};
+
let field_accessor = match &field.data.ident {
Some(ident) => {
let name = ident.to_string();
- quote!(.field(#name))
+ quote!(#ref_value.field(#name))
}
- None => quote!(.field_at(#reflect_index)),
+ None => quote!(#ref_value.field_at(#reflect_index)),
};
reflect_index += 1;
- let missing_field_err_message = format!("the field {error_repr} was not declared");
- let accessor = quote!(#field_accessor .expect(#missing_field_err_message));
- quote! {
- #bevy_reflect_path::FromReflect::from_reflect(#ref_value #accessor)
- #unwrapper
+
+ match &field.attrs.default {
+ DefaultBehavior::Func(path) => quote! {
+ if let #FQOption::Some(field) = #field_accessor {
+ <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(field)
+ #resolve_error
+ } else {
+ #path()
+ }
+ },
+ DefaultBehavior::Default => quote! {
+ if let #FQOption::Some(field) = #field_accessor {
+ <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(field)
+ #resolve_error
+ } else {
+ #FQDefault::default()
+ }
+ },
+ DefaultBehavior::Required => quote! {
+ <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(#field_accessor #resolve_missing)
+ #resolve_error
+ },
}
};
quote! { #field_ident : #field_value }
diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs
index 086441a311..f7f2859d10 100644
--- a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs
+++ b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs
@@ -87,7 +87,8 @@ pub(crate) static REFLECT_VALUE_ATTRIBUTE_NAME: &str = "reflect_value";
/// to improve performance and/or robustness.
/// An example of where this is used is in the [`FromReflect`] derive macro,
/// where adding this attribute will cause the `FromReflect` implementation to create
-/// a base value using its [`Default`] implementation avoiding issues with ignored fields.
+/// a base value using its [`Default`] implementation avoiding issues with ignored fields
+/// (for structs and tuple structs only).
///
/// ## `#[reflect_value]`
///
diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs
index 4e03f6c12e..40a20a40fe 100644
--- a/crates/bevy_reflect/src/lib.rs
+++ b/crates/bevy_reflect/src/lib.rs
@@ -751,6 +751,39 @@ mod tests {
assert_eq!(Some(expected), my_struct);
}
+ #[test]
+ fn from_reflect_should_use_default_variant_field_attributes() {
+ #[derive(Reflect, FromReflect, Eq, PartialEq, Debug)]
+ enum MyEnum {
+ Foo(#[reflect(default)] String),
+ Bar {
+ #[reflect(default = "get_baz_default")]
+ #[reflect(ignore)]
+ baz: usize,
+ },
+ }
+
+ fn get_baz_default() -> usize {
+ 123
+ }
+
+ let expected = MyEnum::Foo(String::default());
+
+ let dyn_enum = DynamicEnum::new("Foo", DynamicTuple::default());
+ let my_enum = ::from_reflect(&dyn_enum);
+
+ assert_eq!(Some(expected), my_enum);
+
+ let expected = MyEnum::Bar {
+ baz: get_baz_default(),
+ };
+
+ let dyn_enum = DynamicEnum::new("Bar", DynamicStruct::default());
+ let my_enum = ::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)]