Fix #14921: Crash during station autorefit if station doesn't accept current cargo type. (#14924)

Add convenience helpers to correctly retrieve goods entry cargo available/totals.

Avoids having to check if cargo data is available before accessing it, which was missing for autorefit.
This commit is contained in:
Peter Nelson
2025-12-15 18:06:43 +00:00
committed by dP
parent 914c6a3421
commit 8aade54f66
7 changed files with 37 additions and 18 deletions

View File

@@ -1523,7 +1523,7 @@ static void HandleStationRefit(Vehicle *v, CargoArray &consist_capleft, Station
* of 0 for all cargoes. */ * of 0 for all cargoes. */
if (refit_capacity > 0 && (consist_capleft[cargo_type] < consist_capleft[new_cargo_type] || if (refit_capacity > 0 && (consist_capleft[cargo_type] < consist_capleft[new_cargo_type] ||
(consist_capleft[cargo_type] == consist_capleft[new_cargo_type] && (consist_capleft[cargo_type] == consist_capleft[new_cargo_type] &&
st->goods[cargo_type].GetData().cargo.AvailableCount() > st->goods[new_cargo_type].GetData().cargo.AvailableCount()))) { st->goods[cargo_type].AvailableCount() > st->goods[new_cargo_type].AvailableCount()))) {
new_cargo_type = cargo_type; new_cargo_type = cargo_type;
} }
} }
@@ -1796,7 +1796,7 @@ static void LoadUnloadVehicle(Vehicle *front)
/* If there's goods waiting at the station, and the vehicle /* If there's goods waiting at the station, and the vehicle
* has capacity for it, load it on the vehicle. */ * has capacity for it, load it on the vehicle. */
if ((v->cargo.ActionCount(VehicleCargoList::MTA_LOAD) > 0 || (ge->HasData() && ge->GetData().cargo.AvailableCount() > 0)) && MayLoadUnderExclusiveRights(st, v)) { if ((v->cargo.ActionCount(VehicleCargoList::MTA_LOAD) > 0 || ge->AvailableCount() > 0) && MayLoadUnderExclusiveRights(st, v)) {
if (v->cargo.StoredCount() == 0) TriggerVehicleRandomisation(v, VehicleRandomTrigger::NewCargo); if (v->cargo.StoredCount() == 0) TriggerVehicleRandomisation(v, VehicleRandomTrigger::NewCargo);
if (_settings_game.order.gradual_loading) cap_left = std::min(cap_left, GetLoadAmount(v)); if (_settings_game.order.gradual_loading) cap_left = std::min(cap_left, GetLoadAmount(v));

View File

@@ -231,7 +231,7 @@ RoadStopResolverObject::RoadStopResolverObject(const RoadStopSpec *roadstopspec,
const Station *station = Station::From(st); const Station *station = Station::From(st);
/* Pick the first cargo that we have waiting */ /* Pick the first cargo that we have waiting */
for (const auto &[cargo, spritegroup] : roadstopspec->grf_prop.spritegroups) { for (const auto &[cargo, spritegroup] : roadstopspec->grf_prop.spritegroups) {
if (cargo < NUM_CARGO && station->goods[cargo].HasData() && station->goods[cargo].GetData().cargo.TotalCount() > 0) { if (cargo < NUM_CARGO && station->goods[cargo].TotalCount() > 0) {
ctype = cargo; ctype = cargo;
this->root_spritegroup = spritegroup; this->root_spritegroup = spritegroup;
break; break;

View File

@@ -439,7 +439,7 @@ uint32_t Station::GetNewGRFVariable(const ResolverObject &object, uint8_t variab
const GoodsEntry *ge = &this->goods[cargo]; const GoodsEntry *ge = &this->goods[cargo];
switch (variable) { switch (variable) {
case 0x60: return ge->HasData() ? std::min(ge->GetData().cargo.TotalCount(), 4095u) : 0; case 0x60: return std::min(ge->TotalCount(), 4095u);
case 0x61: return ge->HasVehicleEverTriedLoading() ? ge->time_since_pickup : 0; case 0x61: return ge->HasVehicleEverTriedLoading() ? ge->time_since_pickup : 0;
case 0x62: return ge->HasRating() ? ge->rating : 0xFFFFFFFF; case 0x62: return ge->HasRating() ? ge->rating : 0xFFFFFFFF;
case 0x63: return ge->HasData() ? ge->GetData().cargo.PeriodsInTransit() : 0; case 0x63: return ge->HasData() ? ge->GetData().cargo.PeriodsInTransit() : 0;
@@ -453,8 +453,8 @@ uint32_t Station::GetNewGRFVariable(const ResolverObject &object, uint8_t variab
if (variable >= 0x8C && variable <= 0xEC) { if (variable >= 0x8C && variable <= 0xEC) {
const GoodsEntry *g = &this->goods[GB(variable - 0x8C, 3, 4)]; const GoodsEntry *g = &this->goods[GB(variable - 0x8C, 3, 4)];
switch (GB(variable - 0x8C, 0, 3)) { switch (GB(variable - 0x8C, 0, 3)) {
case 0: return g->HasData() ? g->GetData().cargo.TotalCount() : 0; case 0: return g->TotalCount();
case 1: return GB(g->HasData() ? std::min(g->GetData().cargo.TotalCount(), 4095u) : 0, 0, 4) | (g->status.Test(GoodsEntry::State::Acceptance) ? (1U << 7) : 0); case 1: return GB(std::min(g->TotalCount(), 4095u), 0, 4) | (g->status.Test(GoodsEntry::State::Acceptance) ? (1U << 7) : 0);
case 2: return g->time_since_pickup; case 2: return g->time_since_pickup;
case 3: return g->rating; case 3: return g->rating;
case 4: return (g->HasData() ? g->GetData().cargo.GetFirstStation() : StationID::Invalid()).base(); case 4: return (g->HasData() ? g->GetData().cargo.GetFirstStation() : StationID::Invalid()).base();
@@ -521,14 +521,13 @@ uint32_t Waypoint::GetNewGRFVariable(const ResolverObject &, uint8_t variable, [
case CargoGRFFileProps::SG_DEFAULT: case CargoGRFFileProps::SG_DEFAULT:
for (const GoodsEntry &ge : st->goods) { for (const GoodsEntry &ge : st->goods) {
if (!ge.HasData()) continue; cargo += ge.TotalCount();
cargo += ge.GetData().cargo.TotalCount();
} }
break; break;
default: { default: {
const GoodsEntry &ge = st->goods[this->station_scope.cargo_type]; const GoodsEntry &ge = st->goods[this->station_scope.cargo_type];
cargo = ge.HasData() ? ge.GetData().cargo.TotalCount() : 0; cargo = ge.TotalCount();
break; break;
} }
} }
@@ -585,7 +584,7 @@ StationResolverObject::StationResolverObject(const StationSpec *statspec, BaseSt
const Station *st = Station::From(this->station_scope.st); const Station *st = Station::From(this->station_scope.st);
/* Pick the first cargo that we have waiting */ /* Pick the first cargo that we have waiting */
for (const auto &[cargo, spritegroup] : statspec->grf_prop.spritegroups) { for (const auto &[cargo, spritegroup] : statspec->grf_prop.spritegroups) {
if (cargo < NUM_CARGO && st->goods[cargo].HasData() && st->goods[cargo].GetData().cargo.TotalCount() > 0) { if (cargo < NUM_CARGO && st->goods[cargo].TotalCount() > 0) {
ctype = cargo; ctype = cargo;
break; break;
} }

View File

@@ -1810,7 +1810,7 @@ bool AfterLoadGame()
for (Station *st : Station::Iterate()) { for (Station *st : Station::Iterate()) {
for (GoodsEntry &ge : st->goods) { for (GoodsEntry &ge : st->goods) {
ge.last_speed = 0; ge.last_speed = 0;
if (ge.HasData() && ge.GetData().cargo.AvailableCount() != 0) ge.status.Set(GoodsEntry::State::Rating); if (ge.AvailableCount() != 0) ge.status.Set(GoodsEntry::State::Rating);
} }
} }
} }

View File

@@ -343,6 +343,26 @@ struct GoodsEntry {
uint8_t ConvertState() const; uint8_t ConvertState() const;
/**
* Returns sum of cargo still available for loading at the station.
* (i.e. not counting cargo which is already reserved for loading)
* @return Cargo on board the vehicle.
*/
inline uint AvailableCount() const
{
return this->HasData() ? this->GetData().cargo.AvailableCount() : 0;
}
/**
* Returns total count of cargo at the station, including
* cargo which is already reserved for loading.
* @return Total cargo count.
*/
inline uint TotalCount() const
{
return this->HasData() ? this->GetData().cargo.TotalCount() : 0;
}
private: private:
std::unique_ptr<GoodsEntryData> data = nullptr; ///< Optional cargo packet and flow data. std::unique_ptr<GoodsEntryData> data = nullptr; ///< Optional cargo packet and flow data.
}; };

View File

@@ -508,7 +508,7 @@ CargoTypes GetEmptyMask(const Station *st)
CargoTypes mask = 0; CargoTypes mask = 0;
for (auto it = std::begin(st->goods); it != std::end(st->goods); ++it) { for (auto it = std::begin(st->goods); it != std::end(st->goods); ++it) {
if (!it->HasData() || it->GetData().cargo.TotalCount() == 0) SetBit(mask, std::distance(std::begin(st->goods), it)); if (it->TotalCount() == 0) SetBit(mask, std::distance(std::begin(st->goods), it));
} }
return mask; return mask;
} }
@@ -3997,7 +3997,7 @@ static void UpdateStationRating(Station *st)
bool skip = false; bool skip = false;
int rating = 0; int rating = 0;
uint waiting = ge->HasData() ? ge->GetData().cargo.AvailableCount() : 0; uint waiting = ge->AvailableCount();
/* num_dests is at least 1 if there is any cargo as /* num_dests is at least 1 if there is any cargo as
* StationID::Invalid() is also a destination. * StationID::Invalid() is also a destination.
@@ -4105,12 +4105,12 @@ static void UpdateStationRating(Station *st)
/* We can't truncate cargo that's already reserved for loading. /* We can't truncate cargo that's already reserved for loading.
* Thus StoredCount() here. */ * Thus StoredCount() here. */
if (waiting_changed && waiting < (ge->HasData() ? ge->GetData().cargo.AvailableCount() : 0)) { if (waiting_changed && waiting < ge->AvailableCount()) {
/* Feed back the exact own waiting cargo at this station for the /* Feed back the exact own waiting cargo at this station for the
* next rating calculation. */ * next rating calculation. */
ge->max_waiting_cargo = 0; ge->max_waiting_cargo = 0;
TruncateCargo(cs, ge, ge->GetData().cargo.AvailableCount() - waiting); TruncateCargo(cs, ge, ge->AvailableCount() - waiting);
} else { } else {
/* If the average number per next hop is low, be more forgiving. */ /* If the average number per next hop is low, be more forgiving. */
ge->max_waiting_cargo = waiting_avg; ge->max_waiting_cargo = waiting_avg;

View File

@@ -389,7 +389,7 @@ protected:
int diff = 0; int diff = 0;
for (CargoType cargo : SetCargoBitIterator(cargo_filter)) { for (CargoType cargo : SetCargoBitIterator(cargo_filter)) {
diff += (a->goods[cargo].HasData() ? a->goods[cargo].GetData().cargo.TotalCount() : 0) - (b->goods[cargo].HasData() ? b->goods[cargo].GetData().cargo.TotalCount() : 0); diff += a->goods[cargo].TotalCount() - b->goods[cargo].TotalCount();
} }
return diff < 0; return diff < 0;
@@ -401,7 +401,7 @@ protected:
int diff = 0; int diff = 0;
for (CargoType cargo : SetCargoBitIterator(cargo_filter)) { for (CargoType cargo : SetCargoBitIterator(cargo_filter)) {
diff += (a->goods[cargo].HasData() ? a->goods[cargo].GetData().cargo.AvailableCount() : 0) - (b->goods[cargo].HasData() ? b->goods[cargo].GetData().cargo.AvailableCount() : 0); diff += a->goods[cargo].AvailableCount() - b->goods[cargo].AvailableCount();
} }
return diff < 0; return diff < 0;
@@ -561,7 +561,7 @@ public:
x -= rating_width + rating_spacing; x -= rating_width + rating_spacing;
if (x < tr.left) break; if (x < tr.left) break;
} }
StationsWndShowStationRating(x, x + rating_width, tr.top, cargo_type, st->goods[cargo_type].HasData() ? st->goods[cargo_type].GetData().cargo.TotalCount() : 0, st->goods[cargo_type].rating); StationsWndShowStationRating(x, x + rating_width, tr.top, cargo_type, st->goods[cargo_type].TotalCount(), st->goods[cargo_type].rating);
if (!rtl) { if (!rtl) {
x += rating_width + rating_spacing; x += rating_width + rating_spacing;
if (x > tr.right) break; if (x > tr.right) break;