From 75b9deb1856bae8355403faa5f55857f3929adb6 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 11 May 2011 18:06:30 +0200 Subject: SCUMM: Get rid of the MemBlkHeader hack This uncovered at least one potentially serious bug in the inventory code, which still needs to be investigated and fixed. --- engines/scumm/he/script_v72he.cpp | 4 +-- engines/scumm/imuse/imuse.cpp | 22 ++++++------ engines/scumm/object.cpp | 9 +++-- engines/scumm/resource.cpp | 70 ++++++++++++++++++--------------------- engines/scumm/resource.h | 42 +++++++++++++++++++++-- engines/scumm/saveload.cpp | 10 +++--- engines/scumm/script.cpp | 12 +++---- engines/scumm/script_v6.cpp | 2 +- engines/scumm/scumm.cpp | 4 +-- engines/scumm/scumm.h | 17 +++------- 10 files changed, 109 insertions(+), 83 deletions(-) diff --git a/engines/scumm/he/script_v72he.cpp b/engines/scumm/he/script_v72he.cpp index b63feeb580..96ffa2af3b 100644 --- a/engines/scumm/he/script_v72he.cpp +++ b/engines/scumm/he/script_v72he.cpp @@ -621,7 +621,7 @@ void ScummEngine_v72he::o72_getArrayDimSize() { } void ScummEngine_v72he::o72_getNumFreeArrays() { - byte **addr = _res->_types[rtString].address; + byte **addr = _res->_types[rtString]._address; int i, num = 0; for (i = 1; i < _numArray; i++) { @@ -629,7 +629,7 @@ void ScummEngine_v72he::o72_getNumFreeArrays() { num++; } - push (num); + push(num); } void ScummEngine_v72he::o72_roomOps() { diff --git a/engines/scumm/imuse/imuse.cpp b/engines/scumm/imuse/imuse.cpp index e6b134a7d8..4a7ddd05fc 100644 --- a/engines/scumm/imuse/imuse.cpp +++ b/engines/scumm/imuse/imuse.cpp @@ -112,12 +112,12 @@ byte *IMuseInternal::findStartOfSound(int sound) { } // Check for old-style headers first, like 'RO' - if (ptr[4] == 'R' && ptr[5] == 'O'&& ptr[6] != 'L') + if (ptr[0] == 'R' && ptr[1] == 'O'&& ptr[2] != 'L') + return ptr; + if (ptr[4] == 'S' && ptr[5] == 'O') return ptr + 4; - if (ptr[8] == 'S' && ptr[9] == 'O') - return ptr + 8; - ptr += 8; + ptr += 4; size = READ_BE_UINT32(ptr); ptr += 4; @@ -145,7 +145,7 @@ bool IMuseInternal::isMT32(int sound) { if (ptr == NULL) return false; - tag = READ_BE_UINT32(ptr + 4); + tag = READ_BE_UINT32(ptr); switch (tag) { case MKTAG('A','D','L',' '): case MKTAG('A','S','F','X'): // Special AD class for old AdLib sound effects @@ -164,17 +164,17 @@ bool IMuseInternal::isMT32(int sound) { case MKTAG('M','I','D','I'): // Occurs in Sam & Max // HE games use Roland music - if (ptr[12] == 'H' && ptr[13] == 'S') + if (ptr[8] == 'H' && ptr[9] == 'S') return true; else return false; } // Old style 'RO' has equivalent properties to 'ROL' - if (ptr[4] == 'R' && ptr[5] == 'O') + if (ptr[0] == 'R' && ptr[1] == 'O') return true; // Euphony tracks show as 'SO' and have equivalent properties to 'ADL' - if (ptr[8] == 'S' && ptr[9] == 'O') + if (ptr[4] == 'S' && ptr[5] == 'O') return false; error("Unknown music type: '%c%c%c%c'", (char)tag >> 24, (char)tag >> 16, (char)tag >> 8, (char)tag); @@ -192,7 +192,7 @@ bool IMuseInternal::isMIDI(int sound) { if (ptr == NULL) return false; - tag = READ_BE_UINT32(ptr + 4); + tag = READ_BE_UINT32(ptr); switch (tag) { case MKTAG('A','D','L',' '): case MKTAG('A','S','F','X'): // Special AD class for old AdLib sound effects @@ -212,11 +212,11 @@ bool IMuseInternal::isMIDI(int sound) { } // Old style 'RO' has equivalent properties to 'ROL' - if (ptr[4] == 'R' && ptr[5] == 'O') + if (ptr[0] == 'R' && ptr[1] == 'O') return true; // Euphony tracks show as 'SO' and have equivalent properties to 'ADL' // FIXME: Right now we're pretending it's GM. - if (ptr[8] == 'S' && ptr[9] == 'O') + if (ptr[4] == 'S' && ptr[5] == 'O') return true; error("Unknown music type: '%c%c%c%c'", (char)tag >> 24, (char)tag >> 16, (char)tag >> 8, (char)tag); diff --git a/engines/scumm/object.cpp b/engines/scumm/object.cpp index 457e2c717c..8b0e22d33e 100644 --- a/engines/scumm/object.cpp +++ b/engines/scumm/object.cpp @@ -192,8 +192,11 @@ void ScummEngine::clearOwnerOf(int obj) { if (!_inventory[i] && _inventory[i+1]) { _inventory[i] = _inventory[i+1]; _inventory[i+1] = 0; - _res->_types[rtInventory].address[i] = _res->_types[rtInventory].address[i + 1]; - _res->_types[rtInventory].address[i + 1] = NULL; + // FIXME FIXME FIXME: This is incomplete, as we do not touch flags, status... BUG + _res->_types[rtInventory]._address[i] = _res->_types[rtInventory]._address[i + 1]; + _res->_types[rtInventory]._size[i] = _res->_types[rtInventory]._size[i + 1]; + _res->_types[rtInventory]._address[i + 1] = NULL; + _res->_types[rtInventory]._size[i + 1] = 0; } } break; @@ -1796,7 +1799,7 @@ int ScummEngine::findLocalObjectSlot() { int ScummEngine::findFlObjectSlot() { int i; for (i = 1; i < _numFlObject; i++) { - if (_res->_types[rtFlObject].address[i] == NULL) + if (_res->_types[rtFlObject]._address[i] == NULL) return i; } error("findFlObjectSlot: Out of FLObject slots"); diff --git a/engines/scumm/resource.cpp b/engines/scumm/resource.cpp index a00630f1ec..009fd5c11d 100644 --- a/engines/scumm/resource.cpp +++ b/engines/scumm/resource.cpp @@ -537,9 +537,10 @@ void ResourceManager::allocResTypeData(int id, uint32 tag, int num, const char * _types[id].num = num; _types[id].tag = tag; _types[id].name = name; - _types[id].address = (byte **)calloc(num, sizeof(void *)); + _types[id]._address = (byte **)calloc(num, sizeof(byte *)); + _types[id]._size = (uint32 *)calloc(num, sizeof(uint32)); _types[id].flags = (byte *)calloc(num, sizeof(byte)); - _types[id].status = (byte *)calloc(num, sizeof(byte)); + _types[id]._status = (byte *)calloc(num, sizeof(byte)); if (mode) { _types[id].roomno = (byte *)calloc(num, sizeof(byte)); @@ -584,8 +585,6 @@ void ScummEngine::nukeCharset(int i) { } void ScummEngine::ensureResourceLoaded(int type, int i) { - void *addr = NULL; - debugC(DEBUG_RESOURCE, "ensureResourceLoaded(%s,%d)", resTypeFromId(type), i); if ((type == rtRoom) && i > 0x7F && _game.version < 7 && _game.heversion <= 71) { @@ -606,10 +605,7 @@ void ScummEngine::ensureResourceLoaded(int type, int i) { if (type != rtCharset && i == 0) return; - if (i <= _res->_types[type].num) - addr = _res->_types[type].address[i]; - - if (addr) + if (i <= _res->_types[type].num && _res->_types[type]._address[i]) return; loadResource(type, i); @@ -715,9 +711,7 @@ uint32 ScummEngine_v70he::getResourceRoomOffset(int type, int idx) { int ScummEngine::getResourceSize(int type, int idx) { byte *ptr = getResourceAddress(type, idx); assert(ptr); - MemBlkHeader *hdr = (MemBlkHeader *)(ptr - sizeof(MemBlkHeader)); - - return hdr->size; + return _res->_types[type]._size[idx]; } byte *ScummEngine::getResourceAddress(int type, int idx) { @@ -729,17 +723,17 @@ byte *ScummEngine::getResourceAddress(int type, int idx) { if (!_res->validateResource("getResourceAddress", type, idx)) return NULL; - if (!_res->_types[type].address) { - debugC(DEBUG_RESOURCE, "getResourceAddress(%s,%d), _res->_types[type].address == NULL", resTypeFromId(type), idx); + if (!_res->_types[type]._address) { + debugC(DEBUG_RESOURCE, "getResourceAddress(%s,%d), _res->_types[type]._address == NULL", resTypeFromId(type), idx); return NULL; } // If the resource is missing, but loadable from the game data files, try to do so. - if (!_res->_types[type].address[idx] && _res->_types[type]._mode != kDynamicResTypeMode) { + if (!_res->_types[type]._address[idx] && _res->_types[type]._mode != kDynamicResTypeMode) { ensureResourceLoaded(type, idx); } - ptr = (byte *)_res->_types[type].address[idx]; + ptr = (byte *)_res->_types[type]._address[idx]; if (!ptr) { debugC(DEBUG_RESOURCE, "getResourceAddress(%s,%d) == NULL", resTypeFromId(type), idx); return NULL; @@ -747,8 +741,8 @@ byte *ScummEngine::getResourceAddress(int type, int idx) { _res->setResourceCounter(type, idx, 1); - debugC(DEBUG_RESOURCE, "getResourceAddress(%s,%d) == %p", resTypeFromId(type), idx, ptr + sizeof(MemBlkHeader)); - return ptr + sizeof(MemBlkHeader); + debugC(DEBUG_RESOURCE, "getResourceAddress(%s,%d) == %p", resTypeFromId(type), idx, ptr); + return ptr; } byte *ScummEngine::getStringAddress(int i) { @@ -807,29 +801,29 @@ byte *ResourceManager::createResource(int type, int idx, uint32 size) { // cases. For instance, Zak tries to reload the intro music // while it's playing. See bug #1253171. - if (_types[type].address[idx] && (type == rtSound || type == rtScript || type == rtCostume)) - return _types[type].address[idx] + sizeof(MemBlkHeader); + if (_types[type]._address[idx] && (type == rtSound || type == rtScript || type == rtCostume)) + return _types[type]._address[idx]; } nukeResource(type, idx); expireResources(size); - void *ptr = calloc(size + sizeof(MemBlkHeader) + SAFETY_AREA, 1); + byte *ptr = (byte *)calloc(size + SAFETY_AREA, 1); if (ptr == NULL) { error("createResource(%s,%d): Out of memory while allocating %d", resTypeFromId(type), idx, size); } _allocatedSize += size; - _types[type].address[idx] = (byte *)ptr; - ((MemBlkHeader *)ptr)->size = size; + _types[type]._address[idx] = ptr; + _types[type]._size[idx] = size; setResourceCounter(type, idx, 1); - return (byte *)ptr + sizeof(MemBlkHeader); /* skip header */ + return ptr; } ResourceManager::ResTypeData::ResTypeData() { - memset(this, 0, sizeof(this)); + memset(this, 0, sizeof(*this)); } ResourceManager::ResTypeData::~ResTypeData() { @@ -864,18 +858,19 @@ bool ResourceManager::validateResource(const char *str, int type, int idx) const void ResourceManager::nukeResource(int type, int idx) { byte *ptr; - if (!_types[type].address) + if (!_types[type]._address) return; assert(idx >= 0 && idx < _types[type].num); - ptr = _types[type].address[idx]; + ptr = _types[type]._address[idx]; if (ptr != NULL) { debugC(DEBUG_RESOURCE, "nukeResource(%s,%d)", resTypeFromId(type), idx); - _types[type].address[idx] = 0; + _types[type]._address[idx] = 0; + _types[type]._size[idx] = 0; _types[type].flags[idx] = 0; - _types[type].status[idx] &= ~RS_MODIFIED; - _allocatedSize -= ((MemBlkHeader *)ptr)->size; + _types[type]._status[idx] &= ~RS_MODIFIED; + _allocatedSize -= _types[type]._size[idx]; free(ptr); } } @@ -957,13 +952,13 @@ bool ScummEngine::isResourceInUse(int type, int i) const { void ResourceManager::setModified(int type, int i) { if (!validateResource("Modified", type, i)) return; - _types[type].status[i] |= RS_MODIFIED; + _types[type]._status[i] |= RS_MODIFIED; } bool ResourceManager::isModified(int type, int i) const { if (!validateResource("isModified", type, i)) return false; - return (_types[type].status[i] & RS_MODIFIED) != 0; + return (_types[type]._status[i] & RS_MODIFIED) != 0; } void ResourceManager::expireResources(uint32 size) { @@ -993,7 +988,7 @@ void ResourceManager::expireResources(uint32 size) { // so we can potentially unload them to free memory. for (j = _types[i].num; --j >= 0;) { flag = _types[i].flags[j]; - if (!(flag & RF_LOCK) && flag >= best_counter && _types[i].address[j] && !_vm->isResourceInUse(i, j)) { + if (!(flag & RF_LOCK) && flag >= best_counter && _types[i]._address[j] && !_vm->isResourceInUse(i, j)) { best_counter = flag; best_type = i; best_res = j; @@ -1018,9 +1013,10 @@ void ResourceManager::freeResources() { if (isResourceLoaded(i, j)) nukeResource(i, j); } - free(_types[i].address); + free(_types[i]._address); + free(_types[i]._size); free(_types[i].flags); - free(_types[i].status); + free(_types[i]._status); free(_types[i].roomno); free(_types[i].roomoffs); @@ -1055,7 +1051,7 @@ void ScummEngine::loadPtrToResource(int type, int resindex, const byte *source) bool ResourceManager::isResourceLoaded(int type, int idx) const { if (!validateResource("isResourceLoaded", type, idx)) return false; - return _types[type].address[idx] != NULL; + return _types[type]._address[idx] != NULL; } void ResourceManager::resourceStats() { @@ -1066,8 +1062,8 @@ void ResourceManager::resourceStats() { for (i = rtFirst; i <= rtLast; i++) for (j = _types[i].num; --j >= 0;) { flag = _types[i].flags[j]; - if (flag & RF_LOCK && _types[i].address[j]) { - lockedSize += ((MemBlkHeader *)_types[i].roomoffs[j])->size; + if (flag & RF_LOCK && _types[i]._address[j]) { + lockedSize += _types[i]._size[j]; lockedNum++; } } diff --git a/engines/scumm/resource.h b/engines/scumm/resource.h index f1bcc14cb6..dd314cb0c8 100644 --- a/engines/scumm/resource.h +++ b/engines/scumm/resource.h @@ -88,14 +88,50 @@ public: uint16 num; uint32 tag; const char *name; - byte **address; + + /** + * Array of size num containing pointers to each resource of this type. + */ + byte **_address; + + /** + * Array of size num containing the sizes of each resource of this type. + */ + uint32 *_size; protected: + /** + * Array of size num containing TODO of each resource of this type. + */ byte *flags; - byte *status; + + /** + * Array of size num containing the status of each resource of this type. + * This is a bitfield of which currently only one bit is used, which indicates + * whether the resource is modified. + */ + byte *_status; public: + /** + * Array of size num containing for each resource of this type the + * id of the room (resp. the disk) the resource is contained in. + */ byte *roomno; + + /** + * Array of size num containing room offsets of each resource of this type. + * That is the offset (in bytes) where the data for this resources + * can be found in the game data file(s), relative to the start + * of the room the resource is contained in. + * + * A value of RES_INVALID_OFFSET indicates a resources that is not contained + * in the game data files. + */ uint32 *roomoffs; - uint32 *globsize; ///!< Occurs in HE 70+, but we don't use it for anything. + + /** + * Array of size num. Occurs in HE 70+, but we don't use it for anything. + */ + uint32 *globsize; public: ResTypeData(); diff --git a/engines/scumm/saveload.cpp b/engines/scumm/saveload.cpp index cd48feee54..0ca16482ce 100644 --- a/engines/scumm/saveload.cpp +++ b/engines/scumm/saveload.cpp @@ -1247,7 +1247,7 @@ void ScummEngine::saveOrLoad(Serializer *s) { s->saveUint16(type); // Save the res type... for (idx = 0; idx < _res->_types[type].num; idx++) { // Only save resources which actually exist... - if (_res->_types[type].address[idx]) { + if (_res->_types[type]._address[idx]) { s->saveUint16(idx); // Save the index of the resource saveResource(s, type, idx); } @@ -1663,14 +1663,14 @@ void ScummEngine::loadResourceOLD(Serializer *ser, int type, int idx) { } void ScummEngine::saveResource(Serializer *ser, int type, int idx) { - assert(_res->_types[type].address[idx]); + assert(_res->_types[type]._address[idx]); if (_res->_types[type]._mode == kDynamicResTypeMode) { - byte *ptr = _res->_types[type].address[idx]; - uint32 size = ((MemBlkHeader *)ptr)->size; + byte *ptr = _res->_types[type]._address[idx]; + uint32 size = _res->_types[type]._size[idx]; ser->saveUint32(size); - ser->saveBytes(ptr + sizeof(MemBlkHeader), size); + ser->saveBytes(ptr, size); if (type == rtInventory) { ser->saveUint16(_inventory[idx]); diff --git a/engines/scumm/script.cpp b/engines/scumm/script.cpp index 4630ec1a5c..7f279a3f0c 100644 --- a/engines/scumm/script.cpp +++ b/engines/scumm/script.cpp @@ -390,7 +390,7 @@ void ScummEngine::getScriptBaseAddress() { break; _scriptOrgPointer = getResourceAddress(rtInventory, idx); assert(idx < _numInventory); - _lastCodePtr = &_res->_types[rtInventory].address[idx]; + _lastCodePtr = &_res->_types[rtInventory]._address[idx]; break; case WIO_LOCAL: @@ -398,18 +398,18 @@ void ScummEngine::getScriptBaseAddress() { if (_game.version == 8) { _scriptOrgPointer = getResourceAddress(rtRoomScripts, _roomResource); assert(_roomResource < _res->_types[rtRoomScripts].num); - _lastCodePtr = &_res->_types[rtRoomScripts].address[_roomResource]; + _lastCodePtr = &_res->_types[rtRoomScripts]._address[_roomResource]; } else { _scriptOrgPointer = getResourceAddress(rtRoom, _roomResource); assert(_roomResource < _numRooms); - _lastCodePtr = &_res->_types[rtRoom].address[_roomResource]; + _lastCodePtr = &_res->_types[rtRoom]._address[_roomResource]; } break; case WIO_GLOBAL: /* global script */ _scriptOrgPointer = getResourceAddress(rtScript, ss->number); assert(ss->number < _numScripts); - _lastCodePtr = &_res->_types[rtScript].address[ss->number]; + _lastCodePtr = &_res->_types[rtScript]._address[ss->number]; break; case WIO_FLOBJECT: /* flobject script */ @@ -418,7 +418,7 @@ void ScummEngine::getScriptBaseAddress() { idx = _objs[idx].fl_object_index; _scriptOrgPointer = getResourceAddress(rtFlObject, idx); assert(idx < _numFlObject); - _lastCodePtr = &_res->_types[rtFlObject].address[idx]; + _lastCodePtr = &_res->_types[rtFlObject]._address[idx]; break; default: error("Bad type while getting base address"); @@ -445,7 +445,7 @@ void ScummEngine::resetScriptPointer() { * collected by ResourceManager::expireResources. */ void ScummEngine::refreshScriptPointer() { - if (*_lastCodePtr + sizeof(MemBlkHeader) != _scriptOrgPointer) { + if (*_lastCodePtr != _scriptOrgPointer) { long oldoffs = _scriptPointer - _scriptOrgPointer; getScriptBaseAddress(); _scriptPointer = _scriptOrgPointer + oldoffs; diff --git a/engines/scumm/script_v6.cpp b/engines/scumm/script_v6.cpp index afc1eb6909..138da3f921 100644 --- a/engines/scumm/script_v6.cpp +++ b/engines/scumm/script_v6.cpp @@ -357,7 +357,7 @@ void ScummEngine_v6::nukeArray(int a) { } int ScummEngine_v6::findFreeArrayId() { - byte **addr = _res->_types[rtString].address; + byte **addr = _res->_types[rtString]._address; int i; for (i = 1; i < _numArray; i++) { diff --git a/engines/scumm/scumm.cpp b/engines/scumm/scumm.cpp index e89100688a..22736973f7 100644 --- a/engines/scumm/scumm.cpp +++ b/engines/scumm/scumm.cpp @@ -1185,7 +1185,7 @@ Common::Error ScummEngine::init() { resetScummVars(); if (_imuse) { - _imuse->setBase(_res->_types[rtSound].address); + _imuse->setBase(_res->_types[rtSound]._address); } if (_game.version >= 5 && _game.version <= 7) @@ -2462,7 +2462,7 @@ void ScummEngine::restart() { resetScummVars(); if (_imuse) { - _imuse->setBase(_res->_types[rtSound].address); + _imuse->setBase(_res->_types[rtSound]._address); } // Reinit sound engine diff --git a/engines/scumm/scumm.h b/engines/scumm/scumm.h index 768c976388..9d64202ce8 100644 --- a/engines/scumm/scumm.h +++ b/engines/scumm/scumm.h @@ -168,17 +168,6 @@ enum { DEBUG_SMUSH = 1 << 10 // Track SMUSH }; -/** - * Internal header for any memory block allocated by the resource manager. - * - * @todo Hide MemBlkHeader; no code outside the resource manager should - * have to use it, ever. Currently script code needs it to detect whether - * some scripts have moved (in fetchScriptByte()). - */ -struct MemBlkHeader { - uint32 size; -}; - struct VerbSlot; struct ObjectData; @@ -622,9 +611,11 @@ protected: protected: /* Script VM - should be in Script class */ uint32 _localScriptOffsets[1024]; - const byte *_scriptPointer, *_scriptOrgPointer; - byte _opcode, _currentScript; + const byte *_scriptPointer; + const byte *_scriptOrgPointer; const byte * const *_lastCodePtr; + byte _opcode; + byte _currentScript; int _scummStackPos; int _vmStack[150]; -- cgit v1.2.3