Gamepad improvements#16222
Merged
alice-i-cecile merged 10 commits intobevyengine:mainfrom Nov 4, 2024
Merged
Conversation
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.
Useful for tests.
Member
There was a problem hiding this comment.
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?
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
alice-i-cecile
approved these changes
Nov 4, 2024
NiseVoid
approved these changes
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
Closes #16221.
Solution
Gamepadfields public and remove delegates / getters.impl IntotoAxismethods (delegates forAxisusedimpl Intoto allow passing bothGamepadAxisandGamepadButton).Testing
Not sure if the migration guide is needed, since it's a feature from RC, but I wrote it just in case.
Migration Guide
Gamepadfields are now public.Gamepaddelegates likeGamepad::just_pressed, call these methods directly on the fields.