News:

Do you need help?
Simutrans Wiki Manual can help you to play and extend Simutrans. In 9 languages.

Fixed cost displayed incorrectly in depot

Started by jk271, December 13, 2014, 10:07:08 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

jk271

The "fixed cost" (time-based vehicle maintenance) is not being correctly displayed in vehicle depot. Only the whole part of the amount is printed, the decimal part is rounded and not printed. See details on figure below.

The bug is caused by formating string  - there is ".f " instead of ".2f"

Patch solving the issue is attached.

The bug has been detected in revision 7421 on 32-bit Linux (Debian 7.7).

DrSuperGood

Ultimately people might wish to avoid the use of floating points entirely seeing how the mechanics are all fixed point internally. Using floating points just subject the UI to possible rounding error (due to how floats work) unnecessarily in exchange for keeping things "simple". Such a change was made recently with the JIT2 patch which changed all industry UI metrics from double to full integer mathematics. I do agree that decimal parts are tricky to do outside of floats.

Ters

Since Simutrans wisely stores currency in cents (rather than the equivalent of dollar or Euro), the proper way to display it would be "%d.%02d" with arguments currency / 100 and currency % 100. 0.1 can't be stored reliably with binary fractions (whether floating or fixed point).

jk271

I fully support calculation using integers (and longs).
On the other hand this is an issue with displaying the amount to the user. There is no calculation of any number to be stored.
The lowest positive number displayed incorrectly using double is 90071992547409.93 being displayed as 90071992547409.92. I do not know any vehicle having so huge running cost, maintenace/fixed_cost, or any other numberic value being displayed as money.

If I use "%d.%02d" to display money I would have to handle negative and positive numbers differently to avoid displaying minus sign before the fraction part.

To sum up. It would be nice to have this bug fixed correctly. And the used approach is less important for me.

Ters

Quote from: jk271 on December 14, 2014, 02:14:41 PM
On the other hand this is an issue with displaying the amount to the user. There is no calculation of any number to be stored.
The lowest positive number displayed incorrectly using double is 90071992547409.93 being displayed as 90071992547409.92.

I thought floating point would have trouble representing 0.1, although I must admit that I haven't got full knowledge of all aspects floating point. Is it reliable that printf will round whatever the result of 1.0 / 10.0 is in floating point to "0.1"?

jk271

The floating point type double cannot store values like 0.1 precisely. I it absolutely true. And calculations in floating arithmetic cannot yield a precise result as inputs are imprecise. I would be the first one saying: replace floating point aritmetic by fixed point.

But the difference between the value stored in the double variable and the value as decimal number rounded to 2 decimal digits is in this case so small, that is would work correctly for reasonable small amounts of money. "Reasonable small" is lower than 90071992547409.93 in this case.

Double has 52+1 bits of significand. Let 7 or 8 bits bits bould be necessary for the fraction part to distinguish correctly between values like 0.01 and 0.02. There are more free bits of rest of significand to represent the integral part than 32-bit integer has.


To get the first incorrect value I wrote a program in C. It prints decimal numbers using both aproaches. Just compare the strings. The first line which differs shows the lowest incorrectly displayed number.

#include <stdio.h>

int main(int argc, char ** argv)
{
    for(int i=0; i<62; ++i){
        long long int num1 = 1LL << i;
        for(int j=0; j<100; ++j)
        {   
            long long int num2 = num1 + j;
            printf("%lli.%02lli %.2f\n", num2 / 100LL, num2 % 100LL, (double) num2 / 100.0);
        }   
    }   
}

prissi

If I would have know the lenght of this, I would have comitted yesterday. Thanks for finding this.