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

Add Control states for more flexible control of theme properties #2498

Open
dalexeev opened this issue Mar 23, 2021 · 12 comments
Open

Add Control states for more flexible control of theme properties #2498

dalexeev opened this issue Mar 23, 2021 · 12 comments

Comments

@dalexeev
Copy link
Member

dalexeev commented Mar 23, 2021

Describe the project you are working on

A game in which only the keyboard is used for input.

Describe the problem or limitation you are having in your project

I want to use different colors for the text of a disabled and disabled focused button:

But only the following theme properties are available:

Button has a large number of combinations of different states: NORMAL (none of the following states), DISABLED, FOCUS, HOVER, PRESSED (if toggle_mode is on), ACTIVE (when the button is pressed but not released). There are 2 ** 5 = 32 states in total. Yes, some of them are impossible (DISABLED & ACTIVE) or hardly need to be distinguished, but in any case, this is too large a number to list them all.

To fix this, I have to dynamically change the style when focus enters/exits.

func _on_focus_entered():
    add_color_override("font_color_disabled", Color("cc6666"))

func _on_focus_exited():
    add_color_override("font_color_disabled", Color("663333"))

It is quite difficult to organically redefine the standard behavior, since, together with other edits, the code becomes less accurate, properties appear that replace the standard ones, and some of the standard ones become properties-which-should-not-be-changed.

Another example: font_color_hover is used for both the focused and the hovered button (and there is also a bug).

Details

A button's focused state activates both it's Hover and Focus styles. When the mouse is then used to hover and unhover the focused button, the button loses it's hovered state. I think this is a bug. I think the behaviour should be that the Hover Style is only activated when the button is Hovered by the mouse. Such that when a button is focused, but not hovered, it should not activate the Hovered state. Does this align with anyone else' experience or am I missing something?

godotengine/godot#30644

Note

This is not directly related to this proposal: there is no Control.FOCUS_KEYBOARD mode, in addition to FOCUS_NONE, FOCUS_CLICK and FOCUS_ALL. Have to use mouse_filter = Control.MOUSE_FILTER_IGNORE.

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

Add the ability to the Control class to set states that will be taken into account by the get_{color,constant,font,icon,stylebox}() functions (further – get_*()). I think it should be the set_state() function that both engine code and custom code (for custom states) will use. For example, when a button is disabled, the engine calls button.set_state("disabled", true), when it receives focus, calls button.set_state("focus", true).

At any given time, Control can have a set of one or many states (they work like flags). For example .active.focus.hover. The order is not important (.focus.disabled is the same as .disabled.focus, it is better to use alphabetical everywhere). If none of the states are active, Control is considered to be in the .normal state. It is also possible to use custom states.

Overrides are added in the same way as before:

button.add_color_override("font_color.normal", Color("666666"))
button.add_color_override("font_color.disabled", Color("663333"))
button.add_color_override("font_color.focus", Color("cccccc"))
button.add_color_override("font_color.active", Color("ffffff"))
button.add_color_override("font_color.disabled.focus", Color("cc6666"))

In the .tscn file:

custom_colors/font_color.normal = Color( 0.4, 0.4, 0.4, 1 )
custom_colors/font_color.disabled = Color( 0.4, 0.2, 0.2, 1 )
custom_colors/font_color.focus = Color( 0.8, 0.8, 0.8, 1 )
custom_colors/font_color.active = Color( 1, 1, 1, 1 )
custom_colors/font_color.disabled.focus = Color( 0.8, 0.4, 0.4, 1 )

The order of overrides is important for rules of the same complexity (more details below).

To get the property value, taking into account the current states of the control, use the get_*("property_name_without_dots") function. In my example, calling button.get_color("font_color") will return:

  • Color("cc6666") if the button is disabled, focused and possibly has some other states;
  • Color("cccccc") if the button is focused, not disabled, inactive and, possibly, has some other states;
  • Color("663333") if the button is disabled, not focused, inactive and, possibly, has some other states;
  • Color("666666") if the button has no special states (since it has the .normal state);
  • Color("666666") if the button has only the state .lalala (the value from .normal, since there are no rules for .lalala).

Also, the get_*() functions can be used to get override values if their argument contains a dot. In my example:

  • button.get_color("font_color.normal") always returns Color("666666") regardless of the current set of button states;
  • button.get_color("font_color.disabled") always returns Color("663333") regardless of the current set of button states.

Note that set_state() only changes the state of the control. Redrawing occurs in the _draw() function (which becomes easier due to the change in the get_*() function logic). For example, when the disabled property changes, the set_disabled() setter calls set_state("disabled", p_value), then calls update().

The theme system works as before. If custom font_color is set for the control, but custom font_color.hover is not set, then the theme rule will be applied in the .hover state.

Override priority

The priority depends on the complexity and order of the rules. If a control has .state1.state2.state3 states, then the following rules will apply:

  • .state1.state2.state3 (priority 1)
  • .state1.state2, .state2.state3, .state1.state3 (priority 2)
  • .state1, .state2, .state3 (priority 3)
  • .normal (priority 4)

The last one is selected from the rules of the same priority. In the example above, the focused active button will have the text of ffffff color, because the .active rule comes after .focus.

Internal elements

Some of the styles are applied not to the entire control itself, but to some of its visual parts. For example, an ItemList consists of a number of items, each of which can be selected or not. For this case, the set_state() function is not suitable, since this state does not apply to the ItemList itself, but to its items. How to solve this problem is worth considering. I have some ideas, I will add them to this proposal later.

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

I don't know which of the options is more optimal, but in addition to the "property - rule" option, there is a "rule - property" option (as in CSS):

custom_colors/normal/font_color = Color( 0.4, 0.4, 0.4, 1 )
custom_colors/disabled/font_color = Color( 0.4, 0.2, 0.2, 1 )
custom_colors/focus/font_color = Color( 0.8, 0.8, 0.8, 1 )
custom_colors/active/font_color = Color( 1, 1, 1, 1 )
custom_colors/disabled.focus/font_color = Color( 0.8, 0.4, 0.4, 1 )

In fact, it seems to me that the difference between these structures is small. The get_*() function algorithm does not have to adhere to the same structure in which this data will be stored. The proposed system is much simpler than CSS, since there are no complex selectors (the scene tree is not considered), so there must be an effective algorithm for determining the desired value according to the rules, taking into account the current state, which avoids a long search for rules.

Option 1 (I like it better)

Option 2

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

No, this requires modifying the Control class and its descendants.

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

This cannot be done with an add-on.

@YuriSizov
Copy link
Contributor

YuriSizov commented Mar 23, 2021

I think it should be set_theme_state instead of set_state, but otherwise you're onto something with this. 👍

PS. I'll try to look into what it would require to properly implement after the Theme Editor remake.

@dalexeev
Copy link
Member Author

I think it should be set_theme_state instead of set_state

I agree that set_state is too general a name, but I'm not sure if set_theme_state is the most appropriate name. I'm not even sure which term is better to use here: "state", "flag" (but flags are usually numbers), "class" (due to the similarity of the .syntax to CSS classes), "feature", or "tag". We need a name that emphasizes that it is a property of the control itself (and not something separate from it) that changes frequently and controls the appearance of the control.

@YuriSizov
Copy link
Contributor

YuriSizov commented Mar 23, 2021

We need a name that emphasizes that it is a property of the control itself

We don't need to go too general. This is a state related to the visuals of a control, so it's a theme state, because everything visuals related is dictated by theme. This state would probably also be defined on a theme for each control, so that theme items can be configured to use it. And all theme items are retrieved and set with get/set_theme_X in master.

@Shadowblitz16
Copy link

what about focused pressed and focused hovered?
Controls don't really have a way to detect being pressed directly but it is very useful for theming

@Shadowblitz16
Copy link

Shadowblitz16 commented Jul 15, 2022

@dalexeev as for this ...
image
you could do that but have a focus and normal style for each catagory..

aka...

  • normal
    • unfocused
    • focused
  • hovered
    • unfocused
    • focused
  • pressed
    • unfocused
    • focused
  • disabled
    • unfocused
    • focused

Also I do think focus should be maybe seperated into focus overlay and focus with focus defaulting to empty.
the reason being is it is useful in pixel art to have a focus that replaces instead of just overlaying on the top of the unfocused style

Or we could just have a focus is overlay flag constant which defaults to true

@Tim-Fronsee
Copy link

Tim-Fronsee commented Sep 13, 2022

I ran into the focus pressed issue as well while testing button UI navigation (using a keyboard) with a custom theme.

I've implemented a possible solution for theming focus states without changing the current implementation for focus.

Namely, by setting focus to StyleBoxEmpty and setting the desired focus_pressed StyleBox, it is possible to customize and display a specialized focus_pressed state.

If it's okay let me know and I'll open a PR.

@Shadowblitz16
Copy link

Any news on this?
I kinda want to download godot source and try moving button stuff to control.
I have no c++ experience though and last time I tried doing this I failed due not being able to follow godot's source code

@dalexeev
Copy link
Member Author

It's too big of a change, so it's unlikely to be added in 4.x. I think in 5.0 the theme system will be improved, in this way or otherwise.

@Shadowblitz16
Copy link

Shadowblitz16 commented Feb 19, 2023

I can't wait another 5 years sorry.
this is something that should have been possible from the beginning

@davthedev
Copy link

This proposal is interesting.

I have been proposing some minor adjustments to enable hover styles on select controls in the meantime, that are ready to implement (#6478 and #6462).

Nevertheless, a real architecture of some theme states as mentioned is a great idea. In web development, I leverage the (actually underused) power of CSS to implement the combination of states I would like to style such as :hover :active :focus and similar. The name of the concept in CSS is "pseudo-classes" FYI.

This pseudo-classes system is used not just for interaction states. It is possible to target the first / last / nth item of a group, or "every nth" to achieve an even-odd table row coloring for example.

Here is a reference of the CSS pseudo-classes and the possibilities: https://developer.mozilla.org/en-US/docs/Web/CSS/Pseudo-classes

I recommend not to hardcode the list of theme states, which would open up possibilities. Each control class would handle its current theme states and pass them to a theme resolver when asking for a StyleBox render, color value, font, constant, etc...

For instance, the ItemList may support equivalents of CSS :first-child, :last-child; so would an HBoxContainer and pass those down to some of its children.

@scottfoxyt
Copy link

I agree that something like this should be implemented in 4.x. Any games with a large amount of ui almost require something like this, so the way it works in 4.x currently I would say is borderline unsuitable for a release. Also, please see the issue just mentioned for some state combinations I personally would like to see.

@Zireael07
Copy link

4.x is too far in release process to go for such extremely large overhauls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants