search

maruohon / malilib

Library mod for masa's client-side Minecraft mods
GNU Lesser General Public License v3.0
294 stars 125 forks source link

Switching JsonUtils reader/writer for UTF-16 reader/writer #45

Closed DarkKronicle closed 3 years ago

DarkKronicle commented 3 years ago

FileReader and FileWriter use default character encoding and decoding. On Window's systems the default is UTF-8 and on Unix based the default is UTF-16. Since Minecraft uses UTF-16, characters that the players put in a configuration text box may not be properly kept depending on their operating system. This pull request enforces UTF-16 for JsonUtils.

skyrising commented 3 years ago

I agree using the default charset is not great, but please don't use UTF-16 for saved files, use UTF-8.

On Unixes it depends on the LANG and LC_* environment variables and will also use UTF-8 by default if you have e.g. LANG=en_US.UTF-8.

I'm surprised that it would default to UTF-8 on Windows, I would have expected something like Windows-1252 or ISO-8859-1, but I don't have a Windows machine available to test right now.

DarkKronicle commented 3 years ago

Looks like I made an assumption that character sets were simpler than they are. Thanks for correcting me :) So it does seem that Windows uses 1252. I just tried my test encoding and decoding ▶ on UTF-8 again and it worked today, but apparently not yesterday. UTF-8 will probably work a lot better. I'll try to test on a different system to make sure it works and then update the Pull Request accordingly.

DarkKronicle commented 3 years ago

One potential problem I found is with old files using the default encoding. A simple way that could work most of the time could be to try UTF-8, and if that fails use the system default, then save in UTF-8. Anyone have any ideas?

maruohon commented 3 years ago

I don't think there are that many "arbitrary string" configs (ie. ones that might have non-ASCII characters) currently in the mods that it would be really worth worrying about properly converting all old configs. The upcoming bigger updates will end up resetting a few config options anyway, because some systems are moving and being configured in a different way in the new versions.