[Windows,Linux] [Input] Return the keyboard ID for raw input#110114
[Windows,Linux] [Input] Return the keyboard ID for raw input#110114knn217 wants to merge 1 commit intogodotengine:masterfrom
Conversation
core/input/input_event.cpp
Outdated
| return physical_keycode; | ||
| } | ||
|
|
||
| void InputEventKey::set_keyboard_id(int64_t p_id) { |
There was a problem hiding this comment.
It would make more sense to have device_id (or use it with existing device, but this will require more changes) in the base InputEvent, the same can be used for mouse as well.
For the reference, native device ID on macOS can be obtained from keyboard/mouse events, using the following:
int64 _get_device_id(NSEvent *p_event) {
return CGEventGetIntegerValueField([p_event CGEvent], /*kCGEventRegistryID*/ CGEventField(87));
}There was a problem hiding this comment.
Ah, that makes sense.
Should've look deeper into the parents.
Do I need to wait until check flows are completed to push?
There was a problem hiding this comment.
Do I need to wait until check flows are completed to push?
No, it will cancel currently running tasks if you push.
There was a problem hiding this comment.
I updated to use device instead, thanks for the review
|
Simply using diff --git a/core/input/input_event.h b/core/input/input_event.h
index 62b89bf0fd..6fc31b802f 100644
--- a/core/input/input_event.h
+++ b/core/input/input_event.h
@@ -52,7 +52,7 @@ class Shortcut;
class InputEvent : public Resource {
GDCLASS(InputEvent, Resource);
- int device = 0;
+ int device = -1; // ALL_DEVICES
protected:
bool canceled = false; |
b302b38 to
bf40ac8
Compare
|
I looked into InputMap and it seems safer to only use ALL_DEVICE for input mapping, since keyboard ID can change when user unplug -> plug keyboard back in, which invalidates the mapping if we use keyboard ID (device)
I made the change and did not find any issue. |
bf40ac8 to
b4613af
Compare
b4613af to
f46c9a3
Compare
|
@knn217 In your first screenshot there are three diferent keyboard ids. Is this because you really plugged 3 different keyboards to test, or perhaps windows simply did changed the keyboard id in between keystrokes? Also, does the windows api expose some way of knowing the device ordering? to know what is the first, the second, the third keyboard and os on? the device ids for joypads that godot returns are always ordered starting from zero, so that if you have 4 joypads they will have device ids 0, 1, 2 and 3. Is it possible to at least emulate this with keyboards? even if the OSs api don't tell you the keyboard ordering, couldn't we store a dictionary mapping the raw keyboard ids returned by the OS into a zero based ordered id? even if we just make up this ordering ourselves? for example, if windows returns ids 687, -94601 and 12687 whe order them ascendingly and give them the ids 0 (for -94601), 1 (for 687) and 2 (for 12687). |
Windows can change the keyboard ID (handle), but only when user unplug -> plug keyboard back in. The ID stays the same while plugged in though.
I have not found any API like that.
I think it's possible to emulate this behavior, by using this API: Here's how I understand the flowchart should be implemented from your description:
The last step is to solve the issue with laptop's keyboards being always plugged in, and also any keyboards that are already plugged in. |
|
I have added another commit to support:
Test:
*NOTE:
|
7ec48dc to
4723971
Compare
4723971 to
f444993
Compare
|
I have added another commit to support:
Future enhancement
|
Hi, Wayland should support this behavior already. Compositors can expose multiple |
I see, but is there a way to make the compositor expose seats from Godot? Compositors are outside of application's control as I understand. |
Hi, I'm not really sure what you mean. Godot can't play with input settings as they're non-standard and outside of the scope of the Wayland protocol. I also don't know how other compositors outside of sway handle it, they could as well make a new seat every time, it's up to them. If the compositor is configured (by default or by the user) to expose multiple input devices, godot will be able to detect and use all of them. In fact, we already do that, we already have logic for handling multiple input devices as we're required to do that. |
|
Support for Wayland seems too complicated for me currently, so I will leave the PR here for now.
@deralmas |
Yeah that's fine, don't worry about that :D
Hi. To be clear, Wayland works this way but we don't expose this distinction to the Godot game, since there are no public Godot APIs. I can make a rough overview of the internal Wayland communication if you'd like. Regarding config, as I said I only know about sway, which has commands like Let me know if you need more info, I'm glad to help. |
|
@deralmas GDscript for test: Here's what I added in my sway config: Here's the seats from: Does it work on your env? |
|
Hi @knn217, are you using the Wayland backend? You might be running under XWayland, which is the default right now. You can change the backend with the editor setting "Prefer Wayland" for the editor, the "display server" project setting for the running game or simply by running the engine binary with the |
|
@deralmas I also used
Please help me test this on your local if possible. |
|
Hi, sorry for the delay. I think that the project might be still running under X11. Note that passing that flag will only affect the editor, unless you run a game directly from the command line. To run a project under Wayland, you'll need to change the linuxbsd display driver property in the project settings. They are separated to allow a developer to use Wayland while exporting to X11, for stability reasons. |
f444993 to
4f1bd1c
Compare
4f1bd1c to
4641836
Compare
|
@deralmas Thanks for the help, I was able to run the project under wayland now I have updated the code and it now works on both X11 and Wayland. Here's
The code for indexing device id is in InputEvent, and the current implementation hides the raw device ID from users, it only return the index of the device (Since I'm not sure if its safe to expose the raw ID) |
|
Awesome!
Depends on what you mean by "safe". It's just a int, users can't do much with it. Now, if the desired API must have sequential IDs then yes, you indeed need to do some housekeeping. That said from the Wayland side of things I can say that using the name of each API-wise I wonder, if the raw ID is hidden from the user, why expose the relevant methods? I think that you can simply keep them internal by not binding them in ClassDB. It becomes an implementation detail FWIW. For everything else I'd need to make a proper review, so I can't vouch for the approach in its entirety. |
Good point, I will remove them in the next push, when resolving PR reviews. This also remind me that get_active_devices does reveal the raw device id, I forgot to remove this function |
…from multiple keyboards (seats in Wayland case)
4641836 to
45b3967
Compare
|
Hello! We recently introduced separate device IDs for keyboard and mouse (see #116274 ), and I think it might make sense for this PR to use IDs 16-31 for keyboards. What do you think? :) |
| protected: | ||
| bool canceled = false; | ||
| bool pressed = false; | ||
| static inline AHashMap<int, bool> device_index; // bool is to mark a device ID as inactive, available for overwriting |
There was a problem hiding this comment.
I feel like the new functionality in this PR should be inside of Input class instead of InputEvent, since joypads are also registered inside Input.






Based on #13083
devicedevicewill be normalized toALL_DEVICE,devicevalue for input map id safer, since raw input's id can change in runtime by unplugging/plugging back the keyboarddeviceto tell which keyboard the event is from, and handle hot-remapping if the ID was changed in runtimedeviceis now ordered starting from 0, and registered by plugin/input. Unplugging a keyboard will leave itsdeviceopen for a new keyboard to register toTest with GDScript:
Test with GDExtension:
Test input mapping:
Tested action created from GDScript, with an event for pressing key right, the action triggered with multiple keyboard ids (0, 1)
Tested with an action from the input map setting
Test on Linux X11 (Ubuntu 24.04):
