Skip to content

[3.x] Improve validation of gamepads on Linux#67414

Open
JyKin wants to merge 1 commit intogodotengine:3.xfrom
JyKin:3.x
Open

[3.x] Improve validation of gamepads on Linux#67414
JyKin wants to merge 1 commit intogodotengine:3.xfrom
JyKin:3.x

Conversation

@JyKin
Copy link
Copy Markdown

@JyKin JyKin commented Oct 14, 2022 •
edited by akien-mga
Loading

EDIT: Changed idea and provide new fix after discussion

Bugsquad edit:

old message:
cherry pick those commits from master :
8fe2ecf
b966ca6

@JyKin JyKin requested a review from a team as a code owner October 14, 2022 22:00
@JyKin JyKin mentioned this pull request Oct 14, 2022
Closed
@Calinou Calinou added bug platform:linuxbsd topic:platforms topic:input cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Oct 15, 2022
@Calinou Calinou added this to the 3.6 milestone Oct 15, 2022
@madmiraal
Copy link
Copy Markdown
Contributor

madmiraal commented Oct 17, 2022

Note: #57612 (b966ca6) caused #59250. So, this probably shouldn't be merged without #59412 (bc17abc) which has not been approved, or some other better solution that more accurately identifies what is and what is not a gamepad.

@JyKin
Improve Linux Gamepad detection
456f128 
Check if device has key: should filter out motion-control input from
controller like recent playstation or nintendo.

Check if device has direction input: joystick or dpad.

Check if device has no touch-like input: should filter out touchscreen
and fingerprint reader.
@JyKin
Copy link
Copy Markdown
Author

JyKin commented Oct 18, 2022 •
edited
Loading

Ok, I went back and made a entirely new commit, without cherry-picking anything, with my solution.
I tested what I could (controller with motion controls, individual joycons) but I don't have wacom like tablet to verify they're not picked-up

@JyKin JyKin changed the title import fix for linux gamepad support provide fix for linux gamepad support Oct 18, 2022
@akien-mga akien-mga changed the title provide fix for linux gamepad support Improve validation of gamepads on Linux Dec 12, 2022
@akien-mga
Copy link
Copy Markdown
Member

akien-mga commented Dec 12, 2022

WDYT about this solution @madmiraal?

I'm a bit unsure about trying yet another approach which would differ from the one now used in 4.0 - but the code has diverged so it's possible that the 4.0 code isn't applicable here as is. The added booleans here are also nice for readability of what this is checking exactly.

madmiraal
madmiraal suggested changes Dec 12, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@madmiraal madmiraal left a comment

Choose a reason for hiding this comment

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

This solution is problematic. It requires a device to have keys to be treated as a gamepad, which is the opposite of the current behaviour, which requires a device not to have keys to be treated as a gamepad.

More importantly, this PR should be made against master not 3.x. As identified in the OP, the code on master has diverged from 3.x, because #40591 and #57612 haven't been backported to 3.x. This is not a reason to start making changes to 3.x without first making them on master.

@madmiraal
Copy link
Copy Markdown
Contributor

madmiraal commented Dec 12, 2022

Accurately detecting gamepads is hard. #40591 made our detection of gamepads the same as SDL's detection of gamepads. However, SDL's detection of gamepads (and devices in general) has since moved on:
https://github.com/libsdl-org/SDL/blob/9eb4d1f020bdc43e32375942120214eee5d70fee/src/core/linux/SDL_evdev_capabilities.c#L78-L105
It's also trying to copy the logic used by systemd:
https://github.com/systemd/systemd/blob/30338b8b66174a87727c8a8493ebf3a043f65b9d/src/udev/udev-builtin-input_id.c#L208-L295

Clearly, getting this right is going to take a more effort than just #40591, #57612, #59412 and #67414.

@akien-mga akien-mga changed the title Improve validation of gamepads on Linux [3.x] Improve validation of gamepads on Linux Aug 18, 2023
AThousandShips
AThousandShips reviewed Aug 18, 2023
View reviewed changes
platform/x11/joypad_linux.cpp
!(test_bit(EV_ABS, evbit) &&
test_bit(ABS_X, absbit) && test_bit(ABS_Y, absbit) &&
test_bit(ABS_RX, absbit) && test_bit(ABS_RY, absbit))) {
// Check if the device supports basic gamepad events and not non-joystick events
Copy link
Copy Markdown
Member

@AThousandShips AThousandShips Aug 18, 2023

Choose a reason for hiding this comment

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

Suggested change
// Check if the device supports basic gamepad events and not non-joystick events
// Check if the device supports basic gamepad events and not non-joystick events.

Per the comment style.

@lawnjelly lawnjelly modified the milestones: 3.6, 3.7 Sep 11, 2024
@MJacred MJacred mentioned this pull request Apr 22, 2025
Open
@akien-mga akien-mga removed the cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release label Jun 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AThousandShips AThousandShips AThousandShips left review comments

+1 more reviewer

@madmiraal madmiraal madmiraal requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

bug platform:linuxbsd topic:input topic:platforms

Projects

3.x release management
Status: PRs for 3.6 beta 1

Milestone

3.7

Development

Successfully merging this pull request may close these issues.

6 participants

@JyKin @madmiraal @akien-mga @AThousandShips @Calinou @lawnjelly