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

Theme.get_type_list() does not list empty type variations #84120

Closed
lamdevhs opened this issue Oct 28, 2023 · 0 comments · Fixed by #84127
Closed

Theme.get_type_list() does not list empty type variations #84120

lamdevhs opened this issue Oct 28, 2023 · 0 comments · Fixed by #84127
Milestone

Comments

@lamdevhs
Copy link

lamdevhs commented Oct 28, 2023

Godot version

v4.1.2.stable.arch_linux

System information

Godot v4.1.2.stable unknown - Arch Linux #1 SMP PREEMPT_DYNAMIC Sat, 23 Sep 2023 16:57:15 +0000 - Tty - Vulkan (Compatibility) - Mesa Intel(R) HD Graphics 4600 (HSW GT2) () - Intel(R) Core(TM) i5-4590 CPU @ 3.30GHz (4 Threads)

Issue description

Theme.get_type_list() does not list theme type variations for which no theme item (no constant, no icon, no font, etc) has been set.

Not only is it inconsistent with the behavior description in the docs, and with how Themes are saved as resources (because you can save empty theme variations by calling Theme.set_type_variation("Foo", "Control")), it is inconsistent with Theme.get_type_variation_list(), which does list all type variations regardless of whether theme items have been set for them or not.

This bug means it's impossible to retrieve a full list of type variations (since as far as I can tell, there's no way to list all base_types for which a type variation exists, for example). In essence, the theme resource is partially write-only.

The reason for this bug is quite simple: the code for get_type_list() goes through all the different theme items (fonts, icons, styles) and gathers all theme types, whereas get_type_variation_list() goes through all the variations of the given base_type, regardless of theme items.

The fix is quite simple: inside get_type_list(), for every base_type in variation_base_map, add to the HashSet every type variation associated with that base_type, mimicking get_type_variation_list().

Code links:
get_type_list():

void Theme::get_type_list(List<StringName> *p_list) const {

get_type_variation_list():
void Theme::get_type_variation_list(const StringName &p_base_type, List<StringName> *p_list) const {

Steps to reproduce

var theme := Theme.new()

theme.set_type_variation("Foo", "BoxContainer")
print(theme.get_type_list().has("Foo")) # should print true, but prints false
print(theme.get_type_variation_list("BoxContainer")) # prints true, as is logical

theme.set_theme_item(Theme.DATA_TYPE_CONSTANT, "separation", "Foo", 42)
print(theme.get_type_list().has("Foo")) # now it prints true
print(theme.get_type_variation_list("BoxContainer")) # still prints true

Minimal reproduction project

N/A

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

Successfully merging a pull request may close this issue.

4 participants