From 296ea3167f4aa2161ea94d4e5f6a082aa3f6ac98 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Tue, 15 Apr 2025 19:42:04 +0100 Subject: [PATCH] Codefix: Information for NewGRF specs remained duplicated after loading. After loading airports+tiles, industries+tiles, houses and objects, their specs are copied from the NewGRF's loading storage to the final global storage. Instead, move the specs to the their new storage, and clear the NewGRF's storage once done. (Stations and RoadStops are different, and the NewGRF's storage is the final storage location.) --- src/newgrf.cpp | 46 ++++++++++++++++++++++++++----------- src/newgrf_airport.cpp | 8 +++---- src/newgrf_airport.h | 2 ++ src/newgrf_airporttiles.cpp | 8 +++---- src/newgrf_airporttiles.h | 2 +- src/newgrf_commons.cpp | 34 +++++++++++++-------------- src/newgrf_commons.h | 12 +++++----- 7 files changed, 67 insertions(+), 45 deletions(-) diff --git a/src/newgrf.cpp b/src/newgrf.cpp index 4ec1f05dc3..224e6ca6fc 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -1027,12 +1027,12 @@ static void FinaliseHouseArray() * On the other hand, why 1930? Just 'fix' the houses with the lowest * minimum introduction date to 0. */ - for (const auto &file : _grf_files) { + for (auto &file : _grf_files) { if (file.housespec.empty()) continue; size_t num_houses = file.housespec.size(); for (size_t i = 0; i < num_houses; i++) { - HouseSpec *hs = file.housespec[i].get(); + auto &hs = file.housespec[i]; if (hs == nullptr) continue; @@ -1042,8 +1042,12 @@ static void FinaliseHouseArray() if (!IsHouseSpecValid(*hs, next1, next2, next3, file.filename)) continue; - _house_mngr.SetEntitySpec(hs); + _house_mngr.SetEntitySpec(std::move(*hs)); } + + /* Won't be used again */ + file.housespec.clear(); + file.housespec.shrink_to_fit(); } for (size_t i = 0; i < HouseSpec::Specs().size(); i++) { @@ -1096,18 +1100,24 @@ static void FinaliseHouseArray() */ static void FinaliseIndustriesArray() { - for (const auto &file : _grf_files) { - for (const auto &indsp : file.industryspec) { + for (auto &file : _grf_files) { + for (auto &indsp : file.industryspec) { if (indsp == nullptr || !indsp->enabled) continue; - _industry_mngr.SetEntitySpec(indsp.get()); + _industry_mngr.SetEntitySpec(std::move(*indsp)); } - for (const auto &indtsp : file.indtspec) { + for (auto &indtsp : file.indtspec) { if (indtsp != nullptr) { - _industile_mngr.SetEntitySpec(indtsp.get()); + _industile_mngr.SetEntitySpec(std::move(*indtsp)); } } + + /* Won't be used again */ + file.industryspec.clear(); + file.industryspec.shrink_to_fit(); + file.indtspec.clear(); + file.indtspec.shrink_to_fit(); } for (auto &indsp : _industry_specs) { @@ -1144,12 +1154,16 @@ static void FinaliseIndustriesArray() */ static void FinaliseObjectsArray() { - for (const auto &file : _grf_files) { + for (auto &file : _grf_files) { for (auto &objectspec : file.objectspec) { if (objectspec != nullptr && objectspec->grf_prop.HasGrfFile() && objectspec->IsEnabled()) { - _object_mngr.SetEntitySpec(objectspec.get()); + _object_mngr.SetEntitySpec(std::move(*objectspec)); } } + + /* Won't be used again */ + file.objectspec.clear(); + file.objectspec.shrink_to_fit(); } ObjectSpec::BindToClasses(); @@ -1162,18 +1176,24 @@ static void FinaliseObjectsArray() */ static void FinaliseAirportsArray() { - for (const auto &file : _grf_files) { + for (auto &file : _grf_files) { for (auto &as : file.airportspec) { if (as != nullptr && as->enabled) { - _airport_mngr.SetEntitySpec(as.get()); + _airport_mngr.SetEntitySpec(std::move(*as)); } } for (auto &ats : file.airtspec) { if (ats != nullptr && ats->enabled) { - _airporttile_mngr.SetEntitySpec(ats.get()); + _airporttile_mngr.SetEntitySpec(std::move(*ats)); } } + + /* Won't be used again */ + file.airportspec.clear(); + file.airportspec.shrink_to_fit(); + file.airtspec.clear(); + file.airtspec.shrink_to_fit(); } } diff --git a/src/newgrf_airport.cpp b/src/newgrf_airport.cpp index fdd5fc3739..a22e87bb9d 100644 --- a/src/newgrf_airport.cpp +++ b/src/newgrf_airport.cpp @@ -134,22 +134,22 @@ void BindAirportSpecs() } -void AirportOverrideManager::SetEntitySpec(AirportSpec *as) +void AirportOverrideManager::SetEntitySpec(AirportSpec &&as) { - uint8_t airport_id = this->AddEntityID(as->grf_prop.local_id, as->grf_prop.grfid, as->grf_prop.subst_id); + uint8_t airport_id = this->AddEntityID(as.grf_prop.local_id, as.grf_prop.grfid, as.grf_prop.subst_id); if (airport_id == this->invalid_id) { GrfMsg(1, "Airport.SetEntitySpec: Too many airports allocated. Ignoring."); return; } - *AirportSpec::GetWithoutOverride(airport_id) = *as; + AirportSpec::specs[airport_id] = std::move(as); /* Now add the overrides. */ for (int i = 0; i < this->max_offset; i++) { AirportSpec *overridden_as = AirportSpec::GetWithoutOverride(i); - if (this->entity_overrides[i] != as->grf_prop.local_id || this->grfid_overrides[i] != as->grf_prop.grfid) continue; + if (this->entity_overrides[i] != AirportSpec::specs[airport_id].grf_prop.local_id || this->grfid_overrides[i] != AirportSpec::specs[airport_id].grf_prop.grfid) continue; overridden_as->grf_prop.override_id = airport_id; overridden_as->enabled = false; diff --git a/src/newgrf_airport.h b/src/newgrf_airport.h index f9faa33921..59e811b80e 100644 --- a/src/newgrf_airport.h +++ b/src/newgrf_airport.h @@ -141,6 +141,8 @@ struct AirportSpec : NewGRFSpecBase { private: static AirportSpec specs[NUM_AIRPORTS]; ///< Specs of the airports. + + friend void AirportOverrideManager::SetEntitySpec(AirportSpec &&as); }; /** Information related to airport classes. */ diff --git a/src/newgrf_airporttiles.cpp b/src/newgrf_airporttiles.cpp index f47e9e4af6..eea0f118c3 100644 --- a/src/newgrf_airporttiles.cpp +++ b/src/newgrf_airporttiles.cpp @@ -66,22 +66,22 @@ void AirportTileSpec::ResetAirportTiles() _airporttile_mngr.ResetOverride(); } -void AirportTileOverrideManager::SetEntitySpec(const AirportTileSpec *airpts) +void AirportTileOverrideManager::SetEntitySpec(AirportTileSpec &&airpts) { - StationGfx airpt_id = this->AddEntityID(airpts->grf_prop.local_id, airpts->grf_prop.grfid, airpts->grf_prop.subst_id); + StationGfx airpt_id = this->AddEntityID(airpts.grf_prop.local_id, airpts.grf_prop.grfid, airpts.grf_prop.subst_id); if (airpt_id == this->invalid_id) { GrfMsg(1, "AirportTile.SetEntitySpec: Too many airport tiles allocated. Ignoring."); return; } - AirportTileSpec::tiles[airpt_id] = *airpts; + AirportTileSpec::tiles[airpt_id] = std::move(airpts); /* Now add the overrides. */ for (int i = 0; i < this->max_offset; i++) { AirportTileSpec *overridden_airpts = &AirportTileSpec::tiles[i]; - if (this->entity_overrides[i] != airpts->grf_prop.local_id || this->grfid_overrides[i] != airpts->grf_prop.grfid) continue; + if (this->entity_overrides[i] != AirportTileSpec::tiles[airpt_id].grf_prop.local_id || this->grfid_overrides[i] != AirportTileSpec::tiles[airpt_id].grf_prop.grfid) continue; overridden_airpts->grf_prop.override_id = airpt_id; overridden_airpts->enabled = false; diff --git a/src/newgrf_airporttiles.h b/src/newgrf_airporttiles.h index 1f2d989c33..68a62d1b07 100644 --- a/src/newgrf_airporttiles.h +++ b/src/newgrf_airporttiles.h @@ -84,7 +84,7 @@ struct AirportTileSpec { private: static AirportTileSpec tiles[NUM_AIRPORTTILES]; - friend void AirportTileOverrideManager::SetEntitySpec(const AirportTileSpec *airpts); + friend void AirportTileOverrideManager::SetEntitySpec(AirportTileSpec &&airpts); }; void AnimateAirportTile(TileIndex tile); diff --git a/src/newgrf_commons.cpp b/src/newgrf_commons.cpp index 6ed612ca81..a46b5477d6 100644 --- a/src/newgrf_commons.cpp +++ b/src/newgrf_commons.cpp @@ -157,9 +157,9 @@ uint16_t OverrideManagerBase::GetSubstituteID(uint16_t entity_id) const * It will find itself the proper slot on which it will go * @param hs HouseSpec read from the grf file, ready for inclusion */ -void HouseOverrideManager::SetEntitySpec(const HouseSpec *hs) +void HouseOverrideManager::SetEntitySpec(HouseSpec &&hs) { - HouseID house_id = this->AddEntityID(hs->grf_prop.local_id, hs->grf_prop.grfid, hs->grf_prop.subst_id); + HouseID house_id = this->AddEntityID(hs.grf_prop.local_id, hs.grf_prop.grfid, hs.grf_prop.subst_id); if (house_id == this->invalid_id) { GrfMsg(1, "House.SetEntitySpec: Too many houses allocated. Ignoring."); @@ -170,13 +170,13 @@ void HouseOverrideManager::SetEntitySpec(const HouseSpec *hs) /* Now that we know we can use the given id, copy the spec to its final destination. */ if (house_id >= house_specs.size()) house_specs.resize(house_id + 1); - house_specs[house_id] = *hs; + house_specs[house_id] = std::move(hs); /* Now add the overrides. */ for (int i = 0; i < this->max_offset; i++) { HouseSpec *overridden_hs = HouseSpec::Get(i); - if (this->entity_overrides[i] != hs->grf_prop.local_id || this->grfid_overrides[i] != hs->grf_prop.grfid) continue; + if (this->entity_overrides[i] != house_specs[house_id].grf_prop.local_id || this->grfid_overrides[i] != house_specs[house_id].grf_prop.grfid) continue; overridden_hs->grf_prop.override_id = house_id; this->entity_overrides[i] = this->invalid_id; @@ -245,18 +245,18 @@ uint16_t IndustryOverrideManager::AddEntityID(uint16_t grf_local_id, uint32_t gr * checking what is available * @param inds Industryspec that comes from the grf decoding process */ -void IndustryOverrideManager::SetEntitySpec(IndustrySpec *inds) +void IndustryOverrideManager::SetEntitySpec(IndustrySpec &&inds) { /* First step : We need to find if this industry is already specified in the savegame data. */ - IndustryType ind_id = this->GetID(inds->grf_prop.local_id, inds->grf_prop.grfid); + IndustryType ind_id = this->GetID(inds.grf_prop.local_id, inds.grf_prop.grfid); if (ind_id == this->invalid_id) { /* Not found. * Or it has already been overridden, so you've lost your place. * Or it is a simple substitute. * We need to find a free available slot */ - ind_id = this->AddEntityID(inds->grf_prop.local_id, inds->grf_prop.grfid, inds->grf_prop.subst_id); - inds->grf_prop.override_id = this->invalid_id; // make sure it will not be detected as overridden + ind_id = this->AddEntityID(inds.grf_prop.local_id, inds.grf_prop.grfid, inds.grf_prop.subst_id); + inds.grf_prop.override_id = this->invalid_id; // make sure it will not be detected as overridden } if (ind_id == this->invalid_id) { @@ -265,27 +265,27 @@ void IndustryOverrideManager::SetEntitySpec(IndustrySpec *inds) } /* Now that we know we can use the given id, copy the spec to its final destination... */ - _industry_specs[ind_id] = *inds; + _industry_specs[ind_id] = std::move(inds); /* ... and mark it as usable*/ _industry_specs[ind_id].enabled = true; } -void IndustryTileOverrideManager::SetEntitySpec(const IndustryTileSpec *its) +void IndustryTileOverrideManager::SetEntitySpec(IndustryTileSpec &&its) { - IndustryGfx indt_id = this->AddEntityID(its->grf_prop.local_id, its->grf_prop.grfid, its->grf_prop.subst_id); + IndustryGfx indt_id = this->AddEntityID(its.grf_prop.local_id, its.grf_prop.grfid, its.grf_prop.subst_id); if (indt_id == this->invalid_id) { GrfMsg(1, "IndustryTile.SetEntitySpec: Too many industry tiles allocated. Ignoring."); return; } - _industry_tile_specs[indt_id] = *its; + _industry_tile_specs[indt_id] = std::move(its); /* Now add the overrides. */ for (int i = 0; i < this->max_offset; i++) { IndustryTileSpec *overridden_its = &_industry_tile_specs[i]; - if (this->entity_overrides[i] != its->grf_prop.local_id || this->grfid_overrides[i] != its->grf_prop.grfid) continue; + if (this->entity_overrides[i] != _industry_tile_specs[indt_id].grf_prop.local_id || this->grfid_overrides[i] != _industry_tile_specs[indt_id].grf_prop.grfid) continue; overridden_its->grf_prop.override_id = indt_id; overridden_its->enabled = false; @@ -300,17 +300,17 @@ void IndustryTileOverrideManager::SetEntitySpec(const IndustryTileSpec *its) * checking what is available * @param spec ObjectSpec that comes from the grf decoding process */ -void ObjectOverrideManager::SetEntitySpec(ObjectSpec *spec) +void ObjectOverrideManager::SetEntitySpec(ObjectSpec &&spec) { /* First step : We need to find if this object is already specified in the savegame data. */ - ObjectType type = this->GetID(spec->grf_prop.local_id, spec->grf_prop.grfid); + ObjectType type = this->GetID(spec.grf_prop.local_id, spec.grf_prop.grfid); if (type == this->invalid_id) { /* Not found. * Or it has already been overridden, so you've lost your place. * Or it is a simple substitute. * We need to find a free available slot */ - type = this->AddEntityID(spec->grf_prop.local_id, spec->grf_prop.grfid, OBJECT_TRANSMITTER); + type = this->AddEntityID(spec.grf_prop.local_id, spec.grf_prop.grfid, OBJECT_TRANSMITTER); } if (type == this->invalid_id) { @@ -322,7 +322,7 @@ void ObjectOverrideManager::SetEntitySpec(ObjectSpec *spec) /* Now that we know we can use the given id, copy the spec to its final destination. */ if (type >= _object_specs.size()) _object_specs.resize(type + 1); - _object_specs[type] = *spec; + _object_specs[type] = std::move(spec); } /** diff --git a/src/newgrf_commons.h b/src/newgrf_commons.h index 37ecc134e7..d553bd6e49 100644 --- a/src/newgrf_commons.h +++ b/src/newgrf_commons.h @@ -210,7 +210,7 @@ public: HouseOverrideManager(uint16_t offset, uint16_t maximum, uint16_t invalid) : OverrideManagerBase(offset, maximum, invalid) {} - void SetEntitySpec(const HouseSpec *hs); + void SetEntitySpec(HouseSpec &&hs); }; @@ -223,7 +223,7 @@ public: uint16_t AddEntityID(uint16_t grf_local_id, uint32_t grfid, uint16_t substitute_id) override; uint16_t GetID(uint16_t grf_local_id, uint32_t grfid) const override; - void SetEntitySpec(IndustrySpec *inds); + void SetEntitySpec(IndustrySpec &&inds); }; @@ -235,7 +235,7 @@ public: IndustryTileOverrideManager(uint16_t offset, uint16_t maximum, uint16_t invalid) : OverrideManagerBase(offset, maximum, invalid) {} - void SetEntitySpec(const IndustryTileSpec *indts); + void SetEntitySpec(IndustryTileSpec &&indts); }; struct AirportSpec; @@ -244,7 +244,7 @@ public: AirportOverrideManager(uint16_t offset, uint16_t maximum, uint16_t invalid) : OverrideManagerBase(offset, maximum, invalid) {} - void SetEntitySpec(AirportSpec *inds); + void SetEntitySpec(AirportSpec &&inds); }; struct AirportTileSpec; @@ -255,7 +255,7 @@ public: AirportTileOverrideManager(uint16_t offset, uint16_t maximum, uint16_t invalid) : OverrideManagerBase(offset, maximum, invalid) {} - void SetEntitySpec(const AirportTileSpec *ats); + void SetEntitySpec(AirportTileSpec &&ats); }; struct ObjectSpec; @@ -266,7 +266,7 @@ public: ObjectOverrideManager(uint16_t offset, uint16_t maximum, uint16_t invalid) : OverrideManagerBase(offset, maximum, invalid) {} - void SetEntitySpec(ObjectSpec *spec); + void SetEntitySpec(ObjectSpec &&spec); }; extern HouseOverrideManager _house_mngr;