Merge lp://staging/~widelands-dev/widelands/lua_mapview_persistence into lp://staging/widelands

I think this is the wrong approach - the LuaClass does not have any data members, so it should not persist anything at all. The MapView (c++ class) should save its data to save games one day, so that you have the same view and so on when loading.

For the Lua fix here, all clases that can get a handle again on the c++ class on a reaload should overwrite the __persists methods which basically does nothing.

The logic here is: if you have a handle to a Window (e.g. a building window of any building) in a lua variable and the game is saved what should the window persist? It cannot reopen the window again after reload (since there is no open_window(name) method), the reloaded game will not have opened the window too, so the variable would contain crap after reloading. For the MapView this is not true: you can always get a hold on the map view again, because there is always a singular one available.

mmh, I fear this was not very well explained. Do you still get what I mean?

I think so. but, then, the fix is already there, as I think mapview saves the state (census/statistics/viewpoint) and they are loaded.

So, in this case, the L_Mapview class shoud override __persits/__unpersist, and make sure the pointer is correct upon loading (or it is not even needed?).

Now for the other window, that would need another data paquet class, which would look for UniqueWindow registries, or buildingwindows serials, or field windows coords, and save positions and so. I am not sure for the scripted windows though.

>
> So, in this case, the L_Mapview class shoud override __persits/__unpersist,
> and make sure the pointer is correct upon loading (or it is not even needed?).
I think nothing is needed - just two empty functions.

> Now for the other window, that would need another data paquet class, which
> would look for UniqueWindow registries, or buildingwindows serials, or field
> windows coords, and save positions and so.
the unique windows would need special treatment. Building windows would need special treatment. And mission texts would be very difficult to get right. Starting with the unique windows would be a possiblity, but I think all of this is pretty low priority right now.

> I am not sure for the scripted windows though.

btw, is this ready for review too?

Yes it is. I just needed to restore the m_panel pointer in the __unpersist method.

I think this is the very right approach to doing this. I am still in the USA right now, and pretty loaded, so I am still slow to review code. However one thing immediately: game_mapview_player_data_paquet should be game_mapview_player_data_packet. Same for the classes. Why did you change this to Paquet?

I revert the renaming interactiveplayer->mapview. I did it because I thought it would make more sense, and I didn't even notice i misspelled 'packet'. And now that I think of it, interactive_player makes more sense. Unfortunately, the diff doesn't show the content differences. The only thing that changed is that it tries to persist interactive_base fields, and only use the interactive_player pointer to persist the player number.

Speaking of which, does game.get_ibase() sometimes return null? Here the check is made, but I remember some place in the code where it assumed the pointed object exists.

I applied a diff of this to trunk and committed this as a new revision. That means your editing history is gone, but I didn't feel comfortable having this removed and added of the same file in the same commit - I do not trust bzr this far :).

This is therefore merged, i.e. it's content is in trunk.