SCI: Fix support for 32-bit SCI3 script offsets

8 files changed, 58 insertions, 61 deletions

diff --git a/engines/sci/engine/kscripts.cpp b/engines/sci/engine/kscripts.cpp
index a7c0d87715..30a1fab71a 100644
--- a/engines/sci/engine/kscripts.cpp
+++ b/engines/sci/engine/kscripts.cpp
@@ -262,7 +262,10 @@ reg_t kScriptID(EngineState *s, int argc, reg_t *argv) {
 		s->variables[VAR_GLOBAL][kGlobalVarSpeed] = make_reg(0, 6);
 	}
 
-	return make_reg(scriptSeg, address);
+	reg_t addr;
+	addr.setSegment(scriptSeg);
+	addr.setOffset(address);
+	return addr;
 }
 
 reg_t kDisposeScript(EngineState *s, int argc, reg_t *argv) {
diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index b507e36a77..177ff9854b 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -298,22 +298,22 @@ bool Object::mustSetViewVisible(const int index) const {
 void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVariables) {
 	const SciSpan<const byte> groupInfo = _baseObj.subspan(16);
 	const SciSpan<const byte> selectorBase = groupInfo.subspan(EXTRA_GROUPS * 32 * 2);
-	int groups = g_sci->getKernel()->getSelectorNamesSize()/32;
-	int methods, properties;
 
+	int numGroups = g_sci->getKernel()->getSelectorNamesSize() / 32;
 	if (g_sci->getKernel()->getSelectorNamesSize() % 32)
-		++groups;
+		++numGroups;
 
-	_mustSetViewVisible.resize(groups);
+	_mustSetViewVisible.resize(numGroups);
 
-	methods = properties = 0;
+	int numMethods = 0;
+	int numProperties = 0;
 
 	// Selectors are divided into groups of 32, of which the first
 	// two selectors are always reserved (because their storage
 	// space is used by the typeMask).
 	// We don't know beforehand how many methods and properties
 	// there are, so we count them first.
-	for (int groupNr = 0; groupNr < groups; ++groupNr) {
+	for (int groupNr = 0; groupNr < numGroups; ++groupNr) {
 		byte groupLocation = groupInfo[groupNr];
 		const SciSpan<const byte> seeker = selectorBase.subspan(groupLocation * 32 * 2);
 
@@ -326,9 +326,9 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 			for (int bit = 2; bit < 32; ++bit) {
 				int value = seeker.getUint16SEAt(bit * 2);
 				if (typeMask & (1 << bit)) { // Property
-					++properties;
+					++numProperties;
 				} else if (value != 0xffff) { // Method
-					++methods;
+					++numMethods;
 				} else {
 					// Undefined selector
 				}
@@ -337,16 +337,15 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 			_mustSetViewVisible[groupNr] = false;
 	}
 
-	_variables.resize(properties);
-	_propertyOffsetsSci3.resize(properties);
-	_baseVars.resize(properties);
-//	uint16 *methodOffsets = (uint16 *)malloc(sizeof(uint16) * 2 * methods);
-	int propertyCounter = 0;
-	int methodCounter = 0;
+	_methodCount = numMethods;
+	_variables.resize(numProperties);
+	_baseVars.resize(numProperties);
+	_propertyOffsetsSci3.resize(numProperties);
 
 	// Go through the whole thing again to get the property values
 	// and method pointers
-	for (int groupNr = 0; groupNr < groups; ++groupNr) {
+	int propertyCounter = 0;
+	for (int groupNr = 0; groupNr < numGroups; ++groupNr) {
 		byte groupLocation = groupInfo[groupNr];
 		const SciSpan<const byte> seeker = selectorBase.subspan(groupLocation * 32 * 2);
 
@@ -367,13 +366,12 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 					++propertyCounter;
 				} else if (value != 0xffff) { // Method
 					_baseMethod.push_back(groupBaseId + bit);
-					_baseMethod.push_back(value + buf.getUint32SEAt(0));
-//					methodOffsets[methodCounter] = (seeker + bit * 2) - buf;
-					++methodCounter;
+					const uint32 offset = value + buf.getUint32SEAt(0);
+					assert(offset <= kOffsetMask);
+					_baseMethod.push_back(offset);
 				} else {
 					// Undefined selector
 				}
-
 			}
 		}
 	}
@@ -382,8 +380,6 @@ void Object::initSelectorsSci3(const SciSpan<const byte> &buf, const bool initVa
 		_speciesSelectorSci3 = make_reg(0, _baseObj.getUint16SEAt(4));
 		_superClassPosSci3 = make_reg(0, _baseObj.getUint16SEAt(8));
 	}
-	_methodCount = methods;
-	//_methodOffsetsSci3 = methodOffsets;
 }
 
 } // End of namespace Sci
diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index 1af3b7b645..992b6cdbc2 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -208,7 +208,11 @@ public:
 		uint16 offset = (getSciVersion() < SCI_VERSION_1_1) ? _methodCount + 1 + i : i * 2 + 2;
 		if (getSciVersion() == SCI_VERSION_3)
 			offset--;
-		return make_reg(_pos.getSegment(), _baseMethod[offset]);
+
+		reg_t addr;
+		addr.setSegment(_pos.getSegment());
+		addr.setOffset(_baseMethod[offset]);
+		return addr;
 	}
 
 	Selector getFuncSelector(uint16 i) const {
@@ -258,7 +262,7 @@ public:
 
 	void cloneFromObject(const Object *obj) {
 		_baseObj = obj ? obj->_baseObj : SciSpan<const byte>();
-		_baseMethod = obj ? obj->_baseMethod : Common::Array<uint16>();
+		_baseMethod = obj ? obj->_baseMethod : Common::Array<uint32>();
 		_baseVars = obj ? obj->_baseVars : Common::Array<uint16>();
 	}
 
@@ -285,14 +289,16 @@ private:
 	SciSpan<const byte> _baseObj;
 
 	/**
-	 * A lookup table from a property index to its corresponding selector number.
+	 * A lookup table from a property index to its corresponding selector
+	 * number.
 	 */
 	Common::Array<uint16> _baseVars;
 
 	/**
 	 * A lookup table from a method index to its corresponding selector number.
+	 * In SCI3, the table contains selector + offset in pairs.
 	 */
-	Common::Array<uint16> _baseMethod;
+	Common::Array<uint32> _baseMethod;
 
 	/**
 	 * A lookup table from a property index to the property's current value.
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index 8479e43479..fa17dc0419 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -113,22 +113,9 @@ void Script::load(int script_nr, ResourceManager *resMan, ScriptPatcher *scriptP
 			error("Script and heap sizes combined exceed 64K. This means a fundamental "
 					"design bug was made regarding SCI1.1 and newer games.\n"
 					"Please report this error to the ScummVM team");
-	} else if (getSciVersion() == SCI_VERSION_3) {
-		// Check for scripts over 64KB. These won't work with the current 16-bit address
-		// scheme. We need an overlaying mechanism, or a mechanism to split script parts
-		// in different segments to handle these. For now, simply stop when such a script
-		// is found.
-		//
-		// Known large SCI 3 scripts are:
-		// Lighthouse: 9, 220, 270, 351, 360, 490, 760, 765, 800
-		// LSL7: 240, 511, 550
-		// Phantasmagoria 2: none (hooray!)
-		// RAMA: 70
-		//
-		// TODO: Remove this once such a mechanism is in place
-		if (script->size() > 65535)
-			warning("TODO: SCI script %d is over 64KB - it's %u bytes long. This can't "
-			      "be fully handled at the moment", script_nr, script->size());
+	} else if (getSciVersion() == SCI_VERSION_3 && script->size() > 0x3FFFF) {
+		error("Script %d size exceeds 256K (it is %u bytes).\n"
+			  "Please report this error to the ScummVM team", script_nr, script->size());
 	}
 
 	uint extraLocalsWorkaround = 0;
@@ -640,14 +627,14 @@ SciSpan<const byte> Script::getSci3ObjectsPointer() {
 	return ptr;
 }
 
-Object *Script::getObject(uint16 offset) {
+Object *Script::getObject(uint32 offset) {
 	if (_objects.contains(offset))
 		return &_objects[offset];
 	else
 		return 0;
 }
 
-const Object *Script::getObject(uint16 offset) const {
+const Object *Script::getObject(uint32 offset) const {
 	if (_objects.contains(offset))
 		return &_objects[offset];
 	else
@@ -866,7 +853,7 @@ SciSpan<const byte> Script::findBlockSCI0(ScriptObjectTypes type, bool findLastB
 
 // memory operations
 
-bool Script::isValidOffset(uint16 offset) const {
+bool Script::isValidOffset(uint32 offset) const {
 	return offset < _buf->size();
 }
 
@@ -995,7 +982,10 @@ void Script::initializeClasses(SegManager *segMan) {
 						  species, species, segMan->classTableSize(), segMan->classTableSize(), _nr);
 
 			SegmentId segmentId = segMan->getScriptSegment(_nr);
-			segMan->setClassOffset(species, make_reg(segmentId, classpos));
+			reg_t classOffset;
+			classOffset.setSegment(segmentId);
+			classOffset.setOffset(classpos);
+			segMan->setClassOffset(species, classOffset);
 		}
 
 		seeker += seeker.getUint16SEAt(2) * mult;
@@ -1182,7 +1172,7 @@ Common::Array<reg_t> Script::listObjectReferences() const {
 	return tmp;
 }
 
-bool Script::offsetIsObject(uint16 offset) const {
+bool Script::offsetIsObject(uint32 offset) const {
 	return _buf->getUint16SEAt(offset + SCRIPT_OBJECT_MAGIC_OFFSET) == SCRIPT_OBJECT_MAGIC_NUMBER;
 }
 
diff --git a/engines/sci/engine/script.h b/engines/sci/engine/script.h
index 01f3e788b7..2be3fd0264 100644
--- a/engines/sci/engine/script.h
+++ b/engines/sci/engine/script.h
@@ -48,7 +48,7 @@ enum ScriptObjectTypes {
 	SCI_OBJ_LOCALVARS
 };
 
-typedef Common::HashMap<uint16, Object> ObjMap;
+typedef Common::HashMap<uint32, Object> ObjMap;
 
 enum ScriptOffsetEntryTypes {
 	SCI_SCR_OFFSET_TYPE_OBJECT = 0, // classes are handled by this type as well
@@ -114,7 +114,7 @@ public:
 	void syncLocalsBlock(SegManager *segMan);
 	ObjMap &getObjectMap() { return _objects; }
 	const ObjMap &getObjectMap() const { return _objects; }
-	bool offsetIsObject(uint16 offset) const;
+	bool offsetIsObject(uint32 offset) const;
 
 public:
 	Script();
@@ -123,7 +123,7 @@ public:
 	void freeScript();
 	void load(int script_nr, ResourceManager *resMan, ScriptPatcher *scriptPatcher);
 
-	virtual bool isValidOffset(uint16 offset) const;
+	virtual bool isValidOffset(uint32 offset) const;
 	virtual SegmentRef dereference(reg_t pointer);
 	virtual reg_t findCanonicAddress(SegManager *segMan, reg_t sub_addr) const;
 	virtual void freeAtAddress(SegManager *segMan, reg_t sub_addr);
@@ -140,8 +140,8 @@ public:
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 
-	Object *getObject(uint16 offset);
-	const Object *getObject(uint16 offset) const;
+	Object *getObject(uint32 offset);
+	const Object *getObject(uint32 offset) const;
 
 	/**
 	 * Initializes an object within the segment manager
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 54f7d526fc..7d186eb788 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -71,7 +71,9 @@ const char *opcodeNames[] = {
 reg_t disassemble(EngineState *s, reg32_t pos, reg_t objAddr, bool printBWTag, bool printBytecode) {
 	SegmentObj *mobj = s->_segMan->getSegment(pos.getSegment(), SEG_TYPE_SCRIPT);
 	Script *script_entity = NULL;
-	reg_t retval = make_reg(pos.getSegment(), pos.getOffset() + 1);
+	reg_t retval;
+	retval.setSegment(pos.getSegment());
+	retval.setOffset(pos.getOffset() + 1);
 	uint16 param_value = 0xffff; // Suppress GCC warning by setting default value, chose value as invalid to getKernelName etc.
 	uint i = 0;
 	Kernel *kernel = g_sci->getKernel();
diff --git a/engines/sci/engine/segment.cpp b/engines/sci/engine/segment.cpp
index fffa7f4d7e..2b7eba3aa6 100644
--- a/engines/sci/engine/segment.cpp
+++ b/engines/sci/engine/segment.cpp
@@ -194,7 +194,7 @@ SegmentRef DataStack::dereference(reg_t pointer) {
 
 Common::Array<reg_t> DataStack::listAllOutgoingReferences(reg_t object) const {
 	Common::Array<reg_t> tmp;
-	for (int i = 0; i < _capacity; i++)
+	for (uint i = 0; i < _capacity; i++)
 		tmp.push_back(_entries[i]);
 
 	return tmp;
diff --git a/engines/sci/engine/segment.h b/engines/sci/engine/segment.h
index f9d151cc83..344977c391 100644
--- a/engines/sci/engine/segment.h
+++ b/engines/sci/engine/segment.h
@@ -93,7 +93,7 @@ public:
 	 * Check whether the given offset into this memory object is valid,
 	 * i.e., suitable for passing to dereference.
 	 */
-	virtual bool isValidOffset(uint16 offset) const = 0;
+	virtual bool isValidOffset(uint32 offset) const = 0;
 
 	/**
 	 * Dereferences a raw memory pointer.
@@ -149,7 +149,7 @@ struct LocalVariables : public SegmentObj {
 public:
 	LocalVariables(): SegmentObj(SEG_TYPE_LOCALS), script_id(0) { }
 
-	virtual bool isValidOffset(uint16 offset) const {
+	virtual bool isValidOffset(uint32 offset) const {
 		return offset < _locals.size() * 2;
 	}
 	virtual SegmentRef dereference(reg_t pointer);
@@ -161,7 +161,7 @@ public:
 
 /** Data stack */
 struct DataStack : SegmentObj {
-	int _capacity; /**< Number of stack entries */
+	uint _capacity; /**< Number of stack entries */
 	reg_t *_entries;
 
 public:
@@ -171,7 +171,7 @@ public:
 		_entries = NULL;
 	}
 
-	virtual bool isValidOffset(uint16 offset) const {
+	virtual bool isValidOffset(uint32 offset) const {
 		return offset < _capacity * 2;
 	}
 	virtual SegmentRef dereference(reg_t pointer);
@@ -276,7 +276,7 @@ public:
 		}
 	}
 
-	virtual bool isValidOffset(uint16 offset) const {
+	virtual bool isValidOffset(uint32 offset) const {
 		return isValidEntry(offset);
 	}
 
@@ -380,7 +380,7 @@ struct HunkTable : public SegmentObjTable<Hunk> {
 
 // Free-style memory
 struct DynMem : public SegmentObj {
-	int _size;
+	uint _size;
 	Common::String _description;
 	byte *_buf;
 
@@ -391,7 +391,7 @@ public:
 		_buf = NULL;
 	}
 
-	virtual bool isValidOffset(uint16 offset) const {
+	virtual bool isValidOffset(uint32 offset) const {
 		return offset < _size;
 	}
 	virtual SegmentRef dereference(reg_t pointer);