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

System 2449/fix gamepad #73

Merged
merged 1 commit into from
Apr 12, 2024
Merged

System 2449/fix gamepad #73

merged 1 commit into from
Apr 12, 2024

Conversation

cmajed
Copy link
Contributor

@cmajed cmajed commented Apr 5, 2024

Description

Rework Gamepad and Gamepad Management to fix:
- handle gamepad pugged/unplugged
- catch and handle confirmation
- send correct vinput gamepad redis message
- Handle axis changes
- Rework catch and dispatch event to dispatch it only when axes changed
- Add vendor id and product id to the plugg in command

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I've read & comply with the contributing guidelines
  • I have tested my code for new features & regressions on both mobile & desktop devices, using the latest version of major browsers.
  • I have made corresponding changes to the documentation (README.md).
  • I've checked my modifications for any breaking changes.

Copy link
Contributor

@jparez jparez left a comment

Choose a reason for hiding this comment

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

LGTM

Edit;
Oups, missed some commit.

The #72 need to be discuss with Julien tomorrow (04/04), the code could change after that

@pgivel
Copy link
Contributor

pgivel commented Apr 8, 2024

I think the first commit should not be in this PR ? Or maybe this PR should not target main ?

@cmajed
Copy link
Contributor Author

cmajed commented Apr 8, 2024

yes it cames from this PR
#72
I could target it instead!

@jparez jparez self-requested a review April 8, 2024 09:20
src/plugins/GamepadManager.js Show resolved Hide resolved
src/plugins/GamepadManager.js Outdated Show resolved Hide resolved
src/plugins/GamepadManager.js Outdated Show resolved Hide resolved
src/plugins/GamepadManager.js Outdated Show resolved Hide resolved
this.currentGamepads[localIndex] = {remoteIndex: remoteIndex, buttons: []};
const gamepad = this.getRawGamepad(remoteIndex);
this.currentGamepads[localIndex] = {remoteIndex: remoteIndex,
buttons: [gamepad.buttons.length],
Copy link
Contributor

Choose a reason for hiding this comment

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

In case of getRawGamepad doesn't return a gamepad and wathever you return from getRawGamepad ( undefined or null) you wil get an uncaught error here : Uncaught TypeError: gamepad is null

src/plugins/Gamepad.js Outdated Show resolved Hide resolved
@cmajed cmajed requested review from jparez and pgivel April 8, 2024 12:27
src/plugins/GamepadManager.js Outdated Show resolved Hide resolved
src/plugins/GamepadManager.js Outdated Show resolved Hide resolved
src/plugins/GamepadManager.js Outdated Show resolved Hide resolved
src/plugins/GamepadManager.js Outdated Show resolved Hide resolved
src/plugins/GamepadManager.js Outdated Show resolved Hide resolved
src/plugins/GamepadManager.js Outdated Show resolved Hide resolved
Comment on lines 373 to 375
this.currentGamepads[hostIndex] = {remoteIndex: hostIndex,
buttons: [gamepad.buttons.length],
axes: [gamepad.axes.length]};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this.currentGamepads[hostIndex] = {remoteIndex: hostIndex,
buttons: [gamepad.buttons.length],
axes: [gamepad.axes.length]};
this.currentGamepads[hostIndex] = {
remoteIndex: hostIndex,
buttons: [gamepad.buttons.length],
axes: [gamepad.axes.length]
};

I think it would be more readable like this. But maybe simply use the yarn validate --fix command to fix the code style everywhere.

@cmajed cmajed requested a review from pgivel April 8, 2024 13:56
@cmajed cmajed force-pushed the SYSTEM-2449/fix_gamepad branch from f40cd2d to 0f1448b Compare April 10, 2024 13:03
@cmajed cmajed force-pushed the SYSTEM-2449/fix_gamepad branch from 8a4ce29 to 0611484 Compare April 12, 2024 06:59
@cmajed cmajed changed the base branch from main to bugfix/gamepad-not-working April 12, 2024 07:00
@cmajed cmajed force-pushed the SYSTEM-2449/fix_gamepad branch from 0611484 to bbc9dae Compare April 12, 2024 07:01
    - handle gamepad pugged/unplugged
    - catch and handle confirmation
    - send correct vinput gamepad redis message
    - Handle axis changes
    - Rework catch and dispatch event to dispatch it only when axes changed
    - Add vendor id and product id to the plugg in command
@cmajed cmajed force-pushed the SYSTEM-2449/fix_gamepad branch from bbc9dae to 2f6c615 Compare April 12, 2024 07:43
Base automatically changed from bugfix/gamepad-not-working to main April 12, 2024 08:06
@jparez jparez merged commit 7551bca into main Apr 12, 2024
1 check passed
@jparez jparez deleted the SYSTEM-2449/fix_gamepad branch April 12, 2024 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants