Merge lp:~widelands-dev/widelands/smaller_building_statistics into lp:widelands

[GunChleoc]

Building Statistics now only show relevant buildings
- Military sites not belonging to the tribe are omitted, unless the payer currently owns one
- Seafaring and allowed buildings are dynamically checked
- Show "under construction" navigation for buildings being enhanced
- Added "needs_seafaring" to port buildings

Fixes the attached bug.

Since the Frisians were added, the building statistics got too big due to all the military sites - the navigation became impossible to reach on 800x600 resolution without moving the window around. So, I have implemented a dynamic approach that will only show allowed and currently owned buildings.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3357. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/363552376.
Appveyor build 3163. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_smaller_building_statistics-3163.

[Klaus Halfmann (klaus-halfmann)]

One question inline, will take a closer look later.

just brached this and will have a look.

Not sure who will be the payer for the other tribes military sites ;-) ?

[Klaus Halfmann (klaus-halfmann)]

Some more commetns inline, I think this deserves some time in the debugger.
Mostly for me to better understand the widelands internal structures.

Maybe this will make things slower, Not sure about this, lets seee.

[GunChleoc (gunchleoc)]

Thanks for the review :)

And you were right about the efficiency concerns, the seafaring check is causing a slowdown. I have reduced it to be checked once every 2 minutes.

[GunChleoc (gunchleoc)]

I have changed my mind about the seafaring check - since this is so expensive, we should generally only recalculate it on map changes. I'll create another branch for it, to be merged before this branch.

[Klaus Halfmann (klaus-halfmann)]

Mhh, scripts might want to add seafaring later in some scenario.
Hopefully this will not break such scripts.

Anyway, I do not have that much time today for a debugging session,
lets see what I can do perhaps next weekend.

[GunChleoc (gunchleoc)]

I know - I have taken care of it. Branch is ready; I'll put it up for review as soon as Launchpad has finished parsing it.

https://code.launchpad.net/~widelands-dev/widelands/allows_seafaring_performance

review: Needs Resubmitting

[Klaus Halfmann (klaus-halfmann)]

fetched it again, no idea if I have time at the weekend.

[Klaus Halfmann (klaus-halfmann)]

I did some testplaying with a debugger now and gained some coverage of that code:
 * Everything worked as expected so far.
 * The Handling of the forward / backward buttons has room for improvement:
  * no need so use large switch blocks -> call the repective function directly.
  * Maybe I can do some refactoring there, later.
 * I could not trigger foreign_tribe_building_is_valid, yet, no Enemy t conquer found so far :-)
 * I need a another map _with_ seafaring, my current one does not allow this ....
 * I was unable to trigger BuildingStatisticsMenu::reset()
   no idea why this was never called. Gun: any idea?

[Klaus Halfmann (klaus-halfmann)]

I testplayed this now on a seafaring Map:
 * I was unable to reach the following lies of code:
  * BuildingStatisticsMenu::foreign_tribe_building_is_valid()
    ...
    if (descr.type() == MapObjectType::CONSTRUCTIONSITE ||
 descr.type() == MapObjectType::DISMANTLESITE) {
 return false;
    // I think this can never be reached I can not build a forign building
    // A dismantled site cannot be reached
  * BuildingStatisticsMenu::reset()
     ...
     if (building_buttons_[current_building_type_] != nullptr) {
 set_current_building_type(current_building_type_); // no idea what this shall be
     } else {
 * I found that some missing soldiers Arrows where incorrect for a foreign military building.

 * I played on a lot of different resolutions

Despite these Issues this can go in:

@bunnybot merge

review: Approve (compile, revied, debug for coverage, testplay)

[GunChleoc (gunchleoc)]

You trigger those by conquering a foreign military site and then again by destroying it. I ran a setup on Golden Peninsula where I gave the opponent the "Village" starting condition. so that I could easily conquer something.

=== modified file 'data/tribes/buildings/warehouses/atlanteans/port/init.lua'
--- data/tribes/buildings/warehouses/atlanteans/port/init.lua	2017-09-02 19:53:03 +0000
+++ data/tribes/buildings/warehouses/atlanteans/port/init.lua	2018-04-12 05:33:14 +0000
@@ -8,6 +8,7 @@
    helptext_script = dirname .. "helptexts.lua",
    icon = dirname .. "menu.png",
    size = "port",
+   needs_seafaring = true,
 
    buildcost = {
       log = 3,
 
=== modified file 'data/tribes/buildings/warehouses/barbarians/port/init.lua'
--- data/tribes/buildings/warehouses/barbarians/port/init.lua	2017-09-02 19:53:03 +0000
+++ data/tribes/buildings/warehouses/barbarians/port/init.lua	2018-04-12 05:33:14 +0000
@@ -8,6 +8,7 @@
    helptext_script = dirname .. "helptexts.lua",
    icon = dirname .. "menu.png",
    size = "port",
+   needs_seafaring = true,
 
    buildcost = {
       log = 3,
 
=== modified file 'data/tribes/buildings/warehouses/empire/port/init.lua'
--- data/tribes/buildings/warehouses/empire/port/init.lua	2017-09-02 19:53:03 +0000
+++ data/tribes/buildings/warehouses/empire/port/init.lua	2018-04-12 05:33:14 +0000
@@ -8,6 +8,7 @@
    helptext_script = dirname .. "helptexts.lua",
    icon = dirname .. "menu.png",
    size = "port",
+   needs_seafaring = true,
 
    buildcost = {
       log = 3,
 
=== modified file 'data/tribes/buildings/warehouses/frisians/port/init.lua'
--- data/tribes/buildings/warehouses/frisians/port/init.lua	2018-02-16 14:53:04 +0000
+++ data/tribes/buildings/warehouses/frisians/port/init.lua	2018-04-12 05:33:14 +0000
@@ -8,6 +8,7 @@
    helptext_script = dirname .. "helptexts.lua",
    icon = dirname .. "menu.png",
    size = "port",
+   needs_seafaring = true,
 
    buildcost = {
       brick = 6,
 
=== modified file 'src/ui_basic/tabpanel.cc'
--- src/ui_basic/tabpanel.cc	2018-04-07 16:59:00 +0000
+++ src/ui_basic/tabpanel.cc	2018-04-12 05:33:14 +0000
@@ -225,6 +225,9 @@
 }
 
 bool TabPanel::remove_last_tab(const std::string& tabname) {
+	if (tabs_.empty()) {
+		return false;
+	}
 	if (tabs_.back()->get_name() == tabname) {
 		tabs_.pop_back();
 		if (active_ > tabs_.size() - 1) {
 
=== modified file 'src/wui/building_statistics_menu.cc'
--- src/wui/building_statistics_menu.cc	2018-04-07 16:59:00 +0000
+++ src/wui/building_statistics_menu.cc	2018-04-12 05:33:14 +0000
@@ -39,9 +39,7 @@
 constexpr int kButtonRowHeight = kButtonHeight + kMargin;
 constexpr int kLabelHeight = 18;
 constexpr int kLabelFontSize = 12;
-constexpr int kTabHeight = 35 + 5 * (kBuildGridCellHeight + kLabelHeight + kLabelHeight);
 constexpr int32_t kWindowWidth = kColumns * kBuildGridCellWidth;
-constexpr int32_t kWindowHeight = kTabHeight + kMargin + 4 * kButtonRowHeight;
 
 constexpr int32_t kUpdateTimeInGametimeMs = 1000;  //  1 second, gametime
 
@@ -63,7 +61,7 @@
                       "building_statistics",
                       &registry,
                       kWindowWidth,
-                      kWindowHeight,
+                      100,
                       _("Building Statistics")),
      tab_panel_(this, g_gr->images().get("images/ui_basic/but1.png")),
      navigation_panel_(this, 0, 0, kWindowWidth, 4 * kButtonRowHeight),
@@ -118,119 +116,12 @@
                             "",
                             UI::Align::kRight),
      low_production_(33),
-     has_selection_(false) {
-
-	for (int i = 0; i < kNoOfBuildingTabs; ++i) {
-		row_counters_[i] = 0;
-		tabs_[i] = new UI::Box(&tab_panel_, 0, 0, UI::Box::Vertical);
-	}
-
-	tab_panel_.add("building_stats_small",
-	               g_gr->images().get("images/wui/fieldaction/menu_tab_buildsmall.png"),
-	               tabs_[BuildingTab::Small], _("Small buildings"));
-	tab_panel_.add("building_stats_medium",
-	               g_gr->images().get("images/wui/fieldaction/menu_tab_buildmedium.png"),
-	               tabs_[BuildingTab::Medium], _("Medium buildings"));
-	tab_panel_.add("building_stats_big",
-	               g_gr->images().get("images/wui/fieldaction/menu_tab_buildbig.png"),
-	               tabs_[BuildingTab::Big], _("Big buildings"));
-	tab_panel_.add("building_stats_mines",
-	               g_gr->images().get("images/wui/fieldaction/menu_tab_buildmine.png"),
-	               tabs_[BuildingTab::Mines], _("Mines"));
-
-	// Only show the ports tab for seafaring maps
-	if (iplayer().game().map().allows_seafaring()) {
-		tab_panel_.add("building_stats_ports",
-		               g_gr->images().get("images/wui/fieldaction/menu_tab_buildport.png"),
-		               tabs_[BuildingTab::Ports], _("Ports"));
-	}
-
-	const DescriptionIndex nr_buildings = parent.egbase().tribes().nrbuildings();
-	building_buttons_ = std::vector<UI::Button*>(nr_buildings);
-	owned_labels_ = std::vector<UI::Textarea*>(nr_buildings);
-	productivity_labels_ = std::vector<UI::Textarea*>(nr_buildings);
-
-	// Column counters
-	int columns[kNoOfBuildingTabs] = {0, 0, 0, 0, 0};
-
-	// Row containers
-	UI::Box* rows[kNoOfBuildingTabs];
-	for (int i = 0; i < kNoOfBuildingTabs; ++i) {
-		rows[i] = new UI::Box(tabs_[i], 0, 0, UI::Box::Horizontal);
-	}
-
-	// We want to add player tribe's buildings in correct order
-	const TribeDescr& tribe = iplayer().player().tribe();
-	std::vector<DescriptionIndex> buildings_to_add;
-	for (DescriptionIndex index : tribe.buildings()) {
-		// Only add headquarter types that are owned by player.
-		const BuildingDescr& descr = *tribe.get_building_descr(index);
-		const Widelands::Player& player = iplayer().player();
-		if (descr.is_buildable() || descr.is_enhanced() ||
-		    !player.get_building_statistics(index).empty()) {
-			buildings_to_add.push_back(index);
-		}
-	}
-
-	// We want to add other tribes' militarysites on the bottom
-	for (DescriptionIndex index = 0; index < nr_buildings; ++index) {
-		const BuildingDescr& descr = *parent.egbase().tribes().get_building_descr(index);
-		if (descr.type() == MapObjectType::MILITARYSITE && !tribe.has_building(index)) {
-			buildings_to_add.push_back(index);
-		}
-	}
-
-	for (DescriptionIndex id : buildings_to_add) {
-		const BuildingDescr& descr = *tribe.get_building_descr(id);
-
-		if (descr.type() != MapObjectType::CONSTRUCTIONSITE &&
-		    descr.type() != MapObjectType::DISMANTLESITE) {
-			if (descr.get_ismine()) {
-				if (add_button(id, descr, BuildingTab::Mines, *rows[BuildingTab::Mines],
-				               &columns[BuildingTab::Mines])) {
-					rows[BuildingTab::Mines] =
-					   new UI::Box(tabs_[BuildingTab::Mines], 0, 0, UI::Box::Horizontal);
-				}
-			} else if (descr.get_isport()) {
-				if (add_button(id, descr, BuildingTab::Ports, *rows[BuildingTab::Ports],
-				               &columns[BuildingTab::Ports])) {
-					rows[BuildingTab::Ports] =
-					   new UI::Box(tabs_[BuildingTab::Ports], 0, 0, UI::Box::Horizontal);
-				}
-			} else {
-				switch (descr.get_size()) {
-				case BaseImmovable::SMALL:
-					if (add_button(id, descr, BuildingTab::Small, *rows[BuildingTab::Small],
-					               &columns[BuildingTab::Small])) {
-						rows[BuildingTab::Small] =
-						   new UI::Box(tabs_[BuildingTab::Small], 0, 0, UI::Box::Horizontal);
-					}
-					break;
-				case BaseImmovable::MEDIUM:
-					if (add_button(id, descr, BuildingTab::Medium, *rows[BuildingTab::Medium],
-					               &columns[BuildingTab::Medium])) {
-						rows[BuildingTab::Medium] =
-						   new UI::Box(tabs_[BuildingTab::Medium], 0, 0, UI::Box::Horizontal);
-					}
-					break;
-				case BaseImmovable::BIG:
-					if (add_button(id, descr, BuildingTab::Big, *rows[BuildingTab::Big],
-					               &columns[BuildingTab::Big])) {
-						rows[BuildingTab::Big] =
-						   new UI::Box(tabs_[BuildingTab::Big], 0, 0, UI::Box::Horizontal);
-					}
-					break;
-				default:
-					throw wexception(
-					   "Building statictics: Found building without a size: %s", descr.name().c_str());
-				}
-			}
-		}
-	}
-
-	for (int i = 0; i < kNoOfBuildingTabs; ++i) {
-		tabs_[i]->add(rows[i]);
-	}
+     has_selection_(false),
+	  nr_building_types_(parent.egbase().tribes().nrbuildings()) {
+
+	building_buttons_ = std::vector<UI::Button*>(nr_building_types_);
+	owned_labels_ = std::vector<UI::Textarea*>(nr_building_types_);
+	productivity_labels_ = std::vector<UI::Textarea*>(nr_building_types_);
 
 	set_label_font(&owned_label_);
 	set_label_font(&construction_label_);
@@ -306,7 +197,7 @@
 	unproductive_percent_.cancel.connect(
 	   boost::bind(&BuildingStatisticsMenu::low_production_reset_focus, boost::ref(*this)));
 
-	update();
+	init();
 }
 
 BuildingStatisticsMenu::~BuildingStatisticsMenu() {
@@ -315,6 +206,189 @@
 	productivity_labels_.clear();
 }
 
+void BuildingStatisticsMenu::reset() {
+	update();  // In case a building got removed, make sure to deselect it first
+
+	const int last_selected_tab = tab_assignments_[tab_panel_.active()];
+
+	tab_panel_.remove_last_tab("building_stats_ports");
+	tab_panel_.remove_last_tab("building_stats_mines");
+	tab_panel_.remove_last_tab("building_stats_big");
+	tab_panel_.remove_last_tab("building_stats_medium");
+	tab_panel_.remove_last_tab("building_stats_small");
+
+	// Clean state if buildings disappear from list
+	building_buttons_.clear();
+	building_buttons_.resize(nr_building_types_);
+	owned_labels_.clear();
+	owned_labels_.resize(nr_building_types_);
+	productivity_labels_.clear();
+	productivity_labels_.resize(nr_building_types_);
+
+	// Ensure that defunct buttons disappear
+	for (int tab_index = 0; tab_index < kNoOfBuildingTabs; ++tab_index) {
+		if (tabs_[tab_index] != nullptr) {
+			tabs_[tab_index]->die();
+		}
+	}
+
+	init(last_selected_tab);
+
+	// Reset navigator
+	building_name_.set_text("");
+	if (has_selection_) {
+		if (building_buttons_[current_building_type_] != nullptr) {
+			set_current_building_type(current_building_type_);
+		} else {
+			has_selection_ = false;
+		}
+	}
+}
+
+void BuildingStatisticsMenu::init(int last_selected_tab) {
+	// We want to add player tribe's buildings in correct order
+	const Widelands::Player& player = iplayer().player();
+	const TribeDescr& tribe = player.tribe();
+	const bool map_allows_seafaring = iplayer().game().map().allows_seafaring();
+	std::vector<DescriptionIndex> buildings_to_add[kNoOfBuildingTabs];
+	// Add the player's own tribe's buildings.
+	for (DescriptionIndex index : tribe.buildings()) {
+		if (own_building_is_valid(player, index, map_allows_seafaring)) {
+			buildings_to_add[find_tab_for_building(*tribe.get_building_descr(index))].push_back(index);
+		}
+	}
+
+	// We want to add other tribes' buildings on the bottom. Only add the ones that the player owns.
+	for (DescriptionIndex index = 0; index < nr_building_types_; ++index) {
+		if (foreign_tribe_building_is_valid(player, index)) {
+			buildings_to_add[find_tab_for_building(*tribe.get_building_descr(index))].push_back(index);
+		}
+	}
+
+	// Now create the tab contents and add the building buttons
+	int row_counters[kNoOfBuildingTabs];
+	for (int tab_index = 0; tab_index < kNoOfBuildingTabs; ++tab_index) {
+		int current_column = 0;
+		tabs_[tab_index] = new UI::Box(&tab_panel_, 0, 0, UI::Box::Vertical);
+		UI::Box* row = new UI::Box(tabs_[tab_index], 0, 0, UI::Box::Horizontal);
+		row_counters[tab_index] = 0;
+
+		for (const Widelands::DescriptionIndex id : buildings_to_add[tab_index]) {
+			const BuildingDescr& descr = *iplayer().egbase().tribes().get_building_descr(id);
+			add_button(id, descr, row);
+			++current_column;
+			if (current_column == 1) {
+				++row_counters[tab_index];
+			} else if (current_column == kColumns) {
+				tabs_[tab_index]->add(row, UI::Box::Resizing::kFullSize);
+				tabs_[tab_index]->add_space(6);
+				row = new UI::Box(tabs_[tab_index], 0, 0, UI::Box::Horizontal);
+				current_column = 0;
+			}
+		}
+		// Add final row
+		if (current_column != 0) {
+			tabs_[tab_index]->add(row, UI::Box::Resizing::kFullSize);
+		}
+	}
+
+	// Show the tabs that have buttons on them
+	int tab_counter = 0;
+	auto add_tab = [this, row_counters, &tab_counter, last_selected_tab](
+	   int tab_index, const std::string& name, const std::string& image, const std::string& descr) {
+		if (row_counters[tab_index] > 0) {
+			tab_panel_.add(name, g_gr->images().get(image), tabs_[tab_index], descr);
+			if (last_selected_tab == tab_index) {
+				tab_panel_.activate(tab_counter);
+			}
+			tab_assignments_[tab_counter] = tab_index;
+			row_counters_[tab_counter] = row_counters[tab_index];
+			++tab_counter;
+		}
+	};
+	add_tab(BuildingTab::Small, "building_stats_small",
+	        "images/wui/fieldaction/menu_tab_buildsmall.png", _("Small buildings"));
+	add_tab(BuildingTab::Medium, "building_stats_medium",
+	        "images/wui/fieldaction/menu_tab_buildmedium.png", _("Medium buildings"));
+	add_tab(BuildingTab::Big, "building_stats_big", "images/wui/fieldaction/menu_tab_buildbig.png",
+	        _("Big buildings"));
+	add_tab(BuildingTab::Mines, "building_stats_mines",
+	        "images/wui/fieldaction/menu_tab_buildmine.png", _("Mines"));
+	add_tab(BuildingTab::Ports, "building_stats_ports",
+	        "images/wui/fieldaction/menu_tab_buildport.png", _("Ports"));
+
+	update();
+}
+
+bool BuildingStatisticsMenu::own_building_is_valid(const Widelands::Player& player, Widelands::DescriptionIndex index, bool map_allows_seafaring) const {
+	const BuildingDescr& descr = *player.tribe().get_building_descr(index);
+	// Skip seafaring buildings if not needed
+	if (descr.needs_seafaring() && !map_allows_seafaring &&
+	    player.get_building_statistics(index).empty()) {
+		return false;
+	}
+	if (descr.type() == MapObjectType::CONSTRUCTIONSITE ||
+	    descr.type() == MapObjectType::DISMANTLESITE) {
+		return false;
+	}
+	// Only add allowed buildings or buildings that are owned by the player.
+	if ((player.is_building_type_allowed(index) && (descr.is_buildable() || descr.is_enhanced())) ||
+	    !player.get_building_statistics(index).empty()) {
+		return true;
+	}
+	return false;
+}
+
+bool BuildingStatisticsMenu::foreign_tribe_building_is_valid(
+   const Widelands::Player& player, Widelands::DescriptionIndex index) const {
+	if (!player.tribe().has_building(index) && !player.get_building_statistics(index).empty()) {
+		const BuildingDescr& descr = *iplayer().egbase().tribes().get_building_descr(index);
+		if (descr.type() == MapObjectType::CONSTRUCTIONSITE ||
+		    descr.type() == MapObjectType::DISMANTLESITE) {
+			return false;
+		}
+		return true;
+	}
+	return false;
+}
+
+int BuildingStatisticsMenu::find_tab_for_building(const Widelands::BuildingDescr& descr) const {
+	assert(descr.type() != MapObjectType::CONSTRUCTIONSITE);
+	assert(descr.type() != MapObjectType::DISMANTLESITE);
+	if (descr.get_ismine()) {
+		return BuildingTab::Mines;
+	} else if (descr.get_isport()) {
+		return BuildingTab::Ports;
+	} else {
+		switch (descr.get_size()) {
+		case BaseImmovable::SMALL:
+			return BuildingTab::Small;
+		case BaseImmovable::MEDIUM:
+			return BuildingTab::Medium;
+		case BaseImmovable::BIG:
+			return BuildingTab::Big;
+		default:
+			throw wexception(
+			   "Building statictics: Found building without a size: %s", descr.name().c_str());
+		}
+	}
+	NEVER_HERE();
+}
+
+void BuildingStatisticsMenu::update_building_list() {
+	const Widelands::Player& player = iplayer().player();
+	const bool map_allows_seafaring = iplayer().game().map().allows_seafaring();
+	for (DescriptionIndex index = 0; index < nr_building_types_; ++index) {
+		const bool should_have_this_building =
+		   own_building_is_valid(player, index, map_allows_seafaring) || foreign_tribe_building_is_valid(player, index);
+		const bool has_this_building = building_buttons_[index] != nullptr;
+		if (should_have_this_building != has_this_building) {
+			reset();
+			return;
+		}
+	}
+}
+
 /**
  * Adds 3 buttons per building type.
  *
@@ -322,10 +396,10 @@
  * - Buildings owned, steps through constructionsites
  * - Productivity, steps though buildings with low productivity and stopped buildings
  */
-bool BuildingStatisticsMenu::add_button(
-   DescriptionIndex id, const BuildingDescr& descr, int tab_index, UI::Box& row, int* column) {
-
-	UI::Box* button_box = new UI::Box(&row, 0, 0, UI::Box::Vertical);
+void BuildingStatisticsMenu::add_button(DescriptionIndex id,
+                                        const BuildingDescr& descr,
+                                        UI::Box* row) {
+	UI::Box* button_box = new UI::Box(row, 0, 0, UI::Box::Vertical);
 	building_buttons_[id] = new UI::Button(
 	   button_box, (boost::format("building_button%s") % id).str(), 0, 0, kBuildGridCellWidth,
 	   kBuildGridCellHeight, g_gr->images().get("images/ui_basic/but1.png"),
@@ -347,25 +421,10 @@
 	productivity_labels_[id]->set_fixed_width(kBuildGridCellWidth);
 	button_box->add(productivity_labels_[id]);
 
-	row.add(button_box);
+	row->add(button_box);
 
 	building_buttons_[id]->sigclicked.connect(
 	   boost::bind(&BuildingStatisticsMenu::set_current_building_type, boost::ref(*this), id));
-
-	// For dynamic window height
-	if (*column == 0) {
-		++row_counters_[tab_index];
-	}
-
-	// Check if the row is full
-	++*column;
-	if (*column == kColumns) {
-		tabs_[tab_index]->add(&row);
-		tabs_[tab_index]->add_space(6);
-		*column = 0;
-		return true;
-	}
-	return false;
 }
 
 void BuildingStatisticsMenu::jump_building(JumpTarget target, bool reverse) {
@@ -496,28 +555,30 @@
  * Update this statistic
  */
 void BuildingStatisticsMenu::think() {
+	// Update statistics
+	const int32_t gametime = iplayer().game().get_gametime();
+
+	if (was_minimized_ || (gametime - lastupdate_) > kUpdateTimeInGametimeMs) {
+		update_building_list();
+		update();
+		lastupdate_ = gametime;
+	}
+	// Make sure we don't have a delay with displaying labels when we restore the window.
+	was_minimized_ = is_minimal();
+
 	// Adjust height to current tab
 	if (is_minimal()) {
 		tab_panel_.set_size(0, 0);
 	} else {
-		int tab_height =
+		const int tab_height =
 		   35 +
-		   row_counters_[tab_panel_.active()] * (kBuildGridCellHeight + kLabelHeight + kLabelHeight);
+		   row_counters_[tab_panel_.active()] * (kBuildGridCellHeight + kLabelHeight + kLabelHeight) +
+		   kMargin;
 		tab_panel_.set_size(kWindowWidth, tab_height);
 		set_size(
-		   get_w(), tab_height + kMargin + 4 * kButtonRowHeight + get_tborder() + get_bborder());
+		   get_w(), tab_height + kMargin + navigation_panel_.get_h() + get_tborder() + get_bborder());
 		navigation_panel_.set_pos(Vector2i(0, tab_height + kMargin));
 	}
-
-	// Update statistics
-	const int32_t gametime = iplayer().game().get_gametime();
-
-	if (was_minimized_ || (gametime - lastupdate_) > kUpdateTimeInGametimeMs) {
-		update();
-		lastupdate_ = gametime;
-	}
-	// Make sure we don't have a delay with displaying labels when we restore the window.
-	was_minimized_ = is_minimal();
 }
 
 /*
@@ -540,7 +601,6 @@
 void BuildingStatisticsMenu::update() {
 	const Player& player = iplayer().player();
 	const TribeDescr& tribe = player.tribe();
-	const DescriptionIndex nr_buildings = iplayer().egbase().tribes().nrbuildings();
 
 	owned_label_.set_visible(false);
 	no_owned_label_.set_visible(false);
@@ -558,7 +618,7 @@
 	navigation_buttons_[NavigationButton::NextUnproductive]->set_visible(false);
 	navigation_buttons_[NavigationButton::PrevUnproductive]->set_visible(false);
 
-	for (DescriptionIndex id = 0; id < nr_buildings; ++id) {
+	for (DescriptionIndex id = 0; id < nr_building_types_; ++id) {
 		const BuildingDescr& building = *tribe.get_building_descr(id);
 		if (building_buttons_[id] == nullptr) {
 			continue;
@@ -676,7 +736,9 @@
 		}
 
 		std::string owned_text;
-		if (player.tribe().has_building(id) && (building.is_buildable() || building.is_enhanced())) {
+		const bool can_construct_this_building = player.tribe().has_building(id) &&
+				(building.is_buildable() || building.is_enhanced());
+		if (can_construct_this_building) {
 			/** TRANSLATORS: Buildings: owned / under construction */
 			owned_text = (boost::format(_("%1%/%2%")) % nr_owned % nr_build).str();
 		} else {
@@ -694,7 +756,7 @@
 			no_owned_label_.set_visible(true);
 			navigation_buttons_[NavigationButton::NextOwned]->set_visible(true);
 			navigation_buttons_[NavigationButton::PrevOwned]->set_visible(true);
-			if (player.tribe().has_building(id) && building.is_buildable()) {
+			if (can_construct_this_building) {
 				no_construction_label_.set_text(nr_build > 0 ? std::to_string(nr_build) : "");
 				navigation_buttons_[NavigationButton::NextConstruction]->set_enabled(nr_build > 0);
 				navigation_buttons_[NavigationButton::PrevConstruction]->set_enabled(nr_build > 0);
 
=== modified file 'src/wui/building_statistics_menu.h'
--- src/wui/building_statistics_menu.h	2018-04-07 16:59:00 +0000
+++ src/wui/building_statistics_menu.h	2018-04-12 05:33:14 +0000
@@ -46,6 +46,8 @@
 	~BuildingStatisticsMenu() override;
 
 	void think() override;
+
+	/// Update state of current building buttons
 	void update();
 
 private:
@@ -65,13 +67,25 @@
 		NextUnproductive
 	};
 
+	/// Initialize the buttons
+	void reset();
+	void init(int last_selected_tab = 0);
+
+	/// Whether a building that is used by the player's tribe should be added
+	bool own_building_is_valid(const Widelands::Player& player, Widelands::DescriptionIndex index, bool map_allows_seafaring) const;
+	/// Whether a building that isn't used by the player's tribe should be added
+	bool foreign_tribe_building_is_valid(const Widelands::Player& player, Widelands::DescriptionIndex index) const;
+	/// Determine which tab a building button should end up on, according to building size etc.
+	int find_tab_for_building(const Widelands::BuildingDescr& descr) const;
+
+	/// If the buildings that should be shown have changes, update the list and reinitialize
+	void update_building_list();
+
 	/// Adds a button for the building type belonging to the id and descr to the tab.
 	/// Returns true when a new row needs to be created.
-	bool add_button(Widelands::DescriptionIndex id,
+	void add_button(Widelands::DescriptionIndex id,
 	                const Widelands::BuildingDescr& descr,
-	                int tab_index,
-	                UI::Box& row,
-	                int* column);
+	                UI::Box* row);
 
 	/// Jumps to the next / previous appropriate building
 	void jump_building(JumpTarget target, bool reverse);
@@ -95,7 +109,10 @@
 	/// UI tabs
 	UI::TabPanel tab_panel_;
 	UI::Box* tabs_[kNoOfBuildingTabs];
+	/// How many button rows each tab has
 	int row_counters_[kNoOfBuildingTabs];
+	/// We can have gaps in the tab sequence, so we need to map the indices for remembering the last selected tab
+	int tab_assignments_[kNoOfBuildingTabs];
 
 	/// Button with building icon
 	std::vector<UI::Button*> building_buttons_;
@@ -134,6 +151,9 @@
 
 	/// Whether a building has been selected
 	bool has_selection_;
+
+	/// The total number of building types available for all the tribes
+	const Widelands::DescriptionIndex nr_building_types_;
 };
 
 #endif  // end of include guard: WL_WUI_BUILDING_STATISTICS_MENU_H