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

Implement gamepads as entities #12770

Draft
wants to merge 49 commits into
base: main
Choose a base branch
from
Draft

Conversation

s-puig
Copy link
Contributor

@s-puig s-puig commented Mar 29, 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, 2 Axis.
  2. ButtonInput/Axis generic methods become really inconvenient to use e.g. any_pressed()
  3. GamepadButton/Axis structs are unnecessary boilerplate:
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);
        }
}
  1. Projects often need to create resources to store the selected gamepad and have to manually check if their gamepad is still valid anyways.

Solution

  • Implement gamepads as entities.

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

  1. Reduce boilerplate and allows iteration
let is_pressed = gamepads_buttons.iter().any(|buttons| buttons.pressed(GamepadButtonType::South))
  1. ButtonInput/Axis generic methods become ergonomic again
gamepad_buttons.any_just_pressed([GamepadButtonType::Start, GamepadButtonType::Select])
  1. Reduces the number of public components significantly (Gamepad, GamepadSettings, GamepadButtons, GamepadAxes)
  2. 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:
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?
  • Recreate the previous GamepadEvent
  • Rumble component

Changelog

Added

TODO

Changed

TODO

Removed

TODO

Migration Guide

TODO

@alice-i-cecile alice-i-cecile added A-Input Player input via keyboard, mouse, gamepad, and more C-Usability A simple quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR labels Mar 29, 2024
@s-puig s-puig marked this pull request as ready for review March 31, 2024 19:37
@s-puig
Copy link
Contributor Author

s-puig commented Apr 1, 2024

@alice-i-cecile @mockersf I think this is mostly done, i will recheck the docs again once you decide if this should be merged or not.

crates/bevy_input/src/button_input.rs Outdated Show resolved Hide resolved
crates/bevy_input/src/button_input.rs Outdated Show resolved Hide resolved
crates/bevy_input/src/gamepad.rs Outdated Show resolved Hide resolved
examples/input/gamepad_input_events.rs Outdated Show resolved Hide resolved
crates/bevy_input/src/gamepad.rs Outdated Show resolved Hide resolved
@alice-i-cecile
Copy link
Member

@cart, just to be sure: you're broadly in favor of representing gamepads as entities, correct?

pub fn convert_gamepad_id(gamepad_id: gilrs::GamepadId) -> Gamepad {
Gamepad::new(gamepad_id.into())
// TODO: bevy_input directly depends on Gilrs::GamepadId's. This could cause conflicts with other plugins.
// Instead, bevy_input should provide a unique GamepadId and bevy_gilrs keep a mapping between them
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I'm fine to leave this for follow-up though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like Entity should be that unique id

Copy link
Contributor Author

@s-puig s-puig Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, i don't think this is a good idea right now until we can re-create entities with a given id.

Gilrs tries to reuse the same ids for reconnecting gamepads which allows us (and users) a way to preserve state. We could keep some mapping between dropped entities->current entity but that might be a bit of a foot-gun with users expecting their id(entity) to also persist.

EDIT: On second thought, on disconnection, we could leave the entity alive, remove the gamepad components and assume that if the user deleted the entity it no longer wants to preserve the state and create a new one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea in your edit. Eventually we could use entity disabling instead.

@cart
Copy link
Member

cart commented Sep 17, 2024

@cart, just to be sure: you're broadly in favor of representing gamepads as entities, correct?

Provided we have a satisfying answer to my comment right above this, then yeah I'm broadly in favor.

@alice-i-cecile alice-i-cecile added X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed X-Controversial There is active debate or serious implications around merging this PR labels Sep 17, 2024
@s-puig s-puig marked this pull request as draft September 19, 2024 12:46
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 simple quality-of-life change that makes Bevy easier to use S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers
Projects
Status: Responded
Development

Successfully merging this pull request may close these issues.

3 participants