Add support for multiple focus layers for Controls

[redsett]

Added support for multiple Controls to be focused at once, via "focus layers".

Summary:
Each Control is assigned to a (int)focus_layer, and the Viewport now has a (int)active_focus_layer. A Viewport will only navigate to Controls that match the active_focus_layer. E.g. If the active_focus_layer is 1, the Viewport will only navigate to Controls with their focus_layer set to 1. You could then set_active_focus_layer() to switch to navigating Controls in another layer.

Example:

@Faless @groud
This mostly solves the split screen issue as well, but doesn't have the multiple input support by default like @Faless's does linked below, but there's no reason his solution couldn't be added on top of this.
#20091

Note: This is my first time contributing to a github project in general. So please excuse my noobness. I'm happy to learn from you all, so feel free to slap me on the wrist if need be. :)

[Calinou]

Nice work! I believe this addresses godotengine/godot-proposals#385 and godotengine/godot-proposals#4295.

[redsett]

1
godotengine/godot-proposals#385 this is exactly what I was adding support for on my personal project.

2
godotengine/godot-proposals#4295 I don't believe this fully adds support for focusing with multiple inputs. But with a bit of extra gdscript hackery I think it could. (would have to test, I've never experimented with multiple-player and have little experience with inputs in godot in general)

Example of how you could probably do this while still supporting multiple focus layers in each Viewport:
Viewport1 only use active_focus_layer's 1-4, Viewport2 only use active_focus_layer's 5-8. Then make sure the focus_layer on the Controls in each Viewport are set correctly.

To fully support multiple inputs without getting fancy(hacky), I think we'd have to add something like this on top of it:
#20091

[redsett]

control.cpp got a big revamp which has many conflicts with this change. I'm just going to close this request. Probably a bad time to be adding features like this one.

[YuriSizov]

control.cpp got a big revamp which has many conflicts with this change. I'm just going to close this request. Probably a bad time to be adding features like this one.

Sorry for the inconvenience. It should be good to make PRs now though, and the change was only superficial. You would likely need to port your work by hand, but it shouldn't require any code changes. That big PR just rearranged things.

Right now is also the best time to make a PR is you want to have this feature in 4.0. On August 3rd we're freezing the feature roadmap.

[redsett]

Sadly, I can't make that deadline. :/

Thanks YuriSizov, no worries.

[KoBeWi]

Technically this was opened before the freeze and a rebased PR counts as the same feature, so...

[YuriSizov]

Yeah, sorry, I didn't imply that you need to resubmit it before August 3rd. Just wanted to dispute that it's not the best time to contribute something like that, when in fact it's the perfect time.

[redsett]

Understood! I'll take a look this weekend at merging.

[redsett]

Merged

[YuriSizov]

Can you please squash your commits, per our guidelines

[redsett]

Can you please squash your commits, per our guidelines

I think I did the thing.

[YeldhamDev]

The current commit message is currently "parent 836fe9a", it should be the actual message instead. The commit's description section could also be improved a little bit, as it seems to have some junk from the squashing.

[YuriSizov]

And CI seems to be failing because of a couple of code style issues, and because you need to run the doctool to update some documentation properly.

[KoBeWi]

I tested the feature. It's great, but feels like there should be an easier way to switch between layers. Maybe an optional argument to grab_focus() that makes it automatically switch layer when true?

[redsett]

After playing around with Sauermann's implementation (split into #78762 and #79480) allows nested input layers/screens without losing focus, local splitscreen menu's, and really whatever the dev wants to do.

Demo

Yeah, both CL seem to achieve "mostly" the same thing in different ways.

You can do multiplayer type stuff with this CL as well. You just have to set_active_focus_layer() when an input happens based on which player pressed the input. (which certainly isn't as nice as Sauermann's)

I.e
get_viewport().gui_set_active_focus_layer(player_index)

[redsett]

@djpeach, here's your example using this PR.

LocalSplitScreenMenu_UsingFocusLayers.zip

Technically the sub viewports are not actually needed in this example. Only the Control's being on separate focus layers(which could be set at runtime as well). If you removed the sub viewports, you'd just do get_viewport().gui_set_active_focus_layer(event is InputEventJoypadButton).

[djpeach]

That's awesome. I was wondering how this PR would play with the others. I can picture using event filtering for the sub-viewports and then the focus layers for nested UI's for each player

[reduz]

I think there is a lot of confusion in this pull request. So I will give my view on it.

1. What this PR solves is very easy to workaround. You don't need focus to highlight something selected. Plenty of controls to do this already without the need of this feature.
2. You also don't need layers to restraint focus movement to an area, you can override this with next/prev focus. If not enough, I think a separate feature to tell a control to restrict focus movement to children would be more useful and more explicit.
3. What pretty much everyone here wants by checking the discussion is having independent focuses per viewport for split screen. This is not what this PR is meant for. I think a separate PR should be opened to address this more specifically, doing the following:
   A. The ability to assign a layer mask to events in the input event editor.
   B. The ability to assign a layer mask to viewports as a viewport property, so events are filtered.
   C. The ability to set a Viewport to manage its own focus, that is, its focus can't be stolen by other viewports and that viewports can't steal others focus.

As such I think this PR is not necessary. My view is that there is mostly misunderstanding around it and that it should be its own separate proposal first to discuss this feature specifically, leaving aside the separate focus per viewport, which is not related.

My view, as always, is that we should not fall into this pattern:

And think for each problem its own solution.

[KoBeWi]

You also don't need layers to restraint focus movement to an area, you can override this with next/prev focus.

This does not solve the problem. It's impossible to restrict the focus movement with the current system.
EDIT:
Ok, it's possible if you make every focus neighbor point to self xd Realistically it can only be done via script.

If not enough, I think a separate feature to tell a control to restrict focus movement to children would be more useful and more explicit.

Except it would force you to assign all focus neighbors explicitly, which is not very feasible in complex menus, especially when you want to later rearrange stuff or the menu itself is dynamic.

What pretty much everyone here wants by checking the discussion is having independent focuses per viewport for split screen.

tbh I did not even consider that when doing my review. I myself ran into the problem of not being able to restrict focus to specific areas, which I think is a common issue if you rely on the built-in Control system for menu layouts, and this PR solves that.

[Sauermann]

If not enough, I think a separate feature to tell a control to restrict focus movement to children would be more useful and more explicit.

Except it would force you to assign all focus neighbors explicitly, which is not very feasible in complex menus, especially when you want to later rearrange stuff or the menu itself is dynamic.

I believe, that I interpret the idea differently than you, KoBeWi:
When a focus movement happens and a parent Control P has a new flag restrict_focus_movement_to_children set to true, then the focus movement-algorithm (Control::find_next_valid_focus, Control::_get_focus_neighbor, ...) should consider as valid Control-nodes only children of the node P.

@reduz

I think a separate PR should be opened to address this more specifically, doing the following:
A. The ability to assign a layer mask to events in the input event editor.
B. The ability to assign a layer mask to viewports as a viewport property, so events are filtered.

The Input Event editor allows configuring only InputEventAction. I wonder if it wouldn't be better to allow SubViewports to filter all InputEvents. #78762 implements this idea.

C. The ability to set a Viewport to manage its own focus, that is, its focus can't be stolen by other viewports and that viewports can't steal others focus.

I believe that it is feasible to remove focus-stealing completely. See #79480 for an implementation of that.

[KoBeWi]

When a focus movement happens and a parent Control P has a new flag restrict_focus_movement_to_children set to true, then the focus movement-algorithm (Control::find_next_valid_focus, Control::_get_focus_neighbor, ...) should consider as valid Control-nodes only children of the node P.

Right, I didn't notice the "children" part. It would need to be "descendants", as the node you want focused doesn't need to be a direct child (not uncommon when you put too many containers for styling), but it sounds acceptable.

[redsett]

I'm personally fine with the sub viewport method that Saurmann made, 79480.

You could do a normal settings menu or nested(smash bros) menu with the sub viewport method. A bit unintuitive and probably has a perf cost, but flexible none the less.

[TheFoxKnocks]

I'm a little confused on which implementation, if any, is looking to be official. I was looking forward to a 4.2 implementation of proper local coop input support, but it looks like that might not be happening now? I'm not fully sure.

Don't get me wrong, not rushing or being ungrateful, really just trying to figure out where this stands.

[redsett]

I'm a little confused on which implementation, if any, is looking to be official. I was looking forward to a 4.2 implementation of proper local coop input support, but it looks like that might not be happening now? I'm not fully sure.

Don't get me wrong, not rushing or being ungrateful, really just trying to figure out where this stands.

It sounds like this PR adds too much complexity. But with these PR that Sauermann made, you can achieve the same result for local/and or split screen co-op. 78762, 79480

You would just put each player's UI on a separate subviewport, then pass the input for each player to the correct viewport.

This PR is certainly more intuitive, but there's no doubt that Sauermann's is far less intrusive. While still being flexible enough to achieve what his PR does.

[juliohq]

I like the idea of having different focus layers. I need this PR to be merged for a project I'm working on, which doesn't have a complex UI but needs tabs that should be focused at the same time as other controls.

I tried to implement them by hand but didn't have much luck, since trying to split the logic is overly complex and still doesn't provide me with useful features the built-in focus provides, like being able to use the built-in ui_* input actions on them and hence requiring more manual work.

[Chaosus]

@redsett This PR need to be rebased, just a reminder

[redsett]

@redsett This PR need to be rebased, just a reminder

Thanks. I'll rebase if you guys decide to go this route. But it sounds like they want to go with another solution.

[TheFoxKnocks]

Am I correct in assuming that ultimately neither choice was decided to be the right one and Godot still has no way to handle multiple focus targets?

[Sauermann]

A discussion on which solution would be preferable is still pending.
So the current status is, that multiple focus targets are not supported.

[Mickeon]

This is a lovely amount of detail and it's a shame it's locked behind this contested PR. Should be a separate PR outright. Aside from a few tweaks I'd accept it in a heartbeat.