From 5f59d0c5b4e85a3bb51936fa05b2c5493f571eca Mon Sep 17 00:00:00 2001 From: rubidium Date: Tue, 6 Oct 2009 17:23:15 +0000 Subject: (svn r17720) -Codechange: guard the CargoPacket variables that are cached in CargoLists so they cannot be written from outside the CargoList class (based on patch by fonsinchen) --- src/cargopacket.cpp | 13 ++++++-- src/cargopacket.h | 68 ++++++++++++++++++++++++++++++++++++----- src/economy.cpp | 15 ++++----- src/economy_base.h | 4 +-- src/saveload/cargopacket_sl.cpp | 43 +++++++++++++++----------- src/saveload/oldloader_sl.cpp | 14 +++++---- src/saveload/station_sl.cpp | 7 +---- src/saveload/vehicle_sl.cpp | 5 +-- src/station_gui.cpp | 4 +-- 9 files changed, 120 insertions(+), 53 deletions(-) diff --git a/src/cargopacket.cpp b/src/cargopacket.cpp index a961e62f4..6e08c6f6a 100644 --- a/src/cargopacket.cpp +++ b/src/cargopacket.cpp @@ -39,6 +39,15 @@ CargoPacket::CargoPacket(StationID source, uint16 count, SourceType source_type, this->source_id = source_id; } +CargoPacket::CargoPacket(uint16 count, byte days_in_transit, Money feeder_share, SourceType source_type, SourceID source_id) : + feeder_share(feeder_share), + count(count), + days_in_transit(days_in_transit), + source_id(source_id) +{ + this->source_type = source_type; +} + /** * Invalidates (sets source_id to INVALID_SOURCE) all cargo packets from given source * @param src_type type of source @@ -149,7 +158,7 @@ bool CargoList::MoveTo(CargoList *dest, uint count, CargoList::MoveToAction mta, break; case MTA_TRANSFER: - payment->PayTransfer(cp, cp->count); + cp->feeder_share += payment->PayTransfer(cp, cp->count); break; case MTA_UNLOAD: @@ -178,7 +187,7 @@ bool CargoList::MoveTo(CargoList *dest, uint count, CargoList::MoveToAction mta, cp_new->count = count; dest->packets.push_back(cp_new); - if (mta == MTA_TRANSFER) payment->PayTransfer(cp_new, count); + if (mta == MTA_TRANSFER) cp_new->feeder_share += payment->PayTransfer(cp_new, count); } else { payment->PayFinalDelivery(cp, count); } diff --git a/src/cargopacket.h b/src/cargopacket.h index 508263332..4d66b0943 100644 --- a/src/cargopacket.h +++ b/src/cargopacket.h @@ -26,34 +26,88 @@ struct CargoPacket; typedef Pool CargoPacketPool; extern CargoPacketPool _cargopacket_pool; +class CargoList; +extern const struct SaveLoad *GetCargoPacketDesc(); + /** * Container for cargo from the same location and time */ struct CargoPacket : CargoPacketPool::PoolItem<&_cargopacket_pool> { +private: + /* Variables used by the CargoList cache. Only let them be modified via + * the proper accessor functions and/or CargoList itself. */ Money feeder_share; ///< Value of feeder pickup to be paid for on delivery of cargo - TileIndex source_xy; ///< The origin of the cargo (first station in feeder chain) - TileIndex loaded_at_xy; ///< Location where this cargo has been loaded into the vehicle - StationID source; ///< The station where the cargo came from first - uint16 count; ///< The amount of cargo in this packet byte days_in_transit; ///< Amount of days this packet has been in transit + /** The CargoList caches, thus needs to know about it. */ + friend class CargoList; + /** We want this to be saved, right? */ + friend const struct SaveLoad *GetCargoPacketDesc(); +public: + + TileIndex source_xy; ///< The origin of the cargo (first station in feeder chain) + TileIndex loaded_at_xy; ///< Location where this cargo has been loaded into the vehicle + StationID source; ///< The station where the cargo came from first SourceTypeByte source_type; ///< Type of #source_id SourceID source_id; ///< Index of source, INVALID_SOURCE if unknown/invalid /** * Creates a new cargo packet - * @param source the source of the packet - * @param count the number of cargo entities to put in this packet + * @param source the source of the packet + * @param count the number of cargo entities to put in this packet * @param source_type the 'type' of source the packet comes from (for subsidies) - * @param source_id the actual source of the packet (for subsidies) + * @param source_id the actual source of the packet (for subsidies) * @pre count != 0 || source == INVALID_STATION */ CargoPacket(StationID source = INVALID_STATION, uint16 count = 0, SourceType source_type = ST_INDUSTRY, SourceID source_id = INVALID_SOURCE); + /** + * Creates a new cargo packet. Initializes the fields that cannot be changed later. + * Used when loading or splitting packets. + * @param count the number of cargo entities to put in this packet + * @param days_in_transit number of days the cargo has been in transit + * @param feeder_share feeder share the packet has already accumulated + * @param source_type the 'type' of source the packet comes from (for subsidies) + * @param source_id the actual source of the packet (for subsidies) + */ + CargoPacket(uint16 count, byte days_in_transit, Money feeder_share = 0, SourceType source_type = ST_INDUSTRY, SourceID source_id = INVALID_SOURCE); + /** Destroy the packet */ ~CargoPacket() { } + + /** + * Gets the number of 'items' in this packet. + * @return the item count + */ + FORCEINLINE uint16 Count() const + { + return this->count; + } + + /** + * Gets the amount of money already paid to earlier vehicles in + * the feeder chain. + * @return the feeder share + */ + FORCEINLINE Money FeederShare() const + { + return this->feeder_share; + } + + /** + * Gets the number of days this cargo has been in transit. + * This number isn't really in days, but in 2.5 days (185 ticks) and + * it is capped at 255. + * @return the length this cargo has been in transit + */ + FORCEINLINE byte DaysInTransit() const + { + return this->days_in_transit; + } + + /** * Checks whether the cargo packet is from (exactly) the same source * in time and location. diff --git a/src/economy.cpp b/src/economy.cpp index faa043b62..15951bafe 100644 --- a/src/economy.cpp +++ b/src/economy.cpp @@ -1028,36 +1028,37 @@ CargoPayment::~CargoPayment() * @param cp The cargo packet to pay for. * @param count The number of packets to pay for. */ -void CargoPayment::PayFinalDelivery(CargoPacket *cp, uint count) +void CargoPayment::PayFinalDelivery(const CargoPacket *cp, uint count) { if (this->owner == NULL) { this->owner = Company::Get(this->front->owner); } /* Handle end of route payment */ - Money profit = DeliverGoods(count, this->ct, this->current_station, cp->source_xy, cp->days_in_transit, this->owner, cp->source_type, cp->source_id); + Money profit = DeliverGoods(count, this->ct, this->current_station, cp->source_xy, cp->DaysInTransit(), this->owner, cp->source_type, cp->source_id); this->route_profit += profit; /* The vehicle's profit is whatever route profit there is minus feeder shares. */ - this->visual_profit += profit - cp->feeder_share; + this->visual_profit += profit - cp->FeederShare(); } /** * Handle payment for transfer of the given cargo packet. - * @param cp The cargo packet to pay for. + * @param cp The cargo packet to pay for; actual payment won't be made!. * @param count The number of packets to pay for. + * @return The amount of money paid for the transfer. */ -void CargoPayment::PayTransfer(CargoPacket *cp, uint count) +Money CargoPayment::PayTransfer(const CargoPacket *cp, uint count) { Money profit = GetTransportedGoodsIncome( count, /* pay transfer vehicle for only the part of transfer it has done: ie. cargo_loaded_at_xy to here */ DistanceManhattan(cp->loaded_at_xy, Station::Get(this->current_station)->xy), - cp->days_in_transit, + cp->DaysInTransit(), this->ct); this->visual_profit += profit; // accumulate transfer profits for whole vehicle - cp->feeder_share += profit; // account for the (virtual) profit already made for the cargo packet + return profit; // account for the (virtual) profit already made for the cargo packet } /** diff --git a/src/economy_base.h b/src/economy_base.h index 9f929f449..7d447b23c 100644 --- a/src/economy_base.h +++ b/src/economy_base.h @@ -39,8 +39,8 @@ struct CargoPayment : CargoPaymentPool::PoolItem<&_cargo_payment_pool> { CargoPayment(Vehicle *front); ~CargoPayment(); - void PayTransfer(CargoPacket *cp, uint count); - void PayFinalDelivery(CargoPacket *cp, uint count); + Money PayTransfer(const CargoPacket *cp, uint count); + void PayFinalDelivery(const CargoPacket *cp, uint count); /** * Sets the currently handled cargo type. diff --git a/src/saveload/cargopacket_sl.cpp b/src/saveload/cargopacket_sl.cpp index 3f91e807d..86354f9c7 100644 --- a/src/saveload/cargopacket_sl.cpp +++ b/src/saveload/cargopacket_sl.cpp @@ -14,21 +14,30 @@ #include "saveload.h" -static const SaveLoad _cargopacket_desc[] = { - SLE_VAR(CargoPacket, source, SLE_UINT16), - SLE_VAR(CargoPacket, source_xy, SLE_UINT32), - SLE_VAR(CargoPacket, loaded_at_xy, SLE_UINT32), - SLE_VAR(CargoPacket, count, SLE_UINT16), - SLE_VAR(CargoPacket, days_in_transit, SLE_UINT8), - SLE_VAR(CargoPacket, feeder_share, SLE_INT64), - SLE_CONDVAR(CargoPacket, source_type, SLE_UINT8, 125, SL_MAX_VERSION), - SLE_CONDVAR(CargoPacket, source_id, SLE_UINT16, 125, SL_MAX_VERSION), - - /* Used to be paid_for, but that got changed. */ - SLE_CONDNULL(1, 0, 120), - - SLE_END() -}; +/** + * Wrapper function to get the CargoPacket's internal structure while + * some of the variables itself are private. + * @return the saveload description for CargoPackets. + */ +const SaveLoad *GetCargoPacketDesc() +{ + static const SaveLoad _cargopacket_desc[] = { + SLE_VAR(CargoPacket, source, SLE_UINT16), + SLE_VAR(CargoPacket, source_xy, SLE_UINT32), + SLE_VAR(CargoPacket, loaded_at_xy, SLE_UINT32), + SLE_VAR(CargoPacket, count, SLE_UINT16), + SLE_VAR(CargoPacket, days_in_transit, SLE_UINT8), + SLE_VAR(CargoPacket, feeder_share, SLE_INT64), + SLE_CONDVAR(CargoPacket, source_type, SLE_UINT8, 125, SL_MAX_VERSION), + SLE_CONDVAR(CargoPacket, source_id, SLE_UINT16, 125, SL_MAX_VERSION), + + /* Used to be paid_for, but that got changed. */ + SLE_CONDNULL(1, 0, 120), + + SLE_END() + }; + return _cargopacket_desc; +} static void Save_CAPA() { @@ -36,7 +45,7 @@ static void Save_CAPA() FOR_ALL_CARGOPACKETS(cp) { SlSetArrayIndex(cp->index); - SlObject(cp, _cargopacket_desc); + SlObject(cp, GetCargoPacketDesc()); } } @@ -46,7 +55,7 @@ static void Load_CAPA() while ((index = SlIterateArray()) != -1) { CargoPacket *cp = new (index) CargoPacket(); - SlObject(cp, _cargopacket_desc); + SlObject(cp, GetCargoPacketDesc()); } } diff --git a/src/saveload/oldloader_sl.cpp b/src/saveload/oldloader_sl.cpp index a0c8d06af..900cbd4ab 100644 --- a/src/saveload/oldloader_sl.cpp +++ b/src/saveload/oldloader_sl.cpp @@ -698,10 +698,8 @@ static bool LoadOldGood(LoadgameState *ls, int num) SB(ge->acceptance_pickup, GoodsEntry::ACCEPTANCE, 1, HasBit(_waiting_acceptance, 15)); SB(ge->acceptance_pickup, GoodsEntry::PICKUP, 1, _cargo_source != 0xFF); if (GB(_waiting_acceptance, 0, 12) != 0) { - CargoPacket *cp = new CargoPacket(); - cp->source = (_cargo_source == 0xFF) ? INVALID_STATION : _cargo_source; - cp->count = GB(_waiting_acceptance, 0, 12); - cp->days_in_transit = _cargo_days; + CargoPacket *cp = new CargoPacket(GB(_waiting_acceptance, 0, 12), _cargo_days); + cp->source = (_cargo_source == 0xFF) ? INVALID_STATION : _cargo_source; ge->cargo.Append(cp); } @@ -1332,8 +1330,12 @@ bool LoadOldVehicle(LoadgameState *ls, int num) v->next = (Vehicle *)(size_t)_old_next_ptr; if (_cargo_count != 0) { - CargoPacket *cp = new CargoPacket((_cargo_source == 0xFF) ? INVALID_STATION : _cargo_source, _cargo_count); - cp->days_in_transit = _cargo_days; + CargoPacket *cp = new CargoPacket(_cargo_count, _cargo_days); + cp->source = (_cargo_source == 0xFF) ? INVALID_STATION : _cargo_source; + cp->source_xy = (cp->source != INVALID_STATION) ? Station::Get(cp->source)->xy : 0; + cp->loaded_at_xy = cp->source_xy; + cp->source_type = ST_INDUSTRY; + cp->source_id = INVALID_SOURCE; v->cargo.Append(cp); } } diff --git a/src/saveload/station_sl.cpp b/src/saveload/station_sl.cpp index 5ddb9f69f..5e11c04c5 100644 --- a/src/saveload/station_sl.cpp +++ b/src/saveload/station_sl.cpp @@ -239,15 +239,10 @@ static void Load_STNS() SB(ge->acceptance_pickup, GoodsEntry::ACCEPTANCE, 1, HasBit(_waiting_acceptance, 15)); if (GB(_waiting_acceptance, 0, 12) != 0) { /* Don't construct the packet with station here, because that'll fail with old savegames */ - CargoPacket *cp = new CargoPacket(); + CargoPacket *cp = new CargoPacket(GB(_waiting_acceptance, 0, 12), _cargo_days, _cargo_feeder_share); /* In old versions, enroute_from used 0xFF as INVALID_STATION */ cp->source = (CheckSavegameVersion(7) && _cargo_source == 0xFF) ? INVALID_STATION : _cargo_source; - cp->count = GB(_waiting_acceptance, 0, 12); - cp->days_in_transit = _cargo_days; - cp->feeder_share = _cargo_feeder_share; cp->source_xy = _cargo_source_xy; - cp->days_in_transit = _cargo_days; - cp->feeder_share = _cargo_feeder_share; SB(ge->acceptance_pickup, GoodsEntry::PICKUP, 1, 1); ge->cargo.Append(cp); } diff --git a/src/saveload/vehicle_sl.cpp b/src/saveload/vehicle_sl.cpp index b6043f62b..fbca9332f 100644 --- a/src/saveload/vehicle_sl.cpp +++ b/src/saveload/vehicle_sl.cpp @@ -719,12 +719,9 @@ void Load_VEHS() if (_cargo_count != 0 && IsCompanyBuildableVehicleType(v)) { /* Don't construct the packet with station here, because that'll fail with old savegames */ - CargoPacket *cp = new CargoPacket(); + CargoPacket *cp = new CargoPacket(_cargo_count, _cargo_days, _cargo_feeder_share); cp->source = _cargo_source; cp->source_xy = _cargo_source_xy; - cp->count = _cargo_count; - cp->days_in_transit = _cargo_days; - cp->feeder_share = _cargo_feeder_share; cp->loaded_at_xy = _cargo_loaded_at_xy; v->cargo.Append(cp); } diff --git a/src/station_gui.cpp b/src/station_gui.cpp index dc5174380..01e1caa63 100644 --- a/src/station_gui.cpp +++ b/src/station_gui.cpp @@ -840,13 +840,13 @@ struct StationViewWindow : public Window { for (CargoDataList::iterator jt = cargolist.begin(); jt != cargolist.end(); jt++) { CargoData *cd = &(*jt); if (cd->cargo == i && cd->source == cp->source) { - cd->count += cp->count; + cd->count += cp->Count(); added = true; break; } } - if (!added) cargolist.push_back(CargoData(i, cp->source, cp->count)); + if (!added) cargolist.push_back(CargoData(i, cp->source, cp->Count())); } } } -- cgit v1.2.3-70-g09d2