Moo-Ack-Productions / MCprep

Blender python addon to increase workflow for creating minecraft renders and animations
https://theduckcow.com/MCprep
GNU General Public License v3.0
287 stars 24 forks source link

i18n Support: PR 1 - Third and Final Attempt #538

Closed StandingPadAnimations closed 9 months ago

StandingPadAnimations commented 10 months ago

(This PR is to resolve some issues related to the second PR)

This is a third attempt at the first PR for i18n support, using Python's gettext module instead of a custom JSON protocol. This ended up being easier since the GNU developers have tons of utilities for automatic generation (such as xgettext)

As of making PR 1, a good chunk of the MCprep N panel is translatable, taking advantage of GNU Gettext (which is the standard for translation). Translators may use utilities like Poedit to aid in translation. In the future, we may also look towards Weblate, though that's not a priority right now.

For developers, what changes is that strings should now use text=env._(str) instead of text=str, in order to make it translatable. In addition, for every release, we need to refresh the base POT file. This is simple though:

xgettext -d base -o MCprep_addon/MCprep_resources/Languages/mcprep.pot "MCPREP_FILE" --keyword="env._

(Now here's the copy paste from #458)

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 will be merged for MCprep 3.6. While not explicitly defined as a blocker for the 3.6 release, it will be treated as such unless decided otherwise.

TheDuckCow commented 10 months ago

Just marking myself as a reviewer so I get back to it, unless you think there will be further structural changes. I'll review this and the bpy-build together (I've held off on that as I had some issue sin the past getting things working again, but will be good to get that merged in too ahead of our developer meeting)

StandingPadAnimations commented 9 months ago

@TheDuckCow finished the port from our old system to the new system: image

This changes on the fly and falls back to English if the user is using a language we don't support

TheDuckCow commented 9 months ago

Ok thank you for the clarity and making the readme! I understand it quite a bit better, and alleviates my concerns for the most part. Give me a bit of time as I want to play around with it myself to see how a few steps would occur.

One thing I recommend is that we also put into our release preparation script or even the compile script, the command which re-compiles the po files to mo. That way there's no chance they are out of date, and while contributors should build and validate their translations live, in case they didn't check it in we'd know pretty quick.

StandingPadAnimations commented 9 months ago

Would be interesting, but for now I think we can just manually compile it ourselves when merging a PR related to translation.

As an aside, this PR is almost done as far as I'm aware, I just need to add some notes for maintainers and verify that we've gotten the entire N panel (obviously we haven't gotten everything, but the N panel is a good start for MCprep 3.6)

StandingPadAnimations commented 9 months ago

I'd say this is complete now. @TheDuckCow, can you double-check and see if there's anything that's missing?

There's currently the following limitations I hope to address in future PRs:

StandingPadAnimations commented 9 months ago

Added conversion script to go from PO files to a Python dictionary that is loaded into bpy.app.translations (with a fallback to directly sourcing the PO and MO files themselves). This does add one small dependency at build time (polib, since gettext only uses MO files, not the source PO files), but overall it works surprisingly well. I'm curious if we should however keep it as a default script, or move it to its own action.

Sadly, even with bpy.app.translations, headers don't seem to update automatically. Will have to debug further.

StandingPadAnimations commented 9 months ago

Determined the issue regarding headers; turns out MCprep was silently falling back to "direct i18n" (i.e. using gettext over bpy.app.translations), but I didn't have a dev build enabled so there were no logs.

Once that issue is fixed, everything changes in real time. We should be good to go to merge this.

StandingPadAnimations commented 9 months ago

@TheDuckCow do you think we're good to merge by the end of this week?

StandingPadAnimations commented 9 months ago

Just realized we can also use polib to build MO files at build time, so I've added that to the default case. For organization reasons, I've removed the MO files from the repo.

StandingPadAnimations commented 9 months ago

Looking further, polib is actually a single Python file, so we could just add the file directly in action-scripts and call it a day.

StandingPadAnimations commented 9 months ago

As an aside, I'm thinking about adding some basic i18n features to https://github.com/Moo-Ack-Productions/bpy-build to assist developers (nothing major, perhaps just the script for auto-generating a file to make using bpy.app.translations easier) as Blender's existing tooling for i18n outside translating Blender is quite lacking, but that's beyond the scope of this PR

TheDuckCow commented 9 months ago

I definitely support the idea of some utility or auto-compile options with bpy-addon-build, this PR is already quite large so maybe worth doing in a follow up PR.

TheDuckCow commented 9 months ago

Regarding being PR-ready @StandingPadAnimations, is everything in order..? I just tried the branch out as-is (not commits or re-running any commands), and found that that some of the simplified chinese lines which are defined in Po don't seem to be translated in the application itself. Do we still need to do the dictionary registration step / load via po per bpy docs? I'd say that would be critical for this PR, otherwise the translations system isn't quite useable

Screen Shot 2024-02-19 at 7 10 24 PM

Would be good to show some screenshots of what you are seeing from your end

StandingPadAnimations commented 9 months ago

You need to add translate to the list of actions when building (since we rebuild all MO files and generate the translation dictionary, which adds to build times). bab -b dev translate will make a dev build with i18n support. I just forgot to change the developer docs to account for this

TheDuckCow commented 9 months ago

Yup, that has it working for me! Thanks for clarifying that, and updating the docs. Should probably update the push_latest.sh script as well, but I could do that in a follow up PR after this one. Will code review now, let's lock this in and do incremental improvements :+1:

TheDuckCow commented 9 months ago

(You can re-request my review one you have replies, so I don't miss them)

StandingPadAnimations commented 9 months ago

Done

StandingPadAnimations commented 9 months ago

Alright, updated with milestone-3-6-0 and updated the POT file, I'll merge this in now