Add some missing features from the gamepads-as-entities change that were needed to update `leafwing-input-manager`. by pcwalton · Pull Request #15685 · bevyengine/bevy

pcwalton · 2024-10-06T23:56:45Z

The gamepads-as-entities change caused several regressions. This patch fixes each of them:

This PR introduces two new fields on GamepadInfo: vendor_id, and product_id, as well as associated methods. These fields are simply mirrored from the gilrs library.
That PR removed the methods that allowed iterating over all pressed and released buttons, as well as the method that allowed iterating over the axis values. (It was still technically possible to do so by using reflection to access the private fields of Gamepad.)
The Gamepad component wasn't marked reflectable. This PR fixes that problem.

These changes allowed me to forward port leafwing-input-manager.

needed to update `leafwing-input-manager`. The gamepads-as-entities change caused several regressions. This patch fixes each of them: 1. After that PR, there was no stable unique identifier available for gamepads. To fix this, this PR introduces several new fields on `GamepadInfo`: `uuid`, `vendor_id`, and `product_id`, as well as associated methods. These fields are simply mirrored from the `gilrs` library. The UUID is the preferred way to identify gamepads across app invocations. 2. That PR removed the methods that allowed iterating over all pressed and released buttons, as well as the method that allowed iterating over the axis values. (It was still technically possible to do so by using reflection to access the private fields of `Gamepad`.) 3. The `Gamepad` component wasn't marked reflectable. This PR fixes that problem. These changes allowed me to forward port `leafwing-input-manager`.

alice-i-cecile · 2024-10-07T00:05:45Z

@s-puig can I get your review here?

bushrat011899

Looks good to me! Just a mild bikeshed-style question around whether these methods should be named iter_x instead of get_x.

bushrat011899 · 2024-10-07T00:10:00Z

crates/bevy_input/src/gamepad.rs

+    /// Returns an iterator over all digital [button]s that are pressed.
+    ///
+    /// [button]: GamepadButton
+    pub fn get_pressed(&self) -> impl Iterator<Item = &GamepadButton> {


Non-blocking bikeshed question: Should these iterator methods instead be called iter_pressed, iter_just_pressed etc.?

We should be consistent with the ButtonInput API: https://docs.rs/bevy/latest/bevy/input/struct.ButtonInput.html#method.

I do think this might be nice to change, but it should be done all at once.

Perfectly reasonable!

…ere needed to update `leafwing-input-manager`. (#15685) The gamepads-as-entities change caused several regressions. This patch fixes each of them: 1. After that PR, there was no stable unique identifier available for gamepads. To fix this, this PR introduces several new fields on `GamepadInfo`: `uuid`, `vendor_id`, and `product_id`, as well as associated methods. These fields are simply mirrored from the `gilrs` library. The UUID is the preferred way to identify gamepads across app invocations. 2. That PR removed the methods that allowed iterating over all pressed and released buttons, as well as the method that allowed iterating over the axis values. (It was still technically possible to do so by using reflection to access the private fields of `Gamepad`.) 3. The `Gamepad` component wasn't marked reflectable. This PR fixes that problem. These changes allowed me to forward port `leafwing-input-manager`.

alice-i-cecile · 2024-10-07T00:40:22Z

Uuid isn't Reflect, so we either need to ignore it for Reflection or do some remote reflect nonsense. I vote the former, but actually the latter may be needed for serialization.

crates/bevy_input/src/axis.rs

crates/bevy_input/src/gamepad.rs

crates/bevy_input/src/lib.rs

MrGVSV · 2024-10-07T01:07:10Z

Uuid isn't Reflect, so we either need to ignore it for Reflection or do some remote reflect nonsense. I vote the former, but actually the latter may be needed for serialization.

It should be? Might need to add the feature?

s-puig · 2024-10-07T09:15:43Z

I will take a proper look later, but we should avoid gilrs::Gamepad::uuid. See #153

pcwalton · 2024-10-08T00:07:09Z

@s-puig Do you want me to not expose the UUID entirely or just stop recommending it?

s-puig · 2024-10-08T00:56:00Z

I would rather not expose it. It's just that broken.
Although it may be worth it to add a comment in bevy_gilrs against implementing uuid until the issue is resolved. You are sadly not its first victim.

crates/bevy_input/src/gamepad.rs

crates/bevy_input/src/lib.rs

Most of the work here was fallout from the gamepads-as-entities change (bevyengine/bevy#12770), as well as the `PartialReflect` changes. This relies on bevyengine/bevy#15685. That PR must be applied to Bevy for this to build. The most recent upstream commit is 4bf647ff3b0ca7c8ca47496db9cfe03702328473.

* Upgrade to Bevy 0.15.0-dev. Most of the work here was fallout from the gamepads-as-entities change (bevyengine/bevy#12770), as well as the `PartialReflect` changes. This relies on bevyengine/bevy#15685. That PR must be applied to Bevy for this to build. The most recent upstream commit is 4bf647ff3b0ca7c8ca47496db9cfe03702328473. * Switch to upstream AccumulatedFoo * Try (unsuccessfully) to fix the failing tests * Bumps bevy_egui requirement from 0.30 to 0.31 * Sets bevy requirement to 0.15.0 * Replaces deprecated SpriteBundle * Applies system ordering suggestion * Use AccumulatedMouseScroll and AccumulatedMouseMotion from bevy * Fixes merge issues * Runs formatter * Gets button value from SpecificGamepadButton * Ignores tests * Fixes compilation issues * Fixes docs --------- Co-authored-by: Patrick Walton <pcwalton@mimiga.net>

pcwalton requested a review from alice-i-cecile October 6, 2024 23:56

pcwalton added A-Input Player input via keyboard, mouse, gamepad, and more 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 Oct 6, 2024

pcwalton added this to the 0.15 milestone Oct 6, 2024

pcwalton force-pushed the leafwing-fixes branch from ed9b2e7 to b44a95f Compare October 6, 2024 23:59

alice-i-cecile approved these changes Oct 7, 2024

View reviewed changes

alice-i-cecile added the D-Straightforward Simple bug fixes and API improvements, docs, test and examples label Oct 7, 2024

bushrat011899 approved these changes Oct 7, 2024

View reviewed changes

alice-i-cecile added this pull request to the merge queue Oct 7, 2024

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 Oct 7, 2024

atlv24 approved these changes Oct 7, 2024

View reviewed changes

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 7, 2024

MrGVSV reviewed Oct 7, 2024

View reviewed changes

crates/bevy_input/src/axis.rs Show resolved Hide resolved

crates/bevy_input/src/gamepad.rs Outdated Show resolved Hide resolved

crates/bevy_input/src/lib.rs Show resolved Hide resolved

mockersf removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 7, 2024

s-puig approved these changes Oct 8, 2024

View reviewed changes

crates/bevy_input/src/gamepad.rs Outdated Show resolved Hide resolved

crates/bevy_input/src/lib.rs Show resolved Hide resolved

pcwalton added 2 commits October 7, 2024 21:06

Address review comments

2881547

Merge remote-tracking branch 'origin/main' into leafwing-fixes

c4761a6

pcwalton mentioned this pull request Oct 8, 2024

Upgrade to Bevy 0.15.0-dev. Leafwing-Studios/leafwing-input-manager#644

Closed

pcwalton added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 8, 2024

alice-i-cecile added this pull request to the merge queue Oct 8, 2024

Merged via the queue into bevyengine:main with commit 48e2027 Oct 8, 2024

Shatur mentioned this pull request Nov 3, 2024

Allow mocking inputs for Gamepad #16221

Closed

Uh oh!

Conversation

pcwalton commented Oct 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alice-i-cecile commented Oct 7, 2024

Uh oh!

bushrat011899 left a comment

Choose a reason for hiding this comment

Uh oh!

bushrat011899 Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

alice-i-cecile Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bushrat011899 Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alice-i-cecile commented Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MrGVSV commented Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

s-puig commented Oct 7, 2024

Uh oh!

pcwalton commented Oct 8, 2024

Uh oh!

s-puig commented Oct 8, 2024

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

pcwalton commented Oct 6, 2024 •

edited

Loading

alice-i-cecile Oct 7, 2024 •

edited

Loading

alice-i-cecile commented Oct 7, 2024 •

edited

Loading

MrGVSV commented Oct 7, 2024 •

edited

Loading