Skip to content

Support Input and UI for multiple players #102412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GreenCrowDev
Copy link
Contributor

@GreenCrowDev GreenCrowDev commented Feb 4, 2025

Closes godotengine/godot-proposals#10070.

Goal

The goal of this PR is to improve the engine support for local multiplayer games.
Also, the intent is to implement the new features without breaking compatibility with previous version.
The new logic will be implemented with default arguments when appropriate.

Content

This PR is based on @coffeebeats proposal Improve local multiplayer support by tracking InputEvents and UI focus per player.
The author stated in the discussion:

I think there are two sub-problems here: (1) segmenting input by player via a "player layer" and (2) allowing the engine to track UI focus for each player individually.

The two key points that will be addressed are:

  1. Introducing a new player layer between the device and game logic;
  2. Support the new player layer in the UI, allowing multiple players have their own Control node in focus.

The max amount of players allowed is 8.

Changes

Input

  • Add PlayerId enum class and PlayerMask enum for bitfield masking purposes. PlayerId is used when you only want to specify one single player, while PlayerMask is used as a bitfield when you want to check conditions for multiple players.
  • Methods like Input::is_action_pressed() and similar now have a new optional PlayerId argument that allow to specify for which player you want to check the condition;
  • InputMap action events binded to joypad devices are now set to ALL_DEVICES by default. This is because now the action itself is going to be triggered by all devices, and will be later filtered by the player id. The possibility of choosing the device id for the event is left incact to allow for custom behaviour, but it is not intended as the normal use case;
  • Keyboard, mouse, and touch inputs can be assigned to players other than P1 through keyboard_player_id_override, mouse_player_id_override, touch_player_id_override;
  • Joypad/player mapping has been taken care in the Joypad struct in Input, adding a PlayerId property in it. This strong coupling seems to work well, since it avoids the complexities of mantaining a synchronization between another map and the number of connected devices, that are very dynamic and real-time defined. Since the device id starts from 0, by default the player_id is assigned to the corresponding device id int, and seems to follow the common behaviour of newly connected controllers;
  • To change the mapping of a joypad, just use e.g. Input.set_joy_player_id(0, PLAYER_ID_P2);

UI

  • Control has a BitField<PlayerMask> player_mask = PLAYER_ALL; property, allowing to control which player can focus on it;

  • Add PROPERTY_HINT_LAYER_PLAYER_MASK for Control player_mask UI editor;

  • Methods like Control::grab_focus() and similar now have a new optional PlayerId argument that allow to specify for which player you want to apply the function;

  • The method Control::calculate_ancestral_player_mask() calculates a BitField<PlayerMask> composed by the union (& operator) of the masks of the node up to its root parent. This has been introduced so that given a parent Container with player_mask = PLAYER_2, all of its children will "indirectly inherit" the player 2 status even if they have their player_mask = PLAYER_ALL, since the internal logic traverses the parent tree to calculate the combined bitmask. This simplifies the creation of split screen UIs with less properties to modify, and allowing a more advanced control if the dev wants;

  • Specific logic has been added in the Control class to calculate focus_neighbor[], focus_next, and focus_prev, considering now there are Controls assigned to other players that you can't focus. Changing focus with arrow keys and TAB/SHIFT+TAB is preserved and works as expected;

  • You can query which players are focusing a Control with Control::get_focused_players_id();

Test Project

https://github.com/GreenCrowDev/godot_input_ui_multiplayer_support_test

Documentation PR

godotengine/godot-docs#10704

Examples

ui_multiplayer_001.mp4
ui_mp_next_prev.mp4

@GreenCrowDev
Copy link
Contributor Author

@Sauermann can I ask why the "breaks compat" flag was added? We want to implement this feature without breaking compatibility. Is there anything you noted that breaks compat and we need to address?

@AThousandShips
Copy link
Member

I can't see any compatibility breakgage from the code, default arguments are applied and it seems the behavior is equivalent with these

@Sauermann
Copy link
Contributor

@AThousandShips Thanks for the explanation. My understanding was, that changing exposed API (even when default parameters are added) is considered compatibility breaking. Looks like I learned something new today. I will remove the flag.

@coffeebeats
Copy link

Thanks for taking this on, @GreenCrowDev! I've left a few comments, but overall this is a great start to the implementation. Are there any parts of this that you would like help with?

One other small, low-priority thing that comes to mind: we may want to add an @export_flags_player_mask export decorator to GDScript.

@GreenCrowDev
Copy link
Contributor Author

Thanks for the comments @coffeebeats!
As I replied, I think I need help with the device/player mapping stuff.
In the meantime, I'm trying to spot where I need to inject the player bitmask stuff in the Control and Viewport methods.

@GreenCrowDev GreenCrowDev force-pushed the input-ui-multiplayer-support branch 4 times, most recently from dbbc61b to e7e8c2f Compare February 8, 2025 00:29
@GreenCrowDev GreenCrowDev force-pushed the input-ui-multiplayer-support branch from e7e8c2f to e9d381f Compare February 16, 2025 01:10
@GreenCrowDev
Copy link
Contributor Author

GreenCrowDev commented Feb 16, 2025

@coffeebeats sorry to bother you, just a little heads-up since I need some help.
Can you give some feedback on the latest messages I sent? I commented your reviews.

In the meantime I'm working on the Control and Viewport stuff.

Also, how would you cleanly approach giving other players control of just some keys of the keyboard (that is generally player 1)?
E.g. player 1 uses WASD, player 2 uses IJKL as arrow keys, player 3 uses arrow keys...
Some pseudocode would help.

I'm using something like this (seems rough):

func _unhandled_key_input(event: InputEvent):
	if event is InputEventKey:
		# print("Device: %s; Player: %s;" % [event.device, event.get_player()])
		manage_ijkl(event)
	else:
		print(event)

func _input(event: InputEvent):
	if event.player == Input.PLAYER_2:
		print("Player 2 Input.")
		pass
	elif event is InputEventAction:
		print(event)
		pass

func manage_ijkl(event: InputEventKey) -> void:
	# Key I pressed.
	if event.player != Input.PLAYER_2 && event.keycode == KEY_I && event.is_pressed():
		get_viewport().set_input_as_handled()
		var p2_event = InputEventAction.new()
		p2_event.player = Input.PLAYER_2
		p2_event.action = "ui_up"
		p2_event.pressed = true
		Input.parse_input_event(p2_event)

EDIT: I improved the code a little bit to support 4 players on the same keyboard. It's in the test project repo: https://github.com/GreenCrowDev/godot_input_ui_multiplayer_support_test/blob/b8dabd0fffaaab6cf1cf2ddc717ecc93f411b156/input_test.gd

@GreenCrowDev GreenCrowDev force-pushed the input-ui-multiplayer-support branch 3 times, most recently from 5cd289c to 5d77afa Compare February 16, 2025 23:06
@Calinou
Copy link
Member

Calinou commented Feb 17, 2025

Implement multiple player focus on different Controls;

Is there a way to distinguish focus on different controls, so that different focus outline colors can be used for each player? Also, can controls query which player(s) is focusing them from a script?

Additionally, if multiple players focus on the same control, the focus outlines should be modified to display this somehow. This could be done by extending the size of one of the focus outlines, so it surrounds the other outline, or by using dashed lines with alternating colors. This may be difficult to implement though, especially for the dashed lines since StyleBoxFlat doesn't support them currently.

@GreenCrowDev
Copy link
Contributor Author

Is there a way to distinguish focus on different controls, so that different focus outline colors can be used for each player?

Right now no, probably, I guess I have to dive a little bit in the StyleBox stuff if I'm not mistaken.
Do you think it is something that should be addressed here or in a subsequent PR?

Also, can controls query which player(s) is focusing them from a script?

That's very interesting, I have to check that. The information is stored in the Viewport. I'll try if it is possible, and eventually expose a function to achieve that. Thanks for the suggestion.

Additionally, if multiple players focus on the same control, the focus outlines should be modified to display this somehow. This could be done by extending the size of one of the focus outlines, so it surrounds the other outline, or by using dashed lines with alternating colors. This may be difficult to implement though, especially for the dashed lines since StyleBoxFlat doesn't support them currently.

Yeah I was thinking about that also.
Not sure what direction we should take here, but I have two points on my mind:

  1. How to manage multiple focus on the same Control is probably very subjective and every game would want a different style (an example later);
  2. I can see this used (e.g. fighting games where different players take the same character), but would it be that common to address this at core?
    We definitely need to check if clearly distinguish multiple players on the same control is something doable without addressing this
    at core, I'll make some experiments.

I can also see this solved by giving eash player a "corner" of the Control rectangle:
immagine

Anyway thank you so much for the feedback!

@GreenCrowDev GreenCrowDev force-pushed the input-ui-multiplayer-support branch from 5d77afa to 9dca687 Compare February 17, 2025 18:07
@coffeebeats
Copy link

Sorry for the delay in responding - I haven't had much time recently to devote to this.

First off, great work so far! The demo video you shared is super exciting!

Also, how would you cleanly approach giving other players control of just some keys of the keyboard (that is generally player 1)?
E.g. player 1 uses WASD, player 2 uses IJKL as arrow keys, player 3 uses arrow keys...
Some pseudocode would help.

The code snippet you shared is exactly how I'd approach it. I don't think it's feasible or necessary to allow configuring keyboard sharing ahead of time. Creating InputEventActions based on per-player keyboard bindings looks to be pretty straightforward.

A few minor details:

  • You may want to create InputEventAction events from _input so that the "raw" InputEventKeys are consumed before any other handlers get to run. The way you have it is totally valid, though, as long as no game or Input/Control logic would attempt to handle the key events before manage_multiplayer_arrow_keys gets a chance to run.
  • Each project could of course tweak how it stores the keyboard bindings (the shown solution is totally valid).
  • You probably don't need to check which player the keyboard events are originating from (i.e. maybe you can omit event.player == PLAYER_ID_P1 from manage_multiplayer_arrow_keys). There's also no guarantee that the keyboard is mapped to Player 1, so if you want to leave that check in you should be checking that the key event matches the player ID the keyboard is assigned to (this of course depends on the device->player mapping we're discussing in the other thread).
  • InputEventActions' device property is left at 0. I think this is correct, but maybe I'm overlooking an advanced use case where someone may also want to change that.

These (minor) points aside, the implementation looks great to me!

Can you give some feedback on the latest messages I sent?

I'm thinking through how the device -> player mapping should work right now. I should have some thoughts to share soon!

@coffeebeats
Copy link

Also, can controls query which player(s) is focusing them from a script?

That's very interesting, I have to check that. The information is stored in the Viewport. I'll try if it is possible, and eventually expose a function to achieve that. Thanks for the suggestion.

I think we'll definitely want to allow Control nodes' to query and manage focus for specific players via scripting (see implementation point 5. from the proposal). Specifically, Control.has_focus, Control.grab_focus, Control.release_focus, Control.find_next_valid_focus, Control.find_prev_valid_focus, Control.find_valid_focus_neighbor, and Control.get_focus_neighbor should all accept a player layer bitmask.

Likewise, Viewport.gui_get_focus_owner and Viewport.gui_release_focus methods should accept a player layer bitmask as well.

Unfortunately, as I'm writing this now, I'm seeing some complexity that I missed in the proposal:

  1. Focus neighbors are currently limited to one per direction, with no notion of player. I'm hoping we can avoid changing this by requiring that the focus neighbor targets have compatible player layer properties (showing an editor warning if the focus neighbor's player layer isn't a complete superset of the source Control node's player layer) and using the triggering input event's player ID when evaluating focus neighbors. My gut feeling is that it's not feasible to define different focus neighbors per player.
  2. I'm not sure how to handle Control.grab_click_focus. I don't really understand yet how click focus works compared with standard focus, and I'm not sure whether the mouse should be assignable to a player separately from the keyboard.
  3. Viewport.gui_focus_changed, Control.focus_entered, and Control.focus_exited signals cannot be updated with player layer bitmask if we want to maintain backwards compatibility. It would be nice to update these signals, but I think it would be best to leave them as is for now and trigger them when focus changes for any player. Perhaps in a future major release we can modify them.

@GreenCrowDev
Copy link
Contributor Author

GreenCrowDev commented Feb 17, 2025

I yet have to read carefully every point, I'll do that soon, but:

  1. Focus neighbors are currently limited to one per direction, with no notion of player. I'm hoping we can avoid changing this by requiring that the focus neighbor targets have compatible player layer properties (showing an editor warning if the focus neighbor's player layer isn't a complete superset of the source Control node's player layer) and using the triggering input event's player ID when evaluating focus neighbors. My gut feeling is that it's not feasible to define different focus neighbors per player.

I already implemented this part. In the video it's not apparent, but if you try to focus a control out of the 3x3 buttons grid of each player, it will not be allowed. Not sure if you meant something more advanced.

EDIT: Do you mean the case where a Control of a different player is between the current focus and the next of the same player, so that it should jump the one of another player?

@GreenCrowDev
Copy link
Contributor Author

The core functionalities have been implemented, so it should be ready to review now! 🎉

Testing needed!

The latest change was about having the triggering of Input actions be separated between each player: e.g. if P1 "just" presses "ui_select" and keep it pressed, when P2 "just" presses "ui_select", the corresponding "just_pressed" action will be triggered for P2. The normal behaviour would have been that the same action could not trigger another "just" pressed, since it should have been released first, but now the logic has been decoupled for each player, as you would expect.

@GreenCrowDev GreenCrowDev marked this pull request as ready for review February 25, 2025 15:34
@GreenCrowDev GreenCrowDev force-pushed the input-ui-multiplayer-support branch 2 times, most recently from 0078ae2 to e0d13f0 Compare February 26, 2025 19:31
@GreenCrowDev GreenCrowDev force-pushed the input-ui-multiplayer-support branch from e0d13f0 to 6f207b5 Compare March 14, 2025 15:08
@GreenCrowDev
Copy link
Contributor Author

Rebased.
Having this merged for 4.5 would be awesome (working on a commercial game with local multiplayer).
I'm ready for fixes/suggestions/reviews.

@DanielSnd
Copy link
Contributor

Not sure why yet, but I got a reproduceable crash on my game using it, it seems to be specifically on code from this PR:

Thread 1 "godot.linuxbsd." received signal SIGSEGV, Segmentation fault.
Control::get_player_mask (this=0x0) at scene/gui/control.cpp:2290
2290            if (data.initialized) {
(gdb) bt
#0  Control::get_player_mask (this=0x0) at scene/gui/control.cpp:2290
#1  0x000055555af0e99d in Control::calculate_ancestral_player_mask (this=0x0) at scene/gui/control.cpp:2303
#2  0x000055555af10f22 in Control::_window_find_focus_neighbor (this=0x55558e3f4c90, p_dir=..., p_at=0x55558c91cdb0, p_rect=..., p_clamp=..., p_min=416, r_closest_dist_squared=@0x7fffffffac74: 1e+14,
r_closest=0x7fffffffac68) at scene/gui/control.cpp:2457
#3  0x000055555af115e6 in Control::_window_find_focus_neighbor (this=0x55558e3f4c90, p_dir=..., p_at=0x55558c919570, p_rect=..., p_clamp=..., p_min=416, r_closest_dist_squared=@0x7fffffffac74: 1e+14,
r_closest=0x7fffffffac68) at scene/gui/control.cpp:2544
#4  0x000055555af115e6 in Control::_window_find_focus_neighbor (this=0x55558e3f4c90, p_dir=..., p_at=0x55556b43bb30, p_rect=..., p_clamp=..., p_min=416, r_closest_dist_squared=@0x7fffffffac74: 1e+14,
r_closest=0x7fffffffac68) at scene/gui/control.cpp:2544
#5  0x000055555af10df0 in Control::_get_focus_neighbor (this=0x55558e3f4c90, p_side=SIDE_BOTTOM, p_count=0, p_player_id=PlayerId::P1) at scene/gui/control.cpp:2439
#6  0x000055555ad9aaab in Viewport::_gui_input_event (this=0x555564b409e0, p_event=...) at scene/main/viewport.cpp:2217
#7  0x000055555ad9ffed in Viewport::push_input (this=0x555564b409e0, p_event=..., p_local_coords=false) at scene/main/viewport.cpp:3299
#8  0x000055555adf9c2c in Window::_window_input (this=0x555564b409e0, p_ev=...) at scene/main/window.cpp:1704
#9  0x000055555ae21e65 in call_with_variant_args_helper<Window, Ref<InputEvent> const&, 0ul> (p_instance=0x555564b409e0,
p_method=(void (Window::*)(Window * const, const Ref<InputEvent> &)) 0x55555adf9ab0 <Window::_window_input(Ref<InputEvent> const&)>, p_args=0x7fffffffbc40, r_error=...) at ./core/variant/binder_common.h:317
#10 0x000055555ae21d9b in call_with_variant_args<Window, Ref<InputEvent> const&> (p_instance=0x555564b409e0,
p_method=(void (Window::*)(Window * const, const Ref<InputEvent> &)) 0x55555adf9ab0 <Window::_window_input(Ref<InputEvent> const&)>, p_args=0x7fffffffbc40, p_argcount=1, r_error=...)
at ./core/variant/binder_common.h:431
#11 0x000055555ae21c6b in CallableCustomMethodPointer<Window, void, Ref<InputEvent> const&>::call (this=0x55556898f2c0, p_arguments=0x7fffffffbc40, p_argcount=1, r_return_value=..., r_call_error=...)
at ./core/object/callable_method_pointer.h:105
#12 0x000055555cf54e44 in Callable::callp (this=0x55558f1914f0, p_arguments=0x7fffffffbc40, p_argcount=1, r_return_value=..., r_call_error=...) at core/variant/callable.cpp:57
#13 0x000055555911af43 in Callable::call<Ref<InputEvent> > (this=0x55558f1914f0, p_args=...) at ./core/variant/variant.h:961
#14 0x000055555910c261 in DisplayServerX11::_dispatch_input_event (this=0x55555e83e360, p_event=...) at platform/linuxbsd/x11/display_server_x11.cpp:4274
#15 0x000055555910be5d in DisplayServerX11::_dispatch_input_events (p_event=...) at platform/linuxbsd/x11/display_server_x11.cpp:4237
#16 0x000055555cee33ce in Input::_parse_input_event_impl (this=0x55555e17dfa0, p_event=..., p_is_emulated=false) at core/input/input.cpp:971
#17 0x000055555cee09af in Input::flush_buffered_events (this=0x55555e17dfa0) at core/input/input.cpp:1264
#18 0x000055555911041a in DisplayServerX11::process_events (this=0x55555e83e360) at platform/linuxbsd/x11/display_server_x11.cpp:5402
#19 0x00005555590e19f5 in OS_LinuxBSD::run (this=0x7fffffffcc50) at platform/linuxbsd/os_linuxbsd.cpp:976
#20 0x00005555590d9035 in main (argc=3, argv=0x7fffffffd288) at platform/linuxbsd/godot_linuxbsd.cpp:85
(gdb)

The line in question is this one, in calculate_ancestral_player_mask() in control.cpp:

	BitField<PlayerMask> mask = get_player_mask();
	```

It happens when I'm using my gamepad to go down in an options menu, in a specific control.

Also, I'm not sure yet if it's related to this PR, but my game is no longer accepting controller input from the steamdeck D:

@GreenCrowDev
Copy link
Contributor Author

I will take a look at it later or tomorrow, but I need to know how to reproduce the error.
@DanielSnd can you provide a very simple project with the setup that causes the issues?

I don't have a steamdeck, but I don't see why it shouldn't work. There should be a mapping issue somewhere.
Can you try to print some info about what player it is assigned to?

@DanielSnd
Copy link
Contributor

I will take a look at it later or tomorrow, but I need to know how to reproduce the error. @DanielSnd can you provide a very simple project with the setup that causes the issues?

I don't have a steamdeck, but I don't see why it shouldn't work. There should be a mapping issue somewhere. Can you try to print some info about what player it is assigned to?

I'll try to find some time to make a MRP. It might take a while as I have a busy week this week.

@GreenCrowDev GreenCrowDev force-pushed the input-ui-multiplayer-support branch 3 times, most recently from b4d1ea4 to 934a1e1 Compare March 25, 2025 23:14
@GreenCrowDev
Copy link
Contributor Author

@DanielSnd I think the crash error should be fixed now.
Still not sure about the Steam Deck issue though.

Tomorrow I'll work on the GDExtension compatibility breakage following the docs and fix the CI error.

@GreenCrowDev GreenCrowDev force-pushed the input-ui-multiplayer-support branch 4 times, most recently from 32fe928 to 149dcb8 Compare March 26, 2025 19:13
@GreenCrowDev GreenCrowDev requested review from a team as code owners March 26, 2025 19:13
@GreenCrowDev GreenCrowDev force-pushed the input-ui-multiplayer-support branch from 149dcb8 to 2e520a4 Compare March 26, 2025 19:18
@GreenCrowDev
Copy link
Contributor Author

I implemented compatibility stuff, now CI is fine!
Ready for review!

@GreenCrowDev
Copy link
Contributor Author

Hello @Faless, sorry for the ping.
I would be very grateful if you could take a look at this PR and give a feedback, since I know you are and have been involved with the multiplayer input stuff.
Thank you for your time.

@GreenCrowDev
Copy link
Contributor Author

Hi @adamscott @Repiteo, just wanted to know how we should follow up on the discussion from the Core meeting. Thanks!

@adamscott
Copy link
Member

Hi @adamscott @Repiteo, just wanted to know how we should follow up on the discussion from the Core meeting. Thanks!

The discussion will happen in the Godot developers chat, on the #input channel.

Here's the link: https://chat.godotengine.org/channel/input

DanielSnd added a commit to DanielSnd/godot that referenced this pull request Apr 13, 2025
DanielSnd added a commit to DanielSnd/godot that referenced this pull request Apr 13, 2025
DanielSnd added a commit to DanielSnd/godot that referenced this pull request Apr 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve local multiplayer support by tracking InputEvents and UI focus per player
9 participants