matjlars / godot-multiplayer-input

This Godot addon provides two simple APIs for using normal Input Actions, but spread out across a Keyboard player and up to 8 Joypad players.
MIT License
104 stars 6 forks source link

Default UI actions stopped working #1

Closed KANAjetzt closed 9 months ago

KANAjetzt commented 1 year ago

Hi, first of all, thanks for this plugin! For some reason, my default UI actions stop working on my joypads after activating the plugin. In the game, the actions I handle with MultiplayerInput work great. Keyboard inputs continue to work fine.

System information

Godot 4.1.1 Windows 10 1 XBOX Series Controller 1 XBOX Elite Series 2

Steps to reproduce

  1. Create 2 buttons.
  2. Add a script to grab focus.
  3. Install the plugin and activate it.

Minimal reproduction project

TestGround.zip

KANAjetzt commented 1 year ago

https://github.com/matjlars/godot-multiplayer-input/blob/930ae9337b4de347251c5f12c329df46bdf14552/addons/multiplayer_input/multiplayer_input.gd#L38-L41

Okay, I found the part of the code that caused it, but I don't fully understand why it's there.

matjlars commented 1 year ago

hi thank you for making an issue. i am chewing on this now. that block of code is to make sure controller input from device 0 does not activate those default actions because then MultiplayerInput would read it as a keyboard/mouse event happening. I'll have to rethink something, not sure what yet. I wonder id there is some way to identify a built-in action and not run that block on those. or perhaps there is a more comprehensive solution that could work with the focus system. i briefly looked into it once and decided there is not because i believe there can only be one focus. hmmmmmmm

KANAjetzt commented 1 year ago

Adding to this, a way to transfer the UI controls from one player to another would be useful. In my use case, each player enters a "shop" phase where they can buy items, and I don't want the other players to have control during that moment.

I'll take a deeper look into this over the weekend, maybe we can figure something out 👍

KANAjetzt commented 1 year ago

Relevant Godot Issue https://github.com/godotengine/godot-proposals/issues/4295

Relevant PRs https://github.com/godotengine/godot/pull/62421 https://github.com/godotengine/godot/pull/79480

matjlars commented 1 year ago

Ok I think I have a solution in mind. Thank you for your idea on switching players, that was good info. I was worried there would be no way to identify a built-in action because there is no "isbuiltin" function or anything in InputMap, but I think a better solution will be to define UI actions as any action that starts with "ui" and only have them connected to one device at a time, like you suggested. I think keeping the option to have default behaviour would be good. In fact I think I'm going to keep the default behavior by default, and add a single method called MultiplayerInput.set_ui_action_device(device: int) that restricts all actions that start with "ui_" to only that device. At first I was thinking of calling it "set_builtin_action_device()" but I like "uiaction" better because then devs can define their own ui actions that inherit this behavior. Not to mention, that allows me to actually do it, since there is no way to detect a built-in action, per se. Let me know your thoughts on that solution. I should have some time tonight.

I'll want to add a function to DeviceInput, too. Something like input.take_ui_actions() or input.enable_ui_actions().

note to self: allow going back to default behavior after a call to set_ui_action_device(). Maybe passing a -2, or another function.

I'll keep my eye on those PRs, thanks for finding and linking those.

KANAjetzt commented 1 year ago

Sounds good to me 👍. There has to be something for the built-in actions because of this toggle: image I guess there is just no way to access that information.

Thanks for taking the time to work on this :)

willerxxmiller commented 1 year ago

Ok I think I have a solution in mind. Thank you for your idea on switching players, that was good info. I was worried there would be no way to identify a built-in action because there is no "isbuiltin" function or anything in InputMap, but I think a better solution will be to define UI actions as any action that starts with "ui" and only have them connected to one device at a time, like you suggested. I think keeping the option to have default behaviour would be good. In fact I think I'm going to keep the default behavior by default, and add a single method called MultiplayerInput.set_ui_action_device(device: int) that restricts all actions that start with "ui_" to only that device. At first I was thinking of calling it "set_builtin_action_device()" but I like "uiaction" better because then devs can define their own ui actions that inherit this behavior. Not to mention, that allows me to actually do it, since there is no way to detect a built-in action, per se. Let me know your thoughts on that solution. I should have some time tonight.

I'll want to add a function to DeviceInput, too. Something like input.take_ui_actions() or input.enable_ui_actions().

note to self: allow going back to default behavior after a call to set_ui_action_device(). Maybe passing a -2, or another function.

I'll keep my eye on those PRs, thanks for finding and linking those.

If the new function looks for actions that begin with "ui_" wouldn't those actions be added twice? Once in the new function and once in the _create_actions_for_device function?

matjlars commented 1 year ago

@KANAjetzt Would you mind testing branch 1-ui-actions? I am not super familiar with the UI actions so I'm not sure what are some good use cases, but if it works for you then I will merge it into main. Thanks!

willerxxmiller commented 1 year ago

It works for me now. Thanks!

KANAjetzt commented 1 year ago

Yes! It's working great! Thank you 😃

ElHerpe commented 1 year ago

https://github.com/matjlars/godot-multiplayer-input/blob/1914d971eaa6cdd5e6d831a9c72be876d3bf4d6f/addons/multiplayer_input/multiplayer_input.gd#L39-L42 Maybe it makes sense to insert this if-statement in line 39/40:

if is_ui_action(action): continue
AleksandarV993 commented 11 months ago

I'm not sure if this is related (it probably is), but if I do something like this:

func handle_join_input():
    for device in get_unjoined_devices():
        if MultiplayerInput.is_action_just_pressed(device, "ui_accept"):
            print("Joined ", device)
            join(device)

I get this printed:

Joined 0
Joined -1

While if I use any non-ui input action, only the joystick joins without the keyboard.

Edit: Nevermind, this was my bad, the keyboard always gets appended whenever any input gets triggered, just wrote my own wrapper in order to avoid that.