When the game-loop is very slow, it was easily possible to start
the loop with _shift_pressed being false, but end with
_shift_pressed being true. This doesn't hurt the game as such,
but for the user this can be very weird: I pressed "Buy Vehicle",
pressed shift a bit later, and I still get a cost indication.
Creating a thread was not thread-safe. The irony.
The video-driver has a function GameLoopPause() which first checks
if the thread is the game-thread or not. For this it needs access
to this->game_thread. This variable is set in StartNewThread().
However, due to timing, it is well possible GameLoopPause() is
called from the thread well before this->game_thread is assigned.
And so we have a race-condition!
Simply solve this by preventing a thread to start till we are
done with our bookkeeping.
This makes it easier to spot chunks that have a save_proc that
is a nullptr, but also prevents confusion, where it looks like
the CH_ type of a chunk has influence on how it is being read.
It is not, it is only used for saving.
Basically it is very similar to Vehicles, where there first is
a type field, followed by data of that type. So this commit makes
it looks like how Vehicles solved that.
This removes a lot of custom "keeping track of length" stuff.
This adds two byte extra to those chunks, and might feel a bit
silly at first. But in later changes we will prefix CH_ARRAY with
a table header, and then this change shines.
Without this, we could still add headers to these chunks, but any
external reader wouldn't know if the CH_RIFF has them or not. This
way is much more practical, as they are now more like any other
chunk.
This means that during loading we can validate that what is saved
is also that what is expected. Additionally, this makes all list
types similar to how they are stored on disk:
First a gamma to indicate length, followed by the data.
The size still depends on the type.
In the end, the code was already doing the right thing, but a few
functions deep, and not really obvious. When validating what objects
can handle SLE_VAR_NULL, it is nicer to just have this obvious.
Using SL_ARR for this gives us a bit of trouble later on, where we
add a length-field to SL_ARR. This of course is not the intention
of SLE_CONDNULL. So better seperate it.
The current SaveLoad is a bit inconsistent how long a length field
is. Sometimes it is a 32bit, sometimes a gamma. Make it consistent
across the board by making them all gammas.
This helps external tooling to understand if a SL_STRUCT should
be skipped when reading. Basically, this transforms an SL_STRUCT
into a SL_STRUCTLIST with either 0 or 1 length.
This wasn't consistently done, and often variables were used that
were read by an earlier blob. By moving it next to the struct
itself, the code becomes a bit more self-contained and easier to
read.
Additionally, this allows for external tooling to know how many
structs to expect, instead of having to know where to find the
length-field or a hard-coded value that can change at any moment.
There was a lot of code duplication for no real reason. Now with
SLEG_STRUCT support, we can just re-use the code, hopefully making
it easier for future-us to make changes to this, without breaking
everything for old games.
With the new SLEG_STRUCT it is much easier to embed a struct
in a struct, where the sub-struct has limitations on when it is
being used.
This makes both the code easier to read (less magic) and avoids
the SaveLoad needing to know all these things about Stations
and Vehicles.
The commits following this will use this new functionality.
Currently, a few places do this manually. This has as drawback that
the Save() and Load() code need to be in sync, and that any change
can result in (old) savegames no longer loading. In general, it is
annoying code to maintain.
By putting everything in a description table, and use that for
both Save() and Load(), it becomes easier to see what is going on,
and hopefully less likely for people to make mistakes.
Both did not support format parameters, so in many places IConsolePrint(CC_ERROR, "message") was used with a style different from what IConsoleError would do.
