Fix #13053: Payment transfers incorrect for non-passenger cargos. (#13054)

CargoPayment required cargo type to be set as state via SetCargo(). This was error prone as CargoPayment is per consist but cargo type can vary per vehicle part. Additionally if SetCargo was not called then the default "uninitialised" state was cargo slot 0, passengers.

Instead of trying to make sure it is set correctly, remove cargo type from CargoPayment and always pass it explicitly to the PayTransfer/PayFinalDelivery methods.

src/cargoaction.cpp

@@ -107,7 +107,7 @@ bool CargoDelivery::operator()(CargoPacket *cp)
 {
 	uint remove = this->Preprocess(cp);
 	this->source->RemoveFromMeta(cp, VehicleCargoList::MTA_DELIVER, remove);
-	this->payment->PayFinalDelivery(cp, remove, this->current_tile);
+	this->payment->PayFinalDelivery(this->cargo, cp, remove, this->current_tile);
 	return this->Postprocess(cp, remove);
 }
 

src/cargoaction.h

@@ -40,9 +40,10 @@ class CargoDelivery : public CargoRemoval<VehicleCargoList> {
 protected:
 	TileIndex current_tile; ///< Current tile cargo delivery is happening.
 	CargoPayment *payment; ///< Payment object where payments will be registered.
+	CargoID cargo; ///< The cargo type of the cargo.
 public:
-	CargoDelivery(VehicleCargoList *source, uint max_move, CargoPayment *payment, TileIndex current_tile) :
+	CargoDelivery(VehicleCargoList *source, uint max_move, CargoID cargo, CargoPayment *payment, TileIndex current_tile) :
-			CargoRemoval<VehicleCargoList>(source, max_move), current_tile(current_tile), payment(payment) {}
+			CargoRemoval<VehicleCargoList>(source, max_move), current_tile(current_tile), payment(payment), cargo(cargo) {}
 	bool operator()(CargoPacket *cp);
 };
 

src/cargopacket.cpp

@@ -432,11 +432,12 @@ void VehicleCargoList::AgeCargo()
  * @param next_station ID of the station the vehicle will go to next.
  * @param order_flags OrderUnloadFlags that will apply to the unload operation.
  * @param ge GoodsEntry for getting the flows.
+ * @param cargo The cargo type of the cargo.
  * @param payment Payment object for registering transfers.
  * @param current_tile Current tile the cargo handling is happening on.
  * return If any cargo will be unloaded.
  */
-bool VehicleCargoList::Stage(bool accepted, StationID current_station, StationIDStack next_station, uint8_t order_flags, const GoodsEntry *ge, CargoPayment *payment, TileIndex current_tile)
+bool VehicleCargoList::Stage(bool accepted, StationID current_station, StationIDStack next_station, uint8_t order_flags, const GoodsEntry *ge, CargoID cargo, CargoPayment *payment, TileIndex current_tile)
 {
 	this->AssertCountConsistency();
 	assert(this->action_counts[MTA_LOAD] == 0);
@@ -512,7 +513,7 @@ bool VehicleCargoList::Stage(bool accepted, StationID current_station, StationID
 			case MTA_TRANSFER:
 				this->packets.push_front(cp);
 				/* Add feeder share here to allow reusing field for next station. */
-				share = payment->PayTransfer(cp, cp->count, current_tile);
+				share = payment->PayTransfer(cargo, cp, cp->count, current_tile);
 				cp->AddFeederShare(share);
 				this->feeder_share += share;
 				cp->next_hop = cargo_next;
@@ -619,11 +620,12 @@ uint VehicleCargoList::Shift(uint max_move, VehicleCargoList *dest)
  * ranges defined by designation_counts.
  * @param dest StationCargoList to add transferred cargo to.
  * @param max_move Maximum amount of cargo to move.
+ * @param cargo The cargo type of the cargo.
  * @param payment Payment object to register payments in.
  * @param current_tile Current tile the cargo handling is happening on.
  * @return Amount of cargo actually unloaded.
  */
-uint VehicleCargoList::Unload(uint max_move, StationCargoList *dest, CargoPayment *payment, TileIndex current_tile)
+uint VehicleCargoList::Unload(uint max_move, StationCargoList *dest, CargoID cargo, CargoPayment *payment, TileIndex current_tile)
 {
 	uint moved = 0;
 	if (this->action_counts[MTA_TRANSFER] > 0) {
@@ -633,7 +635,7 @@ uint VehicleCargoList::Unload(uint max_move, StationCargoList *dest, CargoPaymen
 	}
 	if (this->action_counts[MTA_TRANSFER] == 0 && this->action_counts[MTA_DELIVER] > 0 && moved < max_move) {
 		uint move = std::min(this->action_counts[MTA_DELIVER], max_move - moved);
-		this->ShiftCargo(CargoDelivery(this, move, payment, current_tile));
+		this->ShiftCargo(CargoDelivery(this, move, cargo, payment, current_tile));
 		moved += move;
 	}
 	return moved;

src/cargopacket.h

@@ -478,7 +478,7 @@ public:
 
 	void InvalidateCache();
 
-	bool Stage(bool accepted, StationID current_station, StationIDStack next_station, uint8_t order_flags, const GoodsEntry *ge, CargoPayment *payment, TileIndex current_tile);
+	bool Stage(bool accepted, StationID current_station, StationIDStack next_station, uint8_t order_flags, const GoodsEntry *ge, CargoID cargo, CargoPayment *payment, TileIndex current_tile);
 
 	/**
 	 * Marks all cargo in the vehicle as to be kept. This is mostly useful for
@@ -498,7 +498,7 @@ public:
 	template<MoveToAction Tfrom, MoveToAction Tto>
 	uint Reassign(uint max_move);
 	uint Return(uint max_move, StationCargoList *dest, StationID next_station, TileIndex current_tile);
-	uint Unload(uint max_move, StationCargoList *dest, CargoPayment *payment, TileIndex current_tile);
+	uint Unload(uint max_move, StationCargoList *dest, CargoID cargo, CargoPayment *payment, TileIndex current_tile);
 	uint Shift(uint max_move, VehicleCargoList *dest);
 	uint Truncate(uint max_move = UINT_MAX);
 	uint Reroute(uint max_move, VehicleCargoList *dest, StationID avoid, StationID avoid2, const GoodsEntry *ge);

src/economy.cpp

@@ -1231,18 +1231,19 @@ CargoPayment::~CargoPayment()
 
 /**
  * Handle payment for final delivery of the given cargo packet.
+ * @param cargo The cargo type of the cargo.
  * @param cp The cargo packet to pay for.
  * @param count The number of packets to pay for.
  * @param current_tile Current tile the payment is happening on.
  */
-void CargoPayment::PayFinalDelivery(const CargoPacket *cp, uint count, TileIndex current_tile)
+void CargoPayment::PayFinalDelivery(CargoID cargo, const CargoPacket *cp, uint count, TileIndex current_tile)
 {
 	if (this->owner == nullptr) {
 		this->owner = Company::Get(this->front->owner);
 	}
 
 	/* Handle end of route payment */
-	Money profit = DeliverGoods(count, this->ct, this->current_station, cp->GetDistance(current_tile), cp->GetPeriodsInTransit(), this->owner, cp->GetSourceType(), cp->GetSourceID());
+	Money profit = DeliverGoods(count, cargo, this->current_station, cp->GetDistance(current_tile), cp->GetPeriodsInTransit(), this->owner, cp->GetSourceType(), cp->GetSourceID());
 	this->route_profit += profit;
 
 	/* The vehicle's profit is whatever route profit there is minus feeder shares. */
@@ -1251,12 +1252,13 @@ void CargoPayment::PayFinalDelivery(const CargoPacket *cp, uint count, TileIndex
 
 /**
  * Handle payment for transfer of the given cargo packet.
+ * @param cargo The cargo type of the cargo.
  * @param cp The cargo packet to pay for; actual payment won't be made!.
  * @param count The number of packets to pay for.
  * @param current_tile Current tile the payment is happening on.
  * @return The amount of money paid for the transfer.
  */
-Money CargoPayment::PayTransfer(const CargoPacket *cp, uint count, TileIndex current_tile)
+Money CargoPayment::PayTransfer(CargoID cargo, const CargoPacket *cp, uint count, TileIndex current_tile)
 {
 	/* Pay transfer vehicle the difference between the payment for the journey from
 	 * the source to the current point, and the sum of the previous transfer payments */
@@ -1264,7 +1266,7 @@ Money CargoPayment::PayTransfer(const CargoPacket *cp, uint count, TileIndex cur
 			count,
 			cp->GetDistance(current_tile),
 			cp->GetPeriodsInTransit(),
-			this->ct);
+			cargo);
 
 	profit = profit * _settings_game.economy.feeder_payment_share / 100;
 
@@ -1304,7 +1306,7 @@ void PrepareUnload(Vehicle *front_v)
 						HasBit(ge->status, GoodsEntry::GES_ACCEPTANCE),
 						front_v->last_station_visited, next_station,
 						front_v->current_order.GetUnloadType(), ge,
-						front_v->cargo_payment,
+						v->cargo_type, front_v->cargo_payment,
 						v->GetCargoTile());
 				if (v->cargo.UnloadCount() > 0) SetBit(v->vehicle_flags, VF_CARGO_UNLOADING);
 			}
@@ -1692,9 +1694,6 @@ static void LoadUnloadVehicle(Vehicle *front)
 			uint amount_unloaded = _settings_game.order.gradual_loading ? std::min(cargo_count, GetLoadAmount(v)) : cargo_count;
 			bool remaining = false; // Are there cargo entities in this vehicle that can still be unloaded here?
 
-			assert(payment != nullptr);
-			payment->SetCargo(v->cargo_type);
-
 			if (!HasBit(ge->status, GoodsEntry::GES_ACCEPTANCE) && v->cargo.ActionCount(VehicleCargoList::MTA_DELIVER) > 0) {
 				/* The station does not accept our goods anymore. */
 				if (front->current_order.GetUnloadType() & (OUFB_TRANSFER | OUFB_UNLOAD)) {
@@ -1732,7 +1731,8 @@ static void LoadUnloadVehicle(Vehicle *front)
 				}
 			}
 
-			amount_unloaded = v->cargo.Unload(amount_unloaded, &ge->cargo, payment, v->GetCargoTile());
+			assert(payment != nullptr);
+			amount_unloaded = v->cargo.Unload(amount_unloaded, &ge->cargo, v->cargo_type, payment, v->GetCargoTile());
 			remaining = v->cargo.UnloadCount() > 0;
 			if (amount_unloaded > 0) {
 				dirty_vehicle = true;

src/economy_base.h

@@ -24,7 +24,6 @@ extern CargoPaymentPool _cargo_payment_pool;
 struct CargoPayment : CargoPaymentPool::PoolItem<&_cargo_payment_pool> {
 	/* CargoPaymentID index member of CargoPaymentPool is 4 bytes. */
 	StationID current_station; ///< NOSAVE: The current station
-	CargoID ct; ///< NOSAVE: The currently handled cargo type
 	Company *owner; ///< NOSAVE: The owner of the vehicle
 
 	Vehicle *front;        ///< The front vehicle to do the payment of
@@ -37,14 +36,8 @@ struct CargoPayment : CargoPaymentPool::PoolItem<&_cargo_payment_pool> {
 	CargoPayment(Vehicle *front);
 	~CargoPayment();
 
-	Money PayTransfer(const CargoPacket *cp, uint count, TileIndex current_tile);
+	Money PayTransfer(CargoID cargo, const CargoPacket *cp, uint count, TileIndex current_tile);
-	void PayFinalDelivery(const CargoPacket *cp, uint count, TileIndex current_tile);
+	void PayFinalDelivery(CargoID cargo, const CargoPacket *cp, uint count, TileIndex current_tile);
-
-	/**
-	 * Sets the currently handled cargo type.
-	 * @param ct the cargo type to handle from now on.
-	 */
-	void SetCargo(CargoID ct) { this->ct = ct; }
 };
 
 #endif /* ECONOMY_BASE_H */