godot
godot copied to clipboard
Get joypad's vendor ID, product ID and name on Windows for XInput devices.
Currently, XInput devices (Windows only) lack info on vendor, product and its name. DirectInput also does not provide vendor and product. This PR is to fix that.
TODO
- [x] vendor ID (both)
- [x] product ID (both)
- [x] name (XInput only)
NOTES
- Previously, all XInput joypads had the name
XInput Gamepad, now they have proper names - I'm stripping "Controller (…)" from the name. Also for DirectInput devices. There's no reason for keeping this redundancy.
Ideas for alternative implementation
- using
joyGetDevCapsW, therefore loading Winmm.dll- I haven't tested it, but it should also be possible to get product and vendor ID using
GetRawInputDeviceInfoAorDIDEVICEINSTANCE, basically making loading Winmm.dll obsolete. If I should check it out, I will. I'm not doing that right now, because I'm lacking background logic of the whole device mess (these APIs are all deprecated anyway...)
- I haven't tested it, but it should also be possible to get product and vendor ID using
- I also could have generated a guid, as the DirectInput code does, but I cannot verify if they are correct…
sprintf_s(uid, "%04x%04x%04x%04x%04x%04x%04x%04x", BSWAP16(0x03), 0, jc.wMid, 0, jc.wPid, 0, 0, 0);generated03000000045e0000028e000000000000and03000000046d0000c21f000000000000
Testproject for this PR:
xinput_info.zip
output of Testproject on Windows
OK, so the Godot build artifact for Windows crashes on startup. As it's not a debug build, there's no stacktrace. And I can't get Godot to build on Windows, because it just can't find mingw (yes, I followed Godot's build instructions).
I'll try finding help on Rocket Chat/Discord over the weekend. If nothing helps, I'll try cross-compilation from Linux...
@MJacred 64-bit Windows editor binary of this PR with MinGW debug symbols (in a separate file): https://fromsmash.com/Godot-PR-test-GH-98861
@Calinou: I think I found something: "3221225477", which is apparently an access violation..
As Godot only crashes when I plugin an XInput device, I suspect the issue lies in JoypadWindows::probe_joypads. I noticed I did a sizeof(jc) instead of sizeof(JOYCAPS) (fixed that just now), but ~~I don't think that was it~~ (it wasn't). It might be String(jc.szPname), where szPname is a char[32]...
This is the command I used
gdb --symbols=godot.windows.editor.x86_64.exe.debugsymbols --exec=godot.windows.editor.x86_64.exe
but I got
Reading symbols from C:...\godot.windows.editor.x86_64.exe.debugsymbols...
C:...\godot.windows.editor.x86_64.exe.debugsymbols: Invalid argument.
symbols argument is optional, it will find and read the file. But it will deem it invalid.
Starting Godot with XInput device plugged in
found info on error code
Starting program: C:...\godot.windows.editor.x86_64.console.exe
[New Thread 14628.0x315c]
[New Thread 14628.0x170c]
[New Thread 14628.0x3bc4]
Godot Engine v4.4.dev.custom_build.949f572c7 (2024-11-05 20:27:39 UTC) - https://godotengine.org
================================================================
CrashHandlerException: Program crashed with signal 11
Engine version: Godot Engine v4.4.dev.custom_build (949f572c70cfc15b0711ba23bd745874d8038d2f)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] error(-1): no debug info in PE/COFF executable
[2] error(-1): no debug info in PE/COFF executable
[3] error(-1): no debug info in PE/COFF executable
[4] error(-1): no debug info in PE/COFF executable
[5] error(-1): no debug info in PE/COFF executable
[6] error(-1): no debug info in PE/COFF executable
-- END OF BACKTRACE --
================================================================
[Thread 14628.0x170c exited with code 3221225477]
[Thread 14628.0x3bc4 exited with code 3221225477]
[Thread 14628.0x315c exited with code 3221225477]
Program terminated with signal SIGSEGV, Segmentation fault.
The program no longer exists.
Starting Godot, then plugging in XInput device
meaning of error code unknown
Starting program: C:...\godot.windows.editor.x86_64.console.exe
[New Thread 6944.0x2ba0]
[New Thread 6944.0x2a88]
[New Thread 6944.0x920]
Godot Engine v4.4.dev.custom_build.949f572c7 (2024-11-05 20:27:39 UTC) - https://godotengine.org
OpenGL API 3.3.0 NVIDIA 560.94 - Compatibility - Using Device: NVIDIA - NVIDIA GeForce GTX 970
================================================================
CrashHandlerException: Program crashed with signal 11
Engine version: Godot Engine v4.4.dev.custom_build (949f572c70cfc15b0711ba23bd745874d8038d2f)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] error(-1): no debug info in PE/COFF executable
[2] error(-1): no debug info in PE/COFF executable
[3] error(-1): no debug info in PE/COFF executable
[4] error(-1): no debug info in PE/COFF executable
[5] error(-1): no debug info in PE/COFF executable
[6] error(-1): no debug info in PE/COFF executable
[7] error(-1): no debug info in PE/COFF executable
[8] error(-1): no debug info in PE/COFF executable
-- END OF BACKTRACE --
================================================================
[Thread 6944.0x920 exited with code 3221226525]
[Thread 6944.0x2a88 exited with code 3221226525]
[Thread 6944.0x2ba0 exited with code 3221226525]
[Inferior 1 (process 6944) exited with code 030000002035]
OK, the good news: I fixed my compiling on windows. The bad news is: gdb still reads the *.debugsymbols file as an Invalid argument (both from what @Calinou provided, and my own build), so the debugger isn't really helpful right now. Is GNU gdb (GDB) 11.2 (2022) too old?
Apart from that, I get this warning (even if no XInput device gets plugged in), currently I don't think that is caused by me (but I'll check later):
warning: clientcore\windows\dwm\dwmapi\attribute.cpp(185)\dwmapi.dll!00007FFC17C53647: (caller: 00007FF73D6FE436) ReturnHr(1) tid(1864) 80070057 The parameter is incorrect.
warning: clientcore\windows\dwm\dwmapi\attribute.cpp(185)\dwmapi.dll!00007FFC17C53647: (caller: 00007FF73D707AF4) ReturnHr(2) tid(1864) 80070057 The parameter is incorrect.
Current status: it would work if I used joyGetDevCaps directly, like midiInGetDevCaps from drivers\winmidi\midi_driver_winmidi.cpp. So... I'm doing sth. wrong regarding joyGetDevCaps_t. No idea what exactly...
So help here is more than appreciated.
And the result I got when using the func directly was this:
As you can see, the device name JOYCAPS.szPname is disappointing. I know that Windows knows the name. While the device manager has no clue, the game controllers settings do.
There's also
JOYCAPS.szOEMVxD: EMPTYJOYCAPS.szRegKey:DINPUT.DLL(quite funny, actually)
Right now, I see 2 options on how to get the name:
- Make use of
JoypadWindows::enumCallback-> with winmm, we get product and vendor ID; then fetch them again using DirectInput, wich also gets us the name (additionally to the same IDs). Then simply map back.- Unless DirectInput stays consistent with winmm and renders this approach useless. I have no DirectInput device to test their output
- build a
guid, like DirectInput does, and letInput::joy_connection_changedget the name using the internal gamecontrollerdb. - Reading joypads as devices and then using some generic Windows api for reading device name (should work, but right now no idea how)
Switched to fetching the func joyGetDevCapsW instead of the macro joyGetDevCaps, because it seems it's impossible with the macro. As *W and *A funcs are used in the engine all over the place, I'll stick to *W here.
I'll tackle getting the name next ~~(but not this weekend)~~.
OK, I got the name - ready for review
Applied all requested changes. Thanks for the feedback, everybody!
What do you all think about the change I mentioned in the issue's description regarding changing the joy name for XInput devices? I.e. not adding xinput_name to the dictionary. The "name" XInput Gamepad is completely redundant. We already have the XInput "GUID" __XINPUT_DEVICE__
As an example, for "Xbox Elite Series 2 (wired)", you would get
Xbox One For Windows: { "xinput_index": 0, "vendor_id": "1118", "product_id": "767" }
instead of
XInput Gamepad: { "xinput_index": 0, "vendor_id": "1118", "product_id": "767", "xinput_name": "Xbox One For Windows" }
This would make the whole thing more coherent not only with DirectInput joypads, but also with the other OS.
@bruvzg: Could you take a look at this PR, please? And also give feedback regarding the proposal to not override the joy names for XInput devices to "XInput Gamepad", but use the real name which we can now fetch (see here).
And also give feedback regarding the proposal to not override the joy names for XInput devices to "XInput Gamepad", but use the real name which we can now fetch (see https://github.com/godotengine/godot/pull/98861#issuecomment-2470513052).
Fully agree with it, XInput Gamepad as a name is redundant, and using actual name is better. I'll test PR tomorrow, code looks fine.
Regarding testing the PR: I replaced the test project in the issue description with one that reflects the latest change (i.e. XInput joypad name).
I also noticed just now that the name set in the mapping db replaces the name coming from the OS/driver in joy_connection_changed(…, p_connected = true, …). Which makes sense as it improves naming consistency across platforms - though it won't work for xinput devices and (possibly) devices for regular xr on android (which makes use of fallback mapping).
Anyway… Input::add_joy_mapping(…, p_update_existing = true) didn't replace the joypad's name to reflect the new mapping, even though it acts similar to joy_connection_changed. Which is a bug.
Long story short: I added _set_joypad_mapping to handle mapping changes for a specific Joypad in one place.
Thanks!