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

Add a utility function to get user's locale #1101

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

Add a utility function to get user's locale #1101

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Labels
enhancement New feature or request

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 10, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.1
Reported for operating system, platform: All, All

Comments on the original bug report:

On 2013-10-03 19:21:43 +0000, Julien Jorge wrote:

Created attachment 1354
Patch adding a SDL_AndroidGetLocale() function to get the locale of the phone

When working with Android's NDK, standard locale functions such as C++'s std::getlocale() won't return the user's locale as defined in the phone preferences [1].

It would be nice if the SDL could provide a utility function retrieving the locale as defined in the Java Activity. In my case, I need the locale to pick the right translation files used by gettext.

The attached patch adds such a function to SDL 2. Could you please review it and integrate it in future releases?

The function I have added is inspired by the implementation of SDL_AndroidGetExternalStoragePath(). Here are some decisions I've made during the implementation:

  • I chose to have a single call to a Java method and let this method to do the Java stuff needed to retrieve the locale.
  • Since the user can change the language of the phone at any time, the locale string is computed at each call.
  • The format of the locale should be similar to the one that could be returned by std::getlocale() or equivalent native functions.

[1] https://groups.google.com/forum/#!topic/android-ndk/ve9Yyy9JoS0

On 2013-10-05 15:52:15 +0000, Philipp Wiesemann wrote:

In my opinion it is confusing (without more explicit documentation) if a function returns a "const" pointer but invalidates it with the next return. [1]

If I wanted to compare the locale string returned before "pausing" and "resuming" the application (which could mean the user left to change the locale in the device settings) with the current locale string I would need to copy the last locale string before comparing to prevent undefined behaviour.

If the function would pass the responsibility to SDL_free() the string to the caller this would be different. And a new string is created in the current implementation anyway.

On the other hand if I would just want to get the locale once then the current return value is more useful.

[1] Like C++'s string::c_str().

On 2013-10-05 19:05:27 +0000, Julien Jorge wrote:

Phillip, you've indeed pointed a great weakness of the current implementation. Let's consider the code below:

void refresh_translations(void)
{
    const char* locale = SDL_AndroidGetLocale();
    // marker
    update_app_translations( locale );
}

// elsewhere
int event_filter( void* user_data, SDL_event* event )
{
    if ( event->type == SDL_APP_WILLENTERFOREGROUND )
        refresh_translations();
    
    // …
}

Let's consider I call refresh_translations() explicitely in my code and that the app is paused and restored with a new locale between its two lines (at the marker). Then event_filter() will be triggered, refresh_translations() called again, thus leasing to a call to SDL_AndroidGetLocale() which would invalidate the pointer used in the first call. The program will certainly crash when restoring the first execution.

I see no other solution to this problem than to return a char* the caller will have to SDL_free(). Do you agree with that?

On 2013-10-06 11:56:44 +0000, Philipp Wiesemann wrote:

(In reply to Julien Jorge from comment # 2)

I see no other solution to this problem than to return a char* the caller
will have to SDL_free(). Do you agree with that?

Another solution would be to check the locale and load the assets on the main thread and not in the event filter callback. Working on the main thread would also allow a display of the loading progress and prevent "application not responding" if this takes some time.

My initial criticisms was about the general usage of the function and not about special problems with thread-safety. Most of SDL is not designed to be thread-safe and should therefore not be called (unsynchronized) from other threads.

On 2013-10-13 20:04:51 +0000, Julien Jorge wrote:

Created attachment 1366
Patch adding a SDL_AndroidGetLocale() function to get the locale of the phone, without static variables

On 2013-10-13 20:12:02 +0000, Julien Jorge wrote:

Finally I have implemented SDL_AndroidGetLocale() without static variables. Indeed, I feel bad at using a static variable for something which may change and at leaving the mentioned reentrancy problem, event if the SDL is not thread-safe by default.

The description of the function has been extended to provide all these details. Especially the fact that the caller has to SDL_free() the result when he's done.

On 2014-07-15 19:09:35 +0000, Sylvain wrote:

Created attachment 1759
some code to get the Locales on IOS, Android, Windows !

This should be done for all platforms, not only android.
Here's some code to start !

On 2014-07-25 15:59:04 +0000, Sylvain wrote:

Created attachment 1788
SDL-locales.diff

I have updated the Ryan's patch SDL_PreferredLocales to fit the trunk, and I also added the parts :

  • Android : with the "AConfiguration" choice, but that may be an error because it also requires a JNI to get the assetMgr. so it's increasing the scope of dependencies.
  • WinRT/WindowPhones : code working, but not tested in SDL.
  • some code I was using : to convert a locale to an enum/language. I believe this is more convenient to handle translations, but feel free to modify it !

On 2015-07-03 04:51:04 +0000, wrote:

I have a slightly different implementation I have been using temporarily.

It separates out the current locale's country code and the preferred language list.

On Linux it uses the LANGUAGE environment variable.

https://github.com/x414e54/SDL2_locale/blob/master/src/unix/SDL_syslocale.c
https://github.com/x414e54/SDL2_locale/blob/master/src/cocoa/SDL_syslocale.m

If any of this is useful please feel free to use it.

On 2015-07-03 05:07:55 +0000, wrote:

I think the country code is also important because:

en_GB is different from en_US for example.

Some applications will use the country code to pick different regions messages such as regional ratings boards or legal information.

Also I am not to sure about using hard coded enums. Most systems support iso language codes so it should just pass those up to the application. Otherwise an application will never be able to add additional languages that SDL does not support.

For example I should be able to do something like have all of my assets in a ISO encoded folder:

./assets/en/strings.xml

This allows users to create their own translations once the game is no longer updated.
Then just grab their preferred system iso language code and use those strings.

Also SDL_LanguageToString is not a good idea as it is not localised and biased towards English. Someone who speaks one language may not be able to read latin characters so they need the language string in their native language character-set.

On 2015-07-03 05:37:21 +0000, wrote:

Hi sorry, just to be clear what I mean:

Usually you have just one (current) locale which defines the region (country code) for information such as e.g. currency, legal messages. There may also be separate locales for date/calendar format. This is usually not a list and cannot change, because it states you are in that specific country. e.g. GB (England) or US (United States of America).

You could be English living in a non English speaking country for example you want the currency and legal information of that country but still want the text in English.

You then have a language preference list locale which defines the actual text you can read. This can also include a country code if there are regional specific languages such as en_GB and en_US or just en for whatever.

Hope this is clearer.

On 2015-07-03 06:36:49 +0000, Sylvain wrote:

I agreed and still agree about not using hard-coded enums + helper functions.

I added this as a copy-paste of my code, but this can easily removed from the patch.

Those leaf functions can safely be removed:

typdef enum {} SDL_Language;
const char *SDL_LanguageToString();
SDL_Language SDL_LanguageFromInt(int val);
SDL_Language SDL_LanguageFromLocale(const char *str);

On 2015-07-06 10:55:19 +0000, wrote:

(In reply to Sylvain from comment # 11)

I agreed and still agree about not using hard-coded enums + helper functions.

I added this as a copy-paste of my code, but this can easily removed from
the patch.

Those leaf functions can safely be removed:

typdef enum {} SDL_Language;
const char *SDL_LanguageToString();
SDL_Language SDL_LanguageFromInt(int val);
SDL_Language SDL_LanguageFromLocale(const char *str);

Okay yeah, I see.

It think Windows should use GetUserPreferredUILanguages and fall back to GetLocaleInfoA if nothing is found (e.g. it is not running on Vista or higher). What do you think?

Also it would be nice to rename it to SDL_PreferredLanguages because Win, Mac and Linux separate the idea of Locale (Currency/Region/Calendar) and GUI Language.

On 2020-04-13 19:27:09 +0000, Ryan C. Gordon wrote:

Marking this bug target-2.0.14 because I've needed this several times recently. This sort of stalled on the mailing list a million years ago, but I'll read through all this and make some unilateral decisions this time. :)

--ryan.

On 2020-04-13 20:51:34 +0000, Ryan C. Gordon wrote:

Original mailing list thread on this, to be re-read...

https://discourse.libsdl.org/t/proposal-sdl-preferredlocales/19191

--ryan.

On 2020-04-13 22:14:07 +0000, Ryan C. Gordon wrote:

(In reply to Ryan C. Gordon from comment # 14)

Original mailing list thread on this, to be re-read...

https://discourse.libsdl.org/t/proposal-sdl-preferredlocales/19191

--ryan.

Also this: https://discourse.libsdl.org/t/any-advances-in-the-sdl-locale-api/20830

On 2020-04-14 12:11:35 +0000, Sylvain wrote:

Yes, I think this API would be useful, I´ve been using it since many years now!

On 2020-04-22 04:04:53 +0000, Ryan C. Gordon wrote:

Created attachment 4317
Updated Sylvain's patch

This is just Sylvain's patch in Attachment # 1788, updated to apply cleanly against the latest in revision control, as a first step.

--ryan.

On 2020-04-22 09:48:24 +0000, Sylvain wrote:

BTW there is still the "SDL_Language" enum in the patch which was controversial.
It can be removed easily, or if kept, must be extended (I've more items for it).

Also there are variation, should the country be detected
en_US vs en_GB vs en_XX
Maybe we would need a SDL_country or just give the raw data so the user can parse it.

On 2020-04-24 06:38:35 +0000, Ryan C. Gordon wrote:

Created attachment 4320
Revamping of previous patch

Ok, this is my attempt at reworking this with comments from this bug and those other forum discussions from years ago.

I'm not 100% married to this interface, so we can absolutely change it, if it bugs people.

  • Now it gives you an array of SDL_Locale structs, which just have const char * fields for language (never NULL, except to signal the end of the array), and country (not NULL for "en_GB", is NULL for "en").

  • Enums are gone, now just one API function, total.

  • It always allocates memory and the caller must SDL_free() the return value; we don't have to worry about the lifespan of the list now or if things change about the user's preferences at runtime. Even though there are an array of structs with string pointers, the entire thing is packed into a single malloc'd block, so you don't have to worry about managing the memory for a bunch of strings, etc.

  • You can set a hint ("SDL_PREFERRED_LOCALES") to override the OS, or provide this on platforms without a formal mechanism, or maybe have the Steam Client set this environment variable with Steam user preferences when launching games.

  • Platforms that offer this functionality can send an SDL_LOCALESCHANGED event when this list changes in some way. Interested apps can just call SDL_GetPreferredLocales() again when this event arrives. This is currently only wired up for macOS.

  • Interface to the platform layer is much simpler (we give them a buffer and max buffer length, and just pass it a 128 byte stack array from the higher level; platform just creates a string like "en_GB,en,de" then we parse it the same way as we would the override hint.

  • Wired this up to CMake project files.

  • Several improvements to the existing implementations, filled in a Haiku implementation, etc.

This has a ways to go:

  • Unix should be checking the LANGUAGE environment variable for "fallback" languages (that is, the rest of the preference list after the LANG var). Format is "LANGUAGE=en_AU:en_GB:en"

  • Windows Vista and later offer a formal API for listing locale preferences instead of the single item they currently report. Possible this works for WinRT/UWP too?

  • iOS doesn't have the locale notification wired up.

  • Several platforms have untested implementations, build systems to wire into, etc.

  • etc.

--ryan.

On 2020-04-24 06:40:58 +0000, Ryan C. Gordon wrote:

(Oh, also, try "testlocale --listen" on a Mac and add/remove/reshuffle your favorite languages under System Preferences -> "Languages & Region" to see the new event in action.)

--ryan.

On 2020-04-24 19:25:22 +0000, Sylvain wrote:

yes great, it sounds good to me!

(just notice my code, and I saw there is "zh_CHT" where country is 3 chars).
The C Android NDK doesn't handle it (because 2 chars),
JAva doesn't handle it neither (2chars or other digit code).
https://developer.android.com/reference/java/util/Locale#getCountry()

If you commit it, I'd like move the android asset mgr code, so that it can be re-used with patch from Bug 4297.

On 2020-05-06 06:58:37 +0000, Ryan C. Gordon wrote:

Created attachment 4332
Final attempt before revision control.

Here's what I believe is the final patch. Posting it here for Sam to look at before I push it to revision control. Scroll past the 1300 lines of project file changes to the actual code involved. :)

This fixes and improves the existing code a bunch over the previous patch, and adds in a few new platforms, and gets it integrated into all the major build systems (and Makefile.os2, because why not? It's just using the same Unix environment variable anyhow, lol).

Sylvain, if you want to make any Android changes, I'd say just push them directly to Mercurial once Sam takes a look at this patch. If anything needs to be changed otherwise, please do let me know!

--ryan.

On 2020-05-07 04:47:26 +0000, Ryan C. Gordon wrote:

Sam gave the go-ahead on this today, so the patch is now https://hg.libsdl.org/SDL/rev/55ec5ae4aa0b ... Sylvain, feel free to make any Android (etc!) changes you feel are appropriate right in revision control.

Resolving this bug with a changeset that started 7.5 years ago...that's got to be a record, even for us. :)

--ryan.

On 2020-05-08 09:22:35 +0000, Sylvain wrote:

Great, so I'm finally now using it in SDL!

I've merged the small asset manager factorisation (from bug # 4297)
https://hg.libsdl.org/SDL/rev/458e052261cb

On 2020-05-08 09:50:36 +0000, Julien Jorge wrote:

Finally I can finish my game!

Just kidding :) Thanks for your work Ryan, glad to see it coming into the SDL.

On 2020-05-08 19:42:06 +0000, Sylvain wrote:

Android: send SDL_LOCALECHANGED when locale changes
https://hg.libsdl.org/SDL/rev/1e7e8c85a056

@SDLBugzilla SDLBugzilla added the enhancement New feature or request label Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant