News:

Want to praise Simutrans?
Your feedback is important for us ;D.

Fixes for RNG state

Started by ceeac, October 12, 2020, 06:36:48 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

ceeac

This patch fixes 2 bugs:

  • When reloading a game with the default non-scripted passenger AI activated, the game searches for a random road vehicle for the AI, which might be different than the one used before saving. Instead, the default road vehicle needs to be saved and reloaded, falling back to the semi-random search if reloading fails (for example because the pakset has changed and the vehicle is no longer available).
  • To ensure the RNG provides consistent output across a save/reload cycle, the entire state of the RNG needs to be saved. While the state is fairly large (2.5 kB binary uncompressed), the increase in size is small compared to the total size of the saved game (< 0.1%, except for very small maps).

prissi

The reinitialisation with the saved random seed is enough to ensure that the same sequence of random numbers is created when loading this game again. Ok, they won't be the same if no save/load cycle occurred, but then it does not matter, since those are random numbers and the save load cycle will anyway mess up several things, as discussed previously. Or do I miss something here?

And why does the AI need to save the road vehicle? Variation was intended for this. Also before each building of new routes there is a vehicle search.

ceeac

My goal is for a save/reload cycle to be both idempotent (i.e. save/reload/save is the same as just saving) and transparent (i.e. save/reload is the same as doing nothing), which is currently not the case. This patch is just a small step towards that goal; I want to make several small patches which can be easily reviewed instead of disappearing for several weeks and coming up with a huge patch which cannot be reviewed as easily.

Eventually, when saving/reloading is both idempotent and transparent, one can use loadsave_t for thorough desync checking (by running the game through a SHA1 message digest instead of using checklist_t; I have a working proof-of-concept of this already), and remove the need for other clients to reload the save when a client joins, as mentioned previously. The latter is especially important for Extended because the save games are much larger, for example on the Bridgewater-Brunel server.

prissi

Such a save will require to stop the map much longer while doing the autosave (i.e. no screen updating and time passing), and also needs implementation of many dialoges saving and reloading without applying their settings.

All this is fixable in principle. But this will be a very very long project ahead.

Maybe this is better done a as branch than in trunk, so there will not be lots of not clearly unrelated changes to the savegame. Since it will not afffect the saving data nor the logic, I think it makes much more sense to add it as one big patch.

Dwachs

Maybe this pays off some time? I think it is better to have small patches. As long as the savegame format does not change, i see no problems.
Parsley, sage, rosemary, and maggikraut.

freddyhayward

Quote from: ceeac on October 13, 2020, 08:02:50 AM
Eventually, when saving/reloading is both idempotent and transparent, one can use loadsave_t for thorough desync checking (by running the game through a SHA1 message digest instead of using checklist_t; I have a working proof-of-concept of this already), and remove the need for other clients to reload the save when a client joins, as mentioned previously. The latter is especially important for Extended because the save games are much larger, for example on the Bridgewater-Brunel server.
This would be extremely helpful, so I hope it succeeds.

prissi

#6
So it seems there is a majority in favour of small patches.

I think you best submit those patches yourself. Check you pm, ceeac.

ceeac

Thanks! I submitted the changes in r9283 + r9284. (Someone should contact aburch again so the github mirror updates correctly.)