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

[WIP] Add pseudolocalization support to Godot #49361

Closed
wants to merge 24 commits into from

Conversation

angad-k
Copy link
Contributor

@angad-k angad-k commented Jun 6, 2021

GSoC Pseudolocalization project

This is a work in progress PR
This PR will be updated over the course of the next two months as I implement the pseudolocalization feature in Godot for GSoC'21. The GSoC project link for this can be found here and the original proposal can be found here. Also check out the todo list here. Demo project for this feature can be found here. PR for Godot-Docs tutorial can be found here. PR for Godot-Demo-Projects can be found here.

@angad-k angad-k requested a review from a team as a code owner June 6, 2021 12:07
@Chaosus Chaosus added this to the 4.0 milestone Jun 6, 2021
@angad-k angad-k requested a review from a team as a code owner June 10, 2021 17:46
@angad-k angad-k force-pushed the pseudolocalization branch 3 times, most recently from eb79445 to 69a55f0 Compare June 16, 2021 10:26
@angad-k angad-k force-pushed the pseudolocalization branch 9 times, most recently from 609cd91 to bab9a68 Compare June 25, 2021 18:02
@bruvzg bruvzg self-requested a review June 29, 2021 10:13
core/string/translation.cpp Outdated Show resolved Hide resolved
core/string/translation.h Outdated Show resolved Hide resolved
core/string/translation.cpp Outdated Show resolved Hide resolved
@angad-k angad-k force-pushed the pseudolocalization branch 2 times, most recently from 52e6586 to 9a0ff32 Compare July 19, 2021 15:08
@angad-k angad-k force-pushed the pseudolocalization branch from 9a0ff32 to 0ae14e2 Compare July 19, 2021 15:13
@angad-k angad-k force-pushed the pseudolocalization branch from ac2e76d to 92942cd Compare July 23, 2021 09:49
@Calinou
Copy link
Member

Calinou commented Jul 23, 2021

I tested this pull request locally. Here's how it looks like when debugging pseudolocalization is enabled:

image

It seems to be working well so far 🙂

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Documentation changes:

doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved
doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved
doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved
doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved
doc/classes/TranslationServer.xml Outdated Show resolved Hide resolved
doc/classes/TranslationServer.xml Outdated Show resolved Hide resolved
doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved
Co-authored-by: Hugo Locurcio <[email protected]>
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks pretty good, I left some comments on style and a few possible improvements.

Once ready, the commits should be squashed before merge, so that we have a clean Git history once merged. This can be done with an interactive rebase.

Might be worth doing it in a new branch and new PR if we want to preserve the development history in this PR (so the new PR would supersede this one). The squashed version would also be easy to backport to the 3.x branch with git cherry-pick -x <commit> and then solving merge conflicts.

core/string/translation.h Outdated Show resolved Hide resolved
core/string/translation.cpp Outdated Show resolved Hide resolved
core/string/translation.cpp Outdated Show resolved Hide resolved
core/string/translation.cpp Outdated Show resolved Hide resolved
core/string/translation.cpp Outdated Show resolved Hide resolved
doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved
Comment on lines +1622 to +1623
ClassDB::bind_method(D_METHOD("is_pseudolocalization_enabled"), &TranslationServer::is_pseudolocalization_enabled);
ClassDB::bind_method(D_METHOD("set_pseudolocalization_enabled", "enabled"), &TranslationServer::set_pseudolocalization_enabled);
Copy link
Member

Choose a reason for hiding this comment

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

You can add a pseudolocalization_enabled property with ADD_PROPERTY additionally to the setter and getter. This will remove the setter and getter from the docs and expose the property instead, which is more user-friendly.

For its documentation, you can copy what you had in the setter, and edit it to start following our style for boolean property descriptions: If [code]true[/code], .... (No need to say what happens if false since nothing happens.)

core/string/translation.cpp Outdated Show resolved Hide resolved
Comment on lines +378 to +380
_initial_set("interface/editor/enable_debugging_pseudolocalization", false);
set_restart_if_changed("interface/editor/enable_debugging_pseudolocalization", true);
// Use pseudolocalization in editor.
Copy link
Member

Choose a reason for hiding this comment

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

Instead of this here, you can use EDITOR_DEF_RST where you actually use it in TranslationServer directly. The _RST part will also enable restart if changed. (See my other comment about caching the value too.)

doc/classes/TranslationServer.xml Outdated Show resolved Hide resolved
@angad-k angad-k force-pushed the pseudolocalization branch from 2cb309c to e7e1c19 Compare August 6, 2021 07:58
@angad-k angad-k force-pushed the pseudolocalization branch from e7e1c19 to 4de2a1f Compare August 6, 2021 08:21
@angad-k angad-k force-pushed the pseudolocalization branch 3 times, most recently from ef209cf to e10e446 Compare August 6, 2021 16:55
@angad-k
Copy link
Contributor Author

angad-k commented Aug 8, 2021

Squashed the commits and added PR #51395

@YeldhamDev YeldhamDev closed this Aug 8, 2021
@YeldhamDev YeldhamDev removed this from the 4.0 milestone Aug 8, 2021
@Calinou Calinou added this to the 4.0 milestone Aug 8, 2021
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.

6 participants