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

sluicebox noreply at scummvm.org
Thu Nov 17 23:46:05 UTC 2022


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:
ff9e03c4bd SCI: Convert LSL5 stopGroop workaround to script path


Commit: ff9e03c4bd78b061aa41d42f4fe7d16a4bc47e7c
    https://github.com/scummvm/scummvm/commit/ff9e03c4bd78b061aa41d42f4fe7d16a4bc47e7c
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2022-11-17T15:45:17-08:00

Commit Message:
SCI: Convert LSL5 stopGroop workaround to script path

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


diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index 471b12b13e3..ce8f6f6f393 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -152,6 +152,7 @@ static const char *const selectorNameTable[] = {
 	"owner",        // Longbow, QFG4
 	"fade",         // Longbow, Shivers
 	"enable",       // Longbow, SQ6
+	"alterEgo",     // LSL5
 	"delete",       // EcoQuest 1
 	"size",         // EcoQuest 1
 	"signal",       // EcoQuest 1, GK1
@@ -291,6 +292,7 @@ enum ScriptPatcherSelectors {
 	SELECTOR_owner,
 	SELECTOR_fade,
 	SELECTOR_enable,
+	SELECTOR_alterEgo,
 	SELECTOR_delete,
 	SELECTOR_size,
 	SELECTOR_signal,
@@ -8471,6 +8473,52 @@ static const SciScriptPatcherEntry larry3Signatures[] = {
 
 // ===========================================================================
 // Leisure Suit Larry 5
+
+// When switching between Larry and Patti, global 0 is set to the new actor but
+//  the script fails to update the pointer in stopGroop:client. This causes
+//  Patti to spin instead of walking. In the original this happened to work
+//  because both actors ended up in the same memory location. Script 0 disposed
+//  of the old actor's script immediately before loading the new one.
+//
+// In our implementation, each new object is loaded in a different memory
+//  location, and we can't overwrite the old one.
+//
+// We fix this by updating stopGroop:client when updating global 0 (ego).
+//
+// Applies to: All versions
+// Responsible method: Export 1 in script 0
+static const uint16 larry5SignatureUpdateStopGroopClient[] = {
+	0x38, SIG_SELECTOR16(alterEgo),     // pushi alterEgo
+	SIG_ADDTOOFFSET(+0x0f),
+	0x32, SIG_UINT16(0x0021),           // jmp 0021 [ toss, ret ]
+	SIG_ADDTOOFFSET(+0x0f),
+	SIG_MAGICDWORD,
+	0x38, SIG_SELECTOR16(alterEgo),     // pushi alterEgo
+	0x78,                               // push1
+	0x89, 0x00,                         // lsg 00
+	0x51, SIG_ADDTOOFFSET(+1),          // class User
+	0x4a, 0x06,                         // send 06 [ User alterEgo: ego ]
+	0x38, SIG_ADDTOOFFSET(+2),          // pushi setuUpInv
+	0x76,                               // push0
+	0x81, 0x00,                         // lag 00
+	0x4a, 0x04,                         // send 04 [ ego setUpInv: ]
+	SIG_END
+};
+
+static const uint16 larry5PatchUpdateStopGroopClient[] = {
+	PATCH_ADDTOOFFSET(+0x12),
+	0x32, PATCH_UINT16(0x0011),         // jmp 0011 [ stopGroop client: ego ]
+	PATCH_ADDTOOFFSET(+0x0f),
+	0x33, 0xda,                         // jmp da [ User alterEgo:, ego setUpInv: ]
+	0x39, PATCH_SELECTOR8(client),      // pushi client
+	0x78,                               // push1
+	0x89, 0x00,                         // lsg 00
+	0x81, 0x64,                         // lag 64
+	0x4a, 0x06,                         // send 06 [ stopGroop client: ego ]
+	0x33, 0x05,                         // jmp 05  [ toss, ret ] 
+	PATCH_END
+};
+
 // In Miami the player can call the green card telephone number and get
 //  green card including limo at the same time in the English 1.000 PC release.
 // This results later in a broken game in case the player doesn't read
@@ -8525,6 +8573,7 @@ static const uint16 larry5PatchGermanEndingPattiTalker[] = {
 
 //          script, description,                                      signature                               patch
 static const SciScriptPatcherEntry larry5Signatures[] = {
+	{  true,     0, "update stopGroop client",                     1, larry5SignatureUpdateStopGroopClient,   larry5PatchUpdateStopGroopClient },
 	{  true,   280, "English-only: fix green card limo bug",       1, larry5SignatureGreenCardLimoBug,        larry5PatchGreenCardLimoBug },
 	{  true,   380, "German-only: Enlarge Patti Textbox",          1, larry5SignatureGermanEndingPattiTalker, larry5PatchGermanEndingPattiTalker },
 	SCI_SIGNATUREENTRY_TERMINATOR
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 9bd940fa56e..9aa92e4bd16 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -162,35 +162,6 @@ static reg_t read_var(EngineState *s, int type, int index) {
 static void write_var(EngineState *s, int type, int index, reg_t value) {
 	if (validate_variable(s->variables[type], s->stack_base, type, s->variablesMax[type], index)) {
 
-		// WORKAROUND: This code is needed to work around a probable script bug, or a
-		// limitation of the original SCI engine, which can be observed in LSL5.
-		//
-		// In some games, ego walks via the "Grooper" object, in particular its "stopGroop"
-		// child. In LSL5, during the game, ego is swapped from Larry to Patti. When this
-		// happens in the original interpreter, the new actor is loaded in the same memory
-		// location as the old one, therefore the client variable in the stopGroop object
-		// points to the new actor. This is probably why the reference of the stopGroop
-		// object is never updated (which is why I mentioned that this is either a script
-		// bug or some kind of limitation).
-		//
-		// In our implementation, each new object is loaded in a different memory location,
-		// and we can't overwrite the old one. This means that in our implementation,
-		// whenever ego is changed, we need to update the "client" variable of the
-		// stopGroop object, which points to ego, to the new ego object. If this is not
-		// done, ego's movement will not be updated properly, so the result is
-		// unpredictable (for example in LSL5, Patti spins around instead of walking).
-		if (index == kGlobalVarEgo && type == VAR_GLOBAL && getSciVersion() > SCI_VERSION_0_EARLY) {
-			reg_t stopGroopPos = s->_segMan->findObjectByName("stopGroop");
-			if (!stopGroopPos.isNull()) {	// does the game have a stopGroop object?
-				// Find the "client" member variable of the stopGroop object, and update it
-				ObjVarRef varp;
-				if (lookupSelector(s->_segMan, stopGroopPos, SELECTOR(client), &varp, nullptr) == kSelectorVariable) {
-					reg_t *clientVar = varp.getPointer(s->_segMan);
-					*clientVar = value;
-				}
-			}
-		}
-
 		// If we are writing an uninitialized value into a temp, we remove the uninitialized segment
 		//  this happens at least in sq1/room 44 (slot-machine), because a send is missing parameters, then
 		//  those parameters are taken from uninitialized stack and afterwards they are copied back into temps




More information about the Scummvm-git-logs mailing list