SCI: implementing workarounds for kernel calls, removing original code workaround for kDisposeScript / qfg1 room 64 and adding it to workaround table

svn-id: r50520

4 files changed, 129 insertions, 88 deletions

diff --git a/engines/sci/engine/kernel.cpp b/engines/sci/engine/kernel.cpp
index 7a2e34a207..586c552770 100644
--- a/engines/sci/engine/kernel.cpp
+++ b/engines/sci/engine/kernel.cpp
@@ -206,6 +206,12 @@ static const char *s_defaultKernelNames[] = {
 // i* -> optional multiple integers
 // .* -> any parameters afterwards (or none)
 
+//    gameID,       scriptNr,lvl,         object-name, method-name,    call,index,replace
+static const SciWorkaroundEntry kDisposeScript_workarounds[] = {
+	{ GID_QFG1,           64,  0,               "rm64", "dispose",        -1,    0, { 1,    0 } }, // parameter 0 is an object when leaving graveyard
+	SCI_WORKAROUNDENTRY_TERMINATOR
+};
+
 struct SciKernelMapEntry {
 	const char *name;
 	KernelFunc *function;
@@ -216,7 +222,7 @@ struct SciKernelMapEntry {
 
 	const char *signature;
 	const char *subSignatures; // placeholder
-	const char *workarounds; // placeholder
+	const SciWorkaroundEntry *workarounds;
 };
 
 #define SIG_SCIALL  SCI_VERSION_NONE, SCI_VERSION_NONE
@@ -243,7 +249,7 @@ static SciKernelMapEntry s_kernelMap[] = {
     { MAP_CALL(UnLoad),            SIG_EVERYWHERE,           "i.*",                  NULL,            NULL },
 	// ^^ FIXME - Work around SQ1 bug, when exiting the Ulence flats bar
     { MAP_CALL(ScriptID),          SIG_EVERYWHERE,           "Ioi*",                 NULL,            NULL },
-    { MAP_CALL(DisposeScript),     SIG_EVERYWHERE,           "Oii*",                 NULL,            NULL },
+    { MAP_CALL(DisposeScript),     SIG_EVERYWHERE,           "ii*",                  NULL,            kDisposeScript_workarounds },
 	// ^^ FIXME - Work around QfG1 bug
     { MAP_CALL(Clone),             SIG_EVERYWHERE,           "o",                    NULL,            NULL },
     { MAP_CALL(DisposeClone),      SIG_EVERYWHERE,           "o",                    NULL,            NULL },
@@ -664,6 +670,7 @@ void Kernel::mapFunctions() {
 			if (kernelMap->function) {
 				_kernelFuncs[functNr].func = kernelMap->function;
 				_kernelFuncs[functNr].signature = compileKernelSignature(kernelMap->signature);
+				_kernelFuncs[functNr].workarounds = kernelMap->workarounds;
 				_kernelFuncs[functNr].isDummy = false;
 				++mapped;
 			} else {
diff --git a/engines/sci/engine/kernel.h b/engines/sci/engine/kernel.h
index 6f7b0d569d..b7cd9058e5 100644
--- a/engines/sci/engine/kernel.h
+++ b/engines/sci/engine/kernel.h
@@ -115,11 +115,25 @@ enum {
 /* Generic description: */
 typedef reg_t KernelFunc(EngineState *s, int argc, reg_t *argv);
 
+struct SciWorkaroundEntry {
+	SciGameId gameId;
+	int scriptNr;
+	int16 inheritanceLevel;
+	const char *objectName;
+	const char *methodName;
+	int localCallOffset;
+	int index;
+	reg_t newValue;
+};
+
+#define SCI_WORKAROUNDENTRY_TERMINATOR { (SciGameId)0,       -1,  0,                 NULL, NULL,             -1,    0, { 0,   0 } }
+
 struct KernelFuncWithSignature {
 	KernelFunc *func; /**< The actual function */
-	char *signature;  /**< KernelFunc signature */
 	Common::String origName; /**< Original name, in case we couldn't map it */
 	bool isDummy;
+	char *signature;
+	const SciWorkaroundEntry *workarounds;
 };
 
 enum AutoDetectedFeatures {
diff --git a/engines/sci/engine/kscripts.cpp b/engines/sci/engine/kscripts.cpp
index 8f41fbad2e..4f4df7d875 100644
--- a/engines/sci/engine/kscripts.cpp
+++ b/engines/sci/engine/kscripts.cpp
@@ -228,10 +228,6 @@ reg_t kScriptID(EngineState *s, int argc, reg_t *argv) {
 reg_t kDisposeScript(EngineState *s, int argc, reg_t *argv) {
 	int script = argv[0].offset;
 
-	// Work around QfG1 graveyard bug
-	if (argv[0].segment)
-		return s->r_acc;
-
 	SegmentId id = s->_segMan->getScriptSegment(script);
 	Script *scr = s->_segMan->getScriptIfLoaded(id);
 	if (scr) {
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index a16e048713..46fc46da63 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -263,35 +263,100 @@ static bool validate_variable(reg_t *r, reg_t *stack_base, int type, int max, in
 	return true;
 }
 
-struct UninitializedReadWorkaround {
-	SciGameId gameId;
+struct SciTrackOriginReply {
 	int scriptNr;
-	int16 inheritanceLevel;
-	const char *objectName;
-	const char *methodName;
+	Common::String objectName;
+	Common::String methodName;
 	int localCallOffset;
-	int index;
-	uint16 newValue;
 };
 
-//    gameID,       scriptNr,lvl,         object-name, method-name,    call,index,replace
-static const UninitializedReadWorkaround uninitializedReadWorkarounds[] = {
-	{ GID_LAURABOW2,      24,  0,              "gcWin", "open",           -1,    5, 0xf }, // is used as priority for game menu
-	{ GID_FREDDYPHARKAS,  24,  0,              "gcWin", "open",           -1,    5, 0xf }, // is used as priority for game menu
-	{ GID_FREDDYPHARKAS,  31,  0,            "quitWin", "open",           -1,    5, 0xf }, // is used as priority for game menu
-	{ GID_LSL1,          720,  0,              "rm720", "init",           -1,    0,   0 }, // age check room
-	{ GID_LSL3,          997,  0,         "TheMenuBar", "handleEvent",    -1,    1, 0xf }, // when setting volume the first time, this temp is used to set volume on entry (normally it would have been initialized to 's')
-	{ GID_LSL6,          928,  1,           "Narrator", "startText",      -1,    0,   0 }, // used by various objects that are even translated in foreign versions, that's why we use the base-class
-	{ GID_ISLANDBRAIN,   140,  0,              "piece", "init",           -1,    3,   1 }, // first puzzle right at the start, some initialization variable. bnt is done on it, and it should be non-0
-	{ GID_ISLANDBRAIN,   268,  0,          "anElement", "select",         -1,    0,   0 }, // elements puzzle, gets used before super TextIcon
-	{ GID_KQ5,             0,  0,                   "", "export 29",      -1,    3,   0 }, // called when playing harp for the harpies, is used for kDoAudio
-	{ GID_KQ5,            25,  0,              "rm025", "doit",           -1,    0,   0 }, // inside witch forest, where the walking rock is
-	{ GID_SQ1,           703,  0,                   "", "export 1",       -1,    0,   0 }, // sub that's called from several objects while on sarien battle cruiser
-	{ GID_SQ1,           703,  0,         "firePulsar", "changeState", 0x18a,    0,   0 }, // export 1, but called locally (when shooting at aliens)
-	{ GID_SQ4,           928,  0,           "Narrator", "startText",      -1, 1000,   1 }, // sq4cd: method returns this to the caller
-	{ GID_SQ6,             0,  0,               "Game", "init",           -1,    2,   0 },
-	{ GID_SQ6,         64950,  0,               "View", "handleEvent",    -1,    0,   0 },
-	{ (SciGameId)0,       -1,  0,                 NULL, NULL,             -1,    0,   0 }
+static reg_t trackOriginAndFindWorkaround(int index, const SciWorkaroundEntry *workaroundList, bool &workaroundFound, SciTrackOriginReply *trackOrigin) {
+	EngineState *state = g_sci->getEngineState();
+	ExecStack *lastCall = state->xs;
+	Script *local_script = state->_segMan->getScriptIfLoaded(lastCall->local_segment);
+	int curScriptNr = local_script->getScriptNumber();
+
+	if (lastCall->debugLocalCallOffset != -1) {
+		// if lastcall was actually a local call search back for a real call
+		Common::List<ExecStack>::iterator callIterator = state->_executionStack.end();
+		while (callIterator != state->_executionStack.begin()) {
+			callIterator--;
+			ExecStack loopCall = *callIterator;
+			if ((loopCall.debugSelector != -1) || (loopCall.debugExportId != -1)) {
+				lastCall->debugSelector = loopCall.debugSelector;
+				lastCall->debugExportId = loopCall.debugExportId;
+				break;
+			}
+		}
+	}
+
+	Common::String curObjectName = state->_segMan->getObjectName(lastCall->sendp);
+	Common::String curMethodName;
+	const SciGameId gameId = g_sci->getGameId();
+
+	if (lastCall->type == EXEC_STACK_TYPE_CALL) {
+		if (lastCall->debugSelector != -1) {
+			curMethodName = g_sci->getKernel()->getSelectorName(lastCall->debugSelector);
+		} else if (lastCall->debugExportId != -1) {
+			curObjectName = "";
+			curMethodName = curMethodName.printf("export %d", lastCall->debugExportId);
+		}
+	}
+
+	if (workaroundList) {
+		// Search if there is a workaround for this one
+		const SciWorkaroundEntry *workaround;
+		int16 inheritanceLevel = 0;
+		Common::String searchObjectName = curObjectName;
+		reg_t searchObject = lastCall->sendp;
+		do {
+			workaround = workaroundList;
+			while (workaround->objectName) {
+				if (workaround->gameId == gameId && workaround->scriptNr == curScriptNr && (workaround->inheritanceLevel == inheritanceLevel) && (workaround->objectName == searchObjectName)
+						&& workaround->methodName == curMethodName && workaround->localCallOffset == lastCall->debugLocalCallOffset && workaround->index == index) {
+					// Workaround found
+					workaroundFound = true;
+					return workaround->newValue;
+				}
+				workaround++;
+			}
+
+			// Go back to the parent
+			inheritanceLevel++;
+			searchObject = state->_segMan->getObject(searchObject)->getSuperClassSelector();
+			if (!searchObject.isNull())
+				searchObjectName = state->_segMan->getObjectName(searchObject);
+		} while (!searchObject.isNull()); // no parent left?
+	}
+
+	// give caller origin data
+	trackOrigin->objectName = curObjectName;
+	trackOrigin->methodName = curMethodName;
+	trackOrigin->scriptNr = curScriptNr;
+	trackOrigin->localCallOffset = lastCall->debugLocalCallOffset;
+
+	workaroundFound = false;
+	return make_reg(0xFFFF, 0xFFFF);
+}
+
+//    gameID,       scriptNr,lvl,         object-name, method-name,    call, index,   replace
+static const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
+	{ GID_LAURABOW2,      24,  0,              "gcWin", "open",           -1,    5, { 0, 0xf } }, // is used as priority for game menu
+	{ GID_FREDDYPHARKAS,  24,  0,              "gcWin", "open",           -1,    5, { 0, 0xf } }, // is used as priority for game menu
+	{ GID_FREDDYPHARKAS,  31,  0,            "quitWin", "open",           -1,    5, { 0, 0xf } }, // is used as priority for game menu
+	{ GID_LSL1,          720,  0,              "rm720", "init",           -1,    0, { 0,   0 } }, // age check room
+	{ GID_LSL3,          997,  0,         "TheMenuBar", "handleEvent",    -1,    1, { 0, 0xf } }, // when setting volume the first time, this temp is used to set volume on entry (normally it would have been initialized to 's')
+	{ GID_LSL6,          928,  1,           "Narrator", "startText",      -1,    0, { 0,   0 } }, // used by various objects that are even translated in foreign versions, that's why we use the base-class
+	{ GID_ISLANDBRAIN,   140,  0,              "piece", "init",           -1,    3, { 0,   1 } }, // first puzzle right at the start, some initialization variable. bnt is done on it, and it should be non-0
+	{ GID_ISLANDBRAIN,   268,  0,          "anElement", "select",         -1,    0, { 0,   0 } }, // elements puzzle, gets used before super TextIcon
+	{ GID_KQ5,             0,  0,                   "", "export 29",      -1,    3, { 0,   0 } }, // called when playing harp for the harpies, is used for kDoAudio
+	{ GID_KQ5,            25,  0,              "rm025", "doit",           -1,    0, { 0,   0 } }, // inside witch forest, where the walking rock is
+	{ GID_SQ1,           703,  0,                   "", "export 1",       -1,    0, { 0,   0 } }, // sub that's called from several objects while on sarien battle cruiser
+	{ GID_SQ1,           703,  0,         "firePulsar", "changeState", 0x18a,    0, { 0,   0 } }, // export 1, but called locally (when shooting at aliens)
+	{ GID_SQ4,           928,  0,           "Narrator", "startText",      -1, 1000, { 0,   1 } }, // sq4cd: method returns this to the caller
+	{ GID_SQ6,             0,  0,                "SQ6", "init",           -1,    2, { 0,   0 } },
+	{ GID_SQ6,         64950,  0,               "View", "handleEvent",    -1,    0, { 0,   0 } },
+	SCI_WORKAROUNDENTRY_TERMINATOR
 };
 
 static reg_t validate_read_var(reg_t *r, reg_t *stack_base, int type, int max, int index, int line, reg_t default_value) {
@@ -299,61 +364,11 @@ static reg_t validate_read_var(reg_t *r, reg_t *stack_base, int type, int max, i
 		if (type == VAR_TEMP && r[index].segment == 0xffff) {
 			// Uninitialized read on a temp
 			//  We need to find correct replacements for each situation manually
-			EngineState *state = g_sci->getEngineState();
-			ExecStack *lastCall = state->xs;
-			Script *local_script = state->_segMan->getScriptIfLoaded(lastCall->local_segment);
-			int curScriptNr = local_script->getScriptNumber();
-
-			if (lastCall->debugLocalCallOffset != -1) {
-				// if lastcall was actually a local call search back for a real call
-				Common::List<ExecStack>::iterator callIterator = state->_executionStack.end();
-				while (callIterator != state->_executionStack.begin()) {
-					callIterator--;
-					ExecStack loopCall = *callIterator;
-					if ((loopCall.debugSelector != -1) || (loopCall.debugExportId != -1)) {
-						lastCall->debugSelector = loopCall.debugSelector;
-						lastCall->debugExportId = loopCall.debugExportId;
-						break;
-					}
-				}
-			}
-
-			Common::String curObjectName = state->_segMan->getObjectName(lastCall->sendp);
-			Common::String curMethodName;
-			const SciGameId gameId = g_sci->getGameId();
-
-			if (lastCall->type == EXEC_STACK_TYPE_CALL) {
-				if (lastCall->debugSelector != -1) {
-					curMethodName = g_sci->getKernel()->getSelectorName(lastCall->debugSelector);
-				} else if (lastCall->debugExportId != -1) {
-					curObjectName = "";
-					curMethodName = curMethodName.printf("export %d", lastCall->debugExportId);
-				}
-			}
-
-			// Search if this is a known uninitialized read
-			const UninitializedReadWorkaround *workaround;
-			int16 inheritanceLevel = 0;
-			Common::String searchObjectName = curObjectName;
-			reg_t searchObject = lastCall->sendp;
-			do {
-				workaround = uninitializedReadWorkarounds;
-				while (workaround->objectName) {
-					if (workaround->gameId == gameId && workaround->scriptNr == curScriptNr && (workaround->inheritanceLevel == inheritanceLevel) && (workaround->objectName == searchObjectName)
-							&& workaround->methodName == curMethodName && workaround->localCallOffset == lastCall->debugLocalCallOffset && workaround->index == index) {
-						// Workaround found
-						r[index] = make_reg(0, workaround->newValue);
-						return r[index];
-					}
-					workaround++;
-				}
-				// Go back to the parent
-				inheritanceLevel++;
-				searchObject = state->_segMan->getObject(searchObject)->getSuperClassSelector();
-				if (!searchObject.isNull())
-					searchObjectName = state->_segMan->getObjectName(searchObject);
-			} while (!searchObject.isNull()); // no parent left?
-			error("Uninitialized read for temp %d from method %s::%s (script %d, localCall %x)", index, curObjectName.c_str(), curMethodName.c_str(), curScriptNr, lastCall->debugLocalCallOffset);
+			bool workaroundFound;
+			SciTrackOriginReply originReply;
+			r[index] = trackOriginAndFindWorkaround(index, uninitializedReadWorkarounds, workaroundFound, &originReply);
+			if (!workaroundFound)
+				error("Uninitialized read for temp %d from method %s::%s (script %d, localCall %x)", index, originReply.objectName.c_str(), originReply.methodName.c_str(), originReply.scriptNr, originReply.localCallOffset);
 		}
 		return r[index];
 	} else
@@ -770,7 +785,16 @@ static void callKernelFunc(EngineState *s, int kernelFuncNr, int argc) {
 
 	if (kernelFunc.signature
 			&& !g_sci->getKernel()->signatureMatch(kernelFunc.signature, argc, s->xs->sp + 1)) {
-		error("[VM] Invalid arguments to kernel call k%s (funcNr %x)", g_sci->getKernel()->getKernelName(kernelFuncNr).c_str(), kernelFuncNr);
+		// signature mismatch, check if a workaround is available
+		bool workaroundFound;
+		SciTrackOriginReply originReply;
+		reg_t workaround;
+		workaround = trackOriginAndFindWorkaround(0, kernelFunc.workarounds, workaroundFound, &originReply);
+		if (!workaroundFound)
+			error("[VM] k%s (%x) signature mismatch via method %s::%s (script %d, localCall %x)", g_sci->getKernel()->getKernelName(kernelFuncNr).c_str(), kernelFuncNr, originReply.objectName.c_str(), originReply.methodName.c_str(), originReply.scriptNr, originReply.localCallOffset);
+		// FIXME: implement some real workaround type logic - ignore call, still do call etc.
+		if (!workaround.segment)
+			return;
 	}
 
 	reg_t *argv = s->xs->sp + 1;