Ticket #3815: partial_fix_for_bug_2019355-v1.patch

engines/cine/script_fw.cpp

@@ -230 +230 @@
  * \param fHandle Savefile open for reading
  * \param len Size of array
  */
-ScriptVars::ScriptVars(Common::InSaveFile &fHandle, unsigned int len)
+ScriptVars::ScriptVars(Common::SeekableReadStream &fHandle, unsigned int len)
         : _size(len), _vars(new int16[len]) {
 
         assert(_vars);
@@ -306 +306 @@
 /*! \brief Restore array from savefile
  * \param fHandle Savefile open for reading
  */
-void ScriptVars::load(Common::InSaveFile &fHandle) {
+void ScriptVars::load(Common::SeekableReadStream &fHandle) {
         load(fHandle, _size);
 }
 
@@ -314 +314 @@
  * \param fHandle Savefile open for reading
  * \param len Length of data to be read
  */
-void ScriptVars::load(Common::InSaveFile &fHandle, unsigned int len) {
+void ScriptVars::load(Common::SeekableReadStream &fHandle, unsigned int len) {
         debug(6, "assert(%d <= %d)", len, _size);
         assert(len <= _size);
         for (unsigned int i = 0; i < len; i++) {

engines/cine/script.h

@@ -61 +61 @@
 public:
         // Explicit to prevent var=0 instead of var[i]=0 typos.
         explicit ScriptVars(unsigned int len = 50);
-        ScriptVars(Common::InSaveFile &fHandle, unsigned int len = 50);
+        ScriptVars(Common::SeekableReadStream &fHandle, unsigned int len = 50);
         ScriptVars(const ScriptVars &src);
         ~ScriptVars(void);
 
@@ -71 +71 @@
 
         void save(Common::OutSaveFile &fHandle) const;
         void save(Common::OutSaveFile &fHandle, unsigned int len) const;
-        void load(Common::InSaveFile &fHandle);
-        void load(Common::InSaveFile &fHandle, unsigned int len);
+        void load(Common::SeekableReadStream &fHandle);
+        void load(Common::SeekableReadStream &fHandle, unsigned int len);
         void reset(void);
 };
 

engines/cine/bg_list.cpp

@@ -83 +83 @@
 /*! \brief Restore incrust list from savefile
  * \param fHandle Savefile open for reading
  */
-void loadBgIncrustFromSave(Common::InSaveFile &fHandle) {
+void loadBgIncrustFromSave(Common::SeekableReadStream &fHandle) {
         BGIncrust tmp;
         int size = fHandle.readSint16BE();
 

engines/cine/anim.cpp

@@ -769 +769 @@
 
 /*! \brief Load animDataTable from save
  * \param fHandle Savefile open for reading
- * \param broken Broken/correct file format switch
+ * \param saveGameFormat The used savegame format
  * \todo Add Operation Stealth savefile support
  *
  * Unlike the old code, this one actually rebuilds the table one frame
  * at a time.
  */
-void loadResourcesFromSave(Common::InSaveFile &fHandle, bool broken) {
+void loadResourcesFromSave(Common::SeekableReadStream &fHandle, enum CineSaveGameFormat saveGameFormat) {
         int16 currentAnim, foundFileIdx;
         int8 isMask = 0, isSpl = 0;
         byte *dataPtr, *ptr;
@@ -791 +791 @@
 
         strcpy(part, currentPartName);
 
+        // We only support these variations of the savegame format at the moment.
+        assert(saveGameFormat == ANIMSIZE_23 || saveGameFormat == ANIMSIZE_30_PTRS_INTACT);
+
         for (currentAnim = 0; currentAnim < NUM_MAX_ANIMDATA; currentAnim++) {
                 currentPtr = &animDataTable[currentAnim];
 
@@ -799 +802 @@
                 bpp = fHandle.readUint16BE();
                 height = fHandle.readUint16BE();
 
-                if (!broken) {
-                        if (!fHandle.readUint32BE()) {
-                                fHandle.skip(18);
-                                continue;
-                        }
-                        fHandle.readUint32BE();
+                bool validPtr = false;
+                if (saveGameFormat == ANIMSIZE_30_PTRS_INTACT) {                        
+                        validPtr = (fHandle.readUint32BE() != 0); // Read data pointer
+                        fHandle.readUint32BE(); // Discard mask pointer
                 }
 
                 foundFileIdx = fHandle.readSint16BE();
                 frame = fHandle.readSint16BE();
                 fHandle.read(name, 10);
 
-                if (foundFileIdx < 0 || (broken && !fHandle.readByte())) {
+                if (saveGameFormat == ANIMSIZE_23) {
+                        validPtr = (fHandle.readByte() != 0);
+                }
+
+                // Don't try to load invalid entries.
+                if (foundFileIdx < 0 || !validPtr) {
                         continue;
                 }
 

engines/cine/various.cpp

@@ -246 +246 @@
         return true;
 }
 
+/*! \brief Savegame format detector
+ * \param fHandle Savefile to check
+ * \return Savegame format on success, ANIMSIZE_UNKNOWN on failure
+ *
+ * This function seek 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 entriesCount = 255;
+        static const uint oldEntrySize = 23;
+        static const uint newEntrySize = 30;
+        static const uint defaultEntrySize = newEntrySize;
+        static const uint entrySizeChoices[] = {oldEntrySize, newEntrySize};
+        Common::Array<uint> entrySizeMatches;
+        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(entrySizeChoices); 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 entrySizes[] = {
+                        globalScriptEntrySize,
+                        objectScriptEntrySize,
+                        overlayEntrySize,
+                        bgIncrustEntrySize
+                };
+                
+                uint entrySize = entrySizeChoices[i];           
+                // Jump over the animDataTable entries and the screen parameters
+                uint32 newPos = animDataTableStart + entrySize * entriesCount + 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
+                bool chainWalkSuccess = true;
+                for (uint j = 0; j < ARRAYSIZE(entrySizes); j++) {
+                        // Read entry count and jump over the entries
+                        int entryCount = fHandle.readSint16BE();
+                        newPos = fHandle.pos() + entrySizes[j] * 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
+                        entrySizeMatches.push_back(entrySize);
+                }
+        }
+
+        // Check that we got only one entry size match.
+        // If we didn't, then return an error.
+        enum CineSaveGameFormat result = ANIMSIZE_UNKNOWN;
+        if (entrySizeMatches.size() == 1) {
+                const uint entrySize = entrySizeMatches[0];
+                assert(entrySize == oldEntrySize || entrySize == newEntrySize);
+                if (entrySize == oldEntrySize) {
+                        result = ANIMSIZE_23;
+                } else { // entrySize == newEntrySize           
+                        // 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 < entriesCount; i++) {
+                                fHandle.seek(animDataTableStart + i * entrySize + 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 (entrySizeMatches.size() > 1) {
+                warning("Savegame format detector got confused by input data. Detecting savegame to be using an unknown format");
+        } else { // entrySizeMatches.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 +391 @@
 /*! \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 +407 @@
         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 +425 @@
                 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 +514 @@
 
         checkForPendingDataLoadSwitch = 0;
 
-        broken = brokenSave(*fHandle);
 
         // At savefile position 0x0000:
         currentDisk = fHandle->readUint16BE();
@@ -588 +637 @@
         fHandle->readUint16BE();
 
         // At 0x2315:
-        loadResourcesFromSave(*fHandle, broken);
+        loadResourcesFromSave(*fHandle, saveGameFormat);
 
         // TODO: handle screen params (really required ?)
         fHandle->readUint16BE();
@@ -600 +649 @@
 
         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 +664 @@
 
         loadBgIncrustFromSave(*fHandle);
 
-        delete fHandle;
-
         if (strlen(currentMsgName)) {
                 loadMsg(currentMsgName);
         }

engines/cine/gfx.h

@@ -113 +113 @@
 
         virtual void refreshPalette();
         virtual void reloadPalette();
-        void restorePalette(Common::InSaveFile &fHandle);
+        void restorePalette(Common::SeekableReadStream &fHandle);
         void savePalette(Common::OutSaveFile &fHandle);
         virtual void rotatePalette(int a, int b, int c);
         virtual void transformPalette(int first, int last, int r, int g, int b);

engines/cine/gfx.cpp

@@ -614 +614 @@
 /*! \brief Restore active and backup palette from save
  * \param fHandle Savefile open for reading
  */
-void FWRenderer::restorePalette(Common::InSaveFile &fHandle) {
+void FWRenderer::restorePalette(Common::SeekableReadStream &fHandle) {
         int i;
 
         if (!_palette) {

engines/cine/bg_list.h

@@ -51 +51 @@
 
 void createBgIncrustListElement(int16 objIdx, int16 param);
 void resetBgIncrustList(void);
-void loadBgIncrustFromSave(Common::InSaveFile &fHandle);
+void loadBgIncrustFromSave(Common::SeekableReadStream &fHandle);
 
 } // End of namespace Cine
 

engines/cine/anim.h

@@ -101 +101 @@
 void freeAnimDataRange(byte startIdx, byte numIdx);
 void loadResource(const char *resourceName);
 void loadAbs(const char *resourceName, uint16 idx);
-void loadResourcesFromSave(Common::InSaveFile &fHandle, bool broken);
+void loadResourcesFromSave(Common::SeekableReadStream &fHandle, enum CineSaveGameFormat saveGameFormat);
 void generateMask(const byte *sprite, byte *mask, uint16 size, byte transparency);
 
 } // End of namespace Cine

engines/cine/various.h

@@ -33 +33 @@
 
 namespace Cine {
 
+/**
+ * Cine engine's save game versions.
+ * Enumeration entries (Excluding the one used as an error)
+ * are sorted according to age (i.e. top one is oldest, last one newest etc).
+ *
+ * ANIMSIZE_UNKNOWN:
+ * - Animation data entry size is unknown (Used as an error).
+ *
+ * ANIMSIZE_23:
+ * - Animation data entry size size is 23 bytes.
+ * - Used for an example by the 0.11.1 release of ScummVM.
+ *
+ * ANIMSIZE_30_PTRS_BROKEN:
+ * - Animation data entry size size is 30 bytes.
+ * - Data and mask pointers in the saved structs are always NULL.
+ *
+ * ANIMSIZE_30_PTRS_INTACT:
+ * - Animation data entry size size is 30 bytes.
+ * - Data and mask pointers in the saved structs are intact,
+ *   so you can test them for equality or inequality with NULL
+ *   but don't try using them for anything else, it won't work.
+ */
+enum CineSaveGameFormat {
+        ANIMSIZE_UNKNOWN,
+        ANIMSIZE_23,
+        ANIMSIZE_30_PTRS_BROKEN,
+        ANIMSIZE_30_PTRS_INTACT
+};
+
 void initLanguage(Common::Language lang);
 
 int16 makeMenuChoice(const CommandeType commandList[], uint16 height, uint16 X, uint16 Y, uint16 width, bool recheckValue = false);