From 0da6b15539c4dbb1371cd18ae77b90688ef3f764 Mon Sep 17 00:00:00 2001
From: md5
Date: Mon, 14 Feb 2011 18:16:35 +0200
Subject: SCI2+: Point out that there is a hack in the text splitting code

This particular hack causes issues in GK1, when talking with Grace, because the
width of the associated plane isn't set correctly.
---
 engines/sci/graphics/frameout.cpp | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'engines/sci')

diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index 5b690f289a..026a2ff405 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -584,6 +584,11 @@ void GfxFrameout::kernelFrameout() {
 					uint16 startX = itemEntry->x + it->planeRect.left;
 					uint16 curY = itemEntry->y + it->planeRect.top;
 					const char *txt = text.c_str();
+					// HACK. The plane sometimes doesn't contain the correct width. This
+					// hack breaks the dialog options when speaking with Grace, but it's
+					// the best we got up to now.
+					// TODO: Remove this, and figure out why the plane in question isn't
+					// initialized correctly (its width is 0).
 					uint16 w = it->planeRect.width() >= 20 ? it->planeRect.width() : _screen->getWidth() - 10;
 					int16 charCount;
 
-- 
cgit v1.2.3


From 31539697dc4551790f8da0b0a20f4f8008b1b1aa Mon Sep 17 00:00:00 2001
From: Matthew Hoops
Date: Mon, 14 Feb 2011 16:01:04 -0500
Subject: SCI: Fix loading pre-version 28 saved games

This is a regression from a9b051beff3157e1aa8
---
 engines/sci/engine/savegame.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

(limited to 'engines/sci')

diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index a59d4c000b..ea56a2faff 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -179,7 +179,7 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
 		mobj->saveLoadWithSerializer(s);
 
 
-		if (type == SEG_TYPE_SCRIPT && s.getVersion() >= 28) {
+		if (type == SEG_TYPE_SCRIPT) {
 			Script *scr = (Script *)mobj;
 
 			// If we are loading a script, perform some extra steps
@@ -196,7 +196,8 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
 			}
 
 			// Sync the script's string heap
-			scr->syncStringHeap(s);
+			if (s.getVersion() >= 28)
+				scr->syncStringHeap(s);
 		}
 	}
 
-- 
cgit v1.2.3


From 8ef4594f9b9016f6b93fc176dd8aea8bdd7d85cf Mon Sep 17 00:00:00 2001
From: md5
Date: Tue, 15 Feb 2011 01:30:33 +0200
Subject: SCI2+: Set the correct segment for SCI32 strings/arrays when loading

This was an omission, observed after a discussion with clone2727
---
 engines/sci/engine/savegame.cpp | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

(limited to 'engines/sci')

diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index ea56a2faff..610698fbd9 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -144,17 +144,15 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
 		SegmentType type = (s.isSaving() && mobj) ? mobj->getType() : SEG_TYPE_INVALID;
 		s.syncAsUint32LE(type);
 
-		// If we were saving and mobj == 0, or if we are loading and this is an
-		// entry marked as empty -> skip to next
-		if (type == SEG_TYPE_INVALID)
+		if (type == SEG_TYPE_HUNK) {
+			// Don't save or load HunkTable segments
 			continue;
-
-		// Don't save or load HunkTable segments
-		if (type == SEG_TYPE_HUNK)
+		} else if (type == SEG_TYPE_INVALID) {
+			// If we were saving and mobj == 0, or if we are loading and this is an
+			// entry marked as empty -> skip to next
 			continue;
-
-		// Don't save or load the obsolete system string segments
-		if (type == 5) {
+		} else if (type == 5) {
+			// Don't save or load the obsolete system string segments
 			if (s.isSaving()) {
 				continue;
 			} else {
@@ -169,6 +167,15 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
 				continue;
 			}
 		}
+#ifdef ENABLE_SCI32
+		else if (type == SEG_TYPE_ARRAY) {
+			// Set the correct segment for SCI32 arrays
+			_arraysSegId = i;
+		} else if (type == SEG_TYPE_STRING) {
+			// Set the correct segment for SCI32 strings
+			_stringSegId = i;
+		}
+#endif
 
 		if (s.isLoading())
 			mobj = SegmentObj::createSegmentObj(type);
@@ -178,7 +185,6 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
 		// Let the object sync custom data
 		mobj->saveLoadWithSerializer(s);
 
-
 		if (type == SEG_TYPE_SCRIPT) {
 			Script *scr = (Script *)mobj;
 
-- 
cgit v1.2.3


From ee09af6a12e26f100a9a0457dcb552707abd4288 Mon Sep 17 00:00:00 2001
From: Matthew Hoops
Date: Mon, 14 Feb 2011 22:38:12 -0500
Subject: SCI: Fix loading SCI32 games

The frames/items in GfxFrameout need to be cleared upon loading
---
 engines/sci/engine/savegame.cpp   | 16 +++++++++++++---
 engines/sci/graphics/frameout.cpp |  6 ++++++
 engines/sci/graphics/frameout.h   |  3 +++
 3 files changed, 22 insertions(+), 3 deletions(-)

(limited to 'engines/sci')

diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 610698fbd9..ab355cebb4 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -47,6 +47,10 @@
 #include "sci/sound/audio.h"
 #include "sci/sound/music.h"
 
+#ifdef ENABLE_SCI32
+#include "sci/graphics/frameout.h"
+#endif
+
 namespace Sci {
 
 
@@ -130,6 +134,13 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
 
 		// Reset _scriptSegMap, to be restored below
 		_scriptSegMap.clear();
+
+#ifdef ENABLE_SCI32
+		// Clear any planes/screen items currently showing so they
+		// don't show up after the load.
+		if (getSciVersion() >= SCI_VERSION_2)
+			g_sci->_gfxFrameout->clear();
+#endif
 	}
 
 	s.skip(4, VER(14), VER(18));		// OBSOLETE: Used to be _exportsAreWide
@@ -166,16 +177,15 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
 				_heap[i] = NULL;	// set as freed
 				continue;
 			}
-		}
 #ifdef ENABLE_SCI32
-		else if (type == SEG_TYPE_ARRAY) {
+		} else if (type == SEG_TYPE_ARRAY) {
 			// Set the correct segment for SCI32 arrays
 			_arraysSegId = i;
 		} else if (type == SEG_TYPE_STRING) {
 			// Set the correct segment for SCI32 strings
 			_stringSegId = i;
-		}
 #endif
+		}
 
 		if (s.isLoading())
 			mobj = SegmentObj::createSegmentObj(type);
diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp
index 026a2ff405..ab4a2c9c1a 100644
--- a/engines/sci/graphics/frameout.cpp
+++ b/engines/sci/graphics/frameout.cpp
@@ -58,6 +58,12 @@ GfxFrameout::GfxFrameout(SegManager *segMan, ResourceManager *resMan, GfxCoordAd
 GfxFrameout::~GfxFrameout() {
 }
 
+void GfxFrameout::clear() {
+	_screenItems.clear();
+	_planes.clear();
+	_planePictures.clear();
+}
+
 void GfxFrameout::kernelAddPlane(reg_t object) {
 	PlaneEntry newPlane;
 
diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index 93d61ba22e..347ecb9424 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -28,6 +28,8 @@
 
 namespace Sci {
 
+class GfxPicture;
+
 struct PlaneEntry {
 	reg_t object;
 	uint16 priority;
@@ -99,6 +101,7 @@ public:
 
 	void addPlanePicture(reg_t object, GuiResourceId pictureId, uint16 startX);
 	void deletePlanePictures(reg_t object);
+	void clear();
 
 private:
 	SegManager *_segMan;
-- 
cgit v1.2.3


From 9505becefa114e9da12d3ef40de16f0a46527a2c Mon Sep 17 00:00:00 2001
From: md5
Date: Tue, 15 Feb 2011 11:25:54 +0200
Subject: SCI: Removed several redundant helper functions

Removed validate_arithmetic(), signed_validate_arithmetic(), validate_unsignedInteger() and validate_signedInteger()
---
 engines/sci/engine/vm.cpp     | 173 ++++++++++++++++++++----------------------
 engines/sci/engine/vm_types.h |  26 +++++++
 2 files changed, 107 insertions(+), 92 deletions(-)

(limited to 'engines/sci')

diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 6d11a1ad8a..9c7f52d28e 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -131,21 +131,6 @@ static StackPtr validate_stack_addr(EngineState *s, StackPtr sp) {
 	return 0;
 }
 
-static int validate_arithmetic(reg_t reg) {
-	if (reg.segment) {
-		// The results of this are likely unpredictable... It most likely means that a kernel function is returning something wrong.
-		// If such an error occurs, we usually need to find the last kernel function called and check its return value.
-		error("[VM] Attempt to read arithmetic value from non-zero segment [%04x]. Address: %04x:%04x", reg.segment, PRINT_REG(reg));
-		return 0;
-	}
-
-	return reg.offset;
-}
-
-static int signed_validate_arithmetic(reg_t reg) {
-	return (int16)validate_arithmetic(reg);
-}
-
 static bool validate_variable(reg_t *r, reg_t *stack_base, int type, int max, int index) {
 	const char *names[4] = {"global", "local", "temp", "param"};
 
@@ -176,20 +161,6 @@ static bool validate_variable(reg_t *r, reg_t *stack_base, int type, int max, in
 	return true;
 }
 
-static bool validate_unsignedInteger(reg_t reg, uint16 &integer) {
-	if (reg.segment)
-		return false;
-	integer = reg.offset;
-	return true;
-}
-
-static bool validate_signedInteger(reg_t reg, int16 &integer) {
-	if (reg.segment)
-		return false;
-	integer = (int16)reg.offset;
-	return true;
-}
-
 extern const char *opcodeNames[]; // from scriptdebug.cpp
 
 static reg_t arithmetic_lookForWorkaround(const byte opcode, const SciWorkaroundEntry *workaroundList, reg_t value1, reg_t value2) {
@@ -427,12 +398,12 @@ ExecStack *send_selector(EngineState *s, reg_t send_obj, reg_t work_obj, StackPt
 	int activeBreakpointTypes = g_sci->_debugState._activeBreakpointTypes;
 
 	while (framesize > 0) {
-		selector = validate_arithmetic(*argp++);
-		argc = validate_arithmetic(*argp);
+		selector = argp->requireUint16();
+		argp++;
+		argc = argp->requireUint16();
 
-		if (argc > 0x800) { // More arguments than the stack could possibly accomodate for
+		if (argc > 0x800)	// More arguments than the stack could possibly accomodate for
 			error("send_selector(): More than 0x800 arguments to function call");
-		}
 
 #ifdef VM_DEBUG_SEND
 		debugN("Send to %04x:%04x (%s), selector %04x (%s):", PRINT_REG(send_obj), 
@@ -1014,8 +985,8 @@ void run_vm(EngineState *s) {
 
 		case op_bnot: { // 0x00 (00)
 			// Binary not
-			int16 value;
-			if (validate_signedInteger(s->r_acc, value))
+			int16 value = s->r_acc.toSint16();
+			if (s->r_acc.isNumber())
 				s->r_acc = make_reg(0, 0xffff ^ value);
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, s->r_acc, NULL_REG);
@@ -1087,8 +1058,9 @@ void run_vm(EngineState *s) {
 
 		case op_mul: { // 0x03 (03)
 			r_temp = POP32();
-			int16 value1, value2;
-			if (validate_signedInteger(s->r_acc, value1) && validate_signedInteger(r_temp, value2))
+			int16 value1 = s->r_acc.toSint16();
+			int16 value2 = r_temp.toSint16();
+			if (s->r_acc.isNumber() && r_temp.isNumber())
 				s->r_acc = make_reg(0, value1 * value2);
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, opcodeMulWorkarounds, s->r_acc, r_temp);
@@ -1097,8 +1069,9 @@ void run_vm(EngineState *s) {
 
 		case op_div: { // 0x04 (04)
 			r_temp = POP32();
-			int16 divisor, dividend;
-			if (validate_signedInteger(s->r_acc, divisor) && validate_signedInteger(r_temp, dividend))
+			int16 divisor = s->r_acc.toSint16();
+			int16 dividend = r_temp.toSint16();
+			if (s->r_acc.isNumber() && r_temp.isNumber())
 				s->r_acc = make_reg(0, (divisor != 0 ? dividend / divisor : 0));
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, opcodeDivWorkarounds, s->r_acc, r_temp);
@@ -1109,8 +1082,9 @@ void run_vm(EngineState *s) {
 			r_temp = POP32();
 
 			if (getSciVersion() <= SCI_VERSION_0_LATE) {
-				uint16 modulo, value;
-				if (validate_unsignedInteger(s->r_acc, modulo) && validate_unsignedInteger(r_temp, value))
+				uint16 modulo = s->r_acc.toUint16();
+				uint16 value = r_temp.toUint16();
+				if (s->r_acc.isNumber() && r_temp.isNumber())
 					s->r_acc = make_reg(0, (modulo != 0 ? value % modulo : 0));
 				else
 					s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, s->r_acc, r_temp);
@@ -1121,8 +1095,10 @@ void run_vm(EngineState *s) {
 				// for simplicity's sake and use the new code for SCI01 and newer
 				// games. Fixes the battlecruiser mini game in SQ5 (room 850),
 				// bug #3035755
-				int16 modulo, value, result;
-				if (validate_signedInteger(s->r_acc, modulo) && validate_signedInteger(r_temp, value)) {
+				int16 modulo = s->r_acc.toSint16();
+				int16 value = r_temp.toSint16();
+				int16 result;
+				if (s->r_acc.isNumber() && r_temp.isNumber()) {
 					modulo = ABS(modulo);
 					result = (modulo != 0 ? value % modulo : 0);
 					if (result < 0)
@@ -1137,8 +1113,9 @@ void run_vm(EngineState *s) {
 		case op_shr: { // 0x06 (06)
 			// Shift right logical
 			r_temp = POP32();
-			uint16 value, shiftCount;
-			if (validate_unsignedInteger(r_temp, value) && validate_unsignedInteger(s->r_acc, shiftCount))
+			uint16 value = r_temp.toUint16();
+			uint16 shiftCount = s->r_acc.toUint16();
+			if (r_temp.isNumber() && s->r_acc.isNumber())
 				s->r_acc = make_reg(0, value >> shiftCount);
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, r_temp, s->r_acc);
@@ -1148,8 +1125,9 @@ void run_vm(EngineState *s) {
 		case op_shl: { // 0x07 (07)
 			// Shift left logical
 			r_temp = POP32();
-			uint16 value, shiftCount;
-			if (validate_unsignedInteger(r_temp, value) && validate_unsignedInteger(s->r_acc, shiftCount))
+			uint16 value = r_temp.toUint16();
+			uint16 shiftCount = s->r_acc.toUint16();
+			if (r_temp.isNumber() && s->r_acc.isNumber())
 				s->r_acc = make_reg(0, value << shiftCount);
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, r_temp, s->r_acc);
@@ -1158,8 +1136,9 @@ void run_vm(EngineState *s) {
 
 		case op_xor: { // 0x08 (08)
 			r_temp = POP32();
-			uint16 value1, value2;
-			if (validate_unsignedInteger(r_temp, value1) && validate_unsignedInteger(s->r_acc, value2))
+			uint16 value1 = r_temp.toUint16();
+			uint16 value2 = s->r_acc.toUint16();
+			if (r_temp.isNumber() && s->r_acc.isNumber())
 				s->r_acc = make_reg(0, value1 ^ value2);
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, r_temp, s->r_acc);
@@ -1168,8 +1147,9 @@ void run_vm(EngineState *s) {
 
 		case op_and: { // 0x09 (09)
 			r_temp = POP32();
-			uint16 value1, value2;
-			if (validate_unsignedInteger(r_temp, value1) && validate_unsignedInteger(s->r_acc, value2))
+			uint16 value1 = r_temp.toUint16();
+			uint16 value2 = s->r_acc.toUint16();
+			if (r_temp.isNumber() && s->r_acc.isNumber())
 				s->r_acc = make_reg(0, value1 & value2);
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, opcodeAndWorkarounds, r_temp, s->r_acc);
@@ -1178,8 +1158,9 @@ void run_vm(EngineState *s) {
 
 		case op_or: { // 0x0a (10)
 			r_temp = POP32();
-			uint16 value1, value2;
-			if (validate_unsignedInteger(r_temp, value1) && validate_unsignedInteger(s->r_acc, value2))
+			uint16 value1 = r_temp.toUint16();
+			uint16 value2 = s->r_acc.toUint16();
+			if (r_temp.isNumber() && s->r_acc.isNumber())
 				s->r_acc = make_reg(0, value1 | value2);
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, opcodeOrWorkarounds, r_temp, s->r_acc);
@@ -1187,8 +1168,8 @@ void run_vm(EngineState *s) {
 		}
 
 		case op_neg: { // 0x0b (11)
-			int16 value;
-			if (validate_signedInteger(s->r_acc, value))
+			int16 value = s->r_acc.toSint16();
+			if (s->r_acc.isNumber())
 				s->r_acc = make_reg(0, -value);
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, s->r_acc, NULL_REG);
@@ -1232,8 +1213,9 @@ void run_vm(EngineState *s) {
 				// Happens in SQ1, room 28, when throwing the water at Orat
 				s->r_acc = make_reg(0, 1);
 			} else {
-				int16 compare1, compare2;
-				if (validate_signedInteger(r_temp, compare1) && validate_signedInteger(s->r_acc, compare2))
+				int16 compare1 = r_temp.toSint16();
+				int16 compare2 = s->r_acc.toSint16();
+				if (r_temp.isNumber() && s->r_acc.isNumber())
 					s->r_acc = make_reg(0, compare1 > compare2);
 				else
 					s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, r_temp, s->r_acc);
@@ -1249,8 +1231,9 @@ void run_vm(EngineState *s) {
 					warning("[VM] Comparing pointers in different segments (%04x:%04x vs. %04x:%04x)", PRINT_REG(r_temp), PRINT_REG(s->r_acc));
 				s->r_acc = make_reg(0, (r_temp.segment == s->r_acc.segment) && r_temp.offset >= s->r_acc.offset);
 			} else {
-				int16 compare1, compare2;
-				if (validate_signedInteger(r_temp, compare1) && validate_signedInteger(s->r_acc, compare2))
+				int16 compare1 = r_temp.toSint16();
+				int16 compare2 = s->r_acc.toSint16();
+				if (r_temp.isNumber() && s->r_acc.isNumber())
 					s->r_acc = make_reg(0, compare1 >= compare2);
 				else
 					s->r_acc = arithmetic_lookForWorkaround(opcode, opcodeGeWorkarounds, r_temp, s->r_acc);
@@ -1272,8 +1255,9 @@ void run_vm(EngineState *s) {
 				// Happens in SQ1, room 58, when giving id-card to robot
 				s->r_acc = make_reg(0, 1);
 			} else {
-				int16 compare1, compare2;
-				if (validate_signedInteger(r_temp, compare1) && validate_signedInteger(s->r_acc, compare2))
+				int16 compare1 = r_temp.toSint16();
+				int16 compare2 = s->r_acc.toSint16();
+				if (r_temp.isNumber() && s->r_acc.isNumber())
 					s->r_acc = make_reg(0, compare1 < compare2);
 				else
 					s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, r_temp, s->r_acc);
@@ -1289,8 +1273,9 @@ void run_vm(EngineState *s) {
 					warning("[VM] Comparing pointers in different segments (%04x:%04x vs. %04x:%04x)", PRINT_REG(r_temp), PRINT_REG(s->r_acc));
 				s->r_acc = make_reg(0, (r_temp.segment == s->r_acc.segment) && r_temp.offset <= s->r_acc.offset);
 			} else {
-				int16 compare1, compare2;
-				if (validate_signedInteger(r_temp, compare1) && validate_signedInteger(s->r_acc, compare2))
+				int16 compare1 = r_temp.toSint16();
+				int16 compare2 = s->r_acc.toSint16();
+				if (r_temp.isNumber() && s->r_acc.isNumber())
 					s->r_acc = make_reg(0, compare1 <= compare2);
 				else
 					s->r_acc = arithmetic_lookForWorkaround(opcode, opcodeLeWorkarounds, r_temp, s->r_acc);
@@ -1317,8 +1302,9 @@ void run_vm(EngineState *s) {
 			else if (r_temp.segment && s->r_acc.segment)
 				s->r_acc = make_reg(0, (r_temp.segment == s->r_acc.segment) && r_temp.offset > s->r_acc.offset);
 			else {
-				uint16 compare1, compare2;
-				if (validate_unsignedInteger(r_temp, compare1) && validate_unsignedInteger(s->r_acc, compare2))
+				uint16 compare1 = r_temp.toUint16();
+				uint16 compare2 = s->r_acc.toUint16();
+				if (r_temp.isNumber() && s->r_acc.isNumber())
 					s->r_acc = make_reg(0, compare1 > compare2);
 				else
 					s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, r_temp, s->r_acc);
@@ -1336,8 +1322,9 @@ void run_vm(EngineState *s) {
 			else if (r_temp.segment && s->r_acc.segment)
 				s->r_acc = make_reg(0, (r_temp.segment == s->r_acc.segment) && r_temp.offset >= s->r_acc.offset);
 			else {
-				uint16 compare1, compare2;
-				if (validate_unsignedInteger(r_temp, compare1) && validate_unsignedInteger(s->r_acc, compare2))
+				uint16 compare1 = r_temp.toUint16();
+				uint16 compare2 = s->r_acc.toUint16();
+				if (r_temp.isNumber() && s->r_acc.isNumber())
 					s->r_acc = make_reg(0, compare1 >= compare2);
 				else
 					s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, r_temp, s->r_acc);
@@ -1356,8 +1343,9 @@ void run_vm(EngineState *s) {
 			else if (r_temp.segment && s->r_acc.segment)
 				s->r_acc = make_reg(0, (r_temp.segment == s->r_acc.segment) && r_temp.offset < s->r_acc.offset);
 			else {
-				uint16 compare1, compare2;
-				if (validate_unsignedInteger(r_temp, compare1) && validate_unsignedInteger(s->r_acc, compare2))
+				uint16 compare1 = r_temp.toUint16();
+				uint16 compare2 = s->r_acc.toUint16();
+				if (r_temp.isNumber() && s->r_acc.isNumber())
 					s->r_acc = make_reg(0, compare1 < compare2);
 				else
 					s->r_acc = arithmetic_lookForWorkaround(opcode, opcodeUltWorkarounds, r_temp, s->r_acc);
@@ -1375,8 +1363,9 @@ void run_vm(EngineState *s) {
 			else if (r_temp.segment && s->r_acc.segment)
 				s->r_acc = make_reg(0, (r_temp.segment == s->r_acc.segment) && r_temp.offset <= s->r_acc.offset);
 			else {
-				uint16 compare1, compare2;
-				if (validate_unsignedInteger(r_temp, compare1) && validate_unsignedInteger(s->r_acc, compare2))
+				uint16 compare1 = r_temp.toUint16();
+				uint16 compare2 = s->r_acc.toUint16();
+				if (r_temp.isNumber() && s->r_acc.isNumber())
 					s->r_acc = make_reg(0, compare1 <= compare2);
 				else
 					s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, r_temp, s->r_acc);
@@ -1446,7 +1435,7 @@ void run_vm(EngineState *s) {
 			xs_new = add_exec_stack_entry(s->_executionStack, make_reg(s->xs->addr.pc.segment,
 											localCallOffset),
 											s->xs->sp, s->xs->objp,
-											(validate_arithmetic(*call_base)) + s->restAdjust,
+											(call_base->requireUint16()) + s->restAdjust,
 											call_base, NULL_SELECTOR, -1, localCallOffset, s->xs->objp,
 											s->_executionStack.size()-1, s->xs->local_segment);
 			s->restAdjust = 0; // Used up the &rest adjustment
@@ -1466,7 +1455,7 @@ void run_vm(EngineState *s) {
 			if (!oldScriptHeader)
 				s->xs->sp -= s->restAdjust;
 
-			int argc = validate_arithmetic(s->xs->sp[0]);
+			int argc = s->xs->sp[0].requireUint16();
 
 			if (!oldScriptHeader)
 				argc += s->restAdjust;
@@ -1663,7 +1652,7 @@ void run_vm(EngineState *s) {
 			r_temp.offset = s->variables[var_number] - s->variablesBase[var_number];
 
 			if (temp & 0x08)  // Add accumulator offset if requested
-				r_temp.offset += signed_validate_arithmetic(s->r_acc);
+				r_temp.offset += s->r_acc.requireSint16();
 
 			r_temp.offset += opparams[1];  // Add index
 			r_temp.offset *= 2; // variables are 16 bit
@@ -1710,8 +1699,8 @@ void run_vm(EngineState *s) {
 		case op_ipToa: { // 0x35 (53)
 			// Increment Property and copy To Accumulator
 			reg_t &opProperty = validate_property(s, obj, opparams[0]);
-			uint16 valueProperty;
-			if (validate_unsignedInteger(opProperty, valueProperty))
+			uint16 valueProperty = opProperty.toUint16();
+			if (opProperty.isNumber())
 				s->r_acc = make_reg(0, valueProperty + 1);
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, NULL, opProperty, NULL_REG);
@@ -1722,8 +1711,8 @@ void run_vm(EngineState *s) {
 		case op_dpToa: { // 0x36 (54)
 			// Decrement Property and copy To Accumulator
 			reg_t &opProperty = validate_property(s, obj, opparams[0]);
-			uint16 valueProperty;
-			if (validate_unsignedInteger(opProperty, valueProperty))
+			uint16 valueProperty = opProperty.toUint16();
+			if (opProperty.isNumber())
 				s->r_acc = make_reg(0, valueProperty - 1);
 			else
 				s->r_acc = arithmetic_lookForWorkaround(opcode, opcodeDptoaWorkarounds, opProperty, NULL_REG);
@@ -1734,8 +1723,8 @@ void run_vm(EngineState *s) {
 		case op_ipTos: { // 0x37 (55)
 			// Increment Property and push to Stack
 			reg_t &opProperty = validate_property(s, obj, opparams[0]);
-			uint16 valueProperty;
-			if (validate_unsignedInteger(opProperty, valueProperty))
+			uint16 valueProperty = opProperty.toUint16();
+			if (opProperty.isNumber())
 				valueProperty++;
 			else
 				valueProperty = arithmetic_lookForWorkaround(opcode, NULL, opProperty, NULL_REG).offset;
@@ -1747,8 +1736,8 @@ void run_vm(EngineState *s) {
 		case op_dpTos: { // 0x38 (56)
 			// Decrement Property and push to Stack
 			reg_t &opProperty = validate_property(s, obj, opparams[0]);
-			uint16 valueProperty;
-			if (validate_unsignedInteger(opProperty, valueProperty))
+			uint16 valueProperty = opProperty.toUint16();
+			if (opProperty.isNumber())
 				valueProperty--;
 			else
 				valueProperty = arithmetic_lookForWorkaround(opcode, NULL, opProperty, NULL_REG).offset;
@@ -1871,8 +1860,8 @@ void run_vm(EngineState *s) {
 			// Load global, local, temp or param variable into the accumulator,
 			// using the accumulator as an additional index
 			var_type = opcode & 0x3; // Gets the variable type: g, l, t or p
-			int16 value;
-			if (!validate_signedInteger(s->r_acc, value))
+			int16 value = s->r_acc.toSint16();
+			if (!s->r_acc.isNumber())
 				value = arithmetic_lookForWorkaround(opcode, opcodeLaiWorkarounds, s->r_acc, NULL_REG).offset;
 			var_number = opparams[0] + value;
 			s->r_acc = READ_VAR(var_type, var_number);
@@ -1886,8 +1875,8 @@ void run_vm(EngineState *s) {
 			// Load global, local, temp or param variable into the stack,
 			// using the accumulator as an additional index
 			var_type = opcode & 0x3; // Gets the variable type: g, l, t or p
-			int16 value;
-			if (!validate_signedInteger(s->r_acc, value))
+			int16 value = s->r_acc.toSint16();
+			if (!s->r_acc.isNumber())
 				value = arithmetic_lookForWorkaround(opcode, opcodeLsiWorkarounds, s->r_acc, NULL_REG).offset;
 			var_number = opparams[0] + value;
 			PUSH32(READ_VAR(var_type, var_number));
@@ -1925,7 +1914,7 @@ void run_vm(EngineState *s) {
 			// of sense otherwise, with acc being used for two things
 			// simultaneously...
 			var_type = opcode & 0x3; // Gets the variable type: g, l, t or p
-			var_number = opparams[0] + signed_validate_arithmetic(s->r_acc);
+			var_number = opparams[0] + s->r_acc.requireSint16();
 			s->r_acc = POP32();
 			WRITE_VAR(var_type, var_number, s->r_acc);
 			break;
@@ -1937,7 +1926,7 @@ void run_vm(EngineState *s) {
 			// Save the stack into the global, local, temp or param variable,
 			// using the accumulator as an additional index
 			var_type = opcode & 0x3; // Gets the variable type: g, l, t or p
-			var_number = opparams[0] + signed_validate_arithmetic(s->r_acc);
+			var_number = opparams[0] + s->r_acc.requireSint16();
 			WRITE_VAR(var_type, var_number, POP32());
 			break;
 
@@ -1983,7 +1972,7 @@ void run_vm(EngineState *s) {
 			// Increment the global, local, temp or param variable and save it
 			// to the accumulator, using the accumulator as an additional index
 			var_type = opcode & 0x3; // Gets the variable type: g, l, t or p
-			var_number = opparams[0] + signed_validate_arithmetic(s->r_acc);
+			var_number = opparams[0] + s->r_acc.requireSint16();
 			r_temp = READ_VAR(var_type, var_number);
 			if (r_temp.segment) {
 				// Pointer arithmetics!
@@ -2000,7 +1989,7 @@ void run_vm(EngineState *s) {
 			// Increment the global, local, temp or param variable and save it
 			// to the stack, using the accumulator as an additional index
 			var_type = opcode & 0x3; // Gets the variable type: g, l, t or p
-			var_number = opparams[0] + signed_validate_arithmetic(s->r_acc);
+			var_number = opparams[0] + s->r_acc.requireSint16();
 			r_temp = READ_VAR(var_type, var_number);
 			if (r_temp.segment) {
 				// Pointer arithmetics!
@@ -2053,7 +2042,7 @@ void run_vm(EngineState *s) {
 			// Decrement the global, local, temp or param variable and save it
 			// to the accumulator, using the accumulator as an additional index
 			var_type = opcode & 0x3; // Gets the variable type: g, l, t or p
-			var_number = opparams[0] + signed_validate_arithmetic(s->r_acc);
+			var_number = opparams[0] + s->r_acc.requireSint16();
 			r_temp = READ_VAR(var_type, var_number);
 			if (r_temp.segment) {
 				// Pointer arithmetics!
@@ -2070,7 +2059,7 @@ void run_vm(EngineState *s) {
 			// Decrement the global, local, temp or param variable and save it
 			// to the stack, using the accumulator as an additional index
 			var_type = opcode & 0x3; // Gets the variable type: g, l, t or p
-			var_number = opparams[0] + signed_validate_arithmetic(s->r_acc);
+			var_number = opparams[0] + s->r_acc.requireSint16();
 			r_temp = READ_VAR(var_type, var_number);
 			if (r_temp.segment) {
 				// Pointer arithmetics!
diff --git a/engines/sci/engine/vm_types.h b/engines/sci/engine/vm_types.h
index edf35a122a..cc1f9b2685 100644
--- a/engines/sci/engine/vm_types.h
+++ b/engines/sci/engine/vm_types.h
@@ -56,6 +56,32 @@ struct reg_t {
 	int16 toSint16() const {
 		return (int16) offset;
 	}
+
+	uint16 requireUint16() const {
+		if (isNumber())
+			return toUint16();
+		else
+			// The results of this are likely unpredictable... It most likely
+			// means that a kernel function is returning something wrong. If
+			// such an error occurs, we usually need to find the last kernel
+			// function called and check its return value.
+			error("[VM] Attempt to read unsigned arithmetic value from non-zero segment %04x. Offset: %04x", segment, offset);
+	}
+
+	int16 requireSint16() const {
+		if (isNumber())
+			return toSint16();
+		else
+			// The results of this are likely unpredictable... It most likely
+			// means that a kernel function is returning something wrong. If
+			// such an error occurs, we usually need to find the last kernel
+			// function called and check its return value.
+			error("[VM] Attempt to read signed arithmetic value from non-zero segment %04x. Offset: %04x", segment, offset);
+	}
+
+	bool isNumber() const {
+		return !segment;
+	}
 };
 
 static inline reg_t make_reg(SegmentId segment, uint16 offset) {
-- 
cgit v1.2.3


From bd64c5078c3d07bb9cf80ef8e72f151bec8beec4 Mon Sep 17 00:00:00 2001
From: md5
Date: Tue, 15 Feb 2011 15:43:10 +0200
Subject: SCI: Cleaned up kMapKeyToDir and removed an incorrect heuristic

The heuristic in question was used to detect the pseudo mouse control
functionality, however the change in controls seems to have occurred with the
transition to cursor views. Fixes keypad control in Conquest of the Longbow.
Moreover, the code also checked for key scan code 76 when checking for the middle
keypad button, which seems to be a mistake, as that case never occurred.
---
 engines/sci/engine/kevent.cpp | 71 ++++++++++++++++---------------------------
 engines/sci/event.cpp         | 20 ------------
 engines/sci/event.h           |  3 --
 3 files changed, 27 insertions(+), 67 deletions(-)

(limited to 'engines/sci')

diff --git a/engines/sci/engine/kevent.cpp b/engines/sci/engine/kevent.cpp
index 58de38bd8e..e5a9931605 100644
--- a/engines/sci/engine/kevent.cpp
+++ b/engines/sci/engine/kevent.cpp
@@ -38,8 +38,6 @@
 
 namespace Sci {
 
-#define SCI_VARIABLE_GAME_SPEED 3
-
 reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 	int mask = argv[0].toUint16();
 	reg_t obj = argv[1];
@@ -188,57 +186,42 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 	return s->r_acc;
 }
 
+struct KeyDirMapping {
+	uint16 key;
+	uint16 direction;
+};
+
+const KeyDirMapping keyToDirMap[] = {
+	{ SCI_KEY_HOME,   8 }, { SCI_KEY_UP,     1 }, { SCI_KEY_PGUP,   2 },
+	{ SCI_KEY_LEFT,   7 }, { SCI_KEY_CENTER, 0 }, { SCI_KEY_RIGHT,  3 },
+	{ SCI_KEY_END,    6 }, { SCI_KEY_DOWN,   5 }, { SCI_KEY_PGDOWN, 4 },
+};
+
 reg_t kMapKeyToDir(EngineState *s, int argc, reg_t *argv) {
 	reg_t obj = argv[0];
 	SegManager *segMan = s->_segMan;
 
 	if (readSelectorValue(segMan, obj, SELECTOR(type)) == SCI_EVENT_KEYBOARD) { // Keyboard
-		int mover = -1;
-		switch (readSelectorValue(segMan, obj, SELECTOR(message))) {
-		case SCI_KEY_HOME:
-			mover = 8;
-			break;
-		case SCI_KEY_UP:
-			mover = 1;
-			break;
-		case SCI_KEY_PGUP:
-			mover = 2;
-			break;
-		case SCI_KEY_LEFT:
-			mover = 7;
-			break;
-		case SCI_KEY_CENTER:
-		case 76:
-			mover = 0;
-			break;
-		case SCI_KEY_RIGHT:
-			mover = 3;
-			break;
-		case SCI_KEY_END:
-			mover = 6;
-			break;
-		case SCI_KEY_DOWN:
-			mover = 5;
-			break;
-		case SCI_KEY_PGDOWN:
-			mover = 4;
-			break;
-		default:
-			break;
+		uint16 message = readSelectorValue(segMan, obj, SELECTOR(message));
+		uint16 eventType = SCI_EVENT_DIRECTION;
+		// Check if the game is using cursor views. These games allowed control
+		// of the mouse cursor via the keyboard controls (the so called
+		// "PseudoMouse" functionality in script 933).
+		if (g_sci->_features->detectSetCursorType() == SCI_VERSION_1_1)
+			eventType |= SCI_EVENT_KEYBOARD;
+
+		for (int i = 0; i < 9; i++) {
+			if (keyToDirMap[i].key == message) {
+				writeSelectorValue(segMan, obj, SELECTOR(type), eventType);
+				writeSelectorValue(segMan, obj, SELECTOR(message), keyToDirMap[i].direction);
+				return TRUE_REG;	// direction mapped
+			}
 		}
 
-		if (mover >= 0) {
-			if (g_sci->getEventManager()->getUsesNewKeyboardDirectionType())
-				writeSelectorValue(segMan, obj, SELECTOR(type), SCI_EVENT_KEYBOARD | SCI_EVENT_DIRECTION);
-			else
-				writeSelectorValue(segMan, obj, SELECTOR(type), SCI_EVENT_DIRECTION);
-			writeSelectorValue(segMan, obj, SELECTOR(message), mover);
-			return make_reg(0, 1);
-		} else
-			return NULL_REG;
+		return NULL_REG;	// unknown direction
 	}
 
-	return s->r_acc;
+	return s->r_acc;	// no keyboard event to map, leave accumulator unchanged
 }
 
 reg_t kGlobalToLocal(EngineState *s, int argc, reg_t *argv) {
diff --git a/engines/sci/event.cpp b/engines/sci/event.cpp
index 0c17db6028..d607a5314f 100644
--- a/engines/sci/event.cpp
+++ b/engines/sci/event.cpp
@@ -36,31 +36,11 @@
 namespace Sci {
 
 EventManager::EventManager(bool fontIsExtended) : _fontIsExtended(fontIsExtended), _modifierStates(0) {
-
-	if (getSciVersion() >= SCI_VERSION_1_MIDDLE) {
-		_usesNewKeyboardDirectionType = true;
-	} else if (getSciVersion() <= SCI_VERSION_01) {
-		_usesNewKeyboardDirectionType = false;
-	} else {
-		// they changed this somewhere inbetween SCI1EGA/EARLY
-		_usesNewKeyboardDirectionType = false;
-
-		// We are looking if script 933 exists, that one has the PseudoMouse class in it that handles it
-		//  The good thing is that PseudoMouse seems to only exists in games that use the new method
-		if (g_sci->getResMan()->testResource(ResourceId(kResourceTypeScript, 933)))
-			_usesNewKeyboardDirectionType = true;
-		// Checking the keyboard driver size in here would also be a valid method, but the driver is only available
-		//  in PC versions of the game
-	}
 }
 
 EventManager::~EventManager() {
 }
 
-bool EventManager::getUsesNewKeyboardDirectionType() {
-	return _usesNewKeyboardDirectionType;
-}
-
 struct ScancodeRow {
 	int offset;
 	const char *keys;
diff --git a/engines/sci/event.h b/engines/sci/event.h
index fade7dd337..7c83476294 100644
--- a/engines/sci/event.h
+++ b/engines/sci/event.h
@@ -116,7 +116,6 @@ public:
 
 	void updateScreen();
 	SciEvent getSciEvent(unsigned int mask);
-	bool getUsesNewKeyboardDirectionType();
 
 private:
 	SciEvent getScummVMEvent();
@@ -124,8 +123,6 @@ private:
 	const bool _fontIsExtended;
 	int _modifierStates;
 	Common::List<SciEvent> _events;
-
-	bool _usesNewKeyboardDirectionType;
 };
 
 } // End of namespace Sci
-- 
cgit v1.2.3


From 325a301a4feac17d8aa4bba80c6112116a1ceb1a Mon Sep 17 00:00:00 2001
From: Matthew Hoops
Date: Tue, 15 Feb 2011 11:02:01 -0500
Subject: SCI: Fill in the remaining Mac-specific kPlatform subops

---
 engines/sci/engine/kmisc.cpp | 59 +++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 22 deletions(-)

(limited to 'engines/sci')

diff --git a/engines/sci/engine/kmisc.cpp b/engines/sci/engine/kmisc.cpp
index 6e961f72f3..f95b1dd0f8 100644
--- a/engines/sci/engine/kmisc.cpp
+++ b/engines/sci/engine/kmisc.cpp
@@ -362,20 +362,29 @@ reg_t kIconBar(EngineState *s, int argc, reg_t *argv) {
 		return NULL_REG;
 
 	switch (argv[0].toUint16()) {
-	case 0:
-		// Add the icons
+	case 0: // InitIconBar
 		for (int i = 0; i < argv[1].toUint16(); i++)
 			g_sci->_gfxMacIconBar->addIcon(argv[i + 2]);
 
 		g_sci->_gfxMacIconBar->drawIcons();
+
+		// TODO: Should return icon bar handle
+		// Said handle is then used by DisposeIconBar
+		break;
+	case 1: // DisposeIconBar
+		warning("kIconBar(Dispose)");
+		break;
+	case 2: // EnableIconBar (0xffff = all)
+		warning("kIconBar(Enable, %d)", argv[1].toUint16());
+		break;
+	case 3: // DisableIconBar (0xffff = all)
+		warning("kIconBar(Disable, %d)", argv[1].toUint16());
 		break;
-	case 2:
-	case 3:
-	case 4:
-		// TODO: Other calls seem to handle selecting/deselecting them
+	case 4: // SetIconBarIcon
+		warning("kIconBar(SetIcon, %d, %d)", argv[1].toUint16(), argv[2].toUint16());
 		break;
 	default:
-		warning("Unknown kIconBar subop %d", argv[0].toUint16());
+		error("Unknown kIconBar(%d)", argv[0].toUint16());
 	}
 
 	return NULL_REG;
@@ -389,23 +398,28 @@ reg_t kMacPlatform(EngineState *s, int argc, reg_t *argv) {
 
 	switch (argv[0].toUint16()) {
 	case 0:
-		// Set Mac cursor remap
-		g_sci->_gfxCursor->setMacCursorRemapList(argc - 1, argv + 1);
+		// Subop 0 has changed a few times
+		// In SCI1, its usage is still unknown
+		// In SCI1.1, it's NOP
+		// In SCI32, it's used for remapping cursor ID's
+		if (getSciVersion() >= SCI_VERSION_2_1) // Set Mac cursor remap
+			g_sci->_gfxCursor->setMacCursorRemapList(argc - 1, argv + 1);
+		else if (getSciVersion() != SCI_VERSION_1_1)
+			warning("Unknown SCI1 kMacPlatform(0) call");
 		break;
-	case 1:
-		// Unknown
-		break;
-	case 2:
-		// Unknown
-		break;
-	case 3:
-		// Unknown
-		break;
-	case 4:
-		// Handle icon bar code
+	case 4: // Handle icon bar code
 		return kIconBar(s, argc - 1, argv + 1);
+	case 7: // Unknown, but always return -1
+		return SIGNAL_REG;
+	case 1:	// Unknown, calls QuickDraw region functions (KQ5, QFG1VGA)
+	case 2: // Unknown, "UseNextWaitEvent" (Various)
+	case 3: // Unknown, "ProcessOpenDocuments" (Various)
+	case 5: // Unknown, plays a sound (KQ7)
+	case 6: // Unknown, menu-related (Unused?)
+		warning("Unhandled kMacPlatform(%d)", argv[0].toUint16());
+		break;
 	default:
-		warning("Unknown kMacPlatform subop %d", argv[0].toUint16());
+		error("Unknown kMacPlatform(%d)", argv[0].toUint16());
 	}
 
 	return s->r_acc;
@@ -455,7 +469,8 @@ reg_t kPlatform(EngineState *s, int argc, reg_t *argv) {
 		warning("STUB: kPlatform(CDCheck)");
 		break;
 	case kPlatformUnk0:
-		if (g_sci->getPlatform() == Common::kPlatformMacintosh && getSciVersion() >= SCI_VERSION_1_1 && argc > 1)
+		// For Mac versions, kPlatform(0) with other args has more functionality
+		if (g_sci->getPlatform() == Common::kPlatformMacintosh && argc > 1)
 			return kMacPlatform(s, argc - 1, argv + 1);
 		// Otherwise, fall through
 	case kPlatformGetPlatform:
-- 
cgit v1.2.3