SCI: Clean up kRandom + allow 0 parameters via signatures

Also added in depth comments about the currently known variations
Returning the RNG seed is now implemented (although it seems no
games actually use this functionality, it seems to be just script
bugs).
Also remove Torin kRandom workarounds.

4 files changed, 30 insertions, 24 deletions

diff --git a/engines/sci/engine/kernel_tables.h b/engines/sci/engine/kernel_tables.h
index af8e2d8f70..211d96bc2f 100644
--- a/engines/sci/engine/kernel_tables.h
+++ b/engines/sci/engine/kernel_tables.h
@@ -758,7 +758,7 @@ static SciKernelMapEntry s_kernelMap[] = {
 	{ MAP_CALL(Portrait),          SIG_EVERYWHERE,           "i(.*)",                 NULL,            NULL }, // subop
 	{ MAP_CALL(PrevNode),          SIG_EVERYWHERE,           "n",                     NULL,            NULL },
 	{ MAP_CALL(PriCoord),          SIG_EVERYWHERE,           "i",                     NULL,            NULL },
-	{ MAP_CALL(Random),            SIG_EVERYWHERE,           "i(i)(i)",               NULL,            kRandom_workarounds },
+	{ MAP_CALL(Random),            SIG_EVERYWHERE,           "(i)(i)",                NULL,            NULL },
 	{ MAP_CALL(ReadNumber),        SIG_EVERYWHERE,           "r",                     NULL,            kReadNumber_workarounds },
 	{ MAP_CALL(RemapColors),       SIG_SCI11, SIGFOR_ALL,    "i(i)(i)(i)(i)",         NULL,            NULL },
 #ifdef ENABLE_SCI32
diff --git a/engines/sci/engine/kmath.cpp b/engines/sci/engine/kmath.cpp
index 15bc24e0e3..ee5787af01 100644
--- a/engines/sci/engine/kmath.cpp
+++ b/engines/sci/engine/kmath.cpp
@@ -25,13 +25,22 @@
 
 namespace Sci {
 
+// Following variations existed:
+// up until including 0.530 (Hoyle 1): will always get 2 parameters, even if 2 parameters were not passed
+//                                     it seems if range is 0, it will return the seed.
+// 0.566 (Hero's Quest) to SCI1MID: check for 2 parameters, if not 2 parameters get seed.
+// SCI1LATE+: 2 parameters -> get random number within range
+//            1 parameter -> set seed
+//            any other amount of parameters -> get seed
+//
+// Right now, the weird SCI0 behavior (up until 0.530) for getting parameters and getting the seed is not implemented.
+// We also do not let through more than 2 parameters to kRandom via signatures. In case there is a game doing this,
+// a workaround should be added.
 reg_t kRandom(EngineState *s, int argc, reg_t *argv) {
-	switch (argc) {
-	case 1: // set seed to argv[0]
-		// SCI0/SCI01 just reset the seed to 0 instead of using argv[0] at all
-		return NULL_REG;
+	Common::RandomSource &rng = g_sci->getRNG();
 
-	case 2: { // get random number
+	if (argc == 2) {
+		// get random number
 		// numbers are definitely unsigned, for example lsl5 door code in k rap radio is random
 		//  and 5-digit - we get called kRandom(10000, 65000)
 		//  some codes in sq4 are also random and 5 digit (if i remember correctly)
@@ -54,19 +63,25 @@ reg_t kRandom(EngineState *s, int argc, reg_t *argv) {
 		if (range)
 			range--; // the range value was never returned, our random generator gets 0->range, so fix it
 
-		const int randomNumber = fromNumber + (int)g_sci->getRNG().getRandomNumber(range);
+		const int randomNumber = fromNumber + (int)rng.getRandomNumber(range);
 		return make_reg(0, randomNumber);
 	}
 
-	case 3: // get seed
-		// SCI0/01 did not support this at all
-		// Actually we would have to return the previous seed
-		error("kRandom: scripts asked for previous seed");
-		break;
-
-	default:
-		error("kRandom: unsupported argc");
+	// for other amounts of arguments
+	if (getSciVersion() >= SCI_VERSION_1_LATE) {
+		if (argc == 1) {
+			// 1 single argument is for setting the seed
+			// right now we do not change the Common RNG seed.
+			// It should never be required unless a game reuses the same seed to get the same combination of numbers multiple times.
+			// And in such a case, we would have to add code for such a feature (ScummVM RNG uses a UINT32 seed).
+			warning("kRandom: caller requested to set the RNG seed");
+			return NULL_REG;
+		}
 	}
+
+	// treat anything else as if caller wants the seed
+	warning("kRandom: caller requested to get the RNG seed");
+	return make_reg(0, rng.getSeed());
 }
 
 reg_t kAbs(EngineState *s, int argc, reg_t *argv) {
diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 7c41b1f4e5..ed913b27eb 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -765,14 +765,6 @@ const SciWorkaroundEntry kPlatform32_workarounds[] = {
 };
 
 //    gameID,           room,script,lvl,          object-name, method-name, local-call-signature, index,                workaround
-const SciWorkaroundEntry kRandom_workarounds[] = {
-	{ GID_TORIN,        51400,64928,  0,               "Blink", "init",                     NULL,     0, { WORKAROUND_STILLCALL,  0 } }, // when Lycentia knocks Torin out after he removes her collar
-	{ GID_TORIN,        51400,64928,  0,               "Blink", "cycleDone",                NULL,     0, { WORKAROUND_STILLCALL,  0 } }, // when Lycentia knocks Torin out after he removes her collar
-	SCI_WORKAROUNDENTRY_TERMINATOR
-};
-
-
-//    gameID,           room,script,lvl,          object-name, method-name, local-call-signature, index,                workaround
 const SciWorkaroundEntry kReadNumber_workarounds[] = {
 	{ GID_CNICK_LAURABOW,100,   101,  0,          "dominoes.opt", "doit",                   NULL,     0, { WORKAROUND_STILLCALL, 0 } }, // When dominoes.opt is present, the game scripts call kReadNumber with an extra integer parameter - bug #6425
 	{ GID_HOYLE3,        100,   101,  0,          "dominoes.opt", "doit",                   NULL,     0, { WORKAROUND_STILLCALL, 0 } }, // When dominoes.opt is present, the game scripts call kReadNumber with an extra integer parameter - bug #6425
diff --git a/engines/sci/engine/workarounds.h b/engines/sci/engine/workarounds.h
index 960c01efdd..44f7d857bb 100644
--- a/engines/sci/engine/workarounds.h
+++ b/engines/sci/engine/workarounds.h
@@ -91,7 +91,6 @@ extern const SciWorkaroundEntry kNewWindow_workarounds[];
 extern const SciWorkaroundEntry kPalVarySetPercent_workarounds[];
 extern const SciWorkaroundEntry kPalVaryMergeStart_workarounds[];
 extern const SciWorkaroundEntry kPlatform32_workarounds[];
-extern const SciWorkaroundEntry kRandom_workarounds[];
 extern const SciWorkaroundEntry kReadNumber_workarounds[];
 extern const SciWorkaroundEntry kResCheck_workarounds[];
 extern const SciWorkaroundEntry kPaletteUnsetFlag_workarounds[];