Introduce `ThemeDB`/`ThemeServer`, register theme properties with it to define stable theming API for controls and windows, allow to define theme properties in scripts

Note: This proposal text is kept as it originally was, but the follow-up comments explore and extend the actual implementation, as well as which parts of the proposal to keep and which to reject. As such, this description predates the most recent title of this proposal.

Describe the project you are working on

Godot engine

Describe the problem or limitation you are having in your project

While theming is a powerful tool for customizing the looks of the UI in your Godot projects, the underlying tech has shortcomings and limitations.

First of all, theme properties are still "stringy" as of the current state of Godot 4.0, and remain one of the last bits of Godot scripting which is so. This means worse code completion, more room for typos, less defined API. As we move towards first-class citizens for other "stringy" properties, theme items feel like a relic of a past. Additionally, the built-in theme properties have poor naming convention (none, really), which is partially obfuscated by the use of strings without proper property definitions.

Secondly, theme properties are weakly defined. While controls require them and use them, and have not only visual details but bits of logic based on the theme properties (e.g. margins of a stylebox), controls don't actually define their contract with the theming system. Instead, they kind of hope that the theme applied to them, or one of the higher-order themes, is going to have the necessary items. The actual definition happens in scene/resources/default_theme.cpp instead, where the default project theme is created, used as a fallback for everything.

The default theme definitions is exactly what we use to create the list of control's theme override properties, for example. Same approach is used in the theme editor. And it's all, once again, string-based. This leads to the default theme missing definitions that some controls require, and, vice versa, having outdated or forever invalid definitions which the corresponding controls don't actually need. And as maintainers we have no tools to validate them, make sure we define all we need to define and remove all the bits we don't actually need.

All in all, it's a frustrating gap between a control and its default styling. And as a result, custom controls don't have a nice way to hook into the existing system and can't benefit from having defined theme overrides and default styling. You may be able to hack into an instance of the default theme, but that's not a good and stable solution, and definitely not an intuitive one.

Thirdly, theming is limited to controls, while it can be useful for other parts of your project. For example, you can use a theme to carry color information to your in-game units. You can of course create the necessary boilerplate code to imitate theme application and propagation to Node2D or Node3D/Spatial, but that becomes harder to sell with Godot 4.0, where we have a new kind of node that also implements theming — Window. Implementing support for both controls and windows required hacking into Control's existing theming logic and making Windows inherently dependent on Control's code to reduce code duplication.

But I think it's a good sign that we might want to extend theming to just about any node, while keeping this system opt-in for the most of them (but enabled by default for controls and windows).

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

I propose several interconnected things to make the whole theming system more robust, extensible, and intuitive.

1. Make controls define their theme properties, and store them in ClassDB like normal properties (but separate).

Let's say, we introduce a new macro, ADD_THEME_PROPERTY that would create ClassDB definitions, and provide sensible default values for them if necessary. We'll use these definitions wherever and whenever we need a list of theme properties supported by each control (in the inspector, the theme editor, etc). The default theme specifically and the Theme resource in general remains the same, but using this now defined theming contract we can evaluate if the default theme has all the necessary items and if it has excessive items.

To reduce stringy-ness of theme properties internally, we also introduce a ThemeProperty type, or something similar. We'll use this type to represent each Control's theme property. It will be able to look up definitions in the scene tree the same as control does now, but abstracted away from the rest of the control code. It will also handle local overrides for controls. I think that in the engine code ThemeProperty members can be normal class properties, so accessing them would be natural and intuitive.

2. We expose everything from the first point to scripting.

The ThemeProperty type would be a naturally exposed class. As for properties themselves, I propose we add an annotation @theme_property, to separate them from regular properties (just like, say, signals are separated, despite being member-like). I think this separation is important because theme properties would be their own ClassDB entity. Plus, they have specific pre-defined behavior (like the theme overrides section), and this annotation would be the scripting way to achieve the same as ADD_THEME_PROPERTY internally.

An example would be

extends Control

@theme_property(Theme.DATA_TYPE_COLOR) var background_color : ThemeProperty
# or
@theme_property_color var background_color : ThemeProperty
# or
@theme_property var background_color : ThemePropertyColor

func _draw():
    var rect = Rect2(Vector2(-10, -10), Vector2(20, 20))
    draw_rect(rect, background_color.get())

With such tools, custom controls should become as natural as built-ins, benefiting from the native editor integration and stable, easily discoverable API.

3. Move theming logic from `Control` to `Node`, make it opt-in.

This removes the dependency of Window on Control, but also opens up theming to be usable outside of UI. All nodes would have an innate ability to accept theme and theming notifications, but we won't enable it by default. Instead a node needs to explicitly turn this on.

One of the features of theming is theme propagation and theme merging along a branch of nodes. So I think that opting in can be done by defining a property that would store node's own theme. In engine code it can be a ADD_THEME macro, acting similar to ADD_THEME_PROPERTY, but for the theme itself. And in scripting it can be a @theme annotation, e.g.

extends Node

@theme var theme : Theme

Opting in would make nodes a part of the propagation system, so they can affect underlying "themeable" nodes just like controls affect their child controls. Basically, what now happens between an uninterrupted chain of controls is going to happen between all the nodes, with nodes that opted into theming interacting and the rest of the nodes being ignored.

You can view this as a poor man's multiple inheritance/interface implementation. Nodes that opt into the theming system in a way implement an "IThemeable" interface or extend a "Themeable" class, without breaking their existing inheritance chain. I think that my approach fits Godot's and GDScript vision more than interfaces and multiple inheritance, at least as it currently stands.

What I wouldn't change...

... is the Theme resource and the editor UI for anything related. I think what I propose would improve things without any changes there.

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

See above.

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

Points 1 and 2 are about core changes to how theme properties are defined. Point 3 can be implemented with boilerplate code, but it's not just a few lines.

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

This is a core change.

This would also remove the issue of Window nodes not showing the Theme Overrides section despite having theme properties. Also, theme properties could be added to any potential 3D UI nodes, like the recently-added Label3D.

My views on this:

Short version:

I am not particularly against this, but my feeling is that if we do this, we either more or less leave things as is, or should do it all the way and implement #2832. Then figure out how to improve the usability after having eliminated Theme and its categories in favor of just variants.

Long version:

The thing is that theming in general works pretty well, there are some rough edges but its pretty good. Users are doing all sorts of things with it. It's a very WYSIWYG implementation that is rather simple. This is what is makes me feel less inclined to do further changes to it.

My line of thinking is more or less the following:

Themeing was more or less fine in 3.x
In 4.x, you have some things like Window and Label3D that use themeing (or not, probably Label3D honestly makes no sense).
Implementing everything twice in Window and Control can be a hassle. I don't think its terrible though. If you have OCD I can understand your pain, but to me it's fine.
But, the main source of a hassle when doing code changes is because, due to historical reasons, Theme does not use variants to store configuration. This was not intentional, its because the UI library in Godot, derived from earlier engines I made (and other stuff like Chibitracker), did not work like this. Because of this code is repeated for every single theme category (icon, style, font, etc), which makes changing it a pain.

So based on the above information, what I generally think about here is:

Should theme remove the categories and be changed to use Variants? This would make the code simpler, but the fact it has categories also makes it simpler to use, and easier to edit. This is why I did not touch them so far.
Is there really a need to add theme information to ClassDB? Why not simply have a ThemeDB ? Then why not just keep using things as they are with the default theme?. If the problem is simply that registering all the default theme stuff in default_theme.cpp is hacky, then why not simply register default theme information in _bind_methods? Is it really better to do this? because the default_theme creation is pretty easy to just hack around. Again, this is why I did not change this in all these years.
Is moving Theme information to Node worth it? I think not because its a lot of stuff where anything not UI has nothing to do with it. If it was something for configurations and we got rid of the theme categories (again as proposed in #2832) and in favor of Variant, this would make more sense because it would be a change that simplifies the codebase a lot. But it would hurt usability, so how could usability be improved if we do this? I think this is the path to pursue if we go forward with this proposal.

You can imagine that I went over these topics for more than 10 years in my head, but nothing really convinced me more than doing what we are doing now. I understand that there are pain points, but to me all seem like good tradeoffs. I don't believe in perfect solutions, but I do believe that those who pursue them are wrong most of the time. Not saying we should leave things as it is, but the alternative has to be really good to justify it.

If we do changes to themeing, also, I think we need to make it easier to scale the UI (like change DPI on the fly). This really is one of the biggest features missing to me.

Should theme remove the categories and be changed to use Variants? This would make the code simpler, but the fact it has categories also makes it simpler to use, and easier to edit. This is why I did not touch them so far.

I don't think we should remove categories of theme data types. They give nice separation when looking for the property that you need. Likely, this kind of grouping would still be desired even if you did change it to Variants, we would just have to create groups ad-hoc for the UI, and handle a lot of unexpected cases.

Granted, current categories are lacking. We have numeric constants that are often used as booleans, and at the same time can't be used for floating point values, which are not rare when theming. Adding more categories would almost certainly make code even less readable with a lot of copy-pasting and opportunities to make a mistake. Maybe theme constants can be a variant, and other types can be as is, focused and easy to find? Constant already sounds ambiguous enough.

Is there really a need to add theme information to ClassDB? Why not simply have a ThemeDB ?

I'm cautious about creating new data tables, but if you think it's better to have a dedicated database for themes, that's fine by me. It would probably be even better in practice. My focus was mainly on the API usage, not the backend of it. ClassDB looked like a natural choice, but nothing here is bound to it.

Then why not just keep using things as they are with the default theme?. If the problem is simply that registering all the default theme stuff in default_theme.cpp is hacky, then why not simply register default theme information in _bind_methods?

That's what I propose, yes. That we register theme properties in _bind_methods of each control, so controls have stable, predictable, discoverable and maintainable theming API. Current approach is a huge problem for consistency, because properties are volatile and controls have no knowledge about the API they need to use. We still have properties that are left by mistake, or that were never in use, or that are used but missing from the default theme (and thus, no tool in the editor is aware of them and can suggest them to the user, like icon color properties in buttons).

Is moving Theme information to Node worth it?

I agree that it makes more sense to do it as your proposed "configuration" object rather than theme. I am not adamant about this change anyway, so we can reconsider it later. For now I'd like to improve themes, because I think they work fine already, they just need stability. So I only am adamant about my points 1 and 2.

If we do changes to themeing, also, I think we need to make it easier to scale the UI (like change DPI on the fly). This really is one of the biggest features missing to me.

This is already mostly handled by https://github.com/godotengine/godot/pull/52170 (which was backported to 3.4). If you need Camera2D zoom to not change when changing the UI scale, divide its zoom by the scale factor (in 3.x) or multiply the zoom by the scale factor (in 3.x) at the same time.

SVG icon/stylebox re-rendering when the scale changes isn't implemented yet though.

@YuriSizov That may work, there are now static methods in the binder, so you can do things like static Theme::set_default_style(name,class,value) which you can do from _bind_methods as some ADD_THEME_PROPERTY macro.

Ok, I've been thinking a bit about implementing point 1 — internal changes to make theme properties first class properties of controls (and windows) and make that part of the theming API more stable. Here's what I envision:

1. We introduce a new `ThemeProperty` class.

For simplicity I'll assume it's ref counted. We can introduce subclasses for each data type, but I think we can work with a single class that would store both data type and value, and return Variant. Most places should be able to correctly cast it to the needed value, I believe.

We would also store a reference to the owner node so that we can work with it to look up inherited values. This should allow to move some API from Controls to this new class, making it easier to reuse it in Window, and better grouped. This includes all theme lookup internal logic (available as static methods at the moment). I think it should yield better maintainability down the line.

I also want to separately refactor the theme owner to be a single property, because juggling the window/control logic everywhere is getting tedious and makes code more confusing. So with this ThemeProperty would be able to use a unified interface to work for either Window or Control.


class ThemeProperty : public RefCounted {

private:
    Node *owner = nullptr;
    Theme::DataType data_type;
    Variant value;

public:
    void set_owner(Node *p_owner);

    Variant get() const;
    Variant get_inherited() const;

    void set_override(Variant value);
    bool has_override() const;
    Variant get_override() const;
    void clear_override();
};

2. Controls and Windows would store a collection of their theme properties.

That way we can use it whenever we need to list all control's own theme properties (e.g. for the inspector's theme overrides section, for the theme editor, for the docs). It's protected so extending classes can access it freely. Can also be done with methods, if this is the preferred way.

This collection is grouped by the theme data type, so we can preserve the existing grouping of theme properties without much hassle.

With this collection in place, _get_property_list and other related methods need to be updated to use it instead of fetching default theme definitions.


class Control : public CanvasItem {

protected:
    HashMap<Theme::DataType, List<Ref<ThemeProperty>>> theme_properties;

};

3. We define a new macro to quickly configure theme properties for controls (and windows).

This macro automatically adds theme properties to the default theme, as well as connects the property to the owner node. We may also want a pairing macro for use in node's destructor to clean up the references.

For this to work, we also need to add a new static method to Theme that would return default values for all theme data types. We already have static properties for some of them, but not all of them. So we'd need to introduce those, and then use them with this method.

This macro only defines properties, so the value must be a default one. We could add another macro to also set a value, but I would prefer that default_theme.cpp still described the styling information. This macro is needed to create a firm bond for the property and guarantee its existence, not to define its value.

Edit: On a second thought, this other macro is going to be useful for modules that define their own controls. And similar logic would probably apply to the scripting part of this proposal.

Currently the default theme is created anew by the initialization method, so it also needs to be adjusted to not override these definitions. Luckly, scene types are registered before the theme is created, so the order of operation doesn't need changing.


#define ADD_THEME_PROPERTY(m_data_type, m_property) \
Theme::get_default()->set_theme_item(m_data_type, #m_property, get_class_static(), Theme::get_default_item(m_data_type)) \
m_property->set_owner(self); \
theme_properties[m_data_type].push_back(m_property);

#define ADD_THEME_PROPERTYV(m_data_type, m_property, m_value) \
Theme::get_default()->set_theme_item(m_data_type, #m_property, get_class_static(), m_value) \
m_property->set_owner(self); \
theme_properties[m_data_type].push_back(m_property);

4. We utilize the aforementioned additions to improve Controls and Windows.

We use the new type in our built-in control and window nodes as needed to define the required theme properties. Stretch goal — make sure it also works for the editor specific controls.

The example below shows how the new ThemeProperty can be used to get the value. From its header file overview above it should be obvious how to add local overrides if needed. Basically, for the most part, calls to get_theme_color("my_color") would be replaced with my_color.get().


// my_control.h
class MyControl : public Control {

protected:
    Ref<ThemeProperty> my_color;
    Ref<ThemeProperty> my_separation;

};

// my_control.cpp
void MyControl::_notification(int what) {

    if (what == NOTIFICATION_DRAW) {
        draw_rect(Rect2(), my_color.get());
    }

}

void MyControl::_bind_methods() {

    ADD_THEME_PROPERTY(Theme::DATA_TYPE_COLOR, my_color);

}

Does that make sense to everyone?

Note, that this doesn't remove any existing ways to interact with themes, and neither do I plan to do it. I just want to stabilize the default properties of each control (and window).

PS. On top of that, we will be able to add tests or sanity checks if the default theme adds styling information for every existing theme property or not. This is one example of the maintainability improvements.

I realized that the mock API above doesn't really make it easier to fetch theme properties for docs or the theme editor. In fact, it wouldn't work at all this way, because _bind_methods is obviously static. So for a list of control's properties we'd need to either make the hashmap static, or introduce some engine singleton.

This was what I wanted to use ClassDB for, but reduz is right, we probably better make a new, dedicated one. Some ThemeDB or ThemeServer that would collect property definitions for anything that wants to define its own properties. The biggest change I need to make to the mock API above is that ThemeProperty cannot be used for both singleton definitions and control's internal property. It was mostly designed for the latter, so for ThemeDB/Server another kind of data storage would need to be created.

This also changes ADD_THEME_PROPERTY, as no instance operation would be allowed there. Association between a control and its theme properties needs to happen in some other way.

PS. An introduction of a theme-related singleton would also allow to move some singleton bits from the Theme resource. Like fallback values, or the default theme reference. I think it's a good idea as well, keeping the resource code clean and purposeful.

Are these changes in the code and API ? Or would this change creating themes in the editor and setting them to nodes, as well ?

@me2beats This proposal changes how controls work with themes, but doesn't change how themes work. It also doesn't remove any existing functionality, just creates a framework for more stable API for controls. Plus some reorganization internally. Additionally it would also expose the same improvements to scripting, so user scripted controls could also benefit from these changes. But once again, the themes themselves don't change, neither their structure, nor their tooling. I just see no reason for that. At least, not as a part of this proposal.

@YuriSizov is there a timeline for this whole topic? Seems like the pre-requisites were merged lately so I assume this could be part of 4.2?

@dotlogix Depends on what it is that you want from this proposal 🙂 A lot of internal work has been done and is a part of the upcoming 4.2. I don't think users would notice these changes much (other than some new docs or overrides available, and some invalid ones being gone).

The part about allowing scripts to define theme properties for their GUI classes is not implemented and won't be done for 4.2. So, what interests you here?

Unfortunately I think it is more the second thing you mentioned.

Actually what I want to do is to add some theme properties to my custom container control and also provide the option for theme_overrides. Unfortunately I think there is currently no way of achieving this. I registered the properties with the default theme and they show up in the theme editor. I can also retrieve them when setting the variant to the class_name (workaround as class_name is not respected in the current implementation). But what I can't figure out is how to define theme_overrides.

Maybe you know a workaround until this is fixed properly? I would like to avoid real properties because it would be a breaking change and reset property values afterwards and I plan to maybe publish my addon at some point.

Maybe you know a workaround until this is fixed properly?

You can't hook into the way existing properties are generated, but you can generate them in your class' script in a similar manner. You need to implement _get_property_list, _set, and _get. See how it's done in Control: https://github.com/godotengine/godot/blob/3ed4497113fa10611b90290ce22a751fb9d26e2e/scene/gui/control.cpp#L263 You can do the same thing in GDScript. They will likely end up being displayed under your class and not where built-in overrides are, but in case this is properly supported in the future, the names would be the same so it won't be a breaking change.

Hm looking at this code, wouldn't it be super easy to allow overriding the GetClassName function from scripts? As far as I can tell this would make it possible to just define the theme properties in the default theme and don't need to override anything besides GetClassName. This could even be done behind the scenes using the class_name of the script instead.

That's a different problem entirely and is not the way to solve this particular issue. get_class_name is very much core to a lot of things.

godotengine / godot-proposals