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

Add the ability to get per-platform information for joypads #78539

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

EIREXE
Copy link
Contributor

@EIREXE EIREXE commented Jun 21, 2023

This adds the ability for games to obtain platform-specific information about joypads such as their vendor/product ID, their XInput gamepad index or the real name of the device before it gets swapped out by the gamecontrollerdb's name.

The rationale behind this being returned as a Dictionary is that there's a big disparity between input and OS APIs, so it makes more sense to have a single "extra info" function.

This PR also includes a rebased version of #76045, this is because this PR is intended to be mainly to help implementing Steam Input glyphs, as some information needed for that is not accessible in the engine normally.

@EIREXE
Copy link
Contributor Author

EIREXE commented Jun 22, 2023

Updated with changes suggested by @RandomShaper on the original PR

@EIREXE EIREXE force-pushed the input-info branch 2 times, most recently from a43c172 to 35da933 Compare June 28, 2023 14:52
Copy link
Member

@RandomShaper RandomShaper left a comment

Choose a reason for hiding this comment

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

Looks good!

NOTE: This will have to wait until #76045 is merged as it may need a new rebase.

@EIREXE
Copy link
Contributor Author

EIREXE commented Jun 28, 2023

Sure, i'll wait

@EIREXE
Copy link
Contributor Author

EIREXE commented Jul 16, 2023

@RandomShaper I rebased it, not sure why the test with ASAN fails, is it an issue with CI or is there something I did wrong?

@AThousandShips
Copy link
Member

AThousandShips commented Jul 16, 2023

It's a ci/buildsystem bug, it's being tracked :)

@RandomShaper
Copy link
Member

Good to go then!

@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Jul 25, 2023
doc/classes/Input.xml Outdated Show resolved Hide resolved
@EIREXE
Copy link
Contributor Author

EIREXE commented Jul 27, 2023

Should be good now.

@YuriSizov
Copy link
Contributor

@EIREXE I know this seems minor, but could you please be more careful with formatting? Platform names must be correctly capitalized in your newly added documentation. You also missed one instance of that in my suggestion for "Steam Input", which is a proper name.

It's best to make sure these don't slip into your PRs to begin with, as we will point them out and it will add to your work fixing and rebasing those, and will make the PR process longer for every side. Thanks 🙏

@EIREXE
Copy link
Contributor Author

EIREXE commented Aug 2, 2023

@EIREXE I know this seems minor, but could you please be more careful with formatting? Platform names must be correctly capitalized in your newly added documentation. You also missed one instance of that in my suggestion for "Steam Input", which is a proper name.

It's best to make sure these don't slip into your PRs to begin with, as we will point them out and it will add to your work fixing and rebasing those, and will make the PR process longer for every side. Thanks 🙏

Yeah sorry, I'm very forgetful and easily distracted, i'll take a look.

This adds the ability for games to obtain platform-specific information about joypads such as their vendor/product ID, their XInput gamepad index or the real name of the device before it gets swapped out by the gamecontrollerdb's name.

This PR also includes a rebased version of godotengine#76045, this is because this PR is intended to be mainly to help people implementing Steam Input, as having the gamepad index is essential.
@YuriSizov YuriSizov merged commit 1610fc2 into godotengine:master Aug 3, 2023
14 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

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.

6 participants