widelands-dev team mailing list archive

[Nicolai Hähnle <prefect_@xxxxxxx>]

Nicolai Hähnle has proposed merging lp:~nha/widelands/fixes into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1025014 in widelands: "segmentation fault in widelands"
  https://bugs.launchpad.net/widelands/+bug/1025014
  Bug #1074979 in widelands: "r6433 has an economy mismatch after building a third port"
  https://bugs.launchpad.net/widelands/+bug/1074979
  Bug #1093848 in widelands: "Remove ware removes wares without placing them outside the building (can trigger crash)"
  https://bugs.launchpad.net/widelands/+bug/1093848

For more details, see:
https://code.launchpad.net/~nha/widelands/fixes/+merge/147734

This branch fixes the following three somewhat subtle game logic bugs:
Bug #1025014: segmentation fault in widelands
Bug #1074979: r6433 has an economy mismatch after building a third port
Bug #1093848: Remove ware removes wares without placing them outside the building (can trigger crash)

There are two conceptual changes introduced in this branch:

1. WareInstance can no longer be at a building as its location, because buildings do not track the WareInstances inside them. There were a number of potential situation in which a change of economy could leave a WareInstance in a limbo state.

2. Economies are no longer split immediately when a road or port is removed. Instead, a split check is scheduled for the next request/supply balancing tick. This should improve performance when many objects are destroyed at once; especially noticeable when loading and ending a large game, where economy splitting is now skipped entirely.
-- 
https://code.launchpad.net/~nha/widelands/fixes/+merge/147734
Your team Widelands Developers is requested to review the proposed merge of lp:~nha/widelands/fixes into lp:widelands.

=== modified file 'src/economy/economy.cc'
--- src/economy/economy.cc	2013-02-10 19:36:24 +0000
+++ src/economy/economy.cc	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004, 2006-2011 by the Widelands Development Team
+ * Copyright (C) 2004, 2006-2013 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
@@ -120,8 +120,8 @@
 }
 
 /**
- * Check whether the given flags can still reach each other (pathfinding!).
- * If not, the economy is split in two.
+ * Notify the economy that there may no longer be a connection between
+ * the given flags in the road and seafaring network.
  */
 void Economy::check_split(Flag & f1, Flag & f2)
 {
@@ -133,35 +133,59 @@
 	if (not e)
 		return;
 
-	// Start an A-star search from f1 towards f2.
-	// Keep track of which flags are reachable from f1,
-	// so that we do not have to re-scan everything in case
-	// the economy really does split
-	Map & map = e->owner().egbase().map();
-	RouteAStar<AStarEstimator> astar(*e->m_router, wwWORKER, AStarEstimator(map, f2));
-	astar.push(f1);
-
-	std::set<OPtr<Flag> > reachable;
-	while (RoutingNode * current = astar.step()) {
-		Flag * curflag = static_cast<Flag *>(current);
-		if (curflag == &f2)
-			return;
-
-		reachable.insert(curflag);
-	}
-
-	// When we get to this point, a split really did occur
-	// Attempt to split off only a reasonably small part of the economy
-	// for performance reasons
-	if (reachable.size() <= e->m_flags.size() * 3 / 5) {
-		e->_split(reachable);
-	} else {
-		std::set<OPtr<Flag> > others;
-		container_iterate_const(Flags, e->m_flags, it) {
-			if (reachable.count(*it.current) == 0)
-				others.insert(*it.current);
-		}
-		e->_split(others);
+	e->m_split_checks.push_back(std::make_pair(OPtr<Flag>(&f1), OPtr<Flag>(&f2)));
+	e->rebalance_supply(); // the real split-checking is done during rebalance
+}
+
+void Economy::_check_splits()
+{
+	Map & map = owner().egbase().map();
+
+	while (m_split_checks.size()) {
+		Flag * f1 = m_split_checks.back().first.get(owner().egbase());
+		Flag * f2 = m_split_checks.back().second.get(owner().egbase());
+		m_split_checks.pop_back();
+
+		if (!f1 || !f2) {
+			if (!f1 && !f2)
+				continue;
+			if (!f1)
+				f1 = f2;
+			if (f1->get_economy() != this)
+				continue;
+
+			// Handle the case when two or more roads are removed simultaneously
+			RouteAStar<AStarZeroEstimator> astar(*m_router, wwWORKER, AStarZeroEstimator());
+			astar.push(*f1);
+			std::set<OPtr<Flag> > reachable;
+			while (RoutingNode * current = astar.step())
+				reachable.insert(&current->base_flag());
+			if (reachable.size() != m_flags.size())
+				_split(reachable);
+			continue;
+		}
+
+		// If one (or both) of the flags have already been split off, we do not need to re-check
+		if (f1->get_economy() != this || f2->get_economy() != this)
+			continue;
+
+		// Start an A-star searches from f1 towards f2.
+		// If f2 is not reached, split off all the nodes that have been reached from f1.
+		// This policy means that the newly created economy, which contains all the
+		// flags that have been split, is already connected.
+		RouteAStar<AStarEstimator> astar(*m_router, wwWORKER, AStarEstimator(map, *f2));
+		astar.push(*f1);
+		std::set<OPtr<Flag> > reachable;
+
+		for (;;) {
+			RoutingNode * current = astar.step();
+			if (!current) {
+				_split(reachable);
+				break;
+			} else if (current == f2)
+				break;
+			reachable.insert(&current->base_flag());
+		}
 	}
 }
 
@@ -545,6 +569,8 @@
 	// Fix up Supply/Request after rebuilding
 	m_rebuilding = false;
 
+	m_split_checks.insert(m_split_checks.end(), e.m_split_checks.begin(), e.m_split_checks.end());
+
 	// implicitly delete the economy
 	delete &e;
 }
@@ -747,6 +773,8 @@
  */
 void Economy::_balance_requestsupply(Game & game)
 {
+	_check_splits();
+
 	RSPairStruct rsps;
 	rsps.nexttimer = -1;
 

=== modified file 'src/economy/economy.h'
--- src/economy/economy.h	2013-02-10 19:36:24 +0000
+++ src/economy/economy.h	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004, 2006-2011 by the Widelands Development Team
+ * Copyright (C) 2004, 2006-2013 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
@@ -45,12 +45,26 @@
 /**
  * Each Economy represents all building and flags, which are connected over the same
  * street network. In general a player can own multiple Economys, which
- * operate independent from each other.
- * During the game Economy objects can be merged or splitted due to
- * new roads or destroyed ones.
+ * operate independently from each other.
  *
  * Every Economy tracks the amount of wares inside of it and how high the
  * demand for each ware is.
+ *
+ * \paragraph Merging and splitting
+ *
+ * During the course of a game Economy objects can be merged when new roads and ports are created,
+ * or split when roads and ports are destroyed.
+ *
+ * Splitting and merging economies are relatively expensive operations,
+ * and in particular during game shutdown or when a large network is destroyed
+ * in a military operation, cascading economy splits could take a lot of processing time.
+ * For this reason, economies do not split immediately when a road is destroyed,
+ * but instead keep track of where a potential split occured and evaluate the split lazily.
+ *
+ * This means that two flags which are connected by the road (and seafaring) network
+ * are \b always in the same economy, but two flags in the same economy are not always
+ * connected by roads or the seafaring network - though of course, most code operates
+ * on the assumption that they are, with fallbacks for when they aren't.
  */
 struct Economy {
 	friend class EconomyDataPacket;
@@ -166,6 +180,7 @@
 	void _reset_all_pathfinding_cycles();
 
 	void _merge(Economy &);
+	void _check_splits();
 	void _split(const std::set<OPtr<Flag> > &);
 
 	void _start_request_timer(int32_t delta = 200);
@@ -202,6 +217,9 @@
 	Target_Quantity        * m_worker_target_quantities;
 	Router                 * m_router;
 
+	typedef std::pair<OPtr<Flag>, OPtr<Flag> > SplitPair;
+	std::vector<SplitPair> m_split_checks;
+
 	/**
 	 * ID for the next request balancing timer. Used to throttle
 	 * excessive calls to the request/supply balancing logic.

=== modified file 'src/economy/fleet.cc'
--- src/economy/fleet.cc	2013-02-10 20:07:27 +0000
+++ src/economy/fleet.cc	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011 by the Widelands Development Team
+ * Copyright (C) 2011-2013 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
@@ -241,8 +241,16 @@
 void Fleet::cleanup(Editor_Game_Base & egbase)
 {
 	while (!m_ports.empty()) {
-		m_ports.back()->set_fleet(0);
+		PortDock * pd = m_ports.back();
 		m_ports.pop_back();
+
+		pd->set_fleet(0);
+		if (!m_ports.empty()) {
+			// This is required when, during end-of-game cleanup,
+			// the fleet gets removed before the ports
+			Flag & base = m_ports[0]->base_flag();
+			Economy::check_split(base, pd->base_flag());
+		}
 	}
 	m_portpaths.clear();
 
@@ -380,7 +388,7 @@
 				// since two ports can be connected by land, it is possible that
 				// disconnecting a previous port also disconnects later ports
 				if (base.get_economy() == m_ports[i]->base_flag().get_economy())
-					base.get_economy()->check_split(base, m_ports[i]->base_flag());
+					Economy::check_split(base, m_ports[i]->base_flag());
 			}
 		}
 	}
@@ -532,7 +540,7 @@
 	} else {
 		set_economy(m_ports[0]->get_economy());
 		if (!m_ships.empty())
-			m_ports[0]->get_economy()->check_split(m_ports[0]->base_flag(), port->base_flag());
+			Economy::check_split(m_ports[0]->base_flag(), port->base_flag());
 	}
 
 	if (m_ships.empty() && m_ports.empty())

=== modified file 'src/economy/portdock.cc'
--- src/economy/portdock.cc	2013-02-10 19:36:24 +0000
+++ src/economy/portdock.cc	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011 by the Widelands Development Team
+ * Copyright (C) 2011-2013 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

=== modified file 'src/economy/request.cc'
--- src/economy/request.cc	2013-02-10 19:36:24 +0000
+++ src/economy/request.cc	2013-02-11 18:07:34 +0000
@@ -492,7 +492,7 @@
 	} else {
 		//  Begin the transfer of an item. The item itself is passive.
 		//  launch_item() ensures the WareInstance is transported out of the
-		//  warehouse Once it's on the flag, the flag code will decide what to
+		//  warehouse. Once it's on the flag, the flag code will decide what to
 		//  do with it.
 		WareInstance & item = supp.launch_item(game, *this);
 		ss.Unsigned32(item.serial());

=== modified file 'src/economy/routeastar.h'
--- src/economy/routeastar.h	2011-11-30 21:38:37 +0000
+++ src/economy/routeastar.h	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011 by the Widelands Development Team
+ * Copyright (C) 2011-2013 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
@@ -173,6 +173,14 @@
 	Coords m_dest;
 };
 
+/**
+ * This estimator, for use with @ref RouteAStar, always returns zero,
+ * which means that the resulting search is effectively Dijkstra's algorithm.
+ */
+struct AStarZeroEstimator {
+	int32_t operator()(RoutingNode &) const {return 0;}
+};
+
 } // namespace Widelands
 
 #endif // ECONOMY_ROUTEASTAR_H

=== modified file 'src/economy/shippingitem.cc'
--- src/economy/shippingitem.cc	2011-11-30 21:38:37 +0000
+++ src/economy/shippingitem.cc	2013-02-11 18:07:34 +0000
@@ -73,8 +73,12 @@
 	Worker * worker;
 	get(game, ware, worker);
 
-	if (ware)
-		ware->set_location(game, obj);
+	if (ware) {
+		if (upcast(Building, building, obj))
+			ware->enter_building(game, *building);
+		else
+			ware->set_location(game, obj);
+	}
 	if (worker) {
 		worker->set_location(dynamic_cast<PlayerImmovable *>(obj));
 		if (upcast(Building, building, obj)) {

=== modified file 'src/economy/transfer.cc'
--- src/economy/transfer.cc	2012-02-15 21:25:34 +0000
+++ src/economy/transfer.cc	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004, 2006-2009 by the Widelands Development Team
+ * Copyright (C) 2004, 2006-2013 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
@@ -168,8 +168,12 @@
 		return &locflag == location ? destination : &locflag;
 
 	// Brute force: recalculate the best route every time
-	if (!locflag.get_economy()->find_route(locflag, destflag, &m_route, m_item ? wwWARE : wwWORKER))
-		throw wexception("Transfer::get_next_step: inconsistent economy");
+	if (!locflag.get_economy()->find_route(locflag, destflag, &m_route, m_item ? wwWARE : wwWORKER)) {
+		tlog("destination appears to have become split from current location -> fail\n");
+		Economy::check_split(locflag, destflag);
+		success = false;
+		return 0;
+	}
 
 	if (m_route.get_nrsteps() >= 1)
 		if (upcast(Road const, road, location))
@@ -184,18 +188,6 @@
 				 &m_route.get_flag(m_game, m_route.get_nrsteps() - 1))
 				m_route.truncate(m_route.get_nrsteps() - 1);
 
-#if 1
-	if (m_item && (m_item->serial() == 1265)) {
-		log
-			("Item %u ready at location %u (flag %u) for destination %u\n",
-			 m_item->serial(), location->serial(), locflag.serial(), destination->serial());
-		for (int i = 0; i <= m_route.get_nrsteps() && i < 5; ++i) {
-			log("  %i: flag %u\n", i, m_route.get_flag(m_game, i).serial());
-		}
-		log("---\n");
-	}
-#endif
-
 	// Reroute into PortDocks or the associated warehouse when appropriate
 	if (m_route.get_nrsteps() >= 1) {
 		Flag & curflag(m_route.get_flag(m_game, 0));

=== modified file 'src/economy/ware_instance.cc'
--- src/economy/ware_instance.cc	2013-02-10 19:36:24 +0000
+++ src/economy/ware_instance.cc	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004, 2006-2011 by the Widelands Development Team
+ * Copyright (C) 2004, 2006-2013 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
@@ -261,12 +261,16 @@
 	if (location) {
 		Economy * eco = 0;
 
-		if (upcast(PlayerImmovable const, playerimmovable, location))
-			eco = playerimmovable->get_economy();
+		if (upcast(Flag const, flag, location))
+			eco = flag->get_economy();
 		else if (upcast(Worker const, worker, location))
 			eco = worker->get_economy();
+		else if (upcast(PortDock const, portdock, location))
+			eco = portdock->get_economy();
 		else if (upcast(Ship const, ship, location))
 			eco = ship->get_economy();
+		else
+			throw wexception("WareInstance delivered to bad location %u", location->serial());
 
 		if (oldlocation && get_economy()) {
 			if (get_economy() != eco)
@@ -306,7 +310,7 @@
 {
 	Map_Object * const loc = m_location.get(game);
 
-	if (!m_descr) // Upsy, we're not even intialized. Happens on load
+	if (!m_descr) // Upsy, we're not even initialized. Happens on load
 		return;
 
 	// Reset our state if we're not on location or outside an economy
@@ -316,7 +320,8 @@
 	}
 
 	if (!loc) {
-		remove(game);
+		// If our location gets lost, our owner is supposed to destroy us
+		log("Warning(%u): WareInstance::update has no location\n", serial());
 		return;
 	}
 
@@ -361,15 +366,50 @@
 			}
 		}
 
-		if (upcast(Building, building, location)) {
-			if (nextstep != &location->base_flag()) {
-				if (upcast(PortDock, pd, nextstep)) {
-					pd->add_shippingitem(game, *this);
-					return;
-				}
-
-				throw wexception
-					("MO(%u): ware: move from building to non-baseflag", serial());
+		if (upcast(Flag, flag, location)) {
+			flag->call_carrier
+				(game,
+				 *this,
+				 dynamic_cast<Building const *>(nextstep)
+				 &&
+				 &nextstep->base_flag() != location
+				 ?
+				 &nextstep->base_flag() : nextstep);
+		} else if (upcast(PortDock, pd, location)) {
+			pd->update_shippingitem(game, *this);
+		} else {
+			throw wexception("Ware_Instance::update in bad type of PlayerImmovable %u", location->serial());
+		}
+	}
+}
+
+/**
+ * Called by a worker when it carries the ware into the given building.
+ */
+void WareInstance::enter_building(Game & game, Building & building)
+{
+	if (m_transfer) {
+		if (m_transfer->get_destination(game) == &building) {
+			Transfer * t = m_transfer;
+
+			m_transfer = 0;
+			m_transfer_nextstep = 0;
+
+			t->has_finished();
+			return;
+		}
+
+		bool success;
+		PlayerImmovable * const nextstep =
+			m_transfer->get_next_step(&building, success);
+		m_transfer_nextstep = nextstep;
+
+		if (success) {
+			assert(nextstep);
+
+			if (upcast(PortDock, pd, nextstep)) {
+				pd->add_shippingitem(game, *this);
+				return;
 			}
 
 			// There are some situations where we might end up in a warehouse
@@ -380,30 +420,33 @@
 			//  - we were carried into a harbour/warehouse to be
 			//    shipped across the sea, but a better, land-based route has been
 			//    found
-			if (upcast(Warehouse, warehouse, building)) {
+			if (upcast(Warehouse, warehouse, &building)) {
 				warehouse->do_launch_item(game, *this);
 				return;
 			}
 
 			throw wexception
-				("MO(%u): ware(%s): can not move from building %u (%s at (%u,%u)) "
+				("MO(%u): ware(%s): do not know how to move from building %u (%s at (%u,%u)) "
 				 "to %u (%s) -> not a warehouse!",
-				 serial(), m_descr->name().c_str(), location->serial(),
-				 building->name().c_str(), building->get_position().x,
-				 building->get_position().y, nextstep->serial(),
+				 serial(), m_descr->name().c_str(), building.serial(),
+				 building.name().c_str(), building.get_position().x,
+				 building.get_position().y, nextstep->serial(),
 				 nextstep->name().c_str());
-		} else if (upcast(Flag, flag, location)) {
-			flag->call_carrier
-				(game,
-				 *this,
-				 dynamic_cast<Building const *>(nextstep)
-				 &&
-				 &nextstep->base_flag() != location
-				 ?
-				 &nextstep->base_flag() : nextstep);
-		} else if (upcast(PortDock, pd, location)) {
-			pd->update_shippingitem(game, *this);
+		} else {
+			Transfer * t = m_transfer;
+
+			m_transfer = 0;
+			m_transfer_nextstep = 0;
+
+			t->has_failed();
+			cancel_moving();
+			update(game);
+			return;
 		}
+	} else {
+		// We don't have a transfer, so just enter the building
+		building.receive_ware(game, m_descr_index);
+		remove(game);
 	}
 }
 

=== modified file 'src/economy/ware_instance.h'
--- src/economy/ware_instance.h	2012-04-28 10:57:54 +0000
+++ src/economy/ware_instance.h	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004, 2006-2009 by the Widelands Development Team
+ * Copyright (C) 2004, 2006-2013 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
@@ -28,6 +28,7 @@
 
 namespace Widelands {
 
+struct Building;
 struct Economy;
 struct Editor_Game_Base;
 struct Game;
@@ -42,11 +43,15 @@
  * The WareInstance never draws itself; the carrying worker or the current flag
  * location are responsible for that.
  *
- * The location of a ware can be one of the following:
- * \li a flag
- * \li a worker that is currently carrying the ware
- * \li a building; this should only be temporary until the ware is incorporated
- *     into the building somehow
+ * For robustness reasons, a WareInstance can only exist in a location that
+ * assumes responsible for updating the instance's economy via \ref set_economy,
+ * and that destroys the WareInstance when the location is destroyed.
+ *
+ * Currently, the location of a ware can be one of the following:
+ * \li a \ref Flag
+ * \li a \ref Worker that is currently carrying the ware
+ * \li a \ref PortDock or \ref Ship where the ware is encapsulated in a \ref ShippingItem
+ *     for seafaring
  */
 class WareInstance : public Map_Object {
 	friend struct Map_Waredata_Data_Packet;
@@ -74,6 +79,8 @@
 	void set_location(Editor_Game_Base &, Map_Object * loc);
 	void set_economy(Economy *);
 
+	void enter_building(Game &, Building & building);
+
 	bool is_moving() const throw ();
 	void cancel_moving();
 

=== modified file 'src/graphic/render/gamerenderer_gl.cc'
--- src/graphic/render/gamerenderer_gl.cc	2013-02-10 19:49:54 +0000
+++ src/graphic/render/gamerenderer_gl.cc	2013-02-11 18:07:34 +0000
@@ -504,7 +504,7 @@
 	if (m_player && !m_player->see_all()) {
 		const Map & map = m_egbase->map();
 		const Player::Field & pf = m_player->fields()[Map::get_index(coords, map.get_width())];
-		return pf.roads | map.get_overlay_manager().get_road_overlay(coords);
+		return pf.roads | map.overlay_manager().get_road_overlay(coords);
 	} else {
 		return coords.field->get_roads();
 	}

=== modified file 'src/logic/carrier.cc'
--- src/logic/carrier.cc	2013-02-10 19:36:24 +0000
+++ src/logic/carrier.cc	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002-2004, 2006-2010 by the Widelands Development Team
+ * Copyright (C) 2002-2004, 2006-2013 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
@@ -226,7 +226,7 @@
 	if (dynamic_cast<Flag const *>(pos))
 		return pop_task(game); //  we are done
 	else if (upcast(Building, building, pos)) {
-		// Drop all items addresed to this building
+		// Drop all items addressed to this building
 		while (WareInstance * const item = get_carried_item(game)) {
 			// If the building has disappeared and immediately been replaced
 			// with another building, we might have to return without dropping
@@ -235,8 +235,7 @@
 
 			if (next == pos) {
 				fetch_carried_item(game);
-				item->set_location(game, building);
-				item->update      (game);
+				item->enter_building(game, *building);
 			} else {
 				molog
 					("[Carrier]: Building switch from under us, return to road.\n");

=== modified file 'src/logic/map.h'
--- src/logic/map.h	2013-02-10 19:36:24 +0000
+++ src/logic/map.h	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002-2004, 2006-2011 by the Widelands Development Team
+ * Copyright (C) 2002-2004, 2006-2013 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
@@ -149,8 +149,8 @@
 	Map ();
 	virtual ~Map();
 
-	Overlay_Manager * get_overlay_manager()       {return  m_overlay_manager;}
-	Overlay_Manager & get_overlay_manager() const {return *m_overlay_manager;}
+	Overlay_Manager * get_overlay_manager()       {return m_overlay_manager;}
+	Overlay_Manager * get_overlay_manager() const {return m_overlay_manager;}
 	const Overlay_Manager & overlay_manager() const {return *m_overlay_manager;}
 	Overlay_Manager       & overlay_manager()       {return *m_overlay_manager;}
 

=== modified file 'src/logic/productionsite.cc'
--- src/logic/productionsite.cc	2013-02-10 19:36:24 +0000
+++ src/logic/productionsite.cc	2013-02-11 18:07:34 +0000
@@ -713,7 +713,6 @@
 
 	// Default actions first
 	if (WareInstance * const item = worker.fetch_carried_item(game)) {
-
 		worker.start_task_dropoff(game, *item);
 		return true;
 	}

=== modified file 'src/logic/warehouse.cc'
--- src/logic/warehouse.cc	2013-02-10 20:07:27 +0000
+++ src/logic/warehouse.cc	2013-02-11 18:07:34 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002-2004, 2006-2011 by the Widelands Development Team
+ * Copyright (C) 2002-2004, 2006-2013 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
@@ -919,15 +919,10 @@
 	WareInstance & item =
 		*new WareInstance(ware, tribe().get_ware_descr(ware));
 	item.init(game);
+	do_launch_item(game, item);
 
 	m_supply->remove_wares(ware, 1);
 
-	// Schedule a call of WareInstance::update, which will either carry the
-	// item out of the warehouse via do_launch_item, or move it into the attached
-	// dock.
-	item.set_location(game, this);
-	item.schedule_act(game, 1);
-
 	return item;
 }
 

=== modified file 'src/logic/worker.cc'
--- src/logic/worker.cc	2013-02-10 19:36:24 +0000
+++ src/logic/worker.cc	2013-02-11 18:07:34 +0000
@@ -2239,8 +2239,7 @@
 
 	if (WareInstance * const item = fetch_carried_item(game)) {
 		if (item->get_next_move_step(game) == location) {
-			item->set_location(game, location);
-			item->update(game); //  this might remove the item and ack any requests
+			item->enter_building(game, *dynamic_cast<Building *>(location));
 		} else {
 			// The item changed its mind and doesn't want to go to this building
 			// after all, so carry it back out.
@@ -2761,8 +2760,9 @@
 		("  Try scouting for %i ms with search in radius of %i\n",
 		 action.iparam2, action.iparam1);
 
+	++state.ivar1;
 	start_task_scout(game, action.iparam1, action.iparam2);
-	++state.ivar1;
+	// state reference may be invalid now
 	return true;
 }