SCI32: Add reg_t comparisons for graphics sorting

4 files changed, 28 insertions, 31 deletions

diff --git a/engines/sci/engine/vm_types.cpp b/engines/sci/engine/vm_types.cpp
index 53a5a5c507..00a67fc9cc 100644
--- a/engines/sci/engine/vm_types.cpp
+++ b/engines/sci/engine/vm_types.cpp
@@ -230,6 +230,10 @@ int reg_t::cmp(const reg_t right, bool treatAsUnsigned) const {
 			return toUint16() - right.toUint16();
 		else
 			return toSint16() - right.toSint16();
+#ifdef ENABLE_SCI32
+	} else if (getSciVersion() >= SCI_VERSION_2) {
+		return sci32Comparison(right);
+#endif
 	} else if (pointerComparisonWithInteger(right)) {
 		return 1;
 	} else if (right.pointerComparisonWithInteger(*this)) {
@@ -238,6 +242,24 @@ int reg_t::cmp(const reg_t right, bool treatAsUnsigned) const {
 		return lookForWorkaround(right, "comparison").toSint16();
 }
 
+int reg_t::sci32Comparison(const reg_t right) const {
+	// In SCI32, MemIDs are normally indexes into the memory manager's handle
+	// list, but the engine reserves indexes at and above 20000 for objects
+	// that were created inside the engine (as opposed to inside the VM). The
+	// engine compares these as a tiebreaker for graphics objects that are at
+	// the same priority, and it is necessary to at least minimally handle
+	// this situation.
+	// This is obviously a bogus comparision, but then, this entire thing is
+	// bogus. For the moment, it just needs to be deterministic.
+	if (isNumber() && !right.isNumber()) {
+		return 1;
+	} else if (right.isNumber() && !isNumber()) {
+		return -1;
+	}
+
+	return getOffset() - right.getOffset();
+}
+
 bool reg_t::pointerComparisonWithInteger(const reg_t right) const {
 	// This function handles the case where a script tries to compare a pointer
 	// to a number. Normally, we would not want to allow that. However, SCI0 -
diff --git a/engines/sci/engine/vm_types.h b/engines/sci/engine/vm_types.h
index a646478a8e..e60f52e85c 100644
--- a/engines/sci/engine/vm_types.h
+++ b/engines/sci/engine/vm_types.h
@@ -160,6 +160,10 @@ private:
 	int cmp(const reg_t right, bool treatAsUnsigned) const;
 	reg_t lookForWorkaround(const reg_t right, const char *operation) const;
 	bool pointerComparisonWithInteger(const reg_t right) const;
+
+#ifdef ENABLE_SCI32
+	int sci32Comparison(const reg_t right) const;
+#endif
 };
 
 static inline reg_t make_reg(SegmentId segment, uint16 offset) {
diff --git a/engines/sci/graphics/plane32.h b/engines/sci/graphics/plane32.h
index f60f0cc72b..d3427f11d5 100644
--- a/engines/sci/graphics/plane32.h
+++ b/engines/sci/graphics/plane32.h
@@ -254,34 +254,12 @@ public:
 	void operator=(const Plane &other);
 
 	inline bool operator<(const Plane &other) const {
-		// TODO: In SCI engine, _object is actually a uint16 and can either
-		// contain a MemID (a handle to MemoryMgr, similar to reg_t) or
-		// a serial (Plane::_nextObjectId). These numbers can be compared
-		// directly in the real engine and the lowest MemID wins, but in
-		// ScummVM reg_t pointers are not comparable so we have to use a
-		// different strategy when two planes generated by scripts conflict.
-		// For now we just don't check if the priority is below 0, since
-		// that priority is used to represent hidden planes and is guaranteed
-		// to generate conflicts with script-generated planes. If there are
-		// other future conflicts with script-generated planes then we need
-		// to come up with a solution that works, similar to
-		// reg_t::pointerComparisonWithInteger used by SCI16.
-		//
-		// For now, we check the object offsets, as this will likely work
-		// like in the original SCI engine, without comparing objects.
-		// However, this whole comparison is quite ugly, and if it still
-		// fails, we should try to change it to something equivalent, to avoid
-		// adding loads of workarounds just for this
 		if (_priority < other._priority) {
 			return true;
 		}
 
 		if (_priority == other._priority) {
-			if (_object.isNumber() && other._object.isNumber()) {
-				return _object < other._object;
-			} else if (other._object.isNumber()) {
-				return true;
-			}
+			return _object < other._object;
 		}
 
 		return false;
diff --git a/engines/sci/graphics/screen_item32.h b/engines/sci/graphics/screen_item32.h
index e86efdaf6a..0840570721 100644
--- a/engines/sci/graphics/screen_item32.h
+++ b/engines/sci/graphics/screen_item32.h
@@ -229,14 +229,7 @@ public:
 			}
 
 			if (_position.y + _z == other._position.y + other._z) {
-				// TODO: Failure in SQ6 room 220 when using SCI logic
-				// to compare pointer and numeric memory handles
-
-				if (_object.isNumber() && other._object.isNumber()) {
-					return _object < other._object;
-				} else if (other._object.isNumber()) {
-					return true;
-				}
+				return _object < other._object;
 			}
 		}