[Merged by Bors] - bevy_reflect: Reflect enums

[MrGVSV]

Objective

This is a revival of #1347. Credit for the original PR should go to @Davier.

Currently, enums are treated as ReflectRef::Value types by bevy_reflect. Obviously, there needs to be better a better representation for enums using the reflection API.

Solution

Based on prior work from @Davier, an Enum trait has been added as well as the ability to automatically implement it via the Reflect derive macro. This allows enums to be expressed dynamically:

#[derive(Reflect)]
enum Foo {
  A,
  B(usize),
  C { value: f32 },
}

let mut foo = Foo::B(123);
assert_eq!("B", foo.variant_name());
assert_eq!(1, foo.field_len());

let new_value = DynamicEnum::from(Foo::C { value: 1.23 });
foo.apply(&new_value);
assert_eq!(Foo::C{value: 1.23}, foo);

Features

Derive Macro

Use the #[derive(Reflect)] macro to automatically implement the Enum trait for enum definitions. Optionally, you can use #[reflect(ignore)] with both variants and variant fields, just like you can with structs. These ignored items will not be considered as part of the reflection and cannot be accessed via reflection.

#[derive(Reflect)]
enum TestEnum {
  A,
  // Uncomment to ignore all of `B`
  // #[reflect(ignore)]
  B(usize),
  C {
    // Uncomment to ignore only field `foo` of `C`
    // #[reflect(ignore)]
    foo: f32,
    bar: bool,
  },
}

Dynamic Enums

Enums may be created/represented dynamically via the DynamicEnum struct. The main purpose of this struct is to allow enums to be deserialized into a partial state and to allow dynamic patching. In order to ensure conversion from a DynamicEnum to a concrete enum type goes smoothly, be sure to add FromReflect to your derive macro.

let mut value = TestEnum::A;

// Create from a concrete instance
let dyn_enum = DynamicEnum::from(TestEnum::B(123));

value.apply(&dyn_enum);
assert_eq!(TestEnum::B(123), value);

// Create a purely dynamic instance
let dyn_enum = DynamicEnum::new("TestEnum", "A", ());

value.apply(&dyn_enum);
assert_eq!(TestEnum::A, value);

Variants

An enum value is always represented as one of its variants— never the enum in its entirety.

let value = TestEnum::A;
assert_eq!("A", value.variant_name());

// Since we are using the `A` variant, we cannot also be the `B` variant
assert_ne!("B", value.variant_name());

All variant types are representable within the Enum trait: unit, struct, and tuple.

You can get the current type like:

match value.variant_type() {
  VariantType::Unit => println!("A unit variant!"),
  VariantType::Struct => println!("A struct variant!"),
  VariantType::Tuple => println!("A tuple variant!"),
}

Notice that they don't contain any values representing the fields. These are purely tags.

If a variant has them, you can access the fields as well:

let mut value = TestEnum::C {
  foo: 1.23,
  bar: false
};

// Read/write specific fields
*value.field_mut("bar").unwrap() = true;

// Iterate over the entire collection of fields
for field in value.iter_fields() {
  println!("{} = {:?}", field.name(), field.value());
}

Variant Swapping

It might seem odd to group all variant types under a single trait (why allow iter_fields on a unit variant?), but the reason this was done is to easily allow variant swapping. As I was recently drafting up the Design Decisions section, I discovered that other solutions could have been made to work with variant swapping. So while there are reasons to keep the all-in-one approach, variant swapping is not one of them.

let mut value: Box<dyn Enum> = Box::new(TestEnum::A);
value.set(Box::new(TestEnum::B(123))).unwrap();

Serialization

Enums can be serialized and deserialized via reflection without needing to implement Serialize or Deserialize themselves (which can save thousands of lines of generated code). Below are the ways an enum can be serialized.

Note, like the rest of reflection-based serialization, the order of the keys in these representations is important!

Unit

{
  "type": "my_crate::TestEnum",
  "enum": {
    "variant": "A"
  }
}

Tuple

{
  "type": "my_crate::TestEnum",
  "enum": {
    "variant": "B",
    "tuple": [
      {
        "type": "usize",
        "value": 123
      }
    ]
  }
}

Effects on Option

This ends up making Option look a little ugly:

{
  "type": "core::option::Option<usize>",
  "enum": {
    "variant": "Some",
    "tuple": [
      {
        "type": "usize",
        "value": 123
      }
    ]
  }
}

Struct

{
  "type": "my_crate::TestEnum",
  "enum": {
    "variant": "C",
    "struct": {
      "foo": {
        "type": "f32",
        "value": 1.23
      },
      "bar": {
        "type": "bool",
        "value": false
      }
    }
  }
}

Design Decisions

View Section

This section is here to provide some context for why certain decisions were made for this PR, alternatives that could have been used instead, and what could be improved upon in the future.

Variant Representation

One of the biggest decisions was to decide on how to represent variants. The current design uses a "all-in-one" design where unit, tuple, and struct variants are all simultaneously represented by the Enum trait. This is not the only way it could have been done, though.

Alternatives

1. Variant Traits

One way of representing variants would be to define traits for each variant, implementing them whenever an enum featured at least one instance of them. This would allow us to define variants like:

pub trait Enum: Reflect {
  fn variant(&self) -> Variant;
}

pub enum Variant<'a> {
    Unit,
    Tuple(&'a dyn TupleVariant),
    Struct(&'a dyn StructVariant),
}

pub trait TupleVariant {
  fn field_len(&self) -> usize;
  // ...
}

And then do things like:

fn get_tuple_len(foo: &dyn Enum) -> usize {
  match foo.variant() {
    Variant::Tuple(tuple) => tuple.field_len(),
    _ => panic!("not a tuple variant!")
  }
}

The reason this PR does not go with this approach is because of the fact that variants are not separate types. In other words, we cannot implement traits on specific variants— these cover the entire enum. This means we offer an easy footgun:

let foo: Option<i32> = None;
let my_enum = Box::new(foo) as Box<dyn TupleVariant>;

Here, my_enum contains foo, which is a unit variant. However, since we need to implement TupleVariant for Option as a whole, it's possible to perform such a cast. This is obviously wrong, but could easily go unnoticed. So unfortunately, this makes it not a good candidate for representing variants.

2. Variant Structs

To get around the issue of traits necessarily needing to apply to both the enum and its variants, we could instead use structs that are created on a per-variant basis. This was also considered but was ultimately [removed](71d27ab) due to concerns about allocations.

Each variant struct would probably look something like:

pub trait Enum: Reflect {
  fn variant_mut(&self) -> VariantMut;
}

pub enum VariantMut<'a> {
    Unit,
    Tuple(TupleVariantMut),
    Struct(StructVariantMut),
}

struct StructVariantMut<'a> {
  fields: Vec<&'a mut dyn Reflect>,
  field_indices: HashMap<Cow<'static, str>, usize>
}

This allows us to isolate struct variants into their own defined struct and define methods specifically for their use. It also prevents users from casting to it since it's not a trait. However, this is not an optimal solution. Both field_indices and fields will require an allocation (remember, a Box<[T]> still requires a Vec<T> in order to be constructed). This might be a problem if called frequently enough.

3. Generated Structs

The original design, implemented by @Davier, instead generates structs specific for each variant. So if we had a variant path like Foo::Bar, we'd generate a struct named FooBarWrapper. This would be newtyped around the original enum and forward tuple or struct methods to the enum with the chosen variant.

Because it involved using the Tuple and Struct traits (which are also both bound on Reflect), this meant a bit more code had to be generated. For a single struct variant with one field, the generated code amounted to ~110LoC. However, each new field added to that variant only added ~6 more LoC.

In order to work properly, the enum had to be transmuted to the generated struct:

fn variant(&self) -> crate::EnumVariant<'_> {
  match self {
    Foo::Bar {value: i32} => {
      let wrapper_ref = unsafe { 
        std::mem::transmute::<&Self, &FooBarWrapper>(self) 
      };
      crate::EnumVariant::Struct(wrapper_ref as &dyn crate::Struct)
    }
  }
}

This works because FooBarWrapper is defined as repr(transparent).

Out of all the alternatives, this would probably be the one most likely to be used again in the future. The reasons for why this PR did not continue to use it was because:

• To reduce generated code (which would hopefully speed up compile times)
• To avoid cluttering the code with generated structs not visible to the user
• To keep bevy_reflect simple and extensible (these generated structs act as proxies and might not play well with current or future systems)
• To avoid additional unsafe blocks
• My own misunderstanding of @Davier's code

That last point is obviously on me. I misjudged the code to be too unsafe and unable to handle variant swapping (which it probably could) when I was rebasing it. Looking over it again when writing up this whole section, I see that it was actually a pretty clever way of handling variant representation.

Benefits of All-in-One

As stated before, the current implementation uses an all-in-one approach. All variants are capable of containing fields as far as Enum is concerned. This provides a few benefits that the alternatives do not (reduced indirection, safer code, etc.).

The biggest benefit, though, is direct field access. Rather than forcing users to have to go through pattern matching, we grant direct access to the fields contained by the current variant. The reason we can do this is because all of the pattern matching happens internally. Getting the field at index 2 will automatically return Some(...) for the current variant if it has a field at that index or None if it doesn't (or can't).

This could be useful for scenarios where the variant has already been verified or just set/swapped (or even where the type of variant doesn't matter):

let dyn_enum: &mut dyn Enum = &mut Foo::Bar {value: 123};
// We know it's the `Bar` variant
let field = dyn_enum.field("value").unwrap();

Reflection is not a type-safe abstraction— almost every return value is wrapped in Option<...>. There are plenty of places to check and recheck that a value is what Reflect says it is. Forcing users to have to go through match each time they want to access a field might just be an extra step among dozens of other verification processes.

Some might disagree, but ultimately, my view is that the benefit here is an improvement to the ergonomics and usability of reflected enums.

---

Changelog

Added

• Added Enum trait

• Added Enum impl to Reflect derive macro

• Added DynamicEnum struct

  • Added DynamicVariant

• Added EnumInfo

  • Added VariantInfo
    • Added StructVariantInfo
    • Added TupleVariantInfo
    • Added UnitVariantInfo

• Added serializtion/deserialization support for enums

  • Added EnumSerializer

• Added VariantType

• Added VariantFieldIter

• Added VariantField

• Added enum_partial_eq(...)

• Added enum_hash(...)

Changed

• Option<T> now implements Enum
• bevy_window now depends on bevy_reflect
  • Implemented Reflect and FromReflect for WindowId
• Derive FromReflect on PerspectiveProjection
• Derive FromReflect on OrthographicProjection
• Derive FromReflect on WindowOrigin
• Derive FromReflect on ScalingMode
• Derive FromReflect on DepthCalculation

Migration Guide

• Enums no longer need to be treated as values and usages of #[reflect_value(...)] can be removed or replaced by #[reflect(...)]
• Enums (including Option<T>) now take a different format when serializing. The format is described above, but this may cause issues for existing scenes that make use of enums.

---

Also shout out to @nicopap for helping clean up some of the code here! It's a big feature so help like this is really appreciated!

[MrGVSV]

This is the main change to these impl modules: using ReflectStruct in place of ReflectDeriveData (or similar struct).

[PROMETHIA-27]

Wow, hell of a lot to review, definitely need more eyes on this. But nothing looked obviously wrong, and there's a very nice amount of tests for everything. Looks good to me!

[PROMETHIA-27]

Should we make a macro similar to impl_reflect_struct to impl enums? I could do that in a separate PR if that's preferable.

[MrGVSV]

Yeah that was on my todo list originally but I decided it could be done separately (since Option<T> appears to be the only case). If you wanna write a follow up that would be great!

[MrGVSV]

Wow, hell of a lot to review, definitely need more eyes on this.

Yeah a big part of this is because I decided to split up the impls code into their own modules. That and all the tests (on top of the actual changes) I think really add up. 😵‍💫😅

[5tr1k3r]

I want this :)

[MrGVSV]

I want this :)

Feel free to leave review! Even one or two review comments can be useful (even if they're just questions/remarks lol)

[nicopap]

I only skimmed the implementation on ser/de and didn't look at the code in impls/{structs,tuple_structs,typed,values} since they seem to be the split of what bevy_reflect_derive/src/impls.rs used to be.

There is a few code quality improvements possible on the derive macros, I'll open a PR for them.

Also, I don't remember seeing tests for #[reflect(ignore)].

I think the enum API is great, it has everything you'd expect in line with the other reflect subtraits, and the EnumInfo and VariantInfo struct are really good. I wish there wasn't so much bookkeeping required for the field indices, but there is no way around it.

[nicopap]

Suggested change

	if let Some(false) | None = field_value.reflect_partial_eq(field.value()) {
	// Fields failed comparison
	return Some(false);
	let eq_result = field_value.reflect_partial_eq(field.value());
	if let failed @ (Some(false) | None) = eq_result {
	return failed;
	}

Nit: I feel this is more correct, since if a field doesn't implement reflect_partial_eq, it should be reflected in the enum's implementation.

[MrGVSV]

Yeah that makes sense, but it doesn't seem the other reflect_partial_eq do that:

bevy/crates/bevy_reflect/src/struct_trait.rs

Lines 451 to 453 in 86dd6f0

	if let Some(false) | None = field_value.reflect_partial_eq(value) {
	return Some(false);
	}

So we should probably address that in a separate PR.

[alice-i-cecile]

Can you spin this out into an issue so we don't forget?

[MrGVSV]

Created #5204

[MrGVSV]

I only skimmed the implementation on ser/de and didn't look at the code in impls/{structs,tuple_structs,typed,values} since they seem to be the split of what bevy_reflect_derive/src/impls.rs used to be.

Yeah that's more-or-less what it is. The only real difference is in the parameter. We now take things like ReflectStruct instead of ReflectDeriveData. This was done because I had to split ReflectDeriveData up to support enums.

There is a few code quality improvements possible on the derive macros, I'll open a PR for them.

Awesome. I appreciate it!

Also, I don't remember seeing tests for #[reflect(ignore)].

They're in the src/enums/mod.rs file, I believe.

I think the enum API is great, it has everything you'd expect in line with the other reflect subtraits, and the EnumInfo and VariantInfo struct are really good. I wish there wasn't so much bookkeeping required for the field indices, but there is no way around it.

Yeah, there seemed to always be a tradeoff. But I think this was the simplest way for the time being.

[nicopap]

I've opened the PR. See MrGVSV#1

Question: Should we aim to merge this before bevyengine/rfcs#56? Seems to me there will be a lot of duplicate work if we do not, because of the split f impls.rs.

Also needs to pull to HEAD.

[nicopap]

They're in the src/enums/mod.rs file, I believe.

Great! I did miss them. Pardon me for overlooking them.

[nicopap]

Some suggested changes MrGVSV#1, as per prevision discussion

[alice-i-cecile]

@MrGVSV once you fix up these merge conflicts I think this is good to go!

[alice-i-cecile]

I've done a full pass, and only have very small suggestions. We should consider doing a more thorough pass on "reflect everything" once this is in. IMO that should wait until after this lands to reduce merge conflicts though.

[alice-i-cecile]

Can you spin this out into an issue so we don't forget?

[cart]

Very nicely done. I have a couple of comments about allocations / redundant work. But I have no notes on the general implementation and design. I think you made the right calls in every case.

[cart]

Not something I feel inclined to block on, but we might want to add convenience field and field_at functions here, given that the Enum trait provides this pattern (much like Struct and StructInfo).

[MrGVSV]

Good idea, I'll make a note of it and add it in a followup PR!

[cart]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-and-install-on-iOS
• build-android
• build (macos-latest)
• build (ubuntu-latest)
• build-wasm
• build (windows-latest)
• check-compiles
• check-doc
• check-missing-examples-in-docs
• ci
• markdownlint
• run-examples
• run-examples-on-wasm
• run-examples-on-windows-dx12