Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bevy_reflect: Allow #[reflect(default)] on enum variant fields #8514

Merged

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented Apr 30, 2023

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:

#[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:

(
  "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)]

@MrGVSV MrGVSV added C-Bug An unexpected or incorrect behavior A-Reflection Runtime information about types labels Apr 30, 2023
@MrGVSV MrGVSV force-pushed the reflect-variant-field-defaults branch from 2081e07 to a16a9a7 Compare May 23, 2023 04:21
@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 23, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 29, 2023
Merged via the queue into bevyengine:main with commit 6b292d4 May 29, 2023
@MrGVSV MrGVSV deleted the reflect-variant-field-defaults branch May 29, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

5 participants