(svn r17720) -Codechange: guard the CargoPacket variables that are cached in CargoLists so they cannot be written from outside the CargoList class (based on patch by fonsinchen)

src/cargopacket.cpp

@@ -39,6 +39,15 @@ CargoPacket::CargoPacket(StationID source, uint16 count, SourceType source_type,
 	this->source_id       = source_id;
 }
 
+CargoPacket::CargoPacket(uint16 count, byte days_in_transit, Money feeder_share, SourceType source_type, SourceID source_id) :
+		feeder_share(feeder_share),
+		count(count),
+		days_in_transit(days_in_transit),
+		source_id(source_id)
+{
+	this->source_type = source_type;
+}
+
 /**
  * Invalidates (sets source_id to INVALID_SOURCE) all cargo packets from given source
  * @param src_type type of source
@@ -149,7 +158,7 @@ bool CargoList::MoveTo(CargoList *dest, uint count, CargoList::MoveToAction mta,
 					break;
 
 				case MTA_TRANSFER:
-					payment->PayTransfer(cp, cp->count);
+					cp->feeder_share += payment->PayTransfer(cp, cp->count);
 					break;
 
 				case MTA_UNLOAD:
@@ -178,7 +187,7 @@ bool CargoList::MoveTo(CargoList *dest, uint count, CargoList::MoveToAction mta,
 				cp_new->count = count;
 				dest->packets.push_back(cp_new);
 
-				if (mta == MTA_TRANSFER) payment->PayTransfer(cp_new, count);
+				if (mta == MTA_TRANSFER) cp_new->feeder_share += payment->PayTransfer(cp_new, count);
 			} else {
 				payment->PayFinalDelivery(cp, count);
 			}

src/cargopacket.h

@@ -26,34 +26,88 @@ struct CargoPacket;
 typedef Pool<CargoPacket, CargoPacketID, 1024, 1048576> CargoPacketPool;
 extern CargoPacketPool _cargopacket_pool;
 
+class CargoList;
+extern const struct SaveLoad *GetCargoPacketDesc();
+
 /**
  * Container for cargo from the same location and time
  */
 struct CargoPacket : CargoPacketPool::PoolItem<&_cargopacket_pool> {
+private:
+	/* Variables used by the CargoList cache. Only let them be modified via
+	 * the proper accessor functions and/or CargoList itself. */
 	Money feeder_share;     ///< Value of feeder pickup to be paid for on delivery of cargo
-	TileIndex source_xy;    ///< The origin of the cargo (first station in feeder chain)
-	TileIndex loaded_at_xy; ///< Location where this cargo has been loaded into the vehicle
-	StationID source;       ///< The station where the cargo came from first
-
 	uint16 count;           ///< The amount of cargo in this packet
 	byte days_in_transit;   ///< Amount of days this packet has been in transit
 
+	/** The CargoList caches, thus needs to know about it. */
+	friend class CargoList;
+	/** We want this to be saved, right? */
+	friend const struct SaveLoad *GetCargoPacketDesc();
+public:
+
+	TileIndex source_xy;        ///< The origin of the cargo (first station in feeder chain)
+	TileIndex loaded_at_xy;     ///< Location where this cargo has been loaded into the vehicle
+	StationID source;           ///< The station where the cargo came from first
 	SourceTypeByte source_type; ///< Type of #source_id
 	SourceID source_id;         ///< Index of source, INVALID_SOURCE if unknown/invalid
 
 	/**
 	 * Creates a new cargo packet
-	 * @param source the source of the packet
+	 * @param source      the source of the packet
-	 * @param count  the number of cargo entities to put in this packet
+	 * @param count       the number of cargo entities to put in this packet
 	 * @param source_type the 'type' of source the packet comes from (for subsidies)
-	 * @param source_id the actual source of the packet (for subsidies)
+	 * @param source_id   the actual source of the packet (for subsidies)
 	 * @pre count != 0 || source == INVALID_STATION
 	 */
 	CargoPacket(StationID source = INVALID_STATION, uint16 count = 0, SourceType source_type = ST_INDUSTRY, SourceID source_id = INVALID_SOURCE);
 
+	/**
+	 * Creates a new cargo packet. Initializes the fields that cannot be changed later.
+	 * Used when loading or splitting packets.
+	 * @param count           the number of cargo entities to put in this packet
+	 * @param days_in_transit number of days the cargo has been in transit
+	 * @param feeder_share    feeder share the packet has already accumulated
+	 * @param source_type     the 'type' of source the packet comes from (for subsidies)
+	 * @param source_id       the actual source of the packet (for subsidies)
+	 */
+	CargoPacket(uint16 count, byte days_in_transit, Money feeder_share = 0, SourceType source_type = ST_INDUSTRY, SourceID source_id = INVALID_SOURCE);
+
 	/** Destroy the packet */
 	~CargoPacket() { }
 
+
+	/**
+	 * Gets the number of 'items' in this packet.
+	 * @return the item count
+	 */
+	FORCEINLINE uint16 Count() const
+	{
+		return this->count;
+	}
+
+	/**
+	 * Gets the amount of money already paid to earlier vehicles in
+	 * the feeder chain.
+	 * @return the feeder share
+	 */
+	FORCEINLINE Money FeederShare() const
+	{
+		return this->feeder_share;
+	}
+
+	/**
+	 * Gets the number of days this cargo has been in transit.
+	 * This number isn't really in days, but in 2.5 days (185 ticks) and
+	 * it is capped at 255.
+	 * @return the length this cargo has been in transit
+	 */
+	FORCEINLINE byte DaysInTransit() const
+	{
+		return this->days_in_transit;
+	}
+
+
 	/**
 	 * Checks whether the cargo packet is from (exactly) the same source
 	 * in time and location.

src/economy.cpp

@@ -1028,36 +1028,37 @@ CargoPayment::~CargoPayment()
  * @param cp The cargo packet to pay for.
  * @param count The number of packets to pay for.
  */
-void CargoPayment::PayFinalDelivery(CargoPacket *cp, uint count)
+void CargoPayment::PayFinalDelivery(const CargoPacket *cp, uint count)
 {
 	if (this->owner == NULL) {
 		this->owner = Company::Get(this->front->owner);
 	}
 
 	/* Handle end of route payment */
-	Money profit = DeliverGoods(count, this->ct, this->current_station, cp->source_xy, cp->days_in_transit, this->owner, cp->source_type, cp->source_id);
+	Money profit = DeliverGoods(count, this->ct, this->current_station, cp->source_xy, cp->DaysInTransit(), this->owner, cp->source_type, cp->source_id);
 	this->route_profit += profit;
 
 	/* The vehicle's profit is whatever route profit there is minus feeder shares. */
-	this->visual_profit += profit - cp->feeder_share;
+	this->visual_profit += profit - cp->FeederShare();
 }
 
 /**
  * Handle payment for transfer of the given cargo packet.
- * @param cp The cargo packet to pay for.
+ * @param cp The cargo packet to pay for; actual payment won't be made!.
  * @param count The number of packets to pay for.
+ * @return The amount of money paid for the transfer.
  */
-void CargoPayment::PayTransfer(CargoPacket *cp, uint count)
+Money CargoPayment::PayTransfer(const CargoPacket *cp, uint count)
 {
 	Money profit = GetTransportedGoodsIncome(
 		count,
 		/* pay transfer vehicle for only the part of transfer it has done: ie. cargo_loaded_at_xy to here */
 		DistanceManhattan(cp->loaded_at_xy, Station::Get(this->current_station)->xy),
-		cp->days_in_transit,
+		cp->DaysInTransit(),
 		this->ct);
 
 	this->visual_profit += profit; // accumulate transfer profits for whole vehicle
-	cp->feeder_share    += profit; // account for the (virtual) profit already made for the cargo packet
+	return profit; // account for the (virtual) profit already made for the cargo packet
 }
 
 /**

src/economy_base.h

@@ -39,8 +39,8 @@ struct CargoPayment : CargoPaymentPool::PoolItem<&_cargo_payment_pool> {
 	CargoPayment(Vehicle *front);
 	~CargoPayment();
 
-	void PayTransfer(CargoPacket *cp, uint count);
+	Money PayTransfer(const CargoPacket *cp, uint count);
-	void PayFinalDelivery(CargoPacket *cp, uint count);
+	void PayFinalDelivery(const CargoPacket *cp, uint count);
 
 	/**
 	 * Sets the currently handled cargo type.

src/saveload/cargopacket_sl.cpp

@@ -14,21 +14,30 @@
 
 #include "saveload.h"
 
-static const SaveLoad _cargopacket_desc[] = {
+/**
-	     SLE_VAR(CargoPacket, source,          SLE_UINT16),
+ * Wrapper function to get the CargoPacket's internal structure while
-	     SLE_VAR(CargoPacket, source_xy,       SLE_UINT32),
+ * some of the variables itself are private.
-	     SLE_VAR(CargoPacket, loaded_at_xy,    SLE_UINT32),
+ * @return the saveload description for CargoPackets.
-	     SLE_VAR(CargoPacket, count,           SLE_UINT16),
+ */
-	     SLE_VAR(CargoPacket, days_in_transit, SLE_UINT8),
+const SaveLoad *GetCargoPacketDesc()
-	     SLE_VAR(CargoPacket, feeder_share,    SLE_INT64),
+{
-	 SLE_CONDVAR(CargoPacket, source_type,     SLE_UINT8,  125, SL_MAX_VERSION),
+	static const SaveLoad _cargopacket_desc[] = {
-	 SLE_CONDVAR(CargoPacket, source_id,       SLE_UINT16, 125, SL_MAX_VERSION),
+		     SLE_VAR(CargoPacket, source,          SLE_UINT16),
+		     SLE_VAR(CargoPacket, source_xy,       SLE_UINT32),
+		     SLE_VAR(CargoPacket, loaded_at_xy,    SLE_UINT32),
+		     SLE_VAR(CargoPacket, count,           SLE_UINT16),
+		     SLE_VAR(CargoPacket, days_in_transit, SLE_UINT8),
+		     SLE_VAR(CargoPacket, feeder_share,    SLE_INT64),
+		 SLE_CONDVAR(CargoPacket, source_type,     SLE_UINT8,  125, SL_MAX_VERSION),
+		 SLE_CONDVAR(CargoPacket, source_id,       SLE_UINT16, 125, SL_MAX_VERSION),
 
-	/* Used to be paid_for, but that got changed. */
+		/* Used to be paid_for, but that got changed. */
-	SLE_CONDNULL(1, 0, 120),
+		SLE_CONDNULL(1, 0, 120),
 
-	SLE_END()
+		SLE_END()
-};
+	};
+	return _cargopacket_desc;
+}
 
 static void Save_CAPA()
 {
@@ -36,7 +45,7 @@ static void Save_CAPA()
 
 	FOR_ALL_CARGOPACKETS(cp) {
 		SlSetArrayIndex(cp->index);
-		SlObject(cp, _cargopacket_desc);
+		SlObject(cp, GetCargoPacketDesc());
 	}
 }
 
@@ -46,7 +55,7 @@ static void Load_CAPA()
 
 	while ((index = SlIterateArray()) != -1) {
 		CargoPacket *cp = new (index) CargoPacket();
-		SlObject(cp, _cargopacket_desc);
+		SlObject(cp, GetCargoPacketDesc());
 	}
 }
 

src/saveload/oldloader_sl.cpp

@@ -698,10 +698,8 @@ static bool LoadOldGood(LoadgameState *ls, int num)
 	SB(ge->acceptance_pickup, GoodsEntry::ACCEPTANCE, 1, HasBit(_waiting_acceptance, 15));
 	SB(ge->acceptance_pickup, GoodsEntry::PICKUP, 1, _cargo_source != 0xFF);
 	if (GB(_waiting_acceptance, 0, 12) != 0) {
-		CargoPacket *cp = new CargoPacket();
+		CargoPacket *cp = new CargoPacket(GB(_waiting_acceptance, 0, 12), _cargo_days);
-		cp->source          = (_cargo_source == 0xFF) ? INVALID_STATION : _cargo_source;
+		cp->source = (_cargo_source == 0xFF) ? INVALID_STATION : _cargo_source;
-		cp->count           = GB(_waiting_acceptance, 0, 12);
-		cp->days_in_transit = _cargo_days;
 		ge->cargo.Append(cp);
 	}
 
@@ -1332,8 +1330,12 @@ bool LoadOldVehicle(LoadgameState *ls, int num)
 		v->next = (Vehicle *)(size_t)_old_next_ptr;
 
 		if (_cargo_count != 0) {
-			CargoPacket *cp = new CargoPacket((_cargo_source == 0xFF) ? INVALID_STATION : _cargo_source, _cargo_count);
+			CargoPacket *cp = new CargoPacket(_cargo_count, _cargo_days);
-			cp->days_in_transit = _cargo_days;
+			cp->source       = (_cargo_source == 0xFF) ? INVALID_STATION : _cargo_source;
+			cp->source_xy    = (cp->source != INVALID_STATION) ? Station::Get(cp->source)->xy : 0;
+			cp->loaded_at_xy = cp->source_xy;
+			cp->source_type  = ST_INDUSTRY;
+			cp->source_id    = INVALID_SOURCE;
 			v->cargo.Append(cp);
 		}
 	}

src/saveload/station_sl.cpp

@@ -239,15 +239,10 @@ static void Load_STNS()
 				SB(ge->acceptance_pickup, GoodsEntry::ACCEPTANCE, 1, HasBit(_waiting_acceptance, 15));
 				if (GB(_waiting_acceptance, 0, 12) != 0) {
 					/* Don't construct the packet with station here, because that'll fail with old savegames */
-					CargoPacket *cp = new CargoPacket();
+					CargoPacket *cp = new CargoPacket(GB(_waiting_acceptance, 0, 12), _cargo_days, _cargo_feeder_share);
 					/* In old versions, enroute_from used 0xFF as INVALID_STATION */
 					cp->source          = (CheckSavegameVersion(7) && _cargo_source == 0xFF) ? INVALID_STATION : _cargo_source;
-					cp->count           = GB(_waiting_acceptance, 0, 12);
-					cp->days_in_transit = _cargo_days;
-					cp->feeder_share    = _cargo_feeder_share;
 					cp->source_xy       = _cargo_source_xy;
-					cp->days_in_transit = _cargo_days;
-					cp->feeder_share    = _cargo_feeder_share;
 					SB(ge->acceptance_pickup, GoodsEntry::PICKUP, 1, 1);
 					ge->cargo.Append(cp);
 				}

src/saveload/vehicle_sl.cpp

@@ -719,12 +719,9 @@ void Load_VEHS()
 
 		if (_cargo_count != 0 && IsCompanyBuildableVehicleType(v)) {
 			/* Don't construct the packet with station here, because that'll fail with old savegames */
-			CargoPacket *cp = new CargoPacket();
+			CargoPacket *cp = new CargoPacket(_cargo_count, _cargo_days, _cargo_feeder_share);
 			cp->source          = _cargo_source;
 			cp->source_xy       = _cargo_source_xy;
-			cp->count           = _cargo_count;
-			cp->days_in_transit = _cargo_days;
-			cp->feeder_share    = _cargo_feeder_share;
 			cp->loaded_at_xy    = _cargo_loaded_at_xy;
 			v->cargo.Append(cp);
 		}

src/station_gui.cpp

@@ -840,13 +840,13 @@ struct StationViewWindow : public Window {
 						for (CargoDataList::iterator jt = cargolist.begin(); jt != cargolist.end(); jt++) {
 							CargoData *cd = &(*jt);
 							if (cd->cargo == i && cd->source == cp->source) {
-								cd->count += cp->count;
+								cd->count += cp->Count();
 								added = true;
 								break;
 							}
 						}
 
-						if (!added) cargolist.push_back(CargoData(i, cp->source, cp->count));
+						if (!added) cargolist.push_back(CargoData(i, cp->source, cp->Count()));
 					}
 				}
 			}