widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/remove-anti-congestion-algorithm into lp:widelands.

Commit message:
Remove the anti-congestion algorithm introduced in r8775 because it has too many bugs in it.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1797213 in widelands: "Sometimes ware(s) are lying at a flag and get not transported."
  https://bugs.launchpad.net/widelands/+bug/1797213

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

=== modified file 'src/economy/flag.cc'
--- src/economy/flag.cc	2018-09-04 15:48:47 +0000
+++ src/economy/flag.cc	2018-11-23 16:57:35 +0000
@@ -337,30 +337,7 @@
 }
 
 /**
- * Called by workers wanting to drop a ware to their building's flag.
- * \return true/allow on low congestion-risk.
- */
-bool Flag::has_capacity_for_ware(WareInstance& ware) const {
-	// avoid iteration for the easy cases
-	if (ware_filled_ < ware_capacity_ - 2) {
-		return true;  // more than two free slots, allow
-	}
-	if (ware_filled_ >= ware_capacity_) {
-		return false;  // all slots filled, no room
-	}
-
-	DescriptionIndex const descr_index = ware.descr_index();
-	for (int i = 0; i < ware_filled_; ++i) {
-		if (wares_[i].ware->descr_index() == descr_index) {
-			return false;  // ware of this type already present, leave room for other types
-		}
-	}
-	return true;  // ware of this type not present, allow
-}
-
-/**
- * \return true/allow if the flag can hold more wares of any type.
- * has_capacity_for_ware() also checks ware's type to prevent congestion.
+ * Returns true if the flag can hold more wares.
  */
 bool Flag::has_capacity() const {
 	return (ware_filled_ < ware_capacity_);
@@ -386,24 +363,13 @@
 		capacity_wait_.erase(it);
 }
 
-/**
- * Adds given ware to this flag.
- * Please check has_capacity() or better has_capacity_for_ware() before.
- */
 void Flag::add_ware(EditorGameBase& egbase, WareInstance& ware) {
+
 	assert(ware_filled_ < ware_capacity_);
-	init_ware(egbase, ware, wares_[ware_filled_++]);
-	if (upcast(Game, game, &egbase)) {
-		ware.update(*game);  //  will call call_carrier() if necessary
-	}
-}
 
-/**
- * Properly assigns given ware instance to given pending ware.
- */
-void Flag::init_ware(EditorGameBase& egbase, WareInstance& ware, PendingWare& pi) {
+	PendingWare& pi = wares_[ware_filled_++];
 	pi.ware = &ware;
-	pi.pending = true;
+	pi.pending = false;
 	pi.nextstep = nullptr;
 	pi.priority = 0;
 
@@ -423,24 +389,32 @@
 	}
 
 	ware.set_location(egbase, this);
+
+	if (upcast(Game, game, &egbase)) {
+		ware.update(*game);  //  will call call_carrier() if necessary
+	}
 }
 
 /**
- * \return a ware currently waiting for a carrier to the given destflag.
+ * \return true if a ware is currently waiting for a carrier to the given Flag.
  *
  * \note Due to fetch_from_flag() semantics, this function makes no sense
  * for a  building destination.
 */
-Flag::PendingWare* Flag::get_ware_for_flag(Flag& destflag, bool const pending_only) {
+bool Flag::has_pending_ware(Game&, Flag& dest) {
 	for (int32_t i = 0; i < ware_filled_; ++i) {
-		PendingWare* pw = &wares_[i];
-		if ((!pending_only || pw->pending) && pw->nextstep == &destflag &&
-		    destflag.allow_ware_from_flag(*pw->ware, *this)) {
-			return pw;
-		}
+		if (!wares_[i].pending) {
+			continue;
+		}
+
+		if (wares_[i].nextstep != &dest) {
+			continue;
+		}
+
+		return true;
 	}
 
-	return nullptr;
+	return false;
 }
 
 /**
@@ -450,124 +424,109 @@
 #define MAX_TRANSFER_PRIORITY 16
 
 /**
- * Called by the carriercode when the carrier is called away from his job
- * but has acknowledged a ware before. This ware is then freed again
- * to be picked by another carrier. Returns true if a ware was indeed
- * made pending again.
+ * Called by carrier code to indicate that the carrier is moving to pick up an
+ * ware. Ware with highest transfer priority is chosen.
+ * \return true if an ware is actually waiting for the carrier.
  */
-bool Flag::cancel_pickup(Game& game, Flag& destflag) {
+bool Flag::ack_pickup(Game&, Flag& destflag) {
+	int32_t highest_pri = -1;
+	int32_t i_pri = -1;
+
 	for (int32_t i = 0; i < ware_filled_; ++i) {
-		PendingWare& pw = wares_[i];
-		if (!pw.pending && pw.nextstep == &destflag) {
-			pw.pending = true;
-			pw.ware->update(game);  //  will call call_carrier() if necessary
-			return true;
-		}
+		if (!wares_[i].pending) {
+			continue;
+		}
+
+		if (wares_[i].nextstep != &destflag) {
+			continue;
+		}
+
+		if (wares_[i].priority > highest_pri) {
+			highest_pri = wares_[i].priority;
+			i_pri = i;
+
+			// Increase ware priority, it matters only if the ware has to wait.
+			if (wares_[i].priority < MAX_TRANSFER_PRIORITY) {
+				wares_[i].priority++;
+			}
+		}
+	}
+
+	if (i_pri >= 0) {
+		wares_[i_pri].pending = false;
+		return true;
 	}
 
 	return false;
 }
-
 /**
  * Called by carrier code to find the best among the wares on this flag
  * that are meant for the provided dest.
  * \return index of found ware (carrier will take it)
  * or kNotFoundAppropriate (carrier will leave empty-handed)
  */
-int32_t Flag::find_pending_ware(PlayerImmovable& dest) {
-	int32_t highest_pri = -1;
-	int32_t best_index = kNotFoundAppropriate;
-	bool ware_pended = false;
-
-	for (int32_t i = 0; i < ware_filled_; ++i) {
-		PendingWare& pw = wares_[i];
-		if (pw.nextstep != &dest) {
-			continue;
-		}
-
-		if (pw.priority < MAX_TRANSFER_PRIORITY) {
-			pw.priority++;
-		}
-		// Release promised pickup, in case we find a preferable ware
-		if (!ware_pended && !pw.pending) {
-			ware_pended = pw.pending = true;
-		}
-
-		// If dest is flag, we exclude wares that can stress it
-		if (&dest != building_ && !dynamic_cast<Flag&>(dest).allow_ware_from_flag(*pw.ware, *this)) {
-			continue;
-		}
-
-		if (pw.priority > highest_pri) {
-			highest_pri = pw.priority;
-			best_index = i;
-		}
-	}
-
-	return best_index;
-}
-
-/**
- * Like find_pending_ware() above, but for carriers who have a ware to drop on this flag.
- * \return same as find_pending_ware() above, plus kDenyDrop (carrier will wait)
- */
-int32_t Flag::find_swappable_ware(WareInstance& ware, Flag& destflag) {
-	DescriptionIndex const descr_index = ware.descr_index();
-	int32_t highest_pri = -1;
-	int32_t best_index = kNotFoundAppropriate;
-	bool has_same_ware = false;
-	bool has_allowed = false;
-	bool ware_pended = false;
-
-	for (int32_t i = 0; i < ware_filled_; ++i) {
-		PendingWare& pw = wares_[i];
-		if (pw.nextstep != &destflag) {
-			if (pw.ware->descr_index() == descr_index) {
-				has_same_ware = true;
-			}
-			continue;
-		}
-
-		if (pw.priority < MAX_TRANSFER_PRIORITY) {
-			pw.priority++;
-		}
-		// Release promised pickup, in case we find a preferable ware
-		if (!ware_pended && !pw.pending) {
-			ware_pended = pw.pending = true;
-		}
-
-		// We prefer to retrieve wares that won't stress the destflag
-		if (destflag.allow_ware_from_flag(*pw.ware, *this)) {
-			if (!has_allowed) {
-				has_allowed = true;
-				highest_pri = -1;
-			}
-		} else {
-			if (has_allowed) {
-				continue;
-			}
-		}
-
-		if (pw.priority > highest_pri) {
-			highest_pri = pw.priority;
-			best_index = i;
-		}
-	}
-
-	if (best_index > kNotFoundAppropriate) {
-		return (ware_filled_ > ware_capacity_ - 3 || has_allowed) ? best_index : kNotFoundAppropriate;
-	} else {
-		return (ware_filled_ < ware_capacity_ - 2 ||
-		        (ware_filled_ < ware_capacity_ && !has_same_ware)) ?
-		          kNotFoundAppropriate :
-		          kDenyDrop;
-	}
-}
-
-/**
- * Called by carrier code to retrieve a ware found by the previous methods.
- */
-WareInstance* Flag::fetch_pending_ware(Game& game, int32_t best_index) {
+bool Flag::cancel_pickup(Game& game, Flag& destflag) {
+	int32_t lowest_prio = MAX_TRANSFER_PRIORITY + 1;
+	int32_t i_pri = -1;
+
+	for (int32_t i = 0; i < ware_filled_; ++i) {
+		if (wares_[i].pending) {
+			continue;
+		}
+
+		if (wares_[i].nextstep != &destflag) {
+			continue;
+		}
+
+		if (wares_[i].priority < lowest_prio) {
+			lowest_prio = wares_[i].priority;
+			i_pri = i;
+		}
+	}
+
+	if (i_pri >= 0) {
+		wares_[i_pri].pending = true;
+		wares_[i_pri].ware->update(game);  //  will call call_carrier() if necessary
+		return true;
+	}
+
+	return false;
+}
+
+/**
+ * Wake one sleeper from the capacity queue.
+*/
+void Flag::wake_up_capacity_queue(Game& game) {
+	while (!capacity_wait_.empty()) {
+		Worker* const w = capacity_wait_.front().get(game);
+		capacity_wait_.pop_front();
+		if (w && w->wakeup_flag_capacity(game, *this)) {
+			break;
+		}
+	}
+}
+
+/**
+ * Called by carrier code to retrieve one of the wares on the flag that is meant
+ * for that carrier.
+ *
+ * This function may return 0 even if \ref ack_pickup() has already been
+ * called successfully.
+*/
+WareInstance* Flag::fetch_pending_ware(Game& game, PlayerImmovable& dest) {
+	int32_t best_index = -1;
+
+	for (int32_t i = 0; i < ware_filled_; ++i) {
+		if (wares_[i].nextstep != &dest) {
+			continue;
+		}
+
+		// We prefer to retrieve wares that have already been acked
+		if (best_index < 0 || !wares_[i].pending) {
+			best_index = i;
+		}
+	}
+
 	if (best_index < 0) {
 		return nullptr;
 	}
@@ -579,43 +538,14 @@
 	        sizeof(wares_[0]) * (ware_filled_ - best_index));
 
 	ware->set_location(game, nullptr);
+
+	// wake up capacity wait queue
+	wake_up_capacity_queue(game);
+
 	return ware;
 }
 
 /**
- * Called by carrier code to notify waiting carriers
- * which may be interested in the new state of this flag.
- */
-void Flag::ware_departing(Game& game) {
-	// Wake up one sleeper from the capacity queue.
-	while (!capacity_wait_.empty()) {
-		Worker* const w = capacity_wait_.front().get(game);
-		capacity_wait_.erase(capacity_wait_.begin());
-		if (w && w->wakeup_flag_capacity(game, *this)) {
-			return;
-		}
-	}
-
-	// Consider pending wares of neighboring flags.
-	for (int32_t dir = 1; dir <= WalkingDir::LAST_DIRECTION; ++dir) {
-		Road* const road = get_road(dir);
-		if (!road) {
-			continue;
-		}
-
-		Flag* other = &road->get_flag(Road::FlagEnd);
-		if (other == this) {
-			other = &road->get_flag(Road::FlagStart);
-		}
-
-		PendingWare* pw = other->get_ware_for_flag(*this, kPendingOnly);
-		if (pw && road->notify_ware(game, *other)) {
-			pw->pending = false;
-		}
-	}
-}
-
-/**
  * Accelerate potential promotion of roads adjacent to a newly promoted road.
  */
 void Flag::propagate_promoted_road(Road* const promoted_road) {
@@ -684,7 +614,7 @@
 		memmove(&wares_[i], &wares_[i + 1], sizeof(wares_[0]) * (ware_filled_ - i));
 
 		if (upcast(Game, game, &egbase)) {
-			ware_departing(*game);
+			wake_up_capacity_queue(*game);
 		}
 
 		return;
@@ -755,20 +685,27 @@
 
 	for (int32_t dir = 1; dir <= 6; ++dir) {
 		Road* const road = get_road(dir);
+		Flag* other;
+		Road::FlagId flagid;
+
 		if (!road) {
 			continue;
 		}
 
-		Flag* other = &road->get_flag(Road::FlagEnd);
-		if (other == this) {
+		if (&road->get_flag(Road::FlagStart) == this) {
+			flagid = Road::FlagStart;
+			other = &road->get_flag(Road::FlagEnd);
+		} else {
+			flagid = Road::FlagEnd;
 			other = &road->get_flag(Road::FlagStart);
 		}
+
 		if (other != &nextflag) {
 			continue;
 		}
 
 		// Yes, this is the road we want; inform it
-		if (other->update_ware_from_flag(game, wares_[i], *road, *this)) {
+		if (road->notify_ware(game, flagid)) {
 			return;
 		}
 
@@ -782,72 +719,6 @@
 }
 
 /**
- * Called by neighboring flags, before agreeing for a carrier
- * to take one of their wares heading to this flag.
- * \return true/allow on low congestion-risk.
- */
-bool Flag::allow_ware_from_flag(WareInstance& ware, Flag& flag) {
-	// avoid iteration for the easy cases
-	if (ware_filled_ < ware_capacity_ - 2) {
-		return true;
-	}
-
-	DescriptionIndex const descr_index = ware.descr_index();
-	bool has_swappable = false;
-	for (int i = 0; i < ware_filled_; ++i) {
-		PendingWare& pw = wares_[i];
-		if (pw.pending && pw.nextstep == &flag) {
-			has_swappable = true;
-		} else if (pw.ware->descr_index() == descr_index) {
-			return false;
-		}
-	}
-	return ware_filled_ < ware_capacity_ || has_swappable;
-}
-
-/**
- * Called when a ware is trying to reach this flag through the provided road,
- * having just arrived to the provided flag.
- * Swaps pending wares if possible. Otherwise,
- * asks road for carrier on low congestion-risk.
- * \return false if the ware is not immediately served.
- */
-bool Flag::update_ware_from_flag(Game& game, PendingWare& pw1, Road& road, Flag& flag) {
-	WareInstance& w1 = *pw1.ware;
-	DescriptionIndex const w1_descr_index = w1.descr_index();
-	bool has_same_ware = false;
-	bool has_swappable = false;
-	for (int i = 0; i < ware_filled_; ++i) {
-		PendingWare& pw2 = wares_[i];
-		WareInstance& w2 = *pw2.ware;
-		if (w2.descr_index() == w1_descr_index) {
-			if (pw2.nextstep == &flag) {
-				// swap pending wares remotely
-				init_ware(game, w1, pw2);
-				flag.init_ware(game, w2, pw1);
-				w1.update(game);
-				w2.update(game);
-				return true;
-			}
-
-			has_same_ware = true;
-		} else if (pw2.pending && pw2.nextstep == &flag) {
-			has_swappable = true;
-		}
-	}
-
-	// ask road for carrier on low congestion-risk
-	if (ware_filled_ < ware_capacity_ - 2 ||
-	    (!has_same_ware && (ware_filled_ < ware_capacity_ || has_swappable))) {
-		if (road.notify_ware(game, flag)) {
-			pw1.pending = false;
-			return true;
-		}
-	}
-	return false;
-}
-
-/**
  * Called whenever a road gets broken or split.
  * Make sure all wares on this flag are rerouted if necessary.
  *

=== modified file 'src/economy/flag.h'
--- src/economy/flag.h	2018-09-25 06:32:35 +0000
+++ src/economy/flag.h	2018-11-23 16:57:35 +0000
@@ -47,10 +47,6 @@
 	DISALLOW_COPY_AND_ASSIGN(FlagDescr);
 };
 
-constexpr bool kPendingOnly = true;           // ignore non-pending wares
-constexpr int32_t kNotFoundAppropriate = -1;  // no ware appropiate for carrying
-constexpr int32_t kDenyDrop = -2;             // flag is full and no ware appropiate for swapping
-
 /**
  * Flag represents a flag as you see it on the map.
  *
@@ -125,15 +121,7 @@
 
 	bool is_dead_end() const;
 
-	struct PendingWare {
-		WareInstance* ware;              ///< the ware itself
-		bool pending;                    ///< if the ware is pending
-		int32_t priority;                ///< carrier prefers the ware with highest priority
-		OPtr<PlayerImmovable> nextstep;  ///< next step that this ware is sent to
-	};
-
 	bool has_capacity() const;
-	bool has_capacity_for_ware(WareInstance&) const;
 	uint32_t total_capacity() {
 		return ware_capacity_;
 	}
@@ -143,14 +131,10 @@
 	void wait_for_capacity(Game&, Worker&);
 	void skip_wait_for_capacity(Game&, Worker&);
 	void add_ware(EditorGameBase&, WareInstance&);
-	void init_ware(EditorGameBase&, WareInstance&, PendingWare&);
-	PendingWare* get_ware_for_flag(Flag&, bool pending_only = false);
+	bool has_pending_ware(Game&, Flag& destflag);
+	bool ack_pickup(Game&, Flag& destflag);
 	bool cancel_pickup(Game&, Flag& destflag);
-	void ware_departing(Game&);
-	bool allow_ware_from_flag(WareInstance&, Flag&);
-	int32_t find_swappable_ware(WareInstance&, Flag&);
-	int32_t find_pending_ware(PlayerImmovable&);
-	WareInstance* fetch_pending_ware(Game&, int32_t);
+	WareInstance* fetch_pending_ware(Game&, PlayerImmovable& dest);
 	void propagate_promoted_road(Road* promoted_road);
 	Wares get_wares();
 	uint8_t count_wares_in_queue(PlayerImmovable& dest) const;
@@ -171,13 +155,20 @@
 	void
 	draw(uint32_t gametime, const Vector2f& point_on_dst, float scale, RenderTarget* dst) override;
 
+	void wake_up_capacity_queue(Game&);
+
 	static void
 	flag_job_request_callback(Game&, Request&, DescriptionIndex, Worker*, PlayerImmovable&);
 
 	void set_flag_position(Coords coords);
 
 private:
-	bool update_ware_from_flag(Game&, PendingWare&, Road&, Flag&);
+	struct PendingWare {
+		WareInstance* ware;              ///< the ware itself
+		bool pending;                    ///< if the ware is pending
+		int32_t priority;                ///< carrier prefers the ware with highest priority
+		OPtr<PlayerImmovable> nextstep;  ///< next step that this ware is sent to
+	};
 
 	struct FlagJob {
 		Request* request;

=== modified file 'src/economy/road.cc'
--- src/economy/road.cc	2018-09-04 15:48:47 +0000
+++ src/economy/road.cc	2018-11-23 16:57:35 +0000
@@ -542,8 +542,7 @@
  * Try to pick up a ware from the given flag.
  * \return true if a carrier has been sent on its way, false otherwise.
  */
-bool Road::notify_ware(Game& game, Flag& flag) {
-	FlagId flagid = &flag == flags_[Road::FlagEnd] ? Road::FlagEnd : Road::FlagStart;
+bool Road::notify_ware(Game& game, FlagId const flagid) {
 	// Iterate over all carriers and try to find one which will take the ware
 	for (CarrierSlot& slot : carrier_slots_) {
 		if (Carrier* const carrier = slot.carrier.get(game)) {

=== modified file 'src/economy/road.h'
--- src/economy/road.h	2018-09-25 06:32:35 +0000
+++ src/economy/road.h	2018-11-23 16:57:35 +0000
@@ -113,7 +113,7 @@
 	void presplit(Game&, Coords split);
 	void postsplit(Game&, Flag&);
 
-	bool notify_ware(Game& game, Flag& flag);
+	bool notify_ware(Game& game, FlagId flagid);
 	void update_wallet_chargetime(Game& game);
 	void charge_wallet(Game& game);
 	int32_t wallet() const;
@@ -148,8 +148,8 @@
 	uint8_t carriers_count() const;
 
 private:
-	/// Counter that is incremented for every ware served by this road
-	/// according to the delay of service and decremented over time.
+	/// Counter that is incremented when a ware does not get a carrier for this
+	/// road immediately and decremented over time.
 	int32_t wallet_;
 
 	/// holds the gametime when wallet_ was last charged

=== modified file 'src/logic/map_objects/tribes/carrier.cc'
--- src/logic/map_objects/tribes/carrier.cc	2018-10-30 18:40:57 +0000
+++ src/logic/map_objects/tribes/carrier.cc	2018-11-23 16:57:35 +0000
@@ -51,7 +51,7 @@
 
 	top_state().ivar1 = 0;
 
-	operation_ = INIT;
+	promised_pickup_to_ = NOONE;
 }
 
 /**
@@ -78,14 +78,15 @@
 		return pop_task(game);
 	}
 
-	if (operation_ == INIT) {
-		operation_ = find_source_flag(game);
+	// Check for pending wares
+	if (promised_pickup_to_ == NOONE) {
+		find_pending_ware(game);
 	}
 
-	if (operation_ > NO_OPERATION) {
+	if (promised_pickup_to_ != NOONE) {
 		if (state.ivar1) {
 			state.ivar1 = 0;
-			return start_task_transport(game, operation_);
+			return start_task_transport(game, promised_pickup_to_);
 		} else {
 			// Short delay before we move to pick up
 			state.ivar1 = 1;
@@ -119,10 +120,10 @@
  * a ware there, we have to make sure that they do not count on us anymore.
  */
 void Carrier::road_pop(Game& game, State& /* state */) {
-	if (operation_ > NO_OPERATION && get_location(game)) {
+	if (promised_pickup_to_ != NOONE && get_location(game)) {
 		Road& road = dynamic_cast<Road&>(*get_location(game));
-		Flag& flag = road.get_flag(static_cast<Road::FlagId>(operation_));
-		Flag& otherflag = road.get_flag(static_cast<Road::FlagId>(operation_ ^ 1));
+		Flag& flag = road.get_flag(static_cast<Road::FlagId>(promised_pickup_to_));
+		Flag& otherflag = road.get_flag(static_cast<Road::FlagId>(promised_pickup_to_ ^ 1));
 
 		flag.cancel_pickup(game, otherflag);
 	}
@@ -160,99 +161,35 @@
 		return pop_task(game);
 	}
 
-	int32_t const ivar1 = state.ivar1;
-	if (ivar1 == -1) {
+	if (state.ivar1 == -1) {
 		// If we're "in" the target building, special code applies
-		return deliver_to_building(game, state);
-	}
-
-	WareInstance* ware = get_carried_ware(game);
-	if (ware) {
-		assert(ware->get_location(game) == this);
-	}
-
-	Road& road = dynamic_cast<Road&>(*get_location(game));
-	int32_t const dest = ware ? ivar1 ^ 1 : ivar1;
-	Flag& flag = road.get_flag(static_cast<Road::FlagId>(dest));
-
-	if (ware) {
-		// If the ware should go to the building attached to our flag,
-		// walk directly into said building
+		deliver_to_building(game, state);
+	} else if (!does_carry_ware()) {
+		// If we don't carry something, walk to the flag
+		pickup_from_flag(game, state);
+	} else {
+		Road& road = dynamic_cast<Road&>(*get_location(game));
+		// If the ware should go to the building attached to our flag, walk
+		// directly into said building
+		Flag& flag = road.get_flag(static_cast<Road::FlagId>(state.ivar1 ^ 1));
+
+		WareInstance& ware = *get_carried_ware(game);
+		assert(ware.get_location(game) == this);
+
 		// A sanity check is necessary, in case the building has been destroyed
+		PlayerImmovable* const next = ware.get_next_move_step(game);
 
-		PlayerImmovable* const next = ware->get_next_move_step(game);
 		if (next && next != &flag && &next->base_flag() == &flag) {
 			// pay some coins before entering the building,
 			// to compensate for the time to be spent in its street-segment
 			road.pay_for_building();
-
-			if (!start_task_walktoflag(game, dest)) {
-				// Enter building
-				state.ivar1 = -1;
-				start_task_move(game, WALK_NW, descr().get_right_walk_anims(does_carry_ware()), true);
-			}
-			return;
-		}
-	}
-
-	if (!start_task_walktoflag(game, dest, operation_ == WAIT)) {
-		// If the flag is overloaded we are allowed to drop wares,
-		// as long as we can pick another up. Otherwise we have to wait.
-
-		Flag& otherflag = road.get_flag(static_cast<Road::FlagId>(dest ^ 1));
-		int32_t otherware_idx =
-		   ware ? flag.find_swappable_ware(*ware, otherflag) : flag.find_pending_ware(otherflag);
-		if (operation_ == WAIT) {
-			if (otherware_idx < kNotFoundAppropriate) {
-				return start_task_waitforcapacity(game, flag);  // join flag's wait queue
-			} else {
-				operation_ = dest ^ 1;  // resume transport without joining flag's wait queue
-				set_animation(game, descr().get_animation("idle"));
-				return schedule_act(game, 20);
-			}
-		} else if (otherware_idx < kNotFoundAppropriate) {
-			operation_ = WAIT;  // move one node away
-			set_animation(game, descr().get_animation("idle"));
-			return schedule_act(game, 20);
-		}
-
-		WareInstance* otherware = flag.fetch_pending_ware(game, otherware_idx);
-
-		if (ware) {
-			const bool ware_astray = (ware->get_next_move_step(game) == nullptr);
-			// Drop our ware
-			flag.add_ware(game, *fetch_carried_ware(game));
-			// If the destination of the dropped ware changed while carrying it and we don't have
-			// anything else we should carry, we might pick it up again immediately, so check again
-			if (ware_astray && otherware_idx == kNotFoundAppropriate) {
-				otherware_idx = flag.find_pending_ware(otherflag);
-				otherware = flag.fetch_pending_ware(game, otherware_idx);
-			}
-		}
-
-		// Pick up new load, if any
-		if (otherware) {
-			// pay before getting the ware, while checking for road promotion
-			road.pay_for_road(game, flag.count_wares_in_queue(otherflag));
-
-			set_carried_ware(game, otherware);
-			flag.ware_departing(game);
-
-			operation_ = state.ivar1 = dest;
-			set_animation(game, descr().get_animation("idle"));
-			schedule_act(game, 20);
-		} else {
-			Flag::PendingWare* pw = otherflag.get_ware_for_flag(flag, kPendingOnly);
-			if (pw) {
-				pw->pending = false;
-
-				operation_ = state.ivar1 = dest ^ 1;
-				set_animation(game, descr().get_animation("idle"));
-				schedule_act(game, 20);
-			} else {
-				operation_ = NO_OPERATION;
-				pop_task(game);
-			}
+			enter_building(game, state);
+		} else if ((flag.has_capacity() || !swap_or_wait(game, state)) &&
+				   !start_task_walktoflag(game, state.ivar1 ^ 1)) {
+			// If the flag is overloaded we are allowed to drop wares as
+			// long as we can pick another up. Otherwise we have to wait.
+			// Drop the ware, possible exchanging it with another one
+			drop_ware(game, state);
 		}
 	}
 }
@@ -268,7 +205,6 @@
 	BaseImmovable* const pos = game.map()[get_position()].get_immovable();
 
 	if (dynamic_cast<Flag const*>(pos)) {
-		operation_ = INIT;
 		return pop_task(game);  //  we are done
 	} else if (upcast(Building, building, pos)) {
 		// Drop all wares addressed to this building
@@ -301,51 +237,211 @@
 }
 
 /**
- * Called by road code to indicate that the given flag
- * (0 = start, 1 = end) has a ware ready for transfer.
+ * Walks to the queued flag and picks up one acked ware
+ *
+ * \param g Game the carrier lives on
+ * \param s Flags sent to the task
+ */
+void Carrier::pickup_from_flag(Game& game, State& state) {
+	int32_t const ivar1 = state.ivar1;
+	if (!start_task_walktoflag(game, ivar1)) {
+
+		promised_pickup_to_ = NOONE;
+
+		Road& road = dynamic_cast<Road&>(*get_location(game));
+		Flag& flag = road.get_flag(static_cast<Road::FlagId>(ivar1));
+		Flag& otherflag = road.get_flag(static_cast<Road::FlagId>(ivar1 ^ 1));
+
+		// Are there wares to move between our flags?
+		if (WareInstance* const ware = flag.fetch_pending_ware(game, otherflag)) {
+			// pay before getting the ware, while checking for road promotion
+			road.pay_for_road(game, flag.count_wares_in_queue(otherflag));
+			set_carried_ware(game, ware);
+
+			set_animation(game, descr().get_animation("idle"));
+			return schedule_act(game, 20);
+		} else {
+			molog("[Carrier]: Nothing suitable on flag.\n");
+			return pop_task(game);
+		}
+	}
+}
+
+/**
+ * Drop one ware in a flag, and pick up a new one if we acked it
+ *
+ * \param g Game the carrier lives on.
+ * \param s Flags sent to the task
+ */
+void Carrier::drop_ware(Game& game, State& state) {
+	WareInstance* other = nullptr;
+	Road& road = dynamic_cast<Road&>(*get_location(game));
+	Flag& flag = road.get_flag(static_cast<Road::FlagId>(state.ivar1 ^ 1));
+	Flag& otherflag = road.get_flag(static_cast<Road::FlagId>(state.ivar1));
+
+	if (promised_pickup_to_ == (state.ivar1 ^ 1)) {
+		// If there's a ware we acked, we can drop ours even if the flag is
+		// flooded
+		other = flag.fetch_pending_ware(game, otherflag);
+
+		if (!other && !flag.has_capacity()) {
+			molog("[Carrier]: strange: acked ware from busy flag no longer "
+			      "present.\n");
+
+			promised_pickup_to_ = NOONE;
+			set_animation(game, descr().get_animation("idle"));
+			return schedule_act(game, 20);
+		}
+
+		state.ivar1 = promised_pickup_to_;
+		promised_pickup_to_ = NOONE;
+	}
+
+	// Drop our ware
+	flag.add_ware(game, *fetch_carried_ware(game));
+
+	// Pick up new load, if any
+	if (other) {
+		// pay before getting the ware, while checking for road promotion
+		road.pay_for_road(game, flag.count_wares_in_queue(otherflag));
+		set_carried_ware(game, other);
+
+		set_animation(game, descr().get_animation("idle"));
+		return schedule_act(game, 20);
+	} else {
+		return pop_task(game);
+	}
+}
+
+/**
+ * When picking up wares, if some of them is targeted to the building attached
+ * to target flag walk straight into it and deliver.
+ *
+ * \param g Game the carrier lives on.
+ * \param s Flags sent to the task.
+ */
+void Carrier::enter_building(Game& game, State& state) {
+	if (!start_task_walktoflag(game, state.ivar1 ^ 1)) {
+		state.ivar1 = -1;
+		return start_task_move(game, WALK_NW, descr().get_right_walk_anims(does_carry_ware()), true);
+	}
+}
+
+/**
+ * Swaps wares from an overloaded flag for as long as the carrier can pick
+ * up new wares from it. Otherwise, changes the carrier state to wait.
+ *
+ * \param g Game the carrier lives on.
+ * \param s Flags sent to the task.
+ *
+ * \return true if the carrier must wait before delivering his wares.
+ */
+bool Carrier::swap_or_wait(Game& game, State& state) {
+	// Road that employs us
+	Road& road = dynamic_cast<Road&>(*get_location(game));
+	// Flag we are delivering to
+	Flag& flag = road.get_flag(static_cast<Road::FlagId>(state.ivar1 ^ 1));
+	// The other flag of our road
+	Flag& otherflag = road.get_flag(static_cast<Road::FlagId>(state.ivar1));
+
+	if (promised_pickup_to_ == (state.ivar1 ^ 1)) {
+		// All is well, we already acked a ware that we can pick up
+		// from this flag
+		return false;
+	} else if (flag.has_pending_ware(game, otherflag)) {
+		if (!flag.ack_pickup(game, otherflag)) {
+			throw wexception(
+			   "MO(%u): transport: overload exchange: flag %u is fucked up", serial(), flag.serial());
+		}
+
+		promised_pickup_to_ = state.ivar1 ^ 1;
+		return false;
+	} else if (!start_task_walktoflag(game, state.ivar1 ^ 1, true)) {
+		start_task_waitforcapacity(game, flag);  //  wait one node away
+	}
+
+	return true;
+}
+
+/**
+ * Called by Road code to indicate that a new ware has arrived on a flag
+ * (0 = start, 1 = end).
  * \return true if the carrier is going to fetch it.
  */
 bool Carrier::notify_ware(Game& game, int32_t const flag) {
 	State& state = top_state();
 
-	if (operation_ == WAIT) {
-		if (state.objvar1.get(game) ==
-		    &dynamic_cast<Road&>(*get_location(game)).get_flag(static_cast<Road::FlagId>(flag))) {
-			operation_ = flag;
-			send_signal(game, "wakeup");
-			return true;
-		}
-	} else if (operation_ == NO_OPERATION) {
-		operation_ = flag;
+	// Check if we've already acked something
+	if (promised_pickup_to_ != NOONE)
+		return false;
+
+	// If we are currently in a transport.
+	// Explanation:
+	//  a) a different carrier / road may be better suited for this ware
+	//     (the transport code does not have priorities for the actual
+	//     carrier that is notified)
+	//  b) the transport task has logic that allows it to
+	//     drop a ware on an overloaded flag iff it can pick up a ware
+	//     at the same time.
+	//     We should ack said ware to avoid more confusion before we move
+	//     onto the flag, but we can't do that if we have already acked
+	//     something.
+	//  c) we might ack for a flag that we are actually moving away from;
+	//     this will get us into trouble if wares have arrived on the other
+	//     flag while we couldn't ack them.
+	//
+	// (Maybe the need for this lengthy explanation is proof that the
+	// ack system needs to be reworked.)
+	if (State const* const transport = get_state(taskTransport))
+		if ((transport->ivar1 == -1 && find_closest_flag(game) != flag) || flag == transport->ivar1)
+			return false;
+
+	// Ack it if we haven't
+	promised_pickup_to_ = flag;
+
+	if (state.task == &taskRoad) {
 		send_signal(game, "ware");
-		return true;
+	} else if (state.task == &taskWaitforcapacity) {
+		send_signal(game, "wakeup");
 	}
-
-	return false;
+	return true;
 }
 
 /**
- * Find a pending ware meant for our road,
- * remove its pending status, and
- * \return the flag it is on.
+ * Find a pending ware on one of the road's flags, ack it and set promised_pickup_to_
+ * accordingly.
  */
-int32_t Carrier::find_source_flag(Game& game) {
-	assert(operation_ == INIT);
-
+void Carrier::find_pending_ware(Game& game) {
 	Road& road = dynamic_cast<Road&>(*get_location(game));
-	int32_t near = find_closest_flag(game);
-	Flag& nearflag = road.get_flag(static_cast<Road::FlagId>(near));
-	Flag& farflag = road.get_flag(static_cast<Road::FlagId>(near ^ 1));
-
-	Flag::PendingWare* pw;
-	if ((pw = nearflag.get_ware_for_flag(farflag))) {
-		pw->pending = false;
-		return near;
-	} else if ((pw = farflag.get_ware_for_flag(nearflag, kPendingOnly))) {
-		pw->pending = false;
-		return near ^ 1;
-	} else {
-		return NO_OPERATION;
+	uint32_t havewarebits = 0;
+
+	assert(promised_pickup_to_ == NOONE);
+
+	if (road.get_flag(Road::FlagStart).has_pending_ware(game, road.get_flag(Road::FlagEnd))) {
+		havewarebits |= 1;
+	}
+
+	if (road.get_flag(Road::FlagEnd).has_pending_ware(game, road.get_flag(Road::FlagStart))) {
+		havewarebits |= 2;
+	}
+
+	//  If both flags have a ware, we pick the one closer to us.
+	if (havewarebits == 3) {
+		havewarebits = 1 << find_closest_flag(game);
+	}
+
+	// Ack our decision
+	if (havewarebits == 1) {
+		promised_pickup_to_ = START_FLAG;
+		if (!road.get_flag(Road::FlagStart).ack_pickup(game, road.get_flag(Road::FlagEnd))) {
+			throw wexception("Carrier::find_pending_ware: start flag is messed up");
+		}
+
+	} else if (havewarebits == 2) {
+		promised_pickup_to_ = END_FLAG;
+		if (!road.get_flag(Road::FlagEnd).ack_pickup(game, road.get_flag(Road::FlagStart))) {
+			throw wexception("Carrier::find_pending_ware: end flag is messed up");
+		}
 	}
 }
 
@@ -418,7 +514,7 @@
 
 	Worker::log_general_info(egbase);
 
-	molog("operation = %i\n", operation_);
+	molog("promised_pickup_to = %i\n", promised_pickup_to_);
 }
 
 /*
@@ -428,7 +524,7 @@
 
 ==============================
 */
-constexpr uint8_t kCurrentPacketVersion = 2;
+constexpr uint8_t kCurrentPacketVersion = 3;
 
 Carrier::Loader::Loader() {
 }
@@ -437,10 +533,11 @@
 	Worker::Loader::load(fr);
 
 	try {
-		uint8_t packet_version = fr.unsigned_8();
-		if (packet_version == kCurrentPacketVersion) {
+		const uint8_t packet_version = fr.unsigned_8();
+		// TODO(GunChleoc): Remove savegame compatibility after Build 21.
+		if (packet_version <= kCurrentPacketVersion && packet_version >= 1) {
 			Carrier& carrier = get<Carrier>();
-			carrier.operation_ = fr.signed_32();
+			carrier.promised_pickup_to_ = fr.signed_32();
 		} else {
 			throw UnhandledVersionError("Carrier", packet_version, kCurrentPacketVersion);
 		}
@@ -465,7 +562,7 @@
 	Worker::do_save(egbase, mos, fw);
 
 	fw.unsigned_8(kCurrentPacketVersion);
-	fw.signed_32(operation_);
+	fw.signed_32(promised_pickup_to_);
 }
 
 CarrierDescr::CarrierDescr(const std::string& init_descname,

=== modified file 'src/logic/map_objects/tribes/carrier.h'
--- src/logic/map_objects/tribes/carrier.h	2018-09-04 15:48:47 +0000
+++ src/logic/map_objects/tribes/carrier.h	2018-11-23 16:57:35 +0000
@@ -24,7 +24,6 @@
 #include "logic/map_objects/tribes/worker.h"
 
 namespace Widelands {
-class PendingWare;
 
 class CarrierDescr : public WorkerDescr {
 public:
@@ -50,7 +49,7 @@
 	MO_DESCR(CarrierDescr)
 
 	explicit Carrier(const CarrierDescr& carrier_descr)
-	   : Worker(carrier_descr), operation_(NO_OPERATION) {
+	   : Worker(carrier_descr), promised_pickup_to_(NOONE) {
 	}
 	~Carrier() override {
 	}
@@ -67,7 +66,7 @@
 	static Task const taskRoad;
 
 private:
-	int32_t find_source_flag(Game&);
+	void find_pending_ware(Game&);
 	int32_t find_closest_flag(Game&);
 
 	// internal task stuff
@@ -78,14 +77,17 @@
 	static Task const taskTransport;
 
 	void deliver_to_building(Game&, State&);
+	void pickup_from_flag(Game&, State&);
+	void drop_ware(Game&, State&);
+	void enter_building(Game&, State&);
+	bool swap_or_wait(Game&, State&);
 
+	/// -1: no ware acked; 0/1: acked ware for start/end flag of road
 	// This should be an enum, but this clutters the code with too many casts
-	static const int32_t INIT = -3;          // ready to undertake or resume operations
-	static const int32_t WAIT = -2;          // waiting for flag capacity
-	static const int32_t NO_OPERATION = -1;  // idling
-	static const int32_t START_FLAG = 0;     // serving start flag of road
-	static const int32_t END_FLAG = 1;       // serving end flag of road
-	int32_t operation_;
+	static const int32_t NOONE = -1;
+	static const int32_t START_FLAG = 0;
+	static const int32_t END_FLAG = 1;
+	int32_t promised_pickup_to_;
 
 	// saving and loading
 protected:

=== modified file 'src/logic/map_objects/tribes/worker.cc'
--- src/logic/map_objects/tribes/worker.cc	2018-11-07 10:19:29 +0000
+++ src/logic/map_objects/tribes/worker.cc	2018-11-23 16:57:35 +0000
@@ -137,11 +137,10 @@
 		totalres += amount;
 		totalchance += 8 * amount;
 
-		//  Add penalty for fields that are running out
-		//  Except for totally depleted fields or wrong ressource fields
-		//  if we already know there is no ressource (left) we won't mine there
+		// Add penalty for fields that are running out
 		if (amount == 0)
-			totalchance += 0;
+			// we already know it's completely empty, so punish is less
+			totalchance += 1;
 		else if (amount <= 2)
 			totalchance += 6;
 		else if (amount <= 4)
@@ -1819,11 +1818,12 @@
 		if (upcast(Flag, flag, pos)) {
 			// Is this "our" flag?
 			if (flag->get_building() == location) {
-				WareInstance* const ware = get_carried_ware(game);
-				if (state.ivar1 && ware && flag->has_capacity_for_ware(*ware)) {
-					flag->add_ware(game, *fetch_carried_ware(game));
-					set_animation(game, descr().get_animation("idle"));
-					return schedule_act(game, 20);  //  rest a while
+				if (state.ivar1 && flag->has_capacity()) {
+					if (WareInstance* const ware = fetch_carried_ware(game)) {
+						flag->add_ware(game, *ware);
+						set_animation(game, descr().get_animation("idle"));
+						return schedule_act(game, 20);  //  rest a while
+					}
 				}
 
 				// Don't try to enter building if it is a dismantle site
@@ -2056,18 +2056,16 @@
 	if (ware) {
 		// We're in the building, walk onto the flag
 		if (upcast(Building, building, location)) {
-			Flag& baseflag = building->base_flag();
-			if (baseflag.has_capacity_for_ware(*ware)) {
-				start_task_leavebuilding(game, false);  //  exit throttle
-			} else {
-				start_task_waitforcapacity(game, baseflag);
+			if (start_task_waitforcapacity(game, building->base_flag())) {
+				return;
 			}
-			return;
+
+			return start_task_leavebuilding(game, false);  //  exit throttle
 		}
 
 		// We're on the flag, drop the ware and pause a little
 		if (upcast(Flag, flag, location)) {
-			if (flag->has_capacity_for_ware(*ware)) {
+			if (flag->has_capacity()) {
 				flag->add_ware(game, *fetch_carried_ware(game));
 
 				set_animation(game, descr().get_animation("idle"));
@@ -2147,11 +2145,9 @@
 
 		// The ware has decided that it doesn't want to go to us after all
 		// In order to return to the warehouse, we're switching to State_DropOff
-		Flag& flag = dynamic_cast<Flag&>(*location);
 		if (WareInstance* const ware =
-		       flag.fetch_pending_ware(game, flag.find_pending_ware(employer))) {
+		       dynamic_cast<Flag&>(*location).fetch_pending_ware(game, employer)) {
 			set_carried_ware(game, ware);
-			flag.ware_departing(game);
 		}
 
 		set_animation(game, descr().get_animation("idle"));
@@ -2214,15 +2210,24 @@
    static_cast<Bob::Ptr>(&Worker::waitforcapacity_pop), true};
 
 /**
- * Pushes a wait task and
- * adds the worker to the flag's wait queue.
+ * Checks the capacity of the flag.
+ *
+ * If there is none, a wait task is pushed, and the worker is added to the
+ * flag's wait queue. The function returns true in this case.
+ * If the flag still has capacity, the function returns false and doesn't
+ * act at all.
  */
-void Worker::start_task_waitforcapacity(Game& game, Flag& flag) {
+bool Worker::start_task_waitforcapacity(Game& game, Flag& flag) {
+	if (flag.has_capacity())
+		return false;
+
 	push_task(game, taskWaitforcapacity);
 
 	top_state().objvar1 = &flag;
 
 	flag.wait_for_capacity(game, *this);
+
+	return true;
 }
 
 void Worker::waitforcapacity_update(Game& game, State&) {

=== modified file 'src/logic/map_objects/tribes/worker.h'
--- src/logic/map_objects/tribes/worker.h	2018-09-14 08:46:36 +0000
+++ src/logic/map_objects/tribes/worker.h	2018-11-23 16:57:35 +0000
@@ -163,7 +163,7 @@
 	void start_task_releaserecruit(Game&, Worker&);
 	void start_task_fetchfromflag(Game&);
 
-	void start_task_waitforcapacity(Game&, Flag&);
+	bool start_task_waitforcapacity(Game&, Flag&);
 	void start_task_leavebuilding(Game&, bool changelocation);
 	void start_task_fugitive(Game&);