[Merged by Bors] - Gamepad events refactor

[DevinLeamy]

Objective

• Remove redundant gamepad events
• Simplify consuming gamepad events.
• Refactor: Separate handling of gamepad events into multiple systems.

Solution

• Removed GamepadEventRaw, and GamepadEventType.
• Added bespoke GamepadConnectionEvent, GamepadAxisChangedEvent, and GamepadButtonChangedEvent.
• Refactored gamepad_event_system.
• Added gamepad_button_event_system, gamepad_axis_event_system, and gamepad_connection_system, which update the Input and Axis resources using their corresponding event type.

Gamepad events are now handled in their own systems and have their own types.

This allows for querying for gamepad events without having to match on GamepadEventType and makes creating handlers for specific gamepad event types, like a GamepadConnectionEvent or GamepadButtonChangedEvent possible.

We remove GamepadEventRaw by filtering the gamepad events, using GamepadSettings, at the source, in bevy_gilrs. This way we can create GamepadEvents directly and avoid creating GamepadEventRaw which do not pass the user defined filters.

We expose ordered GamepadEvents and we can respond to individual gamepad event types.

Migration Guide

• Replace GamepadEvent and GamepadEventRaw types with their specific gamepad event type.

[alice-i-cecile]

@bzm3r @Shatur I'd like your opinions on this :)

[vacuus]

Because we have the Input<GamepadButton> resource having a GamepadButtonEvent for indicating button presses seems redundant. I would definitely consider removing it altogether.

I think this is analogous to the KeyboardInput event and Input<KeyCode>. Input<T> is convenient, and I think it makes sense as a default but shouldn't preclude a bespoke accounting of input events. Aside from the overhead of multiple HashMaps, Input<T> isn't suited for only recording a subset of possible inputs (you know what, I'm not sure this point is valid).

[Shatur]

I haven't used a gamepad with Bevy yet, so I don't have a clear opinion on this, sorry :)

[DevinLeamy]

I think this is analogous to the KeyboardInput event and Input<KeyCode>. Input<T> is convenient, and I think it makes sense as a default but shouldn't preclude a bespoke accounting of input events.

The distinction here is that KeyboardInput is used to update Input<KeyCode> and Input<ScanCode>, whereas GamepadButtonEvent (previously GamepadEvent { event_type: GamepadEventType::ButtonChanged(..), ... }) is created after Input<GamepadButton> has been updated. What KeyboardInput and the GamepadButtonEvent event do provide though is between-frame event ordering, which could become a mute point when/if event ordering is added to the Input resource #5984.

[vacuus]

Ah, I didn't know that. Is it possible to provide raw gamepad button events analogous to KeyboardInput? Unless Input<T> can be made to provide flexibility* without overhead, I think the lower-level event streams should still be provided.

* For only caring about a certain set of inputs, whole HashSetss seems excessive, and there's the event ordering you mentioned above

[DevinLeamy]

Is it possible to provide raw gamepad button events analogous to KeyboardInput?

It's possible. We source all our events from gilrs in the bevy_gilrs crate. The thing, to allow users to be able to configure their gamepad's sensitivity to inputs (the "press weight" [0.0-1.0] that registers as a click), we don't use the analogous "on click" event. Rather, we use the "press weight changed" event and then in bevy_input we convert the press weight into a "press" or "release", based on the user's input.

We could convert events directly into some "on click" event inside bevy_gilrs, but it would require accessing a user's gamepad configuration settings inside of bevy_gilrs, which is a little iffy...

There is this issue though where we consider the idea of passing along the entire event stream (winit event stream, in that case), which would allow us to retain relative event ordering information. If we did something like that here, we could do all of the event processing inside of bevy_input so we wouldn't run into this problem of accessing gamepad configurations outside of bevy_input.

[DevinLeamy]

ping

[alice-i-cecile]

Valid! I'll prioritize this on Monday.

[MrGVSV]

LGTM! I think it'll be much nicer to have these split into their own event types!

Just a few non-blocking comments.

[alice-i-cecile]

Once the allow_unused is cleaned up this LGTM. I really like being able to remove GamepadEventsRaw.

Note that in your changelog you called it RawGamepadEvent: this is wrong and should be fixed to improve searchability.

[DevinLeamy]

Note that in your changelog you called it RawGamepadEvent: this is wrong and should be fixed to improve searchability.

Updated!

[cart]

I'm a little worried that the loss of relative event ordering isn't worth the "improved ergonomics" here.
We already have opinionated abstractions like Input<GamepadButton> and Axis<GamepadAxis> for users that need fast / ergonomic access to gamepad state for the current frame and aren't worried about relative event ordering. These cover the majority of cases.

GamepadEvent exists for the (much more niche / low level) case of needing less opinionated / frame-independent access to gamepad input. We want it to be as unopinionated as possible because we want to enable as many niche use cases as possible.

Consider the following case: Someone is building a fighting game where input order matters a lot (ex: within a 500ms window, did someone press the "left analog" or "a button" first ... this affects game outcomes ). Their game has pretty graphics and they've noticed that on lower end hardware sometimes they get lag spikes of ~500ms. They don't want the game experience to completely break down when this happens. In this case, using Input<GamepadButton> and Axis<GamepadAxis> projects the "left analog" and "a button" events onto the "same frame's state", which erases the relevant information. So they choose to write their own input management system to determine what was pressed first.

On Bevy main, GamepadEvent solves that problem. In this PR, that information is completely lost. They can't even use the Gilrs resource directly because we've already consumed the events.

[cart]

Just read this issue / comment: #5984 (comment)

Sounds like you're already thinking about this problem. A global cross event ts counter could work. With the t-based approach, we'd also need to make sure we only allow "ordered event reader merging" for events that come from the same source event feed, as the ts ordering only makes sense relative to things from the same feed. We could use a global cross-event ts counter, but also has problems as it would be "time of processing", not "time of occurrence". It feels like we'd be reinventing the "event ordering wheel" when enums already solve this problem in a simple (and statically checked) way.

[DevinLeamy]

On Bevy main, GamepadEvent solves that problem. In this PR, that information is completely lost.

I hadn't considered this. Definitely not something we want to lose, I see the merits of having the GamepadEvent event stream. Seems reasonable to wrap all the input events in something similar, which would resolve #5984.

I'm going to prod at this a little more because I think there are still other improvements I can make. Thanks for pointing that out!

[alice-i-cecile]

I agree with this critique: thanks for spotting this Cart. I can't quite figure out how to recreate this ordering in the new version of this PR: can you add or extend an example?

[DevinLeamy]

You can read the ordered event stream in the same way you previously could.

fn read_gamepad_events(mut gamepad_events: EventReader<GamepadEvent>) {
   for gamepad_event in gamepad_events.iter() {
       match gamepad_event {
            GamepadEvent::Connection(connection_event) => ()
            GamepadEvent::Button(button_event) => (),
            GamepadEvent::Axis(axis_event) => (),
        }
   }
}

But you can also read them individually.

fn gamepad_axis_event_system(mut axis_events: EventReader<GamepadAxisChangedEvent>) {
    for axis_event in axis_events.iter() {
        // ...
    }
}

The event stream is split after GamepadEvents are created, so the order is maintained.

The example demonstrates the latter as that is the preferred way to read the events, whereas the former is for the more advanced use cases where you require the ordered event stream. I've noted in the doc comment of the GamepadEvent this it should be read (over the individual events) when in-frame relative ordering is required.

Edit: I updated the example to show both usages.

[alice-i-cecile]

Great, thanks for updating the docs and example to more clearly explain this breakdown!

[DevinLeamy]

See the updated description #6965 (comment).

[alice-i-cecile]

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)
• build-without-default-features (bevy)
• build-without-default-features (bevy_ecs)
• build-without-default-features (bevy_reflect)
• check-compiles
• check-doc
• check-missing-examples-in-docs
• ci
• markdownlint
• run-examples
• run-examples-on-wasm
• run-examples-on-windows-dx12