Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve gamepad input handling ergonomics #12674

Closed
wants to merge 2 commits into from

Conversation

s-puig
Copy link
Contributor

@s-puig s-puig commented Mar 23, 2024

Objective

Writing code for gamepads input can be a bit awkward compared to other inputs.

They have many resources (Gamepads, GamepadSettings, GamepadInfo, ButtonInput and 2 Axis), ButtonInput (and Axis) share functionality with other inputs so they often lack ergonomics (due to performance reasons), constantly constructing GamepadButton/Axis, making some functions highly inconvenient to use (any/all_* button functions) or lacking methods that would be expected.

This PR aims to make gamepad input handling more convenient for end-users.

Solution

  • Centralize gamepad input (like most game engines) into a SystemParam, hiding most issues from end-users.

Alternative solution

  • Centralize everything into a single resource (Gamepad/s)
  • Leave as it is

Changelog

Added

  • GamepadSystemParam

Provides GamepadWrapper's and enables convenient gamepad iteration methods (like any or all).

  • GamepadWrapper

Wrapper over gamepad resources (with most of their methods) and specific functions like sticks as vec2

  • GamepadButtonView

Data of a button (just_* input and analog value)

@s-puig
Copy link
Contributor Author

s-puig commented Mar 23, 2024

Posting early to see if there is interest(no docs, lacking methods) or would rather look into another solution.

Names are deliberately bad (although I'm open to suggestions!) to avoid refactoring code for the draft.

@alice-i-cecile
Copy link
Member

I previously attempted to make this less messy in #3419. My initial feeling is that I'd rather resolve this complexity than papering over it, but I definitely agree that the current situation is not good.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events A-Input Player input via keyboard, mouse, gamepad, and more C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR and removed A-ECS Entities, components, systems, and events labels Mar 23, 2024
Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@s-puig
Copy link
Contributor Author

s-puig commented Mar 24, 2024

I previously attempted to make this less messy in #3419. My initial feeling is that I'd rather resolve this complexity than papering over it, but I definitely agree that the current situation is not good.

Why was it closed? I also have an updated branch with everything crammed into Gamepad.

I kinda gave up on it because it was slightly slower (and would only get slower with more functionality) so a systemparam seemed like a good middle-ground.

@alice-i-cecile
Copy link
Member

I mostly ran out of steam: I had other priorities at the time, and not enough experience to really justify why it was the right call.

@s-puig s-puig closed this Mar 26, 2024
@s-puig
Copy link
Contributor Author

s-puig commented Mar 26, 2024

@alice-i-cecile After thinking about it, i think the best call would be to turn gamepads into entities/components.

We would get rid of the annoying GamepadButtons/axis, keep parallelization and allow users to attach components to them e.g GamepadPlayer<1>/GamepadPlayer<2>, tag gamepads each frame with a ActiveGamepad so users can filter them, attach device-specific components, etc..

I can start if you guys give me the greenlight.

@alice-i-cecile
Copy link
Member

@mockersf I'm on board with gamepads-as-entities: what's your opinion?

@mockersf
Copy link
Member

I would be curious to see how that would work, but I worry that it would be too different from how the other inputs work to be nice

@alice-i-cecile
Copy link
Member

Gamepads are already badly special-cased: I don't think that would make that situation worse. Like Francois says, let's see how it looks!

@s-puig
Copy link
Contributor Author

s-puig commented Mar 26, 2024

I would be curious to see how that would work, but I worry that it would be too different from how the other inputs work to be nice

I would argue that they would be too different because they are different, it works perfectly for keyboard/mouse because it can be assumed to have a single device. If this is an issue we could also make a SystemParam (like this PR attempted) that encapsulates every gamepad component, making it similar(and more approachable) to most game engines.

@mockersf
Copy link
Member

what would it mean to despawn the entity of a gamepad? can you add other components to it? can you remove some of its components?

@s-puig
Copy link
Contributor Author

s-puig commented Mar 27, 2024

what would it mean to despawn the entity of a gamepad?

Gamepad disconnected, either physically (bevy removed it) or virtually (dev removed it). Reconnected gamepads would reuse their EntityId (mainly because people will definitely be lazy and store the entity instead of the GamepadId).

This is a debatable topic though, we can always disallow removals external to the system by recreating the gamepad if still physically connected (ideally Bevy would have tools to make protected components/entities?)

can you add other components to it?

Yes, this way users can tag them which can be handy for multiplayer games. In fact, we could also tag active gamepads each frame (most game engines already have some way to get active/main gamepad) or add device-specific components.

can you remove some of its components?

Yes. Updates beyond bevy_gilrs would just get ignored.

IMO, using components should make things way more ergonomic. Queries makes the systems descriptive by their function signature (e.g. Option<Gyro>), allows query filters (With<Player<1>>, With<MainGamepad>) and iterators (most gamepad examples could be replaced by iter.any(), iter.all() or iter.filter() )..

@s-puig
Copy link
Contributor Author

s-puig commented Mar 27, 2024

@alice-i-cecile @mockersf You have a working implementation here trying to keep things 1:1 as much as possible. It is missing filtering (have to be moved from bevy_gilrs into bevy_input), most events have turned into raw events (so we need new processed events) and likely a Rumble component.

Personally, I would also merge Gamepad + GamepadInfo and GamepadButton(Digital+Analog)Component (axis + 2 bitsets?).

Overall, i think it is a positive change.

@alice-i-cecile
Copy link
Member

I like the direction of that :) I'd like to see a bit of a design doc laying out why we should make this change and the steps involved, but it's up to you whether you want to do that via a GIthub Issue, a PR description or a full RFC.

@mockersf
Copy link
Member

Sounds mostly good!

Reconnected gamepads would reuse their EntityId (mainly because people will definitely be lazy and store the entity instead of the GamepadId).

I don't think that's possible...

From a very quick read of your commit, I'm not sure how it would react to disconnecting / reconnecting the same gamepad, I think your cleanup is not complete when you despawn. It would be better also to avoid spawning an empty entity then adding the components in several commands, as that can be slower, though it shouldn't really happen often in this case

@s-puig
Copy link
Contributor Author

s-puig commented Mar 27, 2024

I like the direction of that :) I'd like to see a bit of a design doc laying out why we should make this change and the steps involved, but it's up to you whether you want to do that via a GIthub Issue, a PR description or a full RFC.

Will do once i get things a bit more mature.

Reconnected gamepads would reuse their EntityId (mainly because people will definitely be lazy and store the entity instead of the GamepadId).

I don't think that's possible...

Leaving aside that the implementation is totally wrong (oops!), indeed seems you can't spawn entities with a different generation.

From a very quick read of your commit, I'm not sure how it would react to disconnecting / reconnecting the same gamepad, I think your cleanup is not complete when you despawn.

I assume you are talking about cleaning the entity-id map?

@s-puig s-puig mentioned this pull request Mar 29, 2024
2 tasks
github-merge-queue bot pushed a commit that referenced this pull request Sep 27, 2024
# Objective

- Significantly improve the ergonomics of gamepads and allow new
features

Gamepads are a bit unergonomic to work with, they use resources but
unlike other inputs, they are not limited to a single gamepad, to get
around this it uses an identifier (Gamepad) to interact with anything
causing all sorts of issues.

1. There are too many: Gamepads, GamepadSettings, GamepadInfo,
ButtonInput<T>, 2 Axis<T>.
2. ButtonInput/Axis generic methods become really inconvenient to use
e.g. any_pressed()
3. GamepadButton/Axis structs are unnecessary boilerplate:

```rust
for gamepad in gamepads.iter() {
        if button_inputs.just_pressed(GamepadButton::new(gamepad, GamepadButtonType::South)) {
            info!("{:?} just pressed South", gamepad);
        } else if button_inputs.just_released(GamepadButton::new(gamepad, GamepadButtonType::South))
        {
            info!("{:?} just released South", gamepad);
        }
}
```
4. Projects often need to create resources to store the selected gamepad
and have to manually check if their gamepad is still valid anyways.

- Previously attempted by #3419 and #12674


## Solution

- Implement gamepads as entities.

Using entities solves all the problems above and opens new
possibilities.

1. Reduce boilerplate and allows iteration

```rust
let is_pressed = gamepads_buttons.iter().any(|buttons| buttons.pressed(GamepadButtonType::South))
```
2. ButtonInput/Axis generic methods become ergonomic again 
```rust
gamepad_buttons.any_just_pressed([GamepadButtonType::Start, GamepadButtonType::Select])
```
3. Reduces the number of public components significantly (Gamepad,
GamepadSettings, GamepadButtons, GamepadAxes)
4. Components are highly convenient. Gamepad optional features could now
be expressed naturally (`Option<Rumble> or Option<Gyro>`), allows devs
to attach their own components and filter them, so code like this
becomes possible:
```rust
fn move_player<const T: usize>(
    player: Query<&Transform, With<Player<T>>>,
    gamepads_buttons: Query<&GamepadButtons, With<Player<T>>>,
) {
    if let Ok(gamepad_buttons) = gamepads_buttons.get_single() {
        if gamepad_buttons.pressed(GamepadButtonType::South) {
            // move player
        }
    }
}
```
---

## Follow-up

- [ ] Run conditions?
- [ ] Rumble component

# Changelog

## Added

TODO

## Changed

TODO

## Removed

TODO


## Migration Guide

TODO

---------

Co-authored-by: Carter Anderson <[email protected]>
rudderbucky pushed a commit to rudderbucky/bevy that referenced this pull request Sep 27, 2024
# Objective

- Significantly improve the ergonomics of gamepads and allow new
features

Gamepads are a bit unergonomic to work with, they use resources but
unlike other inputs, they are not limited to a single gamepad, to get
around this it uses an identifier (Gamepad) to interact with anything
causing all sorts of issues.

1. There are too many: Gamepads, GamepadSettings, GamepadInfo,
ButtonInput<T>, 2 Axis<T>.
2. ButtonInput/Axis generic methods become really inconvenient to use
e.g. any_pressed()
3. GamepadButton/Axis structs are unnecessary boilerplate:

```rust
for gamepad in gamepads.iter() {
        if button_inputs.just_pressed(GamepadButton::new(gamepad, GamepadButtonType::South)) {
            info!("{:?} just pressed South", gamepad);
        } else if button_inputs.just_released(GamepadButton::new(gamepad, GamepadButtonType::South))
        {
            info!("{:?} just released South", gamepad);
        }
}
```
4. Projects often need to create resources to store the selected gamepad
and have to manually check if their gamepad is still valid anyways.

- Previously attempted by bevyengine#3419 and bevyengine#12674


## Solution

- Implement gamepads as entities.

Using entities solves all the problems above and opens new
possibilities.

1. Reduce boilerplate and allows iteration

```rust
let is_pressed = gamepads_buttons.iter().any(|buttons| buttons.pressed(GamepadButtonType::South))
```
2. ButtonInput/Axis generic methods become ergonomic again 
```rust
gamepad_buttons.any_just_pressed([GamepadButtonType::Start, GamepadButtonType::Select])
```
3. Reduces the number of public components significantly (Gamepad,
GamepadSettings, GamepadButtons, GamepadAxes)
4. Components are highly convenient. Gamepad optional features could now
be expressed naturally (`Option<Rumble> or Option<Gyro>`), allows devs
to attach their own components and filter them, so code like this
becomes possible:
```rust
fn move_player<const T: usize>(
    player: Query<&Transform, With<Player<T>>>,
    gamepads_buttons: Query<&GamepadButtons, With<Player<T>>>,
) {
    if let Ok(gamepad_buttons) = gamepads_buttons.get_single() {
        if gamepad_buttons.pressed(GamepadButtonType::South) {
            // move player
        }
    }
}
```
---

## Follow-up

- [ ] Run conditions?
- [ ] Rumble component

# Changelog

## Added

TODO

## Changed

TODO

## Removed

TODO


## Migration Guide

TODO

---------

Co-authored-by: Carter Anderson <[email protected]>
robtfm pushed a commit to robtfm/bevy that referenced this pull request Oct 4, 2024
# Objective

- Significantly improve the ergonomics of gamepads and allow new
features

Gamepads are a bit unergonomic to work with, they use resources but
unlike other inputs, they are not limited to a single gamepad, to get
around this it uses an identifier (Gamepad) to interact with anything
causing all sorts of issues.

1. There are too many: Gamepads, GamepadSettings, GamepadInfo,
ButtonInput<T>, 2 Axis<T>.
2. ButtonInput/Axis generic methods become really inconvenient to use
e.g. any_pressed()
3. GamepadButton/Axis structs are unnecessary boilerplate:

```rust
for gamepad in gamepads.iter() {
        if button_inputs.just_pressed(GamepadButton::new(gamepad, GamepadButtonType::South)) {
            info!("{:?} just pressed South", gamepad);
        } else if button_inputs.just_released(GamepadButton::new(gamepad, GamepadButtonType::South))
        {
            info!("{:?} just released South", gamepad);
        }
}
```
4. Projects often need to create resources to store the selected gamepad
and have to manually check if their gamepad is still valid anyways.

- Previously attempted by bevyengine#3419 and bevyengine#12674


## Solution

- Implement gamepads as entities.

Using entities solves all the problems above and opens new
possibilities.

1. Reduce boilerplate and allows iteration

```rust
let is_pressed = gamepads_buttons.iter().any(|buttons| buttons.pressed(GamepadButtonType::South))
```
2. ButtonInput/Axis generic methods become ergonomic again 
```rust
gamepad_buttons.any_just_pressed([GamepadButtonType::Start, GamepadButtonType::Select])
```
3. Reduces the number of public components significantly (Gamepad,
GamepadSettings, GamepadButtons, GamepadAxes)
4. Components are highly convenient. Gamepad optional features could now
be expressed naturally (`Option<Rumble> or Option<Gyro>`), allows devs
to attach their own components and filter them, so code like this
becomes possible:
```rust
fn move_player<const T: usize>(
    player: Query<&Transform, With<Player<T>>>,
    gamepads_buttons: Query<&GamepadButtons, With<Player<T>>>,
) {
    if let Ok(gamepad_buttons) = gamepads_buttons.get_single() {
        if gamepad_buttons.pressed(GamepadButtonType::South) {
            // move player
        }
    }
}
```
---

## Follow-up

- [ ] Run conditions?
- [ ] Rumble component

# Changelog

## Added

TODO

## Changed

TODO

## Removed

TODO


## Migration Guide

TODO

---------

Co-authored-by: Carter Anderson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Input Player input via keyboard, mouse, gamepad, and more C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants