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

Fix Label3D, TextMesh & Font not following project default theme in editor #89311

Merged
merged 1 commit into from
Mar 10, 2024

Conversation

Mickeon
Copy link
Contributor

@Mickeon Mickeon commented Mar 9, 2024

Seems to fix #85693 without side effects, and I honestly have no idea why.

@Mickeon Mickeon added this to the 4.3 milestone Mar 9, 2024
@Mickeon Mickeon requested a review from a team March 9, 2024 11:49
@Mickeon Mickeon requested a review from a team as a code owner March 9, 2024 11:49
for (const Ref<Theme> &theme : global_context->get_themes()) {
List<Ref<Theme>> themes = global_context->get_themes();
if (Engine::get_singleton()->is_editor_hint()) {
themes.push_front(ThemeDB::get_singleton()->get_project_theme()); // Preview in the editor properly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Preview in the editor properly.

Not sure if this comment is necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me it feels weird that it has to be done this way. I can remove the comment though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm content either way

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly the same code is used in the TextMesh and by Font to get the default font, so these might need the same change, but I have no idea if this is a correct way to fix it.

@Mickeon Mickeon force-pushed the label_3d_is_a_mess branch from afbff81 to ba86704 Compare March 9, 2024 16:35
@Mickeon
Copy link
Contributor Author

Mickeon commented Mar 9, 2024

Curious, but you're right. The problem does also exist for those two.
PR before,
image
PR after,
image

@Mickeon Mickeon changed the title Fix Label3D font does not follow project default theme in editor Fix Label3D, TextMesh & Font not following project default theme in editor Mar 9, 2024
@akien-mga akien-mga requested a review from KoBeWi March 9, 2024 20:46
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why it works is that the global theme context in editor does not contain project theme:

if (!Engine::get_singleton()->is_editor_hint()) {
themes.push_back(project_theme);
}

You are pushing it to front of the iterated theme list, so the Label3D checks it and finds the font (get_font() will always return something if a default font is assigned).

btw the get_fallback_theme() code below is most likely dead, as fallback theme is the same as the last theme in the List above.

@akien-mga akien-mga merged commit 54f2916 into godotengine:master Mar 10, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Mickeon Mickeon deleted the label_3d_is_a_mess branch March 10, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Label3D font does not follow project default theme in editor anymore (in-game is fine)
5 participants