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

fingolfin at users.sourceforge.net fingolfin at users.sourceforge.net
Thu Sep 17 02:44:22 CEST 2009


Revision: 44129
          http://scummvm.svn.sourceforge.net/scummvm/?rev=44129&view=rev
Author:   fingolfin
Date:     2009-09-17 00:44:22 +0000 (Thu, 17 Sep 2009)

Log Message:
-----------
SCI: More cleanup

Modified Paths:
--------------
    scummvm/trunk/engines/sci/engine/kmovement.cpp
    scummvm/trunk/engines/sci/engine/kscripts.cpp
    scummvm/trunk/engines/sci/engine/memobj.cpp
    scummvm/trunk/engines/sci/engine/seg_manager.cpp
    scummvm/trunk/engines/sci/engine/seg_manager.h

Modified: scummvm/trunk/engines/sci/engine/kmovement.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/kmovement.cpp	2009-09-16 23:55:11 UTC (rev 44128)
+++ scummvm/trunk/engines/sci/engine/kmovement.cpp	2009-09-17 00:44:22 UTC (rev 44129)
@@ -391,7 +391,6 @@
 }
 
 extern void _k_dirloop(reg_t obj, uint16 angle, EngineState *s, int argc, reg_t *argv);
-int is_heap_object(EngineState *s, reg_t pos);
 extern int get_angle(int xrel, int yrel);
 
 reg_t kDoAvoider(EngineState *s, int, int argc, reg_t *argv) {
@@ -404,14 +403,14 @@
 
 	s->r_acc = make_reg(0, -1);
 
-	if (!is_heap_object(s, avoider)) {
+	if (!s->segMan->isHeapObject(avoider)) {
 		warning("DoAvoider() where avoider %04x:%04x is not an object", PRINT_REG(avoider));
 		return NULL_REG;
 	}
 
 	client = GET_SEL32(avoider, client);
 
-	if (!is_heap_object(s, client)) {
+	if (!s->segMan->isHeapObject(client)) {
 		warning("DoAvoider() where client %04x:%04x is not an object", PRINT_REG(client));
 		return NULL_REG;
 	}
@@ -419,7 +418,7 @@
 	looper = GET_SEL32(client, looper);
 	mover = GET_SEL32(client, mover);
 
-	if (!is_heap_object(s, mover)) {
+	if (!s->segMan->isHeapObject(mover)) {
 		if (mover.segment) {
 			warning("DoAvoider() where mover %04x:%04x is not an object", PRINT_REG(mover));
 		}

Modified: scummvm/trunk/engines/sci/engine/kscripts.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/kscripts.cpp	2009-09-16 23:55:11 UTC (rev 44128)
+++ scummvm/trunk/engines/sci/engine/kscripts.cpp	2009-09-17 00:44:22 UTC (rev 44129)
@@ -318,27 +318,18 @@
 	}
 }
 
-bool is_heap_object(EngineState *s, reg_t pos) {
-	Object *obj = s->segMan->getObject(pos);
-	if (obj == NULL)
-		return false;
-	if (obj->flags & OBJECT_FLAG_FREED)
-		return false;
-	return !s->segMan->scriptIsMarkedAsDeleted(pos.segment);
-}
-
 reg_t kIsObject(EngineState *s, int, int argc, reg_t *argv) {
 	if (argv[0].offset == 0xffff) // Treated specially
 		return NULL_REG;
 	else
-		return make_reg(0, is_heap_object(s, argv[0]));
+		return make_reg(0, s->segMan->isHeapObject(argv[0]));
 }
 
 reg_t kRespondsTo(EngineState *s, int, int argc, reg_t *argv) {
 	reg_t obj = argv[0];
 	int selector = argv[1].toUint16();
 
-	return make_reg(0, is_heap_object(s, obj) && lookup_selector(s->segMan, obj, selector, NULL, NULL) != kSelectorNone);
+	return make_reg(0, s->segMan->isHeapObject(obj) && lookup_selector(s->segMan, obj, selector, NULL, NULL) != kSelectorNone);
 }
 
 } // End of namespace Sci

Modified: scummvm/trunk/engines/sci/engine/memobj.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/memobj.cpp	2009-09-16 23:55:11 UTC (rev 44128)
+++ scummvm/trunk/engines/sci/engine/memobj.cpp	2009-09-17 00:44:22 UTC (rev 44129)
@@ -34,6 +34,10 @@
 
 namespace Sci {
 
+//#define GC_DEBUG // Debug garbage collection
+//#define GC_DEBUG_VERBOSE // Debug garbage verbosely
+
+
 MemObject *MemObject::createMemObject(MemObjectType type) {
 	MemObject *mem = 0;
 	switch (type) {
@@ -115,6 +119,8 @@
 }
 
 bool Script::init(int script_nr, ResourceManager *resMan) {
+	_sciVersion = resMan->sciVersion();
+
 	setScriptSize(script_nr, resMan);
 
 	_buf = (byte *)malloc(_bufSize);
@@ -141,7 +147,6 @@
 
 	_nr = script_nr;
 
-	_sciVersion = resMan->sciVersion();
 	if (_sciVersion >= SCI_VERSION_1_1)
 		_heapStart = _buf + _scriptSize;
 	else
@@ -322,14 +327,12 @@
 }
 
 void Script::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback note, SciVersion version) {
-	Script *script = this;
-
-	if (addr.offset <= script->_bufSize && addr.offset >= -SCRIPT_OBJECT_MAGIC_OFFSET && RAW_IS_OBJECT(script->_buf + addr.offset)) {
+	if (addr.offset <= _bufSize && addr.offset >= -SCRIPT_OBJECT_MAGIC_OFFSET && RAW_IS_OBJECT(_buf + addr.offset)) {
 		Object *obj = getObject(addr.offset);
 		if (obj) {
 			// Note all local variables, if we have a local variable environment
-			if (script->_localsSegment)
-				(*note)(param, make_reg(script->_localsSegment, 0));
+			if (_localsSegment)
+				(*note)(param, make_reg(_localsSegment, 0));
 
 			for (uint i = 0; i < obj->_variables.size(); i++)
 				(*note)(param, obj->_variables[i]);
@@ -346,16 +349,15 @@
 //-------------------- clones --------------------
 
 void CloneTable::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback note, SciVersion version) {
-	CloneTable *clone_table = this;
 	Clone *clone;
 
 //	assert(addr.segment == _segId);
 
-	if (!clone_table->isValidEntry(addr.offset)) {
+	if (!isValidEntry(addr.offset)) {
 		error("Unexpected request for outgoing references from clone at %04x:%04x", PRINT_REG(addr));
 	}
 
-	clone = &(clone_table->_table[addr.offset]);
+	clone = &(_table[addr.offset]);
 
 	// Emit all member variables (including references to the 'super' delegate)
 	for (uint i = 0; i < clone->_variables.size(); i++)
@@ -367,14 +369,13 @@
 }
 
 void CloneTable::freeAtAddress(SegManager *segMan, reg_t addr) {
-	CloneTable *clone_table = this;
+#ifdef GC_DEBUG
 	Object *victim_obj;
 
 //	assert(addr.segment == _segId);
 
-	victim_obj = &(clone_table->_table[addr.offset]);
+	victim_obj = &(_table[addr.offset]);
 
-#ifdef GC_DEBUG
 	if (!(victim_obj->flags & OBJECT_FLAG_FREED))
 		warning("[GC] Clone %04x:%04x not reachable and not freed (freeing now)", PRINT_REG(addr));
 #ifdef GC_DEBUG_VERBOSE
@@ -386,7 +387,7 @@
 		warning("[GC] Clone %04x:%04x: Freeing", PRINT_REG(addr));
 		warning("[GC] Clone had pos %04x:%04x", PRINT_REG(victim_obj->pos));
 	*/
-	clone_table->freeEntry(addr.offset);
+	freeEntry(addr.offset);
 }
 
 

Modified: scummvm/trunk/engines/sci/engine/seg_manager.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/seg_manager.cpp	2009-09-16 23:55:11 UTC (rev 44128)
+++ scummvm/trunk/engines/sci/engine/seg_manager.cpp	2009-09-17 00:44:22 UTC (rev 44129)
@@ -45,9 +45,6 @@
 #define DEFAULT_OBJECTS 8	    // default # of objects per script
 #define DEFAULT_OBJECTS_INCREMENT 4 // Number of additional objects to instantiate if we're running out of them
 
-//#define GC_DEBUG // Debug garbage collection
-//#define GC_DEBUG_VERBOSE // Debug garbage verbosely
-
 #undef DEBUG_segMan // Define to turn on debugging
 
 SegManager::SegManager(ResourceManager *resMan) {
@@ -133,34 +130,33 @@
 	return (Script *)mem;
 }
 
-void Script::setScriptSize(int script_nr, ResourceManager *_resMan) {
-	Script &scr = *this;	// FIXME: Hack
-	Resource *script = _resMan->findResource(ResourceId(kResourceTypeScript, script_nr), 0);
-	Resource *heap = _resMan->findResource(ResourceId(kResourceTypeHeap, script_nr), 0);
-	bool oldScriptHeader = (_resMan->sciVersion() == SCI_VERSION_0_EARLY);
+void Script::setScriptSize(int script_nr, ResourceManager *resMan) {
+	Resource *script = resMan->findResource(ResourceId(kResourceTypeScript, script_nr), 0);
+	Resource *heap = resMan->findResource(ResourceId(kResourceTypeHeap, script_nr), 0);
+	bool oldScriptHeader = (_sciVersion == SCI_VERSION_0_EARLY);
 
-	scr._scriptSize = script->size;
-	scr._heapSize = 0; // Set later
+	_scriptSize = script->size;
+	_heapSize = 0; // Set later
 
-	if (!script || (_resMan->sciVersion() >= SCI_VERSION_1_1 && !heap)) {
+	if (!script || (_sciVersion >= SCI_VERSION_1_1 && !heap)) {
 		error("SegManager::setScriptSize: failed to load %s", !script ? "script" : "heap");
 	}
 	if (oldScriptHeader) {
-		scr._bufSize = script->size + READ_LE_UINT16(script->data) * 2;
+		_bufSize = script->size + READ_LE_UINT16(script->data) * 2;
 		//locals_size = READ_LE_UINT16(script->data) * 2;
-	} else if (_resMan->sciVersion() < SCI_VERSION_1_1) {
-		scr._bufSize = script->size;
+	} else if (_sciVersion < SCI_VERSION_1_1) {
+		_bufSize = script->size;
 	} else {
-		scr._bufSize = script->size + heap->size;
-		scr._heapSize = heap->size;
+		_bufSize = script->size + heap->size;
+		_heapSize = heap->size;
 
 		// Ensure that the start of the heap resource can be word-aligned.
 		if (script->size & 2) {
-			scr._bufSize++;
-			scr._scriptSize++;
+			_bufSize++;
+			_scriptSize++;
 		}
 
-		if (scr._bufSize > 65535) {
+		if (_bufSize > 65535) {
 			error("Script and heap sizes combined exceed 64K."
 			          "This means a fundamental design bug was made in SCI\n"
 			          "regarding SCI1.1 games.\nPlease report this so it can be"
@@ -189,20 +185,19 @@
 	return 1;
 }
 
-bool SegManager::scriptIsMarkedAsDeleted(SegmentId seg) {
-	Script *scr = getScriptIfLoaded(seg);
-	if (!scr)
+bool SegManager::isHeapObject(reg_t pos) {
+	Object *obj = getObject(pos);
+	if (obj == NULL)
 		return false;
-	return scr->_markedAsDeleted;
+	if (obj->flags & OBJECT_FLAG_FREED)
+		return false;
+	Script *scr = getScriptIfLoaded(pos.segment);
+	return !(scr && scr->_markedAsDeleted);
 }
 
-
-int SegManager::deallocateScript(int script_nr) {
+void SegManager::deallocateScript(int script_nr) {
 	SegmentId seg = getScriptSegment(script_nr);
-
 	deallocate(seg, true);
-
-	return 1;
 }
 
 Script *SegManager::getScript(const SegmentId seg) {
@@ -297,10 +292,6 @@
 	return true;
 }
 
-bool SegManager::scriptIsLoaded(SegmentId seg) {
-	return getScriptIfLoaded(seg) != 0;
-}
-
 void SegManager::setExportAreWide(bool flag) {
 	_exportsAreWide = flag;
 }
@@ -346,15 +337,13 @@
 }
 
 void Script::scriptRelocate(reg_t block) {
-	Script *scr = this;	// FIXME: Hack
-
-	VERIFY(block.offset < (uint16)scr->_bufSize && READ_LE_UINT16(scr->_buf + block.offset) * 2 + block.offset < (uint16)scr->_bufSize,
+	VERIFY(block.offset < (uint16)_bufSize && READ_LE_UINT16(_buf + block.offset) * 2 + block.offset < (uint16)_bufSize,
 	       "Relocation block outside of script\n");
 
-	int count = READ_LE_UINT16(scr->_buf + block.offset);
+	int count = READ_LE_UINT16(_buf + block.offset);
 
 	for (int i = 0; i <= count; i++) {
-		int pos = READ_LE_UINT16(scr->_buf + block.offset + 2 + (i * 2));
+		int pos = READ_LE_UINT16(_buf + block.offset + 2 + (i * 2));
 		if (!pos)
 			continue; // FIXME: A hack pending investigation
 
@@ -362,26 +351,26 @@
 			bool done = false;
 			uint k;
 
-			for (k = 0; !done && k < scr->_objects.size(); k++) {
-				if (relocateObject(&scr->_objects[k], block.segment, pos))
+			for (k = 0; !done && k < _objects.size(); k++) {
+				if (relocateObject(&_objects[k], block.segment, pos))
 					done = true;
 			}
 
-			for (k = 0; !done && k < scr->_codeBlocks.size(); k++) {
-				if (pos >= scr->_codeBlocks[k].pos.offset &&
-				        pos < scr->_codeBlocks[k].pos.offset + scr->_codeBlocks[k].size)
+			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) {
 				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 (scr->_localsBlock)
-					printf("- locals: %d at %04x\n", scr->_localsBlock->_locals.size(), scr->_localsOffset);
+				if (_localsBlock)
+					printf("- locals: %d at %04x\n", _localsBlock->_locals.size(), _localsOffset);
 				else
 					printf("- No locals\n");
-				for (k = 0; k < scr->_objects.size(); k++)
-					printf("- obj#%d at %04x w/ %d vars\n", k, scr->_objects[k].pos.offset, scr->_objects[k]._variables.size());
+				for (k = 0; k < _objects.size(); k++)
+					printf("- obj#%d at %04x w/ %d vars\n", k, _objects[k].pos.offset, _objects[k]._variables.size());
 				// SQ3 script 71 has broken relocation entries.
 				printf("Trying to continue anyway...\n");
 			}
@@ -390,38 +379,35 @@
 }
 
 void Script::heapRelocate(reg_t block) {
-	Script *scr = this;	// FIXME: Hack
-
-	VERIFY(block.offset < (uint16)scr->_heapSize && READ_LE_UINT16(scr->_heapStart + block.offset) * 2 + block.offset < (uint16)scr->_bufSize,
+	VERIFY(block.offset < (uint16)_heapSize && READ_LE_UINT16(_heapStart + block.offset) * 2 + block.offset < (uint16)_bufSize,
 	       "Relocation block outside of script\n");
 
-	if (scr->_relocated)
+	if (_relocated)
 		return;
-	scr->_relocated = true;
-	int count = READ_LE_UINT16(scr->_heapStart + block.offset);
+	_relocated = true;
+	int count = READ_LE_UINT16(_heapStart + block.offset);
 
 	for (int i = 0; i < count; i++) {
-		int pos = READ_LE_UINT16(scr->_heapStart + block.offset + 2 + (i * 2)) + scr->_scriptSize;
+		int pos = READ_LE_UINT16(_heapStart + block.offset + 2 + (i * 2)) + _scriptSize;
 
 		if (!relocateLocal(block.segment, pos)) {
 			bool done = false;
 			uint k;
 
-			for (k = 0; !done && k < scr->_objects.size(); k++) {
-				if (relocateObject(&scr->_objects[k], block.segment, pos))
+			for (k = 0; !done && k < _objects.size(); k++) {
+				if (relocateObject(&_objects[k], block.segment, pos))
 					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 (scr->_localsBlock)
-					printf("- locals: %d at %04x\n", scr->_localsBlock->_locals.size(), scr->_localsOffset);
+				if (_localsBlock)
+					printf("- locals: %d at %04x\n", _localsBlock->_locals.size(), _localsOffset);
 				else
 					printf("- No locals\n");
-				for (k = 0; k < scr->_objects.size(); k++)
-					printf("- obj#%d at %04x w/ %d vars\n", k, scr->_objects[k].pos.offset, scr->_objects[k]._variables.size());
-				printf("Triggering breakpoint...\n");
+				for (k = 0; k < _objects.size(); k++)
+					printf("- obj#%d at %04x w/ %d vars\n", k, _objects[k].pos.offset, _objects[k]._variables.size());
 				error("Breakpoint in %s, line %d", __FILE__, __LINE__);
 			}
 		}
@@ -480,16 +466,14 @@
 	SciVersion version = _sciVersion;	// for the offset defines
 	uint base = obj_pos.offset - SCRIPT_OBJECT_MAGIC_OFFSET;
 
-	Script *scr = this;	// FIXME: Hack
+	VERIFY(base < _bufSize, "Attempt to initialize object beyond end of script\n");
 
-	VERIFY(base < scr->_bufSize, "Attempt to initialize object beyond end of script\n");
+	obj = allocateObject(base);
 
-	obj = scr->allocateObject(base);
+	VERIFY(base + SCRIPT_FUNCTAREAPTR_OFFSET  < _bufSize, "Function area pointer stored beyond end of script\n");
 
-	VERIFY(base + SCRIPT_FUNCTAREAPTR_OFFSET  < scr->_bufSize, "Function area pointer stored beyond end of script\n");
-
 	{
-		byte *data = (byte *)(scr->_buf + base);
+		byte *data = (byte *)(_buf + base);
 		int funct_area = READ_LE_UINT16(data + SCRIPT_FUNCTAREAPTR_OFFSET);
 		int variables_nr;
 		int functions_nr;
@@ -499,7 +483,7 @@
 		obj->flags = 0;
 		obj->pos = make_reg(obj_pos.segment, base);
 
-		VERIFY(base + funct_area < scr->_bufSize, "Function area pointer references beyond end of script");
+		VERIFY(base + funct_area < _bufSize, "Function area pointer references beyond end of script");
 
 		variables_nr = READ_LE_UINT16(data + SCRIPT_SELECTORCTR_OFFSET);
 		functions_nr = READ_LE_UINT16(data + funct_area - 2);
@@ -507,12 +491,12 @@
 
 		VERIFY(base + funct_area + functions_nr * 2
 		       // add again for classes, since those also store selectors
-		       + (is_class ? functions_nr * 2 : 0) < scr->_bufSize, "Function area extends beyond end of script");
+		       + (is_class ? functions_nr * 2 : 0) < _bufSize, "Function area extends beyond end of script");
 
 		obj->_variables.resize(variables_nr);
 
 		obj->methods_nr = functions_nr;
-		obj->base = scr->_buf;
+		obj->base = _buf;
 		obj->base_obj = data;
 		obj->base_method = (uint16 *)(data + funct_area);
 		obj->base_vars = NULL;
@@ -528,18 +512,16 @@
 	Object *obj;
 	uint base = obj_pos.offset;
 
-	Script *scr = this;	// FIXME: Hack
+	VERIFY(base < _bufSize, "Attempt to initialize object beyond end of script\n");
 
-	VERIFY(base < scr->_bufSize, "Attempt to initialize object beyond end of script\n");
+	obj = allocateObject(base);
 
-	obj = scr->allocateObject(base);
+	VERIFY(base + SCRIPT_FUNCTAREAPTR_OFFSET < _bufSize, "Function area pointer stored beyond end of script\n");
 
-	VERIFY(base + SCRIPT_FUNCTAREAPTR_OFFSET < scr->_bufSize, "Function area pointer stored beyond end of script\n");
-
 	{
-		byte *data = (byte *)(scr->_buf + base);
-		uint16 *funct_area = (uint16 *)(scr->_buf + READ_LE_UINT16(data + 6));
-		uint16 *prop_area = (uint16 *)(scr->_buf + READ_LE_UINT16(data + 4));
+		byte *data = (byte *)(_buf + base);
+		uint16 *funct_area = (uint16 *)(_buf + READ_LE_UINT16(data + 6));
+		uint16 *prop_area = (uint16 *)(_buf + READ_LE_UINT16(data + 4));
 		int variables_nr;
 		int functions_nr;
 		int is_class;
@@ -548,7 +530,7 @@
 		obj->flags = 0;
 		obj->pos = obj_pos;
 
-		VERIFY((byte *) funct_area < scr->_buf + scr->_bufSize, "Function area pointer references beyond end of script");
+		VERIFY((byte *) funct_area < _buf + _bufSize, "Function area pointer references beyond end of script");
 
 		variables_nr = READ_LE_UINT16(data + 2);
 		functions_nr = READ_LE_UINT16(funct_area);
@@ -557,13 +539,13 @@
 		obj->base_method = funct_area;
 		obj->base_vars = prop_area;
 
-		VERIFY(((byte *) funct_area + functions_nr) < scr->_buf + scr->_bufSize, "Function area extends beyond end of script");
+		VERIFY(((byte *) funct_area + functions_nr) < _buf + _bufSize, "Function area extends beyond end of script");
 
 		obj->variable_names_nr = variables_nr;
 		obj->_variables.resize(variables_nr);
 
 		obj->methods_nr = functions_nr;
-		obj->base = scr->_buf;
+		obj->base = _buf;
 		obj->base_obj = data;
 
 		for (i = 0; i < variables_nr; i++)

Modified: scummvm/trunk/engines/sci/engine/seg_manager.h
===================================================================
--- scummvm/trunk/engines/sci/engine/seg_manager.h	2009-09-16 23:55:11 UTC (rev 44128)
+++ scummvm/trunk/engines/sci/engine/seg_manager.h	2009-09-17 00:44:22 UTC (rev 44129)
@@ -76,17 +76,10 @@
 	/**
 	 * Forcefully deallocate a previously allocated script.
 	 * @param script_nr		number of the script to deallocate
-	 * @return				1 on success, 0 on failure
 	 */
-	int deallocateScript(int script_nr);
+	void deallocateScript(int script_nr);
 
 	/**
-	 * Determines whether a script has been loaded yet.
-	 * @param seg	ID of the script segment to check for
-	 */
-	bool scriptIsLoaded(SegmentId seg);
-
-	/**
 	 * Validate whether the specified public function is exported by 
 	 * the script in the specified segment.
 	 * @param pubfunct		Index of the function to validate
@@ -162,17 +155,6 @@
 	 */
 	void setExportAreWide(bool flag);
 
-	/**
-	 * Determines whether the script referenced by the indicated segment 
-	 * is marked as being deleted.
-	 * Will return 0 when applied to an invalid or non-script seg.
-	 * @param seg	Segment ID of the script to investigate
-	 * @return		1 iff seg points to a script and the segment is 
-	 * 				deleted, 0 otherwise
-	 */
-	bool scriptIsMarkedAsDeleted(SegmentId seg);
-
-
 	// 2. Clones
 
 	/**
@@ -352,6 +334,9 @@
 	 */
 	bool isObject(reg_t obj) { return getObject(obj) != NULL; }
 
+	// TODO: document this
+	bool isHeapObject(reg_t pos);
+
 	/**
 	 * Determines the name of an object
 	 * @param[in] pos	Location (segment, offset) of the object


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