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

Prevent double input events on gamepad when running through steam input #76045

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

Eoin-ONeill-Yokai
Copy link
Contributor

@Eoin-ONeill-Yokai Eoin-ONeill-Yokai commented Apr 14, 2023

Fixes #75480

The Problem

During GDC2023 and when generally testing on Steam Deck units, we found that a single gamepad would often register inputs twice under certain circumstances. This was caused by SteamInput creating a new virtual device, which Godot registers as a second gamepad. This resulted in two gamepad devices reporting the same button presses, often leading to buggy input response on games with no multi-device logic or otherwise could cause Steam controller rebindings to not work as intended (for example, swapping o and x on a PlayStation pad if that feature isn't supported by the game.)

SDL2 gets around this by taking in a list of devices that are to be ignored via system environment. When valve sees a controller that wants to be routed through SteamInput, they push a new VID/PID entry onto the environment variable SDL_GAMECONTROLLER_IGNORE_DEVICES matching the source gamepad so that the SDL based games will only read inputs from the virtual device remappings.

Proposed Solution

This PR fixes this issue by leveraging the same logic that SDL uses. As we are already using SDL gamepad related HID mappings, there shouldn't be much harm in leveraging another environment variable for usb device ignoring capabilities.

This feature is currently only being used on LinuxBSD platform, but can be applied to other platforms that need it due to the implementation in the Input class.

Considerations

  • This environment variable parsing and should_ignore_device api could be moved to another class. For example, they could be stored in JoypadLinux/JoypadOSX/JoypadXXX if we determine that we only really need this functionality for a few select platforms.
  • It's very likely that this logic will also be needed on OSX as SDL2 does so on that platform as well. I don't have a computer to test this so I left it unimplemented. In theory, JoypadOSX would also simply call should_ignore_device before registering controller connection to determine whether or not the device should be ignored.
  • Windows supposedly doesn't need to take this into account, as Valve is doing their own thing on that platform with regards to virtual game inputs. Having said that, if it is determined that Windows does need this, it should also use a pretty similar solution to what's provided here.

Testing Procedure

Collapsed for ease-of-reading

Reproduction, Verification

  • Download the attached testing project
  • Get vanilla Godot 4.0 template_debug export target
  • Verify that only 1 A Button press occurs when running the testing project standalone (no steam)
  • Export the testing project as a game via vanilla 4.0
  • Load the game via Steam, make sure that Steam Input is enabled for your device (on Xbox/Nintendo controllers, this often needs to be done manually.)
  • Observe when you press the A button (or X Button on PS layout, or the B Button on Nintendo controller layout) that there's two button presses registered on a single frame for the single gamepad.

Testing

  • Compile this branch, both editor and template_debug export target.

  • Verify that only 1 button press occurs when running the testing project standalone (no steam)

  • Load the testing project in the editor and export.

  • Reload the game via Steam, making sure to use the same Steam Input settings as the reproduction.

  • Observe when you press the A button (or X button PS layout, B button Nintendo layout) there's now only one button press registered for the single gamepad.

  • Controller input should work when running Godot without steam input

  • Controller input should work when running Godot through steam input, with only one button press registered per button.

  • Controllers should be rebindable via SteamInput when the user wants to take advantage of system-level customization features (for example, swapping buttons or axis.)

bug75480.zip

core/input/input.cpp Outdated Show resolved Hide resolved
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 fine, beside the little things. If no one can confirm about Mac in the short term, we can just merge and iterate if news finally come on that later.

doc/classes/Input.xml Outdated Show resolved Hide resolved
core/input/input.h Outdated Show resolved Hide resolved
core/input/input.cpp Outdated Show resolved Hide resolved
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.

Regarding the suggested changes, TIWAGOS, but I believe they're good in spirit.

@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 22, 2023
@akien-mga akien-mga added cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release and removed cherrypick:4.0 labels Jun 22, 2023
EIREXE added a commit to EIRTeam/shinobu that referenced this pull request Jun 22, 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.

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.
doc/classes/Input.xml Outdated Show resolved Hide resolved
doc/classes/Input.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

Docs look good now. Just a couple of nitpicks.

During GDC and general testing on Steam Deck units, we found that single
gamepads would often register inputs twice under certain circumstances.
This was caused by SteamInput creating a new virtual device, which Godot
registers as a second gamepad. This resulted in two gamepad devices
reporting the same button presses, often leading to buggy input response
on games with no multi-device logic and other-wise could cause intended
Steam rebindings to not work as intended (for example, swapping o and x
on a playstation pad if that feature isn't supported by the game.)

SDL gets around this by taking in a list of devices that are to be
ignored. When valve sees a controller that wants to be rebound via
SteamInput, they push a new VID/PID entry onto the environment
variable `SDL_GAMECONTROLLER_IGNORE_DEVICES` for the original gamepad
so that all game inputs can be read from the virtual gamepad instead.

This leverages the same logic as we are already using SDL gamepad
related HID mappings.
@Eoin-ONeill-Yokai
Copy link
Contributor Author

@YuriSizov Thanks again, went ahead and fixed the last remaining problems.

@YuriSizov YuriSizov merged commit 55b74c7 into godotengine:master Jul 12, 2023
@YuriSizov
Copy link
Contributor

Thanks!

@puzzud
Copy link

puzzud commented Jul 20, 2023

If no one can confirm about Mac in the short term, we can just merge and iterate if news finally come on that later.

@RandomShaper, as I have been able to reproduce from my Linux desktop as well as a Steam Deck, I was (surprisingly) unable to reproduce from my Mac.

I only tested against branch 3.x. But given what I've seen of both versions, it's likely the same behavior for 4.x.

EIREXE added a commit to EIRTeam/shinobu that referenced this pull request Aug 2, 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.

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.
IntangibleMatter pushed a commit to IntangibleMatter/godot that referenced this pull request Aug 13, 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.

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 removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Sep 21, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.2.

mandryskowski pushed a commit to mandryskowski/godot that referenced this pull request Oct 11, 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.

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.
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.

Controller events detected as InputEventKey when Steam is open (due to Steam Input event injection)
5 participants