thirdparty/scummvm
2
0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

SCUMM: Get rid of the MemBlkHeader hack

Browse source 
This uncovered at least one potentially serious bug in the inventory
code, which still needs to be investigated and fixed.
This commit is contained in:
Max Horn 2011-05-11 18:06:30 +02:00
parent 0af2f71c71
commit 75b9deb185
10 changed files with 109 additions and 83 deletions
Download patch file Download diff file Expand all files Collapse all files

2
engines/scumm/he/script_v72he.cpp
View file

@ -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++) {

22
engines/scumm/imuse/imuse.cpp
View file

@ -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);

9
engines/scumm/object.cpp
View file

@ -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");

70
engines/scumm/resource.cpp
View file

@ -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++;
}
}

42
engines/scumm/resource.h
View file

@ -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();

10
engines/scumm/saveload.cpp
View file

@ -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]);

12
engines/scumm/script.cpp
View file

@ -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;

2
engines/scumm/script_v6.cpp
View file

@ -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++) {

4
engines/scumm/scumm.cpp
View file

@ -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

17
engines/scumm/scumm.h
View file

@ -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];