binaries/system/test fmt::sprintf ("%f",...) with ',' separator

[artlog]

-Introduction / My life -

I recently played 0ad alone and with my daughter, and i am very very pleased with that game. Beinig a developper i decided then to dig further into it, and to do so it built it under my debian 10 system.

If built goes well, when i start game i get warning window popup and broken fonts, it seems to affect some fonts, so i did run test to see if my environment has something wrong

- test -

 binaries/system/test
Running cxxtest tests (336 tests)..............................................................................................................................................................................................
In TestFmt::test_basic:
/extmounts/var/develp/games/0ad/0ad/source/ps/tests/test_fmt.h:43: Error: Expected (fmt::sprintf("%f", 0.5f) == "0.500000"), found ("0,500000" != 0.500000)
/extmounts/var/develp/games/0ad/0ad/source/ps/tests/test_fmt.h:44: Error: Expected (fmt::sprintf("%.1f", 0.1111f) == "0.1"), found ("0,1" != 0.1)

 

So problem here is that "," is used to separate float part instead of '.' . this is clearly a problem of locale.

Even with playing with LC_ALL, LC_NUMERIC or other LC_XXX variable, test can't pass.
Here my supported locale :

locale -a
C
C.UTF-8
POSIX
fr_FR.utf8

i don"t support by default en_US.UTF8 this is the origin of my problem.

So i dig into code to find that line within test_CStr.h that is run before

char* old = setlocale(LC_NUMERIC, "fr_FR.UTF-8");

Then in test_fmt.h

char* old = setlocale(LC_ALL, "en_US.UTF-8");

So here are my remarks :
 

• if locale is not supported by system then setlocale does nothing, return NULL and keep previous locale.
• it might be sufficient to use LC_NUMERIC for fmt
• whatever happen old will not be populated with previous locale but with the one currently set
  • so setlocale(LC_ALL, old) won't reset it. 
  • terminology 'old' for variable name is then misleading.
  • perhaps using setlocale(LC,NUMERIC,"") is good enough to reset numeric locale  from environment
• "en_US.UTF-8" is not necessary supported, to be portable either "C" or "POSIX" could be used.

This is a very long text for a very minor detail, but i wanted to talk about it since it took me thee hours to narrow it.

now i can try to propose a fix, still i did get it from gitlab, can i propose a fix by forking on gitlab or should i use a svn account ?

i am less used to svn that to git...

is irc channel #oad a wy to get in touch or what is the best way ?

[Silier]

Hi,
possibly fix is proposed in https://code.wildfiregames.com/D2101
in meantime cppformat was renamed to fmt

[artlog]

yep, great, it look like to match this,  my only remark would then to use "C" instead of "en_US" for default tests.

[Nescio]

5 hours ago, Angen said:

in meantime cppformat was renamed to fmt

Shouldn't fmt be added to the dependencies then?

[Stan`]

Currently its an outdated bundled version. After the next patch it will become an optional one.