Fix: only count distance traveled in vehicles for cargo payment

No longer you can utilize the free (and instant) labour of station
workers, transporting your cargo from one part of the station to
the other. No more!

src/cargoaction.cpp

@@ -120,7 +120,7 @@ bool CargoLoad::operator()(CargoPacket *cp)
 {
 	CargoPacket *cp_new = this->Preprocess(cp);
 	if (cp_new == nullptr) return false;
-	cp_new->SetSourceXY(this->current_tile);
+	cp_new->UpdateLoadingTile(this->current_tile);
 	this->source->RemoveFromCache(cp_new, cp_new->Count());
 	this->destination->Append(cp_new, VehicleCargoList::MTA_KEEP);
 	return cp_new == cp;
@@ -135,7 +135,7 @@ bool CargoReservation::operator()(CargoPacket *cp)
 {
 	CargoPacket *cp_new = this->Preprocess(cp);
 	if (cp_new == nullptr) return false;
-	cp_new->SetSourceXY(this->current_tile);
+	cp_new->UpdateLoadingTile(this->current_tile);
 	this->source->reserved_count += cp_new->Count();
 	this->source->RemoveFromCache(cp_new, cp_new->Count());
 	this->destination->Append(cp_new, VehicleCargoList::MTA_LOAD);
@@ -152,6 +152,7 @@ bool CargoReturn::operator()(CargoPacket *cp)
 	CargoPacket *cp_new = this->Preprocess(cp);
 	if (cp_new == nullptr) cp_new = cp;
 	assert(cp_new->Count() <= this->destination->reserved_count);
+	cp_new->UpdateUnloadingTile(this->current_tile);
 	this->source->RemoveFromMeta(cp_new, VehicleCargoList::MTA_LOAD, cp_new->Count());
 	this->destination->reserved_count -= cp_new->Count();
 	this->destination->Append(cp_new, this->next);
@@ -167,6 +168,7 @@ bool CargoTransfer::operator()(CargoPacket *cp)
 {
 	CargoPacket *cp_new = this->Preprocess(cp);
 	if (cp_new == nullptr) return false;
+	cp_new->UpdateUnloadingTile(this->current_tile);
 	this->source->RemoveFromMeta(cp_new, VehicleCargoList::MTA_TRANSFER, cp_new->Count());
 	/* No transfer credits here as they were already granted during Stage(). */
 	this->destination->Append(cp_new, cp_new->GetNextStation());

src/cargoaction.h

@@ -70,9 +70,11 @@ class CargoMovement {
 
 /** Action of transferring cargo from a vehicle to a station. */
 class CargoTransfer : public CargoMovement<VehicleCargoList, StationCargoList> {
+protected:
+	TileIndex current_tile; ///< Current tile cargo unloading is happening.
 public:
-	CargoTransfer(VehicleCargoList *source, StationCargoList *destination, uint max_move) :
-			CargoMovement<VehicleCargoList, StationCargoList>(source, destination, max_move) {}
+	CargoTransfer(VehicleCargoList *source, StationCargoList *destination, uint max_move, TileIndex current_tile) :
+			CargoMovement<VehicleCargoList, StationCargoList>(source, destination, max_move), current_tile(current_tile) {}
 	bool operator()(CargoPacket *cp);
 };
 
@@ -96,10 +98,12 @@ class CargoReservation : public CargoLoad {
 
 /** Action of returning previously reserved cargo from the vehicle to the station. */
 class CargoReturn : public CargoMovement<VehicleCargoList, StationCargoList> {
+protected:
+	TileIndex current_tile; ///< Current tile cargo unloading is happening.
 	StationID next;
 public:
-	CargoReturn(VehicleCargoList *source, StationCargoList *destination, uint max_move, StationID next) :
-			CargoMovement<VehicleCargoList, StationCargoList>(source, destination, max_move), next(next) {}
+	CargoReturn(VehicleCargoList *source, StationCargoList *destination, uint max_move, StationID next, TileIndex current_tile) :
+			CargoMovement<VehicleCargoList, StationCargoList>(source, destination, max_move), current_tile(current_tile), next(next) {}
 	bool operator()(CargoPacket *cp);
 };
 

src/cargopacket.cpp

@@ -79,8 +79,10 @@ CargoPacket::CargoPacket(uint16_t count, Money feeder_share, CargoPacket &origin
 		periods_in_transit(original.periods_in_transit),
 		feeder_share(feeder_share),
 		source_xy(original.source_xy),
+		cargo_movement(original.cargo_movement),
 		source_id(original.source_id),
 		source_type(original.source_type),
+		in_vehicle(original.in_vehicle),
 		first_station(original.first_station),
 		next_station(original.next_station)
 {
@@ -588,12 +590,13 @@ uint VehicleCargoList::Reassign<VehicleCargoList::MTA_DELIVER, VehicleCargoList:
  * @param max_move Maximum amount of cargo to move.
  * @param dest Station the cargo is returned to.
  * @param next ID of the next station the cargo wants to go to.
+ * @param current_tile Current tile the cargo handling is happening on.
  * @return Amount of cargo actually returned.
  */
-uint VehicleCargoList::Return(uint max_move, StationCargoList *dest, StationID next)
+uint VehicleCargoList::Return(uint max_move, StationCargoList *dest, StationID next, TileIndex current_tile)
 {
 	max_move = std::min(this->action_counts[MTA_LOAD], max_move);
-	this->PopCargo(CargoReturn(this, dest, max_move, next));
+	this->PopCargo(CargoReturn(this, dest, max_move, next, current_tile));
 	return max_move;
 }
 
@@ -624,7 +627,7 @@ uint VehicleCargoList::Unload(uint max_move, StationCargoList *dest, CargoPaymen
 	uint moved = 0;
 	if (this->action_counts[MTA_TRANSFER] > 0) {
 		uint move = std::min(this->action_counts[MTA_TRANSFER], max_move);
-		this->ShiftCargo(CargoTransfer(this, dest, move));
+		this->ShiftCargo(CargoTransfer(this, dest, move, current_tile));
 		moved += move;
 	}
 	if (this->action_counts[MTA_TRANSFER] == 0 && this->action_counts[MTA_DELIVER] > 0 && moved < max_move) {

src/cargopacket.h

@@ -45,9 +45,13 @@ struct CargoPacket : CargoPacketPool::PoolItem<&_cargopacket_pool> {
 	Money feeder_share{0}; ///< Value of feeder pickup to be paid for on delivery of cargo.
 
 	TileIndex source_xy{INVALID_TILE}; ///< The origin of the cargo.
+	TileIndexDiffC cargo_movement{0, 0}; ///< A vector (from 0,0) of how far the cargo got actually transported in a vehicle. Is a vector when in station. Is a temporary variable when in vehicle.
+
 	SourceID source_id{INVALID_SOURCE}; ///< Index of industry/town/HQ, INVALID_SOURCE if unknown/invalid.
 	SourceType source_type{SourceType::Industry}; ///< Type of \c source_id.
 
+	bool in_vehicle{false}; ///< Whether this cargo is in a vehicle or not.
+
 	StationID first_station{INVALID_STATION}; ///< The station where the cargo came from first.
 	StationID next_station{INVALID_STATION}; ///< Station where the cargo wants to go next.
 
@@ -83,22 +87,47 @@ struct CargoPacket : CargoPacketPool::PoolItem<&_cargopacket_pool> {
 	}
 
 	/**
-	 * Set the origin of the packet.
+	 * Update for the cargo being loaded on this tile.
 	 *
-	 * Can only be set once.
+	 * When a CargoPacket is created, it is moved to a station. But at that
+	 * moment in time it is not known yet at which tile the cargo will be
+	 * picked up. As this tile is used for payment information, we delay
+	 * setting the source_xy till first pickup, getting a better idea where
+	 * a cargo started from.
 	 *
-	 * When a packet is created, it is moved to a station. But at that moment
-	 * in time it is not known yet at which tile the cargo will be picked up.
-	 * As this tile is used for payment information, we delay setting the
-	 * source_xy till first pickup.
+	 * Further more, we keep track of the amount of tiles the cargo moved
+	 * inside a vehicle. This is used in GetDistance() below.
 	 *
 	 * @param tile Tile the cargo is being picked up from.
 	 */
-	void SetSourceXY(TileIndex tile)
+	void UpdateLoadingTile(TileIndex tile)
 	{
 		if (this->source_xy == INVALID_TILE) {
 			this->source_xy = tile;
 		}
+
+		assert(!this->in_vehicle);
+		this->in_vehicle = true;
+
+		/* We want to calculate the vector from tile-unload to tile-load. As
+		 * we currently only know the latter, add it. When we know where we unload,
+		 * we subtract is, giving us our vector (unload - load). */
+		this->cargo_movement.x += TileX(tile);
+		this->cargo_movement.y += TileY(tile);
+	}
+
+	/**
+	 * Update for the cargo being unloaded on this tile.
+	 *
+	 * @param tile Tile the cargo is being dropped off at.
+	 */
+	void UpdateUnloadingTile(TileIndex tile)
+	{
+		assert(this->in_vehicle);
+		this->in_vehicle = false;
+
+		this->cargo_movement.x -= TileX(tile);
+		this->cargo_movement.y -= TileY(tile);
 	}
 
 	/**
@@ -188,7 +217,34 @@ struct CargoPacket : CargoPacketPool::PoolItem<&_cargopacket_pool> {
 	inline uint GetDistance(TileIndex current_tile) const
 	{
 		assert(this->source_xy != INVALID_TILE);
-		return DistanceManhattan(this->source_xy, current_tile);
+		assert(this->in_vehicle);
+
+		/* Distance is always requested when the cargo is still inside the
+		 * vehicle. So first finish the calculation for cargo_movement to
+		 * become a vector. */
+		auto local_cargo_movement = cargo_movement;
+		local_cargo_movement.x -= TileX(current_tile);
+		local_cargo_movement.y -= TileY(current_tile);
+
+		/* Cargo-movement is a vector (from 0,0) that indicates how much the
+		 * cargo has actually traveled in a vehicle. This is the distance you
+		 * get paid for. However, one could construct a route where this vector
+		 * would be really long. To not underpay the player, cap out at the
+		 * distance between source and destination.
+		 *
+		 * This way of calculating is to counter people moving cargo for free
+		 * and instantly in stations, where you deliver it in one part of the
+		 * station and pick it up in another. By using the actual distance
+		 * traveled in a vehicle, use this trick doesn't give you more money.
+		 *
+		 * However, especially in large networks with large transfer station,
+		 * etc, one could actually make the route a lot longer. In that case,
+		 * use the actual distance between source and destination.
+		 */
+
+		uint distance_cargo_movement = abs(local_cargo_movement.x) + abs(local_cargo_movement.y);
+		uint distance_source_dest = DistanceManhattan(this->source_xy, current_tile);
+		return std::min(distance_cargo_movement, distance_source_dest);
 	}
 
 	/**
@@ -202,6 +258,7 @@ struct CargoPacket : CargoPacketPool::PoolItem<&_cargopacket_pool> {
 
 	static void InvalidateAllFrom(SourceType src_type, SourceID src);
 	static void InvalidateAllFrom(StationID sid);
+	static void AfterLoadBeforeCargoMovement();
 	static void AfterLoad();
 };
 
@@ -427,7 +484,7 @@ class VehicleCargoList : public CargoList<VehicleCargoList, CargoPacketList> {
 
 	template<MoveToAction Tfrom, MoveToAction Tto>
 	uint Reassign(uint max_move, StationID update = INVALID_STATION);
-	uint Return(uint max_move, StationCargoList *dest, StationID next_station);
+	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 Shift(uint max_move, VehicleCargoList *dest);
 	uint Truncate(uint max_move = UINT_MAX);

src/economy.cpp

@@ -1437,7 +1437,7 @@ struct ReturnCargoAction
 	 */
 	bool operator()(Vehicle *v)
 	{
-		v->cargo.Return(UINT_MAX, &this->st->goods[v->cargo_type].cargo, this->next_hop);
+		v->cargo.Return(UINT_MAX, &this->st->goods[v->cargo_type].cargo, this->next_hop, v->tile);
 		return true;
 	}
 };
@@ -1697,7 +1697,7 @@ static void LoadUnloadVehicle(Vehicle *front)
 					uint new_remaining = v->cargo.RemainingCount() + v->cargo.ActionCount(VehicleCargoList::MTA_DELIVER);
 					if (v->cargo_cap < new_remaining) {
 						/* Return some of the reserved cargo to not overload the vehicle. */
-						v->cargo.Return(new_remaining - v->cargo_cap, &ge->cargo, INVALID_STATION);
+						v->cargo.Return(new_remaining - v->cargo_cap, &ge->cargo, INVALID_STATION, v->tile);
 					}
 
 					/* Keep instead of delivering. This may lead to no cargo being unloaded, so ...*/

src/saveload/afterload.cpp

@@ -3241,6 +3241,50 @@ bool AfterLoadGame()
 		}
 	}
 
+	if (IsSavegameVersionBefore(SLV_CARGO_MOVEMENT)) {
+		/* Find a reasonable source and travel distance for cargo in vehicles. */
+		for (Vehicle *v : Vehicle::Iterate()) {
+			for (auto it = v->cargo.Packets()->begin(); it != v->cargo.Packets()->end(); it++) {
+				auto first_station_id = (*it)->GetFirstStation();
+				auto last_station_id = v->First()->last_loading_station;
+
+				if (first_station_id == INVALID_STATION && last_station_id == INVALID_STATION) {
+					/* No station to calculate distance traveled. Just start counting from the current tile. */
+					(*it)->UpdateLoadingTile(v->tile);
+				} else if (first_station_id == INVALID_STATION) {
+					auto last_station = Station::Get(last_station_id);
+
+					/* We don't know where this cargo started, so let's just say it started from the last station. */
+					(*it)->UpdateLoadingTile(last_station->xy);
+				} else if (last_station_id == INVALID_STATION) {
+					auto first_station = Station::Get(first_station_id);
+
+					/* We don't know where the train was last, so just assume we were picking it up at the source. */
+					(*it)->UpdateLoadingTile(first_station->xy);
+				} else {
+					auto first_station = Station::Get(first_station_id);
+					auto last_station = Station::Get(last_station_id);
+
+					/* Act like we loaded it at the first station and unloaded at the last. */
+					(*it)->UpdateLoadingTile(first_station->xy);
+					(*it)->UpdateUnloadingTile(last_station->xy);
+					(*it)->UpdateLoadingTile(last_station->xy);
+				}
+			}
+		}
+
+		for (Station *st : Station::Iterate()) {
+			for (size_t i = 0; i < NUM_CARGO; i++) {
+				GoodsEntry *ge = &st->goods[i];
+				for (auto it = ge->cargo.Packets()->begin(); it != ge->cargo.Packets()->end(); it++) {
+					for (CargoPacket *cp : it->second) {
+						cp->AfterLoadBeforeCargoMovement();
+					}
+				}
+			}
+		}
+	}
+
 	AfterLoadLabelMaps();
 	AfterLoadCompanyStats();
 	AfterLoadStoryBook();

src/saveload/cargopacket_sl.cpp

@@ -15,8 +15,28 @@
 #include "../vehicle_base.h"
 #include "../station_base.h"
 
+#include "../debug.h"
+
 #include "../safeguards.h"
 
+/**
+ * Savegame conversion for version before cargo_movement was added.
+ */
+/* static */ void CargoPacket::AfterLoadBeforeCargoMovement()
+{
+	for (Station *st : Station::Iterate()) {
+		for (size_t i = 0; i < NUM_CARGO; i++) {
+			GoodsEntry *ge = &st->goods[i];
+			for (auto it = ge->cargo.Packets()->begin(); it != ge->cargo.Packets()->end(); it++) {
+				for (CargoPacket *cp : it->second) {
+					/* For cargo-movement, estimate it as the distance of the current station from the source. */
+					cp->cargo_movement = TileIndexToTileIndexDiffC(cp->source_xy, st->xy);
+				}
+			}
+		}
+	}
+}
+
 /**
  * Savegame conversion for cargopackets.
  */

src/saveload/saveload.h

@@ -360,6 +360,7 @@ enum SaveLoadVersion : uint16_t {
 	SLV_PERIODS_IN_TRANSIT_RENAME,          ///< 316  PR#11112 Rename days in transit to (cargo) periods in transit.
 	SLV_NEWGRF_LAST_SERVICE,                ///< 317  PR#11124 Added stable date_of_last_service to avoid NewGRF trouble.
 	SLV_REMOVE_LOADED_AT_XY,                ///< 318  PR#11276 Remove loaded_at_xy variable from CargoPacket.
+	SLV_CARGO_MOVEMENT,                     ///< 319  PR#????? CargoPacket now tracks how far it traveled inside a vehicle.
 
 	SL_MAX_VERSION,                         ///< Highest possible saveload version
 };

src/vehicle.cpp

@@ -2241,7 +2241,7 @@ void Vehicle::CancelReservation(StationID next, Station *st)
 		VehicleCargoList &cargo = v->cargo;
 		if (cargo.ActionCount(VehicleCargoList::MTA_LOAD) > 0) {
 			Debug(misc, 1, "cancelling cargo reservation");
-			cargo.Return(UINT_MAX, &st->goods[v->cargo_type].cargo, next);
+			cargo.Return(UINT_MAX, &st->goods[v->cargo_type].cargo, next, v->tile);
 		}
 		cargo.KeepAll();
 	}