-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Added I18N support #1769
Added I18N support #1769
Conversation
I don't know the java.util classes. Maybe some examples in the gdx-tests would help me understand why so much code is needed for i18n? For i18n it seems like you want to decide on a language somehow, then have a map containing the text name as a key and the localized text as a value. Seems like this could be done in a single class rather simply and would be GWT compatible. An additional feature might be handling multiple amounts ( |
Yeah sorry I've forgot to commit the test class. I'm gonna do it later when I'll come back home. |
Just committed the test class. It uses 2 bundles, namely "data/i18n/message1" and "data/i18n/message2". Both have 3 locales: default, English and Italian. I wanted to port the ResourceBundle API because is certainly something most Java developers already know and it's well specified too. Also, this PR has a lot of code because ResourceBundle API is extremely versatile and customizable. You can customize almost everything, e.g. cache policy, search strategy, file format and file location. About plurals, you can still implement this feature over the ResourceBundle API. In fact the API does not even force you to use a specific way to parametrize your localized strings. You're free to use java.text.MessageFormat, String.format() or whatever you want. EDIT: |
I really would love to see internationalization support in libgdx. But I'm not sure if it's the correct approach to just adapt the java.util classes. For me it would seem more natural if libgdx had it's own simplified way to handle multiple languages and actually one class should be sufficient to handle all this as NathanSweet already suggests. The following features seem reasonable for most games:
In the end it should be as simple as: Localization gameStrings = new Localization(/* base name: */"game_strings");
gameStrings.setLocale(Locale.ENGLISH); // Optional to override the default locale.
gameStrings.loadPropertiesList(/* internal path: */ "data");
String myString = gameString.getString("my_string"); |
IMO the default behavior of java ResourceBundle is even simpler than your example.
Finally on GWT side there are some intrinsic issues that are properly managed by its native localization system. For example the default locale is determined by the client, not by the JVM which is server side of course. |
Just to keep you informed.
Here is an example of parametric properties using MessageFormat syntax:
Also, on a side note, GWT I18N already support plural forms |
@BlueRiverInteractive i'm actually in big favor of your proposal which On 27.04.14 02:11, BlueRiverInteractive wrote:
|
Well, if the approach is "first PR wins" then I give up immediately. |
Also, I'd like to point out that before doing this PR I tried to discuss together a possible implementation.
|
I don't think thread safety is an issue, very little of libgdx is thread safe. The test helps a little. So I get a ResourceBundle, with or without specifying a locale, and I get strings from that. It's hard for me to judge the java.util stuff as I've not localized a Java app (though I have localized other software). It looks like a lot of code with a number of classes and I think the complexity turns people off. I'd like to see something like:
It seems like this could be done in a single class without getting too complex. Probably a good portion of what you have could be used, just simplified. Most apps only need a single text file, but maybe an RPG or something wants multiple files. We could allow multiple files to be specified and stuffed into a single map. Or we could load them into separate maps where one is the default and the others can optionally be specified when getting a string or the default is changed (whatever keeps API usage simple). |
@badlogic @NathanSweet : Here is something quick that supports all of the features you noted in one simple class: http://pastebin.com/CLdX0dsb. It's just a start, it could be enhanced with cross string linking, automatic escaping of single apostrophes, allowing comments. But if you like it, I can create a new pull request. // First create a new localization of one basename. Supply just any internal file path or filehandle of a language file.
// the language file is split into: basename_locale.extension
Localization loc = new Localization("internal/path/to/any/language/file/text_en.anything");
// Read in the language file and fill the string map.
loc.load();
// Get a string and supply additional parameters. They will be automatically formatted via the common MessageFormat pattern: http://docs.oracle.com/javase/7/docs/api/java/text/MessageFormat.html
System.out.println( loc.get("test", 123456789) ); Example language file:
|
I feel there's nothing wrong with using GNU's gettext po files with java's MessageText + ResourceBundle aside from having to convert them to java classes. Just trying to understand here, but is this issue just saying LibGDX should have its own built in parser/api for PO files to improve ease of use and compatibility with Android? |
I think the real problem is the GWT backend, not Android.
TBH I think I'm in favor of the 2nd approach.
I'm pretty sure that just removing from this PR stuff like thread-safety, cache management, customizable control and customizable search strategy the remaining code size will be about 10% of the current one. Sample strings
Common API
GWT backend
This example clearly shows that you have to use reflection to implement the common API for the GWT backend. |
Ok last night I've quickly removed all the bloated stuff, and the common API is working as described above on non GWT backends. |
@BlueRiverInteractive your pastebin is private. @nhydock Using PO/MO has the benefit of using the various tools for editing/merging. It's not a terribly simple format though, I wonder if it is worth using? @davebaol I meant one file per language. Not sure it's essential we chain string lookups, at least it can be worked around if we don't have this. If it is easy enough it is a nice feature. For GWT's i18n stuff, having to compile property files and then use reflection doesn't seem great. Why can't GWT just load and parse data from a file? Ok, there's no MessageFormat and presumably providing that is too much effort, so we can write something that replaces tokens and GWT just doesn't get the advanced features. If you develop for GWT you have a number of things to keep in mind for your app to work, this is just one more item in the list. |
http://pastebin.com/CLdX0dsb is now public. Sorry |
@NathanSweet For GWT the only thing to adapt would be MessageFormat.format(). Simple arguments (like {0}) should be easily to support with something like: for (int i = 0; i < params.length; i++) {
message = message.replaceFirst("\\{[" + i + "]+\\}", String.valueOf(params[i]));
} [EDIT] Locale probably could be replaced with GwtLocale? |
Sorry, I didn't understand.
Parent chain is already implemented and working. And GWT I18N uses the same search strategy. Well, actually if I got it right GWT I18N has no parent chain at runtime since it resolves missing properties at compile time.
Yeah, I agree. it's a bit redundant but I think there's no other way to implement the common API if you use GWT I18N.
AFAIK If you use properties GWT I18N only works through a compile step (by launching i18nCreator script) regardless of the use of parametric messages. Actually you must tell to i18nCreator if property files are constant or parametric and it will create classes that implement Constants or Messages interfaces respectively. All methods, corresponding to a property, in Constants subclasses have no arguments of course. Finally, if we can't make GWT I18N work via reflection then I'm afraid that the only viable path is the first approach mentioned a few posts above, which likely means more code and less features. 😞 |
@BlueRiverInteractive your class is simple, but doesn't have all the basic features (eg auto choosing a language file). @davebaol only GWT needs to suffer from less features. |
@NathanSweet I'm not sure what you mean by auto choosing a language file. |
After looking over all of this, I'd favor a simple interface that employs a simple parse and replace strategy on GWT. The added complexity of using GWT's built-in i18n implementation isn't justified by the benefits, in my opinion. In any case, the common api should be flexible enough that the underlying mechanism doesn't really matter. |
just came home, didn't look at any source, but why is reflection required @Davebol i understand that this is frustrating, especially if you put in
|
Ok after discussing with Mobidevelop on irc I think we've finally reached a good balance between functionality and simplicity.
This policy falls in line with other APIs that aren't fully supported by GWT. @badlogic Don't worry, code size has dramatically decreased after removing thread-safety, cache management, customizable control and customizable search strategy. Now that the default behavior of java ResourceBundle is implemented directly without all that bloated stuff the source code is rather readable. The PR has not been updated yet. |
Cool, thanks for sticking with us even if it's a bit comlicated at times.
|
Me too. I'll be on vacation the next few days and my beloved PC will stay at home. Every now and then you have to take a break. :) |
Bad weather messed up our vacation, so we stay home. 😞 |
On desktop en != it
on GWT en == it
|
Thanks. We could do the simple token replace ourselves, before passing to MessageFormat. That or a setting to turn off MessageFormat. |
lol I committed while you were commenting. I hope the commit is fine. |
Sorry, I forgot to update TextFormatter GWT emu. |
I think it is looking good. Talked with Mario and he signed off on it. Can you add a note to CHANGES with a link to the wiki page? Then we'll merge! Probably better not to support Output/InputStream in PropertiesUtils. It's character based so should take a reader and make it the responsibility of the user to choose the encoding. One last thought, would it be better to do simple token replace on |
Ok, do I have to sign the CLA?
Ok, I'll remove IO streams support. Actually both methods are never used since I18NBundle already calls the method that takes a reader while the method that takes a writer is there just for completeness' sake, which makes sense for an utility class.
You already said it before and I didn't reply because... well... either I'm misinterpreting your words or you're missing something. Not sure which one of the two. 😄 |
Updated PropertiesUtils. |
Yes, pretty please send a CLA: Yeah it's fine to do the CHANGES later.
We apply MessageFormat to the string. If the string has If we do simple token replace on the string before passing it to MessageFormat, then |
Oh ok, now I see what you mean and of course I was misinterpreting, sorry. 😊 If it's ok for you I'd prefer to keep |
I'd think unexpected behavior should be avoided. |
If it was unexpected, but it's not. |
But the goal of a common api is to have it work "the same" on the different |
Then we should completely remove MessageFormat since GWT only supports simple format. :P |
It's been quite a long discussion. I know you've been patient and put in a lot of work, but I think the discussion has been a good one. Solely because it has been so long isn't a good reason to stop. If we wanted to change how replacement is done, doing so after users are using it isn't great, though I'm not afraid of doing that. Only supporting the lowest common denominator isn't great because not everyone uses GWT. I do think there is some value in having the lowest common denominator be the same, so non-GWT works the same as GWT but has more features. We established earlier that formatting is an optional part of the API, people can get the strings and apply their own formatting if they don't like our (nonstandard) way of doing it. If you feel strongly about GWT and non-GWT being different, I'm ok with it. We'll merge now, feel free to make changes in a new PR if you like. I promise to try not to drag it out into weeks of discussion! :D FWIW and a bit OT, copying a standard Java API is almost never ideal IMO (acronym combo!). Those APIs are not always well designed and they evolve without breaking backward compatibility, often resulting in a mess. It's better to review how it works there and cherry pick the things that are good. People familiar with the Java API can easily learn the libgdx API. Where the Java API is good the libgdx API will be the same, where it isn't the libgdx API will be better. I don't think there is an expectation that the APIs match. For example, Array doesn't match ArrayList in a number of ways. I prefer |
Thanks Nate.
Now I'm going to update the documentation. |
Quick question, is the wiki page up already? I tried finding it but failed. |
Sorry, i'm dumb, you are still working on that. Take your weekend off, no need to rush it! Thanks for the this very valuable contribution and sticking with us silly nitpickers :) |
May I suggest adding an |
Yeah it's already in my todo list. |
Ok I'm investigating your idea and I think there are some non-trivial implications. |
True, if the simple token |
Eh it's more complicated than this if we want it really robust :|
this is true only for simple tokens not escaped with
it means we have to convert our escaping rule to MessageFormat escaping rules while replacing non-escaped simple tokens with
This is tricky because some occurrences of TBH I think that having to use |
It is safer, though I don't think it's common for the MessageFormat replace to output |
Yeah far from being likely but still possible. |
Why
As said here this PR is heavily inspired to ResourceBundle + Control class from java.util.
The main advantage, compared to java ResourceBundle, is that Android 2.2 is fully supported (Control class has been introduced in Android 2.3).
Also, under certain circumstances looks like Android API uses an inconsistent encoding for property files loaded through a ResourceBundle. This issue is fixed.
What's changed
The default behavior has changed. Bundles are loaded from file system (actually from
FileType.Internal
) instead of classpath.This change implies that the bundle name is now expressed as a relative path rather than as a fully qualified class name.
Also, due to the use of relative paths, bundle names are no more unique. For example, the bundle name "foo/bar/Message" on
FileType.Interal
and the same bundle name onFileType.Local
represent different bundles (it might depend on the specific backend though).What's less
The only missing features are ListResourceBundle and multiple class loaders support.
However both features are pretty much useless for our purposes, so it's not really a big loss.
What's more
FileType.Internal
file system just pass one of the following Control instances to getBundle method:PropertyFileControl.INTERNAL_XML_ONLY
to access xml files onlyPropertyFileControl.INTERNAL_XML_AND_PROPERTIES
to access xml and key/value pairs files (xml has higher priority)PropertyFileControl.INTERNAL_PROPERTIES_AND_XML
to access xml and key/value pairs files (xml has lower priority)To load xml files from any file system other than
FileType.Internal
you have to extendPropertyFileControl
and override its getFormats method accordingly.And GWT?
The GWT implementation is not part of this PR. In fact, it's not yet clear what's the best way to implement this feature.
From a first analysis there seem to be two viable approaches:
Locale
,Properties
,CuncurrentHashMap
, and many others.i18nCreator
script and theConstantsWithLookup
interface might be helpful. See the following links:The second approach seems to be more promising.
I'll be on vacation the next days, also I'm not a GWT expert so don't expect a GWT version soon, at least not by me.
Anyone who wants to contribute with ideas or code is welcome, of course.