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

antoniou79 noreply at scummvm.org
Wed Nov 6 15:25:40 UTC 2024


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:
d309d02e64 BLADERUNNER: Clarifications for duplicated branches


Commit: d309d02e640880b2af980f96e6cb3894be3698c4
    https://github.com/scummvm/scummvm/commit/d309d02e640880b2af980f96e6cb3894be3698c4
Author: antoniou79 (a.antoniou79 at gmail.com)
Date: 2024-11-06T17:25:08+02:00

Commit Message:
BLADERUNNER: Clarifications for duplicated branches

And also minor fixes to adhere closely to the original code

Changed paths:
    engines/bladerunner/script/ai/generic_walker_a.cpp
    engines/bladerunner/script/ai/generic_walker_b.cpp
    engines/bladerunner/script/ai/generic_walker_c.cpp
    engines/bladerunner/script/ai/officer_grayford.cpp
    engines/bladerunner/script/scene/ar01.cpp


diff --git a/engines/bladerunner/script/ai/generic_walker_a.cpp b/engines/bladerunner/script/ai/generic_walker_a.cpp
index 747781b6f46..043a442494c 100644
--- a/engines/bladerunner/script/ai/generic_walker_a.cpp
+++ b/engines/bladerunner/script/ai/generic_walker_a.cpp
@@ -537,6 +537,9 @@ bool AIScriptGenericWalkerA::preparePath() {
 		isInside = true;
 		if (Random_Query(0, 1)) {
 			AI_Movement_Track_Append(kActorGenwalkerA, 164, 0);
+			// Original code does indeed have duplication of branches here
+			// TODO This could possible indicate intent of different movement tracks for the actor
+			// based on repeated "coin flips", but as it was the code block for each branch was identical.
 #if 0
 			if (Random_Query(0, 1)) {
 				AI_Movement_Track_Append(kActorGenwalkerA, 163, 0);
@@ -551,18 +554,23 @@ bool AIScriptGenericWalkerA::preparePath() {
 			//}
 		} else {
 			AI_Movement_Track_Append(kActorGenwalkerA, 162, 0);
-#if 0
+			// Original code matches the if/else blocks here
+			// Intent seems to be that if the first coin flip fails (0) then a second one is made
+			// and based on that the actors facing is set or remains unchanged.
+			// The movement track is the same in both cases.
+			// Better to preserve the likely effective duplication here
+			// TODO Check if there's an observable difference between these branches
+			// and whether kActorGenwalkerB and kActorGenwalkerC need to also have this behaviour.
 			if (Random_Query(0, 1)) {
 				AI_Movement_Track_Append(kActorGenwalkerA, 163, 0);
 				AI_Movement_Track_Append(kActorGenwalkerA, 164, 0);
 			} else {
-#endif
 				if (Random_Query(0, 1)) {
 					AI_Movement_Track_Append_With_Facing(kActorGenwalkerA, 166, 0, 30);
 				}
 				AI_Movement_Track_Append(kActorGenwalkerA, 163, 0);
 				AI_Movement_Track_Append(kActorGenwalkerA, 164, 0);
-			//}
+			}
 		}
 		AI_Movement_Track_Repeat(kActorGenwalkerA);
 		return true;
diff --git a/engines/bladerunner/script/ai/generic_walker_b.cpp b/engines/bladerunner/script/ai/generic_walker_b.cpp
index 61a93ff8b88..c562570a554 100644
--- a/engines/bladerunner/script/ai/generic_walker_b.cpp
+++ b/engines/bladerunner/script/ai/generic_walker_b.cpp
@@ -488,6 +488,9 @@ bool AIScriptGenericWalkerB::preparePath() {
 		isInside = true;
 		if (Random_Query(0, 1)) {
 			AI_Movement_Track_Append(kActorGenwalkerB, 164, 0);
+			// Original code does indeed have duplication of branches here
+			// TODO This could possible indicate intent of different movement tracks for the actor
+			// based on repeated "coin flips", but as it was the code block for each branch was identical.
 #if 0
 			if (Random_Query(0, 1)) {
 				AI_Movement_Track_Append(kActorGenwalkerB, 163, 0);
@@ -502,6 +505,11 @@ bool AIScriptGenericWalkerB::preparePath() {
 			//}
 		} else {
 			AI_Movement_Track_Append(kActorGenwalkerB, 162, 0);
+			// Original code does indeed have duplication of branches here (similar to above)
+			// TODO This could possible indicate intent of different movement tracks for the actor
+			// based on repeated "coin flips", but as it was the code block for each branch was identical.
+			// NOTE The code for generic walker A here is slightly different, setting based on a "coin flip"
+			// the actor's "facing".
 #if 0
 			if (Random_Query(0, 1)) {
 				AI_Movement_Track_Append(kActorGenwalkerB, 163, 0);
diff --git a/engines/bladerunner/script/ai/generic_walker_c.cpp b/engines/bladerunner/script/ai/generic_walker_c.cpp
index e284ead7b2b..a9e84ddd125 100644
--- a/engines/bladerunner/script/ai/generic_walker_c.cpp
+++ b/engines/bladerunner/script/ai/generic_walker_c.cpp
@@ -490,6 +490,9 @@ bool AIScriptGenericWalkerC::preparePath() {
 		isInside = true;
 		if (Random_Query(0, 1)) {
 			AI_Movement_Track_Append(kActorGenwalkerC, 164, 0);
+			// Original code does indeed have duplication of branches here
+			// TODO This could possible indicate intent of different movement tracks for the actor
+			// based on repeated "coin flips", but as it was the code block for each branch was identical.
 #if 0
 			if (Random_Query(0, 1)) {
 				AI_Movement_Track_Append(kActorGenwalkerC, 163, 0);
@@ -504,18 +507,23 @@ bool AIScriptGenericWalkerC::preparePath() {
 			//}
 		} else {
 			AI_Movement_Track_Append(kActorGenwalkerC, 162, 0);
-#if 0
+			// Original code does indeed have duplication of branches here (similar to above)
+			// TODO This could possible indicate intent of different movement tracks for the actor
+			// based on repeated "coin flips", but as it was the code block for each branch was identical.
+			// NOTE The code for generic walker A here is slightly different, setting based on a "coin flip"
+			// the actor's "facing".
 			if (Random_Query(0, 1)) {
+#if 0
+				AI_Movement_Track_Append(kActorGenwalkerC, 163, 0);
+				AI_Movement_Track_Append(kActorGenwalkerC, 164, 0);
+			} else if (Random_Query(0, 1)) {
 				AI_Movement_Track_Append(kActorGenwalkerC, 163, 0);
 				AI_Movement_Track_Append(kActorGenwalkerC, 164, 0);
 			} else {
 #endif
-				if (Random_Query(0, 1)) {
-					AI_Movement_Track_Append_With_Facing(kActorGenwalkerC, 166, 0, 30);
-				}
 				AI_Movement_Track_Append(kActorGenwalkerC, 163, 0);
 				AI_Movement_Track_Append(kActorGenwalkerC, 164, 0);
-			//}
+			}
 		}
 		AI_Movement_Track_Repeat(kActorGenwalkerC);
 		return true;
diff --git a/engines/bladerunner/script/ai/officer_grayford.cpp b/engines/bladerunner/script/ai/officer_grayford.cpp
index 59ab23e6320..760475e8ece 100644
--- a/engines/bladerunner/script/ai/officer_grayford.cpp
+++ b/engines/bladerunner/script/ai/officer_grayford.cpp
@@ -331,6 +331,9 @@ void AIScriptOfficerGrayford::ClickedByPlayer() {
 		Actor_Face_Actor(kActorMcCoy, kActorOfficerGrayford, true);
 		Actor_Face_Actor(kActorOfficerGrayford, kActorMcCoy, true);
 		// TODO: Bug in the original? Both branches are identical
+		// The code is indeed like this in the original
+		// The duplication might indicated intend of different quotes by McCoy
+		// based on the coin flip, just like the other cases that use "coin flip"/
 #if 0
 		if (Random_Query(1, 2) == 1) {
 			Actor_Says(kActorMcCoy, 5075, 14); // Hey, pal.
diff --git a/engines/bladerunner/script/scene/ar01.cpp b/engines/bladerunner/script/scene/ar01.cpp
index c3e7a188caf..5aaf5640514 100644
--- a/engines/bladerunner/script/scene/ar01.cpp
+++ b/engines/bladerunner/script/scene/ar01.cpp
@@ -111,14 +111,24 @@ void SceneScriptAR01::InitializeScene() {
 	        && Game_Flag_Query(kFlagHC01toAR01)
 	) {
 		Scene_Loop_Set_Default(kAR01LoopMainLoop);
-	// TODO: Query check not required as NoSpinner is default else branch
+	// TODO: Query check not required as kAR01LoopMainLoopNoSpinner loop is set in the default else branch.
+	// The duplicate branches here reflect duplication in the original code.
+	// However, only the default/final else branch is indeed required to preserve the original logic.
+	// Originally:
+	// "!Game_Flag_Query(kFlagSpinnerAtAR01) &&  Game_Flag_Query(kFlagHC01toAR01)"
+	// corresponds to the case of "No spinner at AR01 and coming from HC01" (which can happen)
+	// The final else branch covered the case of "No spinner at AR01 and not coming from HC01 nor coming from AR02",
+	// which is practically impossible in normal gameplay, and only possible by debugging/hacking,
+	// yet good to have as "default" branch.
+	// Both branches set the same default loop (kAR01LoopMainLoopNoSpinner).
+	// Merging the two branches into one eventual else covers both of these cases.
 #if 0
 	} else if (!Game_Flag_Query(kFlagSpinnerAtAR01)
 	        &&  Game_Flag_Query(kFlagHC01toAR01)
 	) {
 		Scene_Loop_Set_Default(kAR01LoopMainLoopNoSpinner);
 #endif
-	} else { // TODO: bug? branch content is equal to previous branch
+	} else {
 		Scene_Loop_Set_Default(kAR01LoopMainLoopNoSpinner);
 	}
 }




More information about the Scummvm-git-logs mailing list