SCI: Add string support for odd-offset pointers into reg_t-based segments.

svn-id: r47572

7 files changed, 105 insertions, 135 deletions

diff --git a/engines/sci/engine/kgraphics.cpp b/engines/sci/engine/kgraphics.cpp
index caf07fe17a..474b12d0d5 100644
--- a/engines/sci/engine/kgraphics.cpp
+++ b/engines/sci/engine/kgraphics.cpp
@@ -772,10 +772,6 @@ void _k_GenericDrawControl(EngineState *s, reg_t controlObject, bool hilite) {
 			isAlias = true;
 
 		maxChars = GET_SEL32V(s->_segMan, controlObject, x); // max chars per entry
-		// NOTE: most types of pointer dereferencing don't like odd offsets
-		if (maxChars & 1) {
-			warning("List control with odd maxChars %d. This is not yet implemented for all types of segments", maxChars);
-		}
 		cursorOffset = GET_SEL32V(s->_segMan, controlObject, cursor);
 		if (s->_kernel->_selectorCache.topString != -1) {
 			// Games from early SCI1 onwards use topString
diff --git a/engines/sci/engine/kmisc.cpp b/engines/sci/engine/kmisc.cpp
index b982628bc0..229dcb8367 100644
--- a/engines/sci/engine/kmisc.cpp
+++ b/engines/sci/engine/kmisc.cpp
@@ -219,8 +219,11 @@ reg_t kMemory(EngineState *s, int argc, reg_t *argv) {
 		}
 		if (ref.isRaw)
 			return make_reg(0, (int16)READ_LE_UINT16(ref.raw));
-		else
+		else {
+			if (ref.skipByte)
+				error("Attempt to peek memory at odd offset %04X:%04X", PRINT_REG(argv[1]));
 			return *(ref.reg);
+		}
 		break;
 	}
 	case K_MEMORY_POKE : {
@@ -237,8 +240,11 @@ reg_t kMemory(EngineState *s, int argc, reg_t *argv) {
 				return s->r_acc;
 			}
 			WRITE_LE_UINT16(ref.raw, argv[2].offset);
-		} else
+		} else {
+			if (ref.skipByte)
+				error("Attempt to poke memory at odd offset %04X:%04X", PRINT_REG(argv[1]));
 			*(ref.reg) = argv[2];
+		}
 		break;
 	}
 	}
diff --git a/engines/sci/engine/kpathing.cpp b/engines/sci/engine/kpathing.cpp
index cfe3cb38a3..9bd3912aa7 100644
--- a/engines/sci/engine/kpathing.cpp
+++ b/engines/sci/engine/kpathing.cpp
@@ -260,7 +260,7 @@ struct PathfindingState {
 
 static Common::Point read_point(SegManager *segMan, reg_t list, int offset) {
 	SegmentRef list_r = segMan->dereference(list);
-	if (!list_r.isValid()) {
+	if (!list_r.isValid() || list_r.skipByte) {
 		warning("read_point(): Attempt to dereference invalid pointer %04x:%04x", PRINT_REG(list));
 	}
 	Common::Point point;
@@ -1740,7 +1740,7 @@ static reg_t output_path(PathfindingState *p, EngineState *s) {
 	// Allocate memory for path, plus 3 extra for appended point, prepended point and sentinel
 	output = allocateOutputArray(s->_segMan, path_len + 3);
 	SegmentRef arrayRef = s->_segMan->dereference(output);
-	assert(arrayRef.isValid());
+	assert(arrayRef.isValid() && !arrayRef.skipByte);
 
 	if (unreachable) {
 		// If pathfinding failed we only return the path up to vertex_start
@@ -1860,7 +1860,7 @@ reg_t kAvoidPath(EngineState *s, int argc, reg_t *argv) {
 			printf("[avoidpath] Returning direct path from start point to end point\n");
 			output = allocateOutputArray(s->_segMan, 3);
 			SegmentRef arrayRef = s->_segMan->dereference(output);
-			assert(arrayRef.isValid());
+			assert(arrayRef.isValid() && !arrayRef.skipByte);
 
 			writePoint(arrayRef, 0, start);
 			writePoint(arrayRef, 1, end);
diff --git a/engines/sci/engine/kstring.cpp b/engines/sci/engine/kstring.cpp
index ee1e39047f..769964e914 100644
--- a/engines/sci/engine/kstring.cpp
+++ b/engines/sci/engine/kstring.cpp
@@ -115,7 +115,7 @@ reg_t kStrAt(EngineState *s, int argc, reg_t *argv) {
 	}
 
 	SegmentRef dest_r = s->_segMan->dereference(argv[0]);
-	if (!dest_r.raw) {
+	if (!dest_r.isValid()) {
 		warning("Attempt to StrAt at invalid pointer %04x:%04x", PRINT_REG(argv[0]));
 		return NULL_REG;
 	}
@@ -133,11 +133,15 @@ reg_t kStrAt(EngineState *s, int argc, reg_t *argv) {
 		return s->r_acc;
 	}
 
+	// FIXME: Move this to segman
 	if (dest_r.isRaw) {
 		value = dest_r.raw[offset];
 		if (argc > 2) /* Request to modify this char */
 			dest_r.raw[offset] = newvalue;
 	} else {
+		if (dest_r.skipByte)
+			offset++;
+
 		reg_t &tmp = dest_r.reg[offset / 2];
 		if (!(offset & 1)) {
 			value = tmp.offset & 0x00ff;
diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index f995991aa2..17cf855229 100644
--- a/engines/sci/engine/seg_manager.cpp
+++ b/engines/sci/engine/seg_manager.cpp
@@ -923,12 +923,20 @@ static void *_kernel_dereference_pointer(SegManager *segMan, reg_t pointer, int
 			wantRaw ? "raw" : "not raw");
 	}
 
+	if (!wantRaw && ret.skipByte) {
+		warning("Unaligned pointer read: %04x:%04x expected with word alignment", PRINT_REG(pointer));
+		return NULL;
+	}
+
 	if (entries > ret.maxSize) {
 		warning("Trying to dereference pointer %04x:%04x beyond end of segment", PRINT_REG(pointer));
 		return NULL;
 	}
-	return ret.raw;
 
+	if (ret.isRaw)
+		return ret.raw;
+	else
+		return ret.reg;
 }
 
 byte *SegManager::derefBulkPtr(reg_t pointer, int entries) {
@@ -936,18 +944,6 @@ byte *SegManager::derefBulkPtr(reg_t pointer, int entries) {
 }
 
 reg_t *SegManager::derefRegPtr(reg_t pointer, int entries) {
-#ifdef ENABLE_SCI32
-	// HACK: Due to a limitation in the SegManager, we don't know if the pointer needs to be
-	// word aligned. If it's a new style array, then it is just accessing the arrays from a
-	// table and this doesn't need to be true.
-	if (pointer.offset & 1 && pointer.segment != Arrays_seg_id) {
-#else
-	if (pointer.offset & 1) {
-#endif
-		warning("Unaligned pointer read: %04x:%04x expected with word alignment", PRINT_REG(pointer));
-		return NULL;
-	}
-
 	return (reg_t *)_kernel_dereference_pointer(this, pointer, 2*entries, false);
 }
 
@@ -955,6 +951,33 @@ char *SegManager::derefString(reg_t pointer, int entries) {
 	return (char *)_kernel_dereference_pointer(this, pointer, entries, true);
 }
 
+// Helper functions for getting/setting characters in string fragments
+static inline char getChar(const SegmentRef &ref, uint offset) {
+	if (ref.skipByte)
+		offset++;
+
+	reg_t val = ref.reg[offset / 2];
+
+	if (val.segment != 0)
+		warning("Attempt to read character from non-raw data");
+
+	return (offset & 1 ? val.offset >> 8 : val.offset & 0xff);
+}
+
+static inline void setChar(const SegmentRef &ref, uint offset, char value) {
+	if (ref.skipByte)
+		offset++;
+
+	reg_t *val = ref.reg + offset / 2;
+
+	val->segment = 0;
+
+	if (offset & 1)
+		val->offset = (val->offset & 0x00ff) | (value << 8);
+	else
+		val->offset = (val->offset & 0xff00) | value;
+}
+
 // TODO: memcpy, strcpy and strncpy could maybe be folded into a single function
 void SegManager::strncpy(reg_t dest, const char* src, size_t n) {
 	SegmentRef dest_r = dereference(dest);
@@ -966,27 +989,15 @@ void SegManager::strncpy(reg_t dest, const char* src, size_t n) {
 	if (dest_r.isRaw) {
 		// raw -> raw
 		if (n == 0xFFFFFFFFU)
-			::strcpy((char*)dest_r.raw, src);
+			::strcpy((char *)dest_r.raw, src);
 		else
-			::strncpy((char*)dest_r.raw, src, n);
+			::strncpy((char *)dest_r.raw, src, n);
 	} else {
 		// raw -> non-raw
-		reg_t* d = dest_r.reg;
-		while (n > 0) {
-			d->segment = 0; // STRINGFRAG_SEGMENT?
-			if (n > 1 && src[0]) {
-				d->offset = (src[0] & 0x00ff)  | (src[1] << 8);
-			} else {
-				d->offset &= 0xff00;
-				d->offset |= src[0] & 0x00ff;
-				break;
-			}
-
-			if (!src[1])
+		for (uint i = 0; i < n; i++) {
+			setChar(dest_r, i, src[i]);
+			if (!src[i])
 				break;
-			src += 2;
-			n -= 2;
-			d++;
 		}
 	}
 }
@@ -1014,38 +1025,19 @@ void SegManager::strncpy(reg_t dest, reg_t src, size_t n) {
 		strncpy(dest, (const char*)src_r.raw, n);
 	} else if (dest_r.isRaw && !src_r.isRaw) {
 		// non-raw -> raw
-		const reg_t* s = src_r.reg;
-		char *d = (char*)dest_r.raw;
-		reg_t x;
-		while (n > 0) {
-			x = *s++;
-			*d++ = x.offset & 0x00ff;
-			if (n > 1 && x.offset & 0x00ff) {
-				*d++ = x.offset >> 8;
-			} else {
-				break;
-			}
-			if (!(x.offset >> 8))
+		for (uint i = 0; i < n; i++) {
+			char c = getChar(src_r, i);
+			dest_r.raw[i] = c;
+			if (!c)
 				break;
-			n -= 2;
 		}
 	} else {
 		// non-raw -> non-raw
-		const reg_t* s = src_r.reg;
-		reg_t* d = dest_r.reg;
-		reg_t x;
-		while (n > 0) {
-			x = *s++;
-			if (n > 1 && x.offset & 0x00ff) {
-				*d++ = x;
-			} else {
-				d->offset &= 0xff00;
-				d->offset |= x.offset & 0x00ff;
+		for (uint i = 0; i < n; i++) {
+			char c = getChar(src_r, i);
+			setChar(dest_r, i, c);
+			if (!c)
 				break;
-			}
-			if (!(x.offset & 0xff00))
-				break;
-			n -= 2;
 		}
 	}
 }
@@ -1074,20 +1066,8 @@ void SegManager::memcpy(reg_t dest, const byte* src, size_t n) {
 		::memcpy((char*)dest_r.raw, src, n);
 	} else {
 		// raw -> non-raw
-		reg_t* d = dest_r.reg;
-		while (n > 0) {
-			d->segment = 0; // STRINGFRAG_SEGMENT?
-			if (n > 1) {
-				d->offset = (src[0] & 0x00ff) | (src[1] << 8);
-			} else {
-				d->offset &= 0xff00;
-				d->offset |= src[0] & 0x00ff;
-				break;
-			}
-			src += 2;
-			n -= 2;
-			d++;
-		}
+		for (uint i = 0; i < n; i++)
+			setChar(dest_r, i, src[i]);
 	}
 }
 
@@ -1119,19 +1099,9 @@ void SegManager::memcpy(reg_t dest, reg_t src, size_t n) {
 		memcpy(dest_r.raw, src, n);
 	} else {
 		// non-raw -> non-raw
-		const reg_t* s = src_r.reg;
-		reg_t* d = dest_r.reg;
-		reg_t x;
-		while (n > 0) {
-			x = *s++;
-			if (n > 1) {
-				*d++ = x;
-			} else {
-				d->offset &= 0xff00;
-				d->offset |= x.offset & 0x00ff;
-				break;
-			}
-			n -= 2;
+		for (uint i = 0; i < n; i++) {
+			char c = getChar(src_r, i);
+			setChar(dest_r, i, c);
 		}
 	}
 }
@@ -1152,17 +1122,9 @@ void SegManager::memcpy(byte *dest, reg_t src, size_t n) {
 		::memcpy(dest, src_r.raw, n);
 	} else {
 		// non-raw -> raw
-		const reg_t* s = src_r.reg;
-		reg_t x;
-		while (n > 0) {
-			x = *s++;
-			*dest++ = x.offset & 0x00ff;
-			if (n > 1) {
-				*dest++ = x.offset >> 8;
-			} else {
-				break;
-			}
-			n -= 2;
+		for (uint i = 0; i < n; i++) {
+			char c = getChar(src_r, i);
+			dest[i] = c;
 		}
 	}
 }
@@ -1175,17 +1137,12 @@ size_t SegManager::strlen(reg_t str) {
 	}
 
 	if (str_r.isRaw) {
-		return ::strlen((const char*)str_r.raw);
+		return ::strlen((const char *)str_r.raw);
 	} else {
-		const reg_t* s = str_r.reg;
-		size_t n = 0;
-		while ((s->offset & 0x00ff) && (s->offset >> 8)) {
-			++s;
-			n += 2;
-		}
-		if (s->offset & 0x00ff)
-			n++;
-		return n;
+		int i = 0;
+		while (getChar(str_r, i))
+			i++;
+		return i;
 	}
 }
 
@@ -1202,21 +1159,18 @@ Common::String SegManager::getString(reg_t pointer, int entries) {
 		return ret;
 	}
 	if (src_r.isRaw)
-		ret = (char*)src_r.raw;
+		ret = (char *)src_r.raw;
 	else {
-		const reg_t* s = src_r.reg;
-		char c;
-		do {
-			c = s->offset & 0x00ff;
-			if (c) {
-				ret += c;
-				c = s->offset >> 8;
-				if (c) {
-					ret += c;
-					s++;
-				}
-			}
-		} while (c);
+		uint i = 0;
+		for (;;) {
+			char c = getChar(src_r, i);
+
+			if (!c)
+				break;
+
+			i++;
+			ret += c;
+		};
 	}
 	return ret;
 }
diff --git a/engines/sci/engine/segment.cpp b/engines/sci/engine/segment.cpp
index a76644c718..1764109581 100644
--- a/engines/sci/engine/segment.cpp
+++ b/engines/sci/engine/segment.cpp
@@ -247,12 +247,15 @@ bool LocalVariables::isValidOffset(uint16 offset) const {
 }
 
 SegmentRef LocalVariables::dereference(reg_t pointer) {
-	if (pointer.offset & 1)
-		warning("LocalVariables::dereference: Odd offset in pointer  %04x:%04x", PRINT_REG(pointer));
-
 	SegmentRef ret;
 	ret.isRaw = false;	// reg_t based data!
 	ret.maxSize = (_locals.size() - pointer.offset / 2) * 2;
+
+	if (pointer.offset & 1) {
+		ret.maxSize -= 1;
+		ret.skipByte = true;
+	}
+
 	if (ret.maxSize > 0) {
 		ret.reg = &_locals[pointer.offset / 2];
 	} else {
@@ -271,8 +274,11 @@ SegmentRef DataStack::dereference(reg_t pointer) {
 	ret.isRaw = false;	// reg_t based data!
 	ret.maxSize = (_capacity - pointer.offset / 2) * 2;
 
-	if (pointer.offset & 1)
-		warning("LocalVariables::dereference: Odd offset in pointer  %04x:%04x", PRINT_REG(pointer));
+	if (pointer.offset & 1) {
+		ret.maxSize -= 1;
+		ret.skipByte = true;
+	}
+
 	ret.reg = &_entries[pointer.offset / 2];
 	return ret;
 }
diff --git a/engines/sci/engine/segment.h b/engines/sci/engine/segment.h
index 4d9d07c302..82ffd785d6 100644
--- a/engines/sci/engine/segment.h
+++ b/engines/sci/engine/segment.h
@@ -39,13 +39,17 @@ struct SegmentRef {
 		reg_t *reg;
 	};
 	int maxSize;	///< number of available bytes
+
+	// FIXME: Perhaps a generic 'offset' is more appropriate here
+	bool skipByte; ///< true if referencing the 2nd data byte of *reg, false otherwise
+
 	// TODO: Add this?
 	//reg_t pointer;	// Original pointer
 
 	// TODO: Add this?
 	//SegmentType type;
 
-	SegmentRef() : isRaw(true), raw(0), maxSize(0) {}
+	SegmentRef() : isRaw(true), raw(0), maxSize(0), skipByte(false) {}
 
 	bool isValid() const { return (isRaw ? raw != 0 : reg != 0); }
 };