Skip to content
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

Multi-focus, player-specific input events. #29989

Closed
wants to merge 17 commits into from

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Jun 22, 2019

Mostly implement what discussed in #20091 . Focus uses group names (strings) instead of booleans.

A demo project with split screen, separate focus and inputs is available here:
(W,A,S,D,Space ||| LEFT,RIGHT,UP,DOWN, Enter)

ActionPlayer.zip (live HTML5 here, you need to first click into the page after loading for input to start working)

The PR basically add a player field to each input event in the InputMap.
Action related methods have a new parameter for specifying which player to filter (0 for no filtering).

viewport_player_filter

inputmap

Viewport have two new properties under "GUI" section: input_player to specify which player the UI will filter (default 0, i.e. no filtering) and focus_groupproperty decides focus sharing. Viewports in the same focus_group share focus across them.

viewportgui

This change is quite hacky, and we are still trying to figure out if there's a better way then this to get multifocus and UI actions working together. Without overcomplicating things but still allowing people to make split screen games and use Godot UI nodes.

I'm setting this PR to milestone 4.0 so we can get @reduz input once he is done with Vulkan.
If anyone has suggestions, ideas, they are more than welcome.

Fixes #10755

/**
* A special value used to signify that a given Action can be triggered by any device
*/
static int ALL_DEVICES;

struct ActionInput {
ActionPlayer player;
Copy link
Contributor

@LikeLakers2 LikeLakers2 Jun 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiousity, will this being the ActionPlayer enum type limit the number of players to 16 (since the enum itself only goes up to 16)? Or will it still allow arbitrary numbers to be input here?

And since I'm here, I should also ask, what counts as a "player" to the engine?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It somehow limits the number of split-screen players, or local players with same input actions but different input events for such actions. It does not in any way the limit the number of players in a broader sense.

And since I'm here, I should also ask, what counts as a "player" to the engine?

It's just a number used to filter input events.
So you can have the same action names with different input events.

# Check if ui_left is pressed with the key associated to player 1 in InputMap
InputMap.is_action_pressed("ui_left", 1)

# Check if ui_left is pressed with the key associated to player 2 in InputMap
InputMap.is_action_pressed("ui_left", 2)

# Check if ui_left is pressed with any key configured in InputMap
InputMap.is_action_pressed("ui_left", 0)

Copy link
Contributor

@LikeLakers2 LikeLakers2 Jun 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It somehow limits the number of split-screen players, or local players with same input actions but different input events for such actions. It does not in any way the limit the number of players in a broader sense.

Perhaps I worded my question wrong. I was asking if that number could be a number higher than 16, without having to edit and recompile the engine. Although I'll admit that the needs for a number higher than 16 are few and far between (if one even exists), so chances are that this question is moot.

It's just a number used to filter input events.

What does that number refer to? A controller index?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps I worded my question wrong. I was asking if that number could be a number higher than 16

Yes, it does limit it to 16, but as you said, it's very unlikely you need more that that.

What does that number refer to? A controller index?

No, the local order you would give your players. And the value you set in InputMap or pass to action_add_event

@pouleyKetchoupp
Copy link
Contributor

This is a really interesting feature for multiplayer games, but why making it dependent to setting up separate viewports? It seems very specific to split screen, and this feature would be very useful if it could be used in a more generic way.

Maybe one viewport could store more than one focused control (based on different layers, possibly with a maximum set in project settings) and each control would have a layer property in the focus section so you can set any control with a specific layer (or all layers, by default).

I know it makes it a bit more difficult to implement, but this way any multiplayer game could have menus with several controllers even with a shared interface (a player select screen is a perfect example where you want only one viewport).

} else {
events[idx] = ie;
events[idx] = event;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is causing the player index to be reset when you change an input key.

I would suggest this logic instead:

	if (idx < 0 || idx >= events.size()) {
		Dictionary event;
		event["player"] = player;
		event["event"] = ie;
		events.push_back(event);
	} else {
		Dictionary event = events[idx];
		event["event"] = ie;
	}

@aaronfranke
Copy link
Member

@Faless Is this still desired? If so, it needs to be rebased on the latest master branch, and squashed, as per this article. Also, the above feedback should be addressed. Also, since this PR is a feature proposal, you should consider opening a proposal which explains example use cases and how this approach will solve the problem. Just from reading this PR, I can see what this PR does but I can't see why this approach was chosen or why the existing Input map system is unsuitable as-is.

Otherwise, abandoned pull requests will be closed in the future as announced here.

@Faless
Copy link
Collaborator Author

Faless commented Feb 9, 2022

Closing.
Will need formalization in a proposal before a new implementation is attempted.

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.

Add multiple UI focus groups
6 participants