[Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-07-27 Thread cghislai
cghislai has proposed merging lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1203329 in widelands: "Possible to trigger a crash by saving between story dialogs" https://bugs.launchpad.net/wid

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-08-01 Thread SirVer
The proposal to merge lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/lua_mapview_persistence/+merge/177251 -- https://code.launchpad.net/~widela

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-07-27 Thread SirVer
Review: Needs Fixing 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

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-07-27 Thread cghislai
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 f

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-07-28 Thread SirVer
> > 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 > wou

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-07-28 Thread SirVer
btw, is this ready for review too? -- https://code.launchpad.net/~widelands-dev/widelands/lua_mapview_persistence/+merge/177251 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/lua_mapview_persistence. ___ Mailing list

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-07-31 Thread cghislai
Yes it is. I just needed to restore the m_panel pointer in the __unpersist method. -- https://code.launchpad.net/~widelands-dev/widelands/lua_mapview_persistence/+merge/177251 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/lua_mapview_persistence. ___

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-07-31 Thread SirVer
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 t

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-07-31 Thread SirVer
Review: Needs Fixing -- https://code.launchpad.net/~widelands-dev/widelands/lua_mapview_persistence/+merge/177251 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/lua_mapview_persistence. ___ Mailing list: https://la

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-08-01 Thread cghislai
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 tha

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-08-01 Thread SirVer
Review: Approve 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