Autosave should not trigger while game is paused

[Nathan Tuggy]

If I pause the game when I realize I've made a stupid mistake in order to try to figure out how best to do this, the autosave should not overwrite previous saves during the time I have it paused.

[SirVer (sirver)]

I agree. This will make the autosave logic more complicated though. Maybe we should move autosave over to game time instead of realtime now - Widelands is stable enough for this imho.

[SirVer (sirver)]

Setting to incomplete for bug sweeping.

[Nathan Tuggy (tuggyne)]

Hmm, I guess I'm a bit out of touch, but what sort of bug sweeping do you mean exactly?

[SirVer (sirver)]

Moving to gametime and adding a autosave comman din the command queue would also solve timing issues in the Lua testsuite.

[Hans Joachim Desserud (hjd)]

Nathan: For a while now, we have been gradually going through the backlog of old bug reports. Some of them contain enough information to work on, other issues which has been fixed years ago but are still open, while others still may require too much work or added complexity to work on. Basically, we're trying to get a better overview of the issues which have accumalated over the years, and look more into which we want to focus on.

[Nathan Tuggy (tuggyne)]

Thanks, Hans. I figured it might be something like that but didn't want to act on incomplete information.

[Nasenbaer (nasenbaer)]

Bug fixed in rev. 7610 - autosave logic is now based upon gametime, not realtime. To produce an almost clean fix for this bug, I further removed WLApplication::get()->get_time() and replaced it with SDL_GetTicks() were used in the code. Further I changed data types where SDL_GetTicks() values are used from int32_t to uint32_t (at least where possible -> in some cases calculations with possible negative numbers are done - of course I left int32_t in those places (the not so clean leftover parts of this fix)).

[GunChleoc (gunchleoc)]

How often will a game now be saved in realtime if I set the speed to 60x and the autosave interval to 1 minute? Will this trigger a save every second now, or is this accounted for?

[SirVer (sirver)]

Great to see you back in full swing, Peter :).

I think this change should have gone through a code review - the diff is pretty big and I think the solution needs some discussion.

For example I agree with #8 that saving every second on x60 is excessive. How about trying to save every 60s realtime, but according to the current game speed. That would mean that when the game is slowed down, saving would maybe only happen after 3x the time - not great either. Maybe then just limiting the period to a minimum value? Say, autosave is never faster than 3 minutes or so?

Also, I think that saving should be done through a Command in the command queue. It would make saving truly deterministic, also for the regression suite. And it would be less special casing for the save code - it would just be a logic command.

[GunChleoc (gunchleoc)]

How about we revert it to as it was for now, but remember the gametime as well - when the gametime hasn't increased, skip the save.

[Nasenbaer (nasenbaer)]

Sorry for the long delay. Unfortunally I wasn't on the bug notification list for comments.

I understand and agree all of your comments and thought about it a bit as well.
Basically I see the following issues:
* saving based on gametime * gamespeed would imply that gamespeed is monitored all the time - as it will be changed at some time. So this is kind of inconsistent
* saving based on realtime, but ensuring, that the gametime has increased as well is no solution either. E.g. if the autosave time is set to 15 minutes and 14 minutes past since the last autosave, than it's paused before the 15 minutes elapsed, the autosave will hit in anyways, as the gametime is different to the last time the game had been autosaved.

Therefore my idea would be to switch back to realtime and add a check for the current game speed. If the game is paused, no autosaving is done and the autosaving time is set that way, that Widelands tries to autosave again 30 seconds (realtime) later.

What do you think?

[kaputtnik (franku)]

From a players point of view, i think the settings of autosave is always related to realtime, not to gametime.

No autosaving if the game is paused is ok i think. So +1 for Nasenbaers idea.

[GunChleoc (gunchleoc)]

+1 for #11 from me too :)

[Nasenbaer (nasenbaer)]

Refixed in bzr rev. 7623 - autosave is again based upon realtime and doesn't hit in, if the game is paused (but the trigger is simply delayed for 30 seconds for recheck.

[GunChleoc (gunchleoc)]

BTW we also have an open bug concerning the uint/int problem with time:

https://bugs.launchpad.net/widelands/+bug/1380058

The branch linked to the other bug is abandoned - I messed something up and then went on to do something else.

[GunChleoc (gunchleoc)]

Fixed in build19-rc1.