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

Fix guide button detection with XInput and Xbox Series controllers #73200

Merged
merged 1 commit into from
Apr 3, 2023

Conversation

0xafbf
Copy link
Contributor

@0xafbf 0xafbf commented Feb 13, 2023

This PR lets Godot detect the guide button when running on windows and using the XInput API.
Changes the call of the XInputGetState function with some undocumented function that returns the same thing, but with an extra bit that corresponds to the guide button state.
see https://github.com/higan-emu/higan/blob/master/ruby/input/joypad/xinput.cpp as an example for another place where they use the undocumented feature
I think this feels kinda hacky, so let me know if I should fix it with better naming, some documentation comments or maybe put it behind a compile flag.

The PR also adds the guide button to the controller db for the Xbox Series gamepad (maybe works with others, didn't test)

@0xafbf 0xafbf requested review from a team as code owners February 13, 2023 05:39
@akien-mga akien-mga modified the milestones: 4.x, 4.1 Feb 15, 2023
@akien-mga
Copy link
Member

It seems a bit risky to me to rely on undocumented features by casting an integer. Any Windows SDK update might break it and render Godot games unusable with XInput, which is likely worse than not supporting the Guide button.

I'm surprised there wouldn't be a public API available to handle this button.

@0xafbf
Copy link
Contributor Author

0xafbf commented Feb 17, 2023

I had similar thoughts when I implemented this. I think that a Windows SDK wouldn't break it because these functions are loaded at runtime (although I'm not an expert at how dynamically loaded function works). I also don't think that Windows will change their API or the implementation, as this is something that many people rely on, I think that Steam uses the same API to trigger their overlay.

The case where I think could be an issue is if the same XInput code is used for the Xbox platform or when compiling for UWP. Maybe those targets don't have this function. For that case, we could handle it by adding the code behind a compiler define. We can also make it ask for the original XInputGetState function if the GetProcAddress((LPCSTR)100) didn't return any result.

This API is not documented in MS docs, but it seems fairly well known in many other projects. I don't think it will go away anytime soon, or ever at all.

Still, I share the feeling this feels like a hack :P, a hack that everyone else seems to have accepted.

@akien-mga akien-mga merged commit 7aa2242 into godotengine:master Apr 3, 2023
@akien-mga
Copy link
Member

Thanks!

@0xafbf 0xafbf deleted the fix-guide-button-win branch April 4, 2023 14:04
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.0.3.

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.

4 participants