SCI: multiple changes for mother goose vga

which is sci1 and sci1.1
fixes bug #3051145
- separating this mother goose from ega and sci2.1
- adding workaround when restoring saved games in these games, games try to calculate restored savedgameid instead of looking it up, we patch this code out and also set the global to the current savedgameid
- adding workaround for scripts checking savedgameid to be below 13 (our savedgameids begin at 100 now)
- changing official range for savedgameids from 1000->1999 to 100->199, otherwise mother goose would have required much larger patches and this range should be fine even if we replace savedgame dialogs later

svn-id: r52301

8 files changed, 81 insertions, 51 deletions

diff --git a/engines/sci/detection.cpp b/engines/sci/detection.cpp
index e330bd5f30..3b95434182 100644
--- a/engines/sci/detection.cpp
+++ b/engines/sci/detection.cpp
@@ -170,6 +170,7 @@ static const GameIdStrToEnum s_gameIdStrToEnum[] = {
 	{ "lsl6hires",       GID_LSL6HIRES },
 	{ "lsl7",            GID_LSL7 },
 	{ "mothergoose",     GID_MOTHERGOOSE },
+	{ "mothergoose256",  GID_MOTHERGOOSE256 },
 	{ "mothergoosehires",GID_MOTHERGOOSEHIRES },
 	{ "msastrochicken",  GID_MSASTROCHICKEN },
 	{ "pepper",          GID_PEPPER },
diff --git a/engines/sci/detection_tables.h b/engines/sci/detection_tables.h
index f72a71a794..3a0929e83c 100644
--- a/engines/sci/detection_tables.h
+++ b/engines/sci/detection_tables.h
@@ -2032,7 +2032,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 
 	// Mixed-Up Mother Goose v2.000 - English DOS Floppy (supplied by markcoolio in bug report #2723795)
 	// Executable scanning reports "1.001.031"
-	{"mothergoose", "", {
+	{"mothergoose256", "", {
 		{"resource.map", 0, "52aae15e493cafd1da7e1c9b657a5bb9", 7026},
 		{"resource.000", 0, "b7ecd8ae9e254e80310b5a668b276e6e", 2948975},
 		AD_LISTEND},
@@ -2041,7 +2041,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 	// Mixed-Up Mother Goose - English DOS CD (from jvprat)
 	// Executable scanning reports "x.yyy.zzz"
 	// SCI interpreter version 0.000.999 (just a guess)
-	{"mothergoose", "CD", {
+	{"mothergoose256", "CD", {
 		{"resource.map", 0, "1c7f311b0a2c927b2fbe81ae341fb2f6", 5790},
 		{"resource.001", 0, "5a0ed1d745855148364de1b3be099bac", 4369438},
 		AD_LISTEND},
@@ -2049,7 +2049,7 @@ static const struct ADGameDescription SciGameDescriptions[] = {
 
 	// Mixed-Up Mother Goose - English Windows Interactive Demo
 	// Executable scanning reports "x.yyy.zzz"
-	{"mothergoose", "Demo", {
+	{"mothergoose256", "Demo", {
 		{"resource.map", 0, "87f9dc1cafc4d4fa835fb2f00cf3a6ef", 4560},
 		{"resource.001", 0, "5a0ed1d745855148364de1b3be099bac", 2070072},
 		AD_LISTEND},
diff --git a/engines/sci/engine/kfile.cpp b/engines/sci/engine/kfile.cpp
index 76ac6254ac..f2a540b892 100644
--- a/engines/sci/engine/kfile.cpp
+++ b/engines/sci/engine/kfile.cpp
@@ -218,22 +218,18 @@ reg_t kFPuts(EngineState *s, int argc, reg_t *argv) {
 	return s->r_acc;
 }
 
-static bool fgets_wrapper(EngineState *s, char *dest, int maxsize, int handle) {
+static void fgets_wrapper(EngineState *s, char *dest, int maxsize, int handle) {
 	FileHandle *f = getFileFromHandle(s, handle);
 	if (!f)
-		return false;
+		return;
 
 	if (!f->_in) {
 		error("fgets_wrapper: Trying to read from file '%s' opened for writing", f->_name.c_str());
-		return false;
+		return;
 	}
 	if (maxsize > 1) {
 		memset(dest, 0, maxsize);
 		f->_in->readLine(dest, maxsize);
-
-		if (f->_in->eos() || f->_in->err())
-			return false;
-
 		// The returned string must not have an ending LF
 		int strSize = strlen(dest);
 		if (strSize > 0) {
@@ -242,14 +238,9 @@ static bool fgets_wrapper(EngineState *s, char *dest, int maxsize, int handle) {
 		}
 	} else {
 		*dest = f->_in->readByte();
-
-		if (f->_in->eos() || f->_in->err())
-			return false;
 	}
 
 	debugC(2, kDebugLevelFile, "  -> FGets'ed \"%s\"", dest);
-
-	return true;
 }
 
 reg_t kFGets(EngineState *s, int argc, reg_t *argv) {
@@ -258,9 +249,7 @@ reg_t kFGets(EngineState *s, int argc, reg_t *argv) {
 	int handle = argv[2].toUint16();
 
 	debugC(2, kDebugLevelFile, "kFGets(%d, %d)", handle, maxsize);
-	if (!fgets_wrapper(s, buf, maxsize, handle))
-		*buf = 0;
-
+	fgets_wrapper(s, buf, maxsize, handle);
 	s->_segMan->memcpy(argv[0], (const byte*)buf, maxsize);
 	return argv[0];
 }
@@ -471,7 +460,7 @@ static int findSavegame(Common::Array<SavegameDesc> &saves, uint savegameId) {
 	return -1;
 }
 
-// The scripts get IDs ranging from 1000->1999, because the scripts require us to assign unique ids THAT EVEN STAY BETWEEN
+// The scripts get IDs ranging from 100->199, because the scripts require us to assign unique ids THAT EVEN STAY BETWEEN
 //  SAVES and the scripts also use "saves-count + 1" to create a new savedgame slot.
 //  SCI1.1 actually recycles ids, in that case we will currently get "0".
 // This behaviour is required especially for LSL6. In this game, it's possible to quick save. The scripts will use
@@ -646,11 +635,11 @@ reg_t kRestoreGame(EngineState *s, int argc, reg_t *argv) {
 	if (argv[0].isNull()) {
 		// Loading from the launcher, don't adjust the ID of the saved game
 	} else {
-		if ((savegameId < 1000) || (savegameId > 1999)) {
+		if ((savegameId < SAVEGAMEID_OFFICIALRANGE_START) || (savegameId > SAVEGAMEID_OFFICIALRANGE_END)) {
 			warning("Savegame ID %d is not allowed", savegameId);
 			return TRUE_REG;
 		}
-		savegameId -= 1000;
+		savegameId -= SAVEGAMEID_OFFICIALRANGE_START;
 	}
 
 	Common::Array<SavegameDesc> saves;
@@ -669,6 +658,13 @@ reg_t kRestoreGame(EngineState *s, int argc, reg_t *argv) {
 		gamestate_restore(s, in);
 		delete in;
 
+		if (g_sci->getGameId() == GID_MOTHERGOOSE256) {
+			// WORKAROUND: Mother Goose SCI1/SCI1.1 does some weird things for
+			//  saving a previously restored game.
+			// We set the current savedgame-id directly and remove the script
+			//  code concerning this via script patch.
+			s->variables[VAR_GLOBAL][0xB3].offset = SAVEGAMEID_OFFICIALRANGE_START + savegameId;
+		}
 		return s->r_acc;
 	}
 
@@ -784,12 +780,6 @@ reg_t kFileIOReadRaw(EngineState *s, int argc, reg_t *argv) {
 	FileHandle *f = getFileFromHandle(s, handle);
 	if (f) {
 		bytesRead = f->_in->read(buf, size);
-
-		if (f->_in->eos() || f->_in->err()) {
-			*buf = 0;
-			bytesRead = 0;
-		}
-
 		s->_segMan->memcpy(argv[1], (const byte*)buf, size);
 	}
 
@@ -858,9 +848,7 @@ reg_t kFileIOReadString(EngineState *s, int argc, reg_t *argv) {
 	int handle = argv[2].toUint16();
 	debugC(2, kDebugLevelFile, "kFileIO(readString): %d, %d", handle, size);
 
-	if (!fgets_wrapper(s, buf, size, handle))
-		*buf = 0;
-
+	fgets_wrapper(s, buf, size, handle);
 	s->_segMan->memcpy(argv[0], (const byte*)buf, size);
 	delete[] buf;
 	return argv[0];
diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index 60d3369fb6..1e1f389e73 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -25,6 +25,7 @@
 
 #include "sci/sci.h"
 #include "sci/engine/script.h"
+#include "sci/engine/state.h"
 
 #include "common/util.h"
 
@@ -455,6 +456,50 @@ const SciScriptSignature laurabow2Signatures[] = {
 };
 
 // ===========================================================================
+// Mother Goose SCI1/SCI1.1
+// MG::replay somewhat calculates the savedgame-id used when saving again	
+//  this doesn't work right and we remove the code completely.
+//  We set the savedgame-id directly right after restoring in kRestoreGame.
+const byte mothergoose256SignatureReplay[] = {
+	6,
+	0x36,             // push
+	0x35, 0x20,       // ldi 20
+	0x04,             // sub
+	0xa1, 0xb3,       // sag global[b3]
+	0
+};
+
+const uint16 mothergoose256PatchReplay[] = {
+	0x34, 0x00, 0x00, // ldi 0000 (dummy)
+	0x34, 0x00, 0x00, // ldi 0000 (dummy)
+	PATCH_END
+};
+
+// when saving, it also checks if the savegame-id is below 13.
+//  we change this to check if below 113 instead
+const byte mothergoose256SignatureSaveLimit[] = {
+	5,
+	0x89, 0xb3,       // lsg global[b3]
+	0x35, 0x0d,       // ldi 0d
+	0x20,             // ge?
+	0
+};
+
+const uint16 mothergoose256PatchSaveLimit[] = {
+	PATCH_ADDTOOFFSET | +2,
+	0x35, 0x0d + SAVEGAMEID_OFFICIALRANGE_START, // ldi 113d
+	PATCH_END
+};
+
+//    script, description,                                   magic DWORD,                                  adjust
+const SciScriptSignature mothergoose256Signatures[] = {
+    {      0, "replay save issue",                           PATCH_MAGICDWORD(0x20, 0x04, 0xa1, 0xb3),    -2, mothergoose256SignatureReplay,    mothergoose256PatchReplay },
+    {      0, "save limit dialog (SCI1.1)",                  PATCH_MAGICDWORD(0xb3, 0x35, 0x0d, 0x20),    -1, mothergoose256SignatureSaveLimit, mothergoose256PatchSaveLimit },
+    {    994, "save limit dialog (SCI1)",                    PATCH_MAGICDWORD(0xb3, 0x35, 0x0d, 0x20),    -1, mothergoose256SignatureSaveLimit, mothergoose256PatchSaveLimit },
+    {      0, NULL,                                          0,                                            0, NULL,                             NULL }
+};
+
+// ===========================================================================
 //  script 298 of sq4/floppy has an issue. object "nest" uses another property
 //   which isn't included in property count. We return 0 in that case, the game
 //   adds it to nest::x. The problem is that the script also checks if x exceeds
@@ -607,6 +652,8 @@ void Script::matchSignatureAndPatch(uint16 scriptNr, byte *scriptData, const uin
 		signatureTable = laurabow2Signatures;
 	if (g_sci->getGameId() == GID_LSL6)
 		signatureTable = larry6Signatures;
+	if (g_sci->getGameId() == GID_MOTHERGOOSE256)
+		signatureTable = mothergoose256Signatures;
 	if (g_sci->getGameId() == GID_SQ4)
 		signatureTable = sq4Signatures;
 	if (g_sci->getGameId() == GID_SQ5)
diff --git a/engines/sci/engine/state.h b/engines/sci/engine/state.h
index 4f1d686b17..476bd55d6a 100644
--- a/engines/sci/engine/state.h
+++ b/engines/sci/engine/state.h
@@ -80,11 +80,11 @@ enum {
 	MAX_SAVEGAME_NR = 20 /**< Maximum number of savegames */
 };
 
-// We assume that scripts give us savegameId 0->999 for creating a new save slot
-//  and savegameId 1000->1999 for existing save slots ffs. kfile.cpp
+// We assume that scripts give us savegameId 0->99 for creating a new save slot
+//  and savegameId 100->199 for existing save slots ffs. kfile.cpp
 enum {
-	SAVEGAMEID_OFFICIALRANGE_START = 1000,
-	SAVEGAMEID_OFFICIALRANGE_END = 1999
+	SAVEGAMEID_OFFICIALRANGE_START = 100,
+	SAVEGAMEID_OFFICIALRANGE_END = 199
 };
 
 enum {
diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 92b408c135..2bd73141d1 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -85,7 +85,7 @@ const SciWorkaroundEntry opcodeMulWorkarounds[] = {
 
 //    gameID,           room,script,lvl,          object-name, method-name,    call,index,             workaround
 const SciWorkaroundEntry opcodeAndWorkarounds[] = {
-	{ GID_MOTHERGOOSE,     -1,  999,  0,               "Event", "new",           -1,     0, { WORKAROUND_FAKE,   0 } }, // constantly during the game
+	{ GID_MOTHERGOOSE256,  -1,  999,  0,               "Event", "new",           -1,     0, { WORKAROUND_FAKE,   0 } }, // constantly during the game
 	// ^^ TODO: which of the mother goose versions is affected by this? EGA? SCI1? SCI1.1?
 	SCI_WORKAROUNDENTRY_TERMINATOR
 };
@@ -93,7 +93,7 @@ const SciWorkaroundEntry opcodeAndWorkarounds[] = {
 //    gameID,           room,script,lvl,          object-name, method-name,    call,index,             workaround
 const SciWorkaroundEntry opcodeOrWorkarounds[] = {
 	{ GID_ECOQUEST2,      100,    0,  0,                "Rain", "points",     0xcc6,     0, { WORKAROUND_FAKE,   0 } }, // when giving the papers to the customs officer, gets called against a pointer instead of a number - bug #3034464
-	{ GID_MOTHERGOOSE,     -1,    4,  0,               "rm004", "doit",          -1,     0, { WORKAROUND_FAKE,   0 } }, // when going north and reaching the castle (rooms 4 and 37) - bug #3038228
+	{ GID_MOTHERGOOSE256,  -1,    4,  0,               "rm004", "doit",          -1,     0, { WORKAROUND_FAKE,   0 } }, // when going north and reaching the castle (rooms 4 and 37) - bug #3038228
 	SCI_WORKAROUNDENTRY_TERMINATOR
 };
 
@@ -156,9 +156,9 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_LSL6HIRES,       0,    85,  0,             "LL6Inv", "init",           -1,    0, { WORKAROUND_FAKE,   0 } }, // on startup
 	{ GID_LSL6HIRES,      -1, 64950,  1,            "Feature", "handleEvent",    -1,    0, { WORKAROUND_FAKE,   0 } }, // at least when entering swimming pool area
 	{ GID_LSL6HIRES,      -1, 64964,  0,              "DPath", "init",           -1,    1, { WORKAROUND_FAKE,   0 } }, // during the game
-	{ GID_MOTHERGOOSE,    -1,     0,  0,                 "MG", "doit",           -1,    5, { WORKAROUND_FAKE,   0 } }, // SCI1.1: When moving the cursor all the way to the left during the game (bug #3043955)
-	{ GID_MOTHERGOOSE,    -1,   992,  0,             "AIPath", "init",           -1,    0, { WORKAROUND_FAKE,   0 } }, // Happens in the demo and full version. In the demo, it happens when walking two screens from mother goose's house to the north. In the full version, it happens in rooms 7 and 23 - bug #3049146
-	{ GID_MOTHERGOOSE,    94,    94,  0,            "sunrise", "changeState",    -1,  367, { WORKAROUND_FAKE,   0 } }, // At the very end, after the game is completed - bug #3051163
+	{ GID_MOTHERGOOSE256, -1,     0,  0,                 "MG", "doit",           -1,    5, { WORKAROUND_FAKE,   0 } }, // SCI1.1: When moving the cursor all the way to the left during the game (bug #3043955)
+	{ GID_MOTHERGOOSE256, -1,   992,  0,             "AIPath", "init",           -1,    0, { WORKAROUND_FAKE,   0 } }, // Happens in the demo and full version. In the demo, it happens when walking two screens from mother goose's house to the north. In the full version, it happens in rooms 7 and 23 - bug #3049146
+	{ GID_MOTHERGOOSE256, 94,    94,  0,            "sunrise", "changeState",    -1,  367, { WORKAROUND_FAKE,   0 } }, // At the very end, after the game is completed - bug #3051163
 	{ GID_MOTHERGOOSEHIRES,-1,64950,  1,            "Feature", "handleEvent",    -1,    0, { WORKAROUND_FAKE,   0 } }, // right when clicking on a child at the start and probably also later
 	{ GID_MOTHERGOOSEHIRES,-1,64950,  1,               "View", "handleEvent",    -1,    0, { WORKAROUND_FAKE,   0 } }, // see above
 	{ GID_PEPPER,         -1,   894,  0,            "Package", "doVerb",         -1,    3, { WORKAROUND_FAKE,   0 } }, // using the hand on the book in the inventory - bug #3040012
diff --git a/engines/sci/graphics/ports.cpp b/engines/sci/graphics/ports.cpp
index 34fba9b1e4..12ac1d2025 100644
--- a/engines/sci/graphics/ports.cpp
+++ b/engines/sci/graphics/ports.cpp
@@ -105,16 +105,9 @@ void GfxPorts::init(bool usesOldGfxFunctions, GfxPaint16 *paint16, GfxText16 *te
 	case GID_CNICK_KQ:
 		offTop = 0;
 		break;
-	case GID_MOTHERGOOSE:
-		// TODO: if mother goose EGA also uses offTop we can simply remove this check altogether
-		switch (getSciVersion()) {
-		case SCI_VERSION_1_EARLY:
-		case SCI_VERSION_1_1:
-			offTop = 0;
-			break;
-		default:
-			break;
-		}
+	case GID_MOTHERGOOSE256:
+		// only the SCI1 and SCI1.1 (VGA) versions need this
+		offTop = 0;
 		break;
 	case GID_FAIRYTALES:
 		// Mixed-Up Fairy Tales (& its demo) uses -w 26 0 200 320. If we don't
diff --git a/engines/sci/sci.h b/engines/sci/sci.h
index 72d6e7e0cb..0ea90aba36 100644
--- a/engines/sci/sci.h
+++ b/engines/sci/sci.h
@@ -143,8 +143,9 @@ enum SciGameId {
 	GID_LSL6,
 	GID_LSL6HIRES, // We have a separate ID for LSL6 SCI32, because it's actually a completely different game
 	GID_LSL7,
-	GID_MOTHERGOOSE,
-	GID_MOTHERGOOSEHIRES, // We have a separate ID for Mother Goose SCI32, because it's actually a completely different game
+	GID_MOTHERGOOSE, // this one is the SCI0 version
+	GID_MOTHERGOOSE256, // this one handles SCI1 and SCI1.1 variants, at least those 2 share a bit in common
+	GID_MOTHERGOOSEHIRES, // this one is the SCI2.1 hires version, completely different from the other ones
 	GID_MSASTROCHICKEN,
 	GID_PEPPER,
 	GID_PHANTASMAGORIA,