Review: Needs Fixing I suggested some improvements for the i18n. There is also some code duplication that we should get rid of.
Diff comments: > === modified file 'src/network/gamehost.cc' > --- src/network/gamehost.cc 2018-04-07 16:59:00 +0000 > +++ src/network/gamehost.cc 2018-09-12 05:38:27 +0000 > @@ -560,6 +561,25 @@ > ->instantiate(*d->game, p)); > } > > +void GameHost::start_ai_for(uint8_t playernumber, const std::string& ai) { Call this function "replace_client_with_ai"? > + assert(d->game->get_player(playernumber + 1)->get_ai().empty()); > + assert(d->game->get_player(playernumber + 1)->get_ai() > + == d->settings.players.at(playernumber).ai); > + // Inform all players about the change > + // Has to be done at first in this method since the calls later on > overwrite players[].name > + send_system_message_code( > + "CLIENT_X_REPLACED_WITH", > + d->settings.players.at(playernumber).name, > + ComputerPlayer::get_implementation(ai)->descname); > + set_player_ai(playernumber, ai, false); > + d->game->get_player(playernumber + 1)->set_ai(ai); > + // Activate the ai > + init_computer_player(playernumber + 1); > + set_player_state(playernumber, PlayerSettings::State::kComputer); > + assert(d->game->get_player(playernumber + 1)->get_ai() > + == d->settings.players.at(playernumber).ai); > +} > + > void GameHost::init_computer_players() { > const Widelands::PlayerNumber nr_players = > d->game->map().get_nrplayers(); > iterate_players_existing_novar(p, nr_players, *d->game) { > > === added file 'src/wui/game_client_disconnected.cc' > --- src/wui/game_client_disconnected.cc 1970-01-01 00:00:00 +0000 > +++ src/wui/game_client_disconnected.cc 2018-09-12 05:38:27 +0000 > @@ -0,0 +1,197 @@ > +/* > + * Copyright (C) 2002-2018 by the Widelands Development Team > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > 02110-1301, USA. > + * > + */ > + > +#include "wui/game_client_disconnected.h" > + > +#include <boost/bind.hpp> > +#include <boost/format.hpp> > + > +#include "ai/computer_player.h" > +#include "ai/defaultai.h" > +#include "base/i18n.h" > +#include "network/gamehost.h" > +#include "ui_basic/messagebox.h" > +#include "wui/interactive_gamebase.h" > + > +namespace { > + > +const int32_t width = 256; Make these constexpr > +const int32_t margin = 10; > +const int32_t vspacing = 5; > +const uint32_t vgap = 3; > + > +/** > + * Small helper class for a "Really exit game?" message box. > + */ > +class GameClientDisconnectedExitConfirmBox : public UI::WLMessageBox { > +public: > + // TODO(GunChleoc): Arabic: Buttons need more height for Arabic > + GameClientDisconnectedExitConfirmBox(GameClientDisconnected* gcd, > InteractiveGameBase* gb) > + : UI::WLMessageBox(gcd->get_parent(), > + /** TRANSLATORS: Window label when "Exit game" > has been pressed */ > + _("Exit Game Confirmation"), > + _("Are you sure you wish to exit this game?"), > + MBoxType::kOkCancel), > + igb_(gb), gcd_(gcd) { > + } > + > + void clicked_ok() override { > + > igb_->end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack); > + } > + > + void clicked_back() override { > + cancel(); > + die(); > + // Make parent window visible again so player can use another > option > + gcd_->set_visible(true); > + } > + > +private: > + InteractiveGameBase* igb_; > + GameClientDisconnected* gcd_; If we tie into the cancel() signal from GameClientDisconnected, we can get rid of the gcd_ variable and of the code duplication with GameOptionsMenuExitConfirmBox and have 1 common GameExitConfirmBox for both. Also, the UniqueWindow class has a create and destroy hook that you could work with. > +}; > + > +} // end anonymous namespace > + > +GameClientDisconnected::GameClientDisconnected(InteractiveGameBase* gb, > + UI::UniqueWindow::Registry& registry, > + GameHost* host) > + : UI::UniqueWindow(gb, "client_disconnected", ®istry, 2 * margin + > width, 0, > + /** TRANSLATORS: Window label */ > + _("Client got disconnected")), > + igb_(gb), > + host_(host), > + box_(this, margin, margin, UI::Box::Vertical, width, get_h() - > 2 * margin, vspacing), > + box_h_(&box_, margin, margin, UI::Box::Horizontal, width, 35, > vspacing), > + text_(&box_, > + 0, > + 0, > + width, > + 10, // automatic height > + UI::PanelStyle::kWui, > + _("A player disconnected from the game. An automatic > save game has been created. " > + "Do you want to continue playing?"), > + UI::Align::kLeft, > + UI::MultilineTextarea::ScrollMode::kNoScrolling), > + continue_(&box_h_, > + "continue_game", > + 0, > + 0, > + width - 35 - vspacing, > + 35, > + UI::ButtonStyle::kWuiMenu, > + /** TRANSLATORS: Button text */ > + _("Continue game"), > + /** TRANSLATORS: Button tooltip */ > + _("Continue game with...")), Replace ... with https://www.fileformat.info/info/unicode/char/2026/index.htm Also, what is the knowledge gain from this tooltip for the user? > + type_dropdown_(&box_h_, > + width - 50, // x > + 0, // y > + 60, // width of selection box > + 800, // height of selection box, shrinks automatically > + 35, // width/height of button > + /** TRANSLATORS: Dropdown tooltip to select the AI > difficulty, > + entries have the form "Replace with Normal AI" > */ > + _("Continue game and"), Incomplete sentences are always a huge challenge for translators. Is it possible to find something better? > + UI::DropdownType::kPictorial, > + UI::PanelStyle::kWui), > + exit_game_(&box_, > + "exit_game", > + 0, > + 0, > + width, > + 35, > + UI::ButtonStyle::kWuiMenu, > + > g_gr->images().get("images/wui/menus/menu_exit_game.png"), > + /** TRANSLATORS: Button tooltip */ > + _("Exit Game")) { > + > + box_.add(&text_); > + box_.add_space(vgap); > + box_h_.add(&continue_); > + box_h_.add(&type_dropdown_); > + box_.add(&box_h_); > + box_.add(&exit_game_); > + box_.set_size(width, text_.get_h() + vgap + box_h_.get_h() + > exit_game_.get_h() + 3 * vspacing); > + set_inner_size(get_inner_w(), box_.get_h() + 2 * margin); > + > + continue_.sigclicked.connect( > + boost::bind(&GameClientDisconnected::clicked_continue, > boost::ref(*this))); > + exit_game_.sigclicked.connect( > + boost::bind(&GameClientDisconnected::clicked_exit_game, > boost::ref(*this))); > + > + // Add all AI types > + for (const auto* impl : ComputerPlayer::get_implementations()) { > + type_dropdown_.add((boost::format(_("Replace with %s")) % > impl->descname).str(), Please add a translator's comment so that they will know what is being replaced with what here. > + impl->name.c_str(), > + > g_gr->images().get(impl->icon_filename), false, > + > (boost::format(_("Replace with %s")) % impl->descname).str()); > + } > + > + // Set default mode to normal AI > + type_dropdown_.select(DefaultAI::normal_impl.name.c_str()); > + > + if (get_usedefaultpos()) Might as well add some {} :) > + center_to_parent(); > +} > + > +void GameClientDisconnected::die() { > + > + if (host_->forced_pause()) { > + host_->end_forced_pause(); > + } > + if (is_visible()) { > + // Dialog aborted, default to the old behavior and add a normal > AI > + set_ai(DefaultAI::normal_impl.name); > + } > + UI::UniqueWindow::die(); > +} > + > +void GameClientDisconnected::clicked_continue() { > + assert(type_dropdown_.has_selection()); > + > + const std::string selection = type_dropdown_.get_selected(); > + assert(selection != ""); > + > + set_ai(selection); > + // Visibility works as a hint that the window was closed by a button > click > + set_visible(false); > + > + die(); > +} > + > +void GameClientDisconnected::clicked_exit_game() { > + if (SDL_GetModState() & KMOD_CTRL) { > + > igb_->end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack); > + } else { > + new GameClientDisconnectedExitConfirmBox(this, igb_); > + set_visible(false); > + } > +} > + > +void GameClientDisconnected::set_ai(const std::string& ai) { > + const std::string ai_descr = > ComputerPlayer::get_implementation(ai)->descname; > + for (size_t i = 0; i < host_->settings().players.size(); i++) { > + if (host_->settings().players.at(i).state != > PlayerSettings::State::kOpen > + || !igb_->game().get_player(i + > 1)->get_ai().empty()) { > + continue; > + } > + host_->start_ai_for(i, ai); > + } > +} -- https://code.launchpad.net/~widelands-dev/widelands/bug-1434875-net-client-disconnect/+merge/354531 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1434875-net-client-disconnect. _______________________________________________ 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