[Scummvm-cvs-logs] SF.net SVN: scummvm:[54510] scummvm/trunk/engines/sci/engine

[thebluegr at users.sourceforge.net]

Revision: 54510
          http://scummvm.svn.sourceforge.net/scummvm/?rev=54510&view=rev
Author:   thebluegr
Date:     2010-11-27 18:08:47 +0000 (Sat, 27 Nov 2010)

Log Message:
-----------
SCI: Fixed bug #3034713 - "ICEMAN Demo: Fails to find base object"

This could happen because objects in scripts can be in the wrong order.
Same thing happens in the French and German version of KQ5 (bug #3035396).
Removed the scriptObjRemove() method, which is in fact a hack.

Modified Paths:
--------------
    scummvm/trunk/engines/sci/engine/savegame.cpp
    scummvm/trunk/engines/sci/engine/script.cpp
    scummvm/trunk/engines/sci/engine/script.h

Modified: scummvm/trunk/engines/sci/engine/savegame.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/savegame.cpp	2010-11-27 17:29:42 UTC (rev 54509)
+++ scummvm/trunk/engines/sci/engine/savegame.cpp	2010-11-27 18:08:47 UTC (rev 54510)
@@ -194,17 +194,23 @@
 
 		for (ObjMap::iterator it = scr->_objects.begin(); it != scr->_objects.end(); ++it) {
 			reg_t addr = it->_value.getPos();
-			Object *obj = scr->scriptObjInit(addr, false);
+			scr->scriptObjInit(addr, false);
+		}
 
-			if (getSciVersion() < SCI_VERSION_1_1) {
+		// In SCI0-SCI1, we need to make two passes, as the objects in the
+		// script might be in the wrong order (e.g. in the demo of Iceman).
+		// Refer to bug #3034713
+		if (getSciVersion() < SCI_VERSION_1_1) {
+			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 (!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);
+					error("Failed to locate base object for object at %04X:%04X; skipping", PRINT_REG(addr));
+					//scr->scriptObjRemove(addr);
 				}
 			}
 		}
+
 	}
 }
 

Modified: scummvm/trunk/engines/sci/engine/script.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/script.cpp	2010-11-27 17:29:42 UTC (rev 54509)
+++ scummvm/trunk/engines/sci/engine/script.cpp	2010-11-27 18:08:47 UTC (rev 54510)
@@ -255,13 +255,6 @@
 	return obj;
 }
 
-void Script::scriptObjRemove(reg_t obj_pos) {
-	if (getSciVersion() < SCI_VERSION_1_1)
-		obj_pos.offset += 8;
-
-	_objects.erase(obj_pos.toUint16());
-}
-
 // This helper function is used by Script::relocateLocal and Object::relocate
 // Duplicate in segment.cpp and script.cpp
 static bool relocateBlock(Common::Array<reg_t> &block, int block_location, SegmentId segment, int location, size_t scriptSize) {
@@ -580,42 +573,42 @@
 
 void Script::initialiseObjectsSci0(SegManager *segMan, SegmentId segmentId) {
 	bool oldScriptHeader = (getSciVersion() == SCI_VERSION_0_EARLY);
-	const byte *seeker = _buf + (oldScriptHeader ? 2 : 0);
 
-	do {
-		uint16 objType = READ_SCI11ENDIAN_UINT16(seeker);
-		if (!objType)
-			break;
+	// We need to make two passes, as the objects in the script might be in the
+	// wrong order (e.g. in the demo of Iceman) - refer to bug #3034713
+	for (int pass = 1; pass <= 2; pass++) {
+		const byte *seeker = _buf + (oldScriptHeader ? 2 : 0);
 
-		switch (objType) {
-		case SCI_OBJ_OBJECT:
-		case SCI_OBJ_CLASS:
-			{
-				reg_t addr = make_reg(segmentId, seeker - _buf + 4);
-				Object *obj = scriptObjInit(addr);
-				obj->initSpecies(segMan, addr);
+		do {
+			uint16 objType = READ_SCI11ENDIAN_UINT16(seeker);
+			if (!objType)
+				break;
 
-				if (!obj->initBaseObject(segMan, addr)) {
-					if ((_nr == 202 || _nr == 764) && g_sci->getGameId() == GID_KQ5) {
-						// WORKAROUND: Script 202 of KQ5 French and German 
-						// (perhaps Spanish too?) has an invalid object.
-						// This is non-fatal. Refer to bug #3035396.
-						// Same happens with script 764, it seems to contain junk towards its end
-					} else {
-						error("Failed to locate base object for object at %04X:%04X; skipping", PRINT_REG(addr));
+			switch (objType) {
+			case SCI_OBJ_OBJECT:
+			case SCI_OBJ_CLASS:
+				{
+					reg_t addr = make_reg(segmentId, seeker - _buf + 4);
+					Object *obj = scriptObjInit(addr);
+					obj->initSpecies(segMan, addr);
+
+					if (pass == 2) {
+						if (!obj->initBaseObject(segMan, addr)) {
+							error("Failed to locate base object for object at %04X:%04X", PRINT_REG(addr));
+							//scriptObjRemove(addr);
+						}
 					}
-					scriptObjRemove(addr);
 				}
+				break;
+
+			default:
+				break;
 			}
-			break;
 
-		default:
-			break;
-		}
+			seeker += READ_SCI11ENDIAN_UINT16(seeker + 2);
+		} while ((uint32)(seeker - _buf) < getScriptSize() - 2);
+	}
 
-		seeker += READ_SCI11ENDIAN_UINT16(seeker + 2);
-	} while ((uint32)(seeker - _buf) < getScriptSize() - 2);
-
 	byte *relocationBlock = findBlockSCI0(SCI_OBJ_POINTERS);
 	if (relocationBlock)
 		relocateSci0Sci21(make_reg(segmentId, relocationBlock - getBuf() + 4));

Modified: scummvm/trunk/engines/sci/engine/script.h
===================================================================
--- scummvm/trunk/engines/sci/engine/script.h	2010-11-27 17:29:42 UTC (rev 54509)
+++ scummvm/trunk/engines/sci/engine/script.h	2010-11-27 18:08:47 UTC (rev 54510)
@@ -137,12 +137,6 @@
 	Object *scriptObjInit(reg_t obj_pos, bool fullObjectInit = true);
 
 	/**
-	 * Removes a script object
-	 * @param obj_pos	Location (segment, offset) of the object.
-	 */
-	void scriptObjRemove(reg_t obj_pos);
-
-	/**
 	 * Initializes the script's local variables
 	 * @param segMan	A reference to the segment manager
 	 */


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.