LAB: Avoid manual memory management in Action lists

6 files changed, 87 insertions, 133 deletions

diff --git a/engines/lab/console.cpp b/engines/lab/console.cpp
index 0a6167951d..936f91f51c 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<Action>::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<Action>::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 37415057ab..a8f5aa3926 100644
--- a/engines/lab/lab.h
+++ b/engines/lab/lab.h
@@ -206,7 +206,7 @@ private:
 	/**
 	 * Checks whether all the conditions in a condition list are met.
 	 */
-	bool checkConditions(int16 *condition);
+	bool checkConditions(const Common::Array<int16> &cond);
 
 	/**
 	 * Decrements the current inventory number.
@@ -216,7 +216,7 @@ private:
 	/**
 	 * Processes the action list.
 	 */
-	void doActions(Action *actionList, CloseDataPtr *closePtrList);
+	void doActions(const Common::List<Action> &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 312f153234..b9de824612 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<int16> &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<Action> &actionList, CloseDataPtr *closePtrList) {
+	Common::List<Action>::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 f5201b1bd3..c9f4fa9a20 100644
--- a/engines/lab/processroom.h
+++ b/engines/lab/processroom.h
@@ -148,7 +148,7 @@ struct CloseData {
 };
 
 struct ViewData {
-	int16 *_condition;
+	Common::Array<int16> _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<Common::String> _messages;
 };
 
 struct Rule {
 	RuleType _ruleType;
 	int16 _param1;
 	int16 _param2;
-	int16 *_condition;
-	Action *_actionList;
+	Common::Array<int16> _condition;
+	Common::List<Action> _actionList;
 };
 
 struct RoomData {
diff --git a/engines/lab/resource.cpp b/engines/lab/resource.cpp
index d1c915f6c2..a3572036de 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<int16> Resource::readConditions(Common::File *file) {
+	int16 cond;
+	Common::Array<int16> 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<Action> Resource::readAction(Common::File *file) {
+	Common::List<Action> 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 dcb7491c75..b869ac15ad 100644
--- a/engines/lab/resource.h
+++ b/engines/lab/resource.h
@@ -109,10 +109,9 @@ public:
 private:
 	LabEngine *_vm;
 	Common::String readString(Common::File *file);
-	int16 *readConditions(Common::File *file);
+	Common::Array<int16> readConditions(Common::File *file);
 	RuleList *readRule(Common::File *file);
-	void freeRule(RuleList *ruleList);
-	Action *readAction(Common::File *file);
+	Common::List<Action> readAction(Common::File *file);
 	void freeAction(Action *action);
 	CloseData *readCloseUps(uint16 depth, Common::File *file);
 	void freeCloseUps(CloseData *closeUps);