TypePath required for generic #[reflect(ignore)] field

[paul-hansen]

Bevy version

Bevy 0.11

What you did

I'm working on a PR to update leafwing-input-manager: Leafwing-Studios/leafwing-input-manager#364

What went wrong

Leafwing input manager has a struct ActionState that takes a generic that is used on a #[reflect(ignore)] PhantomData field:

#[derive(Resource, Component, Clone, Debug, PartialEq, Serialize, Deserialize, Reflect)]
pub struct ActionState<A: Actionlike> {
    /// The [`ActionData`] of each action
    ///
    /// The position in this vector corresponds to [`Actionlike::index`].
    action_data: Vec<ActionData>,
    #[reflect(ignore)]
    _phantom: PhantomData<A>,
}

We are getting an error when registering this type with bevy saying our generic type must implement TypePath.
This seems potentially limiting to the end users of LWIM and I'm not sure it was intended as it isn't mentioned in migration guides.
Is this restriction something that could be lifted?

Additional info

There's a conversation about this on the LWIM PR starting here:
Leafwing-Studios/leafwing-input-manager#364 (comment)

To try this out, you can checkout the branch for that PR and remove the TypePath requirement I added from this line:
https://github.com/Leafwing-Studios/leafwing-input-manager/blob/7fb55b001a4aa949b1863dd2e0007d9512960625/src/lib.rs#L81

Here's the line and error we are getting:
https://github.com/Leafwing-Studios/leafwing-input-manager/blob/dfbe44b22fae01434737fb498dfb10d5ca20057d/src/plugin.rs#L146

        app.register_type::<ActionState<A>>()

error[E0277]: the trait bound `A: TypePath` is not satisfied
   --> src\plugin.rs:146:29
    |
146 |         app.register_type::<ActionState<A>>()
    |                             ^^^^^^^^^^^^^^ the trait `TypePath` is not implemented for `A`
    |
note: required for `ActionState<A>` to implement `GetTypeRegistration`
   --> src\action_state.rs:85:80
    |
85  | #[derive(Resource, Component, Clone, Debug, PartialEq, Serialize, Deserialize, Reflect)]
    |                                                                                ^^^^^^^ unsatisfied trait bound introduced in this `derive` macro
86  | pub struct ActionState<A: Actionlike> {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^
note: required by a bound in `App::register_type`
   --> D:\Packages\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_app-0.11.0\src\app.rs:816:29
    |
816 |     pub fn register_type<T: bevy_reflect::GetTypeRegistration>(&mut self) -> &mut Self {
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `App::register_type`
    = note: this error originates in the derive macro `Reflect` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider further restricting this bound
    |
82  | impl<A: Actionlike + bevy::reflect::TypePath> Plugin for InputManagerPlugin<A> {
    |                    +++++++++++++++++++++++++

[MrGVSV]

Unfortunately, this cannot be lifted. The new, stable type paths require that all generics also implement TypePath. This is so that we can effectively create the necessary TypePath impl for the type. Without it, serialization and other type-name-specific workflows are not guaranteed to be stable between builds.

The solution would probably be for LWIM to add Actionlike: TypePath or to just ensure that whatever A is passed implements TypePath.

[paul-hansen]

That's what I figured, thought I'd ask though just in case. Thanks for the confirmation! I doubt this will be an issue for any LWIM users since they will likely just be using their own enum, they can just derive reflect on if they haven't already.

Hopefully cases where crates don't own either type can find a work around, I'd guess make a wrapper struct and manually implement TypePath? Is this something worth mentioning in the migration guide? https://bevyengine.org/learn/migration-guides/0.10-0.11/#bevy-reflect-stable-type-path-v2

[paul-hansen]

Created a PR for the migration guide, don't think there's anything else to do here so I'll close this. I welcome feedback on that PR: bevyengine/bevy-website#698

[MrGVSV]

I'm actually going to re-open this issue because I think we may want to consider adding alternative options for when types cannot possibly implement TypePath, as in the case with bevy_rand.

For such scenarios, we have a couple options:

1. Require these types to take in wrappers over other types

   #[derive(Reflect)]    
   struct Wrapper(external_crate::NotReflect);
   
   #[derive(Reflect)]
   struct MyType<T>(T);
   
   app.register_type::<MyType<Wrapper>>();

2. Add an opt-out attribute like #[type_path(unstable)] which implements all of the TypePath methods using std::any::type_name
3. Add an attribute to allow individual type parameters to be excluded from the type path: #[type_path(ignore_params(T))]

1 is annoying but currently possible.

I think 2 is feasible, but would require more work to properly generate the TypePath::short_name impl. I think it also makes sense to use for types that really don't need a stable type path (i.e. there's no expectation they will be the same between builds).

I personally don't think 3 is the way to go, but it's an option.

[Bluefinger]

Option 3 won't really work for types that are meant to be generic and not clash, so that information has to be present somehow. Option 2 seems like the 'safest' bet, but as long as it can be 'contained' to just the part that has been opted out. I don't know whether for the case of bevy_rand that just keeping the opt-out on the generic parameter would suffice for a 'good enough' type path that is mostly stable (or however stable we can guarantee it).

So, just how unstable is std::any::type_name that it can't be used reliably enough? I think that's a bit I want to at least understand so I can better decide what I need to do for bevy_rand or otherwise.