widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

SirVer has proposed merging lp:~widelands-dev/widelands/keep_optr_in_ui into lp:widelands.

Commit message:
A more principled fix to dangling object pointers in the UI.

The EditorGameBase class already tracks liveliness of MapObjects for us. So let's make use of this instead of keeping the state updated through Notifications.

Detailed changes:
- Make BuildingWindow use a OPtr<> to never try to access a building that was deleted. Call die() whenver this happens.
- Remove BuildingNote::kDeleted. It no longer is necessary, since the UI has another method to track livelyness of a building.
- Change BuildingWindow::init() to not be virtual, because it was never used as a virtual function.
- Simplifications and type safety around child classes of BuildingWindow by keeping another OPtr<> for the child classes where needed. Also made more functions private.


Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/keep_optr_in_ui/+merge/334524
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/keep_optr_in_ui into lp:widelands.

=== modified file 'src/logic/map_objects/tribes/building.cc'
--- src/logic/map_objects/tribes/building.cc	2017-08-19 22:22:20 +0000
+++ src/logic/map_objects/tribes/building.cc	2017-11-30 11:02:11 +0000
@@ -443,7 +443,6 @@
 ===============
 */
 void Building::destroy(EditorGameBase& egbase) {
-	Notifications::publish(NoteBuilding(serial(), NoteBuilding::Action::kDeleted));
 	const bool fire = burn_on_destroy();
 	const Coords pos = position_;
 	Player* building_owner = get_owner();

=== modified file 'src/logic/map_objects/tribes/building.h'
--- src/logic/map_objects/tribes/building.h	2017-09-10 18:17:55 +0000
+++ src/logic/map_objects/tribes/building.h	2017-11-30 11:02:11 +0000
@@ -200,7 +200,7 @@
 
 	Serial serial;
 
-	enum class Action { kChanged, kDeleted, kStartWarp, kFinishWarp, kWorkersChanged };
+	enum class Action { kChanged, kStartWarp, kFinishWarp, kWorkersChanged };
 	const Action action;
 
 	NoteBuilding(Serial init_serial, const Action& init_action)

=== modified file 'src/logic/map_objects/tribes/militarysite.cc'
--- src/logic/map_objects/tribes/militarysite.cc	2017-11-24 21:34:17 +0000
+++ src/logic/map_objects/tribes/militarysite.cc	2017-11-30 11:02:11 +0000
@@ -271,7 +271,6 @@
 	// Now we destroy the old building before we place the new one.
 	// Waiting for the destroy playercommand causes crashes with the building window, so we need to
 	// close it right away.
-	Notifications::publish(NoteBuilding(military_site_->serial(), NoteBuilding::Action::kDeleted));
 	military_site_->set_defeating_player(enemyplayer->player_number());
 	military_site_->schedule_destroy(game);
 

=== modified file 'src/wui/buildingwindow.cc'
--- src/wui/buildingwindow.cc	2017-11-29 07:59:49 +0000
+++ src/wui/buildingwindow.cc	2017-11-30 11:02:11 +0000
@@ -52,7 +52,7 @@
    : UI::UniqueWindow(&parent, "building_window", &reg, Width, 0, b.descr().descname()),
      is_dying_(false),
      parent_(&parent),
-     building_(b),
+     building_(&b),
      building_position_(b.get_position()),
      showing_workarea_(false),
      avoid_fastclick_(avoid_fastclick),
@@ -66,11 +66,6 @@
 }
 
 void BuildingWindow::on_building_note(const Widelands::NoteBuilding& note) {
-	if (is_dying_) {
-		// Our building is potentially already destroyed. And we do not care
-		// about anything anymore anyways.
-		return;
-	}
 	if (note.serial == building_.serial()) {
 		switch (note.action) {
 		// The building's state has changed
@@ -79,12 +74,9 @@
 				init(true);
 			}
 			break;
-		// The building is no more
+		// The building is no more. Next think() will call die().
 		case Widelands::NoteBuilding::Action::kStartWarp:
 			igbase()->add_wanted_building_window(building_position_, get_pos(), is_minimal());
-			FALLS_THROUGH;
-		case Widelands::NoteBuilding::Action::kDeleted:
-			die();
 			break;
 		default:
 			break;
@@ -132,11 +124,17 @@
 ===============
 */
 void BuildingWindow::draw(RenderTarget& dst) {
+	Widelands::Building* building = building_.get(parent_->egbase());
+	if (building == nullptr) {
+		die();
+		return;
+	}
+
 	UI::Window::draw(dst);
 
 	// TODO(sirver): chang this to directly blit the animation. This needs support for or removal of
 	// RenderTarget.
-	const Image* image = building().representative_image();
+	const Image* image = building->representative_image();
 	dst.blitrect_scale(
 	   Rectf((get_inner_w() - image->width()) / 2.f, (get_inner_h() - image->height()) / 2.f,
 	         image->width(), image->height()),
@@ -149,15 +147,16 @@
 ===============
 */
 void BuildingWindow::think() {
-	if (!igbase()->can_see(building().owner().player_number())) {
+	Widelands::Building* building = building_.get(parent_->egbase());
+	if (building == nullptr || !igbase()->can_see(building->owner().player_number())) {
 		die();
 		return;
 	}
 
 	if (!caps_setup_ || capscache_player_number_ != igbase()->player_number() ||
-	    building().get_playercaps() != capscache_) {
+	    building->get_playercaps() != capscache_) {
 		capsbuttons_->free_children();
-		create_capsbuttons(capsbuttons_);
+		create_capsbuttons(capsbuttons_, building);
 		if (!avoid_fastclick_) {
 			move_out_of_the_way();
 			warp_mouse_to_fastclick_panel();
@@ -175,11 +174,12 @@
  *
  * \note Children of \p box must be allocated on the heap
  */
-void BuildingWindow::create_capsbuttons(UI::Box* capsbuttons) {
-	capscache_ = building().get_playercaps();
+void BuildingWindow::create_capsbuttons(UI::Box* capsbuttons, Widelands::Building* building) {
+	assert(building != nullptr);
+	capscache_ = building->get_playercaps();
 	capscache_player_number_ = igbase()->player_number();
 
-	const Widelands::Player& owner = building().owner();
+	const Widelands::Player& owner = building->owner();
 	const Widelands::PlayerNumber owner_number = owner.player_number();
 	const bool can_see = igbase()->can_see(owner_number);
 	const bool can_act = igbase()->can_act(owner_number);
@@ -187,7 +187,7 @@
 	bool requires_destruction_separator = false;
 	if (can_act) {
 		// Check if this is a port building and if yes show expedition button
-		if (upcast(Widelands::Warehouse const, warehouse, &building_)) {
+		if (upcast(Widelands::Warehouse const, warehouse, building)) {
 			if (Widelands::PortDock* pd = warehouse->get_portdock()) {
 				expeditionbtn_ =
 				   new UI::Button(capsbuttons, "start_or_cancel_expedition", 0, 0, 34, 34,
@@ -207,7 +207,7 @@
 					      }
 					   });
 			}
-		} else if (upcast(const Widelands::ProductionSite, productionsite, &building_)) {
+		} else if (upcast(const Widelands::ProductionSite, productionsite, building)) {
 			if (!is_a(Widelands::MilitarySite, productionsite)) {
 				const bool is_stopped = productionsite->is_stopped();
 				UI::Button* stopbtn = new UI::Button(
@@ -234,7 +234,7 @@
 		}  // upcast to productionsite
 
 		if (capscache_ & Widelands::Building::PCap_Enhancable) {
-			const Widelands::DescriptionIndex& enhancement = building_.descr().enhancement();
+			const Widelands::DescriptionIndex& enhancement = building->descr().enhancement();
 			const Widelands::TribeDescr& tribe = owner.tribe();
 			if (owner.is_building_type_allowed(enhancement)) {
 				const Widelands::BuildingDescr& building_descr = *tribe.get_building_descr(enhancement);
@@ -268,7 +268,7 @@
 
 		if (capscache_ & Widelands::Building::PCap_Dismantle) {
 			const Widelands::Buildcost wares =
-			   Widelands::DismantleSite::count_returned_wares(&building_);
+			   Widelands::DismantleSite::count_returned_wares(building);
 			if (!wares.empty()) {
 				UI::Button* dismantlebtn = new UI::Button(
 				   capsbuttons, "dismantle", 0, 0, 34, 34,
@@ -293,10 +293,10 @@
 
 	if (can_see) {
 		const WorkareaInfo* wa_info;
-		if (upcast(Widelands::ConstructionSite, csite, &building_)) {
+		if (upcast(Widelands::ConstructionSite, csite, building)) {
 			wa_info = &csite->building().workarea_info();
 		} else {
-			wa_info = &building_.descr().workarea_info();
+			wa_info = &building->descr().workarea_info();
 		}
 		if (!wa_info->empty()) {
 			toggle_workarea_ = new UI::Button(
@@ -335,10 +335,14 @@
 		   g_gr->images().get("images/ui_basic/menu_help.png"), _("Help"));
 
 		UI::UniqueWindow::Registry& registry =
-		   igbase()->unique_windows().get_registry(building_.descr().name() + "_help");
+		   igbase()->unique_windows().get_registry(building->descr().name() + "_help");
 		registry.open_window = [this, &registry] {
-			new UI::BuildingHelpWindow(igbase(), registry, building_.descr(),
-			                           building_.owner().tribe(), &igbase()->egbase().lua());
+			Widelands::Building* building_in_lambda = building_.get(parent_->egbase());
+			if (building_in_lambda == nullptr) {
+				return;
+			}
+			new UI::BuildingHelpWindow(igbase(), registry, building_in_lambda->descr(),
+			                           building_in_lambda->owner().tribe(), &parent_->egbase().lua());
 		};
 
 		helpbtn->sigclicked.connect(
@@ -353,11 +357,17 @@
 ===============
 */
 void BuildingWindow::act_bulldoze() {
+	Widelands::Building* building = building_.get(parent_->egbase());
+	if (building == nullptr) {
+		die();
+		return;
+	}
+
 	if (SDL_GetModState() & KMOD_CTRL) {
-		if (building_.get_playercaps() & Widelands::Building::PCap_Bulldoze)
-			igbase()->game().send_player_bulldoze(building_);
+		if (building->get_playercaps() & Widelands::Building::PCap_Bulldoze)
+			igbase()->game().send_player_bulldoze(*building);
 	} else {
-		show_bulldoze_confirm(dynamic_cast<InteractivePlayer&>(*igbase()), building_);
+		show_bulldoze_confirm(dynamic_cast<InteractivePlayer&>(*igbase()), *building);
 	}
 }
 
@@ -367,11 +377,12 @@
 ===============
 */
 void BuildingWindow::act_dismantle() {
+	Widelands::Building* building = building_.get(parent_->egbase());
 	if (SDL_GetModState() & KMOD_CTRL) {
-		if (building_.get_playercaps() & Widelands::Building::PCap_Dismantle)
-			igbase()->game().send_player_dismantle(building_);
+		if (building->get_playercaps() & Widelands::Building::PCap_Dismantle)
+			igbase()->game().send_player_dismantle(*building);
 	} else {
-		show_dismantle_confirm(dynamic_cast<InteractivePlayer&>(*igbase()), building_);
+		show_dismantle_confirm(dynamic_cast<InteractivePlayer&>(*igbase()), *building);
 	}
 }
 
@@ -381,8 +392,13 @@
 ===============
 */
 void BuildingWindow::act_start_stop() {
-	if (dynamic_cast<const Widelands::ProductionSite*>(&building_)) {
-		igbase()->game().send_player_start_stop_building(building_);
+	Widelands::Building* building = building_.get(parent_->egbase());
+	if (building == nullptr) {
+		return;
+	}
+
+	if (dynamic_cast<const Widelands::ProductionSite*>(building)) {
+		igbase()->game().send_player_start_stop_building(*building);
 	}
 }
 
@@ -392,10 +408,16 @@
 ===============
 */
 void BuildingWindow::act_start_or_cancel_expedition() {
-	if (upcast(Widelands::Warehouse const, warehouse, &building_)) {
+	Widelands::Building* building = building_.get(parent_->egbase());
+	if (building == nullptr) {
+		die();
+		return;
+	}
+
+	if (upcast(Widelands::Warehouse const, warehouse, building)) {
 		if (warehouse->get_portdock()) {
 			expeditionbtn_->set_enabled(false);
-			igbase()->game().send_player_start_or_cancel_expedition(building_);
+			igbase()->game().send_player_start_or_cancel_expedition(*building);
 		}
 		get_tabs()->activate("expedition_wares_queue");
 	}
@@ -410,11 +432,17 @@
 ===============
 */
 void BuildingWindow::act_enhance(Widelands::DescriptionIndex id) {
+	Widelands::Building* building = building_.get(parent_->egbase());
+	if (building == nullptr) {
+		die();
+		return;
+	}
+
 	if (SDL_GetModState() & KMOD_CTRL) {
-		if (building_.get_playercaps() & Widelands::Building::PCap_Enhancable)
-			igbase()->game().send_player_enhance_building(building_, id);
+		if (building->get_playercaps() & Widelands::Building::PCap_Enhancable)
+			igbase()->game().send_player_enhance_building(*building, id);
 	} else {
-		show_enhance_confirm(dynamic_cast<InteractivePlayer&>(*igbase()), building_, id);
+		show_enhance_confirm(dynamic_cast<InteractivePlayer&>(*igbase()), *building, id);
 	}
 }
 
@@ -434,11 +462,17 @@
 	if (showing_workarea_) {
 		return;  // already shown, nothing to be done
 	}
+	Widelands::Building* building = building_.get(parent_->egbase());
+	if (building == nullptr) {
+		die();
+		return;
+	}
+
 	const WorkareaInfo* workarea_info;
-	if (upcast(Widelands::ConstructionSite, csite, &building_)) {
+	if (upcast(Widelands::ConstructionSite, csite, building)) {
 		workarea_info = &csite->building().workarea_info();
 	} else {
-		workarea_info = &building_.descr().workarea_info();
+		workarea_info = &building->descr().workarea_info();
 	}
 	if (workarea_info->empty()) {
 		return;

=== modified file 'src/wui/buildingwindow.h'
--- src/wui/buildingwindow.h	2017-09-01 15:45:59 +0000
+++ src/wui/buildingwindow.h	2017-11-30 11:02:11 +0000
@@ -50,10 +50,6 @@
 
 	virtual ~BuildingWindow();
 
-	Widelands::Building& building() {
-		return building_;
-	}
-
 	InteractiveGameBase* igbase() const {
 		return parent_;
 	}
@@ -62,9 +58,10 @@
 	void think() override;
 
 protected:
-	virtual void init(bool avoid_fastclick);
 	void die() override;
 
+	void init(bool avoid_fastclick);
+
 	UI::TabPanel* get_tabs() {
 		return tabs_;
 	}
@@ -84,11 +81,12 @@
 	void
 	create_input_queue_panel(UI::Box*, Widelands::Building&, Widelands::InputQueue*, bool = false);
 
-	virtual void create_capsbuttons(UI::Box* buttons);
 
 	bool is_dying_;
 
 private:
+	void create_capsbuttons(UI::Box* buttons, Widelands::Building* building);
+
 	// Actions performed when a NoteBuilding is received.
 	void on_building_note(const Widelands::NoteBuilding& note);
 
@@ -97,7 +95,7 @@
 
 	InteractiveGameBase* parent_;
 
-	Widelands::Building& building_;
+	Widelands::OPtr<Widelands::Building> building_;
 
 	// We require this to unregister overlays when we are closed. Since the
 	// building might have been destroyed by then we have to keep a copy of its

=== modified file 'src/wui/constructionsitewindow.cc'
--- src/wui/constructionsitewindow.cc	2017-08-16 10:14:29 +0000
+++ src/wui/constructionsitewindow.cc	2017-11-30 11:02:11 +0000
@@ -31,13 +31,15 @@
                                                UI::UniqueWindow::Registry& reg,
                                                Widelands::ConstructionSite& cs,
                                                bool avoid_fastclick)
-   : BuildingWindow(parent, reg, cs, avoid_fastclick), progress_(nullptr) {
+   : BuildingWindow(parent, reg, cs, avoid_fastclick), construction_site_(&cs), progress_(nullptr) {
 	init(avoid_fastclick);
 }
 
 void ConstructionSiteWindow::init(bool avoid_fastclick) {
+	Widelands::ConstructionSite* construction_site = construction_site_.get(igbase()->egbase());
+	assert (construction_site != nullptr);
+
 	BuildingWindow::init(avoid_fastclick);
-	Widelands::ConstructionSite& cs = dynamic_cast<Widelands::ConstructionSite&>(building());
 	UI::Box& box = *new UI::Box(get_tabs(), 0, 0, UI::Box::Vertical);
 
 	// Add the progress bar
@@ -49,12 +51,13 @@
 	box.add_space(8);
 
 	// Add the wares queue
-	for (uint32_t i = 0; i < cs.get_nrwaresqueues(); ++i)
-		box.add(new InputQueueDisplay(&box, 0, 0, *igbase(), cs, cs.get_waresqueue(i)));
+	for (uint32_t i = 0; i < construction_site->get_nrwaresqueues(); ++i)
+		box.add(new InputQueueDisplay(
+		   &box, 0, 0, *igbase(), *construction_site, construction_site->get_waresqueue(i)));
 
 	get_tabs()->add("wares", g_gr->images().get(pic_tab_wares), &box, _("Building materials"));
 
-	set_title((boost::format("(%s)") % cs.building().descname()).str());
+	set_title((boost::format("(%s)") % construction_site->building().descname()).str());
 	think();
 }
 
@@ -66,7 +69,11 @@
 void ConstructionSiteWindow::think() {
 	BuildingWindow::think();
 
-	const Widelands::ConstructionSite& cs = dynamic_cast<Widelands::ConstructionSite&>(building());
+	Widelands::ConstructionSite* construction_site = construction_site_.get(igbase()->egbase());
+	if (construction_site == nullptr) {
+		die();
+		return;
+	}
 
-	progress_->set_state(cs.get_built_per64k());
+	progress_->set_state(construction_site->get_built_per64k());
 }

=== modified file 'src/wui/constructionsitewindow.h'
--- src/wui/constructionsitewindow.h	2017-02-14 12:11:58 +0000
+++ src/wui/constructionsitewindow.h	2017-11-30 11:02:11 +0000
@@ -36,9 +36,10 @@
 	void think() override;
 
 protected:
-	void init(bool avoid_fastclick) override;
+	void init(bool avoid_fastclick);
 
 private:
+	Widelands::OPtr<Widelands::ConstructionSite> construction_site_;
 	UI::ProgressBar* progress_;
 	DISALLOW_COPY_AND_ASSIGN(ConstructionSiteWindow);
 };

=== modified file 'src/wui/dismantlesitewindow.cc'
--- src/wui/dismantlesitewindow.cc	2017-08-16 10:14:29 +0000
+++ src/wui/dismantlesitewindow.cc	2017-11-30 11:02:11 +0000
@@ -28,13 +28,15 @@
                                          UI::UniqueWindow::Registry& reg,
                                          Widelands::DismantleSite& ds,
                                          bool avoid_fastclick)
-   : BuildingWindow(parent, reg, ds, avoid_fastclick), progress_(nullptr) {
+   : BuildingWindow(parent, reg, ds, avoid_fastclick), dismantle_site_(&ds), progress_(nullptr) {
 	init(avoid_fastclick);
 }
 
 void DismantleSiteWindow::init(bool avoid_fastclick) {
+	Widelands::DismantleSite* dismantle_site = dismantle_site_.get(igbase()->egbase());
+	assert(dismantle_site != nullptr);
+
 	BuildingWindow::init(avoid_fastclick);
-	Widelands::DismantleSite& ds = dynamic_cast<Widelands::DismantleSite&>(building());
 	UI::Box& box = *new UI::Box(get_tabs(), 0, 0, UI::Box::Vertical);
 
 	// Add the progress bar
@@ -46,8 +48,9 @@
 	box.add_space(8);
 
 	// Add the wares queue
-	for (uint32_t i = 0; i < ds.get_nrwaresqueues(); ++i)
-		BuildingWindow::create_input_queue_panel(&box, ds, ds.get_waresqueue(i), true);
+	for (uint32_t i = 0; i < dismantle_site->get_nrwaresqueues(); ++i)
+		BuildingWindow::create_input_queue_panel(
+		   &box, *dismantle_site, dismantle_site->get_waresqueue(i), true);
 
 	get_tabs()->add("wares", g_gr->images().get(pic_tab_wares), &box, _("Building materials"));
 	think();
@@ -61,7 +64,10 @@
 void DismantleSiteWindow::think() {
 	BuildingWindow::think();
 
-	const Widelands::DismantleSite& ds = dynamic_cast<Widelands::DismantleSite&>(building());
-
-	progress_->set_state(ds.get_built_per64k());
+	Widelands::DismantleSite* dismantle_site = dismantle_site_.get(igbase()->egbase());
+	if (dismantle_site == nullptr) {
+		die();
+		return;
+	}
+	progress_->set_state(dismantle_site->get_built_per64k());
 }

=== modified file 'src/wui/dismantlesitewindow.h'
--- src/wui/dismantlesitewindow.h	2017-02-14 12:11:58 +0000
+++ src/wui/dismantlesitewindow.h	2017-11-30 11:02:11 +0000
@@ -35,10 +35,10 @@
 
 	void think() override;
 
-protected:
-	void init(bool avoid_fastclick) override;
-
 private:
+	void init(bool avoid_fastclick);
+
+	Widelands::OPtr<Widelands::DismantleSite> dismantle_site_;
 	UI::ProgressBar* progress_;
 	DISALLOW_COPY_AND_ASSIGN(DismantleSiteWindow);
 };

=== modified file 'src/wui/militarysitewindow.cc'
--- src/wui/militarysitewindow.cc	2017-06-06 05:37:57 +0000
+++ src/wui/militarysitewindow.cc	2017-11-30 11:02:11 +0000
@@ -31,16 +31,13 @@
                                        Widelands::MilitarySite& ms,
                                        bool avoid_fastclick)
    : BuildingWindow(parent, reg, ms, avoid_fastclick) {
-	init(avoid_fastclick);
-}
-
-void MilitarySiteWindow::create_capsbuttons(UI::Box* buttons) {
-	BuildingWindow::create_capsbuttons(buttons);
-}
-
-void MilitarySiteWindow::init(bool avoid_fastclick) {
+	init(avoid_fastclick, &ms);
+}
+
+void MilitarySiteWindow::init(bool avoid_fastclick, Widelands::MilitarySite* military_site) {
+	assert(military_site != nullptr);
 	BuildingWindow::init(avoid_fastclick);
 	get_tabs()->add("soldiers", g_gr->images().get(pic_tab_military),
-	                create_soldier_list(*get_tabs(), *igbase(), militarysite()), _("Soldiers"));
+	                create_soldier_list(*get_tabs(), *igbase(), *military_site), _("Soldiers"));
 	think();
 }

=== modified file 'src/wui/militarysitewindow.h'
--- src/wui/militarysitewindow.h	2017-06-06 05:37:57 +0000
+++ src/wui/militarysitewindow.h	2017-11-30 11:02:11 +0000
@@ -32,15 +32,8 @@
 	                   Widelands::MilitarySite&,
 	                   bool avoid_fastclick);
 
-	Widelands::MilitarySite& militarysite() {
-		return dynamic_cast<Widelands::MilitarySite&>(building());
-	}
-
-protected:
-	void init(bool avoid_fastclick) override;
-	void create_capsbuttons(UI::Box* buttons) override;
-
 private:
+	void init(bool avoid_fastclick, Widelands::MilitarySite* military_site);
 	DISALLOW_COPY_AND_ASSIGN(MilitarySiteWindow);
 };
 

=== modified file 'src/wui/productionsitewindow.cc'
--- src/wui/productionsitewindow.cc	2017-11-29 07:59:49 +0000
+++ src/wui/productionsitewindow.cc	2017-11-30 11:02:11 +0000
@@ -43,31 +43,33 @@
                                            Widelands::ProductionSite& ps,
                                            bool avoid_fastclick)
    : BuildingWindow(parent, reg, ps, avoid_fastclick),
+     production_site_(&ps),
      worker_table_(nullptr),
      worker_caps_(nullptr) {
 	productionsitenotes_subscriber_ = Notifications::subscribe<Widelands::NoteBuilding>(
 	   [this](const Widelands::NoteBuilding& note) {
-		   if (is_dying_) {
-			   // Our building is potentially already destroyed. And we do not care
-			   // about anything anymore anyways.
+		   Widelands::ProductionSite* production_site = production_site_.get(igbase()->egbase());
+		   if (production_site == nullptr) {
 			   return;
 		   }
-		   if (note.serial == building().serial()) {
+		   if (note.serial == production_site->serial()) {
 			   switch (note.action) {
 			   case Widelands::NoteBuilding::Action::kWorkersChanged:
-				   update_worker_table();
+				   update_worker_table(production_site);
 				   break;
 			   default:
 				   break;
 			   }
 		   }
 		});
-	init(avoid_fastclick);
+	init(avoid_fastclick, &ps);
 }
 
-void ProductionSiteWindow::init(bool avoid_fastclick) {
+void ProductionSiteWindow::init(bool avoid_fastclick, Widelands::ProductionSite *production_site) {
+	assert(production_site != nullptr);
+
 	BuildingWindow::init(avoid_fastclick);
-	const std::vector<Widelands::InputQueue*>& inputqueues = productionsite().inputqueues();
+	const std::vector<Widelands::InputQueue*>& inputqueues = production_site->inputqueues();
 
 	if (inputqueues.size()) {
 		// Add the wares tab
@@ -76,14 +78,14 @@
 
 		for (uint32_t i = 0; i < inputqueues.size(); ++i) {
 			prod_box->add(
-			   new InputQueueDisplay(prod_box, 0, 0, *igbase(), productionsite(), inputqueues[i]));
+			   new InputQueueDisplay(prod_box, 0, 0, *igbase(), *production_site, inputqueues[i]));
 		}
 
 		get_tabs()->add("wares", g_gr->images().get(pic_tab_wares), prod_box, _("Wares"));
 	}
 
 	// Add workers tab if applicable
-	if (!productionsite().descr().nr_working_positions()) {
+	if (!production_site->descr().nr_working_positions()) {
 		worker_table_ = nullptr;
 	} else {
 		UI::Box* worker_box = new UI::Box(get_tabs(), 0, 0, UI::Box::Vertical);
@@ -91,16 +93,16 @@
 		worker_caps_ = new UI::Box(worker_box, 0, 0, UI::Box::Horizontal);
 
 		worker_table_->add_column(
-		   210, (ngettext("Worker", "Workers", productionsite().descr().nr_working_positions())));
+		   210, (ngettext("Worker", "Workers", production_site->descr().nr_working_positions())));
 		worker_table_->add_column(60, _("Exp"));
 		worker_table_->add_column(150, _("Next Level"));
 
-		for (unsigned int i = 0; i < productionsite().descr().nr_working_positions(); ++i) {
+		for (unsigned int i = 0; i < production_site->descr().nr_working_positions(); ++i) {
 			worker_table_->add(i);
 		}
 		worker_table_->fit_height();
 
-		if (igbase()->can_act(building().owner().player_number())) {
+		if (igbase()->can_act(production_site->owner().player_number())) {
 			worker_caps_->add_inf_space();
 			UI::Button* evict_button = new UI::Button(
 			   worker_caps_, "evict", 0, 0, 34, 34, g_gr->images().get("images/ui_basic/but4.png"),
@@ -116,34 +118,44 @@
 		worker_box->add(worker_caps_, UI::Box::Resizing::kFullSize);
 		get_tabs()->add(
 		   "workers", g_gr->images().get(pic_tab_workers), worker_box,
-		   (ngettext("Worker", "Workers", productionsite().descr().nr_working_positions())));
-		update_worker_table();
+		   (ngettext("Worker", "Workers", production_site->descr().nr_working_positions())));
+		update_worker_table(production_site);
 	}
 	think();
 }
 
 void ProductionSiteWindow::think() {
 	BuildingWindow::think();
+
+	Widelands::ProductionSite* production_site = production_site_.get(igbase()->egbase());
+	if (production_site == nullptr) {
+		die();
+		return;
+	}
+
 	// If we have pending requests, update table each tick.
 	// This is required to update from 'vacant' to 'coming'
-	for (unsigned int i = 0; i < productionsite().descr().nr_working_positions(); ++i) {
-		Widelands::Request* r = productionsite().working_positions()[i].worker_request;
+	for (unsigned int i = 0; i < production_site->descr().nr_working_positions(); ++i) {
+		Widelands::Request* r = production_site->working_positions()[i].worker_request;
 		if (r) {
-			update_worker_table();
+			update_worker_table(production_site);
 			break;
 		}
 	}
 }
 
-void ProductionSiteWindow::update_worker_table() {
+void ProductionSiteWindow::update_worker_table(Widelands::ProductionSite* production_site) {
+	assert(production_site != nullptr);
+
 	if (worker_table_ == nullptr) {
 		return;
 	}
-	assert(productionsite().descr().nr_working_positions() == worker_table_->size());
-
-	for (unsigned int i = 0; i < productionsite().descr().nr_working_positions(); ++i) {
-		const Widelands::Worker* worker = productionsite().working_positions()[i].worker;
-		const Widelands::Request* request = productionsite().working_positions()[i].worker_request;
+
+	assert(production_site->descr().nr_working_positions() == worker_table_->size());
+
+	for (unsigned int i = 0; i < production_site->descr().nr_working_positions(); ++i) {
+		const Widelands::Worker* worker = production_site->working_positions()[i].worker;
+		const Widelands::Request* request = production_site->working_positions()[i].worker_request;
 		UI::Table<uintptr_t>::EntryRecord& er = worker_table_->get_record(i);
 
 		if (worker) {
@@ -169,7 +181,7 @@
 			}
 		} else if (request) {
 			const Widelands::WorkerDescr* desc =
-			   productionsite().owner().tribe().get_worker_descr(request->get_index());
+			   production_site->owner().tribe().get_worker_descr(request->get_index());
 			er.set_picture(0, desc->icon(), request->is_open() ? _("(vacant)") : _("(coming)"));
 
 			er.set_string(1, "");
@@ -182,9 +194,15 @@
 }
 
 void ProductionSiteWindow::evict_worker() {
+	Widelands::ProductionSite* production_site = production_site_.get(igbase()->egbase());
+	if (production_site == nullptr) {
+		die();
+		return;
+	}
+
 	if (worker_table_->has_selection()) {
 		Widelands::Worker* worker =
-		   productionsite().working_positions()[worker_table_->get_selected()].worker;
+		   production_site->working_positions()[worker_table_->get_selected()].worker;
 		if (worker) {
 			igbase()->game().send_player_evict_worker(*worker);
 		}

=== modified file 'src/wui/productionsitewindow.h'
--- src/wui/productionsitewindow.h	2017-06-06 05:37:57 +0000
+++ src/wui/productionsitewindow.h	2017-11-30 11:02:11 +0000
@@ -32,17 +32,15 @@
 	                     Widelands::ProductionSite&,
 	                     bool avoid_fastclick);
 
-	Widelands::ProductionSite& productionsite() {
-		return dynamic_cast<Widelands::ProductionSite&>(building());
-	}
-	void update_worker_table();
-
 protected:
-	void init(bool avoid_fastclick) override;
 	void think() override;
+	void init(bool avoid_fastclick, Widelands::ProductionSite* production_site);
 	void evict_worker();
 
 private:
+	void update_worker_table(Widelands::ProductionSite* production_site);
+
+	Widelands::OPtr<Widelands::ProductionSite> production_site_;
 	UI::Table<uintptr_t>* worker_table_;
 	UI::Box* worker_caps_;
 	std::unique_ptr<Notifications::Subscriber<Widelands::NoteBuilding>>

=== modified file 'src/wui/trainingsitewindow.cc'
--- src/wui/trainingsitewindow.cc	2017-06-06 05:37:57 +0000
+++ src/wui/trainingsitewindow.cc	2017-11-30 11:02:11 +0000
@@ -34,17 +34,14 @@
                                        Widelands::TrainingSite& ts,
                                        bool avoid_fastclick)
    : ProductionSiteWindow(parent, reg, ts, avoid_fastclick) {
-	init(avoid_fastclick);
+	init(avoid_fastclick, &ts);
 }
 
-void TrainingSiteWindow::init(bool avoid_fastclick) {
-	ProductionSiteWindow::init(avoid_fastclick);
+void TrainingSiteWindow::init(bool avoid_fastclick, Widelands::TrainingSite* training_site) {
+	assert(training_site != nullptr);
+	ProductionSiteWindow::init(avoid_fastclick, training_site);
 	get_tabs()->add("soldiers", g_gr->images().get(pic_tab_military),
-	                create_soldier_list(*get_tabs(), *igbase(), trainingsite()),
+	                create_soldier_list(*get_tabs(), *igbase(), *training_site),
 	                _("Soldiers in training"));
 	think();
 }
-
-void TrainingSiteWindow::create_capsbuttons(UI::Box* buttons) {
-	ProductionSiteWindow::create_capsbuttons(buttons);
-}

=== modified file 'src/wui/trainingsitewindow.h'
--- src/wui/trainingsitewindow.h	2017-06-06 05:37:57 +0000
+++ src/wui/trainingsitewindow.h	2017-11-30 11:02:11 +0000
@@ -33,15 +33,9 @@
 	                   Widelands::TrainingSite&,
 	                   bool avoid_fastclick);
 
-	Widelands::TrainingSite& trainingsite() {
-		return dynamic_cast<Widelands::TrainingSite&>(building());
-	}
-
-protected:
-	void init(bool avoid_fastclick) override;
-	void create_capsbuttons(UI::Box* buttons) override;
-
 private:
+	void init(bool avoid_fastclick, Widelands::TrainingSite* training_site);
+
 	DISALLOW_COPY_AND_ASSIGN(TrainingSiteWindow);
 };
 

=== modified file 'src/wui/warehousewindow.cc'
--- src/wui/warehousewindow.cc	2017-06-06 05:37:57 +0000
+++ src/wui/warehousewindow.cc	2017-11-30 11:02:11 +0000
@@ -177,21 +177,22 @@
                                  Widelands::Warehouse& wh,
                                  bool avoid_fastclick)
    : BuildingWindow(parent, reg, wh, avoid_fastclick) {
-	init(avoid_fastclick);
+	init(avoid_fastclick, &wh);
 }
 
-void WarehouseWindow::init(bool avoid_fastclick) {
+void WarehouseWindow::init(bool avoid_fastclick, Widelands::Warehouse* warehouse) {
+	assert(warehouse != nullptr);
 	BuildingWindow::init(avoid_fastclick);
 	get_tabs()->add(
 	   "wares", g_gr->images().get(pic_tab_wares),
-	   new WarehouseWaresPanel(get_tabs(), Width, *igbase(), warehouse(), Widelands::wwWARE),
+	   new WarehouseWaresPanel(get_tabs(), Width, *igbase(), *warehouse, Widelands::wwWARE),
 	   _("Wares"));
 	get_tabs()->add(
 	   "workers", g_gr->images().get(pic_tab_workers),
-	   new WarehouseWaresPanel(get_tabs(), Width, *igbase(), warehouse(), Widelands::wwWORKER),
+	   new WarehouseWaresPanel(get_tabs(), Width, *igbase(), *warehouse, Widelands::wwWORKER),
 	   _("Workers"));
 
-	if (Widelands::PortDock* pd = warehouse().get_portdock()) {
+	if (Widelands::PortDock* pd = warehouse->get_portdock()) {
 		get_tabs()->add("dock_wares", g_gr->images().get(pic_tab_dock_wares),
 		                create_portdock_wares_display(get_tabs(), Width, *pd, Widelands::wwWARE),
 		                _("Wares waiting to be shipped"));
@@ -200,7 +201,7 @@
 		                _("Workers waiting to embark"));
 		if (pd->expedition_started()) {
 			get_tabs()->add("expedition_wares_queue", g_gr->images().get(pic_tab_expedition),
-			                create_portdock_expedition_display(get_tabs(), warehouse(), *igbase()),
+			                create_portdock_expedition_display(get_tabs(), *warehouse, *igbase()),
 			                _("Expedition"));
 		}
 	}

=== modified file 'src/wui/warehousewindow.h'
--- src/wui/warehousewindow.h	2017-06-06 05:37:57 +0000
+++ src/wui/warehousewindow.h	2017-11-30 11:02:11 +0000
@@ -32,14 +32,9 @@
 	                Widelands::Warehouse&,
 	                bool avoid_fastclick);
 
-	Widelands::Warehouse& warehouse() {
-		return dynamic_cast<Widelands::Warehouse&>(building());
-	}
-
-protected:
-	void init(bool avoid_fastclick) override;
-
 private:
+	void init(bool avoid_fastclick, Widelands::Warehouse* warehouse);
+
 	DISALLOW_COPY_AND_ASSIGN(WarehouseWindow);
 };