From 7595e7c1bec5e56e9ddaabea4b6af43423e8e002 Mon Sep 17 00:00:00 2001 From: Martin Kiewitz Date: Wed, 30 Jun 2010 09:47:04 +0000 Subject: SCI: implementing workarounds for kernel calls, removing original code workaround for kDisposeScript / qfg1 room 64 and adding it to workaround table svn-id: r50520 --- engines/sci/engine/kernel.cpp | 11 ++- engines/sci/engine/kernel.h | 16 +++- engines/sci/engine/kscripts.cpp | 4 - engines/sci/engine/vm.cpp | 186 +++++++++++++++++++++++----------------- 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::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::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; -- cgit v1.2.3