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

i18n Support: PR 1 #458

Closed
wants to merge 13 commits into from
Closed

i18n Support: PR 1 #458

wants to merge 13 commits into from

Conversation

StandingPadAnimations
Copy link
Collaborator

Let no Minecraft animator be confused by MCprep's UI simply because of the language they speak.
- Goal defined in #439

This is the first of a series of PRs to add support for translating the MCprep UI. The current roadmap is the following:

  • PR 1 - Basic infastructure and MCprep N panel
  • PR 2 - First half of all MCprep operators
  • PR 3 - Second half of all MCprep operators
  • PR 4 (To be decided) - Dynamic menus such as rigs and assets

PR 1 will be merged for MCprep 3.5.1. While not explicitly defined as a blocker for the 3.5.1 release, it will be treated as such unless decided otherwise.

As of making PR 1, a good chunk of the MCprep N panel is translatable, with a Simplified Chinese translation on the way (#445). Since I have to juggle between this and https://github.com/Moo-Ack-Productions/bpy-build, a second developer would be appreciated.

Internally, every string is represented by an ID. At runtime, MCprep calls MCprepEnv.translate_str(ID), which takes an enum value defined in MCprep_addon/translate_enum.py and maps it to the string defined in JSON. If no such string exists, English is used as a fallback.

StandingPadAnimations and others added 11 commits July 6, 2023 19:26
Since MCPREP_RESOURCES is used a lot, it makes sense to make it its own
variable to reduce the chance of typos and to make it easier for
developers to use in file paths.
Pop vs Soda

Now MCprep has the ability to load JSON files for i18n support. It
doesn't actually do anything with the data right now, but basic support
is there.

This method of loading also handles subdirectories, which means that we
could have folders for seperate dialects under a larger folders for the
language as a whole, such as Pop English and Soda English in a larger
English folder, or a Chinese folder with different dialects of Chinese.

Overall this is a big moment for MCprep, and paves the way to allow us
to fulfill the goal of "Let no Minecraft animator be confused by
MCprep's UI simply because of the language they speak"
To make it easier for us, a function has been added that will take an ID
for a string and return back the correct string for the language.
During testing, it was found that "is" is incorrect for comparing
strings and == was needed.
Signed-off-by: MisakaCloud <[email protected]>
`language` now defines the language name and `locale` is the actual
locale
The enum will make it way, way painful to use translated strings in the
UI through type safety rather then error-prone string handling.
@StandingPadAnimations
Copy link
Collaborator Author

As per #481, I'll be moving this to MCprep 3.6. Also gives us more time to refine the initial architecture

Copy link
Member

@TheDuckCow TheDuckCow left a comment

Choose a reason for hiding this comment

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

Was otherwise about to approve, but could we rename env.translate_str() calls to be env.tr()? Something I love from godot is how succinct this translate string function is, since it'll be basically everywhere. That's the only reason for my request for changes here.

With my demo over here, I can see it working well and I see no reason to push the first languages through, even if incomplete. I made a comment below about some missing keys, but we could also do this in a future branch.

Copy link
Member

Choose a reason for hiding this comment

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

See comment here for additional keys we'd want to have, not quite exhaustive but gets a number of them

#445 (comment)

@StandingPadAnimations
Copy link
Collaborator Author

env.tr() is a bit too short, I think env.tr_str() or env.tr_s() would be fine. However, at the moment, I'm not working on this PR, so someone else would need to do that final push to the end.

@StandingPadAnimations
Copy link
Collaborator Author

Also, this PR focuses on the N panel and basic infrastructure, so I'm not too concerned about operators rigth now

@TheDuckCow
Copy link
Member

I'm fine to take over for you, thanks for getting it this far! And sure - I can settle on tr_s.

Understood it focussed on the N panel, I was seeing the operator names poke through. I might expand on it a bit more.

@StandingPadAnimations
Copy link
Collaborator Author

I do have one concern with the current JSON format, that being split lines.

At the moment, we have separate keys for split lines, based on how many lines it requires in English to fit a long sentence into the MCprep UI, such as the following:

"world_add_world_1" : "No time controller,",
"world_add_world_2" : "add dynamic MC world.",

However, other languages may require more or less, so I think we should change it to something like this:

	"world_add_world" : [
		"No time controller,",
		"add dynamic MC world."
	]

And use a for loop in the UI for IDs like these. It's more work, but should help with preventing the UI from looking bad with languages other than or similar to English.

@StandingPadAnimations
Copy link
Collaborator Author

I'm fine to take over for you, thanks for getting it this far!

Thanks, I'll move this back into the goals for MCprep 3.5.1 then

@TheDuckCow TheDuckCow modified the milestones: v3.5.1, v3.5.2 Nov 15, 2023
@StandingPadAnimations
Copy link
Collaborator Author

Ok, I just discovered that the Python standard library has a module for i18n that allows the use of standard POT files (10 times easier to work with in this case)

In other words, the past 13 commits are no longer needed, and we should probably redo the entire PR to use standard convention

sigh

@StandingPadAnimations StandingPadAnimations marked this pull request as ready for review November 27, 2023 22:33
@StandingPadAnimations StandingPadAnimations marked this pull request as draft November 27, 2023 22:33
@StandingPadAnimations
Copy link
Collaborator Author

Moving this to MCprep 3.6. This will give some extra time to work on this

@StandingPadAnimations StandingPadAnimations modified the milestones: v3.5.2, v3.6.0 Dec 16, 2023
@StandingPadAnimations
Copy link
Collaborator Author

20220612_201950
^ How I feel now

The way I see it, we have 2 options:

  1. Discard this branch and PR, and make a new branch using gettext from Python's standard library
    • Would be time consuming (especially as I enter spring semester 🥲), but looking at this comment, we can try to salvage what we have, so it's not all for nothing
    • In the end, this route would make it easier for translators, and we could use something like Weblate too (though that's a problem for future us to work out)
  2. Continue with what we have and move on

@StandingPadAnimations
Copy link
Collaborator Author

I'll opt with option one, closing this PR and starting a new one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The Beginning of i18n Support i18n support (adding translations)
3 participants