SCI: Move ScummVM save/restore to GuestAdditions and reimplement for SCI32

7 files changed, 232 insertions, 70 deletions

diff --git a/engines/sci/engine/guest_additions.cpp b/engines/sci/engine/guest_additions.cpp
index a2c64239a5..6477e3284b 100644
--- a/engines/sci/engine/guest_additions.cpp
+++ b/engines/sci/engine/guest_additions.cpp
@@ -29,6 +29,8 @@
 #include "sci/engine/state.h"
 #include "sci/engine/vm.h"
 #ifdef ENABLE_SCI32
+#include "common/translation.h"
+#include "gui/saveload.h"
 #include "sci/graphics/frameout.h"
 #endif
 #include "sci/sound/music.h"
@@ -52,9 +54,10 @@ enum {
 };
 
 
-GuestAdditions::GuestAdditions(EngineState *state, GameFeatures *features) :
+GuestAdditions::GuestAdditions(EngineState *state, GameFeatures *features, Kernel *kernel) :
 	_state(state),
 	_features(features),
+	_kernel(kernel),
 	_segMan(state->_segMan),
 	_messageTypeSynced(false) {}
 
@@ -205,6 +208,178 @@ void GuestAdditions::kDoSoundSetVolumeHook(const reg_t soundObj, const int16 vol
 		syncGK1AudioVolumeToScummVM(soundObj, volume);
 	}
 }
+
+void GuestAdditions::instantiateScriptHook(Script &script) const {
+	if (getSciVersion() < SCI_VERSION_2) {
+		return;
+	}
+
+	// 64990 is the system script containing SRDialog. This script is normally
+	// used by the main Game object, but it is not loaded immediately, so we
+	// wait for it to be loaded before patching it
+	if (!ConfMan.getBool("originalsaveload") && script.getScriptNumber() == 64990) {
+		patchGameSaveRestoreSCI32(script);
+	}
+}
+
+#endif
+
+#pragma mark -
+#pragma mark Save & restore
+
+void GuestAdditions::patchGameSaveRestore() const {
+	if (ConfMan.getBool("originalsaveload") || getSciVersion() >= SCI_VERSION_2)
+		return;
+
+	patchGameSaveRestoreSCI16();
+}
+
+static const byte kSaveRestorePatch[] = {
+	0x39, 0x03,        // pushi 03
+	0x76,              // push0
+	0x38, 0xff, 0xff,  // pushi -1
+	0x76,              // push0
+	0x43, 0xff, 0x06,  // callk kRestoreGame/kSaveGame (will get changed afterwards)
+	0x48               // ret
+};
+
+static void patchKSaveRestore(SegManager *segMan, reg_t methodAddress, byte id) {
+	Script *script = segMan->getScript(methodAddress.getSegment());
+	byte *patchPtr = const_cast<byte *>(script->getBuf(methodAddress.getOffset()));
+	memcpy(patchPtr, kSaveRestorePatch, sizeof(kSaveRestorePatch));
+	patchPtr[8] = id;
+}
+
+void GuestAdditions::patchGameSaveRestoreSCI16() const {
+	const Object *gameObject = _segMan->getObject(g_sci->getGameObject());
+	const Object *gameSuperObject = _segMan->getObject(gameObject->getSuperClassSelector());
+	if (!gameSuperObject)
+		gameSuperObject = gameObject;	// happens in KQ5CD, when loading saved games before r54510
+	byte kernelIdRestore = 0;
+	byte kernelIdSave = 0;
+
+	switch (g_sci->getGameId()) {
+	case GID_HOYLE1: // gets confused, although the game doesn't support saving/restoring at all
+	case GID_HOYLE2: // gets confused, see hoyle1
+	case GID_JONES: // gets confused, when we patch us in, the game is only able to save to 1 slot, so hooking is not required
+	case GID_MOTHERGOOSE: // mother goose EGA saves/restores directly and has no save/restore dialogs
+	case GID_MOTHERGOOSE256: // mother goose saves/restores directly and has no save/restore dialogs
+		return;
+	default:
+		break;
+	}
+
+	uint16 kernelNamesSize = _kernel->getKernelNamesSize();
+	for (uint16 kernelNr = 0; kernelNr < kernelNamesSize; kernelNr++) {
+		Common::String kernelName = _kernel->getKernelName(kernelNr);
+		if (kernelName == "RestoreGame")
+			kernelIdRestore = kernelNr;
+		if (kernelName == "SaveGame")
+			kernelIdSave = kernelNr;
+		if (kernelName == "Save")
+			kernelIdSave = kernelIdRestore = kernelNr;
+	}
+
+	// Search for gameobject superclass ::restore
+	uint16 gameSuperObjectMethodCount = gameSuperObject->getMethodCount();
+	for (uint16 methodNr = 0; methodNr < gameSuperObjectMethodCount; methodNr++) {
+		uint16 selectorId = gameSuperObject->getFuncSelector(methodNr);
+		Common::String methodName = _kernel->getSelectorName(selectorId);
+		if (methodName == "restore") {
+				patchKSaveRestore(_segMan, gameSuperObject->getFunction(methodNr), kernelIdRestore);
+		} else if (methodName == "save") {
+			if (g_sci->getGameId() != GID_FAIRYTALES) {	// Fairy Tales saves automatically without a dialog
+					patchKSaveRestore(_segMan, gameSuperObject->getFunction(methodNr), kernelIdSave);
+			}
+		}
+	}
+
+	// Patch gameobject ::save for now for SCI0 - SCI1.1
+	// TODO: It seems this was never adjusted to superclass, but adjusting it now may cause
+	// issues with some game. Needs to get checked and then possibly changed.
+	const Object *patchObjectSave = gameObject;
+
+	// Search for gameobject ::save, if there is one patch that one too
+	uint16 patchObjectMethodCount = patchObjectSave->getMethodCount();
+	for (uint16 methodNr = 0; methodNr < patchObjectMethodCount; methodNr++) {
+		uint16 selectorId = patchObjectSave->getFuncSelector(methodNr);
+		Common::String methodName = _kernel->getSelectorName(selectorId);
+		if (methodName == "save") {
+			if (g_sci->getGameId() != GID_FAIRYTALES) {	// Fairy Tales saves automatically without a dialog
+					patchKSaveRestore(_segMan, patchObjectSave->getFunction(methodNr), kernelIdSave);
+			}
+			break;
+		}
+	}
+}
+
+#ifdef ENABLE_SCI32
+static const byte SRDialogPatch[] = {
+	0x76,                   // push0
+	0x59, 0x00,             // &rest 0
+	0x43, 0x57, 0x00, 0x00, // callk kScummVMSaveLoad, 0
+	0x48                    // ret
+};
+
+void GuestAdditions::patchGameSaveRestoreSCI32(Script &script) const {
+	ObjMap &objMap = script.getObjectMap();
+	for (ObjMap::iterator it = objMap.begin(); it != objMap.end(); ++it) {
+		Object &obj = it->_value;
+		if (obj.getNameSelector().isNull()) {
+			continue;
+		}
+
+		if (Common::String(_segMan->derefString(obj.getNameSelector())) != "SRDialog") {
+			continue;
+		}
+
+		const uint16 methodCount = obj.getMethodCount();
+		for (uint16 methodNr = 0; methodNr < methodCount; ++methodNr) {
+			const uint16 selectorId = obj.getFuncSelector(methodNr);
+			const Common::String methodName = _kernel->getSelectorName(selectorId);
+			if (methodName == "doit") {
+				const reg_t methodAddress = obj.getFunction(methodNr);
+				byte *patchPtr = const_cast<byte *>(script.getBuf(methodAddress.getOffset()));
+				memcpy(patchPtr, SRDialogPatch, sizeof(SRDialogPatch));
+				break;
+			}
+		}
+	}
+}
+
+reg_t GuestAdditions::kScummVMSaveLoad(EngineState *s, int argc, reg_t *argv) const {
+	const bool isSave = (bool)argv[0].toSint16();
+	int saveNo;
+
+	if (isSave) {
+		GUI::SaveLoadChooser dialog(_("Save game:"), _("Save"), true);
+		saveNo = dialog.runModalWithCurrentTarget();
+		if (saveNo != -1) {
+			reg_t descriptionId;
+			if (_segMan->isObject(argv[1])) {
+				descriptionId = readSelector(_segMan, argv[1], SELECTOR(data));
+			} else {
+				descriptionId = argv[1];
+			}
+			SciArray &description = *_segMan->lookupArray(descriptionId);
+			description.fromString(dialog.getResultString());
+		}
+	} else {
+		GUI::SaveLoadChooser dialog(_("Restore game:"), _("Restore"), false);
+		saveNo = dialog.runModalWithCurrentTarget();
+	}
+
+	if (saveNo > 0) {
+		// The autosave slot in ScummVM takes up slot 0, but in SCI the first
+		// non-autosave save game number needs to be 0, so reduce the save
+		// number here to match what would come from the normal SCI save/restore
+		// dialog. There is additional special code for handling the autosave
+		// game inside of kRestoreGame32.
+		--saveNo;
+	}
+
+	return make_reg(0, saveNo);
+}
 #endif
 
 #pragma mark -
diff --git a/engines/sci/engine/guest_additions.h b/engines/sci/engine/guest_additions.h
index a6875d43ad..ebb328c901 100644
--- a/engines/sci/engine/guest_additions.h
+++ b/engines/sci/engine/guest_additions.h
@@ -29,6 +29,8 @@ namespace Sci {
 
 struct EngineState;
 class GameFeatures;
+class Kernel;
+class Script;
 class SegManager;
 
 /**
@@ -43,7 +45,9 @@ class SegManager;
  */
 class GuestAdditions {
 public:
-	GuestAdditions(EngineState *state, GameFeatures *features);
+	GuestAdditions(EngineState *state, GameFeatures *features, Kernel *kernel);
+
+#pragma mark -
 
 	/**
 	 * Synchronises audio volume settings from ScummVM to the game. Called
@@ -65,6 +69,7 @@ public:
 private:
 	EngineState *_state;
 	GameFeatures *_features;
+	Kernel *_kernel;
 	SegManager *_segMan;
 
 	/**
@@ -117,6 +122,32 @@ public:
 	 * Guest additions hook for kDoSoundSetVolume.
 	 */
 	void kDoSoundSetVolumeHook(const reg_t soundObj, const int16 volume) const;
+
+	/**
+	 * Guest additions hook for SegManager::instantiateScript.
+	 */
+	void instantiateScriptHook(Script &script) const;
+#endif
+
+#pragma mark -
+#pragma mark Save & restore
+
+public:
+	/**
+	 * Patches game scripts to hook into the ScummVM launcher UI when a user
+	 * tries to save or restore a game from inside the game.
+	 */
+	void patchGameSaveRestore() const;
+
+private:
+	void patchGameSaveRestoreSCI16() const;
+
+#ifdef ENABLE_SCI32
+public:
+	reg_t kScummVMSaveLoad(EngineState *s, int argc, reg_t *argv) const;
+
+private:
+	void patchGameSaveRestoreSCI32(Script &script) const;
 #endif
 
 #pragma mark -
diff --git a/engines/sci/engine/kernel.h b/engines/sci/engine/kernel.h
index 3d337017af..e5b84dd089 100644
--- a/engines/sci/engine/kernel.h
+++ b/engines/sci/engine/kernel.h
@@ -491,6 +491,7 @@ reg_t kGetSaveFiles32(EngineState *s, int argc, reg_t *argv);
 reg_t kCheckSaveGame32(EngineState *s, int argc, reg_t *argv);
 reg_t kMakeSaveCatName(EngineState *s, int argc, reg_t *argv);
 reg_t kMakeSaveFileName(EngineState *s, int argc, reg_t *argv);
+reg_t kScummVMSaveLoad(EngineState *s, int argc, reg_t *argv);
 
 reg_t kSetHotRectangles(EngineState *s, int argc, reg_t *argv);
 reg_t kIsHiRes(EngineState *s, int argc, reg_t *argv);
diff --git a/engines/sci/engine/kernel_tables.h b/engines/sci/engine/kernel_tables.h
index 6f696c4a69..5dbd5a5f46 100644
--- a/engines/sci/engine/kernel_tables.h
+++ b/engines/sci/engine/kernel_tables.h
@@ -777,12 +777,13 @@ static SciKernelMapEntry s_kernelMap[] = {
 	{ MAP_CALL(RespondsTo),        SIG_EVERYWHERE,           ".i",                    NULL,            NULL },
 	{ MAP_CALL(RestartGame),       SIG_EVERYWHERE,           "",                      NULL,            NULL },
 #ifdef ENABLE_SCI32
-	{ "RestoreGame", kRestoreGame32, SIG_THRU_SCI21EARLY, SIGFOR_ALL, "[r0]i[r0]",    NULL,            NULL },
+	{ "RestoreGame", kRestoreGame32, SIG_THRU_SCI21EARLY, SIGFOR_ALL, "ri[r0]",       NULL,            NULL },
 #endif
 	{ MAP_CALL(RestoreGame),       SIG_EVERYWHERE,           "[r0]i[r0]",             NULL,            NULL },
 	{ MAP_CALL(Said),              SIG_EVERYWHERE,           "[r0]",                  NULL,            NULL },
 #ifdef ENABLE_SCI32
-	{ "SaveGame", kSaveGame32,     SIG_THRU_SCI21EARLY, SIGFOR_ALL, "[r0]i[r0][r0]",  NULL,            NULL },
+	{ "SaveGame", kSaveGame32,     SIG_THRU_SCI21EARLY, SIGFOR_ALL, "ri[r0][r0]",     NULL,            NULL },
+	{ MAP_CALL(ScummVMSaveLoad),   SIG_SCI32, SIGFOR_ALL,    "i([ro])",               NULL,            NULL },
 #endif
 	{ MAP_CALL(SaveGame),          SIG_SCI16, SIGFOR_ALL,    "[r0]i[r0](r0)",         NULL,            NULL },
 	{ MAP_CALL(ScriptID),          SIG_EVERYWHERE,           "[io](i)",               NULL,            NULL },
@@ -1263,7 +1264,7 @@ static const char *const sci2_default_knames[] = {
 	/*0x54*/ "Dummy",
 	/*0x55*/ "DeleteKey",
 	/*0x56*/ "Dummy",
-	/*0x57*/ "Dummy",
+	/*0x57*/ "ScummVMSaveLoad", // Dummy in SSCI
 	/*0x58*/ "ListAt",
 	/*0x59*/ "ListIndexOf",
 	/*0x5a*/ "ListEachElementDo",
@@ -1427,7 +1428,7 @@ static const char *const sci21_default_knames[] = {
 	/*0x54*/ "HaveMouse",
 	/*0x55*/ "SetCursor",
 	/*0x56*/ "VibrateMouse",	// Dummy in SCI3
-	/*0x57*/ "Dummy",
+	/*0x57*/ "ScummVMSaveLoad", // Dummy in SSCI
 	/*0x58*/ "Dummy",
 	/*0x59*/ "Dummy",
 	/*0x5a*/ "List",
diff --git a/engines/sci/engine/kfile.cpp b/engines/sci/engine/kfile.cpp
index 6f9aa0d998..bdf9bb69aa 100644
--- a/engines/sci/engine/kfile.cpp
+++ b/engines/sci/engine/kfile.cpp
@@ -41,6 +41,7 @@
 #include "sci/sound/audio.h"
 #include "sci/console.h"
 #ifdef ENABLE_SCI32
+#include "sci/engine/guest_additions.h"
 #include "sci/resource.h"
 #endif
 
@@ -1179,34 +1180,10 @@ reg_t kGetSaveFiles(EngineState *s, int argc, reg_t *argv) {
 #ifdef ENABLE_SCI32
 
 reg_t kSaveGame32(EngineState *s, int argc, reg_t *argv) {
-	const bool isScummVMSave = argv[0].isNull();
-	Common::String gameName = "";
-	int16 saveNo;
-	Common::String saveDescription;
-	Common::String gameVersion = (argc <= 3 || argv[3].isNull()) ? "" : s->_segMan->getString(argv[3]);
-
-	if (isScummVMSave) {
-		// ScummVM call, from a patched Game::save
-		g_sci->_soundCmd->pauseAll(true);
-		GUI::SaveLoadChooser dialog(_("Save game:"), _("Save"), true);
-		saveNo = dialog.runModalWithCurrentTarget();
-		g_sci->_soundCmd->pauseAll(false);
-
-		if (saveNo < 0) {
-			// User cancelled save
-			return NULL_REG;
-		}
-
-		saveDescription = dialog.getResultString();
-		if (saveDescription.empty()) {
-			saveDescription = dialog.createDefaultSaveDescription(saveNo);
-		}
-	} else {
-		// Native script call
-		gameName = s->_segMan->getString(argv[0]);
-		saveNo = argv[1].toSint16();
-		saveDescription = argv[2].isNull() ? "" : s->_segMan->getString(argv[2]);
-	}
+	const Common::String gameName = s->_segMan->getString(argv[0]);
+	int16 saveNo = argv[1].toSint16();
+	const Common::String saveDescription = argv[2].isNull() ? "" : s->_segMan->getString(argv[2]);
+	const Common::String gameVersion = (argc <= 3 || argv[3].isNull()) ? "" : s->_segMan->getString(argv[3]);
 
 	debugC(kDebugLevelFile, "Game name %s save %d desc %s ver %s", gameName.c_str(), saveNo, saveDescription.c_str(), gameVersion.c_str());
 
@@ -1218,9 +1195,7 @@ reg_t kSaveGame32(EngineState *s, int argc, reg_t *argv) {
 			// Autosave slot 1 is a "new game" save
 			saveNo = kNewGameId;
 		}
-	} else if (!isScummVMSave) {
-		// ScummVM save screen will give a pre-corrected save number, but native
-		// save-load will not
+	} else {
 		saveNo += kSaveIdShift;
 	}
 
@@ -1252,26 +1227,10 @@ reg_t kSaveGame32(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kRestoreGame32(EngineState *s, int argc, reg_t *argv) {
-	const bool isScummVMRestore = argv[0].isNull();
-	Common::String gameName = "";
+	const Common::String gameName = s->_segMan->getString(argv[0]);
 	int16 saveNo = argv[1].toSint16();
 	const Common::String gameVersion = argv[2].isNull() ? "" : s->_segMan->getString(argv[2]);
 
-	if (isScummVMRestore && saveNo == -1) {
-		// ScummVM call, either from lancher or a patched Game::restore
-		g_sci->_soundCmd->pauseAll(true);
-		GUI::SaveLoadChooser dialog(_("Restore game:"), _("Restore"), false);
-		saveNo = dialog.runModalWithCurrentTarget();
-		g_sci->_soundCmd->pauseAll(false);
-
-		if (saveNo < 0) {
-			// User cancelled restore
-			return s->r_acc;
-		}
-	} else {
-		gameName = s->_segMan->getString(argv[0]);
-	}
-
 	if (gameName == "Autosave" || gameName == "Autosv") {
 		if (saveNo == 0) {
 			// Autosave slot 0 is the autosave
@@ -1279,9 +1238,7 @@ reg_t kRestoreGame32(EngineState *s, int argc, reg_t *argv) {
 			// Autosave slot 1 is a "new game" save
 			saveNo = kNewGameId;
 		}
-	} else if (!isScummVMRestore) {
-		// ScummVM save screen will give a pre-corrected save number, but native
-		// save-load will not
+	} else {
 		saveNo += kSaveIdShift;
 	}
 
@@ -1377,6 +1334,10 @@ reg_t kMakeSaveFileName(EngineState *s, int argc, reg_t *argv) {
 	return argv[0];
 }
 
+reg_t kScummVMSaveLoad(EngineState *s, int argc, reg_t *argv) {
+	return g_sci->_guestAdditions->kScummVMSaveLoad(s, argc, argv);
+}
+
 #endif
 
 } // End of namespace Sci
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index f05fdc5cb9..5a60a6090f 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -1229,19 +1229,6 @@ void gamestate_restore(EngineState *s, Common::SeekableReadStream *fh) {
 		if (g_sci->_gfxScreen)
 			g_sci->_gfxScreen->clearForRestoreGame();
 	}
-#ifdef ENABLE_SCI32
-	// Delete current planes/elements of actively loaded VM, only when our ScummVM dialogs are patched in
-	// We MUST NOT delete all planes/screen items. At least Space Quest 6 has a few in memory like for example
-	// the options plane, which are not re-added and are in memory all the time right from the start of the
-	// game. Sierra SCI32 did not clear planes, only scripts cleared the ones inside planes::elements.
-	if (getSciVersion() >= SCI_VERSION_2) {
-		if (!s->_delayedRestoreFromLauncher) {
-			// Only do it, when we are restoring regulary and not from launcher
-			// As it could result in option planes etc. on the screen (happens in gk1)
-			g_sci->_gfxFrameout->syncWithScripts(false);
-		}
-	}
-#endif
 
 	s->reset(true);
 	s->saveLoadWithSerializer(ser);	// FIXME: Error handling?
diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index 12625691fd..d13d94a957 100644
--- a/engines/sci/engine/seg_manager.cpp
+++ b/engines/sci/engine/seg_manager.cpp
@@ -24,6 +24,9 @@
 #include "sci/engine/seg_manager.h"
 #include "sci/engine/state.h"
 #include "sci/engine/script.h"
+#ifdef ENABLE_SCI32
+#include "sci/engine/guest_additions.h"
+#endif
 
 namespace Sci {
 
@@ -1041,6 +1044,9 @@ int SegManager::instantiateScript(int scriptNum) {
 	scr->initializeLocals(this);
 	scr->initializeClasses(this);
 	scr->initializeObjects(this, segmentId);
+#ifdef ENABLE_SCI32
+	g_sci->_guestAdditions->instantiateScriptHook(*scr);
+#endif
 
 	return segmentId;
 }