[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1842396-no-tribe/+merge/372281 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1842396-no-tribe. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands
Review: Approve Thanks for fixing :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1842396-no-tribe/+merge/372281 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1842396-no-tribe. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1842396-no-tribe/+merge/372281 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1842396-no-tribe. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands
Sounds got to me, the changes are online. Thanks for the reviews! -- https://code.launchpad.net/~widelands-dev/widelands/bug-1842396-no-tribe/+merge/372281 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1842396-no-tribe. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands
I think we should fix all of them. How about sticking them into a new function bool has_players_tribe() in game_settings.h? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1842396-no-tribe/+merge/372281 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1842396-no-tribe. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1842396-no-tribe/+merge/372281 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1842396-no-tribe. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands
Review: Approve Thanks for fixing :) Wouldn't it be better to use if(Widelands::tribe_exists(tribename)) rather than triggering the exception? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1842396-no-tribe/+merge/372281 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1842396-no-tribe. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands
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. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1842396-no-tribe/+merge/372281 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands
Notabilis has proposed merging lp:~widelands-dev/widelands/bug-1842396-no-tribe
into lp:widelands.
Commit message:
Fixing crash when no tribe is selected for a multiplayer client.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1842396 in widelands: "Random tribes crash multiplayer client"
https://bugs.launchpad.net/widelands/+bug/1842396
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1842396-no-tribe/+merge/372281
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.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1842396-no-tribe into lp:widelands.
=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc 2019-08-28 06:12:07 +
+++ src/network/gameclient.cc 2019-09-04 17:13:59 +
@@ -149,10 +149,14 @@
* 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 tipstext;
+ tipstext.push_back("general_game");
+ tipstext.push_back("multiplayer");
+ try {
+ tipstext.push_back(parent->get_players_tribe());
+ } catch (GameSettingsProvider::NoTribe) {
+ }
+ GameTips tips(*loader, tipstext);
modal = loader;
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
