search

AzureDoom / AzureLib

Based off Geckolib but now just for my own needs.
MIT License
41 stars 7 forks source link

AzureLib does not use root locale in toLowerCase, breaking with Turkish language #70

Open PlanetTeamSpeakk opened 3 hours ago

PlanetTeamSpeakk commented 3 hours ago

We have had a couple reports from Turkish users complaining the mod makes the game end up in a resource-loading loop. We found that this was caused by the letter 'i' being replaced by a Turkish letter 'ı'. After some digging, we found the cause of this was that we were calling String#toLowerCase() on key strings. This method by default uses the system locale which is what's responsible for the aforementioned issue, the solution being to simply not use this method and instead use String#toLowerCase(Locale) where the locale used is Locale.ROOT. The issue we were having is also documented in the String#toLowerCase() method's javadoc which also proposes the same solution.

However, after we fixed this issue in our own mod, we found that AzureLib was guilty of the same problem. Namely right here: https://github.com/AzureDoom/AzureLib/blob/1.21-dev/common/src/main/java/mod/azure/azurelib/core/molang/MolangParser.java#L81 (MolangParser.java line 81, the parseExpression function, and perhaps in other places as well). This may be related to #68 and possibly other issues as well.

As mentioned, there's a very simple solution which is to simply call the overload that takes a Locale and pass the root locale. I, and our users, would greatly appreciate it if you could fix this. Thanks in advance.

AzureDoom commented 3 hours ago

That issue linked is for broken animations but I'll look into the change there in the Molang here when I'm free next! I am welcome to PRs as well if you need it faster.

PlanetTeamSpeakk commented 2 hours ago

The issue seems to say the game gets stuck in the same loop as ours and in our case, it also happens when loading animations. The exact problem being that "easeinoutsine" has its 'i's replaced by the toLowerCase call in parseExpression. So I figured they may be related, but I'm obviously not sure. I may have some time tomorrow to comb through the toLowerCase calls here, replace them and make a PR. Should be easy enough.

PlanetTeamSpeakk commented 2 hours ago

Also, any chance this can be backported to 1.20.1? That's the version we're currently on and we're not upgrading yet. If not, the Turkish folks will just have to wait a little longer.

AzureDoom commented 2 hours ago

I'd backport it to all versions!