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

Fix crash when system locale is complex #5158

Merged
merged 3 commits into from
Nov 7, 2023

Conversation

rzats
Copy link
Member

@rzats rzats commented Oct 31, 2023

Contains

Fixes #5157. Detects if a locale looks like e.g. en_UA and converts it to e.g. en, preventing a game crash.

Also re-enables the ancient part-pirate localization by adding it to the "whitelist" of allowed locales.

How to test

Build from source with a "weird locale" (I did this on a non-M1/M2 Mac) and verify the game no longer crashes :)

jdrueckert
jdrueckert previously approved these changes Nov 6, 2023
@jdrueckert jdrueckert dismissed their stale review November 6, 2023 20:45

code changes look good, but it's questionable if they're actually necessary... cannot reproduce the issue in #5157 so far

Copy link
Contributor

@soloturn soloturn left a comment

Choose a reason for hiding this comment

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

i do think mixing language and region is the wrong approach. @rzats to my knowledge en_UA would mean that ukraine has a different english variant, so with maybe different words, grammar, etc. that variant of english seems to be not standardized, as @jdrueckert remarks. setting en_IE, en_US, en_UK, en_CA works. the region you live in is set in different variables, here an example from romania, some english variant is set, as it is perfectly fine to use american english in romania, while the other LC_ variables are set to region romania:
https://www.baeldung.com/linux/locale-environment-variables

what does public static Locale.Category[] values() return for you?

see:
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/Locale.Category.html

@@ -81,7 +83,7 @@ public class SystemConfig extends AutoConfig {

public final Setting<Locale> locale = setting(
type(Locale.class),
defaultValue(Locale.getDefault(Category.DISPLAY)),
defaultValue(getAdjustedLocale()),
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to only do this if the locale cannot be found, e.g. because it's a weird one?
This would allow to keep regional differences like en_US vs en_GB or en_NZ while fixing the macos issue.

@rzats rzats merged commit e679a98 into MovingBlocks:develop Nov 7, 2023
6 of 9 checks passed
@soloturn
Copy link
Contributor

soloturn commented Nov 8, 2023

very cool @rzats, thank you!

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.

Building from source won't work with "complex" system locales
3 participants