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.

svn-id: r54510

3 files changed, 42 insertions, 49 deletions

diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 8d941d38b9..f0df3ff627 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -194,17 +194,23 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
 
 		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);
 				}
 			}
 		}
+
 	}
 }
 
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index ec64e1bfa1..789df63e87 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -255,13 +255,6 @@ Object *Script::scriptObjInit(reg_t obj_pos, bool fullObjectInit) {
 	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,41 +573,41 @@ void Script::initialiseClasses(SegManager *segMan) {
 
 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;
-
-		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 (!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));
+	// 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);
+
+		do {
+			uint16 objType = READ_SCI11ENDIAN_UINT16(seeker);
+			if (!objType)
+				break;
+
+			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;
+				break;
 
-		default:
-			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)
diff --git a/engines/sci/engine/script.h b/engines/sci/engine/script.h
index 03c470b4e0..18d7157747 100644
--- a/engines/sci/engine/script.h
+++ b/engines/sci/engine/script.h
@@ -137,12 +137,6 @@ public:
 	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
 	 */