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

thebluegr at users.sourceforge.net thebluegr at users.sourceforge.net
Sun May 30 22:06:50 CEST 2010


Revision: 49332
          http://scummvm.svn.sourceforge.net/scummvm/?rev=49332&view=rev
Author:   thebluegr
Date:     2010-05-30 20:06:50 +0000 (Sun, 30 May 2010)

Log Message:
-----------
- Merged the SCI0 scriptRelocate() and SCI11 heapRelocate() functions inside relocate(). scriptRelocate checked one more relocation entry, which seems wrong, so we're now checking for the correct number of relocations in all SCI versions
- Re-added the error when script + heap exceed 64KB (better than an assert) - this should theoretically never happen, and it never has for the games tested
- Removed the relocated sanity check - again, it shouldn't occur (else something else is wrong)

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

Modified: scummvm/trunk/engines/sci/engine/script.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/script.cpp	2010-05-30 20:01:25 UTC (rev 49331)
+++ scummvm/trunk/engines/sci/engine/script.cpp	2010-05-30 20:06:50 UTC (rev 49332)
@@ -419,7 +419,7 @@
 	} while (objType != 0 && curOffset < scr->getScriptSize() - 2);
 
 	if (relocation >= 0)
-		scr->scriptRelocate(make_reg(seg_id, relocation));
+		scr->relocate(make_reg(seg_id, relocation));
 
 	return seg_id;		// instantiation successful
 }
@@ -439,7 +439,7 @@
 	int heapStart = scr->getScriptSize();
 	segMan->scriptInitialiseLocals(make_reg(seg_id, heapStart + 4));
 	segMan->scriptInitialiseObjectsSci11(seg_id);
-	scr->heapRelocate(make_reg(seg_id, READ_SCI11ENDIAN_UINT16(scr->_heapStart)));
+	scr->relocate(make_reg(seg_id, READ_SCI11ENDIAN_UINT16(scr->_heapStart)));
 
 	return seg_id;
 }

Modified: scummvm/trunk/engines/sci/engine/segment.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/segment.cpp	2010-05-30 20:01:25 UTC (rev 49331)
+++ scummvm/trunk/engines/sci/engine/segment.cpp	2010-05-30 20:06:50 UTC (rev 49332)
@@ -100,7 +100,6 @@
 	_localsSegment = 0;
 	_localsBlock = NULL;
 
-	_relocated = false;
 	_markedAsDeleted = 0;
 }
 
@@ -125,7 +124,6 @@
 
 	_codeBlocks.clear();
 
-	_relocated = false;
 	_markedAsDeleted = false;
 
 	_nr = script_nr;
@@ -139,6 +137,14 @@
 	if (getSciVersion() == SCI_VERSION_0_EARLY) {
 		_bufSize += READ_LE_UINT16(script->data) * 2;
 	} else if (getSciVersion() >= SCI_VERSION_1_1) {
+		/**
+		 * In SCI11, the heap was in a separate space from the script.
+		 * We append it to the end of the script, and adjust addressing accordingly.
+		 * However, since we address the heap with a 16-bit pointer, the combined
+		 * size of the stack and the heap must be 64KB. So far this has worked
+		 * for SCI11, SCI2 and SCI21 games. SCI3 games use a different script format,
+		 * and theoretically they can exceed the 64KB boundary using relocation.
+		 */
 		Resource *heap = resMan->findResource(ResourceId(kResourceTypeHeap, script_nr), 0);
 		_bufSize += heap->size;
 		_heapSize = heap->size;
@@ -149,7 +155,11 @@
 			_scriptSize++;
 		}
 
-		assert(_bufSize <= 65535);
+		// As mentioned above, the script and the heap together should not exceed 64KB
+		if (_bufSize > 65535)
+			error("Script and heap sizes combined exceed 64K. This means a fundamental "
+					"design bug was made regarding SCI1.1 and newer games.\nPlease "
+					"report this error to the ScummVM team");
 	}
 }
 
@@ -253,14 +263,24 @@
 	_codeBlocks.push_back(cb);
 }
 
-void Script::scriptRelocate(reg_t block) {
-	VERIFY(block.offset < (uint16)_bufSize && READ_SCI11ENDIAN_UINT16(_buf + block.offset) * 2 + block.offset < (uint16)_bufSize,
+void Script::relocate(reg_t block) {
+	byte *heap = _buf;
+	uint16 heapSize = (uint16)_bufSize;
+	uint16 heapOffset = 0;
+
+	if (getSciVersion() >= SCI_VERSION_1_1) {
+		heap = _heapStart;
+		heapSize = (uint16)_heapSize;
+		heapOffset = _scriptSize;
+	}
+
+	VERIFY(block.offset < (uint16)heapSize && READ_SCI11ENDIAN_UINT16(heap + block.offset) * 2 + block.offset < (uint16)heapSize,
 	       "Relocation block outside of script\n");
 
-	int count = READ_SCI11ENDIAN_UINT16(_buf + block.offset);
+	int count = READ_SCI11ENDIAN_UINT16(heap + block.offset);
 
-	for (int i = 0; i <= count; i++) {
-		int pos = READ_SCI11ENDIAN_UINT16(_buf + block.offset + 2 + (i * 2));
+	for (int i = 0; i < count; i++) {
+		int pos = READ_SCI11ENDIAN_UINT16(heap + block.offset + 2 + (i * 2)) + heapOffset;
 		// This occurs in SCI01/SCI1 games where every other export
 		// value is zero. I have no idea what it's supposed to mean.
 		//
@@ -281,10 +301,13 @@
 					done = true;
 			}
 
-			for (k = 0; !done && k < _codeBlocks.size(); k++) {
-				if (pos >= _codeBlocks[k].pos.offset &&
-				        pos < _codeBlocks[k].pos.offset + _codeBlocks[k].size)
-					done = true;
+			// Sanity check for SCI0-SCI1
+			if (getSciVersion() < SCI_VERSION_1_1) {
+				for (k = 0; !done && k < _codeBlocks.size(); k++) {
+					if (pos >= _codeBlocks[k].pos.offset &&
+							pos < _codeBlocks[k].pos.offset + _codeBlocks[k].size)
+						done = true;
+				}
 			}
 
 			if (!done) {
@@ -303,44 +326,6 @@
 	}
 }
 
-void Script::heapRelocate(reg_t block) {
-	VERIFY(block.offset < (uint16)_heapSize && READ_SCI11ENDIAN_UINT16(_heapStart + block.offset) * 2 + block.offset < (uint16)_bufSize,
-	       "Relocation block outside of script\n");
-
-	if (_relocated)
-		return;
-	_relocated = true;
-	int count = READ_SCI11ENDIAN_UINT16(_heapStart + block.offset);
-
-	for (int i = 0; i < count; i++) {
-		int pos = READ_SCI11ENDIAN_UINT16(_heapStart + block.offset + 2 + (i * 2)) + _scriptSize;
-
-		if (!relocateLocal(block.segment, pos)) {
-			bool done = false;
-			uint k;
-
-			ObjMap::iterator it;
-			const ObjMap::iterator end = _objects.end();
-			for (it = _objects.begin(); !done && it != end; ++it) {
-				if (it->_value.relocate(block.segment, pos, _scriptSize))
-					done = true;
-			}
-
-			if (!done) {
-				printf("While processing relocation block %04x:%04x:\n", PRINT_REG(block));
-				printf("Relocation failed for index %04x (%d/%d)\n", pos, i + 1, count);
-				if (_localsBlock)
-					printf("- locals: %d at %04x\n", _localsBlock->_locals.size(), _localsOffset);
-				else
-					printf("- No locals\n");
-				for (it = _objects.begin(), k = 0; it != end; ++it, ++k)
-					printf("- obj#%d at %04x w/ %d vars\n", k, it->_value.getPos().offset, it->_value.getVarCount());
-				error("Breakpoint in %s, line %d", __FILE__, __LINE__);
-			}
-		}
-	}
-}
-
 void Script::incrementLockers() {
 	_lockers++;
 }

Modified: scummvm/trunk/engines/sci/engine/segment.h
===================================================================
--- scummvm/trunk/engines/sci/engine/segment.h	2010-05-30 20:01:25 UTC (rev 49331)
+++ scummvm/trunk/engines/sci/engine/segment.h	2010-05-30 20:06:50 UTC (rev 49332)
@@ -355,7 +355,6 @@
 	LocalVariables *_localsBlock;
 
 	Common::Array<CodeBlock> _codeBlocks;
-	bool _relocated;
 	bool _markedAsDeleted;
 
 public:
@@ -409,10 +408,8 @@
 	 * @param obj_pos	Location (segment, offset) of the block
 	 * @return			Location of the relocation block
 	 */
-	void scriptRelocate(reg_t block);
+	void relocate(reg_t block);
 
-	void heapRelocate(reg_t block);
-
 private:
 	bool relocateLocal(SegmentId segment, int location);
 


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




More information about the Scummvm-git-logs mailing list