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

Added a few device GUIDs to is_xinput_device fixing controller problems #78043

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

mrTag
Copy link
Contributor

@mrTag mrTag commented Jun 9, 2023

The Xbox Elite Wireless gamepad has strange input problems in Windows, which we identified as the same problem as in this PR: #71784 and fixed in the same way (by adding another hardcoded device ID to JoypadWindows::is_xinput_device).

The fix was tested with a Xbox Elite Wireless gamepad and it works.

Update: added more GUIDs from SDL, as discussed below.
Probably also fixes #76724 now.

@akien-mga
Copy link
Member

akien-mga commented Jun 9, 2023

Thanks for the patch! It seems correct and should be fine to merge.

There are more issues with this code that we'll need to address eventually though. Both #71784 and this PR added missing devices, but there are quite a few more Xbox Bluetooth/BLE controllers with a similar issue.

This code was originally based off SDL's, but it seems like they changed their approach in 2019 with libsdl-org/SDL@6509644

We can see that back then they had a few more devices which we still lack:

    static GUID IID_ValveStreamingGamepad = { MAKELONG(0x28DE, 0x11FF), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };
    static GUID IID_X360WiredGamepad = { MAKELONG(0x045E, 0x02A1), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };
    static GUID IID_X360WirelessGamepad = { MAKELONG(0x045E, 0x028E), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };
	// Missing in Godot.
    static GUID IID_XOneWiredGamepad = { MAKELONG(0x045E, 0x02FF), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };
	// Missing in Godot.
    static GUID IID_XOneWirelessGamepad = { MAKELONG(0x045E, 0x02DD), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };
	// Missing in Godot.
    static GUID IID_XOneNewWirelessGamepad = { MAKELONG(0x045E, 0x02D1), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };
	// Missing in Godot.
    static GUID IID_XOneSWirelessGamepad = { MAKELONG(0x045E, 0x02EA), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };
	// Missing in Godot.
    static GUID IID_XOneSBluetoothGamepad = { MAKELONG(0x045E, 0x02E0), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };
	// Missing in Godot.
    static GUID IID_XOneEliteWirelessGamepad = { MAKELONG(0x045E, 0x02E3), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };

And now we have a couple entries which they didn't have back in 2019:

	static GUID IID_XSWirelessGamepad = { MAKELONG(0x045E, 0x0B13), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };
	static GUID IID_XEliteWirelessGamepad = { MAKELONG(0x045E, 0x0B05), 0x0000, 0x0000, { 0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44 } };

Longer term, we want to investigate leveraging SDL2 directly for gamepad mapping instead of trying to play catch with them and vendors.

Short term, I guess adding the other missing ones identified above would be useful. Let me know if you're interested in doing it, or if I should pick it up.

@mrTag mrTag force-pushed the xbox_elite_wireless_xinput branch from 8e636e4 to e3a67ec Compare June 9, 2023 12:32
@mrTag
Copy link
Contributor Author

mrTag commented Jun 9, 2023

Thanks for the list @akien-mga! Absolutely makes sense to include those in godot as well until the problem is solved for good. I can quickly do that, should I just change this PR or open a new one?

@akien-mga
Copy link
Member

It's best if you can amend this PR directly to add all the gamepads we identified so far at once.

@mrTag mrTag force-pushed the xbox_elite_wireless_xinput branch from e3a67ec to 19ce63d Compare June 9, 2023 13:02
@mrTag mrTag changed the title Fix Xbox Elite Wireless gamepad detected as 2 devices Added a few device GUIDs to is_xinput_device fixing controller problems Jun 9, 2023
@akien-mga akien-mga added cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Jun 9, 2023
@akien-mga akien-mga merged commit 859b02e into godotengine:master Jun 9, 2023
@mrTag mrTag deleted the xbox_elite_wireless_xinput branch June 9, 2023 13:38
@akien-mga
Copy link
Member

Thanks! Technically your first merged contribution as far as GitHub is concerned, but I know you've done a ton to help fix issues on threaded resource loading already. Still, congrats for your first merged PR ;)

@mrTag
Copy link
Contributor Author

mrTag commented Jun 9, 2023

Thanks 😊

@akien-mga
Copy link
Member

Cherry-picked for 3.6.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Aug 28, 2023
@akien-mga
Copy link
Member

Cherry-picked for 3.5.3.

@akien-mga akien-mga removed the cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release label Aug 28, 2023
@akien-mga akien-mga changed the title Added a few device GUIDs to is_xinput_device fixing controller problems Added a few device GUIDs to is_xinput_device fixing controller problems Sep 8, 2023
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.

Xbox bluetooth controller detected as 2 devices, triggering 2 events instead of 1
3 participants