← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/savegame-menu into lp:widelands

 

Review: Needs Fixing

I did a round of code diff review and a bit of testing. The code looks good to me, just some small remarks. I haven't retraced the whole refactoring process, but it seems to be okay.

However, there are some issues with the removal of save games:
- Just a suggestion: After removing a game, nothing is selected. Maybe select the next entry in the list?

- Bug: After removing a file, the right column still displays the information about the no longer existing save. Also the OK button is enabled but does nothing.

- Bug: When the removal dialog was open, navigation with arrow keys no longer works.

- Bug: When selecting many entries (around 30 for me in non-fullscreen mode) the "really remove" dialog box can become cut off. First, the entries are all listed normally which is fine. At some point the dialog box uses a scroll box which is fine, too. Between this cases there is some number of entries where the message window is too big to be drawn completely and parts are cut off at the top and bottom.

- A small optical comment: When having slightly less than the "use scroll box" number of entries selected, the window with the list is bigger than with the scrollbox. No need to fix this, though.

- The removal dialog displays the filename when a single file is selected but a list of "mapnames + saved-on-date" when multiple files are selected. The filename is not that useful I think, maybe change it to the name of the save? When I had multiple campaign saves selected, the list contained five times the same entry. Looked a bit strange but is no bug. For campaign save games the name of the save and the filename can both be quite long. So using the full name in the "multiple files selected" list is probably no good idea.


A feel-free-to-ignore comment regarding the minimap preview: From other games I am used to having the preview box always in the same size and scaling the image accordingly. Here the size of the box changes to match the image. That has the advantage of having a good image quality but is a bit "jumpy" when scrolling through the list.

Diff comments:

> === modified file 'src/game_io/game_preload_packet.cc'
> --- src/game_io/game_preload_packet.cc	2017-08-17 15:34:45 +0000
> +++ src/game_io/game_preload_packet.cc	2017-10-02 17:28:52 +0000
> @@ -46,9 +46,11 @@
>  constexpr uint16_t kCurrentPacketVersion = 6;
>  constexpr const char* kMinimapFilename = "minimap.png";
>  
> +// Win condition localization can come from the 'widelands' or 'win_conditions' textdomain.
>  std::string GamePreloadPacket::get_localized_win_condition() const {

When I understand this method correctly, one of the two translation attempts will fail, right? Should work fine, I guess, but it does look a bit messy. It is possible to use only one of the textdomains (i.e. move the translations)?

> +	std::string result = _(win_condition_);
>  	i18n::Textdomain td("win_conditions");
> -	return _(win_condition_);
> +	return _(result);
>  }
>  
>  void GamePreloadPacket::read(FileSystem& fs, Game&, MapObjectLoader* const) {
> 
> === modified file 'src/ui_basic/icon.cc'
> --- src/ui_basic/icon.cc	2017-04-22 12:19:21 +0000
> +++ src/ui_basic/icon.cc	2017-10-02 17:28:52 +0000
> @@ -54,16 +54,16 @@
>  	if (pic_) {
>  		const float scale = std::min(1.f, std::min(static_cast<float>(get_w()) / pic_->width(),
>  		                                           static_cast<float>(get_h()) / pic_->height()));
> -
> -		const float width = scale * get_w();
> -		const float height = scale * get_h();
> -		const float x = (get_w() - width) / 2.f;
> -		const float y = (get_h() - height) / 2.f;
> +		// We need to be pixel perfect, so we use ints.
> +		const int width = scale * get_w();
> +		const int height = scale * get_h();
> +		const int x = (get_w() - width) / 2;
> +		const int y = (get_h() - height) / 2;
>  		dst.blitrect_scale(Rectf(x, y, width, height), pic_,
>  		                   Recti(0, 0, pic_->width(), pic_->height()), 1., BlendMode::UseAlpha);
> -	}
> -	if (draw_frame_) {
> -		dst.draw_rect(Recti(0, 0, get_w(), get_h()), framecolor_);
> +		if (draw_frame_) {
> +			dst.draw_rect(Recti(x, y, width, height), framecolor_);

I think this overdraws one line/column of pixels of the picture on each side.

> +		}
>  	}
>  }
>  }
> 
> === modified file 'src/ui_basic/messagebox.cc'
> --- src/ui_basic/messagebox.cc	2017-05-25 12:30:40 +0000
> +++ src/ui_basic/messagebox.cc	2017-10-02 17:28:52 +0000
> @@ -146,12 +146,16 @@
>  }
>  
>  void WLMessageBox::clicked_ok() {
> +	ok_button_->set_enabled(false);
> +	cancel_button_->set_enabled(false);
>  	ok();

Fine by me, but what is the purpose of this? Avoiding potential bugs through double clicks?

>  	if (is_modal())
>  		end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
>  }
>  
>  void WLMessageBox::clicked_back() {
> +	ok_button_->set_enabled(false);
> +	cancel_button_->set_enabled(false);
>  	cancel();
>  	if (is_modal())
>  		end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack);
> 
> === modified file 'src/ui_basic/multilinetextarea.cc'
> --- src/ui_basic/multilinetextarea.cc	2017-05-21 18:15:17 +0000
> +++ src/ui_basic/multilinetextarea.cc	2017-10-02 17:28:52 +0000
> @@ -51,15 +51,13 @@
>       scrollmode_(scroll_mode),
>       pic_background_(nullptr) {
>  	assert(scrollmode_ == MultilineTextarea::ScrollMode::kNoScrolling || Scrollbar::kSize <= w);
> +	set_scrollmode(scroll_mode);

set_scrollmode() also calls layout(). Is this intentional? If not, maybe move this call to the bottom of this method.
Also, we could remove "scrollmode_(scroll_mode)," above.

>  	set_thinks(false);
>  
>  	scrollbar_.moved.connect(boost::bind(&MultilineTextarea::scrollpos_changed, this, _1));
>  
>  	scrollbar_.set_singlestepsize(text_height());
>  	scrollbar_.set_steps(1);
> -	scrollbar_.set_force_draw(scrollmode_ == ScrollMode::kScrollNormalForced ||
> -	                          scrollmode_ == ScrollMode::kScrollLogForced);
> -
>  	layout();
>  }
>  
> 
> === modified file 'src/ui_fsmenu/loadgame.cc'
> --- src/ui_fsmenu/loadgame.cc	2017-06-24 10:38:19 +0000
> +++ src/ui_fsmenu/loadgame.cc	2017-10-02 17:28:52 +0000
> @@ -19,659 +19,121 @@
>  
>  #include "ui_fsmenu/loadgame.h"
>  
> -#include <algorithm>
> -#include <cstdio>
> -#include <ctime>
> -#include <memory>
> -
> -#include <boost/algorithm/string/predicate.hpp>
> -#include <boost/format.hpp>
> -
>  #include "base/i18n.h"
> -#include "base/log.h"
> -#include "base/time_string.h"
> -#include "game_io/game_loader.h"
> -#include "game_io/game_preload_packet.h"
> -#include "graphic/graphic.h"
> -#include "graphic/image_io.h"
> -#include "graphic/text_constants.h"
> -#include "graphic/texture.h"
> -#include "helper.h"
> -#include "io/filesystem/layered_filesystem.h"
> -#include "logic/game.h"
> -#include "logic/game_controller.h"
> -#include "logic/game_settings.h"
> -#include "logic/replay.h"
> -#include "ui_basic/icon.h"
> -#include "ui_basic/messagebox.h"
> -
> -// TODO(GunChleoc): Arabic: line height broken for descriptions for Arabic.
> -namespace {
> -
> -// This function concatenates the filename and localized map name for a savegame/replay.
> -// If the filename starts with the map name, the map name is omitted.
> -// It also prefixes autosave files with a numbered and localized "Autosave" prefix.
> -std::string map_filename(const std::string& filename, const std::string& mapname) {
> -	std::string result = FileSystem::filename_without_ext(filename.c_str());
> -	std::string mapname_localized;
> -	{
> -		i18n::Textdomain td("maps");
> -		mapname_localized = _(mapname);
> -	}
> -
> -	if (boost::starts_with(result, "wl_autosave")) {
> -		std::vector<std::string> autosave_name;
> -		boost::split(autosave_name, result, boost::is_any_of("_"));
> -		if (autosave_name.empty() || autosave_name.size() < 3) {
> -			/** TRANSLATORS: %1% is a map's name. */
> -			result = (boost::format(_("Autosave: %1%")) % mapname_localized).str();
> -		} else {
> -			/** TRANSLATORS: %1% is a number, %2% a map's name. */
> -			result = (boost::format(_("Autosave %1%: %2%")) % autosave_name.back() % mapname_localized)
> -			            .str();
> -		}
> -	} else if (!(boost::starts_with(result, mapname) ||
> -	             boost::starts_with(result, mapname_localized))) {
> -		/** TRANSLATORS: %1% is a filename, %2% a map's name. */
> -		result = (boost::format(_("%1% (%2%)")) % result % mapname_localized).str();
> -	}
> -	return result;
> -}
> -
> -}  // namespace
> +#include "wui/gamedetails.h"
>  
>  FullscreenMenuLoadGame::FullscreenMenuLoadGame(Widelands::Game& g,
>                                                 GameSettingsProvider* gsp,
>                                                 GameController* gc,
>                                                 bool is_replay)
>     : FullscreenMenuLoadMapOrGame(),
> -     table_(this,
> -            tablex_,
> -            tabley_,
> -            tablew_,
> -            tableh_,
> -            g_gr->images().get("images/ui_basic/but3.png"),
> -            UI::TableRows::kMultiDescending),
> -
> -     is_replay_(is_replay),
> +
> +     main_box_(this, 0, 0, UI::Box::Vertical),
> +     info_box_(&main_box_, 0, 0, UI::Box::Horizontal),
> +
>       // Main title
> -     title_(this,
> -            get_w() / 2,
> -            tabley_ / 3,
> -            is_replay_ ? _("Choose a replay") : _("Choose a saved game"),
> +     title_(&main_box_,
> +            0,
> +            0,
> +            is_replay ? _("Choose a replay") : _("Choose a saved game"),
>              UI::Align::kCenter),
>  
> -     // Savegame description
> -     label_mapname_(this, right_column_x_, tabley_),
> -     ta_mapname_(this,
> -                 right_column_x_ + indent_,
> -                 get_y_from_preceding(label_mapname_) + padding_,
> -                 get_right_column_w(right_column_x_ + indent_),
> -                 2 * label_height_ - padding_),
> -
> -     label_gametime_(this, right_column_x_, get_y_from_preceding(ta_mapname_) + 2 * padding_),
> -     ta_gametime_(this,
> -                  right_column_tab_,
> -                  label_gametime_.get_y(),
> -                  get_right_column_w(right_column_tab_),
> -                  label_height_),
> -
> -     label_players_(this, right_column_x_, get_y_from_preceding(ta_gametime_)),
> -     ta_players_(this,
> -                 right_column_tab_,
> -                 label_players_.get_y(),
> -                 get_right_column_w(right_column_tab_),
> -                 label_height_),
> -
> -     label_version_(this, right_column_x_, get_y_from_preceding(ta_players_)),
> -     ta_version_(this, right_column_tab_, label_version_.get_y()),
> -
> -     label_win_condition_(this, right_column_x_, get_y_from_preceding(ta_version_) + 3 * padding_),
> -     ta_win_condition_(this,
> -                       right_column_x_ + indent_,
> -                       get_y_from_preceding(label_win_condition_) + padding_,
> -                       get_right_column_w(right_column_x_ + indent_),
> -                       label_height_),
> -
> -     delete_(this,
> -             "delete",
> -             right_column_x_,
> -             buty_ - buth_ - 2 * padding_,
> -             butw_,
> -             buth_,
> -             g_gr->images().get("images/ui_basic/but0.png"),
> -             _("Delete")),
> -
> -     ta_long_generic_message_(this,
> -                              right_column_x_,
> -                              get_y_from_preceding(ta_mapname_) + 2 * padding_,
> -                              get_right_column_w(right_column_x_),
> -                              delete_.get_y() - get_y_from_preceding(ta_mapname_) - 6 * padding_),
> -
> -     minimap_y_(get_y_from_preceding(ta_win_condition_) + 3 * padding_),
> -     minimap_w_(get_right_column_w(right_column_x_)),
> -     minimap_h_(delete_.get_y() - get_y_from_preceding(ta_win_condition_) - 6 * padding_),
> -     minimap_icon_(this,
> -                   right_column_x_,
> -                   get_y_from_preceding(ta_win_condition_) + 3 * padding_,
> -                   minimap_w_,
> -                   minimap_h_,
> -                   nullptr),
> -
> -     // "Data container" for the savegame information
> +     load_or_save_(&info_box_,
> +                   g,
> +                   is_replay ? LoadOrSaveGame::FileType::kReplay : gsp->settings().multiplayer ?
> +                               LoadOrSaveGame::FileType::kGameMultiPlayer :
> +                               LoadOrSaveGame::FileType::kGameSinglePlayer,

Maybe spend some brackets around here. The code should be fine for the compiler but isn't that great for humans.

> +                   GameDetails::Style::kFsMenu,
> +                   true),
> +
> +     is_replay_(is_replay),
>       game_(g),
>       settings_(gsp),
>       ctrl_(gc) {
> -	title_.set_fontsize(UI_FONT_SIZE_BIG);
> -	ta_gametime_.set_tooltip(_("The time that elapsed inside this game"));
> -	ta_players_.set_tooltip(_("The number of players"));
> -	ta_version_.set_tooltip(_("The version of Widelands that this game was played under"));
> -	ta_win_condition_.set_tooltip(_("The win condition that was set for this game"));
> +
> +	// Make sure that we have some space to work with.
> +	main_box_.set_size(get_w(), get_w());
> +
> +	main_box_.add_space(padding_);
> +	main_box_.add_inf_space();
> +	main_box_.add(&title_, UI::Box::Resizing::kAlign, UI::Align::kCenter);
> +	main_box_.add_inf_space();
> +	main_box_.add_inf_space();
> +	main_box_.add(&info_box_, UI::Box::Resizing::kExpandBoth);
> +	main_box_.add_space(padding_);
> +
> +	info_box_.add(load_or_save_.table_box(), UI::Box::Resizing::kFullSize);
> +	info_box_.add_space(right_column_margin_);
> +	info_box_.add(load_or_save_.game_details(), UI::Box::Resizing::kFullSize);
> +
> +	button_spacer_ = new UI::Panel(load_or_save_.game_details()->button_box(), 0, 0, 0, 0);
> +	load_or_save_.game_details()->button_box()->add(button_spacer_);
> +
> +	layout();
> +
> +	ok_.set_enabled(false);
> +	set_thinks(false);
>  
>  	if (is_replay_) {
>  		back_.set_tooltip(_("Return to the main menu"));
>  		ok_.set_tooltip(_("Load this replay"));
> -		ta_mapname_.set_tooltip(_("The map that this replay is based on"));
> -		delete_.set_tooltip(_("Delete this replay"));
>  	} else {
> -		back_.set_tooltip(_("Return to the single player menu"));
> +		back_.set_tooltip(gsp->settings().multiplayer ? _("Return to the multiplayer game setup") :
> +		                                                _("Return to the single player menu"));
>  		ok_.set_tooltip(_("Load this game"));
> -		ta_mapname_.set_tooltip(_("The map that this game is based on"));
> -		delete_.set_tooltip(_("Delete this game"));
>  	}
> -	set_thinks(false);
> -	minimap_icon_.set_visible(false);
>  
>  	back_.sigclicked.connect(boost::bind(&FullscreenMenuLoadGame::clicked_back, boost::ref(*this)));
>  	ok_.sigclicked.connect(boost::bind(&FullscreenMenuLoadGame::clicked_ok, boost::ref(*this)));
> -	delete_.sigclicked.connect(
> -	   boost::bind(&FullscreenMenuLoadGame::clicked_delete, boost::ref(*this)));
> -	table_.add_column(130, _("Save Date"), _("The date this game was saved"));
> -	if (is_replay_ || settings_->settings().multiplayer) {
> -		std::vector<std::string> modes;
> -		if (is_replay_) {
> -			/** TRANSLATORS: Tooltip for the "Mode" column when choosing a game/replay to load. */
> -			/** TRANSLATORS: Make sure that you keep consistency in your translation. */
> -			modes.push_back(_("SP = Single Player"));
> -		}
> -		/** TRANSLATORS: Tooltip for the "Mode" column when choosing a game/replay to load. */
> -		/** TRANSLATORS: Make sure that you keep consistency in your translation. */
> -		modes.push_back(_("MP = Multiplayer"));
> -		/** TRANSLATORS: Tooltip for the "Mode" column when choosing a game/replay to load. */
> -		/** TRANSLATORS: Make sure that you keep consistency in your translation. */
> -		modes.push_back(_("H = Multiplayer (Host)"));
> -		const std::string mode_tooltip_1 =
> -		   /** TRANSLATORS: Tooltip for the "Mode" column when choosing a game/replay to load. */
> -		   /** TRANSLATORS: %s is a list of game modes. */
> -		   ((boost::format(_("Game Mode: %s.")) %
> -		     i18n::localize_list(modes, i18n::ConcatenateWith::COMMA)))
> -		      .str();
> -		const std::string mode_tooltip_2 = _("Numbers are the number of players.");
> -
> -		table_.add_column(
> -		   65,
> -		   /** TRANSLATORS: Game Mode table column when choosing a game/replay to load. */
> -		   /** TRANSLATORS: Keep this to 5 letters maximum. */
> -		   /** TRANSLATORS: A tooltip will explain if you need to use an abbreviation. */
> -		   _("Mode"), (boost::format("%s %s") % mode_tooltip_1 % mode_tooltip_2).str());
> -	}
> -	table_.add_column(0, _("Description"),
> -	                  _("The filename that the game was saved under followed by the map’s name, "
> -	                    "or the map’s name followed by the last objective achieved."),
> -	                  UI::Align::kLeft, UI::TableColumnType::kFlexible);
> -	table_.set_column_compare(
> -	   0, boost::bind(&FullscreenMenuLoadGame::compare_date_descending, this, _1, _2));
> -	table_.selected.connect(boost::bind(&FullscreenMenuLoadGame::entry_selected, this));
> -	table_.double_clicked.connect(
> +	load_or_save_.table().selected.connect(
> +	   boost::bind(&FullscreenMenuLoadGame::entry_selected, this));
> +	load_or_save_.table().double_clicked.connect(
>  	   boost::bind(&FullscreenMenuLoadGame::clicked_ok, boost::ref(*this)));
> -	table_.set_sort_column(0);
> -	table_.focus();
> +
>  	fill_table();
> +	if (!load_or_save_.table().empty()) {
> +		load_or_save_.table().select(0);
> +	}
>  }
>  
>  void FullscreenMenuLoadGame::layout() {
> -	// TODO(GunChleoc): Implement when we have box layout for the details.
> -	table_.layout();
> -}
> -
> -void FullscreenMenuLoadGame::think() {
> -	if (ctrl_) {
> -		ctrl_->think();
> -	}
> -}
> -
> -// Reverse default sort order for save date column
> -bool FullscreenMenuLoadGame::compare_date_descending(uint32_t rowa, uint32_t rowb) {
> -	const SavegameData& r1 = games_data_[table_[rowa]];
> -	const SavegameData& r2 = games_data_[table_[rowb]];
> -
> -	return r1.savetimestamp < r2.savetimestamp;
> +	FullscreenMenuLoadMapOrGame::layout();
> +	main_box_.set_size(get_w() - 2 * tablex_, tabley_ + tableh_ + padding_);
> +	main_box_.set_pos(Vector2i(tablex_, 0));
> +	title_.set_fontsize(fs_big());
> +	load_or_save_.delete_button()->set_desired_size(butw_, buth_);
> +	button_spacer_->set_desired_size(butw_, buth_ + 2 * padding_);
> +	load_or_save_.table().set_desired_size(tablew_, tableh_);
> +	load_or_save_.game_details()->set_max_size(
> +	   main_box_.get_w() - tablew_ - right_column_margin_, tableh_);
>  }
>  
>  void FullscreenMenuLoadGame::clicked_ok() {
> -	if (!table_.has_selection()) {
> +	if (load_or_save_.table().selections().size() != 1) {
>  		return;
>  	}
> -	const SavegameData& gamedata = games_data_[table_.get_selected()];
> -	if (gamedata.errormessage.empty()) {
> -		filename_ = gamedata.filename;
> +
> +	const SavegameData* gamedata = load_or_save_.entry_selected();
> +	if (gamedata && gamedata->errormessage.empty()) {
> +		filename_ = gamedata->filename;
>  		end_modal<FullscreenMenuBase::MenuTarget>(FullscreenMenuBase::MenuTarget::kOk);
>  	}
>  }
>  
> -void FullscreenMenuLoadGame::clicked_delete() {
> -	if (!table_.has_selection()) {
> -		return;
> -	}
> -	std::set<uint32_t> selections = table_.selections();
> -	size_t no_selections = selections.size();
> -	std::string message;
> -	if (no_selections > 1) {
> -		if (is_replay_) {
> -			message = (boost::format(ngettext("Do you really want to delete this %d replay?",
> -			                                  "Do you really want to delete these %d replays?",
> -			                                  no_selections)) %
> -			           no_selections)
> -			             .str();
> -		} else {
> -			message = (boost::format(ngettext("Do you really want to delete this %d game?",
> -			                                  "Do you really want to delete these %d games?",
> -			                                  no_selections)) %
> -			           no_selections)
> -			             .str();
> -		}
> -		message = (boost::format("%s\n%s") % message % filename_list_string()).str();
> -
> -	} else {
> -		const SavegameData& gamedata = games_data_[table_.get_selected()];
> -
> -		message = (boost::format("%s %s\n") % label_mapname_.get_text() % gamedata.mapname).str();
> -
> -		message = (boost::format("%s %s %s\n") % message % label_win_condition_.get_text() %
> -		           gamedata.wincondition)
> -		             .str();
> -
> -		message =
> -		   (boost::format("%s %s %s\n") % message % _("Save Date:") % gamedata.savedatestring).str();
> -
> -		message = (boost::format("%s %s %s\n") % message % label_gametime_.get_text() %
> -		           gametimestring(gamedata.gametime))
> -		             .str();
> -
> -		message =
> -		   (boost::format("%s %s %s\n\n") % message % label_players_.get_text() % gamedata.nrplayers)
> -		      .str();
> -
> -		message = (boost::format("%s %s %s\n") % message % _("Filename:") % gamedata.filename).str();
> -
> -		if (is_replay_) {
> -			message =
> -			   (boost::format("%s\n\n%s") % _("Do you really want to delete this replay?") % message)
> -			      .str();
> -		} else {
> -			message =
> -			   (boost::format("%s\n\n%s") % _("Do you really want to delete this game?") % message)
> -			      .str();
> -		}
> -	}
> -
> -	UI::WLMessageBox confirmationBox(
> -	   this, ngettext("Confirm deleting file", "Confirm deleting files", no_selections), message,
> -	   UI::WLMessageBox::MBoxType::kOkCancel);
> -
> -	if (confirmationBox.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kOk) {
> -		for (const uint32_t index : selections) {
> -			const std::string& deleteme = games_data_[table_.get(table_.get_record(index))].filename;
> -			g_fs->fs_unlink(deleteme);
> -			if (is_replay_) {
> -				g_fs->fs_unlink(deleteme + WLGF_SUFFIX);
> -			}
> -		}
> -		fill_table();
> -	}
> -}
> -
> -std::string FullscreenMenuLoadGame::filename_list_string() {
> -	boost::format message;
> -	for (const uint32_t index : table_.selections()) {
> -		const SavegameData& gamedata = games_data_[table_.get(table_.get_record(index))];
> -		if (gamedata.errormessage.empty()) {
> -			message =
> -			   boost::format("%s\n%s") % message %
> -			   /** TRANSLATORS %1% = map name, %2% = save date. */
> -			   (boost::format(_("%1%, saved on %2%")) % gamedata.mapname % gamedata.savedatestring);
> -		} else {
> -			message = boost::format("%s\n%s") % message % gamedata.filename;
> -		}
> -	}
> -	return message.str();
> -}
> -
> -bool FullscreenMenuLoadGame::set_has_selection() {
> -	bool has_selection = table_.selections().size() < 2;
> -	ok_.set_enabled(has_selection);
> -	delete_.set_enabled(table_.has_selection());
> -
> -	if (!has_selection) {
> -		label_mapname_.set_text(std::string());
> -		label_gametime_.set_text(std::string());
> -		label_players_.set_text(std::string());
> -		label_version_.set_text(std::string());
> -		label_win_condition_.set_text(std::string());
> -
> -		ta_mapname_.set_text(std::string());
> -		ta_gametime_.set_text(std::string());
> -		ta_players_.set_text(std::string());
> -		ta_version_.set_text(std::string());
> -		ta_win_condition_.set_text(std::string());
> -		minimap_icon_.set_icon(nullptr);
> -		minimap_icon_.set_visible(false);
> -		minimap_icon_.set_no_frame();
> -		minimap_image_.reset();
> -	} else {
> -		label_mapname_.set_text(_("Map Name:"));
> -		label_gametime_.set_text(_("Gametime:"));
> -		label_players_.set_text(_("Players:"));
> -		label_win_condition_.set_text(_("Win Condition:"));
> -	}
> -	return has_selection;
> -}
> -
>  void FullscreenMenuLoadGame::entry_selected() {
> -	size_t selections = table_.selections().size();
> -	if (set_has_selection()) {
> -
> -		const SavegameData& gamedata = games_data_[table_.get_selected()];
> -		ta_long_generic_message_.set_text(gamedata.errormessage);
> -
> -		if (gamedata.errormessage.empty()) {
> -			ta_long_generic_message_.set_visible(false);
> -			ta_mapname_.set_text(gamedata.mapname);
> -			ta_gametime_.set_text(gametimestring(gamedata.gametime));
> -
> -			uint8_t number_of_players = gamedata.nrplayers;
> -			if (number_of_players > 0) {
> -				ta_players_.set_text(
> -				   (boost::format("%u") % static_cast<unsigned int>(number_of_players)).str());
> -			} else {
> -				label_players_.set_text("");
> -				ta_players_.set_text("");
> -			}
> -
> -			if (gamedata.version.empty()) {
> -				label_version_.set_text("");
> -				ta_version_.set_text("");
> -			} else {
> -				label_version_.set_text(_("Widelands Version:"));
> -				ta_version_.set_text(gamedata.version);
> -			}
> -
> -			{
> -				i18n::Textdomain td("win_conditions");
> -				ta_win_condition_.set_text(_(gamedata.wincondition));
> -			}
> -
> -			std::string minimap_path = gamedata.minimap_path;
> -			// Delete former image
> -			minimap_icon_.set_icon(nullptr);
> -			minimap_icon_.set_visible(false);
> -			minimap_icon_.set_no_frame();
> -			minimap_image_.reset();
> -			// Load the new one
> -			if (!minimap_path.empty()) {
> -				try {
> -					// Load the image
> -					minimap_image_ = load_image(
> -					   minimap_path,
> -					   std::unique_ptr<FileSystem>(g_fs->make_sub_file_system(gamedata.filename)).get());
> -
> -					// Scale it
> -					double scale = double(minimap_w_) / minimap_image_->width();
> -					double scaleY = double(minimap_h_) / minimap_image_->height();
> -					if (scaleY < scale) {
> -						scale = scaleY;
> -					}
> -					if (scale > 1.0)
> -						scale = 1.0;  // Don't make the image too big; fuzziness will result
> -					uint16_t w = scale * minimap_image_->width();
> -					uint16_t h = scale * minimap_image_->height();
> -
> -					// Center the minimap in the available space
> -					int32_t xpos =
> -					   right_column_x_ + (get_w() - right_column_margin_ - w - right_column_x_) / 2;
> -					int32_t ypos = minimap_y_;
> -
> -					// Set small minimaps higher up for a more harmonious look
> -					if (h < minimap_h_ * 2 / 3) {
> -						ypos += (minimap_h_ - h) / 3;
> -					} else {
> -						ypos += (minimap_h_ - h) / 2;
> -					}
> -
> -					minimap_icon_.set_size(w, h);
> -					minimap_icon_.set_pos(Vector2i(xpos, ypos));
> -					minimap_icon_.set_frame(UI_FONT_CLR_FG);
> -					minimap_icon_.set_visible(true);
> -					minimap_icon_.set_icon(minimap_image_.get());
> -				} catch (const std::exception& e) {
> -					log("Failed to load the minimap image : %s\n", e.what());
> -				}
> -			}
> -		} else {
> -			label_mapname_.set_text(_("Filename:"));
> -			ta_mapname_.set_text(gamedata.mapname);
> -			label_gametime_.set_text("");
> -			ta_gametime_.set_text("");
> -			label_players_.set_text("");
> -			ta_players_.set_text("");
> -			label_version_.set_text("");
> -			ta_version_.set_text("");
> -			label_win_condition_.set_text("");
> -			ta_win_condition_.set_text("");
> -
> -			minimap_icon_.set_icon(nullptr);
> -			minimap_icon_.set_visible(false);
> -			minimap_icon_.set_no_frame();
> -			minimap_image_.reset();
> -
> -			ta_long_generic_message_.set_visible(true);
> -			ok_.set_enabled(false);
> -		}
> -	} else if (selections > 1) {
> -		label_mapname_.set_text(
> -		   (boost::format(ngettext("Selected %d file:", "Selected %d files:", selections)) %
> -		    selections)
> -		      .str());
> -		ta_long_generic_message_.set_visible(true);
> -		ta_long_generic_message_.set_text(filename_list_string());
> +	ok_.set_enabled(load_or_save_.table().selections().size() == 1);
> +	load_or_save_.delete_button()->set_enabled(load_or_save_.has_selection());
> +	if (load_or_save_.has_selection()) {
> +		load_or_save_.entry_selected();
>  	}
>  }
>  
> -/**
> - * Fill the file list
> - */
>  void FullscreenMenuLoadGame::fill_table() {
> -
> -	games_data_.clear();
> -	table_.clear();
> -
> -	FilenameSet gamefiles;
> -
> -	if (is_replay_) {
> -		gamefiles = filter(g_fs->list_directory(REPLAY_DIR),
> -		                   [](const std::string& fn) { return boost::ends_with(fn, REPLAY_SUFFIX); });
> -	} else {
> -		gamefiles = g_fs->list_directory("save");
> -	}
> -
> -	Widelands::GamePreloadPacket gpdp;
> -
> -	for (const std::string& gamefilename : gamefiles) {
> -		if (gamefilename == "save/campvis" || gamefilename == "save\\campvis") {
> -			continue;
> -		}
> -
> -		SavegameData gamedata;
> -
> -		std::string savename = gamefilename;
> -		if (is_replay_)
> -			savename += WLGF_SUFFIX;
> -
> -		if (!g_fs->file_exists(savename.c_str())) {
> -			continue;
> -		}
> -
> -		gamedata.filename = gamefilename;
> -
> -		try {
> -			Widelands::GameLoader gl(savename.c_str(), game_);
> -			gl.preload_game(gpdp);
> -
> -			gamedata.gametype = gpdp.get_gametype();
> -
> -			if (!is_replay_) {
> -				if (settings_->settings().multiplayer) {
> -					if (gamedata.gametype == GameController::GameType::kSingleplayer) {
> -						continue;
> -					}
> -				} else if (gamedata.gametype > GameController::GameType::kSingleplayer) {
> -					continue;
> -				}
> -			}
> -
> -			gamedata.mapname = gpdp.get_mapname();
> -			gamedata.gametime = gpdp.get_gametime();
> -			gamedata.nrplayers = gpdp.get_number_of_players();
> -			gamedata.version = gpdp.get_version();
> -
> -			gamedata.savetimestamp = gpdp.get_savetimestamp();
> -			time_t t;
> -			time(&t);
> -			struct tm* currenttime = localtime(&t);
> -			// We need to put these into variables because of a sideeffect of the localtime function.
> -			int8_t current_year = currenttime->tm_year;
> -			int8_t current_month = currenttime->tm_mon;
> -			int8_t current_day = currenttime->tm_mday;
> -
> -			struct tm* savedate = localtime(&gamedata.savetimestamp);
> -
> -			if (gamedata.savetimestamp > 0) {
> -				if (savedate->tm_year == current_year && savedate->tm_mon == current_month &&
> -				    savedate->tm_mday == current_day) {  // Today
> -
> -					// Adding the 0 padding in a separate statement so translators won't have to deal
> -					// with it
> -					const std::string minute = (boost::format("%02u") % savedate->tm_min).str();
> -
> -					/** TRANSLATORS: Display date for choosing a savegame/replay */
> -					/** TRANSLATORS: hour:minute */
> -					gamedata.savedatestring =
> -					   (boost::format(_("Today, %1%:%2%")) % savedate->tm_hour % minute).str();
> -				} else if ((savedate->tm_year == current_year && savedate->tm_mon == current_month &&
> -				            savedate->tm_mday == current_day - 1) ||
> -				           (savedate->tm_year == current_year - 1 && savedate->tm_mon == 11 &&
> -				            current_month == 0 && savedate->tm_mday == 31 &&
> -				            current_day == 1)) {  // Yesterday
> -					// Adding the 0 padding in a separate statement so translators won't have to deal
> -					// with it
> -					const std::string minute = (boost::format("%02u") % savedate->tm_min).str();
> -
> -					/** TRANSLATORS: Display date for choosing a savegame/replay */
> -					/** TRANSLATORS: hour:minute */
> -					gamedata.savedatestring =
> -					   (boost::format(_("Yesterday, %1%:%2%")) % savedate->tm_hour % minute).str();
> -				} else {  // Older
> -
> -					/** TRANSLATORS: Display date for choosing a savegame/replay */
> -					/** TRANSLATORS: month day, year */
> -					gamedata.savedatestring =
> -					   (boost::format(_("%2% %1%, %3%")) % savedate->tm_mday %
> -					    localize_month(savedate->tm_mon) % (1900 + savedate->tm_year))
> -					      .str();
> -				}
> -			}
> -
> -			gamedata.wincondition = _(gpdp.get_localized_win_condition());
> -			gamedata.minimap_path = gpdp.get_minimap_path();
> -			games_data_.push_back(gamedata);
> -
> -			UI::Table<uintptr_t const>::EntryRecord& te = table_.add(games_data_.size() - 1);
> -			te.set_string(0, gamedata.savedatestring);
> -
> -			if (is_replay_ || settings_->settings().multiplayer) {
> -				std::string gametypestring;
> -				switch (gamedata.gametype) {
> -				case GameController::GameType::kSingleplayer:
> -					/** TRANSLATORS: "Single Player" entry in the Game Mode table column. */
> -					/** TRANSLATORS: "Keep this to 6 letters maximum. */
> -					/** TRANSLATORS: A tooltip will explain the abbreviation. */
> -					/** TRANSLATORS: Make sure that this translation is consistent with the tooltip. */
> -					gametypestring = _("SP");
> -					break;
> -				case GameController::GameType::kNetHost:
> -					/** TRANSLATORS: "Multiplayer Host" entry in the Game Mode table column. */
> -					/** TRANSLATORS: "Keep this to 2 letters maximum. */
> -					/** TRANSLATORS: A tooltip will explain the abbreviation. */
> -					/** TRANSLATORS: Make sure that this translation is consistent with the tooltip. */
> -					/** TRANSLATORS: %1% is the number of players */
> -					gametypestring =
> -					   (boost::format(_("H (%1%)")) % static_cast<unsigned int>(gamedata.nrplayers))
> -					      .str();
> -					break;
> -				case GameController::GameType::kNetClient:
> -					/** TRANSLATORS: "Multiplayer" entry in the Game Mode table column. */
> -					/** TRANSLATORS: "Keep this to 2 letters maximum. */
> -					/** TRANSLATORS: A tooltip will explain the abbreviation. */
> -					/** TRANSLATORS: Make sure that this translation is consistent with the tooltip. */
> -					/** TRANSLATORS: %1% is the number of players */
> -					gametypestring =
> -					   (boost::format(_("MP (%1%)")) % static_cast<unsigned int>(gamedata.nrplayers))
> -					      .str();
> -					break;
> -				case GameController::GameType::kReplay:
> -					gametypestring = "";
> -					break;
> -				case GameController::GameType::kUndefined:
> -					NEVER_HERE();
> -				}
> -				te.set_string(1, gametypestring);
> -				te.set_string(2, map_filename(gamedata.filename, gamedata.mapname));
> -			} else {
> -				te.set_string(1, map_filename(gamedata.filename, gamedata.mapname));
> -			}
> -		} catch (const WException& e) {
> -			//  we simply skip illegal entries
> -			gamedata.errormessage =
> -			   ((boost::format("%s\n\n%s\n\n%s"))
> -			    /** TRANSLATORS: Error message introduction for when an old savegame can't be loaded */
> -			    % _("This file has the wrong format and can’t be loaded."
> -			        " Maybe it was created with an older version of Widelands.")
> -			    /** TRANSLATORS: This text is on a separate line with an error message below */
> -			    % _("Error message:") % e.what())
> -			      .str();
> -
> -			const std::string fs_filename =
> -			   FileSystem::filename_without_ext(gamedata.filename.c_str());
> -			gamedata.mapname = fs_filename;
> -			games_data_.push_back(gamedata);
> -
> -			UI::Table<uintptr_t const>::EntryRecord& te = table_.add(games_data_.size() - 1);
> -			te.set_string(0, "");
> -			if (is_replay_ || settings_->settings().multiplayer) {
> -				te.set_string(1, "");
> -				/** TRANSLATORS: Prefix for incompatible files in load game screens */
> -				te.set_string(2, (boost::format(_("Incompatible: %s")) % fs_filename).str());
> -			} else {
> -				te.set_string(1, (boost::format(_("Incompatible: %s")) % fs_filename).str());
> -			}
> -		}
> -	}
> -	table_.sort();
> -
> -	if (table_.size()) {
> -		table_.select(0);
> -	}
> -	set_has_selection();
> +	load_or_save_.fill_table();
> +}
> +
> +const std::string& FullscreenMenuLoadGame::filename() const {
> +	return filename_;
>  }
>  
>  bool FullscreenMenuLoadGame::handle_key(bool down, SDL_Keysym code) {
> 
> === modified file 'src/ui_fsmenu/loadgame.h'
> --- src/ui_fsmenu/loadgame.h	2017-06-24 10:38:19 +0000
> +++ src/ui_fsmenu/loadgame.h	2017-10-02 17:28:52 +0000
> @@ -77,55 +40,36 @@
>  	                       GameController* gc = nullptr,
>  	                       bool is_replay = false);
>  
> -	const std::string& filename() {
> -		return filename_;
> -	}
> -
> -	void think() override;
> +	/// The currently selected filename
> +	const std::string& filename() const;
>  
>  	bool handle_key(bool down, SDL_Keysym code) override;
>  
>  protected:
> +	/// Sets the current selected filename and ends the modal screen with 'Ok' status.
>  	void clicked_ok() override;
> +
> +	/// Update button status and game details
>  	void entry_selected() override;
> +
> +	/// Fill load_or_save_'s table
>  	void fill_table() override;
>  
>  private:
>  	void layout() override;
>  
> -	/// Updates buttons and text labels and returns whether a table entry is selected.
> -	bool set_has_selection();
> -	bool compare_date_descending(uint32_t, uint32_t);
> -	void clicked_delete();
> -	std::string filename_list_string();
> -
> -	UI::Table<uintptr_t const> table_;
> -
> -	bool is_replay_;
> -
> +	UI::Box main_box_;
> +	UI::Box info_box_;
>  	UI::Textarea title_;
> -	UI::Textarea label_mapname_;
> -	UI::MultilineTextarea ta_mapname_;  // Multiline for long names
> -	UI::Textarea label_gametime_;
> -	UI::MultilineTextarea ta_gametime_;  // Multiline because we want tooltips
> -	UI::Textarea label_players_;
> -	UI::MultilineTextarea ta_players_;
> -	UI::Textarea label_version_;
> -	UI::Textarea ta_version_;
> -	UI::Textarea label_win_condition_;
> -	UI::MultilineTextarea ta_win_condition_;
> -
> -	UI::Button delete_;
> -
> -	UI::MultilineTextarea ta_long_generic_message_;
> -
> -	int32_t const minimap_y_, minimap_w_, minimap_h_;
> -	UI::Icon minimap_icon_;
> -	std::unique_ptr<const Image> minimap_image_;
> -
> -	std::vector<SavegameData> games_data_;
> +
> +	LoadOrSaveGame load_or_save_;
> +
> +	UI::Button* delete_;
> +	// TODO(GunChleoc): Get rid of this hack once everything is 100% box layout

Can this be done now? At least I interpreted your merge-commit message that way.

> +	UI::Panel* button_spacer_;
>  	std::string filename_;
>  
> +	bool is_replay_;
>  	Widelands::Game& game_;
>  	GameSettingsProvider* settings_;
>  	GameController* ctrl_;
> 
> === modified file 'src/wui/game_main_menu_save_game.cc'
> --- src/wui/game_main_menu_save_game.cc	2017-08-19 22:22:20 +0000
> +++ src/wui/game_main_menu_save_game.cc	2017-10-02 17:28:52 +0000
> @@ -258,15 +187,12 @@
>  	std::string const filename_;
>  };
>  
> -/**
> - * Called when the Ok button is clicked or the Return key pressed in the edit box.
> - */
>  void GameMainMenuSaveGame::ok() {
> -	if (editbox_.text().empty())
> +	if (filename_editbox_.text().empty())

Maybe some curly brackets? ;-)

>  		return;
>  
>  	std::string const complete_filename =
> -	   igbase().game().save_handler().create_file_name(curdir_, editbox_.text());
> +	   igbase().game().save_handler().create_file_name(curdir_, filename_editbox_.text());
>  
>  	//  Check if file exists. If it does, show a warning.
>  	if (g_fs->file_exists(complete_filename)) {
> 
> === added file 'src/wui/load_or_save_game.cc'
> --- src/wui/load_or_save_game.cc	1970-01-01 00:00:00 +0000
> +++ src/wui/load_or_save_game.cc	2017-10-02 17:28:52 +0000
> @@ -0,0 +1,487 @@
> +/*
> + * Copyright (C) 2002-2016 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/load_or_save_game.h"
> +
> +#include <ctime>
> +
> +#include <boost/algorithm/string.hpp>
> +#include <boost/format.hpp>
> +
> +#include "base/i18n.h"
> +#include "base/log.h"
> +#include "base/time_string.h"
> +#include "game_io/game_loader.h"
> +#include "game_io/game_preload_packet.h"
> +#include "graphic/font_handler1.h"
> +#include "helper.h"
> +#include "io/filesystem/layered_filesystem.h"
> +#include "logic/game.h"
> +#include "logic/game_controller.h"
> +#include "logic/game_settings.h"
> +#include "logic/replay.h"
> +#include "ui_basic/messagebox.h"
> +
> +namespace {
> +// This function concatenates the filename and localized map name for a savegame/replay.
> +// If the filename starts with the map name, the map name is omitted.
> +// It also prefixes autosave files with a numbered and localized "Autosave" prefix.
> +std::string
> +map_filename(const std::string& filename, const std::string& mapname, bool localize_autosave) {
> +	std::string result = FileSystem::filename_without_ext(filename.c_str());
> +
> +	if (localize_autosave && boost::starts_with(result, "wl_autosave")) {
> +		std::vector<std::string> autosave_name;
> +		boost::split(autosave_name, result, boost::is_any_of("_"));
> +		if (autosave_name.empty() || autosave_name.size() < 3) {
> +			/** TRANSLATORS: %1% is a map's name. */
> +			result = (boost::format(_("Autosave: %1%")) % mapname).str();
> +		} else {
> +			/** TRANSLATORS: %1% is a number, %2% a map's name. */
> +			result = (boost::format(_("Autosave %1%: %2%")) % autosave_name.back() % mapname).str();
> +		}
> +	} else if (!(boost::starts_with(result, mapname))) {
> +		/** TRANSLATORS: %1% is a filename, %2% a map's name. */
> +		result = (boost::format(pgettext("filename_mapname", "%1%: %2%")) % result % mapname).str();
> +	}
> +	return result;
> +}
> +}  // namespace
> +
> +LoadOrSaveGame::LoadOrSaveGame(UI::Panel* parent,
> +                               Widelands::Game& g,
> +                               FileType filetype,
> +                               GameDetails::Style style,
> +                               bool localize_autosave)
> +   : parent_(parent),
> +     table_box_(new UI::Box(parent, 0, 0, UI::Box::Vertical)),
> +     table_(table_box_,
> +            0,
> +            0,
> +            0,
> +            0,
> +            g_gr->images().get(style == GameDetails::Style::kFsMenu ? "images/ui_basic/but3.png" :
> +                                                                      "images/ui_basic/but1.png"),
> +            UI::TableRows::kMultiDescending),
> +     filetype_(filetype),
> +     localize_autosave_(localize_autosave),
> +     // Savegame description
> +     game_details_(
> +        parent,
> +        style,
> +        filetype_ == FileType::kReplay ? GameDetails::Mode::kReplay : GameDetails::Mode::kSavegame),
> +     delete_(new UI::Button(game_details()->button_box(),
> +                            "delete",
> +                            0,
> +                            0,
> +                            0,
> +                            0,
> +                            g_gr->images().get("images/ui_basic/but0.png"),
> +                            _("Delete"))),
> +     game_(g) {
> +	table_.add_column(130, _("Save Date"), _("The date this game was saved"), UI::Align::kLeft);
> +	if (filetype_ != FileType::kGameSinglePlayer) {
> +		std::vector<std::string> modes;
> +		if (filetype_ == FileType::kReplay) {
> +			/** TRANSLATORS: Tooltip for the "Mode" column when choosing a game/replay to load. */
> +			/** TRANSLATORS: Make sure that you keep consistency in your translation. */
> +			modes.push_back(_("SP = Single Player"));
> +		}
> +		/** TRANSLATORS: Tooltip for the "Mode" column when choosing a game/replay to load. */
> +		/** TRANSLATORS: Make sure that you keep consistency in your translation. */
> +		modes.push_back(_("MP = Multiplayer"));
> +		/** TRANSLATORS: Tooltip for the "Mode" column when choosing a game/replay to load. */
> +		/** TRANSLATORS: Make sure that you keep consistency in your translation. */
> +		modes.push_back(_("H = Multiplayer (Host)"));
> +		const std::string mode_tooltip_1 =
> +		   /** TRANSLATORS: Tooltip for the "Mode" column when choosing a game/replay to load. */
> +		   /** TRANSLATORS: %s is a list of game modes. */
> +		   ((boost::format(_("Game Mode: %s.")) %
> +		     i18n::localize_list(modes, i18n::ConcatenateWith::COMMA)))
> +		      .str();
> +		const std::string mode_tooltip_2 = _("Numbers are the number of players.");
> +
> +		table_.add_column(
> +		   65,
> +		   /** TRANSLATORS: Game Mode table column when choosing a game/replay to load. */
> +		   /** TRANSLATORS: Keep this to 5 letters maximum. */
> +		   /** TRANSLATORS: A tooltip will explain if you need to use an abbreviation. */
> +		   _("Mode"), (boost::format("%s %s") % mode_tooltip_1 % mode_tooltip_2).str());
> +	}
> +	table_.add_column(0, _("Description"),
> +	                  filetype_ == FileType::kReplay ?
> +	                     _("Map name (start of replay)") :
> +	                     _("The filename that the game was saved under followed by the map’s name, "
> +	                       "or the map’s name followed by the last objective achieved."),
> +	                  UI::Align::kLeft, UI::TableColumnType::kFlexible);
> +	table_.set_column_compare(
> +	   0, boost::bind(&LoadOrSaveGame::compare_date_descending, this, _1, _2));
> +	table_.set_sort_column(0);
> +	table_.focus();
> +	fill_table();
> +
> +	table_box_->add(&table_, UI::Box::Resizing::kExpandBoth);
> +
> +	game_details_.button_box()->add(delete_, UI::Box::Resizing::kAlign, UI::Align::kLeft);
> +	delete_->set_enabled(false);
> +	delete_->sigclicked.connect(boost::bind(&LoadOrSaveGame::clicked_delete, boost::ref(*this)));
> +}
> +
> +const std::string LoadOrSaveGame::filename_list_string() const {
> +	boost::format message;
> +	for (const uint32_t index : table_.selections()) {
> +		const SavegameData& gamedata = games_data_[table_.get(table_.get_record(index))];
> +
> +		if (gamedata.errormessage.empty()) {
> +			std::vector<std::string> listme;
> +			listme.push_back(richtext_escape(gamedata.mapname));
> +			listme.push_back(gamedata.savedonstring);
> +			message = (boost::format("%s\n%s") % message %
> +			           i18n::localize_list(listme, i18n::ConcatenateWith::COMMA));
> +		} else {
> +			message = boost::format("%s\n%s") % message % richtext_escape(gamedata.filename);
> +		}
> +	}
> +	return message.str();
> +}
> +
> +bool LoadOrSaveGame::compare_date_descending(uint32_t rowa, uint32_t rowb) const {
> +	const SavegameData& r1 = games_data_[table_[rowa]];
> +	const SavegameData& r2 = games_data_[table_[rowb]];
> +
> +	return r1.savetimestamp < r2.savetimestamp;
> +}
> +
> +const SavegameData* LoadOrSaveGame::entry_selected() {
> +	SavegameData* result = new SavegameData();
> +	size_t selections = table_.selections().size();
> +	if (selections == 1) {
> +		delete_->set_tooltip(
> +		   filetype_ == FileType::kReplay ?
> +		      /** TRANSLATORS: Tooltip for the delete button. The user has selected 1 file */
> +		      _("Delete this replay") :
> +		      /** TRANSLATORS: Tooltip for the delete button. The user has selected 1 file */
> +		      _("Delete this game"));
> +		result = &games_data_[table_.get_selected()];
> +	} else if (selections > 1) {
> +		delete_->set_tooltip(
> +		   filetype_ == FileType::kReplay ?
> +		      /** TRANSLATORS: Tooltip for the delete button. The user has selected multiple files */
> +		      _("Delete these replays") :
> +		      /** TRANSLATORS: Tooltip for the delete button. The user has selected multiple files */
> +		      _("Delete these games"));
> +		result->mapname =
> +		   (boost::format(ngettext("Selected %d file:", "Selected %d files:", selections)) %
> +		    selections)
> +		      .str();
> +		result->filename_list = filename_list_string();
> +	} else {
> +		delete_->set_tooltip("");
> +	}
> +	game_details_.update(*result);
> +	return result;
> +}
> +
> +bool LoadOrSaveGame::has_selection() const {
> +	return table_.has_selection();
> +}
> +
> +void LoadOrSaveGame::clear_selections() {
> +	table_.clear_selections();
> +	game_details_.clear();
> +}
> +
> +void LoadOrSaveGame::select_by_name(const std::string& name) {
> +	table_.clear_selections();
> +	for (size_t idx = 0; idx < table_.size(); ++idx) {
> +		const SavegameData& gamedata = games_data_[table_[idx]];
> +		if (name == gamedata.filename) {
> +			table_.select(idx);
> +			return;
> +		}
> +	}
> +}
> +
> +UI::Table<uintptr_t const>& LoadOrSaveGame::table() {
> +	return table_;
> +}
> +
> +UI::Box* LoadOrSaveGame::table_box() {
> +	return table_box_;
> +}
> +
> +GameDetails* LoadOrSaveGame::game_details() {
> +	return &game_details_;
> +}
> +
> +const std::string LoadOrSaveGame::get_filename(int index) const {
> +	return games_data_[table_.get(table_.get_record(index))].filename;
> +}
> +
> +void LoadOrSaveGame::clicked_delete() {
> +	if (!has_selection()) {
> +		return;
> +	}
> +	std::set<uint32_t> selections = table().selections();
> +	const SavegameData& gamedata = *entry_selected();
> +	size_t no_selections = selections.size();
> +	std::string header = "";
> +	if (filetype_ == FileType::kReplay) {
> +		header = no_selections == 1 ?
> +		            _("Do you really want to delete this replay?") :
> +		            /** TRANSLATORS: Used with multiple replays, 1 replay has a separate string. */
> +		            (boost::format(ngettext("Do you really want to delete this %d replay?",
> +		                                    "Do you really want to delete these %d replays?",
> +		                                    no_selections)) %
> +		             no_selections)
> +		               .str();
> +	} else {
> +		header = no_selections == 1 ?
> +		            _("Do you really want to delete this game?") :
> +		            /** TRANSLATORS: Used with multiple games, 1 game has a separate string. */
> +		            (boost::format(ngettext("Do you really want to delete this %d game?",
> +		                                    "Do you really want to delete these %d games?",
> +		                                    no_selections)) %
> +		             no_selections)
> +		               .str();
> +	}
> +	std::string message = no_selections > 1 ? gamedata.filename_list : gamedata.filename;
> +	message = (boost::format("%s\n%s") % header % message).str();
> +
> +	bool do_delete = SDL_GetModState() & KMOD_CTRL;
> +	if (!do_delete) {
> +		UI::WLMessageBox confirmationBox(
> +		   parent_, ngettext("Confirm deleting file", "Confirm deleting files", no_selections),
> +		   message, UI::WLMessageBox::MBoxType::kOkCancel);
> +		do_delete = confirmationBox.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kOk;
> +	}
> +	if (do_delete) {
> +		for (const uint32_t index : selections) {
> +			const std::string& deleteme = get_filename(index);
> +			g_fs->fs_unlink(deleteme);
> +			if (filetype_ == FileType::kReplay) {
> +				g_fs->fs_unlink(deleteme + WLGF_SUFFIX);
> +			}
> +		}
> +		fill_table();
> +	}
> +}
> +
> +UI::Button* LoadOrSaveGame::delete_button() {
> +	return delete_;
> +}
> +
> +void LoadOrSaveGame::fill_table() {
> +
> +	games_data_.clear();
> +	table_.clear();
> +
> +	FilenameSet gamefiles;
> +
> +	if (filetype_ == FileType::kReplay) {
> +		gamefiles = filter(g_fs->list_directory(REPLAY_DIR),
> +		                   [](const std::string& fn) { return boost::ends_with(fn, REPLAY_SUFFIX); });
> +	} else {
> +		gamefiles = g_fs->list_directory("save");
> +	}

Optional: Why a constant for the REPLAY_DIR but a string for the save dir? Why no filtering for the possible save files?
(Does not have to be fixed in this branch since its old code.)

> +
> +	Widelands::GamePreloadPacket gpdp;
> +
> +	for (const std::string& gamefilename : gamefiles) {
> +		if (gamefilename == "save/campvis" || gamefilename == "save\\campvis") {

Optional: Maybe use FileSystem::fix_cross_file() instead of this two cases.

> +			continue;
> +		}
> +
> +		SavegameData gamedata;
> +
> +		std::string savename = gamefilename;
> +		if (filetype_ == FileType::kReplay)
> +			savename += WLGF_SUFFIX;
> +
> +		if (!g_fs->file_exists(savename.c_str())) {
> +			continue;
> +		}
> +
> +		gamedata.filename = gamefilename;
> +
> +		try {
> +			Widelands::GameLoader gl(savename.c_str(), game_);
> +			gl.preload_game(gpdp);
> +
> +			gamedata.gametype = gpdp.get_gametype();
> +
> +			if (filetype_ != FileType::kReplay) {
> +				if (filetype_ == FileType::kGame) {
> +					if (gamedata.gametype == GameController::GameType::kReplay) {
> +						continue;
> +					}
> +				} else if (filetype_ == FileType::kGameMultiPlayer) {
> +					if (gamedata.gametype == GameController::GameType::kSingleplayer) {
> +						continue;
> +					}
> +				} else if (gamedata.gametype > GameController::GameType::kSingleplayer) {
> +					continue;
> +				}
> +			}
> +
> +			gamedata.set_mapname(gpdp.get_mapname());
> +			gamedata.set_gametime(gpdp.get_gametime());
> +			gamedata.set_nrplayers(gpdp.get_number_of_players());
> +			gamedata.version = gpdp.get_version();
> +
> +			gamedata.savetimestamp = gpdp.get_savetimestamp();
> +			time_t t;
> +			time(&t);
> +			struct tm* currenttime = localtime(&t);
> +			// We need to put these into variables because of a sideeffect of the localtime function.
> +			int8_t current_year = currenttime->tm_year;
> +			int8_t current_month = currenttime->tm_mon;
> +			int8_t current_day = currenttime->tm_mday;
> +
> +			struct tm* savedate = localtime(&gamedata.savetimestamp);
> +
> +			if (gamedata.savetimestamp > 0) {
> +				if (savedate->tm_year == current_year && savedate->tm_mon == current_month &&
> +				    savedate->tm_mday == current_day) {  // Today
> +
> +					// Adding the 0 padding in a separate statement so translators won't have to deal
> +					// with it
> +					const std::string minute = (boost::format("%02u") % savedate->tm_min).str();
> +
> +					gamedata.savedatestring =
> +					   /** TRANSLATORS: Display date for choosing a savegame/replay. Placeholders are:
> +					      hour:minute */
> +					   (boost::format(_("Today, %1%:%2%")) % savedate->tm_hour % minute).str();
> +					gamedata.savedonstring =
> +					   /** TRANSLATORS: Display date for choosing a savegame/replay. Placeholders are:
> +					      hour:minute. This is part of a list. */
> +					   (boost::format(_("saved today at %1%:%2%")) % savedate->tm_hour % minute).str();
> +				} else if ((savedate->tm_year == current_year && savedate->tm_mon == current_month &&
> +				            savedate->tm_mday == current_day - 1) ||
> +				           (savedate->tm_year == current_year - 1 && savedate->tm_mon == 11 &&
> +				            current_month == 0 && savedate->tm_mday == 31 &&
> +				            current_day == 1)) {  // Yesterday
> +					// Adding the 0 padding in a separate statement so translators won't have to deal
> +					// with it
> +					const std::string minute = (boost::format("%02u") % savedate->tm_min).str();
> +
> +					gamedata.savedatestring =
> +					   /** TRANSLATORS: Display date for choosing a savegame/replay. Placeholders are:
> +					      hour:minute */
> +					   (boost::format(_("Yesterday, %1%:%2%")) % savedate->tm_hour % minute).str();
> +					gamedata.savedonstring =
> +					   /** TRANSLATORS: Display date for choosing a savegame/replay. Placeholders are:
> +					      hour:minute. This is part of a list. */
> +					   (boost::format(_("saved yesterday at %1%:%2%")) % savedate->tm_hour % minute)
> +					      .str();
> +				} else {  // Older
> +					gamedata.savedatestring =
> +					   /** TRANSLATORS: Display date for choosing a savegame/replay. Placeholders are:
> +					      month day, year */
> +					   (boost::format(_("%2% %1%, %3%")) % savedate->tm_mday %
> +					    localize_month(savedate->tm_mon) % (1900 + savedate->tm_year))
> +					      .str();
> +					gamedata.savedonstring =
> +					   /** TRANSLATORS: Display date for choosing a savegame/replay. Placeholders are:
> +					      month day, year. This is part of a list. */
> +					   (boost::format(_("saved on %2% %1%, %3%")) % savedate->tm_mday %
> +					    localize_month(savedate->tm_mon) % (1900 + savedate->tm_year))
> +					      .str();
> +				}
> +			}
> +
> +			gamedata.wincondition = gpdp.get_localized_win_condition();
> +			gamedata.minimap_path = gpdp.get_minimap_path();
> +			games_data_.push_back(gamedata);
> +
> +			UI::Table<uintptr_t const>::EntryRecord& te = table_.add(games_data_.size() - 1);
> +			te.set_string(0, gamedata.savedatestring);
> +
> +			if (filetype_ != FileType::kGameSinglePlayer) {
> +				std::string gametypestring;
> +				switch (gamedata.gametype) {
> +				case GameController::GameType::kSingleplayer:
> +					/** TRANSLATORS: "Single Player" entry in the Game Mode table column. */
> +					/** TRANSLATORS: "Keep this to 6 letters maximum. */
> +					/** TRANSLATORS: A tooltip will explain the abbreviation. */
> +					/** TRANSLATORS: Make sure that this translation is consistent with the tooltip. */
> +					gametypestring = _("SP");
> +					break;
> +				case GameController::GameType::kNetHost:
> +					/** TRANSLATORS: "Multiplayer Host" entry in the Game Mode table column. */
> +					/** TRANSLATORS: "Keep this to 2 letters maximum. */
> +					/** TRANSLATORS: A tooltip will explain the abbreviation. */
> +					/** TRANSLATORS: Make sure that this translation is consistent with the tooltip. */
> +					/** TRANSLATORS: %1% is the number of players */
> +					gametypestring = (boost::format(_("H (%1%)")) % gamedata.nrplayers).str();
> +					break;
> +				case GameController::GameType::kNetClient:
> +					/** TRANSLATORS: "Multiplayer" entry in the Game Mode table column. */
> +					/** TRANSLATORS: "Keep this to 2 letters maximum. */
> +					/** TRANSLATORS: A tooltip will explain the abbreviation. */
> +					/** TRANSLATORS: Make sure that this translation is consistent with the tooltip. */
> +					/** TRANSLATORS: %1% is the number of players */
> +					gametypestring = (boost::format(_("MP (%1%)")) % gamedata.nrplayers).str();
> +					break;
> +				case GameController::GameType::kReplay:
> +					gametypestring = "";
> +					break;
> +				case GameController::GameType::kUndefined:
> +					NEVER_HERE();
> +				}
> +				te.set_string(1, gametypestring);
> +				if (filetype_ == FileType::kReplay) {
> +					te.set_string(2, (boost::format(pgettext("mapname_gametime", "%1% (%2%)")) %
> +					                  gamedata.mapname % gamedata.gametime)
> +					                    .str());
> +				} else {
> +					te.set_string(
> +					   2, map_filename(gamedata.filename, gamedata.mapname, localize_autosave_));
> +				}
> +			} else {
> +				te.set_string(1, map_filename(gamedata.filename, gamedata.mapname, localize_autosave_));
> +			}
> +		} catch (const WException& e) {
> +			std::string errormessage = e.what();
> +			boost::replace_all(errormessage, "\n", "<br>");
> +			gamedata.errormessage =
> +			   ((boost::format("<p>%s</p><p>%s</p><p>%s</p>"))
> +			    /** TRANSLATORS: Error message introduction for when an old savegame can't be loaded */
> +			    % _("This file has the wrong format and can’t be loaded."
> +			        " Maybe it was created with an older version of Widelands.")
> +			    /** TRANSLATORS: This text is on a separate line with an error message below */
> +			    % _("Error message:") % errormessage)
> +			      .str();
> +
> +			gamedata.mapname = FileSystem::filename_without_ext(gamedata.filename.c_str());
> +			games_data_.push_back(gamedata);
> +
> +			UI::Table<uintptr_t const>::EntryRecord& te = table_.add(games_data_.size() - 1);
> +			te.set_string(0, "");
> +			if (filetype_ != FileType::kGameSinglePlayer) {
> +				te.set_string(1, "");
> +				/** TRANSLATORS: Prefix for incompatible files in load game screens */
> +				te.set_string(2, (boost::format(_("Incompatible: %s")) % gamedata.mapname).str());
> +			} else {
> +				te.set_string(1, (boost::format(_("Incompatible: %s")) % gamedata.mapname).str());
> +			}
> +		}
> +	}
> +	table_.sort();
> +}


-- 
https://code.launchpad.net/~widelands-dev/widelands/savegame-menu/+merge/322924
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/savegame-menu.


References