Changing DelayTilMilli to used unsigned; patching "Wait 25 days" bug #122
Conversation
src/cpp/shared/game/event_loop.cpp
Outdated
| * in a very small number, it will never wait for very long. | ||
| */ | ||
| void __fastcall DelayTilMilli(long tick) { | ||
| unsigned long uTick = tick; |
There was a problem hiding this comment.
Can you make this const?
src/cpp/shared/game/event_loop.cpp
Outdated
| */ | ||
| void __fastcall DelayTilMilli(long tick) { | ||
| unsigned long uTick = tick; | ||
| while (UTickCount() < uTick) |
There was a problem hiding this comment.
What about static_cast<unsigned long>(KBTickCount()) instead of a new function? It's ugly, but it makes the key bit (using a big enough unsigned type) obvious.
src/cpp/shared/game/event_loop.cpp
Outdated
|
|
||
| /* | ||
| * This is a slightly hacky fix to the bug described in https://www.celestialheavens.com/forum/7/16792 , | ||
| * where a signed overflow of GetTickCount() can cause the game to wait for up to 25 days |
There was a problem hiding this comment.
Do you want to add an upper bound to prevent the game ever hanging forever again? Is it ever reasonable for DelayTilMilli to wait for more than a few seconds?
There was a problem hiding this comment.
There were two problems that combined to form these long delays:
The first is the possibility of wraparound.
The second is that some code accidentally put in a very small number into DelayTillMilli.
The end result is waiting a very long time.
With this change, the only way DelayTillMilli could wait a very long time is if the caller passed in a very large number. In which case, either they really did want a very long wait, or it was a mistake, in which case we shouldn't hide it (though I guess we can detect it and exit with an error message). In any case, this change cannot introduce any new very-long-hangs where there were none before.
I didn't find an easy way to test this (see https://stackoverflow.com/questions/727918/what-happens-when-gettickcount-wraps ), so I'm asking for manual inspection.