Skip to content

Fix auto-translation of EngineUpdateLabel#119606

Open
KoBeWi wants to merge 1 commit into
godotengine:masterfrom
KoBeWi:language_update_is_bereit
Open

Fix auto-translation of EngineUpdateLabel#119606
KoBeWi wants to merge 1 commit into
godotengine:masterfrom
KoBeWi:language_update_is_bereit

Conversation

@KoBeWi
Copy link
Copy Markdown
Member

@KoBeWi KoBeWi commented May 20, 2026

What problem(s) does this PR solve?

Part of #104574

Additional information

This one was tricky, because the update label has dynamic messages that can't be freely updated. I added a field for current message and for format argument. On language change, the message is updated using vformat(TTR(current_message), stored_format).

@KoBeWi KoBeWi added this to the 4.x milestone May 20, 2026
@KoBeWi KoBeWi requested a review from a team as a code owner May 20, 2026 16:43
Copy link
Copy Markdown
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.

Code looks good, but variable names might be confusing.

if (message_format.get_type() == Variant::NIL) {
set_text(TTR(current_message));
} else {
set_text(vformat(TTR(current_message), message_format));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Calling second argument of the vformat message_format is a bit confusing. Usually it is the other way, and format is the first string with the template (e.g, https://en.cppreference.com/cpp/utility/format/vformat).

I would do:

current_message -> current_format
message_format -> message_argument

or

current_message -> current_message
message_format -> message_argument

Same for the _set_message argument names.

@Repiteo Repiteo modified the milestones: 4.x, 4.8 May 21, 2026
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.

3 participants