Skip to content

Gamepad improvements#16222

Merged
alice-i-cecile merged 10 commits intobevyengine:mainfrom
simgine:gamepad-improvements
Nov 4, 2024
Merged

Gamepad improvements#16222
alice-i-cecile merged 10 commits intobevyengine:mainfrom
simgine:gamepad-improvements

Conversation

@Shatur
Copy link
Copy Markdown
Contributor

@Shatur Shatur commented Nov 4, 2024

Objective

Closes #16221.

Solution

  • Make Gamepad fields public and remove delegates / getters.
  • Move impl Into to Axis methods (delegates for Axis used impl Into to allow passing both GamepadAxis and GamepadButton).
  • Improve docs.

Testing

  • I run tests.

Not sure if the migration guide is needed, since it's a feature from RC, but I wrote it just in case.


Migration Guide

  • Gamepad fields are now public.
  • Instead of using Gamepad delegates like Gamepad::just_pressed, call these methods directly on the fields.

This lets users to mock the input. Analog and digital fields previosly were accesible resources, so I think it makes sense.

Since the fields are now public, I removed all getters and delegates. Delegates for `Axis` used `impl Into` to allow passing `GamepadAxis` and `GamepadButton`, so moved `impl Into` to `Axis` methods.
@Shatur Shatur requested a review from alice-i-cecile November 4, 2024 11:14
Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Dramatically less complicated. I really don't think there's need to lock down these fields, and I much prefer this. A couple of cleanup notes.

@pcwalton @brandon-reinhart: you two worked to port LWIM to these changes. Can I get your reviews / opinions here?

@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Nov 4, 2024
@alice-i-cecile alice-i-cecile added A-Input Player input via keyboard, mouse, gamepad, and more C-Usability A targeted quality-of-life change that makes Bevy easier to use P-Regression Functionality that used to work but no longer does. Add a test for this! S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 4, 2024
Shatur and others added 2 commits November 4, 2024 17:17
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
@Shatur Shatur requested a review from alice-i-cecile November 4, 2024 16:12
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 4, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Nov 4, 2024
Merged via the queue into bevyengine:main with commit b0058dc Nov 4, 2024
mockersf pushed a commit that referenced this pull request Nov 5, 2024
# Objective

Closes #16221.

## Solution

- Make `Gamepad` fields public and remove delegates / getters.
- Move `impl Into` to `Axis` methods (delegates for `Axis` used `impl
Into` to allow passing both `GamepadAxis` and `GamepadButton`).
- Improve docs.

## Testing

- I run tests.

Not sure if the migration guide is needed, since it's a feature from RC,
but I wrote it just in case.

---

## Migration Guide

- `Gamepad` fields are now public.
- Instead of using `Gamepad` delegates like `Gamepad::just_pressed`,
call these methods directly on the fields.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
cart added a commit that referenced this pull request Nov 18, 2024
github-merge-queue bot pushed a commit that referenced this pull request Nov 19, 2024
# Objective

#16222 regressed the user experience of actually using gamepads:

```rust
// Before 16222
gamepad.just_pressed(GamepadButton::South)

// After 16222
gamepad.digital.just_pressed(GamepadButton::South)

// Before 16222
gamepad.get(GamepadButton::RightTrigger2)

// After 16222
gamepad.analog.get(GamepadButton::RighTrigger2)
```

Users shouldn't need to think about "digital vs analog" when checking if
a button is pressed. This abstraction was intentional and I strongly
believe it is in our users' best interest. Buttons and Axes are _both_
digital and analog, and this is largely an implementation detail. I
don't think reverting this will be controversial.

## Solution

- Revert most of #16222
- Add the `Into<T>` from #16222 to the internals
- Expose read/write `digital` and `analog` accessors on gamepad, in the
interest of enabling the mocking scenarios covered in #16222 (and
allowing the minority of users that care about the "digital" vs "analog"
distinction in this context to make that distinction)

---------

Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
mockersf pushed a commit that referenced this pull request Nov 19, 2024
# Objective

#16222 regressed the user experience of actually using gamepads:

```rust
// Before 16222
gamepad.just_pressed(GamepadButton::South)

// After 16222
gamepad.digital.just_pressed(GamepadButton::South)

// Before 16222
gamepad.get(GamepadButton::RightTrigger2)

// After 16222
gamepad.analog.get(GamepadButton::RighTrigger2)
```

Users shouldn't need to think about "digital vs analog" when checking if
a button is pressed. This abstraction was intentional and I strongly
believe it is in our users' best interest. Buttons and Axes are _both_
digital and analog, and this is largely an implementation detail. I
don't think reverting this will be controversial.

## Solution

- Revert most of #16222
- Add the `Into<T>` from #16222 to the internals
- Expose read/write `digital` and `analog` accessors on gamepad, in the
interest of enabling the mocking scenarios covered in #16222 (and
allowing the minority of users that care about the "digital" vs "analog"
distinction in this context to make that distinction)

---------

Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Dec 2, 2024
# Objective

Closes bevyengine#16221.

## Solution

- Make `Gamepad` fields public and remove delegates / getters.
- Move `impl Into` to `Axis` methods (delegates for `Axis` used `impl
Into` to allow passing both `GamepadAxis` and `GamepadButton`).
- Improve docs.

## Testing

- I run tests.

Not sure if the migration guide is needed, since it's a feature from RC,
but I wrote it just in case.

---

## Migration Guide

- `Gamepad` fields are now public.
- Instead of using `Gamepad` delegates like `Gamepad::just_pressed`,
call these methods directly on the fields.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Dec 2, 2024
…#16425)

# Objective

bevyengine#16222 regressed the user experience of actually using gamepads:

```rust
// Before 16222
gamepad.just_pressed(GamepadButton::South)

// After 16222
gamepad.digital.just_pressed(GamepadButton::South)

// Before 16222
gamepad.get(GamepadButton::RightTrigger2)

// After 16222
gamepad.analog.get(GamepadButton::RighTrigger2)
```

Users shouldn't need to think about "digital vs analog" when checking if
a button is pressed. This abstraction was intentional and I strongly
believe it is in our users' best interest. Buttons and Axes are _both_
digital and analog, and this is largely an implementation detail. I
don't think reverting this will be controversial.

## Solution

- Revert most of bevyengine#16222
- Add the `Into<T>` from bevyengine#16222 to the internals
- Expose read/write `digital` and `analog` accessors on gamepad, in the
interest of enabling the mocking scenarios covered in bevyengine#16222 (and
allowing the minority of users that care about the "digital" vs "analog"
distinction in this context to make that distinction)

---------

Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
# Objective

Closes bevyengine#16221.

## Solution

- Make `Gamepad` fields public and remove delegates / getters.
- Move `impl Into` to `Axis` methods (delegates for `Axis` used `impl
Into` to allow passing both `GamepadAxis` and `GamepadButton`).
- Improve docs.

## Testing

- I run tests.

Not sure if the migration guide is needed, since it's a feature from RC,
but I wrote it just in case.

---

## Migration Guide

- `Gamepad` fields are now public.
- Instead of using `Gamepad` delegates like `Gamepad::just_pressed`,
call these methods directly on the fields.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
…#16425)

# Objective

bevyengine#16222 regressed the user experience of actually using gamepads:

```rust
// Before 16222
gamepad.just_pressed(GamepadButton::South)

// After 16222
gamepad.digital.just_pressed(GamepadButton::South)

// Before 16222
gamepad.get(GamepadButton::RightTrigger2)

// After 16222
gamepad.analog.get(GamepadButton::RighTrigger2)
```

Users shouldn't need to think about "digital vs analog" when checking if
a button is pressed. This abstraction was intentional and I strongly
believe it is in our users' best interest. Buttons and Axes are _both_
digital and analog, and this is largely an implementation detail. I
don't think reverting this will be controversial.

## Solution

- Revert most of bevyengine#16222
- Add the `Into<T>` from bevyengine#16222 to the internals
- Expose read/write `digital` and `analog` accessors on gamepad, in the
interest of enabling the mocking scenarios covered in bevyengine#16222 (and
allowing the minority of users that care about the "digital" vs "analog"
distinction in this context to make that distinction)

---------

Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
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 P-Regression Functionality that used to work but no longer does. Add a test for this! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow mocking inputs for Gamepad

3 participants