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 custom methods for adding editor translations #77104

Closed
wants to merge 1 commit into from

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented May 15, 2023

Closes godotengine/godot-proposals#6885
Closes godotengine/godot-proposals#1262

Test addon with translation:
translationtest.zip

The implementation is as described in the proposal, but it has one problem - it doesn't support translation context. Coupled with the fact that duplicate messages trigger a warning, there is a chance for a conflicting messages in translations of multiple plugins.

Alternative suggested approach is to modify TranslationServer methods to work in editor. It would still likely require a separate method, because AFAIK TranslationServer handles only project's translations (I think even in editor?). Editor translations have some dedicated methods that work differently.

@YuriSizov
Copy link
Contributor

I'll repeat my comments from RC.

I don't think this should be done using new custom methods. TranslationServer is already available to scripting, and it also supports loading multiple translation resources and merging them. Adding more methods to EditorPlugin and reimplementing some of that logic doesn't make sense to me. I'd prefer we ensured that TranslationServer just works in the editor. (Last time I tried, it was operational, but completely clueless about which locale the editor used, and translating messages didn't work.)

@KoBeWi KoBeWi force-pushed the mr_worldwide_plugin branch 2 times, most recently from 4691223 to 5d72a71 Compare May 15, 2023 19:51
@KoBeWi KoBeWi requested a review from a team as a code owner May 15, 2023 19:51
@KoBeWi KoBeWi force-pushed the mr_worldwide_plugin branch from 5d72a71 to 4c8fd70 Compare May 15, 2023 20:35
@zedrun00 zedrun00 mentioned this pull request Aug 2, 2023
@timothyqiu
Copy link
Member

timothyqiu commented Sep 23, 2023

I think this is generally a better approach than #82158.

Instead of writing translation logic in EditorPlugin, I think we should put that in TranslationServer. That is, make tool_translation manage multiple translations, similar to translations:

HashSet<Ref<Translation>> translations;
Ref<Translation> tool_translation;
Ref<Translation> doc_translation;
Ref<Translation> property_translation;

Then EditorPlugin code would just be a thin layer of redirection.

@KoBeWi KoBeWi force-pushed the mr_worldwide_plugin branch from 4c8fd70 to e863cec Compare April 8, 2024 11:25
@KoBeWi KoBeWi requested a review from a team as a code owner April 8, 2024 11:25
@KoBeWi
Copy link
Member Author

KoBeWi commented Apr 8, 2024

Updated with the above suggestion. Sorry for the delay 🙃

@KoBeWi KoBeWi requested a review from timothyqiu April 8, 2024 11:26
@KoBeWi KoBeWi force-pushed the mr_worldwide_plugin branch from e863cec to 220c42d Compare April 8, 2024 11:29

Verified

This commit was created on github.com and signed with GitHub’s verified signature.
@KoBeWi KoBeWi force-pushed the mr_worldwide_plugin branch from 220c42d to 9dd97ff Compare April 8, 2024 15:47
@KoBeWi
Copy link
Member Author

KoBeWi commented Apr 8, 2024

Found a problem. When you add translation, it will always be used for translating matching messages, even if the locale doesn't match. How it should be handled? Should non-matching locales get ignored? (i.e. not added) Or it should be simply documented?

@akien-mga
Copy link
Member

Superseded by #95787.

@akien-mga akien-mga closed this Sep 20, 2024
@akien-mga akien-mga removed this from the 4.x milestone Sep 20, 2024
@KoBeWi KoBeWi deleted the mr_worldwide_plugin branch September 20, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants