[Scummvm-git-logs] scummvm master -> 9be84837b328d4f890492f14cd11945267cfec76

sluicebox noreply at scummvm.org
Fri Aug 12 04:33:39 UTC 2022


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

Summary:
ec6d350ba6 SCI: Fix LB2 CD interrupted introduction music
4cffc809b2 SCI: Fix "You wins" message in PQ1 poker
fb35db5260 SCI: Document kGameIsRestarting speed throttler
9be84837b3 SCI: Add speed_throttle debug command


Commit: ec6d350ba6d909ddebc1baab651b9fd23c61d0d2
    https://github.com/scummvm/scummvm/commit/ec6d350ba6d909ddebc1baab651b9fd23c61d0d2
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2022-08-12T00:33:19-04:00

Commit Message:
SCI: Fix LB2 CD interrupted introduction music

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 996ce45850c..fe746e44747 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -10004,6 +10004,32 @@ static const uint16 laurabow2CDPatchFixAct5FinaleMusic[] = {
 	PATCH_END
 };
 
+// During the introduction in room 110 the music is cut off in the CD version
+//  instead of waiting for it to complete. This is the same bug as above where
+//  changes to the newer Sound class broke a script that worked in floppy.
+//
+// We fix this in the same was as the Act 5 music by testing Sound:handle.
+//
+// Applies to: All CD versions
+// Responsible method: sCartoon:changeState(26)
+static const uint16 laurabow2CDSignatureFixIntroMusic[] = {
+	SIG_MAGICDWORD,
+	0x38, SIG_UINT16(0x00ab),           // pushi 00ab [ prevSignal ]
+	0x76,                               // push0
+	0x81, 0x66,                         // lag 66
+	0x4a, 0x04,                         // send 04 [ gameMusic1 prevSignal? ]
+	0x36,                               // push
+	0x35, 0xff,                         // ldi ff
+	SIG_END
+};
+
+static const uint16 laurabow2CDPatchFixIntroMusic[] = {
+	0x38, PATCH_SELECTOR16(handle),     // pushi handle
+	PATCH_ADDTOOFFSET(+6),
+	0x35, 0x00,                         // ldi 00
+	PATCH_END
+};
+
 // LB2CD reduces the music volume significantly during the introduction when
 //  characters talk while disembarking the ship in room 120. This is done so
 //  that their speech can be heard but it also occurs in text-only mode.
@@ -10149,6 +10175,7 @@ static const SciScriptPatcherEntry laurabow2Signatures[] = {
 	{  true,   560, "CD: painting closing immediately",               1, laurabow2CDSignaturePaintingClosing,            laurabow2CDPatchPaintingClosing },
 	{  true,     0, "CD: fix problematic icon bar",                   1, laurabow2CDSignatureFixProblematicIconBar,      laurabow2CDPatchFixProblematicIconBar },
 	{  true,    90, "CD: fix yvette's tut response",                  1, laurabow2CDSignatureFixYvetteTutResponse,       laurabow2CDPatchFixYvetteTutResponse },
+	{  true,   110, "CD: fix intro music",                            1, laurabow2CDSignatureFixIntroMusic,              laurabow2CDPatchFixIntroMusic },
 	{  true,   350, "CD/Floppy: museum party fix entering south 1/2", 1, laurabow2SignatureMuseumPartyFixEnteringSouth1, laurabow2PatchMuseumPartyFixEnteringSouth1 },
 	{  true,   350, "CD/Floppy: museum party fix entering south 2/2", 1, laurabow2SignatureMuseumPartyFixEnteringSouth2, laurabow2PatchMuseumPartyFixEnteringSouth2 },
 	{  true,   430, "CD/Floppy: make wired east door persistent",     1, laurabow2SignatureRememberWiredEastDoor,        laurabow2PatchRememberWiredEastDoor },


Commit: 4cffc809b246f3f42252f45a0c0dcbc671879086
    https://github.com/scummvm/scummvm/commit/4cffc809b246f3f42252f45a0c0dcbc671879086
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2022-08-12T00:33:19-04:00

Commit Message:
SCI: Fix "You wins" message in PQ1 poker

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 fe746e44747..401cbcc3bd6 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -11730,6 +11730,35 @@ static const uint16 pq1vgaPatchFloatOutsideCarols2[] = {
 	PATCH_END
 };
 
+// When all the poker players fold, the game displays the message "You wins."
+//  instead of "You win.". The script that builds the string compares the wrong
+//  objects to see who the winner is, and so it always adds the final "s".
+//
+// We fix this by comparing the correct objects.
+//
+// Applies to: English Floppy
+// Responsible method: HandList:winner
+static const uint16 pq1vgaSignaturePokerWinnerMessage[] = {
+	0x35, 0x00,                          // ldi 00
+	0xa3, 0x4a,                          // sal 4a   [ local74 = 0 ]
+	0x7c,                                // pushSelf [ hand1First or allHands ]
+	0x72, SIG_UINT16(0x033c),            // lofsa hand1
+	0x1c,                                // ne?      [ always true ]
+	SIG_ADDTOOFFSET(+51),
+	SIG_MAGICDWORD,
+	0x8d, 0x06,                          // lst 06 [ winning hand ]
+	0x72, SIG_UINT16(0x033c),            // lofsa hand1
+	0x1a,                                // eq?
+	SIG_END
+};
+
+static const uint16 pq1vgaPatchPokerWinnerMessage[] = {
+	0x76,                                // push0
+	0xab, 0x4a,                          // ssl 4a [ local74 = 0 ]
+	0x8d, 0x06,                          // lst 06 [ winning hand ]
+	PATCH_END
+};
+
 // The GOG release includes New Rising Sun's script patches which throttle speed
 //  in core classes. Since we do our own speed throttling, we patch the scripts
 //  back to the original code for compatibility.
@@ -11774,6 +11803,7 @@ static const SciScriptPatcherEntry pq1vgaSignatures[] = {
 	{  true,    30, "float outside carol's (1/2)",                    7, pq1vgaSignatureFloatOutsideCarols1,  pq1vgaPatchFloatOutsideCarols1 },
 	{  true,    30, "float outside carol's (2/2)",                    1, pq1vgaSignatureFloatOutsideCarols2,  pq1vgaPatchFloatOutsideCarols2 },
 	{  true,   152, "getting stuck while briefing is about to start", 1, pq1vgaSignatureBriefingGettingStuck, pq1vgaPatchBriefingGettingStuck },
+	{  true,   156, "poker winner message",                           1, pq1vgaSignaturePokerWinnerMessage,   pq1vgaPatchPokerWinnerMessage },
 	{  true,   341, "put gun in locker bug",                          1, pq1vgaSignaturePutGunInLockerBug,    pq1vgaPatchPutGunInLockerBug },
 	{  true,   500, "map save/restore bug",                           2, pq1vgaSignatureMapSaveRestoreBug,    pq1vgaPatchMapSaveRestoreBug },
 	{  true,   928, "Narrator lockup fix",                            1, sciNarratorLockupSignature,          sciNarratorLockupPatch },


Commit: fb35db526040d7c771182ae22cb51dd04dc93589
    https://github.com/scummvm/scummvm/commit/fb35db526040d7c771182ae22cb51dd04dc93589
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2022-08-12T00:33:20-04:00

Commit Message:
SCI: Document kGameIsRestarting speed throttler

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


diff --git a/engines/sci/engine/kmisc.cpp b/engines/sci/engine/kmisc.cpp
index 841300cbc4f..6cf5251777e 100644
--- a/engines/sci/engine/kmisc.cpp
+++ b/engines/sci/engine/kmisc.cpp
@@ -55,9 +55,46 @@ reg_t kRestartGame16(EngineState *s, int argc, reg_t *argv) {
 	return NULL_REG;
 }
 
-/* kGameIsRestarting():
-** Returns the restarting_flag in acc
-*/
+/** 
+ * kGameIsRestarting returns the EngineState::gameIsRestarting flag.
+ * If zero is passed then the flag is cleared before returning its previous value.
+ *
+ * kGameIsRestarting(0) is unique because it is called by (almost) every game from
+ * the game loop (Game:doit, etc) and nowhere else. For that reason, we use this
+ * normally simple function as our main speed throttler. It prevents unthrottled
+ * games from running as fast as the CPU will allow. This creates a consistent
+ * playable experience and avoids the many script bugs that occur when thousands
+ * of game cycles run per second on a modern CPU.
+ *
+ * Early games throttle their own speed by calling kWait on every game cycle.
+ * The delay passed to kWait is the game speed selected by the user. At every speed
+ * except the fastest (kWait(1), or 17 ms) the kWait delay is greater than our
+ * speed throttle delay (30 ms). This means that our kGameIsRestarting throttle
+ * has no effect on these games other than to make the user-selected fastest speed
+ * run barely faster than the next-fastest selection (33 ms). It's unclear if that
+ * is intentional or a side-effect of throttling meant for unthrottled games.
+ * The speed throttling code is over a decade old and its details were uncommented.
+ * There have also been significant fixes to ScummVM's SCI timing code since then.
+ * It may be better to remove kGameIsRestarting throttling from kWait games to
+ * simplify things instead of having two overlapping throttlers run. That would
+ * allow those to run at their original speed when the user selects Fastest.
+ * kWait games are not the ones that run too fast at fast CPU speeds.
+ *
+ * If a scene in a game needs to be throttled further for compatibility then we
+ * add a workaround here. We also use script patches to make unthrottled inner loops
+ * call this function so that the program remains responsive and delays are
+ * consistent instead of CPU-bound.
+ *
+ * kGameIsRestarting was removed from SCI2 but Sierra left it in the PC interpreter
+ * as a no-op. Game scripts kept calling it even though it didn't do anything.
+ * That allows us to keep throttling those games this way; this is why this function
+ * is included in the kernel table for all SCI versions. SCI2 Mac scripts removed
+ * the call so they are unthrottled by this mechanism. They currently run faster
+ * than their PC versions. kGameIsRestarting was completely removed from SCI3.
+ *
+ * SCI2+ games (or versions) that don't call kGameIsRestarting are still limited
+ * to 60fps by GfxFrameout::throttle().
+ */
 reg_t kGameIsRestarting(EngineState *s, int argc, reg_t *argv) {
 	// Always return the previous flag value
 	const int16 previousRestartingFlag = s->gameIsRestarting;


Commit: 9be84837b328d4f890492f14cd11945267cfec76
    https://github.com/scummvm/scummvm/commit/9be84837b328d4f890492f14cd11945267cfec76
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2022-08-12T00:33:20-04:00

Commit Message:
SCI: Add speed_throttle debug command

Allows testing the speed throttler for possible improvements
and to demonstrate what it affects and what it doesn't.

`speed_throttle 0` disables kGameIsRestarting throttling.
(Except for game-specific workarounds.)

This command is possible due to several recent cleanups of other
throttling mechanisms that overlapped and were absorbed by this one.

Changed paths:
    engines/sci/console.cpp
    engines/sci/console.h
    engines/sci/engine/features.h
    engines/sci/engine/kmisc.cpp
    engines/sci/sci.cpp
    engines/sci/sci.h


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index aa95cf313c5..0b1c05a3940 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -90,6 +90,7 @@ Console::Console(SciEngine *engine) : GUI::Debugger(),
 	// precaution is taken to assure that all assigned values are in the range
 	// of the enum type. We should handle this more carefully...
 	registerVar("script_abort_flag",	(int *)&_engine->_gamestate->abortScriptProcessing);
+	registerCmd("speed_throttle",   WRAP_METHOD(Console, cmdSpeedThrottle));
 
 	// General
 	registerCmd("help",				WRAP_METHOD(Console, cmdHelp));
@@ -320,6 +321,7 @@ bool Console::cmdHelp(int argc, const char **argv) {
 	debugPrintf("simulated_key: Add a key with the specified scan code to the event list\n");
 	debugPrintf("track_mouse_clicks: Toggles mouse click tracking to the console\n");
 	debugPrintf("script_abort_flag: Set to 1 to abort script execution. Set to 2 to force a replay afterwards\n");
+	debugPrintf("speed_throttle: Displays or changes kGameIsRestarting maximum delay\n");
 	debugPrintf("\n");
 	debugPrintf("Debug flags\n");
 	debugPrintf("-----------\n");
@@ -4714,6 +4716,27 @@ bool Console::cmdAddresses(int argc, const char **argv) {
 	return true;
 }
 
+bool Console::cmdSpeedThrottle(int argc, const char **argv) {
+	if (argc > 2) {
+		debugPrintf("Displays or changes kGameIsRestarting maximum delay in milliseconds\n");
+		debugPrintf("usage: %s [<delay>]\n", argv[0]);
+		return true;
+	}
+	if (argc == 2) {
+		int newDelay;
+		if (!parseInteger(argv[1], newDelay)) {
+			return true;
+		}
+		if (newDelay < 0) {
+			debugPrintf("invalid delay\n");
+			return true;
+		}
+		_engine->_speedThrottleDelay = newDelay;
+	}
+	debugPrintf("kGameIsRestarting maximum delay: %d ms\n", _engine->_speedThrottleDelay);
+	return true;
+}
+
 // Returns 0 on success
 static int parse_reg_t(EngineState *s, const char *str, reg_t *dest) {
 	// Pointer to the part of str which contains a numeric offset (if any)
diff --git a/engines/sci/console.h b/engines/sci/console.h
index 6772768b485..918be2799a5 100644
--- a/engines/sci/console.h
+++ b/engines/sci/console.h
@@ -175,6 +175,8 @@ private:
 	bool cmdViewObject(int argc, const char **argv);
 	bool cmdViewActiveObject(int argc, const char **argv);
 	bool cmdViewAccumulatorObject(int argc, const char **argv);
+	// Variables
+	bool cmdSpeedThrottle(int argc, const char **argv);
 
 	bool parseInteger(const char *argument, int &result);
 	bool parseResourceNumber36(const char *userParameter, uint16 &resourceNumber, uint32 &resourceTuple);
diff --git a/engines/sci/engine/features.h b/engines/sci/engine/features.h
index 0702b007414..f07d58d6e3f 100644
--- a/engines/sci/engine/features.h
+++ b/engines/sci/engine/features.h
@@ -49,6 +49,10 @@ enum MessageTypeSyncStrategy {
 #endif
 };
 
+enum {
+	kSpeedThrottleDefaultDelay = 30 // kGameIsRestarting default max delay in ms
+};
+
 class GameFeatures {
 public:
 	GameFeatures(SegManager *segMan, Kernel *kernel);
diff --git a/engines/sci/engine/kmisc.cpp b/engines/sci/engine/kmisc.cpp
index 6cf5251777e..63a4b8b5248 100644
--- a/engines/sci/engine/kmisc.cpp
+++ b/engines/sci/engine/kmisc.cpp
@@ -108,7 +108,7 @@ reg_t kGameIsRestarting(EngineState *s, int argc, reg_t *argv) {
 		return make_reg(0, previousRestartingFlag);
 	}
 
-	uint32 neededSleep = 30;
+	uint32 neededSleep = g_sci->_speedThrottleDelay; // 30 ms (kSpeedThrottleDefaultDelay)
 
 	// WORKAROUNDS for scripts that are polling too quickly in scenes that
 	// are not animating much
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index 11e73f13d12..2b413e95907 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -119,6 +119,7 @@ SciEngine::SciEngine(OSystem *syst, const ADGameDescription *desc, SciGameId gam
 	_guestAdditions(nullptr),
 	_opcode_formats(nullptr),
 	_debugState(),
+	_speedThrottleDelay(kSpeedThrottleDefaultDelay),
 	_gameDescription(desc),
 	_gameId(gameId),
 	_resMan(nullptr),
diff --git a/engines/sci/sci.h b/engines/sci/sci.h
index fe89fed3d0f..e198e5ddebc 100644
--- a/engines/sci/sci.h
+++ b/engines/sci/sci.h
@@ -308,6 +308,7 @@ public:
 	opcode_format (*_opcode_formats)[4];
 
 	DebugState _debugState;
+	uint32 _speedThrottleDelay; // kGameIsRestarting maximum delay
 
 	Common::MacResManager *getMacExecutable() { return &_macExecutable; }
 




More information about the Scummvm-git-logs mailing list