Change: prefix SL_ARR with the length of the array

This means that during loading we can validate that what is saved
is also that what is expected. Additionally, this makes all list
types similar to how they are stored on disk:
First a gamma to indicate length, followed by the data.
The size still depends on the type.

src/saveload/animated_tile_sl.cpp

@@ -24,7 +24,7 @@ extern std::vector<TileIndex> _animated_tiles;
 static void Save_ANIT()
 {
 	SlSetLength(_animated_tiles.size() * sizeof(_animated_tiles.front()));
-	SlArray(_animated_tiles.data(), _animated_tiles.size(), SLE_UINT32);
+	SlCopy(_animated_tiles.data(), _animated_tiles.size(), SLE_UINT32);
 }
 
 /**
@@ -36,7 +36,7 @@ static void Load_ANIT()
 	if (IsSavegameVersionBefore(SLV_80)) {
 		/* In pre version 6, we has 16bit per tile, now we have 32bit per tile, convert it ;) */
 		TileIndex anim_list[256];
-		SlArray(anim_list, 256, IsSavegameVersionBefore(SLV_6) ? (SLE_FILE_U16 | SLE_VAR_U32) : SLE_UINT32);
+		SlCopy(anim_list, 256, IsSavegameVersionBefore(SLV_6) ? (SLE_FILE_U16 | SLE_VAR_U32) : SLE_UINT32);
 
 		for (int i = 0; i < 256; i++) {
 			if (anim_list[i] == 0) break;
@@ -48,7 +48,7 @@ static void Load_ANIT()
 	uint count = (uint)SlGetFieldLength() / sizeof(_animated_tiles.front());
 	_animated_tiles.clear();
 	_animated_tiles.resize(_animated_tiles.size() + count);
-	SlArray(_animated_tiles.data(), count, SLE_UINT32);
+	SlCopy(_animated_tiles.data(), count, SLE_UINT32);
 }
 
 /**

src/saveload/economy_sl.cpp

@@ -20,8 +20,8 @@ static void Load_PRIC()
 {
 	/* Old games store 49 base prices, very old games store them as int32 */
 	int vt = IsSavegameVersionBefore(SLV_65) ? SLE_FILE_I32 : SLE_FILE_I64;
-	SlArray(nullptr, 49, vt | SLE_VAR_NULL);
+	SlCopy(nullptr, 49, vt | SLE_VAR_NULL);
-	SlArray(nullptr, 49, SLE_FILE_U16 | SLE_VAR_NULL);
+	SlCopy(nullptr, 49, SLE_FILE_U16 | SLE_VAR_NULL);
 }
 
 /** Cargo payment rates in pre 126 savegames */
@@ -29,8 +29,8 @@ static void Load_CAPR()
 {
 	uint num_cargo = IsSavegameVersionBefore(SLV_55) ? 12 : IsSavegameVersionBefore(SLV_EXTEND_CARGOTYPES) ? 32 : NUM_CARGO;
 	int vt = IsSavegameVersionBefore(SLV_65) ? SLE_FILE_I32 : SLE_FILE_I64;
-	SlArray(nullptr, num_cargo, vt | SLE_VAR_NULL);
+	SlCopy(nullptr, num_cargo, vt | SLE_VAR_NULL);
-	SlArray(nullptr, num_cargo, SLE_FILE_U16 | SLE_VAR_NULL);
+	SlCopy(nullptr, num_cargo, SLE_FILE_U16 | SLE_VAR_NULL);
 }
 
 static const SaveLoad _economy_desc[] = {

src/saveload/engine_sl.cpp

@@ -156,7 +156,7 @@ static void Load_ENGS()
 	 * was always 256 entries. */
 	StringID names[256];
 
-	SlArray(names, lengthof(names), SLE_STRINGID);
+	SlCopy(names, lengthof(names), SLE_STRINGID);
 
 	/* Copy each string into the temporary engine array. */
 	for (EngineID engine = 0; engine < lengthof(names); engine++) {

src/saveload/map_sl.cpp

@@ -53,7 +53,7 @@ static void Load_MAPT()
 	TileIndex size = MapSize();
 
 	for (TileIndex i = 0; i != size;) {
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) _m[i++].type = buf[j];
 	}
 }
@@ -66,7 +66,7 @@ static void Save_MAPT()
 	SlSetLength(size);
 	for (TileIndex i = 0; i != size;) {
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) buf[j] = _m[i++].type;
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 	}
 }
 
@@ -76,7 +76,7 @@ static void Load_MAPH()
 	TileIndex size = MapSize();
 
 	for (TileIndex i = 0; i != size;) {
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) _m[i++].height = buf[j];
 	}
 }
@@ -89,7 +89,7 @@ static void Save_MAPH()
 	SlSetLength(size);
 	for (TileIndex i = 0; i != size;) {
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) buf[j] = _m[i++].height;
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 	}
 }
 
@@ -99,7 +99,7 @@ static void Load_MAP1()
 	TileIndex size = MapSize();
 
 	for (TileIndex i = 0; i != size;) {
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) _m[i++].m1 = buf[j];
 	}
 }
@@ -112,7 +112,7 @@ static void Save_MAP1()
 	SlSetLength(size);
 	for (TileIndex i = 0; i != size;) {
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) buf[j] = _m[i++].m1;
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 	}
 }
 
@@ -122,7 +122,7 @@ static void Load_MAP2()
 	TileIndex size = MapSize();
 
 	for (TileIndex i = 0; i != size;) {
-		SlArray(buf.data(), MAP_SL_BUF_SIZE,
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE,
 			/* In those versions the m2 was 8 bits */
 			IsSavegameVersionBefore(SLV_5) ? SLE_FILE_U8 | SLE_VAR_U16 : SLE_UINT16
 		);
@@ -138,7 +138,7 @@ static void Save_MAP2()
 	SlSetLength(size * sizeof(uint16));
 	for (TileIndex i = 0; i != size;) {
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) buf[j] = _m[i++].m2;
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT16);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT16);
 	}
 }
 
@@ -148,7 +148,7 @@ static void Load_MAP3()
 	TileIndex size = MapSize();
 
 	for (TileIndex i = 0; i != size;) {
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) _m[i++].m3 = buf[j];
 	}
 }
@@ -161,7 +161,7 @@ static void Save_MAP3()
 	SlSetLength(size);
 	for (TileIndex i = 0; i != size;) {
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) buf[j] = _m[i++].m3;
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 	}
 }
 
@@ -171,7 +171,7 @@ static void Load_MAP4()
 	TileIndex size = MapSize();
 
 	for (TileIndex i = 0; i != size;) {
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) _m[i++].m4 = buf[j];
 	}
 }
@@ -184,7 +184,7 @@ static void Save_MAP4()
 	SlSetLength(size);
 	for (TileIndex i = 0; i != size;) {
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) buf[j] = _m[i++].m4;
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 	}
 }
 
@@ -194,7 +194,7 @@ static void Load_MAP5()
 	TileIndex size = MapSize();
 
 	for (TileIndex i = 0; i != size;) {
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) _m[i++].m5 = buf[j];
 	}
 }
@@ -207,7 +207,7 @@ static void Save_MAP5()
 	SlSetLength(size);
 	for (TileIndex i = 0; i != size;) {
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) buf[j] = _m[i++].m5;
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 	}
 }
 
@@ -219,7 +219,7 @@ static void Load_MAP6()
 	if (IsSavegameVersionBefore(SLV_42)) {
 		for (TileIndex i = 0; i != size;) {
 			/* 1024, otherwise we overflow on 64x64 maps! */
-			SlArray(buf.data(), 1024, SLE_UINT8);
+			SlCopy(buf.data(), 1024, SLE_UINT8);
 			for (uint j = 0; j != 1024; j++) {
 				_me[i++].m6 = GB(buf[j], 0, 2);
 				_me[i++].m6 = GB(buf[j], 2, 2);
@@ -229,7 +229,7 @@ static void Load_MAP6()
 		}
 	} else {
 		for (TileIndex i = 0; i != size;) {
-			SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+			SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 			for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) _me[i++].m6 = buf[j];
 		}
 	}
@@ -243,7 +243,7 @@ static void Save_MAP6()
 	SlSetLength(size);
 	for (TileIndex i = 0; i != size;) {
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) buf[j] = _me[i++].m6;
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 	}
 }
 
@@ -253,7 +253,7 @@ static void Load_MAP7()
 	TileIndex size = MapSize();
 
 	for (TileIndex i = 0; i != size;) {
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) _me[i++].m7 = buf[j];
 	}
 }
@@ -266,7 +266,7 @@ static void Save_MAP7()
 	SlSetLength(size);
 	for (TileIndex i = 0; i != size;) {
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) buf[j] = _me[i++].m7;
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT8);
 	}
 }
 
@@ -276,7 +276,7 @@ static void Load_MAP8()
 	TileIndex size = MapSize();
 
 	for (TileIndex i = 0; i != size;) {
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT16);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT16);
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) _me[i++].m8 = buf[j];
 	}
 }
@@ -289,7 +289,7 @@ static void Save_MAP8()
 	SlSetLength(size * sizeof(uint16));
 	for (TileIndex i = 0; i != size;) {
 		for (uint j = 0; j != MAP_SL_BUF_SIZE; j++) buf[j] = _me[i++].m8;
-		SlArray(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT16);
+		SlCopy(buf.data(), MAP_SL_BUF_SIZE, SLE_UINT16);
 	}
 }
 

src/saveload/order_sl.cpp

@@ -141,7 +141,7 @@ static void Load_ORDR()
 			len /= sizeof(uint16);
 			uint16 *orders = MallocT<uint16>(len + 1);
 
-			SlArray(orders, len, SLE_UINT16);
+			SlCopy(orders, len, SLE_UINT16);
 
 			for (size_t i = 0; i < len; ++i) {
 				Order *o = new (i) Order();
@@ -153,7 +153,7 @@ static void Load_ORDR()
 			len /= sizeof(uint32);
 			uint32 *orders = MallocT<uint32>(len + 1);
 
-			SlArray(orders, len, SLE_UINT32);
+			SlCopy(orders, len, SLE_UINT32);
 
 			for (size_t i = 0; i < len; ++i) {
 				new (i) Order(orders[i]);

src/saveload/saveload.cpp

@@ -1043,51 +1043,34 @@ static void SlStdString(void *ptr, VarType conv)
 }
 
 /**
- * Return the size in bytes of a certain type of atomic array
+ * Internal function to save/Load a list of SL_VARs.
- * @param length The length of the array counted in elements
+ * SlCopy() and SlArray() are very similar, with the exception of the header.
- * @param conv VarType type of the variable that is used in calculating the size
+ * This function represents the common part.
+ * @param object The object being manipulated.
+ * @param length The length of the object in elements
+ * @param conv VarType type of the items.
  */
-static inline size_t SlCalcArrayLen(size_t length, VarType conv)
+static void SlCopyInternal(void *object, size_t length, VarType conv)
 {
-	return SlCalcConvFileLen(conv) * length;
-}
-
-/**
- * Save/Load an array.
- * @param array The array being manipulated
- * @param length The length of the array in elements
- * @param conv VarType type of the atomic array (int, byte, uint64, etc.)
- */
-void SlArray(void *array, size_t length, VarType conv)
-{
-	if (_sl.action == SLA_PTRS || _sl.action == SLA_NULL) return;
-
-	/* Automatically calculate the length? */
-	if (_sl.need_length != NL_NONE) {
-		SlSetLength(SlCalcArrayLen(length, conv));
-		/* Determine length only? */
-		if (_sl.need_length == NL_CALCLENGTH) return;
-	}
-
 	if (GetVarMemType(conv) == SLE_VAR_NULL) {
 		assert(_sl.action != SLA_SAVE); // Use SL_NULL if you want to write null-bytes
-		SlSkipBytes(SlCalcArrayLen(length, conv));
+		SlSkipBytes(length * SlCalcConvFileLen(conv));
 		return;
 	}
 
 	/* NOTICE - handle some buggy stuff, in really old versions everything was saved
-	 * as a byte-type. So detect this, and adjust array size accordingly */
+	 * as a byte-type. So detect this, and adjust object size accordingly */
 	if (_sl.action != SLA_SAVE && _sl_version == 0) {
-		/* all arrays except difficulty settings */
+		/* all objects except difficulty settings */
 		if (conv == SLE_INT16 || conv == SLE_UINT16 || conv == SLE_STRINGID ||
 				conv == SLE_INT32 || conv == SLE_UINT32) {
-			SlCopyBytes(array, length * SlCalcConvFileLen(conv));
+			SlCopyBytes(object, length * SlCalcConvFileLen(conv));
 			return;
 		}
 		/* used for conversion of Money 32bit->64bit */
 		if (conv == (SLE_FILE_I32 | SLE_VAR_I64)) {
 			for (uint i = 0; i < length; i++) {
-				((int64*)array)[i] = (int32)BSWAP32(SlReadUint32());
+				((int64*)object)[i] = (int32)BSWAP32(SlReadUint32());
 			}
 			return;
 		}
@@ -1096,9 +1079,9 @@ void SlArray(void *array, size_t length, VarType conv)
 	/* If the size of elements is 1 byte both in file and memory, no special
 	 * conversion is needed, use specialized copy-copy function to speed up things */
 	if (conv == SLE_INT8 || conv == SLE_UINT8) {
-		SlCopyBytes(array, length);
+		SlCopyBytes(object, length);
 	} else {
-		byte *a = (byte*)array;
+		byte *a = (byte*)object;
 		byte mem_size = SlCalcConvMemLen(conv);
 
 		for (; length != 0; length --) {
@@ -1108,6 +1091,71 @@ void SlArray(void *array, size_t length, VarType conv)
 	}
 }
 
+/**
+ * Copy a list of SL_VARs to/from a savegame.
+ * These entries are copied as-is, and you as caller have to make sure things
+ * like length-fields are calculated correctly.
+ * @param object The object being manipulated.
+ * @param length The length of the object in elements
+ * @param conv VarType type of the items.
+ */
+void SlCopy(void *object, size_t length, VarType conv)
+{
+	if (_sl.action == SLA_PTRS || _sl.action == SLA_NULL) return;
+
+	/* Automatically calculate the length? */
+	if (_sl.need_length != NL_NONE) {
+		SlSetLength(length * SlCalcConvFileLen(conv));
+		/* Determine length only? */
+		if (_sl.need_length == NL_CALCLENGTH) return;
+	}
+
+	SlCopyInternal(object, length, conv);
+}
+
+/**
+ * Return the size in bytes of a certain type of atomic array
+ * @param length The length of the array counted in elements
+ * @param conv VarType type of the variable that is used in calculating the size
+ */
+static inline size_t SlCalcArrayLen(size_t length, VarType conv)
+{
+	return SlCalcConvFileLen(conv) * length + SlGetArrayLength(length);
+}
+
+/**
+ * Save/Load the length of the array followed by the array of SL_VAR elements.
+ * @param array The array being manipulated
+ * @param length The length of the array in elements
+ * @param conv VarType type of the atomic array (int, byte, uint64, etc.)
+ */
+static void SlArray(void *array, size_t length, VarType conv)
+{
+	switch (_sl.action) {
+		case SLA_SAVE:
+			SlWriteArrayLength(length);
+			SlCopyInternal(array, length, conv);
+			return;
+
+		case SLA_LOAD_CHECK:
+		case SLA_LOAD: {
+			if (!IsSavegameVersionBefore(SLV_SAVELOAD_LIST_LENGTH)) {
+				size_t sv_length = SlReadArrayLength();
+				if (sv_length != length) SlErrorCorrupt("Fixed-length array is of wrong length");
+			}
+
+			SlCopyInternal(array, length, conv);
+			return;
+		}
+
+		case SLA_PTRS:
+		case SLA_NULL:
+			return;
+
+		default:
+			NOT_REACHED();
+	}
+}
 
 /**
  * Pointers cannot be saved to a savegame, so this functions gets

src/saveload/saveload.h

@@ -1008,7 +1008,7 @@ byte SlReadByte();
 void SlWriteByte(byte b);
 
 void SlGlobList(const SaveLoadTable &slt);
-void SlArray(void *array, size_t length, VarType conv);
+void SlCopy(void *object, size_t length, VarType conv);
 void SlObject(void *object, const SaveLoadTable &slt);
 void NORETURN SlError(StringID string, const char *extra_msg = nullptr);
 void NORETURN SlErrorCorrupt(const char *msg);

src/saveload/strings_sl.cpp

@@ -124,7 +124,7 @@ static void Load_NAME()
 		if (index >= NUM_OLD_STRINGS) SlErrorCorrupt("Invalid old name index");
 		if (SlGetFieldLength() > (uint)LEN_OLD_STRINGS) SlErrorCorrupt("Invalid old name length");
 
-		SlArray(&_old_name_array[LEN_OLD_STRINGS * index], SlGetFieldLength(), SLE_UINT8);
+		SlCopy(&_old_name_array[LEN_OLD_STRINGS * index], SlGetFieldLength(), SLE_UINT8);
 		/* Make sure the old name is null terminated */
 		_old_name_array[LEN_OLD_STRINGS * index + LEN_OLD_STRINGS - 1] = '\0';
 	}

src/script/script_instance.cpp

@@ -365,7 +365,7 @@ static const SaveLoad _script_byte[] = {
 			sq_getinteger(vm, index, &res);
 			if (!test) {
 				int value = (int)res;
-				SlArray(&value, 1, SLE_INT32);
+				SlCopy(&value, 1, SLE_INT32);
 			}
 			return true;
 		}
@@ -385,7 +385,7 @@ static const SaveLoad _script_byte[] = {
 			if (!test) {
 				_script_sl_byte = (byte)len;
 				SlObject(nullptr, _script_byte);
-				SlArray(const_cast<char *>(buf), len, SLE_CHAR);
+				SlCopy(const_cast<char *>(buf), len, SLE_CHAR);
 			}
 			return true;
 		}
@@ -565,7 +565,7 @@ bool ScriptInstance::IsPaused()
 	switch (_script_sl_byte) {
 		case SQSL_INT: {
 			int value;
-			SlArray(&value, 1, SLE_INT32);
+			SlCopy(&value, 1, SLE_INT32);
 			if (vm != nullptr) sq_pushinteger(vm, (SQInteger)value);
 			return true;
 		}
@@ -573,7 +573,7 @@ bool ScriptInstance::IsPaused()
 		case SQSL_STRING: {
 			SlObject(nullptr, _script_byte);
 			static char buf[std::numeric_limits<decltype(_script_sl_byte)>::max()];
-			SlArray(buf, _script_sl_byte, SLE_CHAR);
+			SlCopy(buf, _script_sl_byte, SLE_CHAR);
 			StrMakeValidInPlace(buf, buf + _script_sl_byte);
 			if (vm != nullptr) sq_pushstring(vm, buf, -1);
 			return true;

src/table/settings/gameopt_settings.ini

@@ -41,7 +41,7 @@ static const SettingTable _gameopt_settings{
  * XXX - To save file-space and since values are never bigger than about 10? only
  * save the first 16 bits in the savegame. Question is why the values are still int32
  * and why not byte for example?
- * 'SLE_FILE_I16 | SLE_VAR_U16' in "diff_custom" is needed to get around SlArray() hack
+ * 'SLE_FILE_I16 | SLE_VAR_U16' in "diff_custom" is needed to get around SlCopy() hack
  * for savegames version 0 - though it is an array, it has to go through the byteswap process */
 [post-amble]
 };