Fix: Prevent showing company is in trouble again after about 260 months due to overflow #8597
Conversation
…hs due to overflow
|
I never considered this a bug, just a kind reminder that you are still in trouble. So I am very unsure this needs fixing. But if we do fix it, it is lacking a comment to indicate -why- we should decrease the variable at that point again. It also seems like the wrong place to fix it, but that might just be me. First we need to figure out if we want to fix it. My vote: no. |
|
If I were you, I would consider rejecting this PR for a similar reason. To be more specific:
Without that second reminder, the player would never know that his/her company is in trouble besides the negative bank balance. With that reminder (current behavior before PR), the player finally gets notified that his/her company is in trouble, regardless of if he/she ever checked the bank balance. |
| @@ -609,6 +609,7 @@ static void CompanyCheckBankrupt(Company *c) | |||
| * is no THE-END, otherwise mark the client as spectator to make sure | |||
| * he/she is no long in control of this company. However... when you | |||
| * join another company (cheat) the "unowned" company can bankrupt. */ | |||
| c->months_of_bankruptcy--; | |||
There was a problem hiding this comment.
Couldn't we just change the variable type to a short or int? This seems a workaround.
|
Does it really matter for the player to know he's in trouble at that point? If the player is 260 consecutive months with a negative balance, I don't think he cares about the company, or be bothered with those news. For context, that's 21 consecutive years in the red. For me, this usually happens when observing AIs in single player, my company does nothing the whole time, it will eventually get the "in trouble" news once. Displaying it again every 21 years feels kind of pointless. Also of note, the 7th month the company is in negative, the company is offered to its competitors. Sure, it's single player, AIs can't buy human companies, but it's just to say that the months_of_bankruptcy counter is restarting from 0 months, and all inherent code runs again. |
|
It seems 3 people mention it is either the wrong fix, or considered a hack. I guess that settles it :) I agree that the current code is not all that nice, but I strongly think this is the wrong solution. Open for alternatives, but going to close this PR for now. |
Motivation / Problem
When in single player, the company the player is in cannot bankrupt, but c->months_of_bankruptcy counter keeps going up every month the company is in red. It overflows once it reaches 255, restarting at 0. So, at aproximatelly 260 months, (case 4), the newspaper displays again that the company is in trouble.
Description
Bug is fixed by decreasing months_of_bankruptcy counter by 1 inside the check where it maintains the company playing.
Limitations
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.