Merge lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands

[Notabilis]

Fixing crash when no tribe is selected for a multiplayer client.

Fixing a crash with showing loading screen tips when no tribe is selected for a multiplayer client.

Seems to be a copy&paste or refactoring bug: When creating the tips as a single player or multiplayer host, the check for the thrown exception is done.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5387. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/580833741.
Appveyor build 5157. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1842396_no_tribe-5157.

[GunChleoc (gunchleoc)]

Thanks for fixing :)

Wouldn't it be better to use if(Widelands::tribe_exists(tribename)) rather than triggering the exception?

review: Approve

[Notabilis (notabilis27)]

We could use
  if (UserSettings::highest_playernum() >= settings.playernum)
which is the check used inside of the throwing get_players_tribe() function to decide whether to throw.
I decided to catch the exception since that is what is done in the other two cases the function is used, but of course we could change those calls as well. The current fix is a copy of those other calls.

[GunChleoc (gunchleoc)]

I think we should fix all of them. How about sticking them into a new function bool has_players_tribe() in game_settings.h?

[Notabilis (notabilis27)]

Sounds got to me, the changes are online.
Thanks for the reviews!

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5391. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/581403115.
Appveyor build 5161. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1842396_no_tribe-5161.

[GunChleoc (gunchleoc)]

Thanks for fixing :)

@bunnybot merge

review: Approve

=== modified file 'src/logic/game_settings.h'
--- src/logic/game_settings.h	2019-04-27 11:21:21 +0000
+++ src/logic/game_settings.h	2019-09-05 20:05:41 +0000
@@ -222,10 +222,13 @@
 	virtual void set_peaceful_mode(bool peace) = 0;
 	virtual bool is_peaceful_mode() = 0;
 
+	bool has_players_tribe() {
+		return UserSettings::highest_playernum() >= settings().playernum;
+	}
 	// For retrieving tips texts
 	struct NoTribe {};
 	const std::string& get_players_tribe() {
-		if (UserSettings::highest_playernum() < settings().playernum)
+		if (!has_players_tribe())
 			throw NoTribe();
 		return settings().players[settings().playernum].tribe;
 	}

=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc	2019-08-28 06:12:07 +0000
+++ src/network/gameclient.cc	2019-09-05 20:05:41 +0000
@@ -149,10 +149,13 @@
  * Show progress dialog and load map or saved game.
  */
 InteractiveGameBase* GameClientImpl::init_game(GameClient* parent, UI::ProgressWindow* loader) {
-
-	const std::string& tribename = parent->get_players_tribe();
-	assert(Widelands::tribe_exists(tribename));
-	GameTips tips(*loader, {"general_game", "multiplayer", tribename});
+	std::vector<std::string> tipstext;
+	tipstext.push_back("general_game");
+	tipstext.push_back("multiplayer");
+	if (parent->has_players_tribe()) {
+		tipstext.push_back(parent->get_players_tribe());
+	}
+	GameTips tips(*loader, tipstext);
 
 	modal = loader;
 

=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc	2019-08-28 06:12:07 +0000
+++ src/network/gamehost.cc	2019-09-05 20:05:41 +0000
@@ -643,9 +643,8 @@
 		std::vector<std::string> tipstext;
 		tipstext.push_back("general_game");
 		tipstext.push_back("multiplayer");
-		try {
+		if (d->hp.has_players_tribe()) {
 			tipstext.push_back(d->hp.get_players_tribe());
-		} catch (GameSettingsProvider::NoTribe) {
 		}
 		std::unique_ptr<GameTips> tips(new GameTips(*loader_ui, tipstext));
 

=== modified file 'src/wlapplication.cc'
--- src/wlapplication.cc	2019-08-28 06:12:07 +0000
+++ src/wlapplication.cc	2019-09-05 20:05:41 +0000
@@ -1339,9 +1339,8 @@
 			std::vector<std::string> tipstext;
 			tipstext.push_back("general_game");
 			tipstext.push_back("singleplayer");
-			try {
+			if (sp.has_players_tribe()) {
 				tipstext.push_back(sp.get_players_tribe());
-			} catch (GameSettingsProvider::NoTribe) {
 			}
 			GameTips tips(loader_ui, tipstext);