Fix for bug #2019355 (FW: broken compatibility with 0.11.1 saves):

- Changed savegame loading related functions to use SeekableReadStream
  rather than InSaveFile so MemoryReadStream can be used transparently.
- Fixed loadResourcesFromSave to load multiframe animations correctly
  and to load 0.11.0/0.11.1 Future Wars savegames which used a slightly
  different format.
- Added a savegame format detector that tries to detect between the old
  Future Wars savegame format, the new one and a broken revision of the
  new one.
- Changed makeLoad to first load the savegame fully into memory and only
  then handle it (If the savegame's packed then it's unpacked first). If
  the packed savegame can't tell its unpacked size (i.e. it's using zlib
  format) then we'll try to load up to 256kB of the savegame data.
Thanks to wjp for his help with nailing this release critical bug.

svn-id: r33192

1 files changed, 162 insertions, 114 deletions

diff --git a/engines/cine/various.cpp b/engines/cine/various.cpp
index ab2fa645c7..8e6f3fce76 100644
--- a/engines/cine/various.cpp
+++ b/engines/cine/various.cpp
@@ -246,21 +246,130 @@ bool CineEngine::loadSaveDirectory(void) {
 	return true;
 }
 
+/*! \brief Savegame format detector
+ * \param fHandle Savefile to check
+ * \return Savegame format on success, ANIMSIZE_UNKNOWN on failure
+ *
+ * This function seeks through the savefile and tries to determine the
+ * savegame format it uses. There's a miniscule chance that the detection
+ * algorithm could get confused and think that the file uses both the older
+ * and the newer format but that is such a remote possibility that I wouldn't
+ * worry about it at all.
+ */
+enum CineSaveGameFormat detectSaveGameFormat(Common::SeekableReadStream &fHandle) {
+	// The animDataTable begins at savefile position 0x2315.
+	// Each animDataTable entry takes 23 bytes in older saves (Revisions 21772-31443)
+	// and 30 bytes in the save format after that (Revision 31444 and onwards).
+	// There are 255 entries in the animDataTable in both of the savefile formats.
+	static const uint animDataTableStart = 0x2315;
+	static const uint animEntriesCount = 255;
+	static const uint oldAnimEntrySize = 23;
+	static const uint newAnimEntrySize = 30;
+	static const uint defaultAnimEntrySize = newAnimEntrySize;
+	static const uint animEntrySizeChoices[] = {oldAnimEntrySize, newAnimEntrySize};
+	Common::Array<uint> animEntrySizeMatches;
+	const uint32 prevStreamPos = fHandle.pos();
+
+	// Try to walk through the savefile using different animDataTable entry sizes
+	// and make a list of all the successful entry sizes.
+	for (uint i = 0; i < ARRAYSIZE(animEntrySizeChoices); i++) {
+		// 206 = 2 * 50 * 2 + 2 * 3 (Size of global and object script entries)
+		// 20 = 4 * 2 + 2 * 6 (Size of overlay and background incrust entries)
+		static const uint sizeofScreenParams = 2 * 6;
+		static const uint globalScriptEntrySize = 206;
+		static const uint objectScriptEntrySize = 206;
+		static const uint overlayEntrySize = 20;
+		static const uint bgIncrustEntrySize = 20;
+		static const uint chainEntrySizes[] = {
+			globalScriptEntrySize,
+			objectScriptEntrySize,
+			overlayEntrySize,
+			bgIncrustEntrySize
+		};
+		
+		uint animEntrySize = animEntrySizeChoices[i];
+		// Jump over the animDataTable entries and the screen parameters
+		uint32 newPos = animDataTableStart + animEntrySize * animEntriesCount + sizeofScreenParams;
+		// Check that there's data left after the point we're going to jump to
+		if (newPos >= fHandle.size()) {
+			continue;
+		}
+		fHandle.seek(newPos);
+
+		// Jump over the remaining items in the savegame file
+		// (i.e. the global scripts, object scripts, overlays and background incrusts).
+		bool chainWalkSuccess = true;
+		for (uint chainIndex = 0; chainIndex < ARRAYSIZE(chainEntrySizes); chainIndex++) {
+			// Read entry count and jump over the entries
+			int entryCount = fHandle.readSint16BE();
+			newPos = fHandle.pos() + chainEntrySizes[chainIndex] * entryCount;
+			// Check that we didn't go past the end of file.
+			// Note that getting exactly to the end of file is acceptable.
+			if (newPos > fHandle.size()) {
+				chainWalkSuccess = false;
+				break;
+			}
+			fHandle.seek(newPos);
+		}
+		
+		// If we could walk the chain successfully and
+		// got exactly to the end of file then we've got a match.
+		if (chainWalkSuccess && fHandle.pos() == fHandle.size()) {
+			// We found a match, let's save it
+			animEntrySizeMatches.push_back(animEntrySize);
+		}
+	}
+
+	// Check that we got only one entry size match.
+	// If we didn't, then return an error.
+	enum CineSaveGameFormat result = ANIMSIZE_UNKNOWN;
+	if (animEntrySizeMatches.size() == 1) {
+		const uint animEntrySize = animEntrySizeMatches[0];
+		assert(animEntrySize == oldAnimEntrySize || animEntrySize == newAnimEntrySize);
+		if (animEntrySize == oldAnimEntrySize) {
+			result = ANIMSIZE_23;
+		} else { // animEntrySize == newAnimEntrySize		
+			// Check data and mask pointers in all of the animDataTable entries
+			// to see whether we've got the version with the broken data and mask pointers or not.
+			// In the broken format all data and mask pointers were always zero.
+			static const uint relativeDataPos = 2 * 4;
+			bool pointersIntact = false;
+			for (uint i = 0; i < animEntriesCount; i++) {
+				fHandle.seek(animDataTableStart + i * animEntrySize + relativeDataPos);
+				uint32 data = fHandle.readUint32BE();
+				uint32 mask = fHandle.readUint32BE();
+				if (data != NULL || mask != NULL) {
+					pointersIntact = true;
+					break;
+				}
+			}
+			result = (pointersIntact ? ANIMSIZE_30_PTRS_INTACT : ANIMSIZE_30_PTRS_BROKEN);
+		}
+	} else if (animEntrySizeMatches.size() > 1) {
+		warning("Savegame format detector got confused by input data. Detecting savegame to be using an unknown format");
+	} else { // animEtrySizeMatches.size() == 0
+		debug(3, "Savegame format detector was unable to detect savegame's format");
+	}
+
+	fHandle.seek(prevStreamPos);
+	return result;
+}
+
 /*! \brief Restore script list item from savefile
- * \param fHandle Savefile handlem open for reading
+ * \param fHandle Savefile handle open for reading
  * \param isGlobal Restore object or global script?
  */
-void loadScriptFromSave(Common::InSaveFile *fHandle, bool isGlobal) {
+void loadScriptFromSave(Common::SeekableReadStream &fHandle, bool isGlobal) {
 	ScriptVars localVars, labels;
 	uint16 compare, pos;
 	int16 idx;
 
-	labels.load(*fHandle);
-	localVars.load(*fHandle);
+	labels.load(fHandle);
+	localVars.load(fHandle);
 
-	compare = fHandle->readUint16BE();
-	pos = fHandle->readUint16BE();
-	idx = fHandle->readUint16BE();
+	compare = fHandle.readUint16BE();
+	pos = fHandle.readUint16BE();
+	idx = fHandle.readUint16BE();
 
 	// no way to reinitialize these
 	if (idx < 0) {
@@ -283,7 +392,7 @@ void loadScriptFromSave(Common::InSaveFile *fHandle, bool isGlobal) {
 /*! \brief Restore overlay sprites from savefile
  * \param fHandle Savefile open for reading
  */
-void loadOverlayFromSave(Common::InSaveFile &fHandle) {
+void loadOverlayFromSave(Common::SeekableReadStream &fHandle) {
 	overlay tmp;
 
 	fHandle.readUint32BE();
@@ -299,115 +408,17 @@ void loadOverlayFromSave(Common::InSaveFile &fHandle) {
 	overlayList.push_back(tmp);
 }
 
-/*! \brief Savefile format tester
- * \param fHandle Savefile to check
- *
- * This function seeks through savefile and tries to guess if it's the original
- * savegame format or broken format from ScummVM 0.10/0.11
- * The test is incomplete but this should cover 99.99% of cases.
- * If anyone makes a savefile which could confuse this test, assert will
- * report it
- */
-bool brokenSave(Common::InSaveFile &fHandle) {
-	// Backward seeking not supported in compressed savefiles
-	// if you really want it, finish it yourself
-	return false;
-
-	// fixed size part: 14093 bytes (12308 bytes in broken save)
-	// animDataTable begins at byte 6431
-
-	int filesize = fHandle.size();
-	int startpos = fHandle.pos();
-	int pos, tmp;
-	bool correct = false, broken = false;
-
-	// check for correct format
-	while (filesize > 14093) {
-		pos = 14093;
-
-		fHandle.seek(pos);
-		tmp = fHandle.readUint16BE();
-		pos += 2 + tmp * 206;
-		if (pos >= filesize) break;
-
-		fHandle.seek(pos);
-		tmp = fHandle.readUint16BE();
-		pos += 2 + tmp * 206;
-		if (pos >= filesize) break;
-
-		fHandle.seek(pos);
-		tmp = fHandle.readUint16BE();
-		pos += 2 + tmp * 20;
-		if (pos >= filesize) break;
-
-		fHandle.seek(pos);
-		tmp = fHandle.readUint16BE();
-		pos += 2 + tmp * 20;
-
-		if (pos == filesize) correct = true;
-		break;
-	}
-	debug(5, "brokenSave: correct format check %s: size=%d, pos=%d",
-		correct ? "passed" : "failed", filesize, pos);
-
-	// check for broken format
-	while (filesize > 12308) {
-		pos = 12308;
-
-		fHandle.seek(pos);
-		tmp = fHandle.readUint16BE();
-		pos += 2 + tmp * 206;
-		if (pos >= filesize) break;
-
-		fHandle.seek(pos);
-		tmp = fHandle.readUint16BE();
-		pos += 2 + tmp * 206;
-		if (pos >= filesize) break;
-
-		fHandle.seek(pos);
-		tmp = fHandle.readUint16BE();
-		pos += 2 + tmp * 20;
-		if (pos >= filesize) break;
-
-		fHandle.seek(pos);
-		tmp = fHandle.readUint16BE();
-		pos += 2 + tmp * 20;
-
-		if (pos == filesize) broken = true;
-		break;
-	}
-	debug(5, "brokenSave: broken format check %s: size=%d, pos=%d",
-		broken ? "passed" : "failed", filesize, pos);
-
-	// there's a very small chance that both cases will match
-	// if anyone runs into it, you'll have to walk through
-	// the animDataTable and try to open part file for each entry
-	if (!correct && !broken) {
-		error("brokenSave: file format check failed");
-	} else if (correct && broken) {
-		error("brokenSave: both file formats seem to apply");
-	}
-
-	fHandle.seek(startpos);
-	debug(5, "brokenSave: detected %s file format",
-		correct ? "correct" : "broken");
-
-	return broken;
-}
-
 /*! \todo Implement Operation Stealth loading, this is obviously Future Wars only
  * \todo Add support for loading the zoneQuery table (Operation Stealth specific)
  */
 bool CineEngine::makeLoad(char *saveName) {
 	int16 i;
 	int16 size;
-	bool broken;
-	Common::InSaveFile *fHandle;
 	char bgName[13];
 
-	fHandle = g_saveFileMan->openForLoading(saveName);
+	Common::SharedPtr<Common::InSaveFile> saveFile(g_saveFileMan->openForLoading(saveName));
 
-	if (!fHandle) {
+	if (!saveFile) {
 		drawString(otherMessages[0], 0);
 		waitPlayerInput();
 		// restoreScreen();
@@ -415,6 +426,46 @@ bool CineEngine::makeLoad(char *saveName) {
 		return false;
 	}
 
+	uint32 saveSize = saveFile->size();
+	if (saveSize == 0) { // Savefile's compressed using zlib format can't tell their unpacked size, test for it
+		// Can't get information about the savefile's size so let's try
+		// reading as much as we can from the file up to a predefined upper limit.
+		//
+		// Some estimates for maximum savefile sizes (All with 255 animDataTable entries of 30 bytes each):
+		// With 256 global scripts, object scripts, overlays and background incrusts:
+		// 0x2315 + (255 * 30) + (2 * 6) + (206 + 206 + 20 + 20) * 256 = ~129kB
+		// With 512 global scripts, object scripts, overlays and background incrusts:
+		// 0x2315 + (255 * 30) + (2 * 6) + (206 + 206 + 20 + 20) * 512 = ~242kB
+		//
+		// I think it extremely unlikely that there would be over 512 global scripts, object scripts,
+		// overlays and background incrusts so 256kB seems like quite a safe upper limit.		
+		// NOTE: If the savegame format is changed then this value might have to be re-evaluated!
+		// Hopefully devices with more limited memory can also cope with this memory allocation.
+		saveSize = 256 * 1024;
+	}
+	Common::SharedPtr<Common::MemoryReadStream> fHandle(saveFile->readStream(saveSize));
+
+	// Try to detect the used savegame format
+	enum CineSaveGameFormat saveGameFormat = detectSaveGameFormat(*fHandle);
+
+	// Handle problematic savegame formats
+	if (saveGameFormat == ANIMSIZE_30_PTRS_BROKEN) {
+		// One might be able to load the ANIMSIZE_30_PTRS_BROKEN format but
+		// that's not implemented here because it was never used in a stable
+		// release of ScummVM but only during development (From revision 31453,
+		// which introduced the problem, until revision 32073, which fixed it).
+		// Therefore be bail out if we detect this particular savegame format.
+		warning("Detected a known broken savegame format, not loading savegame");
+		return false;
+	} else if (saveGameFormat == ANIMSIZE_UNKNOWN) {
+		// If we can't detect the savegame format
+		// then let's try the default format and hope for the best.
+		warning("Couldn't detect the used savegame format, trying default savegame format. Things may break");
+		saveGameFormat = ANIMSIZE_30_PTRS_INTACT;
+	}
+	// Now we should have either of these formats
+	assert(saveGameFormat == ANIMSIZE_23 || saveGameFormat == ANIMSIZE_30_PTRS_INTACT);
+
 	g_sound->stopMusic();
 	freeAnimDataTable();
 	overlayList.clear();
@@ -464,7 +515,6 @@ bool CineEngine::makeLoad(char *saveName) {
 
 	checkForPendingDataLoadSwitch = 0;
 
-	broken = brokenSave(*fHandle);
 
 	// At savefile position 0x0000:
 	currentDisk = fHandle->readUint16BE();
@@ -588,7 +638,7 @@ bool CineEngine::makeLoad(char *saveName) {
 	fHandle->readUint16BE();
 
 	// At 0x2315:
-	loadResourcesFromSave(*fHandle, broken);
+	loadResourcesFromSave(*fHandle, saveGameFormat);
 
 	// TODO: handle screen params (really required ?)
 	fHandle->readUint16BE();
@@ -600,12 +650,12 @@ bool CineEngine::makeLoad(char *saveName) {
 
 	size = fHandle->readSint16BE();
 	for (i = 0; i < size; i++) {
-		loadScriptFromSave(fHandle, true);
+		loadScriptFromSave(*fHandle, true);
 	}
 
 	size = fHandle->readSint16BE();
 	for (i = 0; i < size; i++) {
-		loadScriptFromSave(fHandle, false);
+		loadScriptFromSave(*fHandle, false);
 	}
 
 	size = fHandle->readSint16BE();
@@ -615,8 +665,6 @@ bool CineEngine::makeLoad(char *saveName) {
 
 	loadBgIncrustFromSave(*fHandle);
 
-	delete fHandle;
-
 	if (strlen(currentMsgName)) {
 		loadMsg(currentMsgName);
 	}