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

Move pseudolocalization into TranslationDomain #96230

Merged

Conversation

timothyqiu
Copy link
Member

@timothyqiu timothyqiu commented Aug 28, 2024

Follow-up to #95787
Partially supersedes #96105

This PR:

  • Moves pseudolocalization related functions into TranslationDomain. So they can be changed per translation domain.
  • Adds --editor-pseudolocalization command-line option to turn on pseudolocalization for the editor and project manager at startup.

How plugins toggle their pseudolocalization

# On the plugin's root UI node
var domain := TranslationServer.get_or_add_domain(get_translation_domain())
domain.pseudolocalization_enabled = enable
# put other pseudolocalization related configurations here
propagate_notification(NOTIFICATION_TRANSLATION_CHANGED)

Demo project: test-4.zip

How to toggle editor pseudolocalization

The pseudolocalization of the project manager and editor can be enabled by using the command line parameter --editor-pseudolocalization.

To toggle pseudolocalization of the editor at runtime, run this editor script:

@tool
extends EditorScript

func _run():
    var domain := TranslationServer.get_or_add_domain("godot.editor")
    domain.pseudolocalization_enabled = not domain.pseudolocalization_enabled
    EditorInterface.get_base_control().propagate_notification(MainLoop.NOTIFICATION_TRANSLATION_CHANGED)

Note that this script is meant for editor developers. The domain name godot.editor is an implementation detail. It might change without considering (i.e., breaking) existing editor scripts.

Why you need to propagate TRANSLATION_CHANGED yourself

Regular users toggle pseudolocalization for their projects via TranslationServer.pseudolocalization_enabled = enabled. This will automatically emit NOTIFICATION_TRANSLATION_CHANGED. So does the previous workflow of modifying Project Settings and calling TranslationServer.reload_pseudolocalization().

Dealing with translation domains is advanced usage:

  • It's better to emit the notification after batch modifying pseudolocalization related properties.
  • It's not clear which subtrees are using a specific translation domain. Therefore, it's more efficient to let the caller decide the scope of the propagation.

@KoBeWi
Copy link
Member

KoBeWi commented Sep 20, 2024

Is editor pseudo-localization supposed to add text to otherwise textless buttons?
image
(though I guess it's unrelated to this PR)

EDIT:
Inspector is not pseudo-localized
image
but it spits error when you hover property
image

EDIT2:
Pseudo-localized placeholder
image

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.

Aside from the issues mentioned above, it works correctly.

@timothyqiu
Copy link
Member Author

timothyqiu commented Sep 21, 2024

Is editor pseudo-localization supposed to add text to otherwise textless buttons?

Yes. This is the same behavior when pseudo-localization is enabled in game projects. Maybe pseudo-localization can skip empty strings. We can tweak this behavior in a later PR.

Inspector is not pseudo-localized

Labels in inspectors are not subject to auto translation.

but it spits error when you hover property

It seems that tooltip texts always go through atr() currently. I'll see what can we do about it. Fixed by #97406.

core/string/translation_domain.h Outdated Show resolved Hide resolved
doc/classes/TranslationDomain.xml Outdated Show resolved Hide resolved
Also adds command-line option `--editor-pseudolocalization`
@akien-mga
Copy link
Member

@bruvzg Does this approach look good to you? You have a conflicting PR #96105 so I'd like to make sure we have the best of both worlds here.

@bruvzg
Copy link
Member

bruvzg commented Oct 4, 2024

Does this approach look good to you?

Yes, this is a better way to handle it. I'll update the remaining part of #96105 (number formatting) after this one is merged.

@akien-mga akien-mga merged commit 05b519f into godotengine:master Oct 4, 2024
19 checks passed
@akien-mga
Copy link
Member

Thanks!

@timothyqiu timothyqiu deleted the per-domain-pseudolocalization branch October 4, 2024 20:57
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.

5 participants