From b614ccc2e8eca8e8159c9772669ee45909abdd81 Mon Sep 17 00:00:00 2001 From: Willem Jan Palenstijn Date: Thu, 24 Dec 2015 17:31:34 +0100 Subject: [PATCH] LAB: Avoid manual memory management in Action lists --- engines/lab/console.cpp | 18 +++---- engines/lab/lab.h | 4 +- engines/lab/processroom.cpp | 98 ++++++++++++++++--------------------- engines/lab/processroom.h | 9 ++-- engines/lab/resource.cpp | 86 +++++++++++--------------------- engines/lab/resource.h | 5 +- 6 files changed, 87 insertions(+), 133 deletions(-) diff --git a/engines/lab/console.cpp b/engines/lab/console.cpp index 0a6167951db2..936f91f51c31 100644 --- a/engines/lab/console.cpp +++ b/engines/lab/console.cpp @@ -86,10 +86,9 @@ bool Console::Cmd_DumpSceneResources(int argc, const char **argv) { debugPrintf(" (from %s to %s)", directions[rule->_param1], directions[rule->_param2]); debugPrintf("\n"); - while (rule->_actionList) { - Action *action = rule->_actionList; + Common::List::iterator action; + for (action = rule->_actionList.begin(); action != rule->_actionList.end(); ++action) { debugPrintf(" - %s ('%s', %d, %d, %d)\n", actionTypes[action->_actionType], action->_messages[0].c_str(), action->_param1, action->_param2, action->_param3); - rule->_actionList = rule->_actionList->_nextAction; } } @@ -111,15 +110,14 @@ bool Console::Cmd_FindAction(int argc, const char **argv) { _vm->_resource->readViews(i); for (RuleList::iterator rule = _vm->_rooms[i]._rules->begin(); rule != _vm->_rooms[i]._rules->end(); ++rule) { - while (rule->_actionList) { - if (rule->_actionList->_actionType == actionId && - (rule->_actionList->_param1 == param1 || param1 == -1) && - (rule->_actionList->_param2 == param2 || param2 == -1) && - (rule->_actionList->_param3 == param3 || param3 == -1)) { + Common::List::iterator action; + for (action = rule->_actionList.begin(); action != rule->_actionList.end(); ++action) { + if (action->_actionType == actionId && + (action->_param1 == param1 || param1 == -1) && + (action->_param2 == param2 || param2 == -1) && + (action->_param3 == param3 || param3 == -1)) { debugPrintf("Found at script %d\n", i); } - - rule->_actionList = rule->_actionList->_nextAction; } } } diff --git a/engines/lab/lab.h b/engines/lab/lab.h index 37415057ab8f..a8f5aa3926af 100644 --- a/engines/lab/lab.h +++ b/engines/lab/lab.h @@ -206,7 +206,7 @@ class LabEngine : public Engine { /** * Checks whether all the conditions in a condition list are met. */ - bool checkConditions(int16 *condition); + bool checkConditions(const Common::Array &cond); /** * Decrements the current inventory number. @@ -216,7 +216,7 @@ class LabEngine : public Engine { /** * Processes the action list. */ - void doActions(Action *actionList, CloseDataPtr *closePtrList); + void doActions(const Common::List &actionList, CloseDataPtr *closePtrList); /** * Goes through the rules if an action is taken. diff --git a/engines/lab/processroom.cpp b/engines/lab/processroom.cpp index 312f15323477..b9de824612e2 100644 --- a/engines/lab/processroom.cpp +++ b/engines/lab/processroom.cpp @@ -44,22 +44,12 @@ namespace Lab { #define NOFILE "no file" -bool LabEngine::checkConditions(int16 *condition) { - if (!condition) - return true; - - if (condition[0] == 0) - return true; - - int counter = 1; - bool res = _conditions->in(condition[0]); +bool LabEngine::checkConditions(const Common::Array &condition) { + for (unsigned int i = 0; i < condition.size(); ++i) + if (!_conditions->in(condition[i])) + return false; - while (condition[counter] && res) { - res = _conditions->in(condition[counter]); - counter++; - } - - return res; + return true; } ViewData *LabEngine::getViewData(uint16 roomNum, uint16 direction) { @@ -233,38 +223,39 @@ bool LabEngine::takeItem(Common::Point pos, CloseDataPtr *closePtrList) { return false; } -void LabEngine::doActions(Action *actionList, CloseDataPtr *closePtrList) { - while (actionList) { +void LabEngine::doActions(const Common::List &actionList, CloseDataPtr *closePtrList) { + Common::List::const_iterator action; + for (action = actionList.begin(); action != actionList.end(); ++action) { updateMusicAndEvents(); - switch (actionList->_actionType) { + switch (action->_actionType) { case kActionPlaySound: _music->_loopSoundEffect = false; - _music->readMusic(actionList->_messages[0], true); + _music->readMusic(action->_messages[0], true); break; case kActionPlaySoundNoWait: _music->_loopSoundEffect = false; - _music->readMusic(actionList->_messages[0], false); + _music->readMusic(action->_messages[0], false); break; case kActionPlaySoundLooping: _music->_loopSoundEffect = true; - _music->readMusic(actionList->_messages[0], false); + _music->readMusic(action->_messages[0], false); break; case kActionShowDiff: - _graphics->readPict(actionList->_messages[0], true); + _graphics->readPict(action->_messages[0], true); break; case kActionShowDiffLooping: - _graphics->readPict(actionList->_messages[0], false); + _graphics->readPict(action->_messages[0], false); break; case kActionLoadDiff: - if (!actionList->_messages[0].empty()) + if (!action->_messages[0].empty()) // Puts a file into memory - _graphics->loadPict(actionList->_messages[0]); + _graphics->loadPict(action->_messages[0]); break; @@ -275,7 +266,7 @@ void LabEngine::doActions(Action *actionList, CloseDataPtr *closePtrList) { error("Unused opcode kActionShowBitmap has been called"); case kActionTransition: - _graphics->doTransition((TransitionType)actionList->_param1, closePtrList, actionList->_messages[0].c_str()); + _graphics->doTransition((TransitionType)action->_param1, closePtrList, action->_messages[0].c_str()); break; case kActionNoUpdate: @@ -298,48 +289,47 @@ void LabEngine::doActions(Action *actionList, CloseDataPtr *closePtrList) { break; case kActionSetElement: - _conditions->inclElement(actionList->_param1); + _conditions->inclElement(action->_param1); break; case kActionUnsetElement: - _conditions->exclElement(actionList->_param1); + _conditions->exclElement(action->_param1); break; case kActionShowMessage: if (_graphics->_longWinInFront) - _graphics->longDrawMessage(actionList->_messages[0], true); + _graphics->longDrawMessage(action->_messages[0], true); else - _graphics->drawMessage(actionList->_messages[0], true); + _graphics->drawMessage(action->_messages[0], true); break; case kActionCShowMessage: if (!*closePtrList) - _graphics->drawMessage(actionList->_messages[0], true); + _graphics->drawMessage(action->_messages[0], true); break; case kActionShowMessages: - _graphics->drawMessage(actionList->_messages[_utils->getRandom(actionList->_param1)], true); + _graphics->drawMessage(action->_messages[_utils->getRandom(action->_param1)], true); break; case kActionChangeRoom: - if (actionList->_param1 & 0x8000) { + if (action->_param1 & 0x8000) { // This is a Wyrmkeep Windows trial version, thus stop at this // point, since we can't check for game payment status _graphics->readPict(getPictName(closePtrList)); - actionList = nullptr; GUI::MessageDialog trialMessage("This is the end of the trial version. You can play the full game using the original interpreter from Wyrmkeep"); trialMessage.runModal(); - continue; + break; } - _roomNum = actionList->_param1; - _direction = actionList->_param2 - 1; + _roomNum = action->_param1; + _direction = action->_param2 - 1; *closePtrList = nullptr; _anim->_doBlack = true; break; case kActionSetCloseup: { - Common::Point curPos = Common::Point(_utils->scaleX(actionList->_param1), _utils->scaleY(actionList->_param2)); + Common::Point curPos = Common::Point(_utils->scaleX(action->_param1), _utils->scaleY(action->_param2)); CloseDataPtr tmpClosePtr = getObject(curPos, *closePtrList); if (tmpClosePtr) @@ -352,17 +342,17 @@ void LabEngine::doActions(Action *actionList, CloseDataPtr *closePtrList) { break; case kActionSubInv: - if (_inventory[actionList->_param1]._quantity) - (_inventory[actionList->_param1]._quantity)--; + if (_inventory[action->_param1]._quantity) + (_inventory[action->_param1]._quantity)--; - if (_inventory[actionList->_param1]._quantity == 0) - _conditions->exclElement(actionList->_param1); + if (_inventory[action->_param1]._quantity == 0) + _conditions->exclElement(action->_param1); break; case kActionAddInv: - (_inventory[actionList->_param1]._quantity) += actionList->_param2; - _conditions->inclElement(actionList->_param1); + (_inventory[action->_param1]._quantity) += action->_param2; + _conditions->inclElement(action->_param1); break; case kActionShowDir: @@ -370,7 +360,7 @@ void LabEngine::doActions(Action *actionList, CloseDataPtr *closePtrList) { break; case kActionWaitSecs: { - uint32 targetMillis = _system->getMillis() + actionList->_param1 * 1000; + uint32 targetMillis = _system->getMillis() + action->_param1 * 1000; _graphics->screenUpdate(); @@ -390,7 +380,7 @@ void LabEngine::doActions(Action *actionList, CloseDataPtr *closePtrList) { break; case kActionChangeMusic: - _music->changeMusic(actionList->_messages[0]); + _music->changeMusic(action->_messages[0]); _music->setMusicReset(false); break; @@ -439,13 +429,13 @@ void LabEngine::doActions(Action *actionList, CloseDataPtr *closePtrList) { break; case kActionSpecialCmd: - if (actionList->_param1 == 0) + if (action->_param1 == 0) _anim->_doBlack = true; - else if (actionList->_param1 == 1) + else if (action->_param1 == 1) _anim->_doBlack = (_closeDataPtr == nullptr); - else if (actionList->_param1 == 2) + else if (action->_param1 == 2) _anim->_doBlack = (_closeDataPtr != nullptr); - else if (actionList->_param1 == 5) { + else if (action->_param1 == 5) { // inverse the palette for (int idx = (8 * 3); idx < (255 * 3); idx++) _anim->_diffPalette[idx] = 255 - _anim->_diffPalette[idx]; @@ -454,18 +444,18 @@ void LabEngine::doActions(Action *actionList, CloseDataPtr *closePtrList) { _graphics->setPalette(_anim->_diffPalette, 256); waitTOF(); waitTOF(); - } else if (actionList->_param1 == 4) { + } else if (action->_param1 == 4) { // white the palette _graphics->whiteScreen(); waitTOF(); waitTOF(); - } else if (actionList->_param1 == 6) { + } else if (action->_param1 == 6) { // Restore the palette waitTOF(); _graphics->setPalette(_anim->_diffPalette, 256); waitTOF(); waitTOF(); - } else if (actionList->_param1 == 7) { + } else if (action->_param1 == 7) { // Quick pause waitTOF(); waitTOF(); @@ -474,8 +464,6 @@ void LabEngine::doActions(Action *actionList, CloseDataPtr *closePtrList) { break; } - - actionList = actionList->_nextAction; } if (_music->_loopSoundEffect) { diff --git a/engines/lab/processroom.h b/engines/lab/processroom.h index f5201b1bd3ba..c9f4fa9a20c2 100644 --- a/engines/lab/processroom.h +++ b/engines/lab/processroom.h @@ -148,7 +148,7 @@ struct CloseData { }; struct ViewData { - int16 *_condition; + Common::Array _condition; Common::String _graphicName; ViewData *_nextCondition; CloseDataPtr _closeUps; @@ -159,16 +159,15 @@ struct Action { int16 _param1; int16 _param2; int16 _param3; - Common::String *_messages; - Action *_nextAction; + Common::Array _messages; }; struct Rule { RuleType _ruleType; int16 _param1; int16 _param2; - int16 *_condition; - Action *_actionList; + Common::Array _condition; + Common::List _actionList; }; struct RoomData { diff --git a/engines/lab/resource.cpp b/engines/lab/resource.cpp index d1c915f6c2e2..a3572036deb1 100644 --- a/engines/lab/resource.cpp +++ b/engines/lab/resource.cpp @@ -162,7 +162,7 @@ void Resource::freeViews(uint16 roomNum) { for (int i = 0; i < 4; i++) freeView(_vm->_rooms[roomNum]._view[i]); - freeRule(_vm->_rooms[roomNum]._rules); + delete _vm->_rooms[roomNum]._rules; } Common::String Resource::translateFileName(const Common::String filename) { @@ -242,16 +242,18 @@ Common::String Resource::readString(Common::File *file) { return result; } -int16 *Resource::readConditions(Common::File *file) { - int16 i = 0, cond; - int16 *list = new int16[25]; - memset(list, 0, 25 * sizeof(int16)); +Common::Array Resource::readConditions(Common::File *file) { + int16 cond; + Common::Array list; - do { - cond = file->readUint16LE(); - if (i < 25) - list[i++] = cond; - } while (cond); + while ((cond = file->readUint16LE()) != 0) + list.push_back(cond); + + if (list.size() > 24) { + // The original only allocated 24 elements, and silently + // dropped remaining parts. + warning("More than 24 parts in condition"); + } return list; } @@ -272,59 +274,28 @@ RuleList *Resource::readRule(Common::File *file) { return rules; } -void Resource::freeRule(RuleList *ruleList) { - if (!ruleList) - return; - - for (RuleList::iterator rule = ruleList->begin(); rule != ruleList->end(); ++rule) { - freeAction(rule->_actionList); - delete[] rule->_condition; - } - - delete ruleList; - ruleList = nullptr; -} - -Action *Resource::readAction(Common::File *file) { - Action *action = nullptr; - Action *prev = nullptr; - Action *head = nullptr; +Common::List Resource::readAction(Common::File *file) { + Common::List list; while (file->readByte() == 1) { - action = new Action(); - if (!head) - head = action; - if (prev) - prev->_nextAction = action; - action->_actionType = (ActionType)file->readSint16LE(); - action->_param1 = file->readSint16LE(); - action->_param2 = file->readSint16LE(); - action->_param3 = file->readSint16LE(); - - if (action->_actionType == kActionShowMessages) { - action->_messages = new Common::String[action->_param1]; - - for (int i = 0; i < action->_param1; i++) - action->_messages[i] = readString(file); + list.push_back(Action()); + Action &action = list.back(); + + action._actionType = (ActionType)file->readSint16LE(); + action._param1 = file->readSint16LE(); + action._param2 = file->readSint16LE(); + action._param3 = file->readSint16LE(); + + if (action._actionType == kActionShowMessages) { + action._messages.reserve(action._param1); + for (int i = 0; i < action._param1; i++) + action._messages.push_back(readString(file)); } else { - action->_messages = new Common::String[1]; - action->_messages[0] = readString(file); + action._messages.push_back(readString(file)); } - - action->_nextAction = nullptr; - prev = action; } - return head; -} - -void Resource::freeAction(Action *action) { - while (action) { - Action *nextAction = action->_nextAction; - delete[] action->_messages; - delete action; - action = nextAction; - } + return list; } CloseData *Resource::readCloseUps(uint16 depth, Common::File *file) { @@ -387,7 +358,6 @@ ViewData *Resource::readView(Common::File *file) { void Resource::freeView(ViewData *view) { while (view) { ViewData *nextView = view->_nextCondition; - delete[] view->_condition; freeCloseUps(view->_closeUps); delete view; view = nextView; diff --git a/engines/lab/resource.h b/engines/lab/resource.h index dcb7491c7573..b869ac15adce 100644 --- a/engines/lab/resource.h +++ b/engines/lab/resource.h @@ -109,10 +109,9 @@ class Resource { private: LabEngine *_vm; Common::String readString(Common::File *file); - int16 *readConditions(Common::File *file); + Common::Array readConditions(Common::File *file); RuleList *readRule(Common::File *file); - void freeRule(RuleList *ruleList); - Action *readAction(Common::File *file); + Common::List readAction(Common::File *file); void freeAction(Action *action); CloseData *readCloseUps(uint16 depth, Common::File *file); void freeCloseUps(CloseData *closeUps);