widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine into lp:widelands.

Commit message:
Fixed desync in the Smugglers scenario by using a single coroutine and selecting the wares round robin. The scenario can now be played with any tribe combination as soon as we implement the selection code in the UI.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1751440 in widelands: "Desync in Smugglers scenario"
  https://bugs.launchpad.net/widelands/+bug/1751440

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine/+merge/354747
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine into lp:widelands.

=== modified file 'data/maps/MP_Scenarios/Smugglers.wmf/scripting/multiplayer_init.lua'
--- data/maps/MP_Scenarios/Smugglers.wmf/scripting/multiplayer_init.lua	2017-12-10 10:52:45 +0000
+++ data/maps/MP_Scenarios/Smugglers.wmf/scripting/multiplayer_init.lua	2018-09-12 06:21:05 +0000
@@ -17,19 +17,6 @@
 
 points_to_win = 2000
 
-route_descrs = {
-   { value = 2, send = map:get_field(35, 52):region(2), recv = map:get_field(96, 77):region(2) },
-   { value = 2, send = map:get_field(98, 55):region(2), recv = map:get_field(34, 76):region(2) },
-
-   { value = 3, send = map:get_field(64, 57):region(2), recv = map:get_field(78, 73):region(2) },
-   { value = 3, send = map:get_field(77, 58):region(2), recv = map:get_field(65, 72):region(2) },
-
-   { value = 2, send = map:get_field(62, 93):region(2), recv = map:get_field(78, 34):region(2) },
-   { value = 2, send = map:get_field(80, 95):region(2), recv = map:get_field(63, 29):region(2) },
-   { value = 2, send = map:get_field(18, 66):region(2), recv = map:get_field(121, 61):region(2) },
-   { value = 2, send = map:get_field(124, 72):region(2), recv = map:get_field(19, 57):region(2) }
-}
-
 -- =================
 -- Global Variables
 -- =================
@@ -41,66 +28,48 @@
 -- ================
 -- Initializations
 -- ================
+
+-- If the tribes don't have any wares in common, nothing can be smuggled
+-- This should not happen, but let's have a safeguard anyway.
+function check_ware_compatiblity(player1, player2)
+   local has_compatible_ware = false
+   for idx,ware in pairs(player1.tribe.wares) do
+      if player2.tribe:has_ware(ware.name) then
+         has_compatible_ware = true
+         break
+      end
+   end
+   if not has_compatible_ware then
+      do_game_over()
+   end
+end
+
 function assign_teams()
    game.players[1].team = 1
    game.players[2].team = 1
    game.players[3].team = 2
    game.players[4].team = 2
+   check_ware_compatiblity(game.players[1], game.players[2])
+   check_ware_compatiblity(game.players[3], game.players[4])
 end
 
 function place_headquarters()
+   include "map:scripting/starting_conditions.lua"
    for idx,player in ipairs(game.players) do
       local sf = map.player_slots[player.number].starting_field
-
-      prefilled_buildings(player, { "barbarians_headquarters", sf.x, sf.y,
-         wares = {
-            ax = 5,
-            bread_paddle = 2,
-            blackwood = 32,
-            cloth = 5,
-            coal = 12,
-            felling_ax = 4,
-            fire_tongs = 2,
-            fish = 6,
-            fishing_rod = 2,
-            gold = 4,
-            grout = 12,
-            hammer = 12,
-            hunting_spear = 2,
-            iron = 12,
-            iron_ore = 5,
-            kitchen_tools = 4,
-            meal = 4,
-            meat = 6,
-            pick = 14,
-            barbarians_bread = 8,
-            ration = 12,
-            granite = 40,
-            scythe = 6,
-            shovel = 4,
-            snack = 3,
-            thatch_reed = 24,
-            log = 80,
-         },
-         workers = {
-            barbarians_blacksmith = 2,
-            barbarians_brewer = 1,
-            barbarians_builder = 10,
-            barbarians_carrier = 40,
-            barbarians_charcoal_burner = 1,
-            barbarians_gardener = 1,
-            barbarians_geologist = 4,
-            barbarians_lime_burner = 1,
-            barbarians_lumberjack = 3,
-            barbarians_miner = 4,
-            barbarians_ranger = 1,
-            barbarians_stonemason = 2,
-            barbarians_ox = 5,
-         },
-         soldiers = {
-            [{0,0,0,0}] = 45,
-         }
-      })
+      local tribename = player.tribe.name
+      if tribename == "barbarians" then
+         set_starting_conditions_barbarians(player, sf)
+      elseif tribename == "empire" then
+         set_starting_conditions_empire(player, sf)
+      elseif tribename == "atlanteans" then
+         set_starting_conditions_atlanteans(player, sf)
+      elseif tribename == "frisians" then
+         set_starting_conditions_frisians(player, sf)
+      else
+         print("We don't have starting conditions for tribe " .. tribename)
+         do_game_over()
+      end
    end
 end
 
@@ -137,6 +106,8 @@
    for idx,descr in ipairs(route_descrs) do
       run(wait_for_established_route, descr)
    end
+
+   do_smuggling()
 end
 
 setup_statistics_hook()

=== modified file 'data/maps/MP_Scenarios/Smugglers.wmf/scripting/smuggling.lua'
--- data/maps/MP_Scenarios/Smugglers.wmf/scripting/smuggling.lua	2016-03-26 09:09:43 +0000
+++ data/maps/MP_Scenarios/Smugglers.wmf/scripting/smuggling.lua	2018-09-12 06:21:05 +0000
@@ -4,6 +4,76 @@
 
 include "scripting/messages.lua"
 
+-- Init routes
+route_descrs = {
+   {
+      value = 2,
+      send = map:get_field(35, 52):region(2),
+      recv = map:get_field(96, 77):region(2),
+      sending_warehouse = nil,
+      receiving_warehouse = nil,
+      wares = {}
+   },
+   {
+      value = 2,
+      send = map:get_field(98, 55):region(2),
+      recv = map:get_field(34, 76):region(2),
+      sending_warehouse = nil,
+      receiving_warehouse = nil,
+      wares = {}
+   },
+
+   {
+      value = 3,
+      send = map:get_field(64, 57):region(2),
+      recv = map:get_field(78, 73):region(2),
+      sending_warehouse = nil,
+      receiving_warehouse = nil,
+      wares = {}
+   },
+   {
+      value = 3,
+      send = map:get_field(77, 58):region(2),
+      recv = map:get_field(65, 72):region(2),
+      sending_warehouse = nil,
+      receiving_warehouse = nil,
+      wares = {}
+   },
+
+   {
+      value = 2,
+      send = map:get_field(62, 93):region(2),
+      recv = map:get_field(78, 34):region(2),
+      sending_warehouse = nil,
+      receiving_warehouse = nil,
+      wares = {}
+   },
+   {
+      value = 2,
+      send = map:get_field(80, 95):region(2),
+      recv = map:get_field(63, 29):region(2),
+      sending_warehouse = nil,
+      receiving_warehouse = nil,
+      wares = {}
+   },
+   {
+      value = 2,
+      send = map:get_field(18, 66):region(2),
+      recv = map:get_field(121, 61):region(2),
+      sending_warehouse = nil,
+      receiving_warehouse = nil,
+      wares = {}
+   },
+   {
+      value = 2,
+      send = map:get_field(124, 72):region(2),
+      recv = map:get_field(19, 57):region(2),
+      sending_warehouse = nil,
+      receiving_warehouse = nil,
+      wares = {}
+   }
+}
+
 -- =================
 -- Helper functions
 -- =================
@@ -35,56 +105,92 @@
    game_over_done = true
 end
 
-function do_smuggling(route_descr, recv_plr, send_plr, recv_whf, send_whf)
-   while 1 do
+function do_smuggling()
+   while true do
       sleep(10000) -- Sleep 10s
-
       if points[1] >= points_to_win or
          points[2] >= points_to_win
       then
          do_game_over()
-         break
-      end
-
-      if not send_whf.immovable or
-         send_whf.immovable.descr.type_name ~= "warehouse" or
-         send_whf.immovable.owner ~= send_plr or
-         not recv_whf.immovable or
-         recv_whf.immovable.descr.type_name ~= "warehouse" or
-         recv_whf.immovable.owner ~= recv_plr
-      then
-         send_to_all(smuggling_route_broken:bformat(
-            (ngettext("%i point", "%i points", route_descr.value)):format(route_descr.value), recv_plr.name, send_plr.name)
-         )
-         run(wait_for_established_route, route_descr)
-         break
-      end
-
-      -- Warp one ware
-      local wares = send_whf.immovable:get_wares("all")
-      local wn = {}
-      for name,count in pairs(wares) do
-         if count > 0 then
-            wn[#wn + 1] = name
+         return
+      end
+
+      for idx, route_descr in ipairs(route_descrs) do
+         if (route_descr.sending_warehouse ~= nil and route_descr.receiving_warehouse ~= nil) then
+            print("NOCOM do smuggling for route " .. idx)
+
+            local send_plr = route_descr.sending_warehouse.owner
+            local recv_plr = route_descr.receiving_warehouse.owner
+            local send_whf = route_descr.sending_warehouse.fields[1]
+            local recv_whf = route_descr.receiving_warehouse.fields[1]
+
+            -- Ensure that the warehouses are still there and owned by the same player
+            if not send_whf.immovable or
+               send_whf.immovable.descr.type_name ~= "warehouse" or
+               send_whf.immovable.owner ~= send_plr or
+               not recv_whf.immovable or
+               recv_whf.immovable.descr.type_name ~= "warehouse" or
+               recv_whf.immovable.owner ~= recv_plr
+            then
+               send_to_all(smuggling_route_broken:bformat(
+                  (ngettext("%i point", "%i points", route_descr.value)):format(route_descr.value), recv_plr.name, send_plr.name)
+               )
+               run(wait_for_established_route, route_descr)
+            else
+               -- Only use ware types that both sending and receiving player can use
+               local wares = route_descr.wares
+
+               -- We start counting at 0 so that we can use the modulo (%) operator
+               -- for going round robin
+               local last_ware_index = 0;
+
+               -- Warp the next available ware, going round robin
+               local empty_warehouse_guard = #wares
+               local warp_index = last_ware_index
+               while empty_warehouse_guard > 0 do
+                  -- Index shift, because Lua tables start counting at 1
+                  local ware_to_warp = wares[warp_index + 1]
+                  if send_whf.immovable:get_wares(ware_to_warp) > 0 then
+                     print("NOCOM Route " .. idx .. " (" .. send_whf.x .. ", " .. send_whf.y .. ") warping ware " .. ware_to_warp ..  ": " .. send_plr.name .. " -> " .. recv_plr.name)
+                     send_whf.immovable:set_wares(ware_to_warp, send_whf.immovable:get_wares(ware_to_warp) - 1)
+                     recv_whf.immovable:set_wares(
+                        ware_to_warp, recv_whf.immovable:get_wares(ware_to_warp) + 1
+                     )
+                     points[recv_plr.team] = points[recv_plr.team] + route_descr.value
+                     break
+                  end
+                  warp_index = (warp_index + 1) % #wares;
+                  empty_warehouse_guard = empty_warehouse_guard - 1
+               end
+               -- Next round robin index
+               last_ware_index = (last_ware_index + 1) % #wares;
+            end
          end
       end
-      if #wn > 0 then
-         local ware_to_warp = wn[math.random(#wn)]
-         send_whf.immovable:set_wares(ware_to_warp, wares[ware_to_warp] - 1)
-         recv_whf.immovable:set_wares(
-            ware_to_warp, recv_whf.immovable:get_wares(ware_to_warp) + 1
-         )
-         points[recv_plr.team] = points[recv_plr.team] + route_descr.value
-      end
    end
 end
 
 function wait_for_established_route(route_descr)
    local receiving_wh, sending_wh
-   while 1 do
+   route_descr.sending_warehouse = nil
+   route_descr.receiving_warehouse = nil
+   route_descr.wares = {}
+
+   while true do
       receiving_wh = find_warehouse(route_descr.recv)
       sending_wh = find_warehouse(route_descr.send)
-      if receiving_wh and sending_wh and receiving_wh.owner.team == sending_wh.owner.team then break end
+      if receiving_wh and sending_wh and receiving_wh.owner.team == sending_wh.owner.team then
+         route_descr.sending_warehouse = sending_wh
+         route_descr.receiving_warehouse = receiving_wh
+
+         -- Collect ware types that both sending and receiving player can use
+         for idx,ware in pairs(sending_wh.owner.tribe.wares) do
+            if receiving_wh.owner.tribe:has_ware(ware.name) then
+               table.insert(route_descr.wares, ware.name)
+            end
+         end
+         break
+      end
       sleep(7138)
    end
 
@@ -104,6 +210,4 @@
    send_message(sending_wh.owner, _"Status",
       smuggling_route_established_sender:format(points), {popup=true, field=sending_wh.fields[1]}
    )
-
-   run(do_smuggling, route_descr, receiving_wh.owner, sending_wh.owner, receiving_wh.fields[1], sending_wh.fields[1])
 end

=== added file 'data/maps/MP_Scenarios/Smugglers.wmf/scripting/starting_conditions.lua'
--- data/maps/MP_Scenarios/Smugglers.wmf/scripting/starting_conditions.lua	1970-01-01 00:00:00 +0000
+++ data/maps/MP_Scenarios/Smugglers.wmf/scripting/starting_conditions.lua	2018-09-12 06:21:05 +0000
@@ -0,0 +1,254 @@
+-- Give the player a Barbarian headquarters
+function set_starting_conditions_barbarians(player, sf)
+   prefilled_buildings(player, { "barbarians_headquarters", sf.x, sf.y,
+      wares = {
+         ax = 5,
+         bread_paddle = 2,
+         blackwood = 32,
+         cloth = 5,
+         coal = 12,
+         felling_ax = 4,
+         fire_tongs = 2,
+         fish = 6,
+         fishing_rod = 2,
+         gold = 4,
+         grout = 12,
+         hammer = 12,
+         hunting_spear = 2,
+         iron = 12,
+         iron_ore = 5,
+         kitchen_tools = 4,
+         meal = 4,
+         meat = 6,
+         pick = 8,
+         barbarians_bread = 8,
+         ration = 12,
+         granite = 40,
+         scythe = 6,
+         shovel = 4,
+         snack = 3,
+         thatch_reed = 24,
+         log = 80,
+      },
+      workers = {
+         barbarians_blacksmith = 2,
+         barbarians_brewer = 1,
+         barbarians_builder = 10,
+         barbarians_charcoal_burner = 1,
+         barbarians_carrier = 40,
+         barbarians_gardener = 1,
+         barbarians_geologist = 4,
+         barbarians_lime_burner = 1,
+         barbarians_lumberjack = 3,
+         barbarians_miner = 4,
+         barbarians_miner_master = 4,
+         barbarians_ranger = 1,
+         barbarians_stonemason = 2,
+         barbarians_trainer = 3,
+         barbarians_ox = 5,
+      },
+      soldiers = {
+         [{0,0,0,0}] = 45,
+      }
+   })
+end
+
+-- Give the player an Empire headquarters
+function set_starting_conditions_empire(player, sf)
+   prefilled_buildings(player, { "empire_headquarters", sf.x, sf.y,
+      wares = {
+         armor_helmet = 4,
+         spear_wooden = 5,
+         felling_ax = 6,
+         bread_paddle = 2,
+         basket = 2,
+         empire_bread = 8,
+         cloth = 5,
+         coal = 12,
+         fire_tongs = 2,
+         fish = 6,
+         fishing_rod = 2,
+         flour = 4,
+         gold = 4,
+         grape = 4,
+         hammer = 14,
+         hunting_spear = 2,
+         iron = 12,
+         iron_ore = 5,
+         kitchen_tools = 4,
+         marble = 25,
+         marble_column = 6,
+         meal = 4,
+         meat = 6,
+         pick = 8,
+         ration = 12,
+         saw = 2,
+         scythe = 5,
+         shovel = 6,
+         granite = 40,
+         log = 30,
+         water = 12,
+         wheat = 4,
+         wine = 8,
+         planks = 45,
+         wool = 2,
+      },
+      workers = {
+         empire_armorsmith = 1,
+         empire_brewer = 1,
+         empire_builder = 10,
+         empire_carrier = 40,
+         empire_charcoal_burner = 1,
+         empire_geologist = 4,
+         empire_lumberjack = 3,
+         empire_miner = 4,
+         empire_stonemason = 2,
+         empire_toolsmith = 2,
+         empire_trainer = 3,
+         empire_weaponsmith = 1,
+         empire_donkey = 5,
+      },
+      soldiers = {
+         [{0,0,0,0}] = 45,
+      }
+   })
+end
+
+-- Give the player an Atlantean headquarters
+function set_starting_conditions_atlanteans(player, sf)
+   prefilled_buildings(player, { "atlanteans_headquarters", sf.x, sf.y,
+      wares = {
+         diamond = 7,
+         iron_ore = 5,
+         quartz = 9,
+         granite = 50,
+         spider_silk = 9,
+         log = 20,
+         coal = 12,
+         gold = 4,
+         gold_thread = 6,
+         iron = 8,
+         planks = 45,
+         spidercloth = 5,
+         blackroot = 5,
+         blackroot_flour = 12,
+         atlanteans_bread = 8,
+         corn = 5,
+         cornmeal = 12,
+         fish = 3,
+         meat = 3,
+         smoked_fish = 6,
+         smoked_meat = 6,
+         water = 12,
+         bread_paddle = 2,
+         buckets = 2,
+         fire_tongs = 2,
+         fishing_net = 4,
+         hammer = 11,
+         hunting_bow = 1,
+         milking_tongs = 2,
+         hook_pole = 2,
+         pick = 8,
+         saw = 9,
+         scythe = 4,
+         shovel = 9,
+         tabard = 5,
+         trident_light = 5,
+      },
+      workers = {
+         atlanteans_armorsmith = 1,
+         atlanteans_blackroot_farmer = 1,
+         atlanteans_builder = 10,
+         atlanteans_charcoal_burner = 1,
+         atlanteans_carrier = 40,
+         atlanteans_fishbreeder = 1,
+         atlanteans_geologist = 4,
+         atlanteans_miner = 4,
+         atlanteans_sawyer = 1,
+         atlanteans_stonecutter = 2,
+         atlanteans_toolsmith = 2,
+         atlanteans_trainer = 3,
+         atlanteans_weaponsmith = 1,
+         atlanteans_woodcutter = 3,
+         atlanteans_horse = 5,
+      },
+      soldiers = {
+         [{0,0,0,0}] = 35,
+      }
+   })
+end
+
+-- Give the player a Frisian headquarters
+function set_starting_conditions_frisians(player, sf)
+   prefilled_buildings(player, { "frisians_headquarters", sf.x, sf.y,
+      wares = {
+         log = 40,
+         granite = 50,
+         thatch_reed = 50,
+         brick = 80,
+         clay = 30,
+         water = 10,
+         fish = 10,
+         meat = 10,
+         fruit = 10,
+         barley = 5,
+         ration = 20,
+         honey = 10,
+         smoked_meat = 5,
+         smoked_fish = 5,
+         mead = 5,
+         meal = 2,
+         coal = 20,
+         iron = 5,
+         gold = 4,
+         iron_ore = 10,
+         bread_frisians = 15,
+         honey_bread = 5,
+         beer = 5,
+         cloth = 5,
+         fur = 10,
+         fur_garment = 5,
+         sword_short = 5,
+         hammer = 5,
+         fire_tongs = 2,
+         bread_paddle = 2,
+         kitchen_tools = 2,
+         felling_ax = 3,
+         needles = 2,
+         basket = 2,
+         pick = 5,
+         shovel = 5,
+         scythe = 3,
+         hunting_spear = 2,
+         fishing_net = 3,
+      },
+      workers = {
+         frisians_blacksmith = 3,
+         frisians_baker = 1,
+         frisians_brewer = 1,
+         frisians_builder = 10,
+         frisians_charcoal_burner = 1,
+         frisians_claydigger = 2,
+         frisians_brickmaker = 2,
+         frisians_carrier = 40,
+         frisians_reed_farmer = 2,
+         frisians_berry_farmer = 2,
+         frisians_fruit_collector = 2,
+         frisians_farmer = 1,
+         frisians_landlady = 1,
+         frisians_smoker = 1,
+         frisians_geologist = 4,
+         frisians_woodcutter = 3,
+         frisians_beekeeper = 1,
+         frisians_miner = 4,
+         frisians_miner_master = 2,
+         frisians_forester = 2,
+         frisians_stonemason = 2,
+         frisians_reindeer = 5,
+         frisians_trainer = 3,
+      },
+      soldiers = {
+         [{0,0,0,0}] = 45,
+      }
+   })
+end

=== modified file 'src/scripting/lua_game.cc'
--- src/scripting/lua_game.cc	2018-07-28 19:08:34 +0000
+++ src/scripting/lua_game.cc	2018-09-12 06:21:05 +0000
@@ -851,7 +851,7 @@
 /* RST
    .. method:: produced_wares_count(what)
 
-      Returns count of wares produced byt the player up to now.
+      Returns count of wares produced by the player up to now.
       'what' can be either an "all" or single ware name or an array of names. If single
       ware name is given, integer is returned, otherwise the table is returned.
 */