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

Proposed by Notabilis
Status: Merged
Merged at revision: 9197
Proposed branch: lp:~widelands-dev/widelands/bug-1842396-no-tribe
Merge into: lp:widelands
Diff against target: 69 lines (+13/-9)
4 files modified
src/logic/game_settings.h (+4/-1)
src/network/gameclient.cc (+7/-4)
src/network/gamehost.cc (+1/-2)
src/wlapplication.cc (+1/-2)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1842396-no-tribe
Reviewer Review Type Date Requested Status
GunChleoc Approve
Review via email: mp+372281@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

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.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks for fixing :)

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

review: Approve
Revision history for this message
Notabilis (notabilis27) wrote :

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.

Revision history for this message
GunChleoc (gunchleoc) wrote :

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

9195. By Notabilis

Changing throw...catch code to a separate function call.

Revision history for this message
Notabilis (notabilis27) wrote :

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

Revision history for this message
bunnybot (widelandsofficial) wrote :

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.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks for fixing :)

@bunnybot merge

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/logic/game_settings.h'
2--- src/logic/game_settings.h 2019-04-27 11:21:21 +0000
3+++ src/logic/game_settings.h 2019-09-05 20:05:41 +0000
4@@ -222,10 +222,13 @@
5 virtual void set_peaceful_mode(bool peace) = 0;
6 virtual bool is_peaceful_mode() = 0;
7
8+ bool has_players_tribe() {
9+ return UserSettings::highest_playernum() >= settings().playernum;
10+ }
11 // For retrieving tips texts
12 struct NoTribe {};
13 const std::string& get_players_tribe() {
14- if (UserSettings::highest_playernum() < settings().playernum)
15+ if (!has_players_tribe())
16 throw NoTribe();
17 return settings().players[settings().playernum].tribe;
18 }
19
20=== modified file 'src/network/gameclient.cc'
21--- src/network/gameclient.cc 2019-08-28 06:12:07 +0000
22+++ src/network/gameclient.cc 2019-09-05 20:05:41 +0000
23@@ -149,10 +149,13 @@
24 * Show progress dialog and load map or saved game.
25 */
26 InteractiveGameBase* GameClientImpl::init_game(GameClient* parent, UI::ProgressWindow* loader) {
27-
28- const std::string& tribename = parent->get_players_tribe();
29- assert(Widelands::tribe_exists(tribename));
30- GameTips tips(*loader, {"general_game", "multiplayer", tribename});
31+ std::vector<std::string> tipstext;
32+ tipstext.push_back("general_game");
33+ tipstext.push_back("multiplayer");
34+ if (parent->has_players_tribe()) {
35+ tipstext.push_back(parent->get_players_tribe());
36+ }
37+ GameTips tips(*loader, tipstext);
38
39 modal = loader;
40
41
42=== modified file 'src/network/gamehost.cc'
43--- src/network/gamehost.cc 2019-08-28 06:12:07 +0000
44+++ src/network/gamehost.cc 2019-09-05 20:05:41 +0000
45@@ -643,9 +643,8 @@
46 std::vector<std::string> tipstext;
47 tipstext.push_back("general_game");
48 tipstext.push_back("multiplayer");
49- try {
50+ if (d->hp.has_players_tribe()) {
51 tipstext.push_back(d->hp.get_players_tribe());
52- } catch (GameSettingsProvider::NoTribe) {
53 }
54 std::unique_ptr<GameTips> tips(new GameTips(*loader_ui, tipstext));
55
56
57=== modified file 'src/wlapplication.cc'
58--- src/wlapplication.cc 2019-08-28 06:12:07 +0000
59+++ src/wlapplication.cc 2019-09-05 20:05:41 +0000
60@@ -1339,9 +1339,8 @@
61 std::vector<std::string> tipstext;
62 tipstext.push_back("general_game");
63 tipstext.push_back("singleplayer");
64- try {
65+ if (sp.has_players_tribe()) {
66 tipstext.push_back(sp.get_players_tribe());
67- } catch (GameSettingsProvider::NoTribe) {
68 }
69 GameTips tips(loader_ui, tipstext);
70

Subscribers

People subscribed via source and target branches

to status/vote changes: