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
