[Scummvm-git-logs] scummvm master -> 5f382329f1927e97d75443635728a5436d5d5965
athrxx
noreply at scummvm.org
Mon Apr 21 11:55:43 UTC 2025
This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://api.github.com/repos/scummvm/scummvm .
Summary:
5f382329f1 SCUMM: fix bug no. 15874
Commit: 5f382329f1927e97d75443635728a5436d5d5965
https://github.com/scummvm/scummvm/commit/5f382329f1927e97d75443635728a5436d5d5965
Author: athrxx (athrxx at scummvm.org)
Date: 2025-04-21T13:21:44+02:00
Commit Message:
SCUMM: fix bug no. 15874
Apart from the SegaCD fix, this generally improves loading
savegames that were created with different sound setting.
E. g. loading a MI1 savegame that was made with a CMS
setting used to crash the engine when loading with a MT-32
or AdLib setting.
This is still not a full compatibility patch, the sound will
often be broken after loading. It is just another step on
our way.
The downside is that I had to inject quite a bit of weird
code to calculate the sound data size for old saves.
Changed paths:
A engines/scumm/serializer.h
engines/scumm/saveload.cpp
engines/scumm/scumm.h
engines/scumm/scumm_v0.h
engines/scumm/scumm_v2.h
engines/scumm/scumm_v5.h
diff --git a/engines/scumm/saveload.cpp b/engines/scumm/saveload.cpp
index 3c19447fd8a..e743bbb4550 100644
--- a/engines/scumm/saveload.cpp
+++ b/engines/scumm/saveload.cpp
@@ -70,7 +70,7 @@ struct SaveInfoSection {
#define SaveInfoSectionSize (4+4+4 + 4+4 + 4+2)
-#define CURRENT_VER 123
+#define CURRENT_VER 124
#define INFOSECTION_VERSION 2
#pragma mark -
@@ -566,7 +566,7 @@ Common::SeekableWriteStream *ScummEngine::openSaveFileForWriting(int slot, bool
return _saveFileMan->openForSaving(fileName);
}
-bool ScummEngine::saveState(Common::WriteStream *out, bool writeHeader) {
+bool ScummEngine::saveState(Common::SeekableWriteStream *out, bool writeHeader) {
SaveGameHeader hdr;
if (writeHeader) {
@@ -582,7 +582,7 @@ bool ScummEngine::saveState(Common::WriteStream *out, bool writeHeader) {
#endif
saveInfos(out);
- Common::Serializer ser(nullptr, out);
+ Serializer ser(nullptr, out);
ser.setVersion(CURRENT_VER);
saveLoadWithSerializer(ser);
return true;
@@ -602,7 +602,7 @@ bool ScummEngine::saveState(int slot, bool compat, Common::String &filename) {
_pauseSoundsDuringSave = true;
- Common::WriteStream *out = openSaveFileForWriting(slot, compat, filename);
+ Common::SeekableWriteStream *out = openSaveFileForWriting(slot, compat, filename);
if (!out) {
saveFailed = true;
} else {
@@ -2050,20 +2050,79 @@ void ScummEngine::saveLoadWithSerializer(Common::Serializer &s) {
}
}
+ // The following code tries to cope with the unfortunate fact that savegames
+ // vary in size/content depending on the sound setting. And historically, until
+ // version 124, it was not even possible to make a definite assumption about the
+ // size of the sound data block. Loading savegames that were made with a different
+ // sound setting than the current one would lead to a crash or to a corrupted
+ // engine state. Now, the engine should recover in most cases, but some issues
+ // still have to be addressed.
+ // TODO: identify the device for the saved sound data blocks (preferably even the
+ // old ones which are prone to have an unreliable VAR_SOUNDCARD value) and/or reset
+ // the sound state, so that even changes from AdLib to MT-32 or vice versa could
+ // eventually work.
+ Serializer *pSer = static_cast<Serializer*>(&s);
+
+ int32 sndDataBlockSize = 0;
+ if (s.isLoading()) {
+ if (s.getVersion() > VER(123))
+ s.syncAsSint32BE(sndDataBlockSize);
+ else if (_game.version < 7 && _game.heversion == 0)
+ sndDataBlockSize = checkSoundEngineSaveDataSize(*pSer);
+ }
- //
- // Save/load the iMuse status
- //
- if (_imuse && (_saveSound || !_saveTemporaryState)) {
- _imuse->saveLoadIMuse(s, this);
+ int64 before = pSer->pos();
+
+ // Unfortunately, it is not totally easy to write the size of the sound engine data before that data,
+ // because we cannot seek around in the compressed write stream. So, we write to a temp stream, first.
+ Common::MemoryWriteStreamDynamic *ws = nullptr;
+ if (s.isSaving()) {
+ ws = new Common::MemoryWriteStreamDynamic(DisposeAfterUse::YES);
+ pSer = new Serializer(nullptr, ws);
+ pSer->setVersion(s.getVersion());
}
+ // This sndDataBlockSize check catches e. g. MI1 saves with a CMS setting when trying to load with AdLib or MT-32.
+ // But it only works because CMS really saves nothing. If it were just a smaller data block, it would still load
+ // garbage data (and crash or corrupt the game state).
+ if (s.isSaving() || sndDataBlockSize > 0) {
+ //
+ // Save/load the iMuse status
+ if (_imuse && (_saveSound || !_saveTemporaryState))
+ _imuse->saveLoadIMuse(*pSer, this);
+ //
+ // Save/load music engine status
+ if (_musicEngine)
+ _musicEngine->saveLoadWithSerializer(*pSer);
+ }
- //
- // Save/load music engine status
- //
- if (_musicEngine) {
- _musicEngine->saveLoadWithSerializer(s);
+ if (s.isSaving()) {
+ // Write the sound data size, then the actual data from the temp stream.
+ ws->finalize();
+ sndDataBlockSize = ws->size();
+ s.syncAsSint32BE(sndDataBlockSize);
+ s.syncBytes(ws->getData(), sndDataBlockSize);
+ delete pSer;
+ delete ws;
+ } else {
+ // Check if the sound engine read the expected number of bytes from the save file.
+ // If not, then the sound device selection was probably changed after the save was made.
+ int64 now = pSer->pos();
+ if (s.err() || (before + sndDataBlockSize != now)) {
+ static const char wmsg1[] = "more than the %d bytes contained in the savegame";
+ static const char wmsg2[] = "%d bytes, savegame has %d bytes";
+ // For SegaCD, we don't need a warning, since nothing can glitch there. We have to compensate
+ // for the fact that there are old savegames that have an unused imuse state inside of them.
+ // But fixing that will not lead to glitches or other surprises.
+ if (_game.platform == Common::kPlatformSegaCD) {
+ Common::String msg = s.err() ? Common::String::format(wmsg1, sndDataBlockSize) : Common::String::format(wmsg2, (int)(now - before), sndDataBlockSize);
+ warning("Savegame sound data mismatch (sound engine tried to read %s). \r\nAdjusting file read position. Sound might start up with glitches...", msg.c_str());
+ }
+ // This will save the day in cases that are the opposite from the ones mentioned above:
+ // When loading a MI1 savegame that was made with a MT-32 or AdLib setting in a CMS setup,
+ // we skip the sound data block here.
+ pSer->seek(before + sndDataBlockSize);
+ }
}
// At least from now on, VAR_SOUNDCARD will have a reliable value.
@@ -2394,4 +2453,80 @@ void ScummEngine::loadResource(Common::Serializer &ser, ResType type, ResId idx)
}
}
+int ScummEngine::checkSoundEngineSaveDataSize(Serializer &s) {
+ if (!s.isLoading())
+ return 0;
+
+ uint8 d8 = 0;
+ uint32 d32 = 0;
+ int64 start = s.pos();
+ // We just perform the steps we would do in the actual sync function, but on dummy vars.
+ s.syncAsByte(d8, VER(73), VER(73));
+ s.syncAsSint32LE(d32, VER(74));
+ s.syncAsByte(d8, VER(73));
+ if (s.getVersion() == VER(72))
+ s.syncAsByte(d8);
+ int64 diff = s.pos() - start;
+ s.seek(start);
+
+ return s.size() - diff - start;
+}
+
+int ScummEngine_v0::checkSoundEngineSaveDataSize(Serializer &s) {
+ if (!s.isLoading())
+ return 0;
+
+ uint8 d8 = 0;
+ uint16 d16 = 0;
+ // We just perform the steps we would do in the actual sync function, but on dummy vars.
+ int64 start = s.pos();
+ s.syncAsByte(d8, VER(78));
+ s.syncAsByte(d8, VER(78));
+ s.syncAsByte(d8, VER(92));
+ s.syncAsUint16LE(d16, VER(92));
+ s.syncAsUint16LE(d16, VER(92));
+ s.syncAsByte(d8, VER(92));
+ s.syncAsUint16LE(d16, VER(92));
+ s.syncAsUint16LE(d16, VER(92));
+ s.syncAsUint16LE(d16, VER(92));
+ s.syncAsByte(d8, VER(92));
+ int64 diff = s.pos() - start;
+ s.seek(start);
+
+ return ScummEngine_v2::checkSoundEngineSaveDataSize(s) - diff;
+}
+
+int ScummEngine_v2::checkSoundEngineSaveDataSize(Serializer &s) {
+ if (!s.isLoading())
+ return 0;
+
+ uint8 d8 = 0;
+ uint16 d16 = 0;
+ // We just perform the steps we would do in the actual sync function, but on dummy vars.
+ int64 start = s.pos();
+ s.syncAsUint16LE(d16, VER(79));
+ s.syncAsByte(d8, VER(99));
+ s.syncAsByte(d8, VER(99));
+ int64 diff = s.pos() - start;
+ s.seek(start);
+
+ return ScummEngine::checkSoundEngineSaveDataSize(s) - diff;
+}
+
+int ScummEngine_v5::checkSoundEngineSaveDataSize(Serializer &s) {
+ if (!s.isLoading())
+ return 0;
+
+ uint8 d[8] = { 0 };
+ int64 start = s.pos();
+ // We just perform the steps we would do in the actual sync function. I doesn't
+ // matter if we fill in a bit garbage data here, since it will be overwritten later.
+ sync2DArray(s, _cursorImages, 4, 16, Common::Serializer::Uint16LE, VER(44));
+ s.syncBytes(d, 8, VER(44));
+ int64 diff = s.pos() - start;
+ s.seek(start);
+
+ return ScummEngine::checkSoundEngineSaveDataSize(s) - diff;
+}
+
} // End of namespace Scumm
diff --git a/engines/scumm/scumm.h b/engines/scumm/scumm.h
index 9723da6f36c..ceed7970d3d 100644
--- a/engines/scumm/scumm.h
+++ b/engines/scumm/scumm.h
@@ -33,7 +33,6 @@
#include "common/random.h"
#include "common/rect.h"
#include "common/rendermode.h"
-#include "common/serializer.h"
#include "common/str.h"
#include "common/textconsole.h"
#include "graphics/surface.h"
@@ -44,6 +43,7 @@
#include "scumm/gfx.h"
#include "scumm/detection.h"
#include "scumm/script.h"
+#include "scumm/serializer.h"
#ifdef __DS__
/* This disables the dual layer mode which is used in FM-Towns versions
@@ -919,7 +919,7 @@ protected:
Common::String _saveLoadFileName;
Common::String _saveLoadDescription;
- bool saveState(Common::WriteStream *out, bool writeHeader = true);
+ bool saveState(Common::SeekableWriteStream *out, bool writeHeader = true);
bool saveState(int slot, bool compat, Common::String &fileName);
bool loadState(int slot, bool compat);
bool loadState(int slot, bool compat, Common::String &fileName);
@@ -928,6 +928,8 @@ protected:
void loadResource(Common::Serializer &ser, ResType type, ResId idx);
void loadResourceOLD(Common::Serializer &ser, ResType type, ResId idx); // "Obsolete"
+ virtual int checkSoundEngineSaveDataSize(Serializer &s);
+
void copyHeapSaveGameToFile(int slot, const char *saveName);
bool changeSavegameName(int slot, char *newName);
virtual Common::SeekableReadStream *openSaveFileForReading(int slot, bool compat, Common::String &fileName);
diff --git a/engines/scumm/scumm_v0.h b/engines/scumm/scumm_v0.h
index 233b6b1cf5b..2e9d925e396 100644
--- a/engines/scumm/scumm_v0.h
+++ b/engines/scumm/scumm_v0.h
@@ -86,6 +86,7 @@ protected:
void processInput() override;
void saveLoadWithSerializer(Common::Serializer &s) override;
+ int checkSoundEngineSaveDataSize(Serializer &s) override;
void terminateSaveMenuScript() override;
bool objIsActor(int obj) override;
diff --git a/engines/scumm/scumm_v2.h b/engines/scumm/scumm_v2.h
index 94fe410223d..62f732b50d6 100644
--- a/engines/scumm/scumm_v2.h
+++ b/engines/scumm/scumm_v2.h
@@ -64,6 +64,7 @@ protected:
void decodeParseString() override;
void saveLoadWithSerializer(Common::Serializer &s) override;
+ int checkSoundEngineSaveDataSize(Serializer &s) override;
void terminateSaveMenuScript() override;
void processKeyboard(Common::KeyState lastKeyHit) override;
diff --git a/engines/scumm/scumm_v5.h b/engines/scumm/scumm_v5.h
index aa7c240f1b1..75e2176f14a 100644
--- a/engines/scumm/scumm_v5.h
+++ b/engines/scumm/scumm_v5.h
@@ -68,6 +68,7 @@ protected:
void printPatchedMI1CannibalString(int textSlot, const byte *ptr);
void saveLoadWithSerializer(Common::Serializer &s) override;
+ int checkSoundEngineSaveDataSize(Serializer &s) override;
void readMAXS(int blockSize) override;
diff --git a/engines/scumm/serializer.h b/engines/scumm/serializer.h
new file mode 100644
index 00000000000..04adb8be92f
--- /dev/null
+++ b/engines/scumm/serializer.h
@@ -0,0 +1,57 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+
+
+#ifndef SCUMM_SERIALIZER_H
+#define SCUMM_SERIALIZER_H
+
+
+#include "common/serializer.h"
+
+
+namespace Scumm {
+
+class Serializer : public Common::Serializer {
+public:
+ Serializer(Common::SeekableReadStream *stream, Common::SeekableWriteStream *writeStream)
+ : Common::Serializer(stream, writeStream), _seekableSaveStr(writeStream) {
+ }
+ ~Serializer() override {
+ }
+ int64 pos() const {
+ return _loadStream ? _loadStream->pos() : (_saveStream ? _saveStream->pos() : 0);
+ }
+ int seek(int64 offset, int whence = 0) {
+ return _loadStream ? _loadStream->seek(offset, whence) : (_seekableSaveStr ? _seekableSaveStr->seek(offset, whence) : -1);
+ }
+ int64 size() const {
+ return _loadStream ? _loadStream->size() : (_seekableSaveStr ? _seekableSaveStr->size() : 0);
+ }
+
+private:
+ Common::SeekableWriteStream *_seekableSaveStr;
+};
+
+
+} // End of namespace Audio
+
+#endif // #ifndef SCUMM_CDDA_H
More information about the Scummvm-git-logs
mailing list