-
Notifications
You must be signed in to change notification settings - Fork 81
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
Prepare supporting additional languages #1397
Conversation
@Hoikas When you have the time, could you pull this PR into TrollLand? It would help with testing H-uru/moul-assets#235. I think this PR has a small conflict with my other PR #1379 though - not sure if that will cause issues for you... |
plLocalization::SetLanguage(plLocalization::kJapanese); | ||
|
||
for (const auto &lang : plLocalization::GetAllLanguages()) { | ||
if (plLocalization::GetLanguageName(lang).compare_i(params[0]) == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would we feel about ISO language codes (i.e., en
, fr
, de
, it
, es
, etc.) rather that names? That would open us to support region-specific localizations (such as spelling "Neighbourhood" properly in en-GB
)
Although I guess that complicates the .loc files too, eh?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we would use standard language codes everywhere instead of custom names, yeah. Migrating the existing .loc files would be quite painful though - it would be a big incompatible change to all existing content, which would break open PRs and require changes to other tools like Korman.
3f17917 is now on TL. |
if ( | ||
plLocalization::IsLanguageUsable(lang) | ||
&& theElement.find(plLocalization::GetLanguageName(lang)) == theElement.end() | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is difficult to parse, IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can pull plLocalization::GetLanguage(lang)
out into a variable I suppose?
ST_LITERAL("Spanish"), // kSpanish | ||
ST_LITERAL("Italian"), // kItalian | ||
ST_LITERAL("Japanese") // kJapanese | ||
const std::array<std::set<ST::string>, plLocalization::kNumLanguages> plLocalization::fLangCodes = {{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not an array of std::unordered_set
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was already an ordered std::set
before, so I kept it that way. Looks like nothing relies on the order though, so it should be safe to change to std::unordered_set
.
And use explicit checks to skip specific languages in certain places, rather than dodgy indexing tricks as before.
Avoids unneeded empty entries in the result. This will be especially relevant when new languages are added to the engine.
No actual translations yet - this is mainly a test for my preceding changes to the localization system. The names, tags, and numerical values are compatible with Myst V.
Co-authored-by: Adam Johnson <[email protected]>
172d3bc
to
083d3d0
Compare
Translations submitted by @Fairyon: H-uru/moul-assets#237
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LTGM, but I'll hold off if you're still waiting on some Russian strings.
Had to make the "Starting URU. Please wait..." popup a little bit wider to fit the Russian version of the text. Co-authored-by: Alexander Diener <[email protected]>
Russian translations of strings added now. As mentioned in H-uru/moul-assets#237 (review), there are still problems with Cyrillic text in journals, but I can also fix those separately later (if they're even caused by a code bug at all). |
Technical improvements to allow introducing new languages gradually and without breaking anything. Specifically:
Standard disclaimer: I cannot test changes to the Max plugin beyond "it compiles".