TINSEL: Implement a container for accessing the INV_OBJECTs

This way we encapsulate the iteration, thus making sure that
the appropriate stride is used (Noir has more fields).

engines/tinsel/dialogs.cpp

@@ -756,7 +756,6 @@ Dialogs::Dialogs() {
 	memset(_configStrings, 0, sizeof(_configStrings));
 
 	_invObjects = nullptr;
-	_numObjects = 0;
 	_invFilms = nullptr;
 	_noLanguage = false;
 
@@ -843,6 +842,7 @@ Dialogs::Dialogs() {
 }
 
 Dialogs::~Dialogs() {
+	delete _invObjects;
 	if (_objArray[0] != NULL) {
 		DumpObjArray();
 		DumpDobjArray();
@@ -1068,42 +1068,42 @@ void Dialogs::DumpObjArray() {
  * i.e. Image data and Glitter code.
  */
 INV_OBJECT *Dialogs::GetInvObject(int id) {
-	INV_OBJECT *pObject = _invObjects;
-
-	for (int i = 0; i < _numObjects; i++, pObject++) {
-		if (pObject->id == id)
-			return pObject;
+	auto object = _invObjects->GetInvObject(id);
+	if (!object) {
+		error("GetInvObject(%d): Trying to manipulate undefined inventory icon", id);
 	}
+	return object;
+}
 
-	error("GetInvObject(%d): Trying to manipulate undefined inventory icon", id);
+/**
+ * Convert item ID number to pointer to item's compiled data
+ * i.e. Image data and Glitter code.
+ */
+INV_OBJECT_T3 *Dialogs::GetInvObjectT3(int id) {
+	auto object = _invObjects->GetInvObjectT3(id);
+	if (!object) {
+		error("GetInvObject(%d): Trying to manipulate undefined inventory icon", id);
+	}
+	return object;
 }
 
 /**
  * Returns true if the given id represents a valid inventory object
  */
 bool Dialogs::GetIsInvObject(int id) {
-	INV_OBJECT *pObject = _invObjects;
-
-	for (int i = 0; i < _numObjects; i++, pObject++) {
-		if (pObject->id == id)
-			return true;
-	}
-
-	return false;
+	int index = _invObjects->GetObjectIndexIfExists(id);
+	return index != -1;
 }
 
 /**
  * Convert item ID number to index.
  */
 int Dialogs::GetObjectIndex(int id) {
-	INV_OBJECT *pObject = _invObjects;
-
-	for (int i = 0; i < _numObjects; i++, pObject++) {
-		if (pObject->id == id)
-			return i;
+	int index = _invObjects->GetObjectIndexIfExists(id);
+	if (index == -1) {
+		error("GetObjectIndex(%d): Trying to manipulate undefined inventory icon", id);
 	}
-
-	error("GetObjectIndex(%d): Trying to manipulate undefined inventory icon", id);
+	return index;
 }
 
 /**
@@ -4978,7 +4978,7 @@ void Dialogs::syncInvInfo(Common::Serializer &s) {
 	}
 
 	if (TinselVersion >= 2) {
-		for (int i = 0; i < _numObjects; ++i)
+		for (int i = 0; i < _invObjects->NumObjects(); ++i)
 			s.syncAsUint32LE(_invFilms[i]);
 		s.syncAsUint32LE(_heldFilm);
 	}
@@ -4994,45 +4994,48 @@ void Dialogs::syncInvInfo(Common::Serializer &s) {
  */
 // Note: the SCHANDLE type here has been changed to a void*
 void Dialogs::RegisterIcons(void *cptr, int num) {
-	_numObjects = num;
-	_invObjects = (INV_OBJECT *)cptr;
+	int numObjects = num;
+	INV_OBJECT *invObjects = (INV_OBJECT *)cptr;
 
 	if (TinselVersion == 0) {
 		// In Tinsel 0, the INV_OBJECT structure doesn't have an attributes field, so we
 		// need to 'unpack' the source structures into the standard Tinsel v1/v2 format
-		MEM_NODE *node = MemoryAllocFixed(_numObjects * sizeof(INV_OBJECT));
+		MEM_NODE *node = MemoryAllocFixed(numObjects * sizeof(INV_OBJECT));
 		assert(node);
-		_invObjects = (INV_OBJECT *)MemoryDeref(node);
-		assert(_invObjects);
+		invObjects = (INV_OBJECT *)MemoryDeref(node);
+		assert(invObjects);
 		byte *srcP = (byte *)cptr;
-		INV_OBJECT *destP = _invObjects;
+		INV_OBJECT *destP = invObjects;
 
 		for (int i = 0; i < num; ++i, ++destP, srcP += 12) {
 			memmove(destP, srcP, 12);
 			destP->attribute = 0;
 		}
+		_invObjects = InstantiateInventoryObjects(invObjects, numObjects);
 	} else if (TinselVersion >= 2) {
+		_invObjects = InstantiateInventoryObjects(invObjects, numObjects);
 		if (_invFilms == NULL) {
 			// First time - allocate memory
-			MEM_NODE *node = MemoryAllocFixed(_numObjects * sizeof(SCNHANDLE));
+			MEM_NODE *node = MemoryAllocFixed(numObjects * sizeof(SCNHANDLE));
 			assert(node);
 			_invFilms = (SCNHANDLE *)MemoryDeref(node);
 			if (_invFilms == NULL)
 				error(NO_MEM, "inventory scripts");
-			memset(_invFilms, 0, _numObjects * sizeof(SCNHANDLE));
+			memset(_invFilms, 0, numObjects * sizeof(SCNHANDLE));
 		}
 
 		// Add defined permanent conversation icons
 		// and store all the films separately
-		int i;
-		INV_OBJECT *pio;
-		for (i = 0, pio = _invObjects; i < _numObjects; i++, pio++) {
+		for (int i = 0; i < numObjects; i++) {
+			INV_OBJECT *pio = _invObjects->GetObjectByIndex(i);
 			if (pio->attribute & PERMACONV)
 				PermaConvIcon(pio->id, pio->attribute & CONVENDITEM);
 
 			_invFilms[i] = pio->hIconFilm;
 		}
 	}
+
+
 }
 
 /**

engines/tinsel/dialogs.h

@@ -26,6 +26,7 @@
 
 #include "tinsel/dw.h"
 #include "tinsel/events.h"	// for PLR_EVENT, PLR_EVENT
+#include "tinsel/inv_objects.h"
 #include "tinsel/object.h"
 #include "tinsel/movers.h"
 
@@ -143,24 +144,6 @@ enum CONFTYPE {
 	TOP_WINDOW
 };
 
-/** structure of each inventory object */
-struct INV_OBJECT {
-	int32 id;		// inventory objects id
-	SCNHANDLE hIconFilm;	// inventory objects animation film
-	SCNHANDLE hScript;	// inventory objects event handling script
-	int32 attribute;		// inventory object's attribute
-
-	// TODO: Commented out because there are variables
-	// with this struct type that are cast from memory blobs,
-	// so this breaks DW1 and DW2. We need to read these
-	// struct members individually instead of casting the blobs
-	// to this struct
-
-	// Noir
-	//int32 unknown;
-	//int32 title;	// id of associated notebook title
-};
-
 struct INV_DEF {
 	int MinHicons; // }
 	int MinVicons; // } Dimension limits
@@ -380,6 +363,7 @@ class Dialogs {
 	void FillInInventory();
 	void InvCursor(InvCursorFN fn, int CurX, int CurY);
 	INV_OBJECT *GetInvObject(int id);
+	INV_OBJECT_T3 *GetInvObjectT3(int id);
 	bool UpdateString(const Common::KeyState &kbd);
 	bool InventoryIsActive() { return _inventoryState == ACTIVE_INV; }
 	bool IsMixingDeskControl() { return _invDragging == ID_MDCONT; }
@@ -473,8 +457,7 @@ class Dialogs {
 
 	INV_DEF _invD[MAX_NUM_INV];        // Conversation + 2 inventories + ...
 	int _activeInv;                      // Which inventory is currently active
-	INV_OBJECT *_invObjects; // Inventory objects' data
-	int _numObjects;               // Number of inventory objects
+	InventoryObjects *_invObjects; // Inventory objects' data
 	SCNHANDLE *_invFilms;
 	DIRECTION _initialDirection;
 

engines/tinsel/inv_objects.cpp

@@ -0,0 +1,94 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "tinsel/inv_objects.h"
+#include "tinsel/tinsel.h"
+
+namespace Tinsel {
+
+template<typename T>
+class InventoryObjectsImpl : public InventoryObjects {
+public:
+	InventoryObjectsImpl(T* invObjects, int numObjects) : _invObjects(invObjects), _numObjects(numObjects) {}
+	/**
+	 * Convert item ID number to pointer to item's compiled data
+	 * i.e. Image data and Glitter code.
+	 */
+	INV_OBJECT* GetInvObject(int id) {
+		int index = GetObjectIndexIfExists(id);
+		if (index == -1) {
+			return nullptr;
+		}
+		return (INV_OBJECT*)&_invObjects[index];
+	}
+
+	INV_OBJECT_T3* GetInvObjectT3(int id);
+
+	/**
+	 * Convert item ID number to index.
+	 */
+	int GetObjectIndexIfExists(int id) const {
+		T *pObject = _invObjects;
+
+		for (int i = 0; i < _numObjects; i++, pObject++) {
+			if (pObject->id == id)
+				return i;
+		}
+		return -1;
+	}
+
+	INV_OBJECT* GetObjectByIndex(int index) const {
+		return (INV_OBJECT*)&_invObjects[index];
+	}
+
+	int NumObjects() const {
+		return _numObjects;
+	}
+private:
+	T *_invObjects = nullptr; // Inventory objects' data
+	int _numObjects = 0;      // Number of inventory objects
+};
+
+// Template specializations for the Noir-query, so that we can error cleanly in non-Noir
+template<>
+INV_OBJECT_T3* InventoryObjectsImpl<INV_OBJECT_T3>::GetInvObjectT3(int id) {
+	int index = GetObjectIndexIfExists(id);
+	if (index == -1) {
+		return nullptr;
+	}
+	return &_invObjects[index];
+}
+
+// Version for TinselVersion <= 2
+template<>
+INV_OBJECT_T3* InventoryObjectsImpl<INV_OBJECT>::GetInvObjectT3(int id) {
+	error("Requesting INV_OBJECT_T3 from non-Noir-game");
+}
+
+InventoryObjects *InstantiateInventoryObjects(INV_OBJECT *invObjects, int numObjects) {
+	if (TinselVersion == 3) {
+		return new InventoryObjectsImpl<INV_OBJECT_T3>((INV_OBJECT_T3*)invObjects, numObjects);
+	} else {
+		return new InventoryObjectsImpl<INV_OBJECT>(invObjects, numObjects);
+	}
+}
+
+} // End of namespace Tinsel

engines/tinsel/inv_objects.h

@@ -0,0 +1,56 @@
+/* ScummVM - Graphic Adventure Engine
+*
+* ScummVM is the legal property of its developers, whose names
+* are too numerous to list here. Please refer to the COPYRIGHT
+* file distributed with this source distribution.
+*
+* This program is free software: you can redistribute it and/or modify
+* it under the terms of the GNU General Public License as published by
+* the Free Software Foundation, either version 3 of the License, or
+* (at your option) any later version.
+*
+* This program is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU General Public License for more details.
+*
+* You should have received a copy of the GNU General Public License
+* along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*
+*/
+
+#ifndef TINSEL_INV_OBJECT_H
+#define TINSEL_INV_OBJECT_H
+
+#include "tinsel/dw.h"
+
+namespace Tinsel {
+
+/** structure of each inventory object */
+struct INV_OBJECT {
+	int32 id;            // inventory objects id
+	SCNHANDLE hIconFilm; // inventory objects animation film
+	SCNHANDLE hScript;   // inventory objects event handling script
+	int32 attribute;     // inventory object's attribute
+};
+
+struct INV_OBJECT_T3 : public INV_OBJECT {
+	int32 unknown;
+	int32 title; // id of associated notebook title
+};
+
+class InventoryObjects {
+public:
+	virtual ~InventoryObjects(){};
+	virtual INV_OBJECT *GetInvObject(int id) = 0;
+	virtual INV_OBJECT_T3 *GetInvObjectT3(int id) = 0;
+	virtual int GetObjectIndexIfExists(int id) const = 0;
+	virtual INV_OBJECT *GetObjectByIndex(int index) const = 0;
+	virtual int NumObjects() const = 0;
+};
+
+InventoryObjects *InstantiateInventoryObjects(INV_OBJECT *invObjects, int numObjects);
+
+} // End of namespace Tinsel
+
+#endif // TINSEL_INV_OBJECT_H

engines/tinsel/module.mk

@@ -21,6 +21,7 @@ MODULE_OBJS := \
 	graphics.o \
 	handle.o \
 	heapmem.o \
+	inv_objects.o \
 	mareels.o \
 	metaengine.o \
 	move.o \

engines/tinsel/noir/notebook.cpp

@@ -26,21 +26,19 @@
 namespace Tinsel {
 
 void Notebook::AddHyperlink(int32 id1, int32 id2) {
-#if 0
-	INV_OBJECT *invObject = _vm->_dialogs->GetInvObject(id1);
+	INV_OBJECT_T3 *invObject = _vm->_dialogs->GetInvObjectT3(id1);
 
 	if (invObject->title != 0) {
 		error("A clue can only be hyperlinked if it only has one title!");
 		return;
 	}
 
-	invObject = _vm->_dialogs->GetInvObject(id2);
+	invObject = _vm->_dialogs->GetInvObjectT3(id2);
 
 	if (invObject->title != 0) {
 		error("A clue can only be hyperlinked if it only has one title!");
 		return;
 	}
-#endif
 
 	uint32 i;
 	for (i = 0; i < MAX_HYPERS; ++i) {