Merge lp:~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine into lp:widelands

[GunChleoc]

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.

[GunChleoc (gunchleoc)]

For testing, hack the player_names packet and change the tribes there. You can also replace some players with empty AI if you don't want to run 4 instances of Widelands.

[GunChleoc (gunchleoc)]

There is also some NOCOM log output for testing that we need to delete before merging.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3948. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/427560056.
Appveyor build 3746. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1751440_smugglers_desync_single_coroutine-3746.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3974. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/428601743.
Appveyor build 3772. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1751440_smugglers_desync_single_coroutine-3772.

[Klaus Halfmann (klaus-halfmann)]

OK after cleaning my confusion about the two builds I will try a 2xWIN/ 2xOSX Smugglers game.

[Klaus Halfmann (klaus-halfmann)]

When trying to Debug WIndows I gita Problem with system.out, as described in: #1398536

I will try to debug this anyway ....

[Klaus Halfmann (klaus-halfmann)]

OK after cleaning my confusion about the two builds I will try a 2xWIN/ 2xOSX Smugglers game.
... some hours later ...
Did so and it was quite ok until I ran into #1794965 I think this is unrelated. But I may not be
able to continue testing. I will try other variants, later

[Klaus Halfmann (klaus-halfmann)]

Played this on OSX/localhost only, stating on the last save, no Issues.

Gun: perhpas we need som real world unreliable network to do more test?

I will try to come online this weekend perhpas we can do some smuggling?

[Klaus Halfmann (klaus-halfmann)]

Played this with kapputnik yesterday for about an hour or such.
We both finally got the Idea about the Map and how smuggling works :-)

We had no desnycs whatsover so this can go in.

Gun: Do you want to open Smugglers for all tribes in R20 or defer this?

@bunnybot merge

review: Approve (compile, review, testplay (osx, win10, linux))

[GunChleoc (gunchleoc)]

We need to defer opening this up for all tribes, because that needs further changes in the game setup logic.

Thanks for testing!

@bunnybot merge

[GunChleoc (gunchleoc)]

Bug filed: https://bugs.launchpad.net/widelands/+bug/1795112

=== 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-26 16:22:20 +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-26 16:22:20 +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-26 16:22:20 +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-26 16:22:20 +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.
 */