bevy_reflect: Type aliases

[MrGVSV]

Objective

Reflection relies on type names to access type registrations. Unfortunately, these type names can be (1) rather long, (2) unstable between a crate's features/versions, and (3) very programmer-focused.

This can be problematic for serialized formats such as Bevy's scenes. Something as innocuous as performing an internal refactor can result in downstream users needing to update all of their scenes. For example, moving my_crate::foo::Foo to my_crate::Foo requires updating the type name everywhere it's used.

Additionally, it's not always programmers that are dealing with this data. Designers may find it difficult to understand that my_crate::inventory::utility::BaseContainer<my_crate::inventory::Item, 10> really just represents PlayerHotbar. In fact, the programmers may have even shortened this themselves using type PlayerHotbar = ... (not fair!).

Ideally there would be a way to refer to types that were shorter, more stable, and non-programmer friendly.

Solution

Add the ability to register aliases for a type.

Aliases are designated using an attribute on the derive macro for Reflect:

#[derive(Reflect)]
#[reflect(alias = "MyCoolAlias")]
#[reflect(alias = "MyOtherCoolAlias")]
struct Foo;

As seen above, we can define multiple aliases by just adding another attribute!

With this, we can now change our scenes from this:

{
  "type": "my_crate::foo::Foo",
  // ...
}

to:

{
  "type": "MyCoolAlias",
  // ...
}

Alternatively, we could have assigned the aliases via the registry:

registry.register_alias::<Foo>("MyCoolAlias");
registry.register_alias::<Foo>("MyOtherCoolAlias");

Generics

Unfortunately, we cannot so easily handle generic types. The reason for this is that a type with generics will monomorphize to many types— so which type gets the alias? Because aliases map to a single type, we cannot specify them via the macro.

This means that registering generic types requires using the registry:

registry.register_alias::<BaseContainer<Item, 10>>("PlayerHotbar");

Deprecation Warnings

As users and crates in the ecosystem start to use aliases, they may at some point decide they want to move from one alias to a new one. We can aid user migration by allowing some aliases to be marked as "deprecated". This means that a user can continue to use a deprecated alias as normal; however, it will print a warning to the console letting them know that the alias may be removed entirely in the future.

To specify a deprecated alias:

#[derive(Reflect)]
#[reflect(alias = "MyNewAlias")]
#[reflect(deprecated_alias = "MyOldAlias")]
struct Foo;

Note: This works in theory, but due to how the deserializer is setup, we only actually warn when deserializing Value types. This should be more useful when something like #4561 or #5723 lands.

Comparison to #5805

Both this PR and #5805 are very similar. However, they have slightly different goals and actually pair well together.

#5805 is concerned with defining a replacement for std::any::type_name. It is supposed to provide a stable and (generally) unique identifier for a given type. There can only be one for a given type.

This PR is concerned with defining multiple names that can optionally be used to refer to a type. These names can be overwritten by users and other crates. It gives users more control over how their type can be represented.

In other words, #5805 defines the "one true" type name, while this PR defines "many optional" type names. Or, to use a serde analogy, #5805 is #[serde(rename)], while this PR is #[serde(alias)].

---

Task List

Feel free to leave comments regarding the items on this list:

• Allow defining deprecated aliases (separate methods or boolean argument?)
• Add warning to deserialization (should we error on unregistered type?)
• Add methods to app (which ones? are they worth having?)

Changelog

TODO

Migration Guide

TODO

[afonsolage]

Allow defining deprecated aliases (separate methods or boolean argument?)

Separated method is more clearer, since deprecation isn't done very often.

Add warning to deserialization (should we error on unregistered type?)

IMO it should be an error, not a warning, since the behavior of deserialized object will be compromised.

Add methods to app (which ones? are they worth having?)

Yes, since usually crate devs register events, assets, reflect types and anything they need on Plugin, so it would be a good idea to keep consistency.

[MrGVSV]

Separated method is more clearer, since deprecation isn't done very often.

Yeah I think you're right. I'll make them a separate set of methods.

IMO it should be an error, not a warning, since the behavior of deserialized object will be compromised.

Well, the warning would only be for usages of deprecated aliases. We don't want to error on those as it allows users to migrate across versions. As discussed on Discord, though, I think I will just include the warning and consider making the error a separate PR/issue.

Yes, since usually crate devs register events, assets, reflect types and anything they need on Plugin, so it would be a good idea to keep consistency.

Makes sense. Should all three methods (register_alias, try_register_alias, and overwrite_alias) be added?

[PROMETHIA-27]

Regarding the task list:

• Deprecated aliases should be added with a separate method, to make it more explicit, split up the docs between it and normal registration, and make it more easily searchable during refactors
• Warnings/errors are unnecessary for this PR and can be added in a follow-up
• App should have a mirror of every method, because it will be common to manually add aliases and having to get the resource manually to do that would be unnecessarily boilerplate-y. At the very least generics will need to be manually added and that alone justifies having the mirrors.

[MrGVSV]

Warnings/errors are unnecessary for this PR and can be added in a follow-up

So you're suggesting just not including any warnings when deserializing deprecated aliases? In this PR at least?

[PROMETHIA-27]

Deprecated ones probably should have a warning, as those are an entirely new feature that this PR is adding, but anything beyond that I don't think is necessary. Like detecting unregistered names.

[afonsolage]

Makes sense. Should all three methods (register_alias, try_register_alias, and overwrite_alias) be added?

Since there will be cases when the user explicitly would like to override some alias, there is room for those three methods. At beginning I was not fan of overwrite_alias and I was about to suggest register_alias to return Option<T> with previous alias, if there was one, much like HashMap::insert does. Then I realized this would be very misleading.

I imagine the most common workflow using those methods would be:

1. Register an alias using register_alias;
2. If there is a warning, replace with overwrite_alias;

Which made me think if we should add the info to use overwrite_alias instead of register_alias on the warning message that is shown by register_alias method, in case user wanna suppress the warning.

[MrGVSV]

Which made me think if we should add the info to use overwrite_alias instead of register_alias on the warning message that is shown by register_alias method, in case user wanna suppress the warning.

Ooh yeah that's a good idea!

[MrGVSV]

Update: I'm currently waiting on #5805 to land so I can base some of the changes around whatever solution ends up there. I also just think we should have stable type names figured out before moving on to things like type aliases.

Note: This PR isn't strictly blocked by #5805. I just think that PR should be released before this one to ensure we make the right/better decisions with this one.

[richchurcher]

Backlog cleanup: closing in favour of linked PR #12387, while having no particular opinion on which approach is better! Both can be considered by reviewers.