Represent user inputs via traits, rather than a large enum

[alice-i-cecile]

The exhaustive approach is lovely, but a) leads to some very tricky nesting b) isn't user extensible c) results in excessive memory use, as every input requires the maximum amount of space.

In addition:

          I think this will be obsolete if we use traits instead of enums to represent the user input, which is probably the better move in the long run.

Originally posted by @TimJentzsch in #241 (comment)

[TimJentzsch]

This could also open the way for some changes to the user-facing API, for example we could implement the trait for tuples of user inputs to represent the chords, e.g. (KeyCode::Shift, KeyCode::Alt, KeyCode::M)

[paul-hansen]

Would just start changing UserInput to a trait and see what we run into be the next step for this? Anything else I should know or we need to think about first? I should have some time to dig in on this starting this weekend.

[alice-i-cecile]

I think that we probably want to create three traits:

• Inputlike
• Buttonlike: Inputlike
• Axislike: Inputlike

Buttons and axes have both shared and distinct methods, so I think this is a good hierarchy.

But yeah, that was about my plan!

[paul-hansen]

So I ran into pretty quickly that we need InputLike to be able to be serialized but adding a Serialize bound to the trait prevents using dynamic dispatch, which we need in order to store them in the same vec. E.g. Vec<Box<InputLike>> would return the error "cannot be made into an object".

Usually when I run into this I just switch to using enums like we already are, but that doesn't allow users of the library extend it. So keep in mind if we can find another way to support user extendibility or ditch that requirement, we could stick with an enum and probably do things like box the Chord variant to reduce the size of UserInput.

For solving the need for InputLike to be cloneable and comparable, I was able to add a clone_dyn and eq_dyn function to the trait so it could be implemented manually for the trait (code here), but that doesn't work for Serialize because the serialize function needs to take a generic Serializer so adding a serialize<S>(&self, serializer: S) function to the InputLike trait would prevent using dynamic dispatch again.

I have a branch with my current attempt where I'm running into this issue here: main...paul-hansen:leafwing-input-manager:userinput-as-trait There are probably major crimes committed in the code but I'm currently just trying to get something to compile as a proof of concept. This code currently errors with:

error[E0277]: the trait bound `dyn InputLike: action_state::_::_serde::Serialize` is not satisfied
    --> src\input_map.rs:531:13
     |
529  |           input_map.serialize_field(
     |                     --------------- required by a bound introduced by this call
530  |               "map",
531  | /             &self
532  | |                 .iter()
533  | |                 .map(|(set, action)| (action, set.iter().collect()))
534  | |                 .collect::<BTreeMap<A, Vec<&Box<dyn InputLike>>>>(),
     | |___________________________________________________________________^ the trait `action_state::_::_serde::Serialize` is not implemented for `dyn InputLike`

tldr; Looking for suggestions on how to make the desired InputLike trait serializable while still being able to store it in a Vec containing multiple different InputLike types i.e. Vec<Box<dyn InputLike>>.

[paul-hansen]

Looks like https://github.com/dtolnay/erased-serde might be a solution? I haven't used it before, looking into it.

Update: Seems promising, got serialize implemented for InputLike, working on Deserialize. Will be afk for awhile, helping my brother move.

[alice-i-cecile]

Awesome; that looks like a promising direction. If we can't get that working fully we can can probably serde the actual raw input types and then reconstruct it.

Unfortunately serde is a pretty critical feature for storing key bindings :)

[paul-hansen]

Agreed that being able to store keybindings is a super important feature. (using it one of the projects I'm working on)

Looking at this more, I'm not sure it's possible to simply implement deserialize for dyn InputLike as it's impossible to know the underlying type it should be deserialized to. To put it another way, when loading an input from your keybindings file, how would it know which type to create?

Currently the only way I can think to get around this would be to have a registry for InputLike that other libraries would be able to register their types that implement InputLike with. We still couldn't implement Deserialize for InputLike (unless we used a static global registry that we could access inside it's deserialize implementation, which I'm not a fan of) but it would allow us to deserialize using the registry, something like let input_map = InputMap::load(file: Path, registry: &InputRegistry).

Does this sound like the way we want to pursue this? Any other ideas?

[alice-i-cecile]

Hmm, right 🤔 I agree with that basic approach. Can we reuse bevy_reflect's type registry for this? Should we?

[paul-hansen]

I was thinking that too, it definitely feels like Bevy's type registry should be a good fit.

I don't think it's as simple as adding #[reflect(InputLike)] to store a fn pointer to deserialize it with, or require FromReflect to build it using reflection, since it looks like both of those would break dynamic dispatch, which we need to be able to box them and store in a vector in the InputMap. Might be able to use an additional trait to get around that though. I'll play around with it and see what I can get working.

[paul-hansen]

I've been distracted from this but haven't forgotten about it. Going to see my parents for mother's day over the weekend and then my schedule should open up a lot more the rest of the month.

Just a quick update, I've had good luck with splitting it into two traits, currently calling them InputLike and InputLikeObject. InputLikeObject contains all the object safe methods and is what is stored in InputMap, InputLike requires InputLikeObject and has all the non-instance specific (doesn't take &self etc.) methods and traits. InputLikeObject requires Any so we can use TypeId to lookup any type data registered for its InputLike etc.

I think we'll need to reimplement InputStreams as a trait too, so we can collect input values from third party crates. Currently trying out having a method on InputLike that has a function that takes &mut world and returns a boxed trait object "InputStreams" that has the methods InputStreams used to have, using the TypeId of InputLikeObject to tell which InputStream to use for each input mapping. That part is mostly theoretical atm, not sure it will survive reality.

[alice-i-cecile]

Excellent: I agree with the need to split out InputStreams to be more extensible as well.

[paul-hansen]

Got a rough proof of concept working. There's a new example that shows what a third party integration could look like.

This example runs and works, there's still major sections of code that are just replaced with todos to make this compile so not much works outside this example atm. For example input mocking is something I haven't given much thought to how it would work yet.

I'm slightly concerned about performance in a few places, but hopefully once we get something working we can identify where it actually matters and address those. For example, I had to clone all the input maps every frame to allow world access to InputStreams, probably not great for perf.

[alice-i-cecile]

Awesome :D I like the look of this: feel free to put up a draft PR early and I can try to chip in on the missing holes too.

IMO we can probably cut input mocking entirely: leafwing_input_playback takes a much more sensible approach.

[alice-i-cecile]

Upstream issue: bevyengine/bevy#10855

[alice-i-cecile]

Merging #534 to fix this.