Add device parameter to Input action methods

[Carsonthemonkey]

Describe the project you are working on

A local multiplayer game relying on multiple controller support.

Describe the problem or limitation you are having in your project

While is_action_pressed, is_action_just_pressed, get_vector, is_action_just_released are convenient for single player games, they don't work well for local multiplayer games, as they query all input devices.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Adding the ability to optionally specify the desired device id in the various input action methods would allow for querying specific devices while keeping the convenience of the input action methods.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

An intuitive interface for this would be adding an optional device parameter to the various action methods. If you wanted to check for input actions on a specific device, you could simply pass it the device id you are interested in. This could be placed at the end of existing arguments for compatibility.

Example usage:

const PLAYER_DEVICE = 1
var input_dir = Input.get_vector("move_left", "move_right", "move_forward", "move_back", -1.0, PLAYER_DEVICE)
if Input.is_action_just_pressed("jump", false, PLAYER_DEVICE):
    jump()

the device parameter would be optional, so querying all devices would be the same as usual:

# Device agnostic
if Input.is_action_just_pressed("jump"):
    jump()

The device input issue has been discussed #10070, #3555, #4295, and #10349, among others but this specific proposal is a simpler and narrower proposal that aims to alleviate this pain point without breaking compatibility.

If this enhancement will not be used often, can it be worked around with a few lines of script?

This will be used often for local multiplayer games, and will not affect the simplicity of the Input API for single player games. One workaround to this are defining input actions separately for each device (jump_player1, jump_player2, etc.) This is tedious and can quickly become unmaintainable if your game has many controllers and/or many actions. Another common solution is to handle these inputs in the _input function, as InputEvents have a device attribute that can be checked. This works, but requires more complex code to manage analog directional input (get_vector is not available on InputEvent, and thus this requires tracking state for directional inputs.)

Is there a reason why this should be core and not an add-on in the asset library?

This is an improvement to make the built in Input methods more useable for local multiplayer, which is likely a common use case.

[CarpenterBlue]

I feel like this is overkill.
There should simply be Input.get_device()
So the user can filter by it.

if Input.get_device() != player_device: return

Edit: reconsidered, commented better solution.

[Carsonthemonkey]

There should simply be Input.get_device()

So the user can filter by it.

Hm I'm not sure I understand how this would work. How would this method determine what device id should be returned?

[CarpenterBlue]

Actually gave it more thought, that wouldn't work with multiple Devices.
var filtered_input := Input.filer_input_by_device(device)
Returns input from the input pool that only fits the device in question.
now you don't have to specify another argument, you just paste it before your Input and change all the Input.is_action_pressed() to filtered_input.is_action_pressed()

[Carsonthemonkey]

I see the convenience of this pattern, but I'm not convinced this should exist at the engine level. This could be implemented as a GDScript wrapper class over the revised input action methods, which would keep core changes minimal.

[coffeebeats]

Hi there - from reading this issue it seems to me that this solution (while definitely a good idea!) is captured in #10070. Would you be able to elaborate on the differences you see between these two proposals?

The key difference I see is that #10070 proposes using a new "player ID" instead of the existing device ID for the various input methods. Using a "player ID" allows for more flexibility: the game developer can easily ask for just the input from players that might be using different devices with the same index (think controller index 0 and m+kb index 0). The developer can also even distinguish between two players sharing the same device (e.g. multiple players on 1 keyboard), provided other aspects of #10070 are implemented.

I bring this up because as proposed, I don't think this solution can be easily implemented with backwards compatibility. Consider what the default argument would be for PLAYER_DEVICE. We can't use null until #162 is implemented. We also can't use any default device index [0,n] because that would cause the method to change behavior (it currently returns input for all devices). Finally, we can't use -1 either, which might be reasonable, because that's the device index for emulated mouse input. I don't think there is a way around this without introducing some other wrapper type, which I think the "player ID" type is well placed to be.

With the above in mind, I see #10070 as a solution that encapsulates this one while providing more flexibility + backwards compatibility. I could definitely be missing something, so please let me know what you think!

[Carsonthemonkey]

I like your proposal, I think there are a few things I don't quite understand about it, so I left some feedback on your issue. It's very possible your implementation is the better way to go, but I was aiming to implement this in a way that preserves the simplicity of the Input api methods.

I wasn't aware of the -1 device id being reserved for mouse emulation. In the implementation I have been working on I have been using InputMap::ALL_DEVICES (equal to -1) as I was under the impression that it was safe to use this to represent all input devices. If -1 is reserved could InputMap::ALL_DEVICES be modified to be a different negative number that is not used as an ID for an existing input method?

[coffeebeats]

Hmm, I'm not sure if we are able to change InputMap::ALL_DEVICES, even though it only seems to be used in InputMap (maybe there are some custom C++ modules using it?). Regardless, the InputEvent.device docs have a note that negative values could be used in special circumstances, so I don't know if picking some other negative number would be better.

Rather than choosing some sentinel value to represent "all devices", however, I think a bitmask is actually the better approach. That way we don't need to worry about interfering with a valid device index, you can easily represent "all devices", and you can flexibly ask for multiple device indices at the same time (two local players on the same team, for example).

For what it's worth, a bitmask is the approach recommended in #10070, though it uses the aforementioned player ID instead of device index.