[Scummvm-git-logs] scummvm master -> c58d619e2042780d16473568ef37719d8b9be93a
bluegr
bluegr at gmail.com
Wed Nov 28 01:23:40 CET 2018
This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
c58d619e20 SCI32: Fix QFG4 conditional void calls (#1416)
Commit: c58d619e2042780d16473568ef37719d8b9be93a
https://github.com/scummvm/scummvm/commit/c58d619e2042780d16473568ef37719d8b9be93a
Author: Vhati (tvtronix at yahoo.com)
Date: 2018-11-28T02:23:37+02:00
Commit Message:
SCI32: Fix QFG4 conditional void calls (#1416)
Fixes unsafe arithmetic in combat, bugs #10138, #10419, #10710, #10814
Supersedes commits 4dc9f0e and 01f3e6c
Changed paths:
engines/sci/engine/script_patches.cpp
engines/sci/engine/workarounds.cpp
diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index bfa0b33..d847644 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -7823,6 +7823,63 @@ static const uint16 qfg4SetScalerPatch[] = {
PATCH_END
};
+// In QFG4, the kernel func SetNowSeen() returns void - meaning it doesn't
+// modify the accumulator to store a result. Yet math was performed on it!
+//
+// The function updates boundary box properties of a given View object, prior
+// to collision tests. IF (collision detection is warranted, and IF those tests
+// are true), THEN respond to the collision.
+//
+// SetNowSeen() was inserted in the middle of the IF block's list of conditions.
+// That way it can be short-circuited, and it runs before the tests.
+//
+// Problem: void functions make no promise about their truth value. After the
+// call, acc will be *whatever* happened to already be in there. This is bad.
+//
+// "(| (SetNowSeen horror) $0001)"
+//
+// Someone wrapped the func in a bitwise OR against 1. Thus *every* value is
+// guaranteed to become non-zero, always true. And the IF block won't break.
+//
+// In later SCI2 versions, SetNowSeen() would change to return a boolean.
+//
+// Whether a lucky confusion or ugly hack, the wrapped void IF condition works.
+// When an object leaks into the accumulator. SSCI doesn't mind OR'ing it, too.
+// ScummVM detects unsafe arithmetic and crashes. ScummVM needs proper numbers.
+//
+// "Invalid arithmetic operation (bitwise OR - params: 002e:1694 and 0000:0001)"
+//
+// We leave the OR wrapper. When the call returns, we manually feed the OR a
+// literal 1. Same effect. The IF block goes on evaluating conditions.
+//
+// Wraith, Vorpal Bunny, and Badder scripts are not affected.
+//
+// Applies to at least: English CD, English floppy, German floppy
+// Responsible method:
+// (ego1, combat hero) Script 41 - xSlash::doit(), xDuck::doit(), xParryLow::doit()
+// (ego1, combat hero) Script 810 - slash::doit()
+// (Revenant) Script 830 - revenantForward::doit()
+// (Wyvern) Script 835 - doRSlash::doit(), doLSlash::doit(), tailAttack::doit()
+// (Chernovy) Script 840 - doLSlash::doit(), doRSlash::doit()
+// (Pit Horror) Script 855 - wipeSpell::doit()
+// (Necrotaur) Script 870 - attackLeft::doit(), attackRight::doit(),
+// headAttack::doit(), hurtMyself::changeState(1)
+// Fixes bug: #10138, #10419, #10710, #10814
+static const uint16 qfg4ConditionalVoidSignature[] = {
+ SIG_MAGICDWORD,
+ 0x43, 0x0a, SIG_UINT16(0x00002), // callk 02 (SetNowSeen stackedView)
+ 0x36, // push (void func didn't set acc!)
+ 0x35, 0x01, // ldi 1d
+ 0x14, // or (whatever that was, make it non-zero)
+ SIG_END
+};
+
+static const uint16 qfg4ConditionalVoidPatch[] = {
+ PATCH_ADDTOOFFSET(+4), //
+ 0x78, // push1 (Feed OR a literal 1)
+ PATCH_END
+};
+
// script, description, signature patch
static const SciScriptPatcherEntry qfg4Signatures[] = {
{ true, 0, "prevent autosave from deleting save games", 1, qg4AutosaveSignature, qg4AutosavePatch },
@@ -7831,6 +7888,7 @@ static const SciScriptPatcherEntry qfg4Signatures[] = {
{ true, 7, "fix consecutive moonrises", 1, qfg4MoonriseSignature, qfg4MoonrisePatch },
{ true, 10, "fix setLooper calls (2/2)", 2, qg4SetLooperSignature2, qg4SetLooperPatch2 },
{ true, 31, "fix setScaler calls", 1, qfg4SetScalerSignature, qfg4SetScalerPatch },
+ { true, 41, "fix conditional void calls", 3, qfg4ConditionalVoidSignature, qfg4ConditionalVoidPatch },
{ true, 83, "fix incorrect array type", 1, qfg4TrapArrayTypeSignature, qfg4TrapArrayTypePatch },
{ true, 83, "fix incorrect array type (floppy)", 1, qfg4TrapArrayTypeFloppySignature, qfg4TrapArrayTypeFloppyPatch },
{ true, 320, "fix pathfinding at the inn", 1, qg4InnPathfindingSignature, qg4InnPathfindingPatch },
@@ -7844,6 +7902,12 @@ static const SciScriptPatcherEntry qfg4Signatures[] = {
{ true, 633, "fix stairway pathfinding", 1, qfg4StairwayPathfindingSignature, qfg4StairwayPathfindingPatch },
{ true, 800, "fix setScaler calls", 1, qfg4SetScalerSignature, qfg4SetScalerPatch },
{ true, 803, "fix sliding down slope", 1, qfg4SlidingDownSlopeSignature, qfg4SlidingDownSlopePatch },
+ { true, 810, "fix conditional void calls", 1, qfg4ConditionalVoidSignature, qfg4ConditionalVoidPatch },
+ { true, 830, "fix conditional void calls", 2, qfg4ConditionalVoidSignature, qfg4ConditionalVoidPatch },
+ { true, 835, "fix conditional void calls", 3, qfg4ConditionalVoidSignature, qfg4ConditionalVoidPatch },
+ { true, 840, "fix conditional void calls", 2, qfg4ConditionalVoidSignature, qfg4ConditionalVoidPatch },
+ { true, 855, "fix conditional void calls", 1, qfg4ConditionalVoidSignature, qfg4ConditionalVoidPatch },
+ { true, 870, "fix conditional void calls", 5, qfg4ConditionalVoidSignature, qfg4ConditionalVoidPatch },
{ true, 64990, "increase number of save games (1/2)", 1, sci2NumSavesSignature1, sci2NumSavesPatch1 },
{ true, 64990, "increase number of save games (2/2)", 1, sci2NumSavesSignature2, sci2NumSavesPatch2 },
{ true, 64990, "disable change directory button", 1, sci2ChangeDirSignature, sci2ChangeDirPatch },
diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 13f3d36..4fbe093 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -88,8 +88,6 @@ const SciWorkaroundEntry arithmeticWorkarounds[] = {
{ GID_QFG2, 200, 200, 0, "astro", "messages", NULL, 0, 0, { WORKAROUND_FAKE, 0 } }, // op_lsi: when getting asked for your name by the astrologer - bug #5152
{ GID_QFG3, 780, 999, 0, "", "export 6", NULL, 0, 0, { WORKAROUND_FAKE, 0 } }, // op_add: trying to talk to yourself at the top of the giant tree - bug #6692
{ GID_QFG4, 710,64941, 0, "RandCycle", "doit", NULL, 0, 0, { WORKAROUND_FAKE, 1 } }, // op_gt: when the tentacle appears in the third room of the caves
- { GID_QFG4, -1, 870, 0, "hurtMyself", "changeState", NULL, 0, 0, { WORKAROUND_FAKE, 1 } }, // op_or: when hitting a necrotaur with a sword as a paladin
- { GID_QFG4, -1, 830, 0, "revenantForward", "doit", NULL, 0, 0, { WORKAROUND_FAKE, 1 } }, // op_or: when starting a fight with a revenant
{ GID_TORIN, 51400,64928, 0, "Blink", "init", NULL, 0, 0, { WORKAROUND_FAKE, 1 } }, // op_div: when Lycentia knocks Torin out after he removes her collar
SCI_WORKAROUNDENTRY_TERMINATOR
};
More information about the Scummvm-git-logs
mailing list