widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1551578-place_building-crash into lp:widelands.

Commit message:
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.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1551578 in widelands: "Fortified Village crashes when building can't be placed"
  https://bugs.launchpad.net/widelands/+bug/1551578

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1551578-place_building-crash/+merge/287660

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.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1551578-place_building-crash into lp:widelands.

=== 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",
     ] ),