From 9602de474d5c0b6201923586e2783f422084d4ed Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Wed, 18 Oct 2023 20:49:01 +0100 Subject: [PATCH] Codechange: Use iterators and/or range-for on cargo related loops. --- src/aircraft_cmd.cpp | 6 ++--- src/console_cmds.cpp | 5 ++-- src/industry_gui.cpp | 3 +-- src/newgrf.cpp | 10 +++----- src/newgrf_station.cpp | 4 +-- src/openttd.cpp | 8 +++--- src/saveload/afterload.cpp | 6 ++--- src/saveload/cargopacket_sl.cpp | 13 ++++------ src/saveload/station_sl.cpp | 35 +++++++++++++------------- src/saveload/town_sl.cpp | 12 ++++----- src/script/api/script_industrytype.cpp | 8 +++--- src/station.cpp | 8 +++--- src/station_cmd.cpp | 33 +++++++++++------------- src/station_gui.cpp | 3 +-- src/town_cmd.cpp | 6 ++--- src/vehicle_cmd.h | 8 +++--- 16 files changed, 79 insertions(+), 89 deletions(-) diff --git a/src/aircraft_cmd.cpp b/src/aircraft_cmd.cpp index a6c3dad2cd..1f5f986282 100644 --- a/src/aircraft_cmd.cpp +++ b/src/aircraft_cmd.cpp @@ -1370,9 +1370,9 @@ static void MaybeCrashAirplane(Aircraft *v) if (GB(Random(), 0, 22) > prob) return; /* Crash the airplane. Remove all goods stored at the station. */ - for (CargoID i = 0; i < NUM_CARGO; i++) { - st->goods[i].rating = 1; - st->goods[i].cargo.Truncate(); + for (GoodsEntry &ge : st->goods) { + ge.rating = 1; + ge.cargo.Truncate(); } CrashAirplane(v); diff --git a/src/console_cmds.cpp b/src/console_cmds.cpp index 2a51df0c6c..60f9e6100f 100644 --- a/src/console_cmds.cpp +++ b/src/console_cmds.cpp @@ -2440,8 +2440,7 @@ static void ConDumpCargoTypes() IConsolePrint(CC_DEFAULT, " S = special"); std::map grfs; - for (CargoID i = 0; i < NUM_CARGO; i++) { - const CargoSpec *spec = CargoSpec::Get(i); + for (const CargoSpec *spec : CargoSpec::Iterate()) { if (!spec->IsValid()) continue; uint32_t grfid = 0; const GRFFile *grf = spec->grffile; @@ -2450,7 +2449,7 @@ static void ConDumpCargoTypes() grfs.emplace(grfid, grf); } IConsolePrint(CC_DEFAULT, " {:02d} Bit: {:2d}, Label: {:c}{:c}{:c}{:c}, Callback mask: 0x{:02X}, Cargo class: {}{}{}{}{}{}{}{}{}{}{}, GRF: {:08X}, {}", - (uint)i, + spec->Index(), spec->bitnum, spec->label >> 24, spec->label >> 16, spec->label >> 8, spec->label, spec->callback_mask, diff --git a/src/industry_gui.cpp b/src/industry_gui.cpp index c847a6eb6d..931f714330 100644 --- a/src/industry_gui.cpp +++ b/src/industry_gui.cpp @@ -2566,8 +2566,7 @@ struct IndustryCargoesWindow : public Window { /* Compute max size of the cargo texts. */ this->cargo_textsize.width = 0; this->cargo_textsize.height = 0; - for (uint i = 0; i < NUM_CARGO; i++) { - const CargoSpec *csp = CargoSpec::Get(i); + for (const CargoSpec *csp : CargoSpec::Iterate()) { if (!csp->IsValid()) continue; this->cargo_textsize = maxdim(this->cargo_textsize, GetStringBoundingBox(csp->name)); } diff --git a/src/newgrf.cpp b/src/newgrf.cpp index 228832dc2d..d6d5be5d5d 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -8852,17 +8852,16 @@ static void BuildCargoTranslationMap() { memset(_cur.grffile->cargo_map, 0xFF, sizeof(_cur.grffile->cargo_map)); - for (CargoID c = 0; c < NUM_CARGO; c++) { - const CargoSpec *cs = CargoSpec::Get(c); + for (const CargoSpec *cs : CargoSpec::Iterate()) { if (!cs->IsValid()) continue; if (_cur.grffile->cargo_list.size() == 0) { /* Default translation table, so just a straight mapping to bitnum */ - _cur.grffile->cargo_map[c] = cs->bitnum; + _cur.grffile->cargo_map[cs->Index()] = cs->bitnum; } else { /* Check the translation table for this cargo's label */ int idx = find_index(_cur.grffile->cargo_list, {cs->label}); - if (idx >= 0) _cur.grffile->cargo_map[c] = idx; + if (idx >= 0) _cur.grffile->cargo_map[cs->Index()] = idx; } } } @@ -9184,8 +9183,7 @@ static void FinaliseEngineArray() /** Check for invalid cargoes */ static void FinaliseCargoArray() { - for (CargoID c = 0; c < NUM_CARGO; c++) { - CargoSpec *cs = CargoSpec::Get(c); + for (CargoSpec *cs : CargoSpec::Iterate()) { if (!cs->IsValid()) { cs->name = cs->name_single = cs->units_volume = STR_NEWGRF_INVALID_CARGO; cs->quantifier = STR_NEWGRF_INVALID_CARGO_QUANTITY; diff --git a/src/newgrf_station.cpp b/src/newgrf_station.cpp index 17f38d2cb8..167eeb2af1 100644 --- a/src/newgrf_station.cpp +++ b/src/newgrf_station.cpp @@ -502,8 +502,8 @@ uint32_t Waypoint::GetNewGRFVariable(const ResolverObject &, byte variable, [[ma break; case CT_DEFAULT: - for (CargoID cargo_type = 0; cargo_type < NUM_CARGO; cargo_type++) { - cargo += st->goods[cargo_type].cargo.TotalCount(); + for (const GoodsEntry &ge : st->goods) { + cargo += ge.cargo.TotalCount(); } break; diff --git a/src/openttd.cpp b/src/openttd.cpp index 7c281a5eb2..8b05cfe9af 100644 --- a/src/openttd.cpp +++ b/src/openttd.cpp @@ -1300,11 +1300,11 @@ static void CheckCaches() for (Industry *ind : Industry::Iterate()) old_industry_stations_near.push_back(ind->stations_near); for (Station *st : Station::Iterate()) { - for (CargoID c = 0; c < NUM_CARGO; c++) { + for (GoodsEntry &ge : st->goods) { byte buff[sizeof(StationCargoList)]; - memcpy(buff, &st->goods[c].cargo, sizeof(StationCargoList)); - st->goods[c].cargo.InvalidateCache(); - assert(memcmp(&st->goods[c].cargo, buff, sizeof(StationCargoList)) == 0); + memcpy(buff, &ge.cargo, sizeof(StationCargoList)); + ge.cargo.InvalidateCache(); + assert(memcmp(&ge.cargo, buff, sizeof(StationCargoList)) == 0); } /* Check docking tiles */ diff --git a/src/saveload/afterload.cpp b/src/saveload/afterload.cpp index 714b0332c1..d94b17306d 100644 --- a/src/saveload/afterload.cpp +++ b/src/saveload/afterload.cpp @@ -1690,9 +1690,9 @@ bool AfterLoadGame() if (IsSavegameVersionBefore(SLV_74)) { for (Station *st : Station::Iterate()) { - for (CargoID c = 0; c < NUM_CARGO; c++) { - st->goods[c].last_speed = 0; - if (st->goods[c].cargo.AvailableCount() != 0) SetBit(st->goods[c].status, GoodsEntry::GES_RATING); + for (GoodsEntry &ge : st->goods) { + ge.last_speed = 0; + if (ge.cargo.AvailableCount() != 0) SetBit(ge.status, GoodsEntry::GES_RATING); } } } diff --git a/src/saveload/cargopacket_sl.cpp b/src/saveload/cargopacket_sl.cpp index b5ee7312ca..ba0988e0e8 100644 --- a/src/saveload/cargopacket_sl.cpp +++ b/src/saveload/cargopacket_sl.cpp @@ -43,10 +43,8 @@ * information is lost. In that case we set it to the position of this * station */ for (Station *st : Station::Iterate()) { - for (CargoID c = 0; c < NUM_CARGO; c++) { - GoodsEntry *ge = &st->goods[c]; - - const StationCargoPacketMap *packets = ge->cargo.Packets(); + for (GoodsEntry &ge : st->goods) { + const StationCargoPacketMap *packets = ge.cargo.Packets(); for (StationCargoList::ConstIterator it(packets->begin()); it != packets->end(); it++) { CargoPacket *cp = *it; cp->source_xy = Station::IsValidID(cp->first_station) ? Station::Get(cp->first_station)->xy : st->xy; @@ -69,7 +67,7 @@ for (Vehicle *v : Vehicle::Iterate()) v->cargo.InvalidateCache(); for (Station *st : Station::Iterate()) { - for (CargoID c = 0; c < NUM_CARGO; c++) st->goods[c].cargo.InvalidateCache(); + for (GoodsEntry &ge : st->goods) ge.cargo.InvalidateCache(); } } @@ -81,9 +79,8 @@ 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 (GoodsEntry &ge : st->goods) { + 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); diff --git a/src/saveload/station_sl.cpp b/src/saveload/station_sl.cpp index 8544b65428..e2b1524e08 100644 --- a/src/saveload/station_sl.cpp +++ b/src/saveload/station_sl.cpp @@ -412,8 +412,8 @@ public: SlSetStructListLength(NUM_CARGO); - for (CargoID i = 0; i < NUM_CARGO; i++) { - SlObject(&st->goods[i], this->GetDescription()); + for (GoodsEntry &ge : st->goods) { + SlObject(&ge, this->GetDescription()); } } @@ -429,15 +429,15 @@ public: std::copy(std::begin(_old_st_persistent_storage.storage), std::end(_old_st_persistent_storage.storage), std::begin(st->airport.psa->storage)); } - size_t num_cargo = this->GetNumCargo(); - for (size_t i = 0; i < num_cargo; i++) { - GoodsEntry *ge = &st->goods[i]; - SlObject(ge, this->GetLoadDescription()); + auto end = std::next(std::begin(st->goods), std::min(this->GetNumCargo(), std::size(st->goods))); + for (auto it = std::begin(st->goods); it != end; ++it) { + GoodsEntry &ge = *it; + SlObject(&ge, this->GetLoadDescription()); if (IsSavegameVersionBefore(SLV_183)) { - SwapPackets(ge); + SwapPackets(&ge); } if (IsSavegameVersionBefore(SLV_68)) { - SB(ge->status, GoodsEntry::GES_ACCEPTANCE, 1, HasBit(_waiting_acceptance, 15)); + SB(ge.status, GoodsEntry::GES_ACCEPTANCE, 1, HasBit(_waiting_acceptance, 15)); if (GB(_waiting_acceptance, 0, 12) != 0) { /* In old versions, enroute_from used 0xFF as INVALID_STATION */ StationID source = (IsSavegameVersionBefore(SLV_7) && _cargo_source == 0xFF) ? INVALID_STATION : _cargo_source; @@ -450,8 +450,8 @@ public: /* Don't construct the packet with station here, because that'll fail with old savegames */ CargoPacket *cp = new CargoPacket(GB(_waiting_acceptance, 0, 12), _cargo_periods, source, _cargo_source_xy, _cargo_feeder_share); - ge->cargo.Append(cp, INVALID_STATION); - SB(ge->status, GoodsEntry::GES_RATING, 1, 1); + ge.cargo.Append(cp, INVALID_STATION); + SB(ge.status, GoodsEntry::GES_RATING, 1, 1); } } } @@ -461,15 +461,16 @@ public: { Station *st = Station::From(bst); - uint num_cargo = IsSavegameVersionBefore(SLV_55) ? 12 : IsSavegameVersionBefore(SLV_EXTEND_CARGOTYPES) ? 32 : NUM_CARGO; - for (CargoID i = 0; i < num_cargo; i++) { - GoodsEntry *ge = &st->goods[i]; + size_t num_cargo = IsSavegameVersionBefore(SLV_55) ? 12 : IsSavegameVersionBefore(SLV_EXTEND_CARGOTYPES) ? 32 : NUM_CARGO; + auto end = std::next(std::begin(st->goods), std::min(num_cargo, std::size(st->goods))); + for (auto it = std::begin(st->goods); it != end; ++it) { + GoodsEntry &ge = *it; if (IsSavegameVersionBefore(SLV_183)) { - SwapPackets(ge); // We have to swap back again to be in the format pre-183 expects. - SlObject(ge, this->GetDescription()); - SwapPackets(ge); + SwapPackets(&ge); // We have to swap back again to be in the format pre-183 expects. + SlObject(&ge, this->GetDescription()); + SwapPackets(&ge); } else { - SlObject(ge, this->GetDescription()); + SlObject(&ge, this->GetDescription()); } } } diff --git a/src/saveload/town_sl.cpp b/src/saveload/town_sl.cpp index 82b2d85151..10c0763599 100644 --- a/src/saveload/town_sl.cpp +++ b/src/saveload/town_sl.cpp @@ -139,9 +139,9 @@ public: void Save(Town *t) const override { - SlSetStructListLength(NUM_CARGO); - for (CargoID i = 0; i < NUM_CARGO; i++) { - SlObject(&t->supplied[i], this->GetDescription()); + SlSetStructListLength(std::size(t->supplied)); + for (auto &supplied : t->supplied) { + SlObject(&supplied, this->GetDescription()); } } @@ -166,9 +166,9 @@ public: void Save(Town *t) const override { - SlSetStructListLength(NUM_TE); - for (size_t i = TE_BEGIN; i < TE_END; i++) { - SlObject(&t->received[i], this->GetDescription()); + SlSetStructListLength(std::size(t->received)); + for (auto &received : t->received) { + SlObject(&received, this->GetDescription()); } } diff --git a/src/script/api/script_industrytype.cpp b/src/script/api/script_industrytype.cpp index 7f97f47684..64ccd1260c 100644 --- a/src/script/api/script_industrytype.cpp +++ b/src/script/api/script_industrytype.cpp @@ -71,8 +71,8 @@ const IndustrySpec *ins = ::GetIndustrySpec(industry_type); ScriptList *list = new ScriptList(); - for (size_t i = 0; i < lengthof(ins->produced_cargo); i++) { - if (::IsValidCargoID(ins->produced_cargo[i])) list->AddItem(ins->produced_cargo[i]); + for (const CargoID &c : ins->produced_cargo) { + if (::IsValidCargoID(c)) list->AddItem(c); } return list; @@ -85,8 +85,8 @@ const IndustrySpec *ins = ::GetIndustrySpec(industry_type); ScriptList *list = new ScriptList(); - for (size_t i = 0; i < lengthof(ins->accepts_cargo); i++) { - if (::IsValidCargoID(ins->accepts_cargo[i])) list->AddItem(ins->accepts_cargo[i]); + for (const CargoID &c : ins->accepts_cargo) { + if (::IsValidCargoID(c)) list->AddItem(c); } return list; diff --git a/src/station.cpp b/src/station.cpp index 76b1c7d3ac..2e462d457f 100644 --- a/src/station.cpp +++ b/src/station.cpp @@ -83,8 +83,8 @@ Station::Station(TileIndex tile) : Station::~Station() { if (CleaningPool()) { - for (CargoID c = 0; c < NUM_CARGO; c++) { - this->goods[c].cargo.OnCleanPool(); + for (GoodsEntry &ge : this->goods) { + ge.cargo.OnCleanPool(); } return; } @@ -148,8 +148,8 @@ Station::~Station() /* Remove all news items */ DeleteStationNews(this->index); - for (CargoID c = 0; c < NUM_CARGO; c++) { - this->goods[c].cargo.Truncate(); + for (GoodsEntry &ge : this->goods) { + ge.cargo.Truncate(); } CargoPacket::InvalidateAllFrom(this->index); diff --git a/src/station_cmd.cpp b/src/station_cmd.cpp index 683b94e6cc..85fb5fc54c 100644 --- a/src/station_cmd.cpp +++ b/src/station_cmd.cpp @@ -496,8 +496,8 @@ CargoTypes GetAcceptanceMask(const Station *st) { CargoTypes mask = 0; - for (CargoID i = 0; i < NUM_CARGO; i++) { - if (HasBit(st->goods[i].status, GoodsEntry::GES_ACCEPTANCE)) SetBit(mask, i); + for (auto it = std::begin(st->goods); it != std::end(st->goods); ++it) { + if (HasBit(it->status, GoodsEntry::GES_ACCEPTANCE)) SetBit(mask, std::distance(std::begin(st->goods), it)); } return mask; } @@ -511,8 +511,8 @@ CargoTypes GetEmptyMask(const Station *st) { CargoTypes mask = 0; - for (CargoID i = 0; i < NUM_CARGO; i++) { - if (st->goods[i].cargo.TotalCount() == 0) SetBit(mask, i); + for (auto it = std::begin(st->goods); it != std::end(st->goods); ++it) { + if (it->cargo.TotalCount() == 0) SetBit(mask, std::distance(std::begin(st->goods), it)); } return mask; } @@ -704,10 +704,10 @@ static void UpdateStationSignCoord(BaseStation *st) if (!Station::IsExpected(st)) return; Station *full_station = Station::From(st); - for (CargoID c = 0; c < NUM_CARGO; ++c) { - LinkGraphID lg = full_station->goods[c].link_graph; + for (const GoodsEntry &ge : full_station->goods) { + LinkGraphID lg = ge.link_graph; if (!LinkGraph::IsValidID(lg)) continue; - (*LinkGraph::Get(lg))[full_station->goods[c].node].UpdateLocation(st->xy); + (*LinkGraph::Get(lg))[ge.node].UpdateLocation(st->xy); } } @@ -3595,8 +3595,8 @@ static bool StationHandleBigTick(BaseStation *st) if (Station::IsExpected(st)) { TriggerWatchedCargoCallbacks(Station::From(st)); - for (CargoID i = 0; i < NUM_CARGO; i++) { - ClrBit(Station::From(st)->goods[i].status, GoodsEntry::GES_ACCEPTED_BIGTICK); + for (GoodsEntry &ge : Station::From(st)->goods) { + ClrBit(ge.status, GoodsEntry::GES_ACCEPTED_BIGTICK); } } @@ -4027,10 +4027,9 @@ void OnTick_Station() static IntervalTimer _stations_monthly({TimerGameCalendar::MONTH, TimerGameCalendar::Priority::STATION}, [](auto) { for (Station *st : Station::Iterate()) { - for (CargoID i = 0; i < NUM_CARGO; i++) { - GoodsEntry *ge = &st->goods[i]; - SB(ge->status, GoodsEntry::GES_LAST_MONTH, 1, GB(ge->status, GoodsEntry::GES_CURRENT_MONTH, 1)); - ClrBit(ge->status, GoodsEntry::GES_CURRENT_MONTH); + for (GoodsEntry &ge : st->goods) { + SB(ge.status, GoodsEntry::GES_LAST_MONTH, 1, GB(ge.status, GoodsEntry::GES_CURRENT_MONTH, 1)); + ClrBit(ge.status, GoodsEntry::GES_CURRENT_MONTH); } } }); @@ -4039,11 +4038,9 @@ void ModifyStationRatingAround(TileIndex tile, Owner owner, int amount, uint rad { ForAllStationsRadius(tile, radius, [&](Station *st) { if (st->owner == owner && DistanceManhattan(tile, st->xy) <= radius) { - for (CargoID i = 0; i < NUM_CARGO; i++) { - GoodsEntry *ge = &st->goods[i]; - - if (ge->status != 0) { - ge->rating = ClampTo(ge->rating + amount); + for (GoodsEntry &ge : st->goods) { + if (ge.status != 0) { + ge.rating = ClampTo(ge.rating + amount); } } } diff --git a/src/station_gui.cpp b/src/station_gui.cpp index b73ca59c41..6787841420 100644 --- a/src/station_gui.cpp +++ b/src/station_gui.cpp @@ -330,8 +330,7 @@ protected: byte minr1 = 255; byte minr2 = 255; - for (CargoID j = 0; j < NUM_CARGO; j++) { - if (!HasBit(cargo_filter, j)) continue; + for (CargoID j : SetCargoBitIterator(cargo_filter)) { if (a->goods[j].HasRating()) minr1 = std::min(minr1, a->goods[j].rating); if (b->goods[j].HasRating()) minr2 = std::min(minr2, b->goods[j].rating); } diff --git a/src/town_cmd.cpp b/src/town_cmd.cpp index 528a8ce49a..9a6dd3297b 100644 --- a/src/town_cmd.cpp +++ b/src/town_cmd.cpp @@ -3277,7 +3277,7 @@ static CommandCost TownActionBribe(Town *t, DoCommandFlag flags) /* set all close by station ratings to 0 */ for (Station *st : Station::Iterate()) { if (st->town == t && st->owner == _current_company) { - for (CargoID i = 0; i < NUM_CARGO; i++) st->goods[i].rating = 0; + for (GoodsEntry &ge : st->goods) ge.rating = 0; } } @@ -3554,8 +3554,8 @@ static void UpdateTownGrowth(Town *t) static void UpdateTownAmounts(Town *t) { - for (CargoID i = 0; i < NUM_CARGO; i++) t->supplied[i].NewMonth(); - for (int i = TE_BEGIN; i < TE_END; i++) t->received[i].NewMonth(); + for (auto &supplied : t->supplied) supplied.NewMonth(); + for (auto &received : t->received) received.NewMonth(); if (t->fund_buildings_months != 0) t->fund_buildings_months--; SetWindowDirty(WC_TOWN_VIEW, t->index); diff --git a/src/vehicle_cmd.h b/src/vehicle_cmd.h index bdd5d9124c..9e989516fc 100644 --- a/src/vehicle_cmd.h +++ b/src/vehicle_cmd.h @@ -47,16 +47,16 @@ void CcStartStopVehicle(Commands cmd, const CommandCost &result, VehicleID veh_i template inline EndianBufferWriter &operator <<(EndianBufferWriter &buffer, const CargoArray &cargo_array) { - for (CargoID c = 0; c < NUM_CARGO; c++) { - buffer << cargo_array[c]; + for (const uint &amt : cargo_array) { + buffer << amt; } return buffer; } inline EndianBufferReader &operator >>(EndianBufferReader &buffer, CargoArray &cargo_array) { - for (CargoID c = 0; c < NUM_CARGO; c++) { - buffer >> cargo_array[c]; + for (uint &amt : cargo_array) { + buffer >> amt; } return buffer; }