SCI: Find and store the original static names of objects

See code comment in Object::init for more details.

Fixes Trac#9780.

5 files changed, 48 insertions, 34 deletions

diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 640feb1418..b0e9939348 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -23,6 +23,7 @@
 
 #include "sci/engine/kernel.h"
 #include "sci/engine/object.h"
+#include "sci/engine/script.h"
 #include "sci/engine/seg_manager.h"
 #ifdef ENABLE_SCI32
 #include "sci/engine/features.h"
@@ -32,9 +33,9 @@ namespace Sci {
 
 extern bool relocateBlock(Common::Array<reg_t> &block, int block_location, SegmentId segment, int location, uint32 heapOffset);
 
-
-void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariables) {
-	const SciSpan<const byte> data = buf.subspan(obj_pos.getOffset());
+void Object::init(const Script &owner, reg_t obj_pos, bool initVariables) {
+	const SciSpan<const byte> buf = owner.getSpan(0);
+	const SciSpan<const byte> data = owner.getSpan(obj_pos.getOffset());
 	_baseObj = data;
 	_pos = obj_pos;
 
@@ -81,6 +82,35 @@ void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariab
 #endif
 	}
 
+	// Some objects, like the unnamed LarryTalker instance in LSL6hires script
+	// 610, and the File class in Torin script 64993, have a `name` property
+	// that is assigned dynamically by game scripts, overriding the static name
+	// value that is normally created by the SC compiler. When this happens, the
+	// value can be set to anything: in LSL6hires it becomes a Str object; in
+	// Torin, it becomes a dynamically allocated string that is disposed before
+	// the corresponding File instance is disposed.
+	// To ensure `SegManager::getObjectName` works consistently and correctly,
+	// without hacks to bypass unexpected/invalid types of dynamic `name` data,
+	// the reg_t pointer to the original static name value for the object is
+	// stored here, ensuring that it is constant and guaranteed to be either a
+	// valid dereferenceable string or NULL_REG.
+	if (getSciVersion() != SCI_VERSION_3) {
+		const uint32 heapOffset = owner.getHeapOffset();
+		const uint32 nameOffset = (obj_pos.getOffset() - heapOffset) + (_offset + 3) * sizeof(uint16);
+		const uint32 relocOffset = owner.getRelocationOffset(nameOffset);
+		if (relocOffset != kNoRelocation) {
+			_name = make_reg(obj_pos.getSegment(), relocOffset + _baseObj.getUint16SEAt((_offset + 3) * sizeof(uint16)));
+		}
+#ifdef ENABLE_SCI32
+	} else if (_propertyOffsetsSci3.size()) {
+		const uint32 nameOffset = _propertyOffsetsSci3[0];
+		const uint32 relocOffset = owner.getRelocationOffset(nameOffset);
+		if (relocOffset != kNoRelocation) {
+			_name = make_reg(obj_pos.getSegment(), relocOffset + buf.getUint16SEAt(nameOffset));
+		}
+#endif
+	}
+
 	if (initVariables) {
 #ifdef ENABLE_SCI32
 		if (getSciVersion() == SCI_VERSION_3) {
diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index be20e3bef4..12387bcb09 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -34,6 +34,7 @@
 namespace Sci {
 
 class SegManager;
+class Script;
 
 enum infoSelectorFlags {
 	kInfoFlagClone        = 0x0001,
@@ -69,6 +70,7 @@ enum ObjectOffsets {
 class Object {
 public:
 	Object() :
+		_name(NULL_REG),
 		_offset(getSciVersion() < SCI_VERSION_1_1 ? 0 : 5),
 		_isFreed(false),
 		_baseObj(),
@@ -81,6 +83,7 @@ public:
 		{}
 
 	Object &operator=(const Object &other) {
+		_name = other._name;
 		_baseObj = other._baseObj;
 		_baseMethod = other._baseMethod;
 		_variables = other._variables;
@@ -181,12 +184,7 @@ public:
 #endif
 
 	reg_t getNameSelector() const {
-#ifdef ENABLE_SCI32
-		if (getSciVersion() == SCI_VERSION_3)
-			return _variables.size() ? _variables[0] : NULL_REG;
-		else
-#endif
-			return _offset + 3 < (uint16)_variables.size() ? _variables[_offset + 3] : NULL_REG;
+		return _name;
 	}
 
 	// No setter for the name selector
@@ -278,7 +276,7 @@ public:
 
 	uint getVarCount() const { return _variables.size(); }
 
-	void init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariables = true);
+	void init(const Script &owner, reg_t obj_pos, bool initVariables = true);
 
 	reg_t getVariable(uint var) const { return _variables[var]; }
 	reg_t &getVariableRef(uint var) { return _variables[var]; }
@@ -289,6 +287,7 @@ public:
 	void saveLoadWithSerializer(Common::Serializer &ser);
 
 	void cloneFromObject(const Object *obj) {
+		_name = obj ? obj->_name : NULL_REG;
 		_baseObj = obj ? obj->_baseObj : SciSpan<const byte>();
 		_baseMethod = obj ? obj->_baseMethod : Common::Array<uint32>();
 		_baseVars = obj ? obj->_baseVars : Common::Array<uint16>();
@@ -320,6 +319,11 @@ private:
 #endif
 
 	/**
+	 * The name of the object.
+	 */
+	reg_t _name;
+
+	/**
 	 * A pointer to the raw object data within the object's owner script.
 	 */
 	SciSpan<const byte> _baseObj;
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index ea1b6f2f5f..4932c16e73 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -656,7 +656,7 @@ Object *Script::scriptObjInit(reg_t obj_pos, bool fullObjectInit) {
 	// Get the object at the specified position and init it. This will
 	// automatically "allocate" space for it in the _objects map if necessary.
 	Object *obj = &_objects[obj_pos.getOffset()];
-	obj->init(*_buf, obj_pos, fullObjectInit);
+	obj->init(*this, obj_pos, fullObjectInit);
 
 	return obj;
 }
diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index a941dd38b4..168303684d 100644
--- a/engines/sci/engine/seg_manager.cpp
+++ b/engines/sci/engine/seg_manager.cpp
@@ -275,30 +275,10 @@ const char *SegManager::getObjectName(reg_t pos) {
 	if (nameReg.isNull())
 		return "<no name>";
 
-	const char *name = nullptr;
-
-	if (nameReg.getSegment()) {
-#ifdef ENABLE_SCI32
-		// At least Torin script 64000 creates objects with names that are
-		// pointed to dynamically generated strings which are freed before the
-		// objects themselves are freed. This causes a crash when using
-		// `findObjectByName`, since the name of the object is no longer valid
-		if (nameReg.getSegment() != _arraysSegId ||
-			_heap[_arraysSegId]->isValidOffset(nameReg.getOffset())) {
-#endif
-			name = derefString(nameReg);
-#ifdef ENABLE_SCI32
-		}
-#endif
-	}
+	const char *name = derefString(nameReg);
 
 	if (!name) {
-		// Crazy Nick Laura Bow is missing some object names needed for the static
-		// selector vocabulary
-		if (g_sci->getGameId() == GID_CNICK_LAURABOW && pos == make_reg(1, 0x2267))
-			return "Character";
-		else
-			return "<invalid name>";
+		return "<invalid name>";
 	}
 
 	return name;
diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 9301fe60b7..c471e6a989 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -257,7 +257,7 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_CNICK_KQ,       -1,   700,  0,           "gcWindow", "open",                            NULL,    -1, { WORKAROUND_FAKE,   0 } }, // when entering the control menu, like in hoyle 3
 	{ GID_CNICK_KQ,      300,   303,  0,      "theDoubleCube", "<noname520>",                     NULL,     5, { WORKAROUND_FAKE,   0 } }, // while playing backgammon with doubling enabled - bug #6426 (same as the theDoubleCube::make workaround for Hoyle 3)
 	{ GID_CNICK_KQ,      300,   303,  0,      "theDoubleCube", "<noname519>",                     NULL,     9, { WORKAROUND_FAKE,   0 } }, // when accepting a double, while playing backgammon with doubling enabled (same as the theDoubleCube::accept workaround for Hoyle 3)
-	{ GID_CNICK_LAURABOW, -1,     0,  1,          "Character", "say",                             NULL,    -1, { WORKAROUND_FAKE,   0 } }, // Yatch, like in hoyle 3 - temps 504 and 505 - bug #6424
+	{ GID_CNICK_LAURABOW,500,     0,  1,          "<no name>", "<noname446>",                     NULL,    -1, { WORKAROUND_FAKE,   0 } }, // Yacht, like in hoyle 3 - temps 504 and 505 - bug #6424
 	{ GID_CNICK_LAURABOW, -1,   700,  0,                 NULL, "open",                            NULL,    -1, { WORKAROUND_FAKE,   0 } }, // when entering control menu - bug #6423 (same as the gcWindow workaround for Hoyle 3)
 	{ GID_CNICK_LAURABOW,100,   100,  0,                 NULL, "<noname144>",                     NULL,     1, { WORKAROUND_FAKE,   0 } }, // while playing domino - bug #6429 (same as the dominoHand2 workaround for Hoyle 3)
 	{ GID_CNICK_LAURABOW,100,   110,  0,                 NULL, "doit",                            NULL,    -1, { WORKAROUND_FAKE,   0 } }, // when changing the "Dominoes per hand" setting - bug #6430