Merge lp:~widelands-dev/widelands/bug-1638280-coroutine-messages into lp:widelands

[GunChleoc]

Do not broadcast Lua coroutine error messages to empty player slots. Also, use richtext_escape on them.

To reproduce the crash, add an unused ware to a prefilled building in a starting condition and close a player slot.

With the changes in this branch, players should get a coroutine error message instead of a crash.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3628. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/400902520.
Appveyor build 3427. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1638280_coroutine_messages-3427.

[Notabilis (notabilis27)]

Worked fine when testing, thanks.

Two comments regarding the code:
1) Maybe move richtext_escape() in front of the loop?
2) Maybe only create msg if we already know that the player is in use, e.g., move if(!recipient)continue; to the top of the loop.

review: Approve

[GunChleoc (gunchleoc)]

I have implemented both suggestions, thanks!

@bunnybot merge

[Notabilis (notabilis27)]

Uh... from the diff it seems as if error_message is inside the for-loop. Is this intentional?

[GunChleoc (gunchleoc)]

Duh. Thanks for having my back.

[Notabilis (notabilis27)]

You're welcome, looking better now. :)

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3649. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/401502950.
Appveyor build 3448. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1638280_coroutine_messages-3448.

=== modified file 'src/logic/cmd_luacoroutine.cc'
--- src/logic/cmd_luacoroutine.cc	2018-05-05 17:10:37 +0000
+++ src/logic/cmd_luacoroutine.cc	2018-07-08 18:38:45 +0000
@@ -23,6 +23,7 @@
 
 #include "base/log.h"
 #include "base/macros.h"
+#include "graphic/text_layout.h"
 #include "io/fileread.h"
 #include "io/filewrite.h"
 #include "logic/game.h"
@@ -50,11 +51,17 @@
 		log("Error in Lua Coroutine\n");
 		log("%s\n", e.what());
 		log("Send message to all players and pause game\n");
+		const std::string error_message = richtext_escape(e.what());
 		for (int i = 1; i <= game.map().get_nrplayers(); i++) {
-			std::unique_ptr<Message> msg(new Widelands::Message(
-			   Message::Type::kGameLogic, game.get_gametime(), "Coroutine",
-			   "images/ui_basic/menu_help.png", "Lua Coroutine Failed", e.what()));
-			game.get_player(i)->add_message(game, std::move(msg), true);
+			// Send message only to open player slots
+			Player* recipient = game.get_player(i);
+			if (recipient) {
+				std::unique_ptr<Message> msg(new Widelands::Message(
+				   Message::Type::kGameLogic, game.get_gametime(), "Coroutine",
+				   "images/ui_basic/menu_help.png", "Lua Coroutine Failed", error_message));
+
+				recipient->add_message(game, std::move(msg), true);
+			}
 		}
 		game.game_controller()->set_desired_speed(0);
 	}