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!

Based on patch by dP.

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->GetNextHop());

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,12 @@ 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),
+		travelled(original.travelled),
 		source_id(original.source_id),
 		source_type(original.source_type),
+#ifdef WITH_ASSERT
+		in_vehicle(original.in_vehicle),
+#endif /* WITH_ASSERT */
 		first_station(original.first_station),
 		next_hop(original.next_hop)
 {
@@ -587,12 +591,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;
 }
 
@@ -623,7 +628,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

@@ -39,15 +39,27 @@ extern SaveLoadTable GetCargoPacketDesc();
  */
 struct CargoPacket : CargoPacketPool::PoolItem<&_cargopacket_pool> {
 private:
+	/* A mathematical vector from (0,0). */
+	struct Vector {
+		int16_t x;
+		int16_t y;
+	};
+
 	uint16_t count{0}; ///< The amount of cargo in this packet.
 	uint16_t periods_in_transit{0}; ///< Amount of cargo aging periods this packet has been in transit.
 
 	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.
+	Vector travelled{0, 0}; ///< If cargo is in station: the vector from the unload tile to the source tile. If in vehicle: an intermediate value.
+
 	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.
 
+#ifdef WITH_ASSERT
+	bool in_vehicle{false}; ///< NOSAVE: Whether this cargo is in a vehicle or not.
+#endif /* WITH_ASSERT */
+
 	StationID first_station{INVALID_STATION}; ///< The station where the cargo came from first.
 	StationID next_hop{INVALID_STATION}; ///< Station where the cargo wants to go next.
 
@@ -83,22 +95,51 @@ 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;
 		}
+
+#ifdef WITH_ASSERT
+		assert(!this->in_vehicle);
+		this->in_vehicle = true;
+#endif /* WITH_ASSERT */
+
+		/* 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->travelled.x += TileX(tile);
+		this->travelled.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)
+	{
+#ifdef WITH_ASSERT
+		assert(this->in_vehicle);
+		this->in_vehicle = false;
+#endif /* WITH_ASSERT */
+
+		this->travelled.x -= TileX(tile);
+		this->travelled.y -= TileY(tile);
 	}
 
 	/**
@@ -188,7 +229,36 @@ 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);
+#ifdef WITH_ASSERT
+		assert(this->in_vehicle);
+#endif /* WITH_ASSERT */
+
+		/* Distance is always requested when the cargo is still inside the
+		 * vehicle. So first finish the calculation for travelled to
+		 * become a vector. */
+		auto local_travelled = travelled;
+		local_travelled.x -= TileX(current_tile);
+		local_travelled.y -= TileY(current_tile);
+
+		/* Cargo-movement is a vector 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 overpay 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, using 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_travelled = abs(local_travelled.x) + abs(local_travelled.y);
+		uint distance_source_dest = DistanceManhattan(this->source_xy, current_tile);
+		return std::min(distance_travelled, distance_source_dest);
 	}
 
 	/**
@@ -427,7 +497,7 @@ class VehicleCargoList : public CargoList<VehicleCargoList, CargoPacketList> {
 
 	template<MoveToAction Tfrom, MoveToAction Tto>
 	uint Reassign(uint max_move);
-	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

@@ -1438,7 +1438,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;
 	}
 };
@@ -1698,7 +1698,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/cargopacket_sl.cpp

@@ -76,6 +76,42 @@
 	if (IsSavegameVersionBefore(SLV_181)) {
 		for (Vehicle *v : Vehicle::Iterate()) v->cargo.KeepAll();
 	}
+
+	/* Before this version, we didn't track how far cargo actually traveled in vehicles. Make best-effort estimates of this. */
+	if (IsSavegameVersionBefore(SLV_CARGO_TRAVELLED)) {
+		/* Update the cargo-traveled in stations as if they arrived from the source tile. */
+		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) {
+						if (cp->source_xy != INVALID_TILE && cp->source_xy != st->xy) {
+							cp->travelled.x = TileX(cp->source_xy) - TileX(st->xy);
+							cp->travelled.y = TileY(cp->source_xy) - TileY(st->xy);
+						}
+					}
+				}
+			}
+		}
+
+		/* Update the cargo-traveled in vehicles as if they were loaded at the source tile. */
+		for (Vehicle *v : Vehicle::Iterate()) {
+			for (auto it = v->cargo.Packets()->begin(); it != v->cargo.Packets()->end(); it++) {
+				if ((*it)->source_xy != INVALID_TILE) {
+					(*it)->UpdateLoadingTile((*it)->source_xy);
+				}
+			}
+		}
+	}
+
+#ifdef WITH_ASSERT
+	/* in_vehicle is a NOSAVE; it tells if cargo is in a vehicle or not. Restore the value in here. */
+	for (Vehicle *v : Vehicle::Iterate()) {
+		for (auto it = v->cargo.Packets()->begin(); it != v->cargo.Packets()->end(); it++) {
+			(*it)->in_vehicle = true;
+		}
+	}
+#endif /* WITH_ASSERT */
 }
 
 /**
@@ -97,6 +133,8 @@ SaveLoadTable GetCargoPacketDesc()
 		SLE_VAR(CargoPacket, feeder_share,    SLE_INT64),
 		SLE_CONDVAR(CargoPacket, source_type,     SLE_UINT8,  SLV_125, SL_MAX_VERSION),
 		SLE_CONDVAR(CargoPacket, source_id,       SLE_UINT16, SLV_125, SL_MAX_VERSION),
+		SLE_CONDVAR(CargoPacket, travelled.x, SLE_INT16, SLV_CARGO_TRAVELLED, SL_MAX_VERSION),
+		SLE_CONDVAR(CargoPacket, travelled.y, SLE_INT16, SLV_CARGO_TRAVELLED, SL_MAX_VERSION),
 	};
 	return _cargopacket_desc;
 }

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_TRAVELLED,                    ///< 319  PR#11283 CargoPacket now tracks how far it travelled 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();
 	}