Fixed max tolerable journey time display bug

[neroden]

On my emergency-depot-bugfix branch, though it should be easy to cherry-pick this one.

[jamespetts]

Thank you for that.  Haven't had a chance to test yet, as am working on the other issues relating to the saved game issues. May I ask - what was the effect of this bug?

[neroden]

Thank you for that.  Haven't had a chance to test yet, as am working on the other issues relating to the saved game issues. May I ask - what was the effect of this bug?

The displayed "maximum tolerable journey times" were displayed wrong -- and much too small.  It appeared that the maximum tolerable journey time in a hackney cab was 27 seconds, for instance.  It was very confusing.

[jamespetts]

Ahh, I see - you have changed the display from HH:MM to MM:SS or HH:MM:SS? We need to make a small alteration to the position of the text in the depot window, in that case, as when we display it as HH:MM:SS with a comfort in three figures, the last character of the seconds and the close bracket is now outside the right edge of the depot window.

[neroden]

Ahh, I see - you have changed the display from HH:MM to MM:SS or HH:MM:SS?

Yeah -- everything else in the window is MM:SS or HH:MM:SS, which made it extremely confusing.  Worse was when the tolerable journey time was more than 60 hours; for instance, 61 hours would incorrectly show as 1:01:00 (!!!!)

We need to make a small alteration to the position of the text in the depot window, in that case, as when we display it as HH:MM:SS with a comfort in three figures, the last character of the seconds and the close bracket is now outside the right edge of the depot window.

OK.

[jamespetts]

I have now fixed the depot text on my 112.x-private-car-merge branch. Thank you for fixing the smoothness - it does look better now.

Do you think that storing things in seconds is necessary? Do we need that much precision?  Might it lead to integer overflows? Remember, many timings are stored in 16-bit integers, which is one reason that tenths of minutes were used in the first place (originally, minutes were used, but these were found to be insufficiently precise).

[neroden]

I have now fixed the depot text on my 112.x-private-car-merge branch. Thank you for fixing the smoothness - it does look better now.

Do you think that storing things in seconds is necessary? Do we need that much precision?

It would help with short-distance travel.

Might it lead to integer overflows? Remember, many timings are stored in 16-bit integers, which is one reason that tenths of minutes were used in the first place (originally, minutes were used, but these were found to be insufficiently precise).

Guh.  It will lead to integer overflows, unfortunately.  Right now with uint16 (I assume you're using unsigned ints) the max time is 109 hours; it would go down to 18 hours, which is far too low (I have trips longer than that on a routine basis).
Unfortunately this is all very... cramped, shall we say.  109 hours is actually a very uncomfortable thing for a hard journey time limit in the era of sail -- it's only 4.5 days.

More unfortunately, I believe we already have plently of integer overflows with delayed ships, which probably accounts for the weirdness I was having earlier with journey times for long ship trips.  With a hard max of 109 hours, anything over a 36 hr. 24 minute trip (which is quite easy to arrange in the age of sail) and "thrice_journey" overflows and becomes a very small number.  Journey *4 overflows even earlier.

You need to do the computations in 32-bit arithmetic.  I'm going to go fix some of this now, but you need to go through and fix more of it.  Anywhere where you're using 16-bit arithmetic you need to be very careful: there are only a few places where it's safe (when the values involved can be guaranteed to be very small).

  It's probably OK... for now... to save them as 16 bit data despite the 109 hour *hard maximum*.    I'd much rather have more bits to work with, as we could be a lot lazier about range checking.

[jamespetts]

Hmm - won't increasing them to 32-bits for storage increase memory consumption by potentially a large amount, especially in extremely large and densely populated maps? We can convert them to uint32s on the stack for calculations such as thrice_journey.

[neroden]

Hmm - won't increasing them to 32-bits for storage increase memory consumption by potentially a large amount, especially in extremely large and densely populated maps? We can convert them to uint32s on the stack for calculations such as thrice_journey.

I just did that.  And I fixed another roundoff error computation (in transfer_times).

I think we can leave them at 16 bits for *storage* for now, we just have to be very careful about it.  The 109 hour limit should be documented.

[jamespetts]

Thank you - but I don't see your latest changes on Github; have you pushed them? (Unless you made these changes before I wrote my above post?)