Add device parameter to input action methods

[Carsonthemonkey]

Implementation of 10852
Also would help with 3555

Currently, using the Input singleton, actions can only be queried for all devices. This is problematic for local multiplayer games, as these rely on processing different inputs for each connected device separately. This PR adds an optional device argument to the following Input methods:

• is_action_pressed
• is_action_just_pressed
• is_action_just_released
• get_action_raw_strength
• get_action_strength
• get_axis
• get_vector

This allows the user to filter action queries per device. For example,

const DEVICE_ID = 1
if Input.is_action_just_pressed("jump", false, DEVICE_ID):
    jump()

This parameter is optional and placed after existing ones to ensure backwards compatibility. By default the device parameter uses InputMap::ALL_DEVICES to represent the normal behavior of querying all devices.

NOTE: This clashes with DEVICE_ID_EMULATION, in Godot's current state, but is fixed by #97707

• Bugsquad edit, closes: Add device parameter to Input action methods godot-proposals#10852

[groud]

I am not super convinced this is the right approach to this. Having a per-player or per-device ID kind of goes again the simple design of the Input mapping API. The common approach is usually to suffix actions with a player ID, like this:

player_jump_0 -> player 0 jumps
player_attack_0 -> player 0 attacks
player_jump_0 -> player 1 jumps
player_attack_0 -> player 1 attacks
...

This allow remapping each user to a new device if needed.

I am not strictly about having a player ID / device ID as an argument to those functions, but I think they should still be separate actions in the action mapping settings.

[Carsonthemonkey]

I'm not sure I fully understand your concern. I mentioned this in 10852, but I think the common workaround of defining an action separately per device is a poor solution to this. For one it requires manually setting up the same action multiple times, which is error prone and not very maintainable. It means that you have multiple sources of truth for what each button does which very much violates the DRY principle. Secondly, to actually evaluate if these actions are pressed, the programmer must do some string concatenation to query the action. For example, here is the equivalent code to what I have above using the multiple actions solution.

const DEVICE_ID = 1
if Input.is_action_just_pressed("jump_" + str(DEVICE_ID)):
    jump()

Besides the difficulties with setting up your action map for each device separately you must also do a string concatenation in the function call and lose out on autocomplete for the action name (as jump_ is not the full name of an action itself).

This allow remapping each user to a new device if needed.

I think I don't understand what you are saying here. In this PR, remapping the user to a new device is as simple as changing the device id passed to the input action functions. Furthermore, in the above example using string concat, the solution would be just changing the device id as well. I don't see what additional advantage multiple actions afford. Can you clarify this a bit more?

Thanks for the feedback!

[groud]

I think I don't understand what you are saying here. In this PR, remapping the user to a new device is as simple as changing the device id passed to the input action functions. Furthermore, in the above example using string concat, the solution would be just changing the device id as well. I don't see what additional advantage multiple actions afford. Can you clarify this a bit more?

Mapping a device ID to a user isn't as straightforward either, especially when a device disconnects/is replaced by another. Assuming the device ID can be mapped to a user is wrong. Like, the role of an action mapping is to "abstract" the device behind it, whether it is a keyboard, a joypad, or anything else.

In any case, your code cannot work to distinguish users anyway as the keyboard device ID is always 0, same for the mouse, and the first gamepad. The ID only changes for mapped events of the same type (like, it will change for two different gamepads). That means you will not be able to distinguish your users if one of them plays with the keyboard and another one plays with a gamepad (the device ID will be the same).

As a consequence, the clean solution to this problem (if we want to avoid the string concatenation issue), is to modify the editor UI / Action event mapping API to allow specifying a player ID too. The internal key to map events would then include both the action name AND player ID, so you could assign different action to each player if you need. It's likely a bit tricky to get the UI right though.

Along with that, the mapping API could maybe benefit for a simpler way to remap whole sets of controls from a players to another (like, changing the device ID for all events in a player's mapping. Or replacing all keyboard mapping by a gamepad one). But it's likely not a simple task.

Maybe a solution would be to be able to store the action to event mapping as a resource, with the possibility to provide an optional device ID there ? And then make everything swappable easily? I don't really know, this needs more thinking.

[Carsonthemonkey]

Ah I see what you are saying. Having a player ID system separate from the device identifiers could make sense. Certainly this is not a perfect solution to local multiplayer, but I do still think that having the ability to filter by device in these methods is helpful and would get us some of the way there. A similar capability on InputEvent exists, although it misses the functionality of, is_action_just_pressed, get_vector, etc. so this PR is aims to make this much easier to handle. I'll be interested to hear more feedback and ideas on the implementation of this issue.

[groud]

Ah I see what you are saying. Having a player ID system separate from the device identifiers could make sense. Certainly this is not a perfect solution to local multiplayer, but I do still think that having the ability to filter by device in these methods is helpful and would get us some of the way there. A similar capability on InputEvent exists, although it misses the functionality of, is_action_just_pressed, get_vector, etc. so this PR is aims to make this much easier to handle. I'll be interested to hear more feedback and ideas on the implementation of this issue.

Well, my opinion on the matter is that exposing the device ID here is just going against the base idea of abstracting the input event behind it. As I kind of implied in my message, if you start to expose the device ID, it makes as mush sense to expose the type of event too (if you want to distiguish two devices, then that's what you need to do it). At that point, then it simply makes no sense to use the input mapping system, and just use events directly instead. So well, for me merging this kind of makes no sense.

[coffeebeats]

Apologies for the drive-by comment:

@groud the issues you've noted here are identical to what I've worked through in godotengine/godot-proposals#10070. Just as you argued, we could introduce a player ID concept, a means to map (device, device_index) tuples to player IDs, and update Input-related methods to accept a bitmask of what players to check for (which offers backwards-compatibility + flexibility).

It's important to note that, in making these additions, no changes are required by developers making single player games: Input maps can still be defined just once and the new (device, device_index) tuple to player ID mapping can use defaults to match existing behavior. Because the rest of that proposal builds on this solution, I strongly agree with @Carsonthemonkey that Input-related methods need to allow a developer to distinguish between players (and I feel that checking for player IDs wouldn't break the Input abstraction).

godotengine/godot-proposals#10070 proposes additional changes which would provide multi-focus support for local multiplayer, but the first half of it, in isolation, would work to solve for all of our requirements here.

I'd love to see a solution to this problem implemented, so I'd be more than happy to collaborate with whomever to help see these changes through. Additionally, I don't think the nice work @Carsonthemonkey has done here conflicts with that I've proposed - it would just offer more flexibility to developers.

[groud]

I strongly agree with @Carsonthemonkey that Input-related methods need to allow a developer to distinguish between players (and I feel that checking for player IDs wouldn't break the Input abstraction).

Oh I don't disagree with that. I think having a way to specify a player ID makes sense for input methods (all action needing an action name as a parameter will need an additional player ID parameter). But, as I mentioned, a player ID is not equivalent to a device ID, which is what is used here.

Additionally, I don't think the nice work @Carsonthemonkey has done here conflicts with that I've proposed - it would just offer more flexibility to developers.

I mean, I explained in my last comment why exposing the device ID like it's done here does not really help, so I would avoid adding a parameter here that isn't really solving the problem.

In any case, I'll try finding a better way to solve the issue and open a proposal.

[coffeebeats]

@groud can you comment on the proposal I linked above? I feel it includes a nearly identical solution to what you've proposed.

[groud]

@groud can you comment on the proposal I linked above? I feel it includes a nearly identical solution to what you've proposed.

No my proposal is different, it should be another one. Some things are very similar though. I've opened it there: godotengine/godot-proposals#10887

Edit: my proposal does not cover focus for now. It's mainly about remapping/identifying different players.