From d5671030b16fb5e9be1982da70560efdda56b1e3 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Tue, 7 May 2024 12:13:46 +0100 Subject: [PATCH] Codechange: Add NewGRFSpecsBase class to hold class/index information. Standardises how the class index is stored in the spec, instead of relying ot the Spec structs having the same members. This allows retrieving class_index and index without searching or using pointer arithmetic. 'cls_id' is renamed to 'class_index' to make it clearer that it is an index rather than the multichar label of the class. --- src/newgrf.cpp | 16 +++---- src/newgrf_airport.h | 3 +- src/newgrf_class.h | 26 +++++++---- src/newgrf_class_func.h | 82 +++++++++++++++++----------------- src/newgrf_object.cpp | 6 +-- src/newgrf_object.h | 3 +- src/newgrf_roadstop.h | 3 +- src/newgrf_station.h | 5 +-- src/saveload/station_sl.cpp | 4 +- src/script/api/script_rail.cpp | 5 +-- src/station_cmd.cpp | 4 +- src/table/airport_defaults.h | 2 +- src/table/object_land.h | 2 +- 13 files changed, 83 insertions(+), 78 deletions(-) diff --git a/src/newgrf.cpp b/src/newgrf.cpp index 479552a880..01f96961f2 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -1945,7 +1945,7 @@ static ChangeInfoResult StationChangeInfo(uint stid, int numinfo, int prop, Byte /* Swap classid because we read it in BE meaning WAYP or DFLT */ uint32_t classid = buf->ReadDWord(); - statspec->cls_id = StationClass::Allocate(BSWAP32(classid)); + statspec->class_index = StationClass::Allocate(BSWAP32(classid)); break; } @@ -2141,7 +2141,7 @@ static ChangeInfoResult StationChangeInfo(uint stid, int numinfo, int prop, Byte break; case 0x1D: // Station Class name - AddStringForMapping(buf->ReadWord(), [statspec](StringID str) { StationClass::Get(statspec->cls_id)->name = str; }); + AddStringForMapping(buf->ReadWord(), [statspec](StringID str) { StationClass::Get(statspec->class_index)->name = str; }); break; default: @@ -4103,12 +4103,12 @@ static ChangeInfoResult ObjectChangeInfo(uint id, int numinfo, int prop, ByteRea /* Swap classid because we read it in BE. */ uint32_t classid = buf->ReadDWord(); - spec->cls_id = ObjectClass::Allocate(BSWAP32(classid)); + spec->class_index = ObjectClass::Allocate(BSWAP32(classid)); break; } case 0x09: { // Class name - AddStringForMapping(buf->ReadWord(), [spec](StringID str) { ObjectClass::Get(spec->cls_id)->name = str; }); + AddStringForMapping(buf->ReadWord(), [spec](StringID str) { ObjectClass::Get(spec->class_index)->name = str; }); break; } @@ -4790,7 +4790,7 @@ static ChangeInfoResult RoadStopChangeInfo(uint id, int numinfo, int prop, ByteR } uint32_t classid = buf->ReadDWord(); - rs->cls_id = RoadStopClass::Allocate(BSWAP32(classid)); + rs->class_index = RoadStopClass::Allocate(BSWAP32(classid)); break; } @@ -4803,7 +4803,7 @@ static ChangeInfoResult RoadStopChangeInfo(uint id, int numinfo, int prop, ByteR break; case 0x0B: // Road Stop Class name - AddStringForMapping(buf->ReadWord(), [rs](StringID str) { RoadStopClass::Get(rs->cls_id)->name = str; }); + AddStringForMapping(buf->ReadWord(), [rs](StringID str) { RoadStopClass::Get(rs->class_index)->name = str; }); break; case 0x0C: // The draw mode @@ -6270,8 +6270,8 @@ static void FeatureNewName(ByteReader *buf) if (GB(id, 0, 8) >= _cur.grffile->stations.size() || _cur.grffile->stations[GB(id, 0, 8)] == nullptr) { GrfMsg(1, "FeatureNewName: Attempt to name undefined station 0x{:X}, ignoring", GB(id, 0, 8)); } else { - StationClassID cls_id = _cur.grffile->stations[GB(id, 0, 8)]->cls_id; - StationClass::Get(cls_id)->name = AddGRFString(_cur.grffile->grfid, id, lang, new_scheme, false, name, STR_UNDEFINED); + StationClassID class_index = _cur.grffile->stations[GB(id, 0, 8)]->class_index; + StationClass::Get(class_index)->name = AddGRFString(_cur.grffile->grfid, id, lang, new_scheme, false, name, STR_UNDEFINED); } break; diff --git a/src/newgrf_airport.h b/src/newgrf_airport.h index 057ae346fc..21ca905e37 100644 --- a/src/newgrf_airport.h +++ b/src/newgrf_airport.h @@ -102,7 +102,7 @@ struct AirportTileLayout { /** * Defines the data structure for an airport. */ -struct AirportSpec { +struct AirportSpec : NewGRFSpecBase { const struct AirportFTAClass *fsm; ///< the finite statemachine for the default airports std::vector layouts; ///< List of layouts composing the airport. std::span depots; ///< Position of the depots on the airports. @@ -114,7 +114,6 @@ struct AirportSpec { TimerGameCalendar::Year max_year; ///< last year the airport is available StringID name; ///< name of this airport TTDPAirportType ttd_airport_type; ///< ttdpatch airport type (Small/Large/Helipad/Oilrig) - AirportClassID cls_id; ///< the class to which this airport type belongs SpriteID preview_sprite; ///< preview sprite for this airport uint16_t maintenance_cost; ///< maintenance cost multiplier /* Newgrf data */ diff --git a/src/newgrf_class.h b/src/newgrf_class.h index 2097a9eaca..a4c12f1cfc 100644 --- a/src/newgrf_class.h +++ b/src/newgrf_class.h @@ -12,21 +12,31 @@ #include "strings_type.h" +/* Base for each type of NewGRF spec to be used with NewGRFClass. */ +template +struct NewGRFSpecBase { + Tindex class_index; ///< Class index of this spec, invalid until class is allocated. + uint16_t index; ///< Index within class of this spec, invalid until inserted into class. +}; /** * Struct containing information relating to NewGRF classes for stations and airports. */ -template +template class NewGRFClass { private: + /* Tspec must be of NewGRFSpecBase. */ + static_assert(std::is_base_of_v, Tspec>); + uint ui_count = 0; ///< Number of specs in this class potentially available to the user. + Tindex index; ///< Index of class within the list of classes. std::vector spec; ///< List of specifications. /** * The actual classes. * @note This may be reallocated during initialization so pointers may be invalidated. */ - static inline std::vector> classes; + static inline std::vector> classes; /** Initialise the defaults. */ static void InsertDefaults(); @@ -48,11 +58,11 @@ public: * Get read-only span of all classes of this type. * @return Read-only span of classes. */ - static std::span const> Classes() { return NewGRFClass::classes; } + static std::span const> Classes() { return NewGRFClass::classes; } void Insert(Tspec *spec); - Tid Index() const { return static_cast(std::distance(&*std::cbegin(NewGRFClass::classes), this)); } + Tindex Index() const { return this->index; } /** Get the number of allocated specs within the class. */ uint GetSpecCount() const { return static_cast(this->spec.size()); } /** Get the number of potentially user-available specs within the class. */ @@ -66,14 +76,14 @@ public: bool IsUIAvailable(uint index) const; static void Reset(); - static Tid Allocate(uint32_t global_id); + static Tindex Allocate(uint32_t global_id); static void Assign(Tspec *spec); static uint GetClassCount(); static uint GetUIClassCount(); - static Tid GetUIClass(uint index); - static NewGRFClass *Get(Tid cls_id); + static Tindex GetUIClass(uint index); + static NewGRFClass *Get(Tindex class_index); - static const Tspec *GetByGrf(uint32_t grfid, uint16_t local_id, int *index); + static const Tspec *GetByGrf(uint32_t grfid, uint16_t local_id); }; #endif /* NEWGRF_CLASS_H */ diff --git a/src/newgrf_class_func.h b/src/newgrf_class_func.h index ef9490a0c1..dce269649b 100644 --- a/src/newgrf_class_func.h +++ b/src/newgrf_class_func.h @@ -12,8 +12,8 @@ #include "table/strings.h" /** Reset the classes, i.e. clear everything. */ -template -void NewGRFClass::Reset() +template +void NewGRFClass::Reset() { NewGRFClass::classes.clear(); NewGRFClass::classes.shrink_to_fit(); @@ -28,8 +28,8 @@ void NewGRFClass::Reset() * @note Upon allocating the same global class ID for a * second time, this first allocation will be given. */ -template -Tid NewGRFClass::Allocate(uint32_t global_id) +template +Tindex NewGRFClass::Allocate(uint32_t global_id) { auto found = std::find_if(std::begin(NewGRFClass::classes), std::end(NewGRFClass::classes), [global_id](const auto &cls) { return cls.global_id == global_id; }); @@ -38,56 +38,59 @@ Tid NewGRFClass::Allocate(uint32_t global_id) /* More slots available, allocate a slot to the global id. */ if (NewGRFClass::classes.size() < Tmax) { - auto &cls = NewGRFClass::classes.emplace_back(global_id, STR_EMPTY); - return cls.Index(); + auto it = NewGRFClass::classes.emplace(std::end(NewGRFClass::classes), global_id, STR_EMPTY); + it->index = static_cast(std::distance(std::begin(NewGRFClass::classes), it)); + return it->Index(); } GrfMsg(2, "ClassAllocate: already allocated {} classes, using default", Tmax); - return static_cast(0); + return static_cast(0); } /** - * Insert a spec into the class. + * Insert a spec into the class, and update its index. * @param spec The spec to insert. */ -template -void NewGRFClass::Insert(Tspec *spec) +template +void NewGRFClass::Insert(Tspec *spec) { - this->spec.push_back(spec); + auto it = this->spec.insert(std::end(this->spec), spec); + uint16_t index = static_cast(std::distance(std::begin(this->spec), it)); + if (spec != nullptr) (*it)->index = index; - if (this->IsUIAvailable(static_cast(this->spec.size() - 1))) this->ui_count++; + if (this->IsUIAvailable(index)) this->ui_count++; } /** * Assign a spec to one of the classes. * @param spec The spec to assign. - * @note The spec must have a valid class id set. + * @note The spec must have a valid class index set. */ -template -void NewGRFClass::Assign(Tspec *spec) +template +void NewGRFClass::Assign(Tspec *spec) { - assert(static_cast(spec->cls_id) < NewGRFClass::classes.size()); - Get(spec->cls_id)->Insert(spec); + assert(static_cast(spec->class_index) < NewGRFClass::classes.size()); + Get(spec->class_index)->Insert(spec); } /** * Get a particular class. - * @param cls_id The id for the class. - * @pre cls_id < Tmax + * @param class_index The index of the class. + * @pre class_index < Tmax */ -template -NewGRFClass *NewGRFClass::Get(Tid cls_id) +template +NewGRFClass *NewGRFClass::Get(Tindex class_index) { - assert(static_cast(cls_id) < NewGRFClass::classes.size()); - return &NewGRFClass::classes[cls_id]; + assert(static_cast(class_index) < NewGRFClass::classes.size()); + return &NewGRFClass::classes[class_index]; } /** * Get the number of allocated classes. * @return The number of classes. */ -template -uint NewGRFClass::GetClassCount() +template +uint NewGRFClass::GetClassCount() { return static_cast(NewGRFClass::classes.size()); } @@ -96,8 +99,8 @@ uint NewGRFClass::GetClassCount() * Get the number of classes available to the user. * @return The number of classes. */ -template -uint NewGRFClass::GetUIClassCount() +template +uint NewGRFClass::GetUIClassCount() { return std::count_if(std::begin(NewGRFClass::classes), std::end(NewGRFClass::classes), [](const auto &cls) { return cls.GetUISpecCount() > 0; }); } @@ -107,8 +110,8 @@ uint NewGRFClass::GetUIClassCount() * @param index UI index of a class. * @return The class ID of the class. */ -template -Tid NewGRFClass::GetUIClass(uint index) +template +Tindex NewGRFClass::GetUIClass(uint index) { for (const auto &cls : NewGRFClass::classes) { if (cls.GetUISpecCount() == 0) continue; @@ -122,8 +125,8 @@ Tid NewGRFClass::GetUIClass(uint index) * @param index The index where to find the spec. * @return The spec at given location. */ -template -const Tspec *NewGRFClass::GetSpec(uint index) const +template +const Tspec *NewGRFClass::GetSpec(uint index) const { /* If the custom spec isn't defined any more, then the GRF file probably was not loaded. */ return index < this->GetSpecCount() ? this->spec[index] : nullptr; @@ -134,8 +137,8 @@ const Tspec *NewGRFClass::GetSpec(uint index) const * @param ui_index UI index of the spec. * @return index of the spec, or -1 if out of range. */ -template -int NewGRFClass::GetIndexFromUI(int ui_index) const +template +int NewGRFClass::GetIndexFromUI(int ui_index) const { if (ui_index < 0) return -1; for (uint i = 0; i < this->GetSpecCount(); i++) { @@ -150,8 +153,8 @@ int NewGRFClass::GetIndexFromUI(int ui_index) const * @param index index of the spec. * @return UI index of the spec, or -1 if out of range. */ -template -int NewGRFClass::GetUIFromIndex(int index) const +template +int NewGRFClass::GetUIFromIndex(int index) const { if ((uint)index >= this->GetSpecCount()) return -1; uint ui_index = 0; @@ -168,16 +171,13 @@ int NewGRFClass::GetUIFromIndex(int index) const * @param index Pointer to return the index of the spec in its class. If nullptr then not used. * @return The spec. */ -template -const Tspec *NewGRFClass::GetByGrf(uint32_t grfid, uint16_t local_id, int *index) +template +const Tspec *NewGRFClass::GetByGrf(uint32_t grfid, uint16_t local_id) { for (const auto &cls : NewGRFClass::classes) { for (const auto &spec : cls.spec) { if (spec == nullptr) continue; - if (spec->grf_prop.grffile->grfid == grfid && spec->grf_prop.local_id == local_id) { - if (index != nullptr) *index = static_cast(std::distance(cls.spec.data(), &spec)); - return spec; - } + if (spec->grf_prop.grffile->grfid == grfid && spec->grf_prop.local_id == local_id) return spec; } } diff --git a/src/newgrf_object.cpp b/src/newgrf_object.cpp index 7d9765f1f6..8491c98382 100644 --- a/src/newgrf_object.cpp +++ b/src/newgrf_object.cpp @@ -111,7 +111,7 @@ uint ObjectSpec::Index() const /* static */ void ObjectSpec::BindToClasses() { for (auto &spec : _object_specs) { - if (spec.IsEnabled() && spec.cls_id != INVALID_OBJECT_CLASS) { + if (spec.IsEnabled() && spec.class_index != INVALID_OBJECT_CLASS) { ObjectClass::Assign(&spec); } } @@ -132,8 +132,8 @@ void ResetObjects() } /* Set class for originals. */ - _object_specs[OBJECT_LIGHTHOUSE].cls_id = ObjectClass::Allocate('LTHS'); - _object_specs[OBJECT_TRANSMITTER].cls_id = ObjectClass::Allocate('TRNS'); + _object_specs[OBJECT_LIGHTHOUSE].class_index = ObjectClass::Allocate('LTHS'); + _object_specs[OBJECT_TRANSMITTER].class_index = ObjectClass::Allocate('TRNS'); } template <> diff --git a/src/newgrf_object.h b/src/newgrf_object.h index 9b124ab0f8..2308d100ed 100644 --- a/src/newgrf_object.h +++ b/src/newgrf_object.h @@ -57,11 +57,10 @@ DECLARE_POSTFIX_INCREMENT(ObjectClassID) * @note If you change this struct, adopt the initialization of * default objects in table/object_land.h */ -struct ObjectSpec { +struct ObjectSpec : NewGRFSpecBase { /* 2 because of the "normal" and "buy" sprite stacks. */ GRFFilePropsBase<2> grf_prop; ///< Properties related the the grf file AnimationInfo animation; ///< Information about the animation. - ObjectClassID cls_id; ///< The class to which this spec belongs. StringID name; ///< The name for this object. uint8_t climate; ///< In which climates is this object available? diff --git a/src/newgrf_roadstop.h b/src/newgrf_roadstop.h index 4a6980883d..4c031c560a 100644 --- a/src/newgrf_roadstop.h +++ b/src/newgrf_roadstop.h @@ -118,7 +118,7 @@ struct RoadStopResolverObject : public ResolverObject { }; /** Road stop specification. */ -struct RoadStopSpec { +struct RoadStopSpec : NewGRFSpecBase { /** * Properties related the the grf file. * NUM_CARGO real cargo plus three pseudo cargo sprite groups. @@ -126,7 +126,6 @@ struct RoadStopSpec { * evaluating callbacks. */ GRFFilePropsBase grf_prop; - RoadStopClassID cls_id; ///< The class to which this spec belongs. StringID name; ///< Name of this stop RoadStopAvailabilityType stop_type = ROADSTOPTYPE_ALL; diff --git a/src/newgrf_station.h b/src/newgrf_station.h index f1c619153e..3fd47d8676 100644 --- a/src/newgrf_station.h +++ b/src/newgrf_station.h @@ -108,8 +108,8 @@ enum StationRandomTrigger { }; /** Station specification. */ -struct StationSpec { - StationSpec() : cls_id(STAT_CLASS_DFLT), name(0), +struct StationSpec : NewGRFSpecBase { + StationSpec() : name(0), disallowed_platforms(0), disallowed_lengths(0), cargo_threshold(0), cargo_triggers(0), callback_mask(0), flags(0), pylons(0), wires(0), blocked(0), @@ -121,7 +121,6 @@ struct StationSpec { * evaluating callbacks. */ GRFFilePropsBase grf_prop; - StationClassID cls_id; ///< The class to which this spec belongs. StringID name; ///< Name of this station. /** diff --git a/src/saveload/station_sl.cpp b/src/saveload/station_sl.cpp index 0db9509e53..c964ceb46a 100644 --- a/src/saveload/station_sl.cpp +++ b/src/saveload/station_sl.cpp @@ -114,11 +114,11 @@ void AfterLoadStations() for (BaseStation *st : BaseStation::Iterate()) { for (auto &sm : GetStationSpecList(st)) { if (sm.grfid == 0) continue; - sm.spec = StationClass::GetByGrf(sm.grfid, sm.localidx, nullptr); + sm.spec = StationClass::GetByGrf(sm.grfid, sm.localidx); } for (auto &sm : GetStationSpecList(st)) { if (sm.grfid == 0) continue; - sm.spec = RoadStopClass::GetByGrf(sm.grfid, sm.localidx, nullptr); + sm.spec = RoadStopClass::GetByGrf(sm.grfid, sm.localidx); } if (Station::IsExpected(st)) { diff --git a/src/script/api/script_rail.cpp b/src/script/api/script_rail.cpp index 92188c2ab7..ac1b9fc5a9 100644 --- a/src/script/api/script_rail.cpp +++ b/src/script/api/script_rail.cpp @@ -193,13 +193,12 @@ bool adjacent = station_id != ScriptStation::STATION_JOIN_ADJACENT; StationID to_join = ScriptStation::IsValidStation(station_id) ? station_id : INVALID_STATION; if (res != CALLBACK_FAILED) { - int index = 0; - const StationSpec *spec = StationClass::GetByGrf(file->grfid, res, &index); + const StationSpec *spec = StationClass::GetByGrf(file->grfid, res); if (spec == nullptr) { Debug(grf, 1, "{} returned an invalid station ID for 'AI construction/purchase selection (18)' callback", file->filename); } else { /* We might have gotten an usable station spec. Try to build it, but if it fails we'll fall back to the original station. */ - if (ScriptObject::Command::Do(tile, (::RailType)GetCurrentRailType(), axis, num_platforms, platform_length, spec->cls_id, index, to_join, adjacent)) return true; + if (ScriptObject::Command::Do(tile, (::RailType)GetCurrentRailType(), axis, num_platforms, platform_length, spec->class_index, spec->index, to_join, adjacent)) return true; } } diff --git a/src/station_cmd.cpp b/src/station_cmd.cpp index de949f9a2d..dad0f4561c 100644 --- a/src/station_cmd.cpp +++ b/src/station_cmd.cpp @@ -3346,7 +3346,7 @@ void FillTileDescRailStation(TileIndex tile, TileDesc *td) const StationSpec *spec = GetStationSpec(tile); if (spec != nullptr) { - td->station_class = StationClass::Get(spec->cls_id)->name; + td->station_class = StationClass::Get(spec->class_index)->name; td->station_name = spec->name; if (spec->grf_prop.grffile != nullptr) { @@ -3363,7 +3363,7 @@ void FillTileDescRailStation(TileIndex tile, TileDesc *td) void FillTileDescAirport(TileIndex tile, TileDesc *td) { const AirportSpec *as = Station::GetByTile(tile)->airport.GetSpec(); - td->airport_class = AirportClass::Get(as->cls_id)->name; + td->airport_class = AirportClass::Get(as->class_index)->name; td->airport_name = as->name; const AirportTileSpec *ats = AirportTileSpec::GetByTile(tile); diff --git a/src/table/airport_defaults.h b/src/table/airport_defaults.h index d9db0c15b6..0a41f7ec8c 100644 --- a/src/table/airport_defaults.h +++ b/src/table/airport_defaults.h @@ -378,7 +378,7 @@ static const std::initializer_list _tile_table_helistation = /** General AirportSpec definition. */ #define AS_GENERIC(fsm, layouts, depots, size_x, size_y, noise, catchment, min_year, max_year, maint_cost, ttdpatch_type, class_id, name, preview, enabled) \ - {fsm, layouts, depots, size_x, size_y, noise, catchment, min_year, max_year, name, ttdpatch_type, class_id, preview, maint_cost, enabled, GRFFileProps(AT_INVALID)} + {{class_id, 0}, fsm, layouts, depots, size_x, size_y, noise, catchment, min_year, max_year, name, ttdpatch_type, preview, maint_cost, enabled, GRFFileProps(AT_INVALID)} /** AirportSpec definition for airports without any depot. */ #define AS_ND(ap_name, size_x, size_y, min_year, max_year, catchment, noise, maint_cost, ttdpatch_type, class_id, name, preview) \ diff --git a/src/table/object_land.h b/src/table/object_land.h index ac1653c97b..ee4ab59c8c 100644 --- a/src/table/object_land.h +++ b/src/table/object_land.h @@ -121,7 +121,7 @@ static const DrawTileSprites _object_hq[] = { #undef TILE_SPRITE_LINE -#define M(name, size, build_cost_multiplier, clear_cost_multiplier, height, climate, gen_amount, flags) { GRFFilePropsBase<2>(), {0, 0, 0, 0}, INVALID_OBJECT_CLASS, name, climate, size, build_cost_multiplier, clear_cost_multiplier, 0, CalendarTime::MAX_DATE + 1, flags, 0, height, 1, gen_amount } +#define M(name, size, build_cost_multiplier, clear_cost_multiplier, height, climate, gen_amount, flags) {{INVALID_OBJECT_CLASS, 0}, GRFFilePropsBase<2>(), {0, 0, 0, 0}, name, climate, size, build_cost_multiplier, clear_cost_multiplier, 0, CalendarTime::MAX_DATE + 1, flags, 0, height, 1, gen_amount} /* Climates * T = Temperate