Rubidium wrote:

That'd mean you're adding a byte to the money somewhere. When playing with loans I do not think you want to work with max 255 loans.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1946#comment3962

PhilSophus wrote:

No, I was calculating the current interest rate. And the interest rates are expressed as byte.

The problematic expression was as follows:

return _economy.min_interest_rate + (int64)(_economy.max_interest_rate-_economy.min_interest_rate)*p->current_loan+p->max_loan/2)/p->max_loan;

So the types were:

byte + ((int64)(byte-byte)*Money + Money/int)/Money

So, the sum calculated in the last step was (byte + Money). Of course, the second summand is in the range [0,max_interest_rate-min_interest_rate] so clearly in the byte range.

So, I should probably cast the second summand to byte. The return type of the function is byte anyways.

Nevertheless, if a byte + Money is provided it should work on all supported compilers. This is not the first time, I have read complaints about this compiling problem in the forum. So, patch authors seem to be caught by this problem regularly (e.g.: http://www.tt-forums.net/viewtopic.php?f=33&t=36127&p=664589# p664589 )

This comment was imported from FlySpray: https://bugs.openttd.org/task/1946#comment3963

PhilSophus wrote:

Oops, I just noticed that the just cited occurrence was probably caused by my improved loans patch, too. This was as part of a patchpack, so I didn't notice the relation to my patch before.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1946#comment3964

Rubidium wrote:

It is more by design that adding bytes isn't implemented. Primarily because adding bytes to Money isn't 'normal'. Furthermore your patch should (conceptually) cast the whole result of variable part of the interest calculation to a byte because the interest rate does not need to be 64 bits if you want it to be between 0 and maybe 20-50.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1946#comment3975

PhilSophus wrote:

Well, in fact, it is implemented (you explicitly provide an operator+(byte,OverflowSafeInt<>) implicitly forwarding to OverflowSafeInt<>::operator+(int)). And it works correctly on standard-conforming compilers, so it really can't be by design that it doesn't work on a non-conforming (let's call it buggy) but otherwise supported compiler. Also, why you consider adding integral types of differing bit-length as so abnormal that it should be avoided by design escapes me. Obviously, the author of the code thought differently.

But, if inserting (otherwise harmless) casts to int in operator+(byte,OverflowSafeInt<>) and operator- to also make it work on a bit buggy but supported compiler is such a big deal, just leave it as it is. It's just ridiculous and a waste of time to discuss the abnormality of adding byte and Money any further.

And, as I said: "So, I should probably cast the second summand to byte. The return type of the function is byte anyways." I already did this in my local git repository.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1946#comment3977

Rubidium closed the ticket.

Reason for closing: Fixed

This comment was imported from FlySpray: https://bugs.openttd.org/task/1946