[Scummvm-git-logs] scummvm master -> d10fd9f1241e9b4069875207aed03bd6ae578b98

sluicebox noreply at scummvm.org
Wed Dec 21 20:27:12 UTC 2022


This automated email contains information about 5 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
ae5db52c2e SCI: Remove speed test detector
2c01722f9b SCI: Fix QFG1VGA Sierra logo animation speed
76044dc51a SCI: Fix CNICK_LONGBOW uninit read workaround
c7099d137e SCI: Add detection for another GK1 DEMO variant
d10fd9f124 SCI: Revert disabling LB2 control panel in tunnel


Commit: ae5db52c2e5f9d95b44f15b8b05d0ee392e4a2a7
    https://github.com/scummvm/scummvm/commit/ae5db52c2e5f9d95b44f15b8b05d0ee392e4a2a7
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2022-12-21T12:26:51-08:00

Commit Message:
SCI: Remove speed test detector

This heuristic was originally how all SCI16 speed tests were handled.
It has been gradually replaced with script patches, until all games
were patched in: ea48986006a1d85b5302f0c3d2ac828e29a66813

At the time, I left this in because it had the benefit of speeding up
the SCI11 test variants so that they didn't produce a startup delay.
Now we know that this heuristic has been identifying regular rooms as
speed tests and unthrottling them too, causing unintended effects.
Some of this behavior was masked by fast-cast throttling occurring
everywhere, until: e09010f7d8b7cc30ae3c8477d1a8b61b24d96306

For example, the QFG1VGA Sierra logo animation changes speed and runs
very fast as soon as the sparkle is finished. The Longbow map rooms
were also detected as speed test rooms and animated too fast. Cast-less
rooms like LB2's title screen run unthrottled and consume CPU.

There are only a few SCI11 speed test rooms, so now they're explicitly
unthrottled in kGameIsRestarting with the other throttling exceptions.

Changed paths:
    engines/sci/engine/kmisc.cpp
    engines/sci/engine/script_patches.cpp
    engines/sci/graphics/animate.cpp
    engines/sci/graphics/animate.h


diff --git a/engines/sci/engine/kmisc.cpp b/engines/sci/engine/kmisc.cpp
index cbe7de8c62c..95c447ea5b6 100644
--- a/engines/sci/engine/kmisc.cpp
+++ b/engines/sci/engine/kmisc.cpp
@@ -137,16 +137,20 @@ reg_t kGameIsRestarting(EngineState *s, int argc, reg_t *argv) {
 			neededSleep = 60;
 		}
 		break;
-	case GID_LONGBOW:
-		// LONWBOW map rooms have no cast, so kAnimate doesn't trigger throttling
-		// because it thinks it's a speed test room. See: GfxAnimate::throttleSpeed.
-		// This causes the palette animation to run unthrottled. Sierra also attempts
-		// to throttle these screens in a conflicting way based on speed test results,
-		// and we patch that out.
-		if (s->currentRoomNumber() == 100 || s->currentRoomNumber() == 260) {
-			s->_throttleTrigger = true;
-		}
-		break;
+
+	// Don't throttle SCI1.1 speed test rooms. Prevents delays at startup.
+	// We generically patch these scripts to calculate a passing result,
+	// but each script performs a different test, so to speed them all up
+	// it's easier to just let them run unthrottled. See: sci11SpeedTestPatch
+	case GID_ECOQUEST2:     if (s->currentRoomNumber() ==  10) s->_throttleTrigger = false; break;
+	case GID_FREDDYPHARKAS: if (s->currentRoomNumber() ==  28) s->_throttleTrigger = false; break;
+	case GID_GK1DEMO:       if (s->currentRoomNumber() ==  17) s->_throttleTrigger = false; break;
+	case GID_KQ5:           if (s->currentRoomNumber() ==  99) s->_throttleTrigger = false; break;
+	case GID_KQ6:           if (s->currentRoomNumber() ==  99) s->_throttleTrigger = false; break;
+	case GID_LAURABOW2:     if (s->currentRoomNumber() ==  28) s->_throttleTrigger = false; break;
+	case GID_LSL6:          if (s->currentRoomNumber() ==  99) s->_throttleTrigger = false; break;
+	case GID_QFG1VGA:       if (s->currentRoomNumber() == 299) s->_throttleTrigger = false; break;
+
 	default:
 		break;
 	}
diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index e8afb8da5e9..5c4ec0e469e 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -555,9 +555,6 @@ static const uint16 torinLarry7NumSavesPatch[] = {
 // We disable speed tests by patching them to return fixed results so that all
 //  graphics are enabled and games behave consistently. This also fixes the bug
 //  where tests fail on fast CPUs because their scores overflow. (bug #13529)
-//  We still have a heuristic in GfxAnimate::throttleSpeed() that attempts to
-//  detect tests and unthrottle the engine so that they score well. That should
-//  reasonably handle any games or versions that haven't been patched yet.
 //
 // Note that these patches are only for SCI16 games; the test was rewritten for
 //  SCI32 and we have separate patches for those.
@@ -618,6 +615,9 @@ static const uint16 sci01SpeedTestLocalPatch[] = {
 
 // The second generation of speed tests measured how much time it takes to do a
 //  fixed amount of work. Patch the duration calculation to always be zero.
+//  We also disable speed throttling in these rooms in kGameIsRestarting, but
+//  that's just to prevent the delay these tests create. Each game's script does
+//  different work, so that part can't be generically patched.
 static const uint16 sci11SpeedTestSignature[] = {
 	0x76,                               // push0
 	0x43, 0x42, 0x00,                   // callk GetTime 00
diff --git a/engines/sci/graphics/animate.cpp b/engines/sci/graphics/animate.cpp
index 63ce5761ef4..73f1d548e23 100644
--- a/engines/sci/graphics/animate.cpp
+++ b/engines/sci/graphics/animate.cpp
@@ -710,47 +710,7 @@ void GfxAnimate::kernelAnimate(reg_t listReference, bool cycle, int argc, reg_t
 	_ports->setPort(oldPort);
 
 	// Now trigger speed throttler
-	throttleSpeed();
-}
-
-void GfxAnimate::throttleSpeed() {
-	switch (_lastCastData.size()) {
-	case 0:
-		// No entries drawn -> no speed throttler triggering
-		break;
-	case 1: {
-
-		// One entry drawn -> check if that entry was a speed benchmark view, if not enable speed throttler
-		AnimateEntry *onlyCast = &_lastCastData[0];
-
-		// Note that we now use script patches disable speed tests and avoid their overflow errors.
-		// This heuristic is still useful for any games or versions that haven't been patched yet
-		// and it does make some of the patched tests complete faster and reduce startup delay.
-
-		// first loop and first cel used?
-		if ((onlyCast->loopNo == 0) && (onlyCast->celNo == 0)) {
-			// and that cel has a known speed benchmark resolution
-			int16 onlyHeight = onlyCast->celRect.height();
-			int16 onlyWidth = onlyCast->celRect.width();
-			if (((onlyWidth == 12) && (onlyHeight == 35)) || // regular benchmark view ("fred", "Speedy", "ego")
-				((onlyWidth == 29) && (onlyHeight == 45)) || // King's Quest 5 french "fred"
-				((onlyWidth == 1) && (onlyHeight == 5)) || // Freddy Pharkas "fred"
-				((onlyWidth == 1) && (onlyHeight == 1))) { // Laura Bow 2 Talkie
-				// check further that there is only one cel in that view
-				GfxView *onlyView = _cache->getView(onlyCast->viewId);
-				if ((onlyView->getLoopCount() == 1) && (onlyView->getCelCount(0))) {
-					return;
-				}
-			}
-		}
-		_s->_throttleTrigger = true;
-		break;
-	}
-	default:
-		// More than 1 entry drawn -> time for speed throttling
-		_s->_throttleTrigger = true;
-		break;
-	}
+	_s->_throttleTrigger = true;
 }
 
 void GfxAnimate::addToPicSetPicNotValid() {
diff --git a/engines/sci/graphics/animate.h b/engines/sci/graphics/animate.h
index 6ecb0834ca1..ea91ad6e32d 100644
--- a/engines/sci/graphics/animate.h
+++ b/engines/sci/graphics/animate.h
@@ -117,7 +117,6 @@ private:
 
 	void addToPicSetPicNotValid();
 	void animateShowPic();
-	void throttleSpeed();
 	void adjustInvalidCels(GfxView *view, AnimateList::iterator it);
 	void processViewScaling(GfxView *view, AnimateList::iterator it);
 	void setNsRect(GfxView *view, AnimateList::iterator it);


Commit: 2c01722f9bda8996de2124095ebc988cc2a87947
    https://github.com/scummvm/scummvm/commit/2c01722f9bda8996de2124095ebc988cc2a87947
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2022-12-21T12:26:51-08:00

Commit Message:
SCI: Fix QFG1VGA Sierra logo animation speed

The previous commit fixed inconsistent animation speed in
this room due to our throttling.

This patch fixes slow animation due to Sierra's throttling.

Changed paths:
    engines/sci/engine/script_patches.cpp


diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index 5c4ec0e469e..1bab14c9aa2 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -14332,6 +14332,27 @@ static const uint16 qfg1vgaPatchDrinkWaterMessage[] = {
 	PATCH_END
 };
 
+// QFG1VGA throttles the Sierra logo palette animation to once every third
+//  game cycle. This runs very slow when combined with our own throttling.
+//  This appears to be the only game that does this, so we just disable it.
+//
+// Applies to: All versions
+// Responsible method: LogoRoom:doit
+static const uint16 qfg1vgaSignatureSierraLogoSpeed[] = {
+	SIG_MAGICDWORD,
+	0x0a,                                   // mod
+	0x36,                                   // push
+	0x35, 0x00,                             // ldi 00
+	0x1a,                                   // eq?
+	SIG_END
+};
+
+static const uint16 qfg1vgaPatchSierraLogoSpeed[] = {
+	PATCH_ADDTOOFFSET(+1),
+	0x76,                                   // push0
+	PATCH_END
+};
+
 //          script, description,                                      signature                            patch
 static const SciScriptPatcherEntry qfg1vgaSignatures[] = {
 	{  true,     0, "inventory weight warning",                    1, qfg1vgaSignatureInventoryWeightWarn, qfg1vgaPatchInventoryWeightWarn },
@@ -14357,6 +14378,7 @@ static const SciScriptPatcherEntry qfg1vgaSignatures[] = {
 	{  true,   340, "dagnabit inventory fix",                      1, qfg1vgaSignatureDagnabitInventory,   qfg1vgaPatchDagnabitInventory },
 	{  true,   340, "mac: dagnabit icon bar fix",                  1, qfg1vgaSignatureMacDagnabitIconBar,  qfg1vgaPatchMacDagnabitIconBar },
 	{  true,   603, "mac: logo mouse-up fix",                      1, qfg1vgaSignatureMacLogoIntroSkip,    qfg1vgaPatchMacLogoIntroSkip },
+	{  true,   603, "sierra logo speed",                           1, qfg1vgaSignatureSierraLogoSpeed,     qfg1vgaPatchSierraLogoSpeed },
 	{  true,   814, "window text temp space",                      1, qfg1vgaSignatureTempSpace,           qfg1vgaPatchTempSpace },
 	{  true,   814, "dialog header offset",                        3, qfg1vgaSignatureDialogHeader,        qfg1vgaPatchDialogHeader },
 	{  true,   928, "Narrator lockup fix",                         1, sciNarratorLockupSignature,          sciNarratorLockupPatch },


Commit: 76044dc51a44a2ba5432aad162225be615b0e27c
    https://github.com/scummvm/scummvm/commit/76044dc51a44a2ba5432aad162225be615b0e27c
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2022-12-21T12:26:51-08:00

Commit Message:
SCI: Fix CNICK_LONGBOW uninit read workaround

This uninitialized variable read wasn't being detected until the
variable layout was fixed in: 30fad94e9a768d67287906c15386ae212af8fbac

Changed paths:
    engines/sci/engine/workarounds.cpp


diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 6103375956e..7a782c29c10 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -376,7 +376,7 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_CNICK_LAURABOW,100,   100,  0,              nullptr, "<noname144>",                  nullptr,     1,     1, { WORKAROUND_FAKE,   0 } }, // while playing domino - bug #6429 (same as the dominoHand2 workaround for Hoyle 3)
 	{ GID_CNICK_LAURABOW,100,   110,  0,              nullptr, "doit",                         nullptr,    -1,    -1, { WORKAROUND_FAKE,   0 } }, // when changing the "Dominoes per hand" setting - bug #6430
 	{ GID_CNICK_LSL,     250,   250,  0,           "increase", "handleEvent",                  nullptr,     2,     2, { WORKAROUND_FAKE,   0 } }, // when increasing own bet for blackjack - bug #10184
-	{ GID_CNICK_LONGBOW,   0,     0,  0,          "RH Budget", "init",                         nullptr,     1,     1, { WORKAROUND_FAKE,   0 } }, // when starting the game
+	{ GID_CNICK_LONGBOW,   0,     0,  0,          "RH Budget", "init",                         nullptr,     0,     1, { WORKAROUND_FAKE,   0 } }, // when starting the game
 	{ GID_ECOQUEST,       -1,    -1,  0,              nullptr, "doVerb",                       nullptr,     0,     0, { WORKAROUND_FAKE,   0 } }, // almost clicking anywhere triggers this in almost all rooms
 	{ GID_ECOQUEST2,      -1,    50,  0,         "talkButton", "cue",                          nullptr,     0,     0, { WORKAROUND_FAKE,   0 } }, // clicking Ecorder talk button before clicking power button
 	{ GID_FANMADE,       516,   979,  0,                   "", "export 0",                     nullptr,    20,    20, { WORKAROUND_FAKE,   0 } }, // Happens in Grotesteing after the logos


Commit: c7099d137e87ff751ef72c596e0d6992eec59c66
    https://github.com/scummvm/scummvm/commit/c7099d137e87ff751ef72c596e0d6992eec59c66
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2022-12-21T12:26:52-08:00

Commit Message:
SCI: Add detection for another GK1 DEMO variant

Thanks to @einstein95 for finding this

Changed paths:
    engines/sci/detection_tables.h


diff --git a/engines/sci/detection_tables.h b/engines/sci/detection_tables.h
index c6a3853ef43..ae04b3f5e26 100644
--- a/engines/sci/detection_tables.h
+++ b/engines/sci/detection_tables.h
@@ -846,6 +846,17 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 		AD_LISTEND},
 		Common::EN_ANY, Common::kPlatformDOS, 0, GUIO_STD16_SPEECH	},
 
+	// Gabriel Knight - English DOS Demo
+	// SCI interpreter version 1.001.092
+	// Note: we are not using ADGF_DEMO here, to avoid a game ID like gk1demo-demo.
+	// Same resource volume as previous entry, but resource.map has one different
+	// byte within the junk padding between directory headers and directory entries.
+	{"gk1demo", "Demo", {
+		{"resource.map", 0, "1f6643045cab8546c9e6bddfbce4ea80", 2490},
+		{"resource.000", 0, "eb3ed7477ca4110813fe1fcf35928561", 1718450},
+		AD_LISTEND},
+		Common::EN_ANY, Common::kPlatformDOS, 0, GUIO_STD16_SPEECH	},
+
 #define GUIO_GK1_FLOPPY GUIO3(GUIO_NOSPEECH, \
 							  GAMEOPTION_ORIGINAL_SAVELOAD, \
 							  GAMEOPTION_TTS)


Commit: d10fd9f1241e9b4069875207aed03bd6ae578b98
    https://github.com/scummvm/scummvm/commit/d10fd9f1241e9b4069875207aed03bd6ae578b98
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2022-12-21T12:26:52-08:00

Commit Message:
SCI: Revert disabling LB2 control panel in tunnel

These rooms disabled the control panel to prevent the player from
accessing the game's speed setting. Ego is set to a fixed speed and
the timing of the room scripts depend on that speed.

See: 11ca903f606e0b5d155652e9b8c80f3a4d0ddbec

Changed paths:
    engines/sci/engine/script_patches.cpp


diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index 1bab14c9aa2..39b21085e20 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -9700,8 +9700,8 @@ static const uint16 laurabow2CDPatchPaintingClosing[] = {
 //  restoring the floppy code in Inset:dispose. We make room by overwriting
 //  unnecessary code in Inset:onMe that sets variables and doesn't use them.
 //  While we're at it, we also remove IconBar:disable(7) from the vat room (610)
-//  and the final passage (740) in all versions. As with the CD restrictions,
-//  they're unnecessary and inconsistently applied.
+//  in all versions. This restriction was only to make searching the vats harder
+//  when the puzzle hadn't been solved, but it didn't work consistently.
 //
 // Applies to: English PC-CD for most patches, floppy versions for others
 // Responsible methods: LB2:handsOn, Inset:dispose, many others
@@ -10550,6 +10550,9 @@ static const uint16 laurabow2PatchHandleArmorRoomEvents[] = {
 //
 // We fix this by adding a test to rm448:notify to detect if sHeKills is already
 //  the current room script and ignore the notification.
+//
+// Applies to: All versions
+// Responsible method: rm448:notify
 static const uint16 laurabow2SignatureAct5TimerCrash[] = {
 	0x31, 0x29,                         // bnt 29 [ ret ]
 	0x38, SIG_ADDTOOFFSET(+2),          // pushi script
@@ -11061,12 +11064,8 @@ static const SciScriptPatcherEntry laurabow2Signatures[] = {
 	{ false,   610, "CD: enable control panel",                       3, laurabow2CDSignatureEnableControlPanel,         laurabow2PatchEnableControlPanel },
 	{ false,   700, "CD: enable control panel",                       6, laurabow2CDSignatureEnableControlPanel,         laurabow2PatchEnableControlPanel },
 	{ false,   710, "CD: enable control panel",                       1, laurabow2CDSignatureEnableControlPanel,         laurabow2PatchEnableControlPanel },
-	{ false,   730, "CD: enable control panel",                       5, laurabow2CDSignatureEnableControlPanel,         laurabow2PatchEnableControlPanel },
-	{ false,   740, "CD: enable control panel",                       4, laurabow2CDSignatureEnableControlPanel,         laurabow2PatchEnableControlPanel },
 	{ false,   923, "CD: enable control panel",                       1, laurabow2CDSignatureInsetEnableControlPanel,    laurabow2CDPatchInsetEnableControlPanel },
 	{ false,   610, "Floppy: enable control panel",                   3, laurabow2FloppySignatureEnableControlPanel,     laurabow2PatchEnableControlPanel },
-	{ false,   730, "Floppy: enable control panel",                   5, laurabow2FloppySignatureEnableControlPanel,     laurabow2PatchEnableControlPanel },
-	{ false,   740, "Floppy: enable control panel",                   3, laurabow2FloppySignatureEnableControlPanel,     laurabow2PatchEnableControlPanel },
 	// King's Quest 6 and Laura Bow 2 share basic patches for audio + text support
 	{ false,   924, "CD: audio + text support 1",                     1, kq6laurabow2CDSignatureAudioTextSupport1,       kq6laurabow2CDPatchAudioTextSupport1 },
 	{ false,   924, "CD: audio + text support 2",                     1, kq6laurabow2CDSignatureAudioTextSupport2,       kq6laurabow2CDPatchAudioTextSupport2 },




More information about the Scummvm-git-logs mailing list