widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1724145-corrupt-zip-when-saving into lp:widelands.

Commit message:
Do not save LuaEconomy objects in Lua scrips for use later

- Due to economy merging, the economy object can become unavailable and cause crashes.
- Fixed Lua scripts and added warnings to documentation.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1724145 in widelands: "Crashes in master-2511_release_x64 on save"
  https://bugs.launchpad.net/widelands/+bug/1724145

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1724145-corrupt-zip-when-saving/+merge/349087

The underlying economy object can disappear from eco:ware_target_quantity calls due to economy merging. I found no way to check for that in the ware_target_quantity function itself, so I have added warnings to the documentation and fixed the Lua scripts.

To reproduce the Bug:

1. Play Empire 3: Neptune's Revenge until you trigger the "Lower Marble Columns" objective. Do not take any action on the objective.

2. Delete a road so that you end up with 2 economies

3. Connect the outer economy to the Headquarters Shipwreck economy: Pick a flag in the outer economy to start road building, then a flag in the original economy.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1724145-corrupt-zip-when-saving into lp:widelands.

=== modified file 'data/campaigns/emp03.wmf/scripting/mission_thread.lua'
--- data/campaigns/emp03.wmf/scripting/mission_thread.lua	2018-01-25 19:28:03 +0000
+++ data/campaigns/emp03.wmf/scripting/mission_thread.lua	2018-07-07 11:01:21 +0000
@@ -99,8 +99,7 @@
    local objective = add_campaign_objective(obj_lower_marble_column_demand)
 
    --- Check the headquarters' flag's economy
-   local eco = sf.brn.immovable.economy
-   while eco:ware_target_quantity("marble_column") ~= 4 do
+   while sf.brn.immovable.economy:ware_target_quantity("marble_column") ~= 4 do
       sleep(2434)
    end
    sleep(4000)

=== modified file 'data/campaigns/emp04.wmf/scripting/starting_conditions.lua'
--- data/campaigns/emp04.wmf/scripting/starting_conditions.lua	2018-05-29 20:14:16 +0000
+++ data/campaigns/emp04.wmf/scripting/starting_conditions.lua	2018-07-07 11:01:21 +0000
@@ -20,8 +20,7 @@
    r4 = p3:place_road(field_mill.immovable.flag, "br", "r", true)
 
 p3:forbid_buildings("all")
-local eco = field_warehouse.brn.immovable.economy
-eco:set_ware_target_quantity("beer", 180)
+field_warehouse.brn.immovable.economy:set_ware_target_quantity("beer", 180)
 
 -- =======================================================================
 --                                 Player 1

=== modified file 'data/campaigns/tutorial04_economy.wmf/scripting/mission_thread.lua'
--- data/campaigns/tutorial04_economy.wmf/scripting/mission_thread.lua	2018-05-03 06:01:14 +0000
+++ data/campaigns/tutorial04_economy.wmf/scripting/mission_thread.lua	2018-07-07 11:01:21 +0000
@@ -152,8 +152,7 @@
    message_box_objective(plr, economy_settings2)
    o = message_box_objective(plr, economy_settings3)
 
-   local eco = sf.brn.immovable.economy
-   while eco:ware_target_quantity("marble_column") ~= 20 do
+   while sf.brn.immovable.economy:ware_target_quantity("marble_column") ~= 20 do
       sleep(200)
    end
    -- wait that the player has really changed the target quantity

=== modified file 'src/scripting/lua_map.cc'
--- src/scripting/lua_map.cc	2018-04-27 06:11:05 +0000
+++ src/scripting/lua_map.cc	2018-07-07 11:01:21 +0000
@@ -3585,6 +3585,10 @@
 
       Returns the amount of the given ware that should be kept in stock for this economy.
 
+      **Warning**: Since economies can disappear when a player merges them
+      through placing/deleting roads and flags, you should get a fresh economy
+      object every time you use this function.
+
       :arg warename: the name of the ware.
       :type warename: :class:`string`
 */
@@ -3605,6 +3609,10 @@
 
       Returns the amount of the given worker that should be kept in stock for this economy.
 
+      **Warning**: Since economies can disappear when a player merges them
+      through placing/deleting roads and flags, you should get a fresh economy
+      object every time you use this function.
+
       :arg workername: the name of the worker.
       :type workername: :class:`string`
 */
@@ -3625,6 +3633,10 @@
 
       Sets the amount of the given ware type that should be kept in stock for this economy.
 
+      **Warning**: Since economies can disappear when a player merges them
+      through placing/deleting roads and flags, you should get a fresh economy
+      object every time you use this function.
+
       :arg warename: the name of the ware type.
       :type warename: :class:`string`
 
@@ -3651,6 +3663,10 @@
 
       Sets the amount of the given worker type that should be kept in stock for this economy.
 
+      **Warning**: Since economies can disappear when a player merges them
+      through placing/deleting roads and flags, you should get a fresh economy
+      object every time you use this function.
+
       :arg workername: the name of the worker type.
       :type workername: :class:`string`
 
@@ -4043,6 +4059,10 @@
 
       (RO) Returns the economy that this flag belongs to.
 
+      **Warning**: Since economies can disappear when a player merges them
+      through placing/deleting roads and flags, you should get a fresh economy
+      object every time you call another function on the resulting economy object.
+
       :returns: The :class:`Economy` associated with the flag.
 */
 int LuaFlag::get_economy(lua_State* L) {