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

Language setting is not working properly #169

Closed
egofree71 opened this issue Nov 11, 2014 · 30 comments
Closed

Language setting is not working properly #169

egofree71 opened this issue Nov 11, 2014 · 30 comments

Comments

@egofree71
Copy link
Collaborator

I have a french version of Windows 7. If is set the language to english, the labels are still displayed in french.

@IsmAvatar
Copy link
Owner

This may just be a configuration issue, but thanks for the report - if nothing else it will help us update our documentation on how to properly set the language for windows.

@RobertBColton
Copy link
Collaborator

@IsmAvatar @egofree71 I already know what the issue is, and it's not really LGM's fault. All the English translations in egofree's screenshot are the missing translations from the French translations. For some odd reason though, Java is deciding to use the English one as a backup for missing translations. That must have something to do with the way the file's were named, did you plan that @IsmAvatar ?

Compare the screenshot egofree posted on the forum with the one I just took and you'll notice all the English translations on his end were actually the missing translations on my end.
French Missing Translations
Missing Translations

@egofree71 The only way to fix it is for someone who knows French, which is not me, to go through and complete the French translations. Since nobody wants to maintain these translations, this is why I want to externalize them so that users can plug in any language and we aren't responsible for maintaining them.

@IsmAvatar
Copy link
Owner

This is intended. See https://docs.oracle.com/javase/8/docs/api/java/util/ResourceBundle.html#getBundle-java.lang.String-java.util.Locale-java.lang.ClassLoader-
Basically, ResourceBundle.getBundle is what we use to set our our i18n provider, and then we call .getString on that object. It has a list of locations to try, all starting with the Bundle Name, which is "org.lateralgm.messages.messages", and ending with ".properties".
To understand which file it will pick, we must first understand Locale. Quebec primarily speaks French, however they speak a variant of french. This is generally accepted as Quebec French. They still understand France French, UK English speakers understand US English, so it is classified as the French Language. To accommodate this, we append both the language and the country codes to the filename, messages_fr_CA.properties. We can go even deeper, specifying the Script (e.g. Latin vs Cyrillic for russian) and variant (e.g. Quebec French vs Acadian French, both in Canada), but I won't go into those details right now.
Now, by default, it will try to get a string from messages_fr_CA.properties, but if that string doesn't exist, it will then rollover to messages_fr.properties, figuring that the official French translation will probably have better coverage, and since Canadians can still understand official French. But if that string still doesn't exist, it will roll over to the Program Default translation, messages.properties, which at least presents some sort of human readable string. Failing that, of course, it will display the infamous Key, which we've chosen to surround by exclamation marks to make it stand out even more.
Java's Resource Bundling handles all of this for us. All we have to do is follow the Resource Bundling naming convention. The exclamation mark key failover is our own doing, since it is slightly more useful than the null that it actually returns.

@RobertBColton
Copy link
Collaborator

@IsmAvatar So my assumptions were correct. Another thing, how do you feel about special symbols in the translations? As you can currently see I tried to remove them from the English version mostly, thus why the French version has double semicolons :: on the object frame. Should we try to avoid symbols in the translations?

@IsmAvatar
Copy link
Owner

What kind of special symbols? All of our JLabels and whatnot should be compatible with UTF8, just the occasional default value or combo selection here or there might end up in the Data - which, again, should be compatible with UTF8.
I see no reason to eschew umlauts, accents, and cyrillic.

@egofree71
Copy link
Collaborator Author

I don't understand. The language in LateralGm is set to english, so why in the first place it wants to display french labels ???? It should not make any difference if there are missing translations in french or not. The problem it seems is that it doesn't take into account LateralGM setting but only the OS's language.

@egofree71
Copy link
Collaborator Author

If it's not possible to fix easily this problem, i would prefer to remove the french resources altogether. Let's keep them in the repository, but let's not use them. The problem is that there are a lot of missing translations but also some sentences are really horrible and not understandable. I guess they were automatically translated by a program. I fixed a few ones, but i don't have time right now to do this work.
To conclude, it's better no french version at all than a horrible one !

@IsmAvatar
Copy link
Owner

Like I said, perhaps you have not configured the language setting properly or it's getting overridden somehow. But you are correct, if your language is set to English, it should certainly never be attempting to use French. Not unless your language is actually set to French and you simply configured it wrong.
If the French translation is very poor, I would not be opposed to removing it, but it would not fix your configuration.

How are you setting the language to English, by the way?

@egofree71
Copy link
Collaborator Author

IsmAvatar:
I didn't change anything. By default the language in LateralGM is set to english, but i see french labels. For testing I tried to switch to french, and again to english and to start again LateralGM, but this doesn't change anything.
I am currently running Windows 7, but to be sure i did more tests and on a virtual machine i installed from scratch a french version of Windows XP and Enigma. I had the same problem. So i can tell with confidence that this is not related to some configuration problems on my pc. This means all french users will have the same problems.

@egofree71
Copy link
Collaborator Author

I forgot : when i am talking about the language setting in LateralGm, it's the setting found in general preferences. (See the first screenshot of Robert on this thread).

@IsmAvatar
Copy link
Owner

Ah, wasn't aware LGM had an option for it now. We're probably not setting it right, then.

@RobertBColton
Copy link
Collaborator

@egofree71 I can not confirm this yet but I will test I see now what the issue is and it does need corrected, when you set English it should be explicitly English.

We also need external language support so users can plug in languages, a little difficult with LGM's hectic working directory. When that is implemented I would like to remove all translations, keep them in the repo, and require users to maintain them their selves.

@IsmAvatar By special symbols, I just meant symbols. Like for instance "Sprite:"

My question is whether we should include ':' in the translation or hard code that to avoid it just being redundant in every translation.

@IsmAvatar
Copy link
Owner

It should be translated. Some languages have different symbols, and many languages will put the symbol in a different location, and we should cater to them. Also, in some places it might be useful to have the words on both sides replaceable, in which case we can do "%s: %s" as I'm sure you've seen in some places.
When it comes to i18n, it's a different world than programming. Duplicate text is not something to be refactored away - it's perfectly acceptable (unless the string really is the exact same thing, in which case you can re-use its key). You should also avoid building strings (like concatenation) in favor of strformat, because, like I said, different langauges will order things differently (although I think you've been good about that - just repeating it for the sake of having it written here).

@RobertBColton
Copy link
Collaborator

@IsmAvatar Actually I haven't, I have been concatenating strings in some occasions, I have in fact gone back and undone some of LGM's earlier translations to do that. I was already slightly aware of this, I don't know why it is that I sometimes get too hasty and make rash decisions and go against my own better judgement. I am going to have to make a pass through and redo all of that, including additional places where I've added translations, namely the preferences frame I believe.

@IsmAvatar
Copy link
Owner

While we're on the topic, I suppose I'll mention the other 2 major things for i18n:
Plurals - different languages pluralize things differently, so please keep that in mind. Some research may be required.
Lists - this is something I haven't figured out how to translate easily, especially variable size lists. As such, take care to avoid them if possible. If needed, a common solution is to simply delimit them with a newline, or possibly table columns.

Of course, not to distract you from what you're working on - but if you do find issues with things like these, it might be a good idea to at least write it down somewhere so you know to revisit it later.

@RobertBColton
Copy link
Collaborator

@IsmAvatar @egofree71 Ok I have fixed the original issue in 14ecabc by passing a bogus locale, please see my comments in https://github.com/IsmAvatar/LateralGM/blob/master/org/lateralgm/messages/Messages.java#L69 because there is also an alternative way to fix the issue that may be better by postfixing the default messages pack "_en_US" otherwise without either fixes you get egofree's bug where Java always tries to use the default pack regardless of locale.

@IsmAvatar In 14ecabc and 3d7163e I reverted the changes where I removed symbol translation, luckily I only did it in one editor, I'll go through and make sure to clean these up in other places if I missed any.

@IsmAvatar I also discovered another issue, this bug only propagates in 1 class, the Resource class that provides the names singular and plural for resources. See my comments on the following line.
https://github.com/IsmAvatar/LateralGM/blob/master/org/lateralgm/messages/Messages.java#L76

Basically, because the resource names for groups in the tree are loaded statically they happen to get loaded before preferences have applied the correct language bundle. I am wondering what the correct solution here is.

  1. Should I initialize RESOURCE_BUNDLE to null by default to avoid loading the language packages twice and just load the correct bundle the first time getString() is called?
  2. Should I remove places where messages are being loaded statically like the resource names and make them load at a more appropriate time?

Currently this bug causes the resource name translations to always be English and not able to be overridden/translated.

@IsmAvatar
Copy link
Owner

We shouldn't be hard-coding what the JVM is supposed to do dynamically for us, like identifying the bundle based on the locale.

@RobertBColton
Copy link
Collaborator

@IsmAvatar So would you be happier postfixing messages.properties with "en_US" because it seems just plain messages.properties is supposed to be for non-localized messages used by each translation?

Second you still didn't address whether we should be static loading messages or not, or if I should delay this until after messages and preferences are loaded. Currently we have a race condition between static loading the resource prefix preferences and loading the message bundle. Also preferences are getting kind of big, do you think they should be delayed altogether until the splash screen? If so then I also need to remove all static loaded messages.

Edit: I temporarily worked around the second issue in 3165a36 by using a second for loop so that kindNames3 is loaded first without causing a race condition in the static method.
https://github.com/IsmAvatar/LateralGM/blob/master/org/lateralgm/resources/Resource.java#L56

@RobertBColton
Copy link
Collaborator

@egofree71 @IsmAvatar Just an update, until I get a response from IsmAvatar if she'd like me to fix it the other way, the current patch is available in a new jar, you can get it from the release topic.
http://enigma-dev.org/forums/index.php?topic=2269

@IsmAvatar
Copy link
Owner

I don't have a strong opinion on the matter, but I do feel that this will come back to bite us if and when we get a stronger translation effort if we hard-code stuff.
From my understanding, plain messages.properties is for the program default language (in our case, English), and as such our program should be providing English strings in places where a translation is not available, rather than the ugly "!KEY.name!". This is especially true for users whose system default is, say, Bulgarian. We don't provide a Bulgarian translation file (yet), so we should default to English, rather than having the entire product filled with "!KEY.name!". Again, this should not be hard-coded, as Java's bundled resource loader already has handling for this.

@egofree71
Copy link
Collaborator Author

Robert:
I tested the latest version, and the bug is gone. Excellent ! :) Thanks for fixing this.

@RobertBColton
Copy link
Collaborator

@egofree71 Excellent, but I'd like IsmAvatar to tell me what she wants as the correct fix because she doesn't like the fix. There are only two ways to fix this.

  1. Obtain the language bundle with a neutral locale, which is the fix I did use, passing new Locale("","")
  2. Use messages.properties for messages that stay the same for all translations, or don't even use it since we don't need it at the moment, and establish an "_en_US" message bundle as the default.

@IsmAvatar You need to pick which fix you want, I seem to like the latter because it seems to be the correct way the Java system is intended to be used, I don't think messages.properties is meant as the default language.

@IsmAvatar
Copy link
Owner

I don't see why we'd even keep strings that "stay the same for all translations" in a resource bundle. Why don't we just non-localize (nls) those strings and keep them hard-coded? Placing them in a resource bundle allows them to be overridden by l12n files.

Again, I don't have a strong opinion on this matter, so if you feel that converting messages.properties into _en_US and defaulting it is the solution, you're welcome to implement it that way. It just doesn't make sense that the JavaDocs say it falls back on messages.properties. To me that makes it seem like that would be the "program default language", and having an _en_US would just be a duplicate file.

Also, reading through the code around the changeset you proposed, I saw a bunch of hard-coded locales, which made even less sense to me.

@RobertBColton
Copy link
Collaborator

@IsmAvatar We wouldn't and we don't have to, I'm just saying that's the way it seems to work. Such that, messages.properties are always taking precedence over any other language regardless of locale or anything, so messages.properties are always used, and messages_xx_XX are used when missing from messages.properties and it matches the locale. The JavaDoc seems kind of like it's contradictory to the actual behavior, but then again it's broad enough that it could just be the way you are interpreting it, I don't have it in front of me so I can't tell.

I am not sure what you mean by me passing multiple locales, I select the message bundle the user has selected in preferences and retrieve it using the neutral new Locale("","");
https://github.com/IsmAvatar/LateralGM/blob/master/org/lateralgm/messages/Messages.java#L72

But if you agree with what I described as the functionality of message bundles then yes the second solution is better and messages.properties should be renamed with the postfix _en_US, but we seem to disagree about how these resource bundles are intended to work. And as I said, we can just hardcode the messages that don't need translated, so we would no longer need messages.properties, just rename the one we have with the english locale postfix.

@IsmAvatar
Copy link
Owner

Oh, I see. You're grabbing the language that the user selects. That makes sense then. We could probably modularize that a bit, but that's beside the point for this discussion.

As for messages.properties - I don't believe it takes precedence, I believe it's the opposite - everything takes precedence over it. Basically, let's assume messages.properties has two strings
STR.a : Hello World
STR.b : Goodbye World

And then we have messages.properties_en_US with only the following:
STR.a : Overridden

If we have the language set to English, STR.a will be "Overridden", STR.b will be "Goodbye World", and STR.c will be "!STR.c!". If we have the language set to Russian, STR.a will be "Hello World", STR.b will be "Goodbye World", and STR.c will be "!STR.c!".

This is why our french users are seeing some strings in English and some in French - because some of the strings do not have a French translation available, so it defaults to English.
The fact that french users are still seeing french translations even when they select English does indicate that we need to force the locale. I'm wondering if forcing the locale to "en","US" would cause it to select messages.properties and abandon the french translations.

@egofree71
Copy link
Collaborator Author

There is still a problem with the buttons of java dialog windows : the labels are still in french. For instance, in the 'Shift all instances' dialog window, the cancel button is displayed in french ('Annuler').

@RobertBColton
Copy link
Collaborator

@egofree71 Probably because it's loaded statically and happens to load the messages before the message bundle gets updated, there were similar issues with the main resource tree.

@RobertBColton
Copy link
Collaborator

RobertBColton commented Sep 22, 2015

@IsmAvatar @egofree71 I am solving this once and for all. Java 7 has a global JVM override for it. We have long since dropped Java 6 support so I am going with this solution.
http://docs.oracle.com/javase/7/docs/api/java/util/Locale.html#setDefault%28java.util.Locale%29

I will be removing all of the previously added ugly hacks and replacing the preference with a new one. It will ask the JVM for available locales and then print their display names in a new ComboBox. A warning label is added to mention that LGM may not have an existing translation for the selected locale. The layout is complete in my local copy and will be finished in the next LGM release.

New Locale Preference/General Preferences Layout

@RobertBColton
Copy link
Collaborator

Translations in the tree are broke again as of 1.8.7 according to @egofree71.

@RobertBColton
Copy link
Collaborator

RobertBColton commented Sep 5, 2020

Closing this one out as long resolved, and #429 remains open to cover the remaining issue with translating the primary tree nodes. I tested and verified that the original issue here, that setting the language override does not override the system locale, is in fact resolved. I installed the Danish Windows 10 translation, including signing out and back in again. When the LGM preferences are cleared they pick up the system locale as the current locale. If egofree then overrides it to English and restarts LGM, he will have nothing but English translations in LGM's UI as he correctly expected. This is also the behavior of the Godot game engine.

Here's a summary of everything we discussed and concluded in this issue.

  • Colons on labels belong in the translation because some languages, like Chinese, have their own colon that embeds a space.
  • Concatenation of localized strings should be handled with string interpolation or formatting in the translation. Some languages, like Arabic, are right to left, and that's also a reason to put the colons into the translation too.
  • What was referred to as "Program Default" could also be considered the region-neutral locale which is just more general than the locale-neutral locale.
  • The ResourceBundle goes from the most specific locale to the most general and neutral locale.
  • The language preference we have now using Locale.setDefault() overrides the system default locale.
  • There is a bug in Java 11/13/14 I have that makes JFileChooser ignore our locale preference, and is fixed for Java 14. What's funny is originally we had fixed this issue that egofree had with the new way of forcing the locale only for it to crop back up in a later JDK version. Surprisingly I don't seem to have the issue though if I set "Chinese (simplified)" and it does work.
  • We do not postfix our "messages.properties" with "en_US" so that other languages with partially missing keys will fall back on displaying the English version. We've decided this is better than displaying the error key as we do when it can't be found at all even in English.
  • We still do not have a way for the user to easily provide their own translations external to the jar, but Installing a Custom Resource Bundle as an Extension would be a good place to start. I tested by creating a German translation next to the jar and even moving it into a directory structure mimicking the package name inside the jar, but none of it worked. It seems that may need explicitly enabled by adding the directories to the classpath. If that worked, we could have actually done the same thing for icon packs too.

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

No branches or pull requests

3 participants