Ability to disable focus on a control (and its children) with out modify the Control.SetFocusMode property

Delsin-Yu commented 1 year ago

- Describe the project you are working on

I am working on creating an open-sourced UI management system for our team for our next title on Godot.

- Describe the problem or limitation you are having in your project

This proposal is requesting a feature similar to godotengine/godot#20234 for a different purpose.

When creating a game that has complex UI designs, we usually group a bunch of controls under one specific node, let's name it "Logical Panel", and it is not uncommon that a game requires opening "Logical Panel" one above another.

When the game aims to support gamepad or keyboard-only control schemes, "Focus" in Godot comes into play, for a majority of the games, the player always uses their device to navigate through the GUI Controls inside one logical panel, and open other one with their "ui_accept" key, I want to highlight the fact that it is crucial for developers to keep the navigation range within a specific set of controls, so the navigation focus does not "leak" into the panels behind it and causes undesirable behavior.

It is intuitive to structure ui "panels" in the form of "a complex tree of controls (children nodes) under one single control node (root panel node)", which conforms to the idea of a "Logical Panel", so I am working on a method to toggle the Focus for the root panel node and its children.

However, I find it rather difficult to achieve this, in order to disable focus on a control, I have to manually set the Control.SetFocusMode property to "Control.FocusModeEnum.None"; in order to reenable the focus, I have to cache the original value and restore to it, (instead of setting the focus mode directly to "Control.FocusModeEnum.All", which makes label, texture rect, and such focusable as well, not a default behavior). In addition, iterating through every child on a scripting level (non-engine level) also increases frame time.

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

By adding this feature, developers are able to manage the focus in a simple and intuitive way, without the hassle of complex coding and caching.

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

Add 3 toggles to Control Node

A toggle to enable/disable the global focus of a control node (e.g. "EnableFocus").
A toggle to enable/disable propagating the value of "EnableFocus" to the recursive children.
A toggle to ignore the "EnableFocus" value propagated from the parent.

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

Here I am providing my current implementation to somehow achieve this requirement, and I will explain why this implementation is bad and does not achieve all the requirements in the next section.

/// <summary>
/// Enable or Disables the focus mode for <paramref name="root"/> and its recursive children, <paramref name="cachedNodeFocusMode"/> and the focus mode for the node or any affected child should not be tampered with after calling this method with <paramref name="enable"/> set to false. 
/// </summary>
/// <param name="root">Root node of the enumeration</param>
/// <param name="enable">When set to false, the method sets the focus mode for <paramref name="root"/> and its recursive children to None, and writes their original values into <paramref name="cachedNodeFocusMode"/>, when set to true, the method restore the focus mode for <paramref name="root"/> and its recursive children to the values stored inside <paramref name="cachedNodeFocusMode"/> and clears it.</param>
/// <param name="includeInternal">If includeInternal is false, the recursive enumeration excludes nodes' internal children (see <c>internal</c> parameter in <see cref="M:Godot.Node.AddChild(Godot.Node,System.Boolean,Godot.Node.InternalMode)" />)</param>
private static void ToggleFocusModeRecursive(Node root, bool enable, Dictionary<Control, Control.FocusModeEnum> cachedNodeFocusMode, bool includeInternal = false)
{
    if (!enable)
    {
        cachedNodeFocusMode.Clear();
        DisableFocusModeRecursive(root, cachedNodeFocusMode, includeInternal);
    }
    else
    {
        EnableFocusModeRecursive(root, cachedNodeFocusMode, includeInternal);
        cachedNodeFocusMode.Clear();
    }
}

/// <summary>
/// Restore the focus mode for <paramref name="root"/> and its recursive children to the values stored inside <paramref name="cachedNodeFocusMode"/>
/// </summary>
private static void EnableFocusModeRecursive(Node root, Dictionary<Control, Control.FocusModeEnum> cachedNodeFocusMode, bool includeInternal = false)
{
    if (root is Control control)
    {
        // Only enable if the node is present in the cache
        if (cachedNodeFocusMode.Remove(control, out var cachedFocusMode))
        {
            control.FocusMode = cachedFocusMode;
        }
    }

    foreach (var child in root.GetChildren(includeInternal))
    {
        EnableFocusModeRecursive(child, cachedNodeFocusMode, includeInternal);
    }
}

/// <summary>
/// Set the focus mode for <paramref name="root"/> and its recursive children to None, and write their original values into <paramref name="cachedNodeFocusMode"/>   
/// </summary>
private static void DisableFocusModeRecursive(Node root, Dictionary<Control, Control.FocusModeEnum> cachedNodeFocusMode, bool includeInternal = false)
{
    if (root is Control control)
    {
        var controlFocusMode = control.FocusMode;
        // Only cache the control when it is in any form of focusable
        if (controlFocusMode != Control.FocusModeEnum.None)
        {
            cachedNodeFocusMode[control] = controlFocusMode;
            control.FocusMode = Control.FocusModeEnum.None;
        }
    }

    foreach (var child in root.GetChildren(includeInternal))
    {
        DisableFocusModeRecursive(child, cachedNodeFocusMode, includeInternal);
    }
}

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

As the code provided above, it is clear that a dedicated Dictionary was used for every Node (root panel node), which requires memory allocation.
In addition, enumerating every child of a node by script does have an extra performance penalty (compared to implemented in core).
Another downside is the inflexible nature of the methods, which requires that "the focus mode for the node (root panel node) or any affected child should not be tampered with after calling this method". Tracking the changes may be possible, but currently, I don't have any idea on to implement it without causing severe performance degradation.
Finally, this method also does not support the "Ignore parent" feature, which requires an additional value to be stored in every affected control, a dedicated user script as a "marker" for checking may be possible, but that forces the developer to attach the marker script to the control, which is inconvenient when additional function is required for that control as well.

I have currently not looked into the source code of the engine of this section, however, I believe it is far easier to implement this feature at the core level. By adding these properties at the core level, all of these behaviors can be evaluated when processing the scene tree.

Delsin-Yu commented 1 year ago

For any developer from Unity Engine, this proposal is requesting the feature of "CanvasGroup(Unity)"

Delsin-Yu commented 1 year ago

This is a follow-up of the previous script, it turns out that modifying the "control.FocusMode" property does not block the pointer input, in order to achieve that here I have an updated version in case someone needs it.

/// <summary>
/// Enable or Disables the focus mode for <paramref name="root"/> and its recursive children, <paramref name="cachedNodeFocusMode"/> and the focus mode for the node or any affected child should not be tampered with after calling this method with <paramref name="enable"/> set to false. 
/// </summary>
/// <param name="root">Root node of the enumeration</param>
/// <param name="enable">When set to false, the method sets the focus mode for <paramref name="root"/> and its recursive children to None, and writes their original values into <paramref name="cachedNodeFocusMode"/>, when set to true, the method restore the focus mode for <paramref name="root"/> and its recursive children to the values stored inside <paramref name="cachedNodeFocusMode"/> and clears it.</param>
/// <param name="includeInternal">If includeInternal is false, the recursive enumeration excludes nodes' internal children (see <c>internal</c> parameter in <see cref="M:Godot.Node.AddChild(Godot.Node,System.Boolean,Godot.Node.InternalMode)" />)</param>
private static void ToggleFocusModeRecursive(Node root, bool enable, Dictionary<Control, CachedControlInteractableInfo> cachedNodeFocusMode, bool includeInternal = false)
{
    if (!enable)
    {
        cachedNodeFocusMode.Clear();
        DisableFocusModeRecursive(root, cachedNodeFocusMode, includeInternal);
    }
    else
    {
        EnableFocusModeRecursive(root, cachedNodeFocusMode, includeInternal);
        cachedNodeFocusMode.Clear();
    }
}

public readonly struct CachedControlInteractableInfo
{
    public readonly Control.FocusModeEnum CachedFocusMode;
    public readonly Control.MouseFilterEnum CachedMouseFilterEnum;

    public CachedControlInteractableInfo(Control.FocusModeEnum cachedFocusMode, Control.MouseFilterEnum cachedMouseFilterEnum)
    {
        CachedFocusMode = cachedFocusMode;
        CachedMouseFilterEnum = cachedMouseFilterEnum;
    }
} 

/// <summary>
/// Restore the focus mode for <paramref name="root"/> and its recursive children to the values stored inside <paramref name="cachedNodeFocusMode"/>
/// </summary>
private static void EnableFocusModeRecursive(Node root, Dictionary<Control, CachedControlInteractableInfo> cachedNodeFocusMode, bool includeInternal = false)
{
    if (root is Control control)
    {
        // Only enable if the node is present in the cache
        if (cachedNodeFocusMode.Remove(control, out var cachedFocusMode))
        {
            control.FocusMode = cachedFocusMode.CachedFocusMode;
            control.MouseFilter = cachedFocusMode.CachedMouseFilterEnum;
        }
    }

    foreach (var child in root.GetChildren(includeInternal))
    {
        // if (child is UIPanelBaseImpl) continue; <=== This line is the "ignore_parent" feature in my framework, change this to your desired type to achieve similar functionality.
        EnableFocusModeRecursive(child, cachedNodeFocusMode, includeInternal);
    }
}

/// <summary>
/// Set the focus mode for <paramref name="root"/> and its recursive children to None, and write their original values into <paramref name="cachedNodeFocusMode"/>   
/// </summary>
private static void DisableFocusModeRecursive(Node root, Dictionary<Control, CachedControlInteractableInfo> cachedNodeFocusMode, bool includeInternal = false)
{
    if (root is Control control)
    {
        var controlFocusMode = control.FocusMode;
        var controlMouseFilter = control.MouseFilter;
        // Only cache the control when it is in any form of focusable
        if (controlFocusMode != Control.FocusModeEnum.None || controlMouseFilter != Control.MouseFilterEnum.Ignore)
        {
            cachedNodeFocusMode[control] = new(controlFocusMode, controlMouseFilter);
            control.FocusMode = Control.FocusModeEnum.None;
            control.MouseFilter = Control.MouseFilterEnum.Ignore;
        }
    }

    foreach (var child in root.GetChildren(includeInternal))
    {
        // if (child is UIPanelBaseImpl) continue; <=== This line is the "ignore_parent" feature in my framework, change this to your desired type to achieve similar functionality.
        DisableFocusModeRecursive(child, cachedNodeFocusMode, includeInternal);
    }
}

ievr commented 4 months ago

The below is the above in GDscript which solved my problem. Added the mouse scroll caching, but removed the internal node thing and parent ignores.

class_name FocusUtil
extends Node

class CachedModes:
    var focus_mode : Control.FocusMode
    var mouse_filter : Control.MouseFilter
    var mouse_scroll : bool
    func _init(focus_mode : Control.FocusMode, mouse_filter : Control.MouseFilter, mouse_scroll : bool):
        self.focus_mode = focus_mode
        self.mouse_filter = mouse_filter
        self.mouse_scroll = mouse_scroll

static func toggle_focus_recursive(root : Node, cache : Dictionary, enable : bool) -> void:
    if not enable:
        cache.clear()
        _disable_focus(root, cache)
    else:
        _enable_focus(root, cache)
        cache.clear()

static func _enable_focus(root : Node, cache : Dictionary) -> void:
    if root is Control and cache.has(root):
        var control := root as Control
        var previous := cache[root] as CachedModes
        control.focus_mode = previous.focus_mode
        control.mouse_filter = previous.mouse_filter
        control.mouse_scroll = previous.mouse_scroll
        cache.erase(root)
    for child in root.get_children():
        _enable_focus(child, cache)

static func _disable_focus(root : Node, cache : Dictionary) -> void:
    if root is Control:
        var control := root as Control
        var focus_mode := control.focus_mode
        var mouse_filter := control.mouse_filter
        var mouse_scroll := control.mouse_force_pass_scroll_events
        if ( 
                focus_mode != Control.FocusMode.FOCUS_NONE or 
                mouse_filter != Control.MouseFilter.MOUSE_FILTER_IGNORE or 
                mouse_scroll == false
        ):
            var cache_item = CachedModes.new(focus_mode, mouse_filter, mouse_scroll)
            cache[root] = cache_item
            control.focus_mode = Control.FocusMode.FOCUS_NONE
            control.mouse_filter = Control.MouseFilter.MOUSE_FILTER_IGNORE
            control.mouse_force_pass_scroll_events == false
    for child in root.get_children():
        _disable_focus(child, cache)

godotengine / godot-proposals