Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Very large company values are not graphed correctly #8253

Closed
James103 opened this issue Jul 2, 2020 · 3 comments
Closed

Very large company values are not graphed correctly #8253

James103 opened this issue Jul 2, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@James103
Copy link
Contributor

James103 commented Jul 2, 2020

Version of OpenTTD

master (2020-07-01)

Expected result

  • Company value of 2^63-1 is recorded on the company value graph.
  • Company value of e.g. 2^63-10000 is recorded on the company value graph, with data points being visible.
  • The scale in these cases runs from 0 to 2^63-1.

Actual result

  • Company value of 2^63-1 is not recorded on the company value graph at all (scale still runs from -5 to 4).
    image

  • Company value of e.g. 2^63-10000 is still recorded on the company value graph, but the points do not appear.
    image

  • The scale runs from 0 to (2^63-1)/10 (~922 billion million) instead of from 0 to 2^63-1 (~9.22 trillion million).
    (see above images)

Steps to reproduce

  1. Load the attached savegame.
  2. Fast forward a few years. The money (and company value) will max out.
  3. Open the company value graph. It appears blank.
  4. Open your company finances window, and hit the "Borrow £10,000" button a couple of times.
  5. Fast forward a few months.
  6. Take a look at the company info window and company value graph.
  7. Hit the "Repay £10,000" button until the loan has been repaid.
  8. Take a look at the company info window and company value graph.

Savegame

Gronnway Transport, 363576-09-15.zip
(The savegame contains a single company with maxed-out (2^63-1) company value and cash, and has been optimized to run at up to several in-game years per second on high-end modern hardware.)
@James103 James103 mentioned this issue Aug 5, 2020
Closed
@TrueBrain
Copy link
Member

You are an absolute madman to make a savegame for this and clear steps to reproduce :D Awesome, tnx :)

@TrueBrain TrueBrain added the bug Something isn't working label Jan 1, 2021
@LordAro
Copy link
Member

LordAro commented Jul 19, 2021

The first part's quite fun - the graphing code uses INT64_MAX as "INVALID_DATAPOINT" . Of course, if it actually gets that value, it just ignores all the data.

OpenTTD/src/graph_gui.cpp

Line 39 in 1e529e1

static const OverflowSafeInt64 INVALID_DATAPOINT(INT64_MAX); // Value used for a datapoint that shouldn't be drawn.

A hacky fix might be to use INT64_MIN as a sentinel value, but I'd say that in real world conditions one is no less likely to appear than the other, so probably wouldn't actually fix anything

I'm not quite sure about the second issue. The y value that it's trying to draw the datapoint at is a very small negative number. I suspect it has something to do with the bitshifting of the very large datapoints to create the "x million" value

OpenTTD/src/graph_gui.cpp

Lines 437 to 457 in 1e529e1

/*
* Check whether we need to reduce the 'accuracy' of the
* datapoint value and the highest value to split overflows.
* And when 'drawing' 'one million' or 'one million and one'
* there is no significant difference, so the least
* significant bits can just be removed.
*
* If there are more bits needed than would fit in a 32 bits
* integer, so at about 31 bits because of the sign bit, the
* least significant bits are removed.
*/
int mult_range = FindLastBit(x_axis_offset) + FindLastBit(abs(datapoint));
int reduce_range = std::max(mult_range - 31, 0);
/* Handle negative values differently (don't shift sign) */
if (datapoint < 0) {
datapoint = -(abs(datapoint) >> reduce_range);
} else {
datapoint >>= reduce_range;
}
y = r.top + x_axis_offset - ((r.bottom - r.top) * datapoint) / (interval_size >> reduce_range);

@James103 James103 mentioned this issue May 29, 2022
Closed
This was referenced Jan 28, 2024
Merged
Merged
@TrueBrain
Copy link
Member

I really do not know why I just spend 2 hours fixing this bug. It is so unlikely for anyone to ever hit it in any real (or sane) game .. but there we go :P

It was a collection of problems starting when you were ~2**31 / 10 away from the max ... and hopefully soon they are all part of the past.

Please stop abusing OpenTTD :P (nah, it is okay; it was a fun little bug, that escalated into the madness of how compilers and computers actually work. I learnt things today :D)

TrueBrain added a commit that referenced this issue Feb 1, 2024
@TrueBrain
Fix #8253: allow rendering graphs with values over 2**31 / 10 (#11915)
dce7d5d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@TrueBrain @LordAro @James103