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

Enabled UI localisation for the Red Alert mod #8675

Closed
wants to merge 4 commits into from

Conversation

Mailaender
Copy link
Member

To get started on this. Using the automation from #5152. Also reverts a99d7fe.

@pchote
Copy link
Member

pchote commented Jul 7, 2015

My main complaint from the last two versions of this PR still hasn't applies: We are getting way ahead of ourselves by replacing the yaml strings without having any mechanism in place for the giant bundle of hardcoded UI strings. I'm still against against doing this until we at least have a mechanism to translate those.

@pchote
Copy link
Member

pchote commented Jul 7, 2015

Please lets have at least two PRs finishing our translation support before we run the chrome extraction command:

  1. Adjusting the translation code to support directly translating ids from the chrome logic files, and then extract these IDs into mods.common.
  2. Adjust the server code to send untranslated string ids to the player, and then translate them client side (with strings also extracted to mods.common).

This chrome string translation is automated, so can be done at any time. By doing it now, before the translation support is ready, you are just forcing additional overheads on everyone who touches the yaml files and we won't even have proper translations to justify that extra work.

@@ -1,2 +1,450 @@
english:
english: English

# mods/ra/chrome/install.yaml:
Copy link
Member

Choose a reason for hiding this comment

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

Most of these will need to live in Mods.Common.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, install.yaml is very mod specific.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also it seems language files don't merge so this is not possible yet.

Copy link
Member

Choose a reason for hiding this comment

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

I am referring to the whole file, not specifically the install strings.

@Mailaender
Copy link
Member Author

I added translation support for most of the internal widget logic strings now.

@pchote
Copy link
Member

pchote commented Jul 8, 2015

Also it seems language files don't merge so this is not possible yet.

This falls under point 1, and will need to be fixed before we do the extraction. Otherwise you are forcing more trouble and manual work down the road.

@@ -86,8 +86,8 @@ public static SoundDevice[] AvailableDevices()
{
var defaultDevices = new[]
{
new SoundDevice("AL", null, "Default Output"),
new SoundDevice("Null", null, "Output Disabled")
new SoundDevice("AL", null, FieldLoader.Translate("SOUND-DEFAULT")),
Copy link
Member

Choose a reason for hiding this comment

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

As I mentioned in IRC before you did any of these changes - FieldLoader has nothing conceptually to do with translations (except that it uses them), so it is not an appropriate class to do the translations. This should be done via ModData instead.

@pchote
Copy link
Member

pchote commented Jul 8, 2015

We're also going to need to think about how we deal with map-provided translations (for scripting via Lua, and maybe also map names?), as that will impact the design of our ModData Translate functions.

@Mailaender Mailaender closed this Jul 8, 2015
@Mailaender Mailaender deleted the l10n-ra branch August 6, 2015 17:30
@Mailaender Mailaender mentioned this pull request Aug 6, 2015
11 tasks
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.

None yet

2 participants