From 4280c413a6a409ecc6fd37f75226da5db0e26591 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. (cherry picked from commit df400ef84a4fb71c4a8491393335ca57283098c9) --- src/cargoaction.cpp | 6 ++- src/cargoaction.h | 12 +++-- src/cargopacket.cpp | 11 +++-- src/cargopacket.h | 84 +++++++++++++++++++++++++++++---- src/economy.cpp | 4 +- src/saveload/cargopacket_sl.cpp | 2 + src/sl/cargopacket_sl.cpp | 41 ++++++++++++++++ src/sl/extended_ver_sl.cpp | 1 + src/sl/extended_ver_sl.h | 1 + src/sl/saveload_common.h | 1 + src/vehicle.cpp | 2 +- 11 files changed, 144 insertions(+), 21 deletions(-) diff --git a/src/cargoaction.cpp b/src/cargoaction.cpp index ca54692eb0..ab4d6f5324 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 c842def480..0c056e55b6 100644 --- a/src/cargoaction.h +++ b/src/cargoaction.h @@ -71,9 +71,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); }; @@ -97,10 +99,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 8ec0b276a8..a997c773c3 100644 --- a/src/cargopacket.cpp +++ b/src/cargopacket.cpp @@ -155,12 +155,16 @@ CargoPacket::CargoPacket(uint16_t count, Money feeder_share, const CargoPacket & 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), first_station(original.first_station), next_hop(original.next_hop) { dbg_assert(count != 0); +#ifdef WITH_FULL_ASSERTS + this->flags |= (original.flags & CPF_IN_VEHICLE); +#endif /* WITH_FULL_ASSERTS */ } /** Destroy the packet. */ @@ -783,12 +787,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; } @@ -819,7 +824,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 872cdd2517..b419240f96 100644 --- a/src/cargopacket.h +++ b/src/cargopacket.h @@ -51,10 +51,17 @@ void ChangeOwnershipOfCargoPacketDeferredPayments(Owner old_owner, Owner new_own */ struct CargoPacket : CargoPacketPool::PoolItem<&_cargopacket_pool> { private: + /* A mathematical vector from (0,0). */ + struct Vector { + int32_t x; + int32_t y; + }; + uint16 count = 0; ///< The amount of cargo in this packet. uint16 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. uint8 flags = 0; ///< NOSAVE: temporary flags @@ -64,6 +71,7 @@ private: /** Cargo packet flag bits in CargoPacket::flags. */ enum CargoPacketFlags { CPF_HAS_DEFERRED_PAYMENT = 0x01, ///< Cargo packet has 1 or more deferred payment(s) + CPF_IN_VEHICLE = 0x02, ///< Whether this cargo is in a vehicle or not. Only used when: defined(WITH_FULL_ASSERTS) }; /** The CargoList caches, thus needs to know about it. */ @@ -98,22 +106,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_FULL_ASSERTS + assert((this->flags & CPF_IN_VEHICLE) == 0); + this->flags |= CPF_IN_VEHICLE; +#endif /* WITH_FULL_ASSERTS */ + + /* 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_FULL_ASSERTS + assert((this->flags & CPF_IN_VEHICLE) != 0); + this->flags &= ~CPF_IN_VEHICLE; +#endif /* WITH_FULL_ASSERTS */ + + this->travelled.x -= TileX(tile); + this->travelled.y -= TileY(tile); } /** @@ -205,7 +242,36 @@ public: inline uint GetDistance(TileIndex current_tile) const { assert(this->source_xy != INVALID_TILE); - return DistanceManhattan(this->source_xy, current_tile); +#ifdef WITH_FULL_ASSERTS + assert((this->flags & CPF_IN_VEHICLE) != 0); +#endif /* WITH_FULL_ASSERTS */ + + /* 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); } /** @@ -475,7 +541,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 1db31190d2..2664e1a0c9 100644 --- a/src/economy.cpp +++ b/src/economy.cpp @@ -1717,7 +1717,7 @@ struct ReturnCargoAction */ bool operator()(Vehicle *v) { - v->cargo.Return(UINT_MAX, &this->st->goods[v->cargo_type].CreateData().cargo, this->next_hop); + v->cargo.Return(UINT_MAX, &this->st->goods[v->cargo_type].CreateData().cargo, this->next_hop, v->tile); return true; } }; @@ -2097,7 +2097,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, &ged->cargo, INVALID_STATION); + v->cargo.Return(new_remaining - v->cargo_cap, &ged->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 cfe4647bb5..31de878b1b 100644 --- a/src/saveload/cargopacket_sl.cpp +++ b/src/saveload/cargopacket_sl.cpp @@ -38,6 +38,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_FILE_I16 | SLE_VAR_I32, SLV_CARGO_TRAVELLED, SL_MAX_VERSION), + SLE_CONDVAR(CargoPacket, travelled.y, SLE_FILE_I16 | SLE_VAR_I32, SLV_CARGO_TRAVELLED, SL_MAX_VERSION), }; return _cargopacket_desc; } diff --git a/src/sl/cargopacket_sl.cpp b/src/sl/cargopacket_sl.cpp index ebf070504b..846578594a 100644 --- a/src/sl/cargopacket_sl.cpp +++ b/src/sl/cargopacket_sl.cpp @@ -81,6 +81,45 @@ extern btree::btree_map _cargo_packet_deferred_payments; 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) && SlXvIsFeatureMissing(XSLFI_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]; + if (ge.data == nullptr) continue; + auto *packets = ge.data->cargo.Packets(); + for (auto it = packets->begin(); it != 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()) { + auto *packets = v->cargo.Packets(); + for (auto it = packets->begin(); it != packets->end(); it++) { + if ((*it)->source_xy != INVALID_TILE) { + (*it)->UpdateLoadingTile((*it)->source_xy); + } + } + } + } + +#ifdef WITH_FULL_ASSERTS + /* CPF_IN_VEHICLE in flags 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)->flags |= CPF_IN_VEHICLE; + } + } +#endif /* WITH_FULL_ASSERTS */ } /** @@ -121,6 +160,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_X(CargoPacket, travelled.x, SLE_INT32, SL_MIN_VERSION, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_CARGO_TRAVELLED)), + SLE_CONDVAR_X(CargoPacket, travelled.y, SLE_INT32, SL_MIN_VERSION, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_CARGO_TRAVELLED)), /* Used to be paid_for, but that got changed. */ SLE_CONDNULL(1, SL_MIN_VERSION, SLV_121), diff --git a/src/sl/extended_ver_sl.cpp b/src/sl/extended_ver_sl.cpp index e251129bf7..8ea9f47e05 100644 --- a/src/sl/extended_ver_sl.cpp +++ b/src/sl/extended_ver_sl.cpp @@ -206,6 +206,7 @@ const SlxiSubChunkInfo _sl_xv_sub_chunk_infos[] = { { XSLFI_DISASTER_VEH_STATE, XSCF_NULL, 1, 1, "slv_disaster_veh_state", nullptr, nullptr, nullptr }, { XSLFI_SAVEGAME_ID, XSCF_NULL, 1, 1, "slv_savegame_id", nullptr, nullptr, nullptr }, { XSLFI_NEWGRF_LAST_SERVICE, XSCF_NULL, 1, 1, "slv_newgrf_last_service", nullptr, nullptr, nullptr }, + { XSLFI_CARGO_TRAVELLED, XSCF_NULL, 1, 1, "slv_cargo_travelled", nullptr, nullptr, nullptr }, { XSLFI_NULL, XSCF_NULL, 0, 0, nullptr, nullptr, nullptr, nullptr },// This is the end marker }; diff --git a/src/sl/extended_ver_sl.h b/src/sl/extended_ver_sl.h index b9dc9ac7be..9d52dbd0bf 100644 --- a/src/sl/extended_ver_sl.h +++ b/src/sl/extended_ver_sl.h @@ -156,6 +156,7 @@ enum SlXvFeatureIndex { XSLFI_DISASTER_VEH_STATE, ///< See: SLV_DISASTER_VEH_STATE (PR #10798) XSLFI_SAVEGAME_ID, ///< See: SLV_SAVEGAME_ID (PR #10719) XSLFI_NEWGRF_LAST_SERVICE, ///< See: SLV_NEWGRF_LAST_SERVICE (PR #11124) + XSLFI_CARGO_TRAVELLED, ///< See: SLV_CARGO_TRAVELLED (PR #11283) XSLFI_RIFF_HEADER_60_BIT, ///< Size field in RIFF chunk header is 60 bit XSLFI_HEIGHT_8_BIT, ///< Map tile height is 8 bit instead of 4 bit, but savegame version may be before this became true in trunk diff --git a/src/sl/saveload_common.h b/src/sl/saveload_common.h index b23b366579..3f13a27e47 100644 --- a/src/sl/saveload_common.h +++ b/src/sl/saveload_common.h @@ -372,6 +372,7 @@ enum SaveLoadVersion : uint16 { 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 8655e03c5c..d0f57ba18b 100644 --- a/src/vehicle.cpp +++ b/src/vehicle.cpp @@ -3385,7 +3385,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].CreateData().cargo, next); + cargo.Return(UINT_MAX, &st->goods[v->cargo_type].CreateData().cargo, next, v->tile); } cargo.KeepAll(); }