From df400ef84a4fb71c4a8491393335ca57283098c9 Mon Sep 17 00:00:00 2001 From: Patric Stout Date: Tue, 19 Sep 2023 22:16:31 +0200 Subject: [PATCH] Fix: only count distance traveled in vehicles for cargo payment (#11283) 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 | 6 ++- src/cargoaction.h | 12 +++-- src/cargopacket.cpp | 11 +++-- src/cargopacket.h | 88 +++++++++++++++++++++++++++++---- src/economy.cpp | 4 +- src/saveload/cargopacket_sl.cpp | 38 ++++++++++++++ src/saveload/saveload.h | 1 + src/vehicle.cpp | 2 +- 8 files changed, 141 insertions(+), 21 deletions(-) diff --git a/src/cargoaction.cpp b/src/cargoaction.cpp index 4766b10882..c70eee9bfb 100644 --- a/src/cargoaction.cpp +++ b/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()); diff --git a/src/cargoaction.h b/src/cargoaction.h index e58a213c61..c0c58b12cd 100644 --- a/src/cargoaction.h +++ b/src/cargoaction.h @@ -70,9 +70,11 @@ public: /** Action of transferring cargo from a vehicle to a station. */ class CargoTransfer : public CargoMovement { +protected: + TileIndex current_tile; ///< Current tile cargo unloading is happening. public: - CargoTransfer(VehicleCargoList *source, StationCargoList *destination, uint max_move) : - CargoMovement(source, destination, max_move) {} + CargoTransfer(VehicleCargoList *source, StationCargoList *destination, uint max_move, TileIndex current_tile) : + CargoMovement(source, destination, max_move), current_tile(current_tile) {} bool operator()(CargoPacket *cp); }; @@ -96,10 +98,12 @@ public: /** Action of returning previously reserved cargo from the vehicle to the station. */ class CargoReturn : public CargoMovement { +protected: + TileIndex current_tile; ///< Current tile cargo unloading is happening. StationID next; public: - CargoReturn(VehicleCargoList *source, StationCargoList *destination, uint max_move, StationID next) : - CargoMovement(source, destination, max_move), next(next) {} + CargoReturn(VehicleCargoList *source, StationCargoList *destination, uint max_move, StationID next, TileIndex current_tile) : + CargoMovement(source, destination, max_move), current_tile(current_tile), next(next) {} bool operator()(CargoPacket *cp); }; diff --git a/src/cargopacket.cpp b/src/cargopacket.cpp index 2de793f64f..fc5031d7eb 100644 --- a/src/cargopacket.cpp +++ b/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::Reassignaction_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) { diff --git a/src/cargopacket.h b/src/cargopacket.h index 3ff52d7716..6498760822 100644 --- a/src/cargopacket.h +++ b/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 @@ public: } /** - * 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 @@ public: 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 @@ public: template 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); diff --git a/src/economy.cpp b/src/economy.cpp index 758363d3c0..bc2bd93878 100644 --- a/src/economy.cpp +++ b/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 ...*/ diff --git a/src/saveload/cargopacket_sl.cpp b/src/saveload/cargopacket_sl.cpp index dd7036c03f..b5ee7312ca 100644 --- a/src/saveload/cargopacket_sl.cpp +++ b/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; } diff --git a/src/saveload/saveload.h b/src/saveload/saveload.h index 71e86aecea..df02b235f4 100644 --- a/src/saveload/saveload.h +++ b/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 }; diff --git a/src/vehicle.cpp b/src/vehicle.cpp index 56c17b89ef..901965f379 100644 --- a/src/vehicle.cpp +++ b/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(); }