godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.05k stars 65 forks source link

Add seperate bulk theme override functions to be used seperately from editor #9672

Closed peachey2k2 closed 1 week ago

peachey2k2 commented 2 weeks ago

Describe the project you are working on

I'm making a plugin that lets the user add a background to their Godot editor. For that to be effective, I use an editor theme to make most of the GUI semi-transparent. I also give them a color picker to decide on the colors for that. When they pick a color, the plugin overwrites some of the theme colors.

Describe the problem or limitation you are having in your project

Right now, Godot offers the begin_bulk_theme_override() and end_bulk_theme_override() functions if you want to edit multiple components of a theme. This is to prevent the engine from recompiling the theme every time you make a change. This works great in most cases but for editor themes, it just doesn't work. This is because the editor itself also uses those same functions, and every change will cause it to do so under the hood. When I call begin_bulk_theme_override() and edit some styleboxes, engine automatically calls begin_bulk_theme_override() and end_bulk_theme_override() on the nodes that need to be changed.

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

A solution would be to use seperate functions (and seperate variables tied to them) for uses in the editor code and user code. It'd block this entire thing from happening while not affecting other use cases.

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

It'll be the same in code. It just wouldn't interact with the editor's own calls for that function. Here's a demonstration using my plugin code:

func change_theme_color(col:Color):
    #Benchmark.start("change theme color")

    var controls_list = get_all_controls([base])

    for node:Control in controls_list:
        node.begin_bulk_theme_override()

    var col2 := Color(col, col.a/2.0)
    var col3 := Color(col, min(col.a/col.v, col.a/4.0))
    change_color("EditorStyles", "Background", col)
    change_color("EditorStyles", "BottomPanel", col)
    change_color("EditorStyles", "BottomPanelDebuggerOverride", col)
    change_color("EditorStyles", "Content", col)
    change_color("EditorStyles", "LaunchPadNormal", col)

    change_color("TabContainer", "panel", col)
    change_color("TabContainer", "tab_selected", col)
    change_color("TabContainer", "tab_unselected", col2)

    change_color("TabBar", "tab_selected", col)
    change_color("TabBar", "tab_unselected", col2)

    change_color("TabContainerOdd", "tab_selected", col)
    change_color("TabContainerOdd", "panel", col2)

    change_color("Button", "normal", col3)
    change_color("MenuButton", "normal", col3)
    change_color("OptionButton", "normal", col3)
    change_color("RichTextLabel", "normal", col3)
    change_color("LineEdit", "normal", col3)
    change_color("LineEdit", "read_only", col3)

    change_color("EditorInspectorCategory", "bg", col2)

    for node:Control in controls_list:
        node.end_bulk_theme_override()

    #Benchmark.end("change theme color")

func get_all_controls(nodes:Array[Node]) -> Array[Node]:
    var out:Array[Node] = []
    for node in nodes:
        if node is Control: out.append(node)
        var children := node.get_children() as Array[Node]
        out += get_all_controls(children)
    return out

func change_color(type:String, name:String, col:Color):
    var box:StyleBoxFlat = theme.get_stylebox(name, type)
    box.bg_color = col

As you can see from the comments, I did some bechmarking, and it takes around 10-12 seconds for this function to do all its work. The more change_color()s I add, the slower it gets, since it will reapply the theme with every change. It also prints a whole bunch of errors.

image

If we look at the code, this shows the error is called when end_bulk_theme_override() realizes that bulk_theme_override isn't enabled.

image

But in reality, I did enable it, and the editor itself re-enabled and disabled it, and this is the issue. If we get a second function to do this where the editor itself doesn't edit it, that'd solve the problem.

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

I tried a whole bunch of things like resetting the custom theme editor setting, using a seperate theme that i apply the changes and then merge with the old one, disabling processing or hiding the editor base node etc. None of those worked.

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

Nobody wants to install a separate plugin just to edit themes without informing the engine.

RedMser commented 2 weeks ago

Instead of a new function, it could be like a counter that increments for begin and decrements for end bulk calls. 0 applies the bulk operations, but now you can have multiple begins and it'll work fine as long as they're paired with the same amount of end calls.

peachey2k2 commented 2 weeks ago

Agreed, that sounds way more elegant and wouldn't cause much confusion either. The current bulk theme boolean isn't exposed so this wouldn't even break the old functionality either.

dugramen commented 2 weeks ago

So for StyleBox in particular, you can use set_block_signals(true) to temporarily block signals. Something like this:

func change_color(type:String, name:String, col:Color):
    var box:StyleBoxFlat = theme.get_stylebox(name, type)
        box.set_block_signals(true)
    box.bg_color = col
        box.set_block_signals(false)

And I think that should prevent it from updating everything with every little edit

peachey2k2 commented 1 week ago

Yup, just realized that works. I just have to call emit_changed() after all the changes to update it. Also considering @KoBeWi's comment in the related pull request, it doesn't make much sense to change that.