← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/bug-1358880-ship-statistics into lp:widelands

 

Thanks for the review! Everything that I haven't added a comment to, I will implement exactly as you suggested.

As to the name - I guess that's for consistency, just like we have the "Building Statistics", which is also a mix of info and unit access. I am open to suggestions though :)

Diff comments:

> 
> === modified file 'src/logic/map_objects/tribes/ship.cc'
> --- src/logic/map_objects/tribes/ship.cc	2018-04-07 16:59:00 +0000
> +++ src/logic/map_objects/tribes/ship.cc	2018-04-16 15:39:30 +0000
> @@ -847,14 +849,14 @@
>  	             pgettext("ship", "Expedition"), _("Expedition Ready"),
>  	             _("An expedition ship is waiting for your commands."),
>  	             "images/wui/buildings/start_expedition.png");
> -	Notifications::publish(NoteShipMessage(this, NoteShipMessage::Message::kWaitingForCommand));
> +	Notifications::publish(NoteShip(this, NoteShip::Action::kWaitingForCommand));

I think there's a reason that I set the state at the top of the function and notify on the bottom. I implemented this a ear ago, so I don't remember the particulars. I dimly remember that there was a problem somewhere that made me split this in some instances. I have taken a note though and will experiment and ad a comment if the separation is indeed needed here.

>  }
>  
>  /// Initializes / changes the direction of scouting to @arg direction
>  /// @note only called via player command
>  void Ship::exp_scouting_direction(Game&, WalkingDir scouting_direction) {
>  	assert(expedition_);
> -	ship_state_ = ShipStates::kExpeditionScouting;
> +	set_ship_state_and_notify(ShipStates::kExpeditionScouting, NoteShip::Action::kDestinationChanged);
>  	expedition_->scouting_direction = scouting_direction;
>  	expedition_->island_exploration = false;
>  }
> 
> === modified file 'src/logic/player.cc'
> --- src/logic/player.cc	2018-04-07 16:59:00 +0000
> +++ src/logic/player.cc	2018-04-16 15:39:30 +0000
> @@ -372,6 +373,18 @@
>  	game->cmdqueue().enqueue(new CmdDeleteMessage(game->get_gametime(), player_number_, message_id));
>  }
>  
> +const std::set<Serial>& Player::ships() const {
> +	return ships_;
> +}
> +void Player::add_ship(Serial ship) {
> +	ships_.insert(ship);
> +}
> +void Player::remove_ship(Serial ship) {
> +	if (ships_.count(ship) == 1) {
> +		ships_.erase(ship);
> +	}

It would, the complexity of find() is logarithmic. So, it does matter :)

> +}
> +
>  /*
>  ===============
>  Return filtered buildcaps that take the player's territory into account.
> 
> === modified file 'src/scripting/lua_game.cc'
> --- src/scripting/lua_game.cc	2018-04-07 16:59:00 +0000
> +++ src/scripting/lua_game.cc	2018-04-16 15:39:30 +0000
> @@ -672,30 +672,16 @@
>  */
>  int LuaPlayer::get_ships(lua_State* L) {
>  	EditorGameBase& egbase = get_egbase(L);
> -	const Map& map = egbase.map();
>  	PlayerNumber p = (get(L, egbase)).player_number();
>  	lua_newtable(L);
>  	uint32_t cidx = 1;
> -
> -	std::set<OPtr<Ship>> found_ships;
> -	for (int16_t y = 0; y < map.get_height(); ++y) {
> -		for (int16_t x = 0; x < map.get_width(); ++x) {

ROFL yep. Spending a bit of memory to keep track of them is certainly a lot easier to deal with!

> -			FCoords f = map.get_fcoords(Coords(x, y));
> -			// there are too many bobs on the map so we investigate
> -			// only bobs on water
> -			if (f.field->nodecaps() & MOVECAPS_SWIM) {
> -				for (Bob* bob = f.field->get_first_bob(); bob; bob = bob->get_next_on_field()) {
> -					if (upcast(Ship, ship, bob)) {
> -						if (ship->get_owner()->player_number() == p && !found_ships.count(ship)) {
> -							found_ships.insert(ship);
> -							lua_pushuint32(L, cidx++);
> -							LuaMaps::upcasted_map_object_to_lua(L, ship);
> -							lua_rawset(L, -3);
> -						}
> -					}
> -				}
> -			}
> -		}
> +	for (const auto& serial : egbase.player(p).ships()) {
> +		Widelands::MapObject* obj = egbase.objects().get_object(serial);
> +		assert(obj->descr().type() == Widelands::MapObjectType::SHIP);
> +		upcast(Widelands::Ship, ship, obj);
> +		lua_pushuint32(L, cidx++);
> +		LuaMaps::upcasted_map_object_to_lua(L, ship);
> +		lua_rawset(L, -3);
>  	}
>  	return 1;
>  }
> 
> === modified file 'src/wui/interactive_player.cc'
> --- src/wui/interactive_player.cc	2018-04-07 16:59:00 +0000
> +++ src/wui/interactive_player.cc	2018-04-16 15:39:30 +0000
> @@ -459,6 +460,14 @@
>  			}
>  			return true;
>  
> +		case SDLK_e:

Good catch! We now have a cheap function to detect that too :)

> +			if (main_windows_.seafaring_stats.window == nullptr) {
> +				new SeafaringStatisticsMenu(*this, main_windows_.seafaring_stats);
> +			} else {
> +				main_windows_.seafaring_stats.toggle();
> +			}
> +			return true;
> +
>  		case SDLK_s:
>  			if (code.mod & (KMOD_LCTRL | KMOD_RCTRL))
>  				new GameMainMenuSaveGame(*this, main_windows_.savegame);
> 
> === added file 'src/wui/seafaring_statistics_menu.cc'
> --- src/wui/seafaring_statistics_menu.cc	1970-01-01 00:00:00 +0000
> +++ src/wui/seafaring_statistics_menu.cc	2018-04-16 15:39:30 +0000
> @@ -0,0 +1,568 @@
> +/*
> + * Copyright (C) 2017-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/seafaring_statistics_menu.h"
> +
> +#include <memory>
> +
> +#include <boost/bind.hpp>
> +#include <boost/format.hpp>
> +
> +#include "economy/fleet.h"
> +#include "graphic/graphic.h"
> +#include "logic/game.h"
> +#include "logic/player.h"
> +#include "logic/playercommand.h"
> +#include "ui_basic/box.h"
> +#include "wui/interactive_player.h"
> +#include "wui/shipwindow.h"
> +#include "wui/watchwindow.h"
> +
> +inline InteractivePlayer& SeafaringStatisticsMenu::iplayer() const {
> +	return dynamic_cast<InteractivePlayer&>(*get_parent());
> +}
> +
> +constexpr int kPadding = 5;
> +constexpr int kButtonSize = 34;
> +
> +SeafaringStatisticsMenu::SeafaringStatisticsMenu(InteractivePlayer& plr,
> +                                                 UI::UniqueWindow::Registry& registry)
> +   : UI::UniqueWindow(&plr, "seafaring_statistics", &registry, 355, 375, _("Seafaring Statistics")),
> +     main_box_(this, kPadding, kPadding, UI::Box::Vertical, get_inner_w(), get_inner_h(), kPadding),
> +     filter_box_(
> +        &main_box_, 0, 0, UI::Box::Horizontal, get_inner_w() - 2 * kPadding, kButtonSize, kPadding),
> +     idle_btn_(&filter_box_,
> +               "filter_ship_idle",
> +               0,
> +               0,
> +               kButtonSize,
> +               kButtonSize,
> +               g_gr->images().get("images/ui_basic/but0.png"),
> +               status_to_image(ShipFilterStatus::kIdle)),
> +     waiting_btn_(&filter_box_,
> +                  "filter_ship_waiting",
> +                  0,
> +                  0,
> +                  kButtonSize,
> +                  kButtonSize,
> +                  g_gr->images().get("images/ui_basic/but0.png"),
> +                  status_to_image(ShipFilterStatus::kExpeditionWaiting)),
> +     scouting_btn_(&filter_box_,
> +                   "filter_ship_scouting",
> +                   0,
> +                   0,
> +                   kButtonSize,
> +                   kButtonSize,
> +                   g_gr->images().get("images/ui_basic/but0.png"),
> +                   status_to_image(ShipFilterStatus::kExpeditionScouting)),
> +     portspace_btn_(&filter_box_,
> +                    "filter_ship_portspace",
> +                    0,
> +                    0,
> +                    kButtonSize,
> +                    kButtonSize,
> +                    g_gr->images().get("images/ui_basic/but0.png"),
> +                    status_to_image(ShipFilterStatus::kExpeditionPortspaceFound)),
> +     shipping_btn_(&filter_box_,
> +                   "filter_ship_transporting",
> +                   0,
> +                   0,
> +                   kButtonSize,
> +                   kButtonSize,
> +                   g_gr->images().get("images/ui_basic/but0.png"),
> +                   status_to_image(ShipFilterStatus::kShipping)),
> +     ship_filter_(ShipFilterStatus::kAll),
> +     navigation_box_(
> +        &main_box_, 0, 0, UI::Box::Horizontal, get_inner_w() - 2 * kPadding, kButtonSize, kPadding),
> +     watchbtn_(&navigation_box_,
> +               "seafaring_stats_watch_button",
> +               0,
> +               0,
> +               kButtonSize,
> +               kButtonSize,
> +               g_gr->images().get("images/ui_basic/but2.png"),
> +               g_gr->images().get("images/wui/menus/menu_watch_follow.png"),
> +               (boost::format(_("%1% (Hotkey: %2%)")) %
> +                /** TRANSLATORS: Tooltip in the seafaring statistics window */
> +                _("Watch the selected ship") %
> +                pgettext("hotkey", "W"))
> +                  .str()),
> +     openwindowbtn_(&navigation_box_,
> +                    "seafaring_stats_watch_button",
> +                    0,
> +                    0,
> +                    kButtonSize,
> +                    kButtonSize,
> +                    g_gr->images().get("images/ui_basic/but2.png"),
> +                    g_gr->images().get("images/ui_basic/fsel.png"),
> +                    (boost::format(_("%1% (Hotkey: %2%)")) %
> +                     /** TRANSLATORS: Tooltip in the seafaring statistics window */
> +                     _("Go to the selected ship and open its window") %
> +                     pgettext("hotkey", "O"))
> +                       .str()),
> +     centerviewbtn_(&navigation_box_,
> +                    "seafaring_stats_center_main_mapview_button",
> +                    0,
> +                    0,
> +                    kButtonSize,
> +                    kButtonSize,
> +                    g_gr->images().get("images/ui_basic/but2.png"),
> +                    g_gr->images().get("images/wui/ship/menu_ship_goto.png"),
> +                    (boost::format(_("%1% (Hotkey: %2%)")) %
> +                     /** TRANSLATORS: Tooltip in the seafaring statistics window */
> +                     _("Center the map on the selected ship") %
> +                     pgettext("hotkey", "G"))
> +                       .str()),
> +     table_(&main_box_,
> +            0,
> +            0,
> +            get_inner_w() - 2 * kPadding,
> +            100,
> +            g_gr->images().get("images/ui_basic/but1.png")) {
> +
> +	const Widelands::TribeDescr& tribe = iplayer().player().tribe();
> +	colony_icon_ = tribe.get_worker_descr(tribe.builder())->icon();
> +
> +	// Buttons for ship states
> +	main_box_.add(&filter_box_, UI::Box::Resizing::kFullSize);
> +	filter_box_.add(&idle_btn_);
> +	filter_box_.add(&shipping_btn_);
> +	filter_box_.add(&waiting_btn_);
> +	filter_box_.add(&scouting_btn_);
> +	filter_box_.add(&portspace_btn_);
> +
> +	main_box_.add(&table_, UI::Box::Resizing::kExpandBoth);
> +
> +	// Navigation buttons
> +	main_box_.add(&navigation_box_, UI::Box::Resizing::kFullSize);
> +	navigation_box_.add(&watchbtn_);
> +	navigation_box_.add_inf_space();
> +	navigation_box_.add(&openwindowbtn_);
> +	navigation_box_.add(&centerviewbtn_);
> +	main_box_.set_size(get_inner_w() - 2 * kPadding, get_inner_h() - 2 * kPadding);
> +
> +	// Configure actions
> +	idle_btn_.sigclicked.connect(
> +	   boost::bind(&SeafaringStatisticsMenu::filter_ships, this, ShipFilterStatus::kIdle));
> +	shipping_btn_.sigclicked.connect(
> +	   boost::bind(&SeafaringStatisticsMenu::filter_ships, this, ShipFilterStatus::kShipping));
> +	waiting_btn_.sigclicked.connect(boost::bind(
> +	   &SeafaringStatisticsMenu::filter_ships, this, ShipFilterStatus::kExpeditionWaiting));
> +	scouting_btn_.sigclicked.connect(boost::bind(
> +	   &SeafaringStatisticsMenu::filter_ships, this, ShipFilterStatus::kExpeditionScouting));
> +	portspace_btn_.sigclicked.connect(boost::bind(
> +	   &SeafaringStatisticsMenu::filter_ships, this, ShipFilterStatus::kExpeditionPortspaceFound));
> +	ship_filter_ = ShipFilterStatus::kAll;
> +	set_filter_ships_tooltips();
> +
> +	watchbtn_.sigclicked.connect(boost::bind(&SeafaringStatisticsMenu::watch_ship, this));
> +	openwindowbtn_.sigclicked.connect(boost::bind(&SeafaringStatisticsMenu::open_ship_window, this));
> +	centerviewbtn_.sigclicked.connect(boost::bind(&SeafaringStatisticsMenu::center_view, this));
> +
> +	// Configure table
> +	table_.selected.connect(boost::bind(&SeafaringStatisticsMenu::selected, this));
> +	table_.double_clicked.connect(boost::bind(&SeafaringStatisticsMenu::double_clicked, this));
> +	table_.add_column(
> +	   0, pgettext("ship", "Name"), "", UI::Align::kLeft, UI::TableColumnType::kFlexible);
> +	table_.add_column(200, pgettext("ship", "Status"));
> +	table_.set_sort_column(ColName);
> +	fill_table();
> +
> +	set_can_focus(true);
> +	set_thinks(false);
> +	table_.focus();
> +
> +	shipnotes_subscriber_ =
> +	   Notifications::subscribe<Widelands::NoteShip>([this](const Widelands::NoteShip& note) {
> +		   if (iplayer().get_player() == note.ship->get_owner()) {
> +			   switch (note.action) {
> +			   case Widelands::NoteShip::Action::kDestinationChanged:
> +			   case Widelands::NoteShip::Action::kWaitingForCommand:
> +			   case Widelands::NoteShip::Action::kGained:
> +				   update_ship(*note.ship);
> +				   break;
> +			   case Widelands::NoteShip::Action::kLost:
> +				   remove_ship(note.ship->serial());
> +				   break;
> +			   default:
> +				   NEVER_HERE();
> +			   }
> +		   }
> +		});
> +}
> +
> +const std::string
> +SeafaringStatisticsMenu::status_to_string(SeafaringStatisticsMenu::ShipFilterStatus status) const {
> +	switch (status) {
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kIdle:
> +		return pgettext("ship_state", "Idle");
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kShipping:
> +		return pgettext("ship_state", "Shipping");
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kExpeditionWaiting:
> +		return pgettext("ship_state", "Waiting");
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kExpeditionScouting:
> +		return pgettext("ship_state", "Scouting");
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kExpeditionPortspaceFound:
> +		return pgettext("ship_state", "Port Space Found");
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kExpeditionColonizing:
> +		return pgettext("ship_state", "Founding a Colony");
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kAll:
> +		return "All";  // The user shouldn't see this, so we don't localize
> +	default:
> +		NEVER_HERE();
> +	}
> +}
> +
> +const Image*
> +SeafaringStatisticsMenu::status_to_image(SeafaringStatisticsMenu::ShipFilterStatus status) const {
> +	std::string filename = "";
> +	switch (status) {
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kIdle:
> +		filename = "images/wui/stats/ship_stats_idle.png";
> +		break;
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kShipping:
> +		filename = "images/wui/stats/ship_stats_shipping.png";
> +		break;
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kExpeditionWaiting:
> +		filename = "images/wui/buildings/start_expedition.png";
> +		break;
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kExpeditionScouting:
> +		filename = "images/wui/ship/ship_explore_island_cw.png";
> +		break;
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kExpeditionPortspaceFound:
> +		filename = "images/wui/ship/ship_construct_port_space.png";
> +		break;
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kExpeditionColonizing:
> +		return colony_icon_;
> +	case SeafaringStatisticsMenu::ShipFilterStatus::kAll:
> +		filename = "images/wui/ship/ship_scout_ne.png";
> +		break;
> +	default:
> +		NEVER_HERE();
> +	}
> +	return g_gr->images().get(filename);
> +}
> +
> +const SeafaringStatisticsMenu::ShipInfo*
> +SeafaringStatisticsMenu::create_shipinfo(const Widelands::Ship& ship) const {
> +	if (&ship == nullptr) {
> +		return new ShipInfo();
> +	}
> +	const Widelands::Ship::ShipStates state = ship.get_ship_state();
> +	ShipFilterStatus status = ShipFilterStatus::kAll;
> +	switch (state) {
> +	case Widelands::Ship::ShipStates::kTransport:
> +		if (ship.get_destination(iplayer().game()) != nullptr) {
> +			status = ShipFilterStatus::kShipping;
> +		} else {
> +			status = ShipFilterStatus::kIdle;
> +		}
> +		break;
> +	case Widelands::Ship::ShipStates::kExpeditionWaiting:
> +		status = ShipFilterStatus::kExpeditionWaiting;
> +		break;
> +	case Widelands::Ship::ShipStates::kExpeditionScouting:
> +		status = ShipFilterStatus::kExpeditionScouting;
> +		break;
> +	case Widelands::Ship::ShipStates::kExpeditionPortspaceFound:
> +		status = ShipFilterStatus::kExpeditionPortspaceFound;
> +		break;
> +	case Widelands::Ship::ShipStates::kExpeditionColonizing:
> +		status = ShipFilterStatus::kExpeditionColonizing;
> +		break;
> +	case Widelands::Ship::ShipStates::kSinkRequest:
> +	case Widelands::Ship::ShipStates::kSinkAnimation:
> +		status = ShipFilterStatus::kAll;
> +	}
> +	return new ShipInfo(ship.get_shipname(), status, ship.serial());
> +}
> +
> +void SeafaringStatisticsMenu::set_entry_record(UI::Table<uintptr_t>::EntryRecord* er,
> +                                               const ShipInfo& info) {
> +	if (info.status != ShipFilterStatus::kAll) {
> +		er->set_string(ColName, info.name);
> +		er->set_picture(ColStatus, status_to_image(info.status), status_to_string(info.status));
> +	}
> +}
> +
> +Widelands::Ship* SeafaringStatisticsMenu::serial_to_ship(Widelands::Serial serial) const {
> +	Widelands::MapObject* obj = iplayer().game().objects().get_object(serial);
> +	assert(obj->descr().type() == Widelands::MapObjectType::SHIP);
> +	upcast(Widelands::Ship, ship, obj);
> +	return ship;
> +}
> +
> +void SeafaringStatisticsMenu::update_ship(const Widelands::Ship& ship) {
> +	assert(iplayer().get_player() == ship.get_owner());
> +	const ShipInfo* info = create_shipinfo(ship);
> +	// Remove ships that don't satisfy the filter
> +	if (ship_filter_ != ShipFilterStatus::kAll && !satisfies_filter(*info, ship_filter_)) {
> +		remove_ship(info->serial);
> +		return;
> +	}
> +	// Try to find the ship in the table
> +	if (data_.count(info->serial) == 1) {
> +		const ShipInfo* old_info = data_[info->serial].get();
> +		if (info->status != old_info->status) {
> +			// The status has changed - we need an update
> +			data_[info->serial] = std::unique_ptr<const ShipInfo>(info);
> +			UI::Table<uintptr_t>::EntryRecord* er = table_.find(info->serial);
> +			set_entry_record(er, *info);
> +		}
> +	} else {
> +		// This is a new ship or it was filtered away before
> +		data_.insert(std::make_pair(info->serial, std::unique_ptr<const ShipInfo>(info)));
> +		UI::Table<uintptr_t>::EntryRecord& er = table_.add(info->serial);
> +		set_entry_record(&er, *info);
> +	}
> +	table_.sort();
> +	set_buttons_enabled();
> +}
> +
> +void SeafaringStatisticsMenu::remove_ship(Widelands::Serial serial) {
> +	if (data_.count(serial) == 1) {
> +		table_.remove_entry(serial);
> +		data_.erase(data_.find(serial));
> +		if (!table_.empty() && !table_.has_selection()) {
> +			table_.select(0);
> +		}
> +		set_buttons_enabled();
> +	}
> +}
> +
> +void SeafaringStatisticsMenu::update_entry_record(UI::Table<uintptr_t>::EntryRecord& er,
> +                                                  const ShipInfo& info) {
> +	er.set_picture(ColStatus, status_to_image(info.status), status_to_string(info.status));
> +}
> +
> +void SeafaringStatisticsMenu::selected() {
> +	set_buttons_enabled();
> +}
> +
> +void SeafaringStatisticsMenu::double_clicked() {
> +	if (table_.has_selection()) {
> +		center_view();
> +	}
> +}
> +
> +void SeafaringStatisticsMenu::set_buttons_enabled() {
> +	centerviewbtn_.set_enabled(table_.has_selection());
> +	openwindowbtn_.set_enabled(table_.has_selection());
> +	watchbtn_.set_enabled(table_.has_selection());
> +}
> +
> +bool SeafaringStatisticsMenu::handle_key(bool down, SDL_Keysym code) {
> +	if (down) {
> +		switch (code.sym) {
> +		// Don't forget to change the tooltips if any of these get reassigned
> +		case SDLK_g:
> +			center_view();
> +			return true;
> +		case SDLK_o:
> +			open_ship_window();
> +			return true;
> +		case SDLK_w:
> +			watch_ship();
> +			return true;
> +		case SDLK_0:
> +			if (code.mod & KMOD_ALT) {
> +				filter_ships(ShipFilterStatus::kAll);
> +				return true;
> +			}
> +			return false;
> +		case SDLK_1:
> +			if (code.mod & KMOD_ALT) {
> +				filter_ships(ShipFilterStatus::kIdle);
> +				return true;
> +			}
> +			return false;
> +		case SDLK_2:
> +			if (code.mod & KMOD_ALT) {
> +				filter_ships(ShipFilterStatus::kShipping);
> +				return true;
> +			}
> +			return false;
> +		case SDLK_3:
> +			if (code.mod & KMOD_ALT) {
> +				filter_ships(ShipFilterStatus::kExpeditionWaiting);
> +				return true;
> +			}
> +			return false;
> +		case SDLK_4:
> +			if (code.mod & KMOD_ALT) {
> +				filter_ships(ShipFilterStatus::kExpeditionScouting);
> +				return true;
> +			}
> +			return false;
> +		case SDLK_5:
> +			if (code.mod & KMOD_ALT) {
> +				filter_ships(ShipFilterStatus::kExpeditionPortspaceFound);
> +				return true;
> +			}
> +			return false;
> +		case SDL_SCANCODE_KP_PERIOD:
> +		case SDLK_KP_PERIOD:
> +			if (code.mod & KMOD_NUM)
> +				break;
> +		/* no break */
> +		default:
> +			break;  // not handled
> +		}
> +	}
> +
> +	return table_.handle_key(down, code);
> +}
> +
> +void SeafaringStatisticsMenu::center_view() {
> +	if (table_.has_selection()) {
> +		Widelands::Ship* ship = serial_to_ship(table_.get_selected());
> +		iplayer().map_view()->scroll_to_field(ship->get_position(), MapView::Transition::Smooth);
> +	}
> +}
> +
> +void SeafaringStatisticsMenu::watch_ship() {
> +	if (table_.has_selection()) {
> +		Widelands::Ship* ship = serial_to_ship(table_.get_selected());
> +		WatchWindow* window = show_watch_window(iplayer(), ship->get_position());
> +		window->follow(ship);
> +	}
> +}
> +
> +void SeafaringStatisticsMenu::open_ship_window() {
> +	if (table_.has_selection()) {
> +		center_view();
> +		Widelands::Ship* ship = serial_to_ship(table_.get_selected());
> +		iplayer().show_ship_window(ship);
> +	}
> +}
> +
> +void SeafaringStatisticsMenu::filter_ships(ShipFilterStatus status) {
> +	switch (status) {
> +	case ShipFilterStatus::kExpeditionWaiting:
> +		toggle_filter_ships_button(waiting_btn_, status);
> +		break;
> +	case ShipFilterStatus::kExpeditionScouting:
> +		toggle_filter_ships_button(scouting_btn_, status);
> +		break;
> +	// We're grouping the "colonizing" status with the port space.
> +	case ShipFilterStatus::kExpeditionColonizing:
> +	case ShipFilterStatus::kExpeditionPortspaceFound:
> +		toggle_filter_ships_button(portspace_btn_, status);
> +		break;
> +	case ShipFilterStatus::kShipping:
> +		toggle_filter_ships_button(shipping_btn_, status);
> +		break;
> +	case ShipFilterStatus::kIdle:
> +		toggle_filter_ships_button(idle_btn_, status);
> +		break;
> +	case ShipFilterStatus::kAll:
> +		set_filter_ships_tooltips();
> +		ship_filter_ = ShipFilterStatus::kAll;
> +		waiting_btn_.set_perm_pressed(false);
> +		scouting_btn_.set_perm_pressed(false);
> +		portspace_btn_.set_perm_pressed(false);
> +		shipping_btn_.set_perm_pressed(false);
> +		idle_btn_.set_perm_pressed(false);
> +		break;
> +	}
> +	fill_table();
> +}
> +
> +void SeafaringStatisticsMenu::toggle_filter_ships_button(UI::Button& button,
> +                                                         ShipFilterStatus status) {
> +	set_filter_ships_tooltips();
> +	if (button.style() == UI::Button::Style::kPermpressed) {
> +		button.set_perm_pressed(false);
> +		ship_filter_ = ShipFilterStatus::kAll;
> +	} else {
> +		waiting_btn_.set_perm_pressed(false);
> +		scouting_btn_.set_perm_pressed(false);
> +		portspace_btn_.set_perm_pressed(false);
> +		shipping_btn_.set_perm_pressed(false);
> +		idle_btn_.set_perm_pressed(false);
> +		button.set_perm_pressed(true);
> +		ship_filter_ = status;
> +
> +		/** TRANSLATORS: %1% is a tooltip, %2% is the corresponding hotkey */
> +		button.set_tooltip((boost::format(_("%1% (Hotkey: %2%)"))
> +		                    /** TRANSLATORS: Tooltip in the messages window */
> +		                    % _("Show all ships") % pgettext("hotkey", "Alt + 0"))
> +		                      .str());
> +	}
> +}
> +
> +void SeafaringStatisticsMenu::set_filter_ships_tooltips() {
> +
> +	idle_btn_.set_tooltip((boost::format(_("%1% (Hotkey: %2%)"))
> +	                       /** TRANSLATORS: Tooltip in the messages window */
> +	                       % _("Show idle ships") % pgettext("hotkey", "Alt + 1"))
> +	                         .str());
> +	shipping_btn_.set_tooltip((boost::format(_("%1% (Hotkey: %2%)"))
> +	                           /** TRANSLATORS: Tooltip in the messages window */
> +	                           % _("Show ships shipping wares and workers") %
> +	                           pgettext("hotkey", "Alt + 2"))
> +	                             .str());
> +	waiting_btn_.set_tooltip((boost::format(_("%1% (Hotkey: %2%)"))
> +	                          /** TRANSLATORS: Tooltip in the messages window */
> +	                          % _("Show waiting expeditions") % pgettext("hotkey", "Alt + 3"))
> +	                            .str());
> +	scouting_btn_.set_tooltip((boost::format(_("%1% (Hotkey: %2%)"))
> +	                           /** TRANSLATORS: Tooltip in the messages window */
> +	                           % _("Show scouting expeditions") % pgettext("hotkey", "Alt + 4"))
> +	                             .str());
> +	portspace_btn_.set_tooltip(
> +	   (boost::format(_("%1% (Hotkey: %2%)"))
> +	    /** TRANSLATORS: Tooltip in the messages window */
> +	    % _("Show colonizing expeditions and expeditions with port space found") %
> +	    pgettext("hotkey", "Alt + 5"))
> +	      .str());
> +}
> +
> +bool SeafaringStatisticsMenu::satisfies_filter(const ShipInfo& info, ShipFilterStatus filter) {
> +	return filter == info.status || (filter == ShipFilterStatus::kExpeditionPortspaceFound &&
> +	                                 info.status == ShipFilterStatus::kExpeditionColonizing);
> +}
> +
> +void SeafaringStatisticsMenu::fill_table() {

I am against moving it down, because I want them disabled while the new table is calculated, just in case. table_.select(0) will trigger a boost::signal, which then updates the button state again.

I have added some comments.

> +	const Widelands::Serial last_selection =
> +	   table_.has_selection() ? table_.get_selected() : Widelands::INVALID_INDEX;
> +	table_.clear();
> +	data_.clear();
> +	set_buttons_enabled();
> +	for (const auto& serial : iplayer().player().ships()) {
> +		Widelands::Ship* ship = serial_to_ship(serial);
> +		assert(iplayer().get_player() == ship->get_owner());
> +		const ShipInfo* info = create_shipinfo(*ship);
> +		if (info->status != ShipFilterStatus::kAll) {
> +			if (ship_filter_ == ShipFilterStatus::kAll || satisfies_filter(*info, ship_filter_)) {
> +				data_.insert(std::make_pair(serial, std::unique_ptr<const ShipInfo>(info)));
> +				UI::Table<uintptr_t const>::EntryRecord& er =
> +				   table_.add(serial, serial == last_selection);
> +				set_entry_record(&er, *info);
> +			}
> +		}
> +	}
> +
> +	if (!table_.empty()) {
> +		table_.sort();
> +		if (!table_.has_selection()) {
> +			table_.select(0);
> +		}
> +	}
> +}


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1358880-ship-statistics/+merge/343293
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ships_optr.


References