Possible to trigger a crash by saving between story dialogs

[Hans Joachim Desserud]

Ages ago I noticed a minor difference; in the campaign maps I can adjust the speed of the game between dialogs telling the story, but in the tutorial I cannot. Thus I was able to speed up the game, making the camera move faster to get to the next dialog. I never thought much of it, but earlier today I discovered the following:
1. Start a new campaign map (Barbarians - Eyes in the dark).
2. Click Ok to close the first dialog.
3. Immediately press "Page Down" to pause the game.
4. Open the menu and attempt to save the game.

Game: Writing Command Queue Data ... PANIC: unprotected error in call to Lua API (Trying to persist a User Interface Panel which is no supported!
stack traceback:)

I suppose the campaign maps should ignore the user trying to change the speed like the tutorial does.

(I don't see this as a something we need to deal with before build18, since it's a rather extreme edge case which doesn't seem to have been reported earlier and is unlikely to be triggered by anyone. If someone do end up fixing it, that's of course cool too :) )

Widelands r6643 on Ubuntu 13.04.

[cghislai (charlyghislain)]

Normally saving is prevented when lua dialogs are opened, then authorized back when they are closed. So the code to handle that is already there, and an easy fix may exist. I will check that

[cghislai (charlyghislain)]

Well, I'm not sure how lua scripts are parsed, but it looks like the lua panel is being attempted to be persisted even when none is opened. Sirver do you have an idea of what's going on here?

Actually it is easy to trigger now that the game pause on save dialog. Just press ctrl-s during a move leading to a dialog or after closing one.

[SirVer (sirver)]

The panel in question is the MapView - smooth_move (see scripting/ui.lua) holds to a local reference of it. There are a bunch of ways to solve this:

1) do not hold on to a MapView instead always recreate it in the lua code when needed.
2) add persistence support for MapView only (simple to add in the lua code, but a test will be needed too).
3) forbid saving/user input during scrolling.

I think 2 is the best solution and solves 90% of the ui-is-not-persistable problem. 1 is easiest. 3 is probably not such a good idea.

[cghislai (charlyghislain)]

I had a look into this. I tried to persist all fields that were used in the lua interface. Some worked (the viewpoint for instance), some didn't (the statistics/census flags) - looks like something goes wrong between the initial game loading and the replay save/load-, and some probably needs to be discarded (the road building).

So is it ok to persist only these fields, and is it ok to forget about the flags and current road building?

[SirVer (sirver)]

> I had a look into this. I tried to persist all fields that were used in the lua interface.
I do not understand what you mean by this - could you elaborate?

> So is it ok to persist only these fields, and is it ok to forget about the flags and current road building?
sure - currently nothing is saved. Saving only a bit seems better to me. The correct approach of course would be to save the state of the UI to save games (open windows and so on), then Lua could just piggy back on this. This is a big more work though - the UI was not designed to be savable.

[Hans Joachim Desserud (hjd)]

As discovered by MP in duplicate bug 1205448, we also have (at least!) one campaign where this issue will be triggered; Empire map 2.

1. Start the game.
2. Wait for the objective to build military buildings to secure your territory.
3. Build one on the eastern border, and wait for it to complete.
4. Once the building is done, you have completed the objective and since you have opened up to the east and discovered the mountains you get a new objective.

However, now that the autosaving on completing objectives is in place (bug 536507) it will first try to attempt to save the game upon completion before displaying the new dialog. This triggers the same crash. This means we will need to fix this issue before shipping build18.

We also need to play through all of campaign maps (and probably scenarios too) to make sure they are still working with all the changes which has been merged in during this release cycle.

[cghislai (charlyghislain)]

>> I had a look into this. I tried to persist all fields that were used in the lua interface.
> I do not understand what you mean by this - could you elaborate?
I meant all properties that were used through the lua_interface.

I will push the branch so we can get some code to look at while discussing.
And indeed it is quite a serious crash that needs to be fixed

[cghislai (charlyghislain)]

Also concerning #6: This may be even trickier. If upon objective completion the script display a dialog, the dialog may be created directly while the saving is defered to the game loop. When the save will occur the dialog will be displayed.
For such case, I was thinking of, while waiting for proper ui persistence, silently discarding open dialogs.

[MP (pagel-d)]

potentially not fixed in 6691. I had a crash in the same empire campaign #2, but at a later point. It may have been when my tower and/or my mining infrastructure was completed...

Unfortunately this was not reproducible on the next go-around. I made it to the "kill the barbarians" autosave and dialogue.

This scenario might be worth further investigation. I'm attaching my last autosave prior to the crash just in case you guys have any ideas as to how to induce.

[cghislai (charlyghislain)]

Your savegame didn't load, blocking at the same assert (see bug 1205010).
Are you sure you didn't continue a game saved before the fix was in?

[cghislai (charlyghislain)]

And thanks for the intensive testing!

[MP (pagel-d)]

seeing as this scenario doesn't have any buildings conquered and only started with an HQ, I don't know how that would be relevant. I guess it might be if the AI started with some building they wish to get rid of?

I'm not 100% certain if I started fresh or with a savegame from 6667.

In any case, I *think* this will be my last playing/testing of the day.

[cghislai (charlyghislain)]

I tried to play that scenario, just to see, and ran into bug 1207477 which crashed the game as well. It is related to another issue with message expiration.
Anyway, I will investigate. Thanks for reporting!

[SirVer (sirver)]

I played the scenario and tried specfically to trigger the bug without any luck. Calling this fixed for now. I ran however in a soldier battle bug which I assume is the one that is already reported.

[SirVer (sirver)]

Released in build-18 rc1.