bevy_reflect: Require type registrations
- Dominant language
- Rust
- Stars
- 48.2k
- Forks
- 4.8k
- Avg merge
- 3d 16h
- Merged PRs (30d)
- 171
Description
## What problem does this solve or what need does it fill?
Currently, (de)serializing via reflection can be done without registering _most_ types. The only types that need to be registered for (de)serialization to work are non-Value types (i.e. not `ReflectRef::Value`) and types with specific (de)serialization type data (i.e. `ReflectSerialize`, `ReflectDeserialize`, or `SerializationData`).
This means we can readily serialize this:
```rust
#[derive(Reflect)]
struct Foo {
bar: usize
}
```
But this needs to be registered first:
```rust
#[derive(Reflect, Serialize)]
#[reflect(Serialize)]
struct Foo {
#[serde(rename = "baz")]
bar: usize
}
```
The interesting thing is, if we fail to register the second one, we still get an output:
```ron
{
"my_crate::Foo": (
bar: 123,
),
}
```
It worked.... just not in the way we intended. And what's worse is we don't get any warning or error indicating that we forgot to register our type.
> The example above concerns (de)serialization, but this whole issue could extend beyond (de)serialization and affect future reflection APIs (both first and third party).
## What solution would you like?
Make it so that type registrations are required for reflected (de)serialization.
This might be slightly annoying (though, could be aided by something like #4154), but it ensures users never forget to register a type that actually needs it.
Additionally, as the API grows, we may find the type registry to be more and more necessary for accomplishing certain things. This would also ensure types are properly setup for any future use cases as well (this goes for third-party crates too!).
## What alternative(s) have you considered?
##### Printing a warning when a type is missing a registration
This won't really work because we have no way of knowing if a type needs to be registered or not. We _could_ check by calling `GetTypeRegistration::get_type_registration` for every value serialized, but this is not very realistic.
And since we can't know what really requires the registration, we'd end up printing a warning for _everything_ that isn't registered— even if it doesn't have to be. And this imo indicates to the user that everything still needs to be registered, which defeats the whole purpose of this.
##### Keep the silent failures
We could, of course, just do nothing and allow these silent failures. With proper documentation, we could help users be more aware of this issue. However, this doesn't really fix the problem, just raise awareness about it.
Contributor guide
Research direction
Start by tracing the bevy_reflect serialization and type-registration paths described in the issue, including how ReflectRef::Value and serialization type data are handled. Define the registration behavior for reflected (de)serialization and verify that unregistered types can no longer serialize silently, including the Foo examples from the issue.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100