Jump to content
artlog

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

Recommended Posts

-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 ?

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.


×
×
  • Create New...