SCI: Some changes regarding the string heap in saved games

- Maintain the state of the string heap space in saved games
- Merged SegManager::reconstructScripts() inside SegManager::saveLoadWithSerializer()
- Disabled a now unnecessary script patch for the cipher puzzle in Castle of Dr. Brain, and performed some cleanup for another disabled patch
- Removed direct access to the _baseObj variable of objects

svn-id: r54133

6 files changed, 159 insertions, 113 deletions

diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index d0dcbd2993..072c4db57b 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -156,9 +156,26 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
 		// Let the object sync custom data
 		mobj->saveLoadWithSerializer(s);
 
-		// If we are loading a script, hook it up in the script->segment map.
-		if (s.isLoading() && type == SEG_TYPE_SCRIPT)
-			_scriptSegMap[((Script *)mobj)->getScriptNumber()] = i;
+		// If we are saving a script, save its string heap space too
+		if (s.isSaving() && type == SEG_TYPE_SCRIPT)
+			((Script *)mobj)->syncStringHeap(s);
+	
+		// If we are loading a script, perform some extra steps
+		if (s.isLoading() && type == SEG_TYPE_SCRIPT) {
+			Script *scr = (Script *)mobj;
+			// Hook the script up in the script->segment map
+			_scriptSegMap[scr->getScriptNumber()] = i;
+
+			// Now, load the script itself
+			scr->load(g_sci->getResMan());
+
+			for (ObjMap::iterator it = scr->_objects.begin(); it != scr->_objects.end(); ++it)
+				it->_value.syncBaseObject(scr->getBuf(it->_value.getPos().offset));
+
+			// Load the script's string heap
+			if (s.getVersion() >= 28)
+				scr->syncStringHeap(s);
+		}
 	}
 
 	s.syncAsSint32LE(_clonesSegId);
@@ -166,6 +183,29 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
 	s.syncAsSint32LE(_nodesSegId);
 
 	syncArray<Class>(s, _classTable);
+
+	// Now that all scripts are loaded, init their objects
+	for (uint i = 0; i < _heap.size(); i++) {
+		if (!_heap[i] ||  _heap[i]->getType() != SEG_TYPE_SCRIPT)
+			continue;
+
+		Script *scr = (Script *)_heap[i];
+		scr->_localsBlock = (scr->_localsSegment == 0) ? NULL : (LocalVariables *)(_heap[scr->_localsSegment]);
+
+		for (ObjMap::iterator it = scr->_objects.begin(); it != scr->_objects.end(); ++it) {
+			reg_t addr = it->_value.getPos();
+			Object *obj = scr->scriptObjInit(addr, false);
+
+			if (getSciVersion() < SCI_VERSION_1_1) {
+				if (!obj->initBaseObject(this, addr, false)) {
+					// TODO/FIXME: This should not be happening at all. It might indicate a possible issue
+					// with the garbage collector. It happens for example in LSL5 (German, perhaps English too).
+					warning("Failed to locate base object for object at %04X:%04X; skipping", PRINT_REG(addr));
+					scr->scriptObjRemove(addr);
+				}
+			}
+		}
+	}
 }
 
 
@@ -360,6 +400,43 @@ void HunkTable::saveLoadWithSerializer(Common::Serializer &s) {
 	// Do nothing, hunk tables are not actually saved nor loaded.
 }
 
+void Script::syncStringHeap(Common::Serializer &s) {
+	if (getSciVersion() < SCI_VERSION_1_1) {
+		// Sync all if the SCI_OBJ_STRINGS blocks
+		byte *buf = _buf;
+		bool oldScriptHeader = (getSciVersion() == SCI_VERSION_0_EARLY);
+
+		if (oldScriptHeader)
+			buf += 2;
+
+		do {
+			int blockType = READ_LE_UINT16(buf);
+			int blockSize = READ_LE_UINT16(buf + 2);
+			assert(blockSize > 0);
+
+			if (blockType == 0)
+				break;
+			if (blockType == SCI_OBJ_STRINGS)
+				s.syncBytes(buf, blockSize);
+
+			buf += blockSize;
+			if (_buf - buf == 0)
+				break;
+		} while (1);
+
+ 	} else {
+		// Strings in SCI1.1 come after the object instances
+		byte *buf = _heapStart + 4 + READ_SCI11ENDIAN_UINT16(_heapStart + 2) * 2;
+
+		// Skip all of the objects
+		while (READ_SCI11ENDIAN_UINT16(buf) == SCRIPT_OBJECT_MAGIC_NUMBER)
+			buf += READ_SCI11ENDIAN_UINT16(buf + 2) * 2;
+
+		// Now, sync everything till the end of the buffer
+		s.syncBytes(buf, _heapSize - (buf - _heapStart));
+	}
+}
+
 void Script::saveLoadWithSerializer(Common::Serializer &s) {
 	s.syncAsSint32LE(_nr);
 
@@ -667,44 +744,6 @@ void SegManager::reconstructStack(EngineState *s) {
 	s->stack_top = s->stack_base + stack->_capacity;
 }
 
-// TODO: Move this function to a more appropriate place, such as vm.cpp or script.cpp
-void SegManager::reconstructScripts(EngineState *s) {
-	uint i;
-
-	for (i = 0; i < _heap.size(); i++) {
-		if (!_heap[i] ||  _heap[i]->getType() != SEG_TYPE_SCRIPT)
-			continue;
-
-		Script *scr = (Script *)_heap[i];
-		scr->load(g_sci->getResMan());
-		scr->_localsBlock = (scr->_localsSegment == 0) ? NULL : (LocalVariables *)(_heap[scr->_localsSegment]);
-
-		for (ObjMap::iterator it = scr->_objects.begin(); it != scr->_objects.end(); ++it)
-			it->_value._baseObj = scr->getBuf(it->_value.getPos().offset);
-	}
-
-	for (i = 0; i < _heap.size(); i++) {
-		if (!_heap[i] ||  _heap[i]->getType() != SEG_TYPE_SCRIPT)
-			continue;
-
-		Script *scr = (Script *)_heap[i];
-
-		for (ObjMap::iterator it = scr->_objects.begin(); it != scr->_objects.end(); ++it) {
-			reg_t addr = it->_value.getPos();
-			Object *obj = scr->scriptObjInit(addr, false);
-
-			if (getSciVersion() < SCI_VERSION_1_1) {
-				if (!obj->initBaseObject(this, addr, false)) {
-					// TODO/FIXME: This should not be happening at all. It might indicate a possible issue
-					// with the garbage collector. It happens for example in LSL5 (German, perhaps English too).
-					warning("Failed to locate base object for object at %04X:%04X; skipping", PRINT_REG(addr));
-					scr->scriptObjRemove(addr);
-				}
-			}
-		}
-	}
-}
-
 void SegManager::reconstructClones() {
 	for (uint i = 0; i < _heap.size(); i++) {
 		SegmentObj *mobj = _heap[i];
@@ -823,7 +862,6 @@ void gamestate_restore(EngineState *s, Common::SeekableReadStream *fh) {
 	// Now copy all current state information
 
 	s->_segMan->reconstructStack(s);
-	s->_segMan->reconstructScripts(s);
 	s->_segMan->reconstructClones();
 	s->initGlobals();
 	s->gcCountDown = GC_INTERVAL - 1;
diff --git a/engines/sci/engine/savegame.h b/engines/sci/engine/savegame.h
index 0a228063af..65a65f7cd5 100644
--- a/engines/sci/engine/savegame.h
+++ b/engines/sci/engine/savegame.h
@@ -36,7 +36,7 @@ namespace Sci {
 struct EngineState;
 
 enum {
-	CURRENT_SAVEGAME_VERSION = 27,
+	CURRENT_SAVEGAME_VERSION = 28,
 	MINIMUM_SAVEGAME_VERSION = 14
 };
 
diff --git a/engines/sci/engine/script.h b/engines/sci/engine/script.h
index 04603ad829..bc263edc8c 100644
--- a/engines/sci/engine/script.h
+++ b/engines/sci/engine/script.h
@@ -243,6 +243,11 @@ public:
 	 */
 	byte *findBlockSCI0(int type, int startBlockIndex = -1);
 
+	/**
+	 * Syncs the string heap of a script. Used when saving/loading.
+	 */
+	void syncStringHeap(Common::Serializer &ser);
+
 private:
 	/**
 	 * Processes a relocation block witin a script
@@ -268,8 +273,6 @@ private:
 	 * @param segmentId	The script's segment id
 	 */
 	void initialiseObjectsSci11(SegManager *segMan, SegmentId segmentId);
-
-	void syncHeap(Common::Serializer &ser);
 };
 
 } // End of namespace Sci
diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index 7317248021..7654290a5c 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -60,6 +60,8 @@ struct SciScriptSignature {
 //  - if not EOS, an adjust offset and the actual bytes
 //  - rinse and repeat
 
+#if 0
+
 // ===========================================================================
 // Castle of Dr. Brain
 // cipher::init (script 391) is called on room 380 init. This resets the word
@@ -105,6 +107,8 @@ const SciScriptSignature castlebrainSignatures[] = {
 	SCI_SIGNATUREENTRY_TERMINATOR
 };
 
+#endif
+
 // ===========================================================================
 // stayAndHelp::changeState (0) is called when ego swims to the left or right
 //  boundaries of room 660. Normally a textbox is supposed to get on screen
@@ -402,69 +406,73 @@ const SciScriptSignature gk1Signatures[] = {
 	SCI_SIGNATUREENTRY_TERMINATOR
 };
 
+#if 0
+
 // ===========================================================================
 // this here gets called on entry and when going out of game windows
 //  uEvt::port will not get changed after kDisposeWindow but a bit later, so
 //  we would get an invalid port handle to a kSetPort call. We just patch in
 //  resetting of the port selector. We destroy the stop/fade code in there,
 //  it seems it isn't used at all in the game.
-//const byte hoyle4SignaturePortFix[] = {
-//	28,
-//	0x39, 0x09,        // pushi 09
-//	0x89, 0x0b,        // lsg 0b
-//	0x39, 0x64,        // pushi 64
-//	0x38, 0xc8, 0x00,  // pushi 00c8
-//	0x38, 0x2c, 0x01,  // pushi 012c
-//	0x38, 0x90, 0x01,  // pushi 0190
-//	0x38, 0xf4, 0x01,  // pushi 01f4
-//	0x38, 0x58, 0x02,  // pushi 0258
-//	0x38, 0xbc, 0x02,  // pushi 02bc
-//	0x38, 0x20, 0x03,  // pushi 0320
-//	0x46,              // calle [xxxx] [xxxx] [xx]
-//	+5, 43,            // [skip 5 bytes]
-//	0x30, 0x27, 0x00,  // bnt 0027 -> end of routine
-//	0x87, 0x00,        // lap 00
-//	0x30, 0x19, 0x00,  // bnt 0019 -> fade out
-//	0x87, 0x01,        // lap 01
-//	0x30, 0x14, 0x00,  // bnt 0014 -> fade out
-//	0x38, 0xa7, 0x00,  // pushi 00a7
-//	0x76,              // push0
-//	0x80, 0x29, 0x01,  // lag 0129
-//	0x4a, 0x04,        // send 04 - call song::stop
-//	0x39, 0x27,        // pushi 27
-//	0x78,              // push1
-//	0x8f, 0x01,        // lsp 01
-//	0x51, 0x54,        // class 54
-//	0x4a, 0x06,        // send 06 - call PlaySong::play
-//	0x33, 0x09,        // jmp 09 -> end of routine
-//	0x38, 0xaa, 0x00,  // pushi 00aa
-//	0x76,              // push0
-//	0x80, 0x29, 0x01,  // lag 0129
-//	0x4a, 0x04,        // send 04
-//	0x48,              // ret
-//	0
-//};
-
-//const uint16 hoyle4PatchPortFix[] = {
-//	PATCH_ADDTOOFFSET | +33,
-//	0x38, 0x31, 0x01,  // pushi 0131 (selector curEvent)
-//	0x76,              // push0
-//	0x80, 0x50, 0x00,  // lag 0050 (global var 80h, "User")
-//	0x4a, 0x04,        // send 04 - read User::curEvent
-//
-//	0x38, 0x93, 0x00,  // pushi 0093 (selector port)
-//	0x78,              // push1
-//	0x76,              // push0
-//	0x4a, 0x06,        // send 06 - write 0 to that object::port
-//	0x48,              // ret
-//	PATCH_END
-//};
+const byte hoyle4SignaturePortFix[] = {
+	28,
+	0x39, 0x09,        // pushi 09
+	0x89, 0x0b,        // lsg 0b
+	0x39, 0x64,        // pushi 64
+	0x38, 0xc8, 0x00,  // pushi 00c8
+	0x38, 0x2c, 0x01,  // pushi 012c
+	0x38, 0x90, 0x01,  // pushi 0190
+	0x38, 0xf4, 0x01,  // pushi 01f4
+	0x38, 0x58, 0x02,  // pushi 0258
+	0x38, 0xbc, 0x02,  // pushi 02bc
+	0x38, 0x20, 0x03,  // pushi 0320
+	0x46,              // calle [xxxx] [xxxx] [xx]
+	+5, 43,            // [skip 5 bytes]
+	0x30, 0x27, 0x00,  // bnt 0027 -> end of routine
+	0x87, 0x00,        // lap 00
+	0x30, 0x19, 0x00,  // bnt 0019 -> fade out
+	0x87, 0x01,        // lap 01
+	0x30, 0x14, 0x00,  // bnt 0014 -> fade out
+	0x38, 0xa7, 0x00,  // pushi 00a7
+	0x76,              // push0
+	0x80, 0x29, 0x01,  // lag 0129
+	0x4a, 0x04,        // send 04 - call song::stop
+	0x39, 0x27,        // pushi 27
+	0x78,              // push1
+	0x8f, 0x01,        // lsp 01
+	0x51, 0x54,        // class 54
+	0x4a, 0x06,        // send 06 - call PlaySong::play
+	0x33, 0x09,        // jmp 09 -> end of routine
+	0x38, 0xaa, 0x00,  // pushi 00aa
+	0x76,              // push0
+	0x80, 0x29, 0x01,  // lag 0129
+	0x4a, 0x04,        // send 04
+	0x48,              // ret
+	0
+};
+
+const uint16 hoyle4PatchPortFix[] = {
+	PATCH_ADDTOOFFSET | +33,
+	0x38, 0x31, 0x01,  // pushi 0131 (selector curEvent)
+	0x76,              // push0
+	0x80, 0x50, 0x00,  // lag 0050 (global var 80h, "User")
+	0x4a, 0x04,        // send 04 - read User::curEvent
+
+	0x38, 0x93, 0x00,  // pushi 0093 (selector port)
+	0x78,              // push1
+	0x76,              // push0
+	0x4a, 0x06,        // send 06 - write 0 to that object::port
+	0x48,              // ret
+	PATCH_END
+};
 
 //    script, description,                                   magic DWORD,                                 adjust
-//const SciScriptSignature hoyle4Signatures[] = {
-//    {      0, "port fix when disposing windows",             PATCH_MAGICDWORD(0x64, 0x38, 0xC8, 0x00),    -5, hoyle4SignaturePortFix,   hoyle4PatchPortFix },
-//    {      0, NULL,                                          0,                                            0, NULL,                     NULL }
-//};
+const SciScriptSignature hoyle4Signatures[] = {
+    {      0, "port fix when disposing windows",             PATCH_MAGICDWORD(0x64, 0x38, 0xC8, 0x00),    -5, hoyle4SignaturePortFix,   hoyle4PatchPortFix },
+    {      0, NULL,                                          0,                                            0, NULL,                     NULL }
+};
+
+#endif
 
 // ===========================================================================
 // at least during harpy scene export 29 of script 0 is called in kq5cd and
@@ -874,9 +882,12 @@ int32 Script::findSignature(const SciScriptSignature *signature, const byte *scr
 void Script::matchSignatureAndPatch(uint16 scriptNr, byte *scriptData, const uint32 scriptSize) {
 	const SciScriptSignature *signatureTable = NULL;
 	switch (g_sci->getGameId()) {
+	// Dr. Brain now works because we properly maintain the state of the string heap in savegames
+#if 0
 	case GID_CASTLEBRAIN:
 		signatureTable = castlebrainSignatures;
 		break;
+#endif
 	case GID_ECOQUEST:
 		signatureTable = ecoquest1Signatures;
 		break;
@@ -890,9 +901,11 @@ void Script::matchSignatureAndPatch(uint16 scriptNr, byte *scriptData, const uin
 		signatureTable = gk1Signatures;
 		break;
 	// hoyle4 now works due to workaround inside GfxPorts
-	//case GID_HOYLE4:
-	//	signatureTable = hoyle4Signatures;
-	//	break;
+#if 0
+	case GID_HOYLE4:
+		signatureTable = hoyle4Signatures;
+		break;
+#endif
 	case GID_KQ5:
 		signatureTable = kq5Signatures;
 		break;
diff --git a/engines/sci/engine/seg_manager.h b/engines/sci/engine/seg_manager.h
index 59ac6f39b6..55dd39d89b 100644
--- a/engines/sci/engine/seg_manager.h
+++ b/engines/sci/engine/seg_manager.h
@@ -92,11 +92,6 @@ public:
 	void deallocateScript(int script_nr);
 
 	/**
-	 * Reconstructs scripts. Used when restoring saved games
-	 */
-	void reconstructScripts(EngineState *s);
-
-	/**
 	 * Reconstructs the stack. Used when restoring saved games
 	 */
 	void reconstructStack(EngineState *s);
diff --git a/engines/sci/engine/segment.h b/engines/sci/engine/segment.h
index 99a41137e9..a2fe61729a 100644
--- a/engines/sci/engine/segment.h
+++ b/engines/sci/engine/segment.h
@@ -318,13 +318,10 @@ public:
 	void initSpecies(SegManager *segMan, reg_t addr);
 	void initSuperClass(SegManager *segMan, reg_t addr);
 	bool initBaseObject(SegManager *segMan, reg_t addr, bool doInitSuperClass = true);
-
-	// TODO: make private
-	// Only SegManager::reconstructScripts() is left needing direct access to these
-public:
-	const byte *_baseObj; /**< base + object offset within base */
+	void syncBaseObject(const byte *ptr) { _baseObj = ptr; }
 
 private:
+	const byte *_baseObj; /**< base + object offset within base */
 	const uint16 *_baseVars; /**< Pointer to the varselector area for this object */
 	const uint16 *_baseMethod; /**< Pointer to the method selector area for this object */