[BUG] Possibility of factory smoke crash

[Ranran(retired)]

I was doing a desync check in a loacal environment and the server side crashed, so I quit the client that was alive, loaded the save in a debug build, and after a while, it crashed.

I'm not sure what to report, so I'll paste the image. (´・ω・｀)









gr seems to be NULL.

By the way, I did a test in the local environment for a while, but I could not reproduce desync at all.

[jamespetts]

Interesting, thank you for this. Can you check what sort of industry it was with a NULL ground? If it was a water industry, the fix should be a simple check to see whether gr is NULL. That should stop the crash in other cases, but something is wrong if welt->lookup_kartenboden() is returning NULL on any valid land tile (or if a non-water industry is giving it a tile other than a valid land tile here).

[Ranran(retired)]

Can you check what sort of industry it was with a NULL ground?

It was a coal mine and a coal power plant around 1910. I was connecting the two and playing for a while.

[TurfIt]

Rotated factory near the map edge? Standard used to crash from that - already fixed.

[jamespetts]

Rotated factory near the map edge? Standard used to crash from that - already fixed.

Interesting, thank you for letting me know. May I ask how one would go about finding the fix in the Standard Github repository that mirrors the SVN?

[Ranran(retired)]

Rotated factory

No. (´・ω・｀)

near the map edge?

Yes.


I was wondering if the saved game was lost due to the crash, but fortunately the first data was left.
So I managed to make a saved game that can reliably reproduce this crash.

Load this saved game and fast forward a bit. Soon your game will be burnt in smoke.
You can check if this issue is fixed.

[jamespetts]

Thank you for that - that is helpful. What it appears was happening was that gr was being set from an invalid tile (-1, 185), and so was becoming NULL. This was because nothing in the code constrained the tile to one within the map. I have added code so constraining it, so now it does not crash, and, unlike the simple solution of simply checking whether gr is NULL, also does not prevent the factory smoke from working. The smoke does not move as much as it would in a different position, but this is preferable either to crashing or there being no smoke at all.

I should be grateful if you could re-test with the latest version on the master branch. Thank you.

[Ranran(retired)]

I've confirmed that this bug has been fixed.
Thank you for your work. (´・ω・｀)b

[jamespetts]

I've confirmed that this bug has been fixed.
Thank you for your work. (´・ω・｀)b

Splendid, thank you for confirming.