(svn r25009) -Codechange: refactor bits of cargo packet (lists) to reduce duplicated code (fonsinchen)

3 files changed, 90 insertions, 32 deletions

diff --git a/src/cargopacket.cpp b/src/cargopacket.cpp
index 0603d9125..42643ffa3 100644
--- a/src/cargopacket.cpp
+++ b/src/cargopacket.cpp
@@ -79,14 +79,14 @@ CargoPacket::CargoPacket(uint16 count, byte days_in_transit, StationID source, T
 
 /**
  * Split this packet in two and return the split off part.
- * @param new_size Size of the remaining part.
+ * @param new_size Size of the split part.
  * @return Split off part, or NULL if no packet could be allocated!
  */
 inline CargoPacket *CargoPacket::Split(uint new_size)
 {
 	if (!CargoPacket::CanAllocateItem()) return NULL;
 
-	Money fs = this->feeder_share * new_size / static_cast<uint>(this->count);
+	Money fs = this->FeederShare(new_size);
 	CargoPacket *cp_new = new CargoPacket(new_size, this->days_in_transit, this->source, this->source_xy, this->loaded_at_xy, fs, this->source_type, this->source_id);
 	this->feeder_share -= fs;
 	this->count -= new_size;
@@ -105,6 +105,17 @@ inline void CargoPacket::Merge(CargoPacket *cp)
 }
 
 /**
+ * Reduce the packet by the given amount and remove the feeder share.
+ * @param count Amount to be removed.
+ */
+inline void CargoPacket::Reduce(uint count)
+{
+	assert(count < this->count);
+	this->feeder_share -= this->FeederShare(count);
+	this->count -= count;
+}
+
+/**
  * Invalidates (sets source_id to INVALID_SOURCE) all cargo packets from given source.
  * @param src_type Type of source.
  * @param src Index of source.
@@ -157,15 +168,17 @@ void CargoList<Tinst>::OnCleanPool()
 }
 
 /**
- * Update the cached values to reflect the removal of this packet.
+ * Update the cached values to reflect the removal of this packet or part of it.
  * Decreases count and days_in_transit.
  * @param cp Packet to be removed from cache.
+ * @param count Amount of cargo from the given packet to be removed.
  */
 template <class Tinst>
-void CargoList<Tinst>::RemoveFromCache(const CargoPacket *cp)
+void CargoList<Tinst>::RemoveFromCache(const CargoPacket *cp, uint count)
 {
-	this->count                 -= cp->count;
-	this->cargo_days_in_transit -= cp->days_in_transit * cp->count;
+	assert(count <= cp->count);
+	this->count                 -= count;
+	this->cargo_days_in_transit -= cp->days_in_transit * count;
 }
 
 /**
@@ -195,11 +208,7 @@ void CargoList<Tinst>::Append(CargoPacket *cp)
 	static_cast<Tinst *>(this)->AddToCache(cp);
 
 	for (List::reverse_iterator it(this->packets.rbegin()); it != this->packets.rend(); it++) {
-		CargoPacket *icp = *it;
-		if (Tinst::AreMergable(icp, cp) && icp->count + cp->count <= CargoPacket::MAX_COUNT) {
-			icp->Merge(cp);
-			return;
-		}
+		if (CargoList<Tinst>::TryMerge(*it, cp)) return;
 	}
 
 	/* The packet could not be merged with another one */
@@ -222,7 +231,7 @@ uint CargoList<Tinst>::Truncate(uint max_move)
 		if (max_remaining == 0) {
 			/* Nothing should remain, just remove the packets. */
 			it = this->packets.erase(it);
-			static_cast<Tinst *>(this)->RemoveFromCache(cp);
+			static_cast<Tinst *>(this)->RemoveFromCache(cp, cp->count);
 			delete cp;
 			continue;
 		}
@@ -230,9 +239,8 @@ uint CargoList<Tinst>::Truncate(uint max_move)
 		uint local_count = cp->count;
 		if (local_count > max_remaining) {
 			uint diff = local_count - max_remaining;
-			this->count -= diff;
-			this->cargo_days_in_transit -= cp->days_in_transit * diff;
-			cp->count = max_remaining;
+			static_cast<Tinst *>(this)->RemoveFromCache(cp, diff);
+			cp->Reduce(diff);
 			max_remaining = 0;
 		} else {
 			max_remaining -= local_count;
@@ -283,7 +291,7 @@ bool CargoList<Tinst>::MoveTo(Tother_inst *dest, uint max_move, MoveToAction mta
 			/* Can move the complete packet */
 			max_move -= cp->count;
 			it = this->packets.erase(it);
-			static_cast<Tinst *>(this)->RemoveFromCache(cp);
+			static_cast<Tinst *>(this)->RemoveFromCache(cp, cp->count);
 			switch (mta) {
 				case MTA_FINAL_DELIVERY:
 					payment->PayFinalDelivery(cp, cp->count);
@@ -311,14 +319,8 @@ bool CargoList<Tinst>::MoveTo(Tother_inst *dest, uint max_move, MoveToAction mta
 			payment->PayFinalDelivery(cp, max_move);
 
 			/* Remove the delivered data from the cache */
-			uint left = cp->count - max_move;
-			cp->count = max_move;
-			static_cast<Tinst *>(this)->RemoveFromCache(cp);
-
-			/* Final delivery payment pays the feeder share, so we have to
-			 * reset that so it is not 'shown' twice for partial unloads. */
-			cp->feeder_share = 0;
-			cp->count = left;
+			static_cast<Tinst *>(this)->RemoveFromCache(cp, max_move);
+			cp->Reduce(max_move);
 		} else {
 			/* But... the rest needs package splitting. */
 			CargoPacket *cp_new = cp->Split(max_move);
@@ -326,7 +328,7 @@ bool CargoList<Tinst>::MoveTo(Tother_inst *dest, uint max_move, MoveToAction mta
 			/* We could not allocate a CargoPacket? Is the map that full? */
 			if (cp_new == NULL) return false;
 
-			static_cast<Tinst *>(this)->RemoveFromCache(cp_new); // this reflects the changes in cp.
+			static_cast<Tinst *>(this)->RemoveFromCache(cp_new, max_move); // this reflects the changes in cp.
 
 			if (mta == MTA_TRANSFER) {
 				/* Add the feeder share before inserting in dest. */
@@ -357,14 +359,40 @@ void CargoList<Tinst>::InvalidateCache()
 }
 
 /**
- * Update the cached values to reflect the removal of this packet.
+ * Tries to merge the second packet into the first and return if that was
+ * successful.
+ * @param icp Packet to be merged into.
+ * @param cp Packet to be eliminated.
+ * @return If the packets could be merged.
+ */
+template <class Tinst>
+/* static */ bool CargoList<Tinst>::TryMerge(CargoPacket *icp, CargoPacket *cp)
+{
+	if (Tinst::AreMergable(icp, cp) &&
+				icp->count + cp->count <= CargoPacket::MAX_COUNT) {
+		icp->Merge(cp);
+		return true;
+	} else {
+		return false;
+	}
+}
+
+/*
+ *
+ * Vehicle cargo list implementation.
+ *
+ */
+
+/**
+ * Update the cached values to reflect the removal of this packet or part of it.
  * Decreases count, feeder share and days_in_transit.
  * @param cp Packet to be removed from cache.
+ * @param count Amount of cargo from the given packet to be removed.
  */
-void VehicleCargoList::RemoveFromCache(const CargoPacket *cp)
+void VehicleCargoList::RemoveFromCache(const CargoPacket *cp, uint count)
 {
-	this->feeder_share -= cp->feeder_share;
-	this->Parent::RemoveFromCache(cp);
+	this->feeder_share -= cp->FeederShare(count);
+	this->Parent::RemoveFromCache(cp, count);
 }
 
 /**
diff --git a/src/cargopacket.h b/src/cargopacket.h
index 1e2093134..c23df8800 100644
--- a/src/cargopacket.h
+++ b/src/cargopacket.h
@@ -64,6 +64,19 @@ public:
 
 	CargoPacket *Split(uint new_size);
 	void Merge(CargoPacket *cp);
+	void Reduce(uint count);
+
+	/**
+	 * Sets the tile where the packet was loaded last.
+	 * @param load_place Tile where the packet was loaded last.
+	 */
+	void SetLoadPlace(TileIndex load_place) { this->loaded_at_xy = load_place; }
+
+	/**
+	 * Adds some feeder share to the packet.
+	 * @param new_share Feeder share to be added.
+	 */
+	void AddFeederShare(Money new_share) { this->feeder_share += new_share; }
 
 	/**
 	 * Gets the number of 'items' in this packet.
@@ -85,6 +98,17 @@ public:
 	}
 
 	/**
+	 * Gets part of the amount of money already paid to earlier vehicles in
+	 * the feeder chain.
+	 * @param part Amount of cargo to get the share for.
+	 * @return Feeder share for the given amount of cargo.
+	 */
+	inline Money FeederShare(uint part) const
+	{
+		return this->feeder_share * part / static_cast<uint>(this->count);
+	}
+
+	/**
 	 * Gets the number of days this cargo has been in transit.
 	 * This number isn't really in days, but in 2.5 days (CARGO_AGING_TICKS = 185 ticks) and
 	 * it is capped at 255.
@@ -170,8 +194,12 @@ public:
 	typedef std::list<CargoPacket *> List;
 	/** The iterator for our container. */
 	typedef List::iterator Iterator;
+	/** The reverse iterator for our container. */
+	typedef List::reverse_iterator ReverseIterator;
 	/** The const iterator for our container. */
 	typedef List::const_iterator ConstIterator;
+	/** The const reverse iterator for our container. */
+	typedef List::const_reverse_iterator ConstReverseIterator;
 
 	/** Kind of actions that could be done with packets on move. */
 	enum MoveToAction {
@@ -189,7 +217,9 @@ protected:
 
 	void AddToCache(const CargoPacket *cp);
 
-	void RemoveFromCache(const CargoPacket *cp);
+	void RemoveFromCache(const CargoPacket *cp, uint count);
+
+	static bool TryMerge(CargoPacket *cp, CargoPacket *icp);
 
 public:
 	/** Create the cargo list. */
@@ -265,7 +295,7 @@ protected:
 	Money feeder_share; ///< Cache for the feeder share.
 
 	void AddToCache(const CargoPacket *cp);
-	void RemoveFromCache(const CargoPacket *cp);
+	void RemoveFromCache(const CargoPacket *cp, uint count);
 
 public:
 	/** The super class ought to know what it's doing. */
diff --git a/src/economy.cpp b/src/economy.cpp
index 39cb8ddaa..7d055227e 100644
--- a/src/economy.cpp
+++ b/src/economy.cpp
@@ -1176,7 +1176,7 @@ void CargoPayment::PayFinalDelivery(const CargoPacket *cp, uint count)
 	this->route_profit += profit;
 
 	/* The vehicle's profit is whatever route profit there is minus feeder shares. */
-	this->visual_profit += profit - cp->FeederShare();
+	this->visual_profit += profit - cp->FeederShare(count);
 }
 
 /**