Merge lp:~widelands-dev/widelands/bug-1551578-place_building-crash into lp:widelands

[GunChleoc]

Fixed crash with playing buildings in Fortified Village when there isn't enough space by redesigning the error message. This also unmasked some other error messages, which showed some outdated building names in Barbarians Fortified Village.

Fixed crash with playing buildings in Fortified Village when there isn't enough space by redesigning the error message. This also unmasked some other error messages, which showed some outdated building names in Barbarians Fortified Village.

[TiborB (tiborb95)]

Nice

review: Approve

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 789. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/112898878.
Appveyor build 635. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1551578_place_building_crash-635.

[GunChleoc (gunchleoc)]

Thanks for the review :)

@bunnybot merge

=== modified file 'data/scripting/infrastructure.lua'
--- data/scripting/infrastructure.lua	2015-11-24 18:19:22 +0000
+++ data/scripting/infrastructure.lua	2016-03-01 16:19:34 +0000
@@ -90,9 +90,8 @@
 --          are filled with workers by default.
 --    :type b1_descr: :class:`array`
 function prefilled_buildings(p, ...)
-   -- TODO(GunChleoc) this should produce an error message if a building/ware/worker doesn't exist.
    for idx,bdescr in ipairs({...}) do
-      b = p:place_building(bdescr[1], wl.Game().map:get_field(bdescr[2],bdescr[3]), false, true)
+      local b = p:place_building(bdescr[1], wl.Game().map:get_field(bdescr[2],bdescr[3]), false, true)
       -- Fill with workers
       if b.valid_workers then b:set_workers(b.valid_workers) end
       if bdescr.workers then b:set_workers(bdescr.workers) end
@@ -153,9 +152,11 @@
       end
       table.remove(fields, idx)
    end
-   error(string.format(
-      "Could not find a suitable position for building '%s' for player %i",
-      building, plr.number)
+   plr:send_message(
+      -- TRANSLATORS: Short for "Not enough space"
+      _"No Space",
+      rt(p(_([[Some of your starting buildings didn’t have enough room and weren’t built. You are at a disadvantage with this; consider restarting this map with a fair starting condition.]]))),
+      {popup=true, heading=_"Not enough space"}
    )
 end
 

=== modified file 'data/tribes/scripting/starting_conditions/atlanteans/fortified_village.lua'
--- data/tribes/scripting/starting_conditions/atlanteans/fortified_village.lua	2016-01-28 05:24:34 +0000
+++ data/tribes/scripting/starting_conditions/atlanteans/fortified_village.lua	2016-03-01 16:19:34 +0000
@@ -10,17 +10,16 @@
    descname = _ "Fortified Village",
    func =  function(plr, shared_in_start)
 
-   local sf = wl.Game().map.player_slots[plr.number].starting_field
-   if shared_in_start then
-      sf = shared_in_start
-   else
-      plr:allow_workers("all")
-   end
-
-   local h = plr:place_building("atlanteans_castle", sf, false, true)
-   h:set_soldiers{[{0,0,0,0}] = 12}
-
-   if not pcall(function()
+      local sf = wl.Game().map.player_slots[plr.number].starting_field
+      if shared_in_start then
+         sf = shared_in_start
+      else
+         plr:allow_workers("all")
+      end
+
+      local h = plr:place_building("atlanteans_castle", sf, false, true)
+      h:set_soldiers{[{0,0,0,0}] = 12}
+
       place_building_in_region(plr, "atlanteans_warehouse", sf:region(7), {
          wares = {
             diamond = 7,
@@ -97,13 +96,5 @@
       place_building_in_region(plr, "atlanteans_sawmill", sf:region(11), {
          wares = { log = 1 }
       })
-   end) then
-      plr:send_message(
-         -- TRANSLATORS: Short for "Not enough space"
-         _"No Space",
-         rt(p(_([[Some of your starting buildings didn’t have enough room and weren’t built. You are at a disadvantage with this; consider restarting this map with a fair starting condition.]]))),
-         {popup=true, heading=_"Not enough space"}
-      )
    end
-end
 }

=== modified file 'data/tribes/scripting/starting_conditions/barbarians/fortified_village.lua'
--- data/tribes/scripting/starting_conditions/barbarians/fortified_village.lua	2016-01-28 05:24:34 +0000
+++ data/tribes/scripting/starting_conditions/barbarians/fortified_village.lua	2016-03-01 16:19:34 +0000
@@ -10,18 +10,17 @@
    descname = _ "Fortified Village",
    func =  function(plr, shared_in_start)
 
-   local sf = wl.Game().map.player_slots[plr.number].starting_field
-
-   if shared_in_start then
-      sf = shared_in_start
-   else
-      plr:allow_workers("all")
-   end
-
-   local h = plr:place_building("barbarians_citadel", sf, false, true)
-   h:set_soldiers{[{0,0,0,0}] = 12}
-
-   if not pcall(function()
+      local sf = wl.Game().map.player_slots[plr.number].starting_field
+
+      if shared_in_start then
+         sf = shared_in_start
+      else
+         plr:allow_workers("all")
+      end
+
+      local h = plr:place_building("barbarians_citadel", sf, false, true)
+      h:set_soldiers{[{0,0,0,0}] = 12}
+
        place_building_in_region(plr, "barbarians_warehouse", sf:region(7), {
          wares = {
             ax = 5,
@@ -78,25 +77,17 @@
       place_building_in_region(plr, "barbarians_helmsmithy", sf:region(12), {
          wares = { iron = 4, gold = 4 }
       })
-      place_building_in_region(plr, "barbarians_metalworks", sf:region(12), {
+      place_building_in_region(plr, "barbarians_metal_workshop", sf:region(12), {
          wares = { iron = 8 },
       })
       place_building_in_region(plr, "barbarians_ax_workshop", sf:region(12), {
          wares = { coal = 8 },
       })
-      place_building_in_region(plr, "barbarians_hardener", sf:region(12), {
+      place_building_in_region(plr, "barbarians_wood_hardener", sf:region(12), {
          wares = { log = 1 },
       })
       place_building_in_region(plr, "barbarians_lime_kiln", sf:region(12), {
          wares = { granite = 6, coal = 3 },
       })
-   end) then
-      plr:send_message(
-         -- TRANSLATORS: Short for "Not enough space"
-         _"No Space",
-         rt(p(_([[Some of your starting buildings didn’t have enough room and weren’t built. You are at a disadvantage with this; consider restarting this map with a fair starting condition.]]))),
-         {popup=true, heading=_"Not enough space"}
-      )
    end
-end,
 }

=== modified file 'data/tribes/scripting/starting_conditions/empire/fortified_village.lua'
--- data/tribes/scripting/starting_conditions/empire/fortified_village.lua	2016-01-28 05:24:34 +0000
+++ data/tribes/scripting/starting_conditions/empire/fortified_village.lua	2016-03-01 16:19:34 +0000
@@ -12,16 +12,15 @@
 
    local sf = wl.Game().map.player_slots[plr.number].starting_field
 
-   if shared_in_start then
-      sf = shared_in_start
-   else
-      plr:allow_workers("all")
-   end
-
-   local h = plr:place_building("empire_castle", sf, false, true)
-   h:set_soldiers{[{0,0,0,0}] = 12}
-
-   if not pcall(function()
+      if shared_in_start then
+         sf = shared_in_start
+      else
+         plr:allow_workers("all")
+      end
+
+      local h = plr:place_building("empire_castle", sf, false, true)
+      h:set_soldiers{[{0,0,0,0}] = 12}
+
       place_building_in_region(plr, "empire_warehouse", sf:region(7), {
          wares = {
             armor_helmet = 2,
@@ -115,13 +114,5 @@
       })
 
       place_building_in_region(plr, "empire_stonemasons_house", sf:region(11))
-   end) then
-      plr:send_message(
-         -- TRANSLATORS: Short for "Not enough space"
-         _"No Space",
-         rt(p(_([[Some of your starting buildings didn’t have enough room and weren’t built. You are at a disadvantage with this; consider restarting this map with a fair starting condition.]]))),
-         {popup=true, heading=_"Not enough space"}
-      )
    end
-end
 }

=== modified file 'utils/buildcat.py'
--- utils/buildcat.py	2016-03-01 09:26:35 +0000
+++ utils/buildcat.py	2016-03-01 16:19:34 +0000
@@ -50,6 +50,7 @@
                     "../../src/*/*/*/*.h",
                     "../../src/*/*/*/*/*.h",
                     "../../src/*/*/*/*/*/*.h",
+                    "../../data/scripting/*.lua",
                     "../../data/scripting/editor/*.lua",
                     "../../data/scripting/widelands/*.lua",
     ] ),