Merge lp:~widelands-dev/widelands/safeguard-multiplayer-messages into lp:widelands

[Toni Förster]

Safeguard to avoid desyncs when using send_message() from ui.lua

This should make sure that we don't have desyncs when in multiplayer and sending messages. Could potentially solve some desyncs in smugglers multiplayer campaign.

[GunChleoc (gunchleoc)]

Code LGTM :)

review: Approve

[Toni Förster (stonerl)]

Thanks.

And BTW: Welcome back GunChleoc

@bunnybot merge

[kaputtnik (franku)]

What is the reason for the change in win_condition_functions.lua?

wait_for_roadbuilding() acts now different when in multiplayer: The sentence "Sleeps while player is in roadbuilding mode." is now only valid in singleplayer games. How should this be documented?

I fear we get things messsed up if we have such constructions, at least for people doing scripting it get much more difficult.

Just my opinion :-)

[GunChleoc (gunchleoc)]

Add the info to the documentation, like for the send_message function which now states that it isn't multiplayer-safe.

[kaputtnik (franku)]

Sorry if my sentences sounds too hard...

> Add the info to the documentation, like for the send_message function which now states that it isn't multiplayer-safe.

This statement is not valid anymore, if this branch get merged. Or do i miss some thing?

Tony, this is nothing against your merge request, nor against you. But this branch solves an issue which is not really there. Or do i miss anything again? IMHO this should be targeted to build21.

Once we had a feature freeze, but now we get far away from that :(

[Toni Förster (stonerl)]

@kaputtnik

Neither do your sentences sound to hard nor rude. :-) Please let me explain why I think the changes are necessary for build_20.

send_message() from messages.lua is also used in mp_scenarios and some other win_conditions. We could either change the method calls there as well and change them back for build_21, or we fix it now.

We have a hodgepodge methods we use for sending messages ATM. It would be sensible to unify them, and use the same method for the same kind of message.

We shouldn't forget that the win_conditions are also used in single player and there it is absolutely okay to wait for a message to pop-up. The changes we did in the other branch were a hot-fix, specifically for multiplayer. But being able to distinguish between single- and multiplayer, and making this method multiplayer-aware is the way to go IMHO.

I don't think that this change interferes with the feature freeze. We could have searched through all the lua files and locate where the aforementioned method is used (there are still many left) and replace it with something like this: "player:send_message(HEADER, BODY)". But after the release of build_20, we then would have to revert all back to use the method from messages.lua.

@GunChleoc & @kaputtnik

I changed the documentation as well.

The changes in win_condition_functions just restore the old behaviour; using the method from messages.lua.

I hope my explanations sound sensible.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4444. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/491032831.
Appveyor build 4232. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_safeguard_multiplayer_messages-4232.

[GunChleoc (gunchleoc)]

This has a chance of fixing some desyncs for us, which is why I want it for Build 20. We do need to get rid of those desyncs.

@bunnybot merge

=== modified file 'data/scripting/messages.lua'
--- data/scripting/messages.lua	2019-01-27 10:10:29 +0000
+++ data/scripting/messages.lua	2019-02-09 18:50:14 +0000
@@ -15,11 +15,7 @@
 --    Sends a message to the player.
 --    If the popup parameter is true and the player is in building mode,
 --    the function waits until the player leaves the building mode
---    before sending the message
---
---    Do not use this with 'popup = true' for multiplayer scenarios or
---    winconditions, because a game will likely desync then.
---    Use :meth:`wl.game.Player.send_message <wl.game.Player.send_message>` instead.
+--    before sending the message (only in singleplayer)
 --
 --    :arg player: the recipient of the message
 --    :arg title: the localized title of the message
@@ -43,10 +39,6 @@
 --
 --    Sends a game status message to all players.
 --
---    Do not use this for multiplayer scenarios or winconditions, because a
---    game will likely desync then. Use :meth:`win_condition_functions.broadcast <broadcast>`
---    instead.
---
 --    :arg text: the localized body of the message. You can use rt functions here.
 --    :type text: :class:`string`
 --    :arg heading: the localized title of the message (optional)
 
=== modified file 'data/scripting/ui.lua'
--- data/scripting/ui.lua	2019-01-27 18:57:30 +0000
+++ data/scripting/ui.lua	2019-02-09 18:50:14 +0000
@@ -181,8 +181,10 @@
 --    Sleeps while player is in roadbuilding mode.
 --
 function wait_for_roadbuilding()
-   _await_animation()
-   while (wl.ui.MapView().is_building_road) do sleep(2000) end
+   if wl.Game().type == "singleplayer" then
+      _await_animation()
+      while (wl.ui.MapView().is_building_road) do sleep(2000) end
+   end
 end
 
 
 
=== modified file 'data/scripting/win_conditions/collectors.lua'
--- data/scripting/win_conditions/collectors.lua	2019-02-09 07:51:55 +0000
+++ data/scripting/win_conditions/collectors.lua	2019-02-09 18:50:14 +0000
@@ -194,10 +194,10 @@
          local lost_or_won = 0
          if (info[2] < win_points) then
             lost_or_won = 0
-            player:send_message(lost_game_over.title, lost_game_over.body)
+            send_message(player, lost_game_over.title, lost_game_over.body)
          else
             lost_or_won = 1
-            player:send_message(won_game_over.title, won_game_over.body)
+            send_message(player, won_game_over.title, won_game_over.body)
          end
          if (count_factions(plrs) > 1) then
             if (player.team == 0) then
 
=== modified file 'data/scripting/win_conditions/win_condition_functions.lua'
--- data/scripting/win_conditions/win_condition_functions.lua	2019-02-09 07:51:55 +0000
+++ data/scripting/win_conditions/win_condition_functions.lua	2019-02-09 18:50:14 +0000
@@ -108,7 +108,7 @@
 function broadcast(plrs, header, msg, goptions)
    local options = goptions or {}
    for idx, p in ipairs(plrs) do
-      p:send_message(header, msg, options)
+      send_message(p, header, msg, options)
    end
 end