ares-emulator / ares

ares is a cross-platform, open source, multi-system emulator, focusing on accuracy and preservation.
https://ares-emu.net
Other
864 stars 105 forks source link

[bug] ares uses locale-aware formatting for configuration #1526

Closed Dragorn421 closed 2 weeks ago

Dragorn421 commented 3 weeks ago

Describe the bug ares settings inconsistently use locale-aware formatting for configuration (settings.bml)

To Reproduce Steps to reproduce the behavior:

  1. Be on linux / posix-y OS (?) (I am on Kubuntu Linux)
  2. Run ares with French numeric locale LC_NUMERIC=fr_FR.UTF-8 ares
  3. Open audio settings, change the volume, close ares
  4. Open ares' settings.bml (e.g. ~/.local/share/ares/settings.bml)
  5. Observe Volume: 0,460000
  6. Start ares again (any locale), open audio settings
  7. Observe Volume is 0%
  8. Bonus: Close ares, open settings.bml again, observe Volume: 0,000000

The issue does not happen with running ares with English numeric locale like LC_NUMERIC=en_US.UTF-8 ares

Expected behavior Volume should be saved and loaded properly. Configuration should not use locale-aware formatting at all.

Additional context Kubuntu Linux ares v138, built from source

LukeUsher commented 3 weeks ago

Thanks, this looks like a genuine issue with the way nall (custom standard library) handles string->float and float->string conversion.

LukeUsher commented 2 weeks ago

patched in b5ee412

Dragorn421 commented 2 weeks ago

Still a problem in v139

Context

Following the b5ee4124e15f36720e3a9a2724718e4ef229a8f1 patch, @rasky mentioned on Discord that this fix had thread safety issues and suggested just setting the numeric locale to C program-wide. The previous patch was reverted and program-wide numeric locale set to C in 3372230021a4aeb6e3fcb5f5498efd61d54c74b3 , which now shipped in v139

The issue now

Unfortunately I am sorry to report the issue is not fixed, or only half fixed. With the changes, ares is somehow still writing the config using my not-english locale. (but reading an english-locale config is fine)

For test purposes I compiled ares with the following change, which fixed the problem (but idk if this is any good of a change wrt thread safety and possibly other considerations):

diff --git a/nall/string/utility.hpp b/nall/string/utility.hpp
index e7c92ab0d..7cd8af4e2 100644
--- a/nall/string/utility.hpp
+++ b/nall/string/utility.hpp
@@ -159,6 +159,7 @@ template<typename T> inline auto fromHex(char* result, T value) -> char* {
 //hand, digit-by-digit, results in subtle rounding errors.
 template<typename T> inline auto fromReal(char* result, T value) -> u32 {
   char buffer[256];
+  setlocale(LC_NUMERIC, "C");
   #ifdef _WIN32
   //Windows C-runtime does not support long double via sprintf()
   snprintf(buffer, sizeof(buffer), "%f", (double)value);