Change: Make NewGRF persistent storage variably sized.

Replace fixed array of 256 elements with a vector.

This potentially reduces the amount of memory (and savegame space) required as usually the full 256 entries are not used.

src/newgrf_debug_gui.cpp

@@ -186,7 +186,7 @@ public:
 	 * @param grfid Parameter for the PSA. Only required for items with parameters.
 	 * @return Span of the storage array or an empty span when not present.
 	 */
-	virtual const std::span<int32_t> GetPSA([[maybe_unused]] uint index, [[maybe_unused]] uint32_t grfid) const
+	virtual std::span<const int32_t> GetPSA([[maybe_unused]] uint index, [[maybe_unused]] uint32_t grfid) const
 	{
 		return {};
 	}
@@ -468,9 +468,11 @@ struct NewGRFInspectWindow : Window {
 			} else {
 				this->DrawString(r, i++, "Persistent storage:");
 			}
-			assert(psa.size() % 4 == 0);
+			for (uint index = 0; const int32_t &value : psa) {
-			for (size_t j = 0; j < psa.size(); j += 4) {
+				if (value != 0) {
-				this->DrawString(r, i++, fmt::format("  {}: {} {} {} {}", j, psa[j], psa[j + 1], psa[j + 2], psa[j + 3]));
+					this->DrawString(r, i++, fmt::format("  {:02x}: {:08x} ({})", index, value, value));
+				}
+				++index;
 			}
 		}
 

src/newgrf_storage.cpp

@@ -39,11 +39,47 @@ BasePersistentStorageArray::~BasePersistentStorageArray()
  * arrays, which saves quite a few clears, etc. after callbacks.
  * @param storage the array that has changed
  */
-void AddChangedPersistentStorage(BasePersistentStorageArray *storage)
+static void AddChangedPersistentStorage(BasePersistentStorageArray *storage)
 {
 	_changed_storage_arrays->insert(storage);
 }
 
+/**
+ * Stores some value at a given position.
+ * If there is no backup of the data that backup is made and then
+ * we write the data.
+ * @param pos   the position to write at
+ * @param value the value to write
+ */
+void PersistentStorageArray::StoreValue(uint pos, int32_t value)
+{
+	/* Out of the scope of the array */
+	if (pos >= PersistentStorageArray::SIZE) return;
+
+	if (pos >= std::size(this->storage)) {
+		if (value == 0) return;
+
+		this->storage.resize(pos + 1);
+	} else {
+		/* The value hasn't changed, so we pretend nothing happened.
+		 * Saves a few cycles and such and it's pretty easy to check. */
+		if (value == this->storage[pos]) return;
+	}
+
+	/* We do not have made a backup; lets do so */
+	if (AreChangesPersistent()) {
+		assert(!this->prev_storage);
+	} else if (!this->prev_storage) {
+		this->prev_storage = std::make_unique<StorageType>(this->storage);
+
+		/* We only need to register ourselves when we made the backup
+		 * as that is the only time something will have changed */
+		AddChangedPersistentStorage(this);
+	}
+
+	this->storage[pos] = value;
+}
+
 /**
  * Clear temporary changes made since the last call to SwitchMode, and
  * set whether subsequent changes shall be persistent or temporary.

src/newgrf_storage.h

@@ -60,59 +60,30 @@ private:
 /**
  * Class for persistent storage of data.
  * On #ClearChanges that data is either reverted or saved.
- * @tparam SIZE the size of the array.
  */
-template <uint SIZE>
 struct PersistentStorageArray : BasePersistentStorageArray {
-	using StorageType = std::array<int32_t, SIZE>;
+	using StorageType = std::vector<int32_t>;
+	static constexpr size_t SIZE = 256;
 
 	StorageType storage{}; ///< Memory for the storage array
 	std::unique_ptr<StorageType> prev_storage{}; ///< Temporary memory to store previous state so it can be reverted, e.g. for command tests.
 
-	/**
+	void StoreValue(uint pos, int32_t value);
-	 * Stores some value at a given position.
-	 * If there is no backup of the data that backup is made and then
-	 * we write the data.
-	 * @param pos   the position to write at
-	 * @param value the value to write
-	 */
-	void StoreValue(uint pos, int32_t value)
-	{
-		/* Out of the scope of the array */
-		if (pos >= SIZE) return;
-
-		/* The value hasn't changed, so we pretend nothing happened.
-		 * Saves a few cycles and such and it's pretty easy to check. */
-		if (this->storage[pos] == value) return;
-
-		/* We do not have made a backup; lets do so */
-		if (AreChangesPersistent()) {
-			assert(!this->prev_storage);
-		} else if (!this->prev_storage) {
-			this->prev_storage = std::make_unique<StorageType>(this->storage);
-
-			/* We only need to register ourselves when we made the backup
-			 * as that is the only time something will have changed */
-			AddChangedPersistentStorage(this);
-		}
-
-		this->storage[pos] = value;
-	}
 
 	/**
 	 * Gets the value from a given position.
 	 * @param pos the position to get the data from
 	 * @return the data from that position
 	 */
-	int32_t GetValue(uint pos) const
+	inline int32_t GetValue(uint pos) const
 	{
 		/* Out of the scope of the array */
-		if (pos >= SIZE) return 0;
+		if (pos >= std::size(this->storage)) return 0;
 
 		return this->storage[pos];
 	}
 
-	void ClearChanges() override
+	inline void ClearChanges() override
 	{
 		if (this->prev_storage) {
 			this->storage = *this->prev_storage;
@@ -180,10 +151,6 @@ struct TemporaryStorageArray {
 	}
 };
 
-void AddChangedPersistentStorage(BasePersistentStorageArray *storage);
-
-typedef PersistentStorageArray<16> OldPersistentStorage;
-
 typedef uint32_t PersistentStorageID;
 
 struct PersistentStorage;
@@ -194,7 +161,7 @@ extern PersistentStoragePool _persistent_storage_pool;
 /**
  * Class for pooled persistent storage of data.
  */
-struct PersistentStorage : PersistentStorageArray<256>, PersistentStoragePool::PoolItem<&_persistent_storage_pool> {
+struct PersistentStorage : PersistentStorageArray, PersistentStoragePool::PoolItem<&_persistent_storage_pool> {
 	/** We don't want GCC to zero our struct! It already is zeroed and has an index! */
 	PersistentStorage(const uint32_t new_grfid, uint8_t feature, TileIndex tile)
 	{
@@ -204,6 +171,7 @@ struct PersistentStorage : PersistentStorageArray<256>, PersistentStoragePool::P
 	}
 };
 
-static_assert(std::tuple_size_v<decltype(OldPersistentStorage::storage)> <= std::tuple_size_v<decltype(PersistentStorage::storage)>);
+/* storage_sl.cpp */
+PersistentStorage *ConvertOldPersistentStorage(std::span<const int32_t> old_storage);
 
 #endif /* NEWGRF_STORAGE_H */

src/saveload/afterload.cpp

@@ -2800,16 +2800,7 @@ bool AfterLoadGame()
 			for (Industry *ind : Industry::Iterate()) {
 				assert(ind->psa != nullptr);
 
-				/* Check if the old storage was empty. */
+				if (!ind->psa->storage.empty()) {
-				bool is_empty = true;
-				for (uint i = 0; i < sizeof(ind->psa->storage); i++) {
-					if (ind->psa->GetValue(i) != 0) {
-						is_empty = false;
-						break;
-					}
-				}
-
-				if (!is_empty) {
 					ind->psa->grfid = _industry_mngr.GetGRFID(ind->type);
 				} else {
 					delete ind->psa;
@@ -2823,21 +2814,11 @@ bool AfterLoadGame()
 				if (!(st->facilities & FACIL_AIRPORT)) continue;
 				assert(st->airport.psa != nullptr);
 
-				/* Check if the old storage was empty. */
+				if (!st->airport.psa->storage.empty()) {
-				bool is_empty = true;
-				for (uint i = 0; i < sizeof(st->airport.psa->storage); i++) {
-					if (st->airport.psa->GetValue(i) != 0) {
-						is_empty = false;
-						break;
-					}
-				}
-
-				if (!is_empty) {
 					st->airport.psa->grfid = _airport_mngr.GetGRFID(st->airport.type);
 				} else {
 					delete st->airport.psa;
 					st->airport.psa = nullptr;
-
 				}
 			}
 		}

src/saveload/industry_sl.cpp

@@ -17,7 +17,8 @@
 
 #include "../safeguards.h"
 
-static OldPersistentStorage _old_ind_persistent_storage;
+/** Old persistent storage for industries was a fixed array of 16 elements. */
+static std::array<int32_t, 16> _old_ind_persistent_storage;
 
 class SlIndustryAccepted : public VectorSaveLoadHandler<SlIndustryAccepted, Industry, Industry::AcceptedCargo, INDUSTRY_NUM_INPUTS> {
 public:
@@ -156,7 +157,7 @@ static const SaveLoad _industry_desc[] = {
 	SLE_CONDVAR(Industry, exclusive_supplier,         SLE_UINT8,                 SLV_GS_INDUSTRY_CONTROL, SL_MAX_VERSION),
 	SLE_CONDVAR(Industry, exclusive_consumer,         SLE_UINT8,                 SLV_GS_INDUSTRY_CONTROL, SL_MAX_VERSION),
 
-	SLEG_CONDARR("storage", _old_ind_persistent_storage.storage, SLE_UINT32, 16, SLV_76, SLV_161),
+	SLEG_CONDARR("storage", _old_ind_persistent_storage, SLE_FILE_U32 | SLE_VAR_I32, 16, SLV_76, SLV_161),
 	SLE_CONDREF(Industry, psa,                        REF_STORAGE,              SLV_161, SL_MAX_VERSION),
 
 	SLE_CONDVAR(Industry, random,                     SLE_UINT16,                SLV_82, SL_MAX_VERSION),
@@ -207,6 +208,7 @@ struct INDYChunkHandler : ChunkHandler {
 	{
 		const std::vector<SaveLoad> slt = SlCompatTableHeader(_industry_desc, _industry_sl_compat);
 
+		_old_ind_persistent_storage.fill(0);
 		int index;
 
 		SlIndustryAccepted::ResetOldStructure();
@@ -219,9 +221,7 @@ struct INDYChunkHandler : ChunkHandler {
 			/* Before savegame version 161, persistent storages were not stored in a pool. */
 			if (IsSavegameVersionBefore(SLV_161) && !IsSavegameVersionBefore(SLV_76)) {
 				/* Store the old persistent storage. The GRFID will be added later. */
-				assert(PersistentStorage::CanAllocateItem());
+				i->psa = ConvertOldPersistentStorage(_old_ind_persistent_storage);
-				i->psa = new PersistentStorage(0, 0, TileIndex{});
-				std::copy(std::begin(_old_ind_persistent_storage.storage), std::end(_old_ind_persistent_storage.storage), std::begin(i->psa->storage));
 			}
 			if (IsSavegameVersionBefore(SLV_EXTEND_INDUSTRY_CARGO_SLOTS)) {
 				LoadMoveAcceptsProduced(i, INDUSTRY_ORIGINAL_NUM_INPUTS, INDUSTRY_ORIGINAL_NUM_OUTPUTS);

src/saveload/saveload.h

@@ -395,6 +395,7 @@ enum SaveLoadVersion : uint16_t {
 	SLV_PATH_CACHE_FORMAT,                  ///< 346  PR#12345 Vehicle path cache format changed.
 	SLV_ANIMATED_TILE_STATE_IN_MAP,         ///< 347  PR#13082 Animated tile state saved for improved performance.
 	SLV_INCREASE_HOUSE_LIMIT,               ///< 348  PR#12288 Increase house limit to 4096.
+	SLV_VARIABLE_PERSISTENT_STORAGE,        ///< 349  PR#10670 NewGRF persistent storage moved from fixed array to dynamic.
 
 	SL_MAX_VERSION,                         ///< Highest possible saveload version
 };

src/saveload/station_sl.cpp

@@ -182,7 +182,8 @@ struct FlowSaveLoad {
 
 typedef std::pair<const StationID, std::list<CargoPacket *> > StationCargoPair;
 
-static OldPersistentStorage _old_st_persistent_storage;
+/** Old persistent storage for stations was a fixed array of 16 elements. */
+static std::array<int32_t, 16> _old_st_persistent_storage;
 
 /**
  * Swap the temporary packets with the packets without specific destination in
@@ -399,9 +400,7 @@ public:
 		/* Before savegame version 161, persistent storages were not stored in a pool. */
 		if (IsSavegameVersionBefore(SLV_161) && !IsSavegameVersionBefore(SLV_145) && st->facilities & FACIL_AIRPORT) {
 			/* Store the old persistent storage. The GRFID will be added later. */
-			assert(PersistentStorage::CanAllocateItem());
+			st->airport.psa = ConvertOldPersistentStorage(_old_st_persistent_storage);
-			st->airport.psa = new PersistentStorage(0, 0, TileIndex{});
-			std::copy(std::begin(_old_st_persistent_storage.storage), std::end(_old_st_persistent_storage.storage), std::begin(st->airport.psa->storage));
 		}
 
 		auto end = std::next(std::begin(st->goods), std::min(this->GetNumCargo(), std::size(st->goods)));
@@ -610,7 +609,7 @@ public:
 		SLE_CONDVAR(Station, airport.layout,             SLE_UINT8,                 SLV_145, SL_MAX_VERSION),
 		    SLE_VAR(Station, airport.flags,              SLE_UINT64),
 		SLE_CONDVAR(Station, airport.rotation,           SLE_UINT8,                 SLV_145, SL_MAX_VERSION),
-		SLEG_CONDARR("storage", _old_st_persistent_storage.storage,  SLE_UINT32, 16, SLV_145, SLV_161),
+		SLEG_CONDARR("storage", _old_st_persistent_storage,  SLE_FILE_U32 | SLE_VAR_I32, 16, SLV_145, SLV_161),
 		SLE_CONDREF(Station, airport.psa,                REF_STORAGE,               SLV_161, SL_MAX_VERSION),
 
 		    SLE_VAR(Station, indtype,                    SLE_UINT8),
@@ -709,6 +708,7 @@ struct STNNChunkHandler : ChunkHandler {
 	{
 		const std::vector<SaveLoad> slt = SlCompatTableHeader(_station_desc, _station_sl_compat);
 
+		_old_st_persistent_storage.fill(0);
 		_old_num_flows = 0;
 
 		int index;

src/saveload/storage_sl.cpp

@@ -16,11 +16,51 @@
 
 #include "../safeguards.h"
 
+/**
+ * Convert old fixed-sized array of persistent storage.
+ * @param old_storage Span containing range of old persistent storage.
+ * @returns Pointer to pool-allocated PersistentStorage object.
+ */
+PersistentStorage *ConvertOldPersistentStorage(std::span<const int32_t> old_storage)
+{
+	/* Find last non-zero value to truncate the storage. */
+	auto last = std::find_if(std::rbegin(old_storage), std::rend(old_storage), [](uint32_t value) { return value != 0; }).base();
+	if (last == std::begin(old_storage)) return nullptr;
+
+	assert(PersistentStorage::CanAllocateItem());
+	PersistentStorage *ps = new PersistentStorage(0, 0, TileIndex{});
+
+	ps->storage.reserve(std::distance(std::begin(old_storage), last));
+	for (auto it = std::begin(old_storage); it != last; ++it) {
+		ps->storage.push_back(*it);
+	}
+
+	return ps;
+}
+
+class SlPersistentStorage : public VectorSaveLoadHandler<SlPersistentStorage, PersistentStorage, int32_t> {
+public:
+	struct PersistentStorageWrapper {
+		int32_t value;
+	};
+
+	inline static const SaveLoad description[] = {
+		SLE_VAR(PersistentStorageWrapper, value, SLE_INT32),
+	};
+	inline const static SaveLoadCompatTable compat_description = {};
+
+	std::vector<int32_t> &GetVector(PersistentStorage *ps) const override { return ps->storage; }
+};
+
+/** Old persistent storage was a fixed array of up to 256 elements. */
+static std::array<int32_t, 256> _old_persistent_storage;
+
 /** Description of the data to save and load in #PersistentStorage. */
 static const SaveLoad _storage_desc[] = {
 	 SLE_CONDVAR(PersistentStorage, grfid,    SLE_UINT32,                  SLV_6, SL_MAX_VERSION),
-	 SLE_CONDARR(PersistentStorage, storage,  SLE_UINT32,  16,           SLV_161, SLV_EXTEND_PERSISTENT_STORAGE),
+	 SLEG_CONDARR("storage", _old_persistent_storage, SLE_FILE_U32 | SLE_VAR_I32,  16, SLV_161, SLV_EXTEND_PERSISTENT_STORAGE),
-	 SLE_CONDARR(PersistentStorage, storage,  SLE_UINT32, 256,           SLV_EXTEND_PERSISTENT_STORAGE, SL_MAX_VERSION),
+	 SLEG_CONDARR("storage", _old_persistent_storage, SLE_FILE_U32 | SLE_VAR_I32, 256, SLV_EXTEND_PERSISTENT_STORAGE, SLV_VARIABLE_PERSISTENT_STORAGE),
+	SLEG_CONDSTRUCTLIST("storage", SlPersistentStorage, SLV_VARIABLE_PERSISTENT_STORAGE, SL_MAX_VERSION),
 };
 
 /** Persistent storage data. */
@@ -31,12 +71,21 @@ struct PSACChunkHandler : ChunkHandler {
 	{
 		const std::vector<SaveLoad> slt = SlCompatTableHeader(_storage_desc, _storage_sl_compat);
 
+		_old_persistent_storage.fill(0);
 		int index;
 
 		while ((index = SlIterateArray()) != -1) {
 			assert(PersistentStorage::CanAllocateItem());
 			PersistentStorage *ps = new (index) PersistentStorage(0, 0, TileIndex{});
 			SlObject(ps, slt);
+
+			if (IsSavegameVersionBefore(SLV_VARIABLE_PERSISTENT_STORAGE)) {
+				auto last = std::find_if(std::rbegin(_old_persistent_storage), std::rend(_old_persistent_storage), [](uint32_t value) { return value != 0; }).base();
+				ps->storage.reserve(std::distance(std::begin(_old_persistent_storage), last));
+				for (auto it = std::begin(_old_persistent_storage); it != last; ++it) {
+					ps->storage.push_back(*it);
+				}
+			}
 		}
 	}
 

src/table/newgrf_debug_data.h

@@ -381,7 +381,7 @@ class NIHIndustry : public NIHelper {
 		return ro.GetScope(VSG_SCOPE_SELF)->GetVariable(var, param, avail);
 	}
 
-	const std::span<int32_t> GetPSA(uint index, uint32_t) const override
+	std::span<const int32_t> GetPSA(uint index, uint32_t) const override
 	{
 		const Industry *i = (const Industry *)this->GetInstance(index);
 		if (i->psa == nullptr) return {};
@@ -557,7 +557,7 @@ class NIHAirport : public NIHelper {
 		return ro.GetScope(VSG_SCOPE_SELF)->GetVariable(var, param, avail);
 	}
 
-	const std::span<int32_t> GetPSA(uint index, uint32_t) const override
+	std::span<const int32_t> GetPSA(uint index, uint32_t) const override
 	{
 		const Station *st = (const Station *)this->GetInstance(index);
 		if (st->airport.psa == nullptr) return {};
@@ -603,7 +603,7 @@ class NIHTown : public NIHelper {
 		return ro.GetScope(VSG_SCOPE_SELF)->GetVariable(var, param, avail);
 	}
 
-	const std::span<int32_t> GetPSA(uint index, uint32_t grfid) const override
+	std::span<const int32_t> GetPSA(uint index, uint32_t grfid) const override
 	{
 		Town *t = Town::Get(index);