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 editor pseudolocalization support. #96105

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Aug 26, 2024

  • Moves localized number formatting methods from TextServer to TranslationServer. This is a more appropriate place for these (it's using custom code and not related to any of the other TextServer parts), and better handle differnet tool/project locales.
  • Adds support for localized numbers pseudolocalization (currently we use it only for some editor parts, so it will be useful for making it more uniform).
  • Adds editor pseudolocalization support (was available for projects only).
Screenshot 2024-08-26 at 11 55 12

@bruvzg bruvzg added this to the 4.x milestone Aug 26, 2024
@bruvzg bruvzg marked this pull request as ready for review August 26, 2024 12:09
@bruvzg bruvzg requested review from a team as code owners August 26, 2024 12:09
@bruvzg
Copy link
Member Author

bruvzg commented Aug 26, 2024

Note: The doubled square brackets seem to indicate that a lot of editor strings are running through translation twice, e.g., "Scene" tab name is:

  • Translated onec with TTR
editor_dock_manager->add_dock(SceneTreeDock::get_singleton(), TTR("Scene"), EditorDockManager::DOCK_SLOT_LEFT_UR, nullptr, "PackedScene");
  • Already translated string is translated again with atr.
tabs.write[p_tab].text_buf->add_string(atr(tabs[p_tab].text), theme_cache.font, theme_cache.font_size, tabs[p_tab].language);

@KoBeWi
Copy link
Member

KoBeWi commented Aug 26, 2024

Yeah editor has auto-translation enabled, but all strings are set pre-translated. We could be using TTRC almost everywhere.

@RedMser
Copy link
Contributor

RedMser commented Aug 27, 2024

The removal of TextServerExtension methods without deprecation is a breaking change.

@timothyqiu
Copy link
Member

I wonder how users would benefit from toggling pseudolocalization translation in the Editor Settings dialog.

You need to recompile the editor in order to test your own translation anyway. Translations in official builds are almost always out of sync with Weblate.

@KoBeWi
Copy link
Member

KoBeWi commented Aug 27, 2024

It could be useful for plugins, once we support translating them.

@bruvzg
Copy link
Member Author

bruvzg commented Aug 27, 2024

I wonder how users would benefit from toggling pseudolocalization translation in the Editor Settings dialog.

It's not useful for most of the editor users, but useful for editor development. I would hide it under "advanced" toggle, but we only have it for project settings, and adding it specifically for this seems like overkill.

@bruvzg
Copy link
Member Author

bruvzg commented Aug 27, 2024

The removal of TextServerExtension methods without deprecation is a breaking change.

For external TextServer it is, but I do not think there are any external text servers. This option was added to allow us to split text servers to GDExtensions, but it's not used right now (and probably won't be, but it's useful to keep it as a test for GDExtension). So we probably should not care much about keeping compatibility for it.

@akien-mga
Copy link
Member

akien-mga commented Aug 27, 2024

We actually had but just removed support for editor pseudolocalization in #93554, because:

  • Users were mistakenly enabling it somehow, and then making bug reports that the editor is in gibberish
  • Anecdotally, I haven't ever seen a contributor mentioned that they used it when developing the editor UI to ensure it's localization friendly

@bruvzg
Copy link
Member Author

bruvzg commented Aug 27, 2024

Anecdotally, I haven't ever seen a contributor mentioned that they used it when developing the editor UI to ensure it's localization friendly

And editor localization is an absolute mess of half untranslated and half double translated strings, which is instantly visible with pseudolocalization and impossible to detect without it.

Users were mistakenly enabling it somehow, and then making bug reports that the editor is in gibberish

Not sure how it's possible, but I guess it can be build option disabled by default, it's definitely a useful feature.

@Mickeon
Copy link
Contributor

Mickeon commented Aug 27, 2024

I think that the main problem with that feature was in a pretty nasty spot in the Editor Settings, where it was a bit too easy to enable on accident when messing around with the other interface/editor settings. Given that bruvzg was able to find an oddity by enabling it, the feature can be useful.

@bruvzg
Copy link
Member Author

bruvzg commented Aug 27, 2024

Added build flag (disabled by default).

@Mickeon
Copy link
Contributor

Mickeon commented Aug 27, 2024

I suppose that... a build flag is reasonably accessible for just this purpose? Never really looked into them much, just so long as it's possibile to know their existence. If it needs to be documented somewhere too, so be it.

@bruvzg
Copy link
Member Author

bruvzg commented Aug 27, 2024

Never really looked into them much, just so long as it's possibile to know their existence

scons --help prints all flags (with a single line descriptions). But it (and how to use pseudolocalization) probably should be mentioned on the Engine development docs.

@akien-mga
Copy link
Member

I haven't had time to review the PR and discussion properly so TIWAGOS, but IMO a build option for this might be overkill. A command line option to enable pseudolocalization makes IMO more sense (and we can make it DEV_ENABLED only if we really want it out of regular user builds to avoid them enabling it by error and being confused).

@timothyqiu
Copy link
Member

timothyqiu commented Aug 27, 2024

I really wish this was done after #95787 is merged 😅 That PR makes TranslationServer handle different parts of the editor uniformly.

Pseudolocalization was kept project-only (only affects main translation domain) to keep that PR's changes minimum.

After moving pseudolocalization logic into TranslationDomain, it'll be easy to make a command-line option to enable pseudolocalization for the editor, or just ask devs to call something like TranslationServer.get_domain("godot.editor").enable_pseudolocalization = true in a tool script.


Update: after writing this, I realized that there is a prerequisite for proper editor pseudolocalization: changing most TTRs to TTRC (and also updating the text that uses TTRN and TTRs with context in NOTIFICATION_TRANSLATION_CHANGED if we want to do the runtime switching described earlier).

Moves localized number formatting methods from TextServer to TranslationServer.
Adds support for localized numbers pseudolocalization.
Adds editor/project manager pseudolocalization support.
@Calinou
Copy link
Member

Calinou commented Sep 16, 2024

A command line option to enable pseudolocalization makes IMO more sense (and we can make it DEV_ENABLED only if we really want it out of regular user builds to avoid them enabling it by error and being confused).

I agree with that approach as well, although I don't think we need to put it behind DEV_ENABLED if it needs a CLI argument to be used. The CLI argument should be forwarded from the project manager to the editor for convenience still.

@bruvzg bruvzg marked this pull request as draft October 4, 2024 09:04
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.

7 participants