Saveload: Fix NGRF chunks being saved with incorrect table SL_ARR format

Array length prefix was missing

See also: OpenTTD/bananas-api#456

src/sl/extended_ver_sl.cpp

@@ -216,7 +216,7 @@ const SlxiSubChunkInfo _sl_xv_sub_chunk_infos[] = {
 	{ XSLFI_TABLE_PATS,                       XSCF_NULL,                1,   1, "table_pats",                       nullptr, nullptr, nullptr          },
 	{ XSLFI_TABLE_MISC_SL,                    XSCF_NULL,                2,   2, "table_misc_sl",                    nullptr, nullptr, nullptr          },
 	{ XSLFI_TABLE_SCRIPT_SL,                  XSCF_NULL,                1,   1, "table_script_sl",                  nullptr, nullptr, nullptr          },
-	{ XSLFI_TABLE_NEWGRF_SL,                  XSCF_NULL,                1,   1, "table_newgrf_sl",                  nullptr, nullptr, nullptr          },
+	{ XSLFI_TABLE_NEWGRF_SL,                  XSCF_NULL,                2,   2, "table_newgrf_sl",                  nullptr, nullptr, nullptr          },
 	{ XSLFI_TABLE_INDUSTRY_SL,                XSCF_NULL,                1,   1, "table_industry_sl",                nullptr, nullptr, nullptr          },
 
 	{ XSLFI_NULL, XSCF_NULL, 0, 0, nullptr, nullptr, nullptr, nullptr }, // This is the end marker

src/sl/extended_ver_sl.h

@@ -168,6 +168,7 @@ enum SlXvFeatureIndex {
 	                                              ///<     v2: SUBS, CMDL, CMPU, ERNW, DEPT, CAPY, ECMY, EIDS, ENGN, GOAL, GRPS, RAIL, OBJS, SIGN, PSAC, STPE, STPA
 	XSLFI_TABLE_SCRIPT_SL,                        ///< Use upstream table format for script chunks
 	XSLFI_TABLE_NEWGRF_SL,                        ///< Use upstream table format for NewGRF/ID mapping chunks
+	                                              ///<     In v1, NGRF chunks were saved incorrectly: see SLBF_TABLE_ARRAY_LENGTH_PREFIX_MISSING
 	XSLFI_TABLE_INDUSTRY_SL,                      ///< Use upstream table format for industry chunks: IBLD, ITBL
 
 	XSLFI_RIFF_HEADER_60_BIT,                     ///< Size field in RIFF chunk header is 60 bit

src/sl/newgrf_sl.cpp

@@ -104,6 +104,9 @@ static void Save_NGRF()
 
 static void Load_NGRF_common(GRFConfig *&grfconfig)
 {
+	if (SlXvIsFeaturePresent(XSLFI_TABLE_NEWGRF_SL, 1, 1)) {
+		SlLoadTableWithArrayLengthPrefixesMissing();
+	}
 	std::vector<SaveLoad> sld = SlTableHeaderOrRiff(_grfconfig_desc);
 	ClearGRFConfigList(&grfconfig);
 	while (SlIterateArray() != -1) {

src/sl/saveload.cpp

@@ -230,11 +230,16 @@ size_t MemoryDumper::GetSize() const
 	return this->completed_block_bytes + (this->bufe ? (MEMORY_CHUNK_SIZE - (this->bufe - this->buf)) : 0);
 }
 
+enum SaveLoadBlockFlags {
+	SLBF_TABLE_ARRAY_LENGTH_PREFIX_MISSING, ///< Table chunk arrays were incorrectly saved without the length prefix, skip reading the length prefix on load
+};
+
 /** The saveload struct, containing reader-writer functions, buffer, version, etc. */
 struct SaveLoadParams {
 	SaveLoadAction action;               ///< are we doing a save or a load atm.
 	NeedLength need_length;              ///< working in NeedLength (Autolength) mode?
 	byte block_mode;                     ///< ???
+	uint8_t block_flags;                 ///< block flags: SaveLoadBlockFlags
 	bool error;                          ///< did an error occur or not
 
 	size_t obj_len;                      ///< the length of the current object we are busy with
@@ -1308,6 +1313,36 @@ void SlArray(void *array, size_t length, VarType conv)
 {
 	if (_sl.action == SLA_PTRS || _sl.action == SLA_NULL) return;
 
+	if (SlIsTableChunk()) {
+		assert(_sl.need_length == NL_NONE);
+
+		switch (_sl.action) {
+			case SLA_SAVE:
+				SlWriteArrayLength(length);
+				break;
+
+			case SLA_LOAD_CHECK:
+			case SLA_LOAD: {
+				if (!HasBit(_sl.block_flags, SLBF_TABLE_ARRAY_LENGTH_PREFIX_MISSING)) {
+					size_t sv_length = SlReadArrayLength();
+					if (GetVarMemType(conv) == SLE_VAR_NULL) {
+						/* We don't know this field, so we assume the length in the savegame is correct. */
+						length = sv_length;
+					} else if (sv_length != length) {
+						/* If the SLE_ARR changes size, a savegame bump is required
+						 * and the developer should have written conversion lines.
+						 * Error out to make this more visible. */
+						SlErrorCorrupt("Fixed-length array is of wrong length");
+					}
+				}
+				break;
+			}
+
+			default:
+				break;
+		}
+	}
+
 	/* Automatically calculate the length? */
 	if (_sl.need_length != NL_NONE) {
 		SlSetLength(SlCalcArrayLen(length, conv));
@@ -2286,6 +2321,11 @@ void SlLoadTableOrRiffFiltered(const SaveLoadTable &slt)
 	}
 }
 
+void SlLoadTableWithArrayLengthPrefixesMissing()
+{
+	SetBit(_sl.block_flags, SLBF_TABLE_ARRAY_LENGTH_PREFIX_MISSING);
+}
+
 /**
  * Save or Load (a list of) global variables.
  * @param slt The SaveLoad table with objects to save/load.
@@ -2428,6 +2468,7 @@ static void SlLoadChunk(const ChunkHandler &ch)
 	size_t endoffs;
 
 	_sl.block_mode = m;
+	_sl.block_flags = 0;
 	_sl.obj_len = 0;
 
 	SaveLoadChunkExtHeaderFlags ext_flags = static_cast<SaveLoadChunkExtHeaderFlags>(0);
@@ -2514,6 +2555,7 @@ static void SlLoadCheckChunk(const ChunkHandler *ch, uint32_t chunk_id)
 	size_t endoffs;
 
 	_sl.block_mode = m;
+	_sl.block_flags = 0;
 	_sl.obj_len = 0;
 
 	SaveLoadChunkExtHeaderFlags ext_flags = static_cast<SaveLoadChunkExtHeaderFlags>(0);
@@ -2637,6 +2679,7 @@ static void SlSaveChunk(const ChunkHandler &ch)
 	if (_debug_sl_level >= 3) written = SlGetBytesWritten();
 
 	_sl.block_mode = ch.type;
+	_sl.block_flags = 0;
 	_sl.expect_table_header = (_sl.block_mode == CH_TABLE || _sl.block_mode == CH_SPARSE_TABLE);
 	_sl.need_length = (_sl.expect_table_header || _sl.block_mode == CH_RIFF) ? NL_WANTLENGTH : NL_NONE;
 

src/sl/saveload.h

@@ -1075,6 +1075,7 @@ std::vector<SaveLoad> SlTableHeader(const NamedSaveLoadTable &slt);
 std::vector<SaveLoad> SlTableHeaderOrRiff(const NamedSaveLoadTable &slt);
 void SlSaveTableObjectChunk(const SaveLoadTable &slt);
 void SlLoadTableOrRiffFiltered(const SaveLoadTable &slt);
+void SlLoadTableWithArrayLengthPrefixesMissing();
 
 inline void SlSaveTableObjectChunk(const NamedSaveLoadTable &slt)
 {