Problems with string representation of thousand and fraction separator

[sanna]

AFAICT there exists two methods in Simutrans that use the value of the two string variables SEP_THOUSAND and SEP_FRACTION to output a number as a string formatted according to the current translation. These two methods are money_to_string and number_to_string. AFAICT these work just fine. However, there seems to be quite a few places where numbers are output as string without calling any of these methods, and consequently the "wrong" thousand and/or fraction separator is used.

I do not know if there is an intention to move away from using these methods (in favour for a locale for numbers setting or whatever), but if they are to be used in then I presume that all places that output numbers as strings should use them.

The places I have encountered so far are:

Missing SEP_FRACTION

• stadt_info.cc:151 - the city growth percentage is output by "c->get_wachstum() / 10.0" as input to a "%+.1f" sprintf call
• citylist_stats_t.cc:121 - the growth is output by "sint32 growth = stadt->get_wachstum();" further divided by /10.0 formatted by a sprintf %+.1f
• citylist_stats_t.cc:139 also uses "sprintf(total_bev_string,"%s %d (%+.1f)", total_bev_translation, total_bev, total_growth/10.0 );" - i.e,
  both a missing SEP_FRACTION and a missing SEP_THOUSAND. I am frankly not sure which of the lines in citylist_stats.cc that produces the top line of the city list dialogue.
• depot_frame.cc:1394 & 1416 - maintenance cost output by a "veh_type->get_betriebskosten()/100.0" call with no further formatting (also SEP_THOUSAND missing see below)
• depot_frame.cc:1442 - gear is output by "sprintf(buf+n, "%s %0.2f : 1\n", translator::translate("Gear:"),    veh_type->get_gear()/64.0);"
• convoi_detail_t.cc:205 - gear is output by  v->get_besch()->get_gear()/64.0 formatted by sprintf "%.2f"

Missing SEP_THOUSAND

• citylist_stats_t.cc:121 - the city size is output by "sint32 bev = stadt->get_einwohner();" formatted simply by a sprintf %i call
• citylist_stats_t.cc:139 - see above
• goods_stats_t.cc:49 - price is shown by "sprintf(buf, "%.2f$", price/300000.0);"
• goods_stats_t.cc:58 - unit weight is shown by "sprintf(buf, "%dKg", wtyp->get_weight_per_unit());"
• factorylist_stats_t::zeichnen - none of the various output values (eingang, ausgang, production) is formatted at all
• depot_frame.cc:1393 & 1415 - price is output by "veh_type->get_preis()/100" with no further formatting
• depot_frame.cc:1395 - power is output by "veh_type->get_leistung()"
• convoi_detail_t.cc:205 - power is out by veh_type->get_leistung() formatted by a sprintf %i

Long list, and yet I am sure there are more....

Please note that the missing SEP_THOUSAND:s does not really worry me, they seem more like eye candy to me, but the missing SEP_FRACTION:s are really confusing. See f ex the following screenshot where the profit (tagged Vinst:) is output with SEP_FRACTION (which in Swedish should be a comma, not a point), and km-maintenance (in parenthesis on same line) without it.


I am very sorry that I have not been able to produce patches to rectify the above situation, it is slightly out of my league (as a couple tries with resulting segfaults and/or overflow problems has shown me.. *smile*), and I hope someone with a better understanding of C++ will find the time to do better than I have.

[prissi]

The depot frame stuff is not easy to solve, since it uses a predefined string, which would mess up stuff in amm other languages, unfortunately. Same for convoi_detail_t.

[sanna]

The depot frame stuff is not easy to solve, since it uses a predefined string, which would mess up stuff in amm other languages, unfortunately. Same for convoi_detail_t.

I am afraid that I do not quite understand; which is the predefined string that is used in depot_frame.cc? The sprintf:s I cited all use methods calls that return integers (get_leistung and get_preis returns a uint32) which then either are used as integers or divided by a float and then are formatted by the sprintf. The idea I had (which proved to be to simple) was to store the return values of these methods, send them through money_to_string and number_to_string before storing them in "buf" by the sprintf calls.

Clearly I do not understand the ramifications here. Does this imply that the places that do use the money_to_string and number_to_string methods already are messed up in those other languages?

[prissi]

The problem is, that the actual formatting is in a string called LOCO_INFO which is then translated. Changing this to proper display numbers, one would have to change this completely. Which is desirable in the long term, but prevents a quick change like for the other cases.

[sanna]

Thank you for taking the time to explain things like this for me. I do know understand what you mean. I agree that it is desirable for long-term, but I also realize that with the new stable fast approaching, this is not something to prioritize.

[jamespetts]

But if the formatting is in "LOCO_INFO", presumably it is easy to solve simply by ensuring that the languages' translation of "LOCO_INFO" has the correct implementation of decimal and thousands separators?

[sanna]

Unfortunately I do not think it so easy. The LOCO_INFO string needs to produce a string that will be accepted by sprintf, currently the English translation of this string is:

Code Select Expand

%s\nCost: %d$ (%1.2f$/km)\nPower: %dkW, %dkm/h\nWeight: %dt\n In order to take advantage of the number_to_string/money_to_string methods, the variables filling the positions in the string need to be converted to strings prior to the sprintf call, and all existing translations need to be updated to use %s instead of the various numeral representations. And when preconverting the numbers to strings, the right number of decimal precision needs to be set, variables need to be cast I think (the two methods need a double while the methods obtaining the values to be output returns int:s of varying types) etc. In short, it is not something that is inherently difficult, it will just require quite a lot of manual (or semi-manual) tweaking to ensure that nothing gets lost; so I think it is post-103 material.

But then, I am not a c++-programmer...

[z9999]

Seems to be a bug.

[prissi]

Sorry, cannot reproduce this on my system. Even in  japanese everything looks fine.

[sanna]

z9999:s bug seems to be same as this: http://forum.simutrans.com/index.php?topic=3454

[einself]

Same here:
Nightly 2702 - Pak.german 103.0

As you can see on the first image I chosed the "Marge" to be displayed in the graph. Unfortunately it shows the "Marge" initiating in the year 1887. But I started to play a lot years befor. In addition all the values of "Marge" are "1..00", which isn't true. They differ beetween 0,80 and 1,30. If i switch over to display the "Marge" of the last months everthing is perfect, except of the fact that it also shows the number in the format of "1..xx".
This problem with the format also happens while displaying other variables. (Picture 2)

Hope I could help a little bit

[prissi]

Aparently this only happens with GCC. Will dig deeper in it.

Ok, should work now also with GCC.

[sanna]

What was broken seems to be fixed (yes I use gcc) now, thank you! And many more places have correct thousand and fraction separators. Lovely!

But, there seem to be more decimals around than before, possibly unnecessarily many. I am thinking f ex of in the factory details where both the units and the percentages of production and consumption have a single decimal. Also profit and other monetary values in the Finances window, the two decimals are static at 00, so perhaps not needed?

[prissi]

Profit is not two decimals of zero, at least this would be heighly unlikely. Only at the start of the game; but since formating would look wierd without, I rather kept them this way.