WL does not load a savegame when a conf file has changed; error meassage should be shown

[Astuur]

I have saved a savegame while a new Warehouse is being erected.

I have then editied the \widelands\tribes\empire\warehouse\conf
from:

size=medium

[buildcost]
trunk=2
wood=2
stone=2
marble=3
marblecolumn=2

[idle]
pics=warehouse_i_??.png # ???
hotspot=58 55

to

size=big

[buildcost]
trunk=8
wood=8
stone=8
marble=8
marblecolumn=8

[idle]
pics=warehouse_i_??.png # ???
hotspot=58 55

After having done that, not only the savegame with the warehouse being built cannot be loaded,
but also any other game where an empire warehouse exists. (have tried a few that I could remember)
WL simply crashes while loading, after the initial screen with no error message.

I can start a new game and built a costly warehouse, or load such games, where no empire wares exists.

I have attached the stdout.txt

[Hans Joachim Desserud (hjd)]

I have been unable to reproduce this issue. What I did was:
Start a new new game as Empire.
Start building a warehouse
Save the game
Edit tribes/empire/warehouse/conf replacing the values for size and buildcosts
Load the game again.

Works fine here. The old warehouse is still under construction according to the old values, and if I want to build a new warehouse it uses the new settings.

Have you done any other changes than editing warehouse conf?

[Hans Joachim Desserud (hjd)]

Alternatively, are you able to reproduce this for other buildings/tribes?

[Astuur (wolfsteinmetz)]

No, I did not change anthing else than that conf file.
(before the crash appeared) I have changed other things in conf files
in my installation though, but had no problems.(bzr5895)
I have double checked that my editor has not inserted DOS CR/LF line endings.
I might add, that after changing the conf file back to the original
(replacing with the installation file) the savegame loads again.

Does it help to attach the savegame in question?
I'll do that.
Maybe you can try to load it and see what happens.
It was a subdir (saved uncompressed) and I made it into a .rar file.
I will try tomorrow whether other buildings/tribes show the same.
Also, I will try to do exactly what you have done (with a new game) and report back.
Anything else I can do?

[Astuur (wolfsteinmetz)]

I just tested again with a clean new installation.
I only changed the size= entry to big.
Again the crash as reported.
OpenGL is on
Savedir is"C:\Dokumente und Einstellungen\Wolf\.widelands\save"
could it be that you need to place your warehouse at a spot that only allows
a medium size building to trugger that bug?
Did you build in such a place when you tested?
good night for now.

[Hans Joachim Desserud (hjd)]

"could it be that you need to place your warehouse at a spot that only allows
a medium size building to trugger that bug?"
Yep. Additionally it looks like the area around affects this too. When I built at a medium house near a border it went fine, but when I built near some trees blocking the area, I got a crash. The following error message is printed to the terminal:

widelands: /home/user/widelands/src/logic/immovable.cc:77: void Widelands::BaseImmovable::set_position(Widelands::Editor_Game_Base&, Widelands::Coords): Assertion `f.field->immovable->get_size() == NONE' failed.

I'll attach parts of the output I got, along with a backtrace.
I guess the reason why I couldn't reproduce it initially was because I built in areas where big buildings already fit. Thanks for reporting this.

[Hans Joachim Desserud (hjd)]

The aforementioned backtrace.
Triggered by :
Creating a new game.
Started building a warehouse in a spot which allowed medium buildings at the most.
Saved the game.
Edited the size in warehouse conf from medium to large
Loaded the game.

Widelands bzr r5895

[SirVer (sirver)]

Widelands rely on the conf files to be compatible to the savegame. That's why we always have to add compatibility hacks when we change balancing stuff around. If we'd decide to make one building bigger/smaller, we would need a special case in the code for loading these buildings at the moment. It is just that widelands - the engine does not support changing configurations in the game.

I am for Won't fix on this. It is just not something we planned for and it seems not so terrible useful for me either.

[Astuur (wolfsteinmetz)]

That's all right with me.
I almost thought that I did something illegal there :)
But how about an informative error message?

[SirVer (sirver)]

yep, an error message would be nice. Crashing is never polite :).

[SirVer (sirver)]

Setting to incomplete for bug sweeping.

[Hans Joachim Desserud (hjd)]

This issue is still reproducible in r6935.

As SirVer mentioned above, the engine is not able to deal with changes like these without special-case code for how to deal with it. However, if such a situation occurs, the engine should receive feedback in the form of an error message explaining why the save game failed to load. (This should be displayed in a dialog similar to how other loading errors are caught.)

[SirVer (sirver)]

Setting to incomplete for bug sweeping.

[Launchpad Janitor (janitor)]

[Expired for widelands because there has been no activity for 60 days.]

[GunChleoc (gunchleoc)]

I did some digging. Increasing the size if the building doesn't fit on the map results in the following crash:

#1 0x0000000000ed355c in Widelands::Immovable::Loader::load_finish (
    this=0x9950640)
    at /home/bratzbert/sources/widelands/trunk/src/logic/map_objects/immovable.cc:641
#2 0x00000000011ca6b5 in Widelands::MapObjectPacket::load_finish (
    this=0x7fffffff9780)
    at /home/bratzbert/sources/widelands/trunk/src/map_io/map_object_packet.cc:128
#3 0x0000000000fc3b30 in Widelands::WidelandsMapLoader::load_map_complete (
    this=0x344d6a0, egbase=...,
    load_type=Widelands::MapLoader::LoadType::kScenario)
    at /home/bratzbert/sources/widelands/trunk/src/map_io/widelands_map_loader.cc:280
#4 0x00000000012ab06a in Widelands::GameMapPacket::read_complete (
    this=0x7fffffff9960, game=...)
    at /home/bratzbert/sources/widelands/trunk/src/game_io/game_map_packet.cc:54
#5 0x00000000012a93e4 in Widelands::GameLoader::load_game (
    this=0x7fffffff9b30, multiplayer=false)

And the crashing line is:

    Immovable& imm = dynamic_cast<Immovable&>(*get_object());

I have found no easy way to check the validity of the object.

There is also no size to compare to catch any changes, since the size is only defined in MapObjectDescription, not in MapObject.

So, unless somebody else figures out something clever, the only way to fix this that I see right now is to add an extra size variable to each map object. Is it worth the extra memory consumption? Maybe we could avoid having the extra variable in-game and use it during saveloading only.

[SirVer (sirver)]

> Is it worth the extra memory consumption?

Probably neglibile. a byte for a few hundred objects? I am more concerned about the duplication of the information in MapObject and its description. The code path that deals with these two being different will basically never be exercised and therefore bitrot rather quickly. I think we accept the crash and make this a wontfix.

[GunChleoc (gunchleoc)]

Yep, duplicate info is always bad. Will set this to Won't Fix.