AGI: Clean up VM opcode handling

so that invalid opcodes won't crash ScummVM anymore

13 files changed, 194 insertions, 191 deletions

diff --git a/engines/agi/agi.cpp b/engines/agi/agi.cpp
index b293a8e9e5..55872940f5 100644
--- a/engines/agi/agi.cpp
+++ b/engines/agi/agi.cpp
@@ -394,7 +394,6 @@ AgiEngine::AgiEngine(OSystem *syst, const AGIGameDescription *gameDesc) : AgiBas
 
 	resetControllers();
 
-	setupOpcodes();
 	_game._curLogic = NULL;
 	_veryFirstInitialCycle = true;
 	_instructionCounter = 0;
@@ -498,6 +497,8 @@ void AgiEngine::initialize() {
 	} else {
 		warning("Could not open AGI game");
 	}
+	// finally set up actual VM opcodes, because we should now have figured out the right AGI version
+	setupOpCodes(getVersion());
 
 	debugC(2, kDebugLevelMain, "Init sound");
 }
diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index 2b62f9757a..46d0a341df 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -466,8 +466,6 @@ struct AgiGame {
 
 	ScreenObjEntry addToPicView;
 
-	int32 ver;                      /**< detected game version */
-
 	bool automaticSave;             /**< set by CmdSetSimple() */
 	char automaticSaveDescription[SAVEDGAME_DESCRIPTION_LEN + 1];
 
@@ -718,7 +716,20 @@ struct AgiArtificialDelayEntry {
 	uint16 millisecondsDelay;
 };
 
-typedef void (*AgiCommand)(AgiGame *state, AgiEngine *vm, uint8 *p);
+typedef void (*AgiOpCodeFunction)(AgiGame *state, AgiEngine *vm, uint8 *p);
+
+struct AgiOpCodeEntry {
+	const char *name;
+	const char *parameters;
+	AgiOpCodeFunction functionPtr;
+	uint16     parameterSize;
+};
+
+struct AgiOpCodeDefinitionEntry {
+	const char *name;
+	const char *parameters;
+	AgiOpCodeFunction functionPtr;
+};
 
 class AgiEngine : public AgiBase {
 protected:
@@ -985,10 +996,13 @@ private:
 	uint32 _passedPlayTimeCycles; // increased by 1 every time we passed a cycle
 
 private:
-	AgiCommand _agiCommands[183];
-	AgiCommand _agiCondCommands[256];
+	AgiOpCodeEntry _opCodes[256]; // always keep those at 256, so that there is no way for invalid memory access
+	AgiOpCodeEntry _opCodesCond[256];
+
+	void setupOpCodes(uint16 version);
 
-	void setupOpcodes();
+public:
+	const AgiOpCodeEntry *getOpCodesTable() { return _opCodes; }
 };
 
 } // End of namespace Agi
diff --git a/engines/agi/console.cpp b/engines/agi/console.cpp
index 9a4a357b44..3729cd3be6 100644
--- a/engines/agi/console.cpp
+++ b/engines/agi/console.cpp
@@ -94,16 +94,18 @@ bool Console::Cmd_SetObj(int argc, const char **argv) {
 }
 
 bool Console::Cmd_RunOpcode(int argc, const char **argv) {
+	const AgiOpCodeEntry *opCodes = _vm->getOpCodesTable();
+
 	if (argc < 2) {
 		debugPrintf("Usage: runopcode <name> <parameter0> ....\n");
 		return true;
 	}
 
-	for (int i = 0; logicNamesCmd[i].name; i++) {
-		if (!strcmp(argv[1], logicNamesCmd[i].name)) {
+	for (int i = 0; opCodes[i].name; i++) {
+		if (!strcmp(argv[1], opCodes[i].name)) {
 			uint8 p[16];
-			if ((argc - 2) != logicNamesCmd[i].argumentsLength()) {
-				debugPrintf("AGI command wants %d arguments\n", logicNamesCmd[i].argumentsLength());
+			if ((argc - 2) != opCodes[i].parameterSize) {
+				debugPrintf("AGI command wants %d arguments\n", opCodes[i].parameterSize);
 				return 0;
 			}
 			p[0] = argv[2] ? (char)strtoul(argv[2], NULL, 0) : 0;
@@ -112,7 +114,7 @@ bool Console::Cmd_RunOpcode(int argc, const char **argv) {
 			p[3] = argv[5] ? (char)strtoul(argv[5], NULL, 0) : 0;
 			p[4] = argv[6] ? (char)strtoul(argv[6], NULL, 0) : 0;
 
-			debugC(5, kDebugLevelMain, "Opcode: %s %s %s %s", logicNamesCmd[i].name, argv[1], argv[2], argv[3]);
+			debugC(5, kDebugLevelMain, "Opcode: %s %s %s %s", opCodes[i].name, argv[1], argv[2], argv[3]);
 
 			_vm->executeAgiCommand(i, p);
 
@@ -392,24 +394,26 @@ bool Console::Cmd_Room(int argc, const char **argv) {
 }
 
 bool Console::Cmd_BT(int argc, const char **argv) {
+	const AgiOpCodeEntry *opCodes = _vm->getOpCodesTable();
+
 	debugPrintf("Current script: %d\nStack depth: %d\n", _vm->_game.curLogicNr, _vm->_game.execStack.size());
 
 	uint8 *code = NULL;
 	uint8 op = 0;
 	uint8 p[CMD_BSIZE] = { 0 };
-	int num;
+	int parameterSize;
 	Common::Array<ScriptPos>::iterator it;
 
 	for (it = _vm->_game.execStack.begin(); it != _vm->_game.execStack.end(); ++it) {
 		code = _vm->_game.logics[it->script].data;
 		op = code[it->curIP];
-		num = logicNamesCmd[op].argumentsLength();
-		memmove(p, &code[it->curIP], num);
-		memset(p + num, 0, CMD_BSIZE - num);
+		parameterSize = opCodes[op].parameterSize;
+		memmove(p, &code[it->curIP], parameterSize);
+		memset(p + parameterSize, 0, CMD_BSIZE - parameterSize);
 
-		debugPrintf("%d(%d): %s(", it->script, it->curIP, logicNamesCmd[op].name);
+		debugPrintf("%d(%d): %s(", it->script, it->curIP, opCodes[op].name);
 
-		for (int i = 0; i < num; i++)
+		for (int i = 0; i < parameterSize; i++)
 			debugPrintf("%d, ", p[i]);
 
 		debugPrintf(")\n");
diff --git a/engines/agi/id.cpp b/engines/agi/id.cpp
deleted file mode 100644
index 7985d3b9e4..0000000000
--- a/engines/agi/id.cpp
+++ /dev/null
@@ -1,97 +0,0 @@
-/* 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 2
- * 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, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
- *
- */
-
-#include "agi/agi.h"
-#include "agi/opcodes.h"
-
-namespace Agi {
-
-//
-// Currently, there is no known difference between v3.002.098 -> v3.002.149
-// So version emulated;
-//
-// 0x0086,
-// 0x0149
-//
-
-/**
- *
- */
-int AgiEngine::setupV2Game(int ver) {
-	int ec = errOK;
-
-	// Should this go above the previous lines, so we can force emulation versions
-	// even for AGDS games? -- dsymonds
-	if (getFeatures() & GF_AGDS)
-		setVersion(ver = 0x2440);   // ALL AGDS games built for 2.440
-
-	debug(0, "Setting up for version 0x%04X", ver);
-
-	// 'quit' takes 0 args for 2.089
-	if (ver == 0x2089)
-//		logicNamesCmd[0x86].numArgs = 0;
-		logicNamesCmd[0x86].args = "";
-
-	// 'print.at' and 'print.at.v' take 3 args before 2.272
-	// This is documented in the specs as only < 2.440, but it seems
-	// that KQ3 (2.272) needs a 'print.at' taking 4 args.
-	if (ver < 0x2272) {
-//		logicNamesCmd[0x97].numArgs = 3;
-//		logicNamesCmd[0x98].numArgs = 3;
-		logicNamesCmd[0x97].args = "vvv";
-		logicNamesCmd[0x98].args = "vvv";
-	}
-
-	return ec;
-}
-
-/**
- *
- */
-int AgiEngine::setupV3Game(int ver) {
-	int ec = errOK;
-
-	debug(0, "Setting up for version 0x%04X", ver);
-
-	// 'unknown176' takes 1 arg for 3.002.086, not 0 args.
-	// 'unknown173' also takes 1 arg for 3.002.068, not 0 args.
-	// Is this actually used anywhere? -- dsymonds
-	if (ver == 0x3086) {
-//		logicNamesCmd[0xb0].numArgs = 1;
-//		logicNamesCmd[0xad].numArgs = 1;
-		logicNamesCmd[0xb0].args = "n";
-		logicNamesCmd[0xad].args = "n";
-	}
-
-	// FIXME: Apply this fix to other games also that use 2 arguments for command 182.
-	// 'adj.ego.move.to.x.y' (i.e. command 182) takes 2 arguments for at least the
-	// Amiga Gold Rush! (v2.05 1989-03-09) using Amiga AGI 2.316. Amiga's Gold Rush
-	// has been set to use AGI 3.149 in ScummVM so that's why this initialization is
-	// here and not in setupV2Game.
-	if (getGameID() == GID_GOLDRUSH && getPlatform() == Common::kPlatformAmiga)
-//		logicNamesCmd[182].numArgs = 2;
-		logicNamesCmd[182].args = "vv";
-
-	return ec;
-}
-
-} // End of namespace Agi
diff --git a/engines/agi/loader_v1.cpp b/engines/agi/loader_v1.cpp
index 0c569382cf..159e13772a 100644
--- a/engines/agi/loader_v1.cpp
+++ b/engines/agi/loader_v1.cpp
@@ -61,7 +61,7 @@ int AgiLoader_v1::detectGame() {
 	_filenameDisk0 = _vm->getDiskName(BooterDisk1);
 	_filenameDisk1 = _vm->getDiskName(BooterDisk2);
 
-	return _vm->setupV2Game(_vm->getVersion());
+	return errOK;
 }
 
 int AgiLoader_v1::loadDir_DDP(AgiDir *agid, int offset, int max) {
diff --git a/engines/agi/loader_v2.cpp b/engines/agi/loader_v2.cpp
index 43ef46bd72..bebde69fe9 100644
--- a/engines/agi/loader_v2.cpp
+++ b/engines/agi/loader_v2.cpp
@@ -34,7 +34,12 @@ int AgiLoader_v2::detectGame() {
 	        !Common::File::exists(VIEWDIR))
 		return errInvalidAGIFile;
 
-	return _vm->setupV2Game(_vm->getVersion());
+	// Should this go above the previous lines, so we can force emulation versions
+	// even for AGDS games? -- dsymonds
+	if (_vm->getFeatures() & GF_AGDS)
+		_vm->setVersion(0x2440);   // ALL AGDS games built for 2.440
+
+	return errOK;
 }
 
 int AgiLoader_v2::loadDir(AgiDir *agid, const char *fname) {
diff --git a/engines/agi/loader_v3.cpp b/engines/agi/loader_v3.cpp
index 5a208a5114..c21ad41cc5 100644
--- a/engines/agi/loader_v3.cpp
+++ b/engines/agi/loader_v3.cpp
@@ -52,7 +52,7 @@ int AgiLoader_v3::detectGame() {
 			strncpy(_vm->_game.name, f.c_str(), MIN((uint)8, f.size() > 5 ? f.size() - 5 : f.size()));
 			debugC(3, kDebugLevelMain, "game.name = %s", _vm->_game.name);
 
-			ec = _vm->setupV3Game(_vm->getVersion());
+			ec = errOK;
 
 			found = true;
 		}
diff --git a/engines/agi/module.mk b/engines/agi/module.mk
index 32c3ac23ed..8ca261f55b 100644
--- a/engines/agi/module.mk
+++ b/engines/agi/module.mk
@@ -9,7 +9,6 @@ MODULE_OBJS := \
 	font.o \
 	global.o \
 	graphics.o \
-	id.o \
 	inv.o \
 	keyboard.o \
 	loader_v1.o \
diff --git a/engines/agi/op_cmd.cpp b/engines/agi/op_cmd.cpp
index 8a62fce86c..32400b5afe 100644
--- a/engines/agi/op_cmd.cpp
+++ b/engines/agi/op_cmd.cpp
@@ -1051,9 +1051,10 @@ void cmdReleaseKey(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 }
 
 void cmdAdjEgoMoveToXY(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
+	const AgiOpCodeEntry *opCodeTable = vm->getOpCodesTable();
 	int8 x, y;
 
-	switch (logicNamesCmd[182].argumentsLength()) {
+	switch (opCodeTable[182].parameterSize) {
 	// The 2 arguments version is used at least in Amiga Gold Rush!
 	// (v2.05 1989-03-09, Amiga AGI 2.316) in logics 130 and 150
 	// (Using arguments (0, 0), (0, 7), (0, 8), (9, 9) and (-9, 9)).
@@ -2308,7 +2309,7 @@ int AgiEngine::runLogic(int16 logicNr) {
 	AgiGame *state = &_game;
 	uint8 op = 0;
 	uint8 p[CMD_BSIZE] = { 0 };
-	int num = 0;
+	int curParameterSize = 0;
 	ScriptPos sp;
 	//int logic_index = 0;
 
@@ -2390,14 +2391,18 @@ int AgiEngine::runLogic(int16 logicNr) {
 			_game.execStack.pop_back();
 			return 1;
 		default:
-			num = logicNamesCmd[op].argumentsLength();
-			memmove(p, state->_curLogic->data + state->_curLogic->cIP, num);
-			memset(p + num, 0, CMD_BSIZE - num);
+			curParameterSize = _opCodes[op].parameterSize;
+			memmove(p, state->_curLogic->data + state->_curLogic->cIP, curParameterSize);
+			memset(p + curParameterSize, 0, CMD_BSIZE - curParameterSize);
 
-			debugC(2, kDebugLevelScripts, "%s%s(%d %d %d)", st, logicNamesCmd[op].name, p[0], p[1], p[2]);
+			debugC(2, kDebugLevelScripts, "%s%s(%d %d %d)", st, _opCodes[op].name, p[0], p[1], p[2]);
 
-			_agiCommands[op](&_game, this, p);
-			state->_curLogic->cIP += num;
+			if (!_opCodes[op].functionPtr) {
+				error("Illegal opcode %x in logic %d, ip %d", op, state->curLogicNr, state->_curLogic->cIP);
+			}
+
+			_opCodes[op].functionPtr(&_game, this, p);
+			state->_curLogic->cIP += curParameterSize;
 		}
 
 //		if ((op == 0x0B || op == 0x3F || op == 0x40) && logic_index < state->max_logics) {
@@ -2418,9 +2423,9 @@ int AgiEngine::runLogic(int16 logicNr) {
 }
 
 void AgiEngine::executeAgiCommand(uint8 op, uint8 *p) {
-	debugC(2, kDebugLevelScripts, "%s(%d %d %d)", logicNamesCmd[op].name, p[0], p[1], p[2]);
+	debugC(2, kDebugLevelScripts, "%s(%d %d %d)", _opCodes[op].name, p[0], p[1], p[2]);
 
-	_agiCommands[op](&_game, this, p);
+	_opCodes[op].functionPtr(&_game, this, p);
 }
 
 } // End of namespace Agi
diff --git a/engines/agi/op_dbg.cpp b/engines/agi/op_dbg.cpp
index c57782acd5..b35bd3928c 100644
--- a/engines/agi/op_dbg.cpp
+++ b/engines/agi/op_dbg.cpp
@@ -28,16 +28,14 @@ namespace Agi {
 #define ip   (_game.logics[lognum].cIP)
 #define code (_game.logics[lognum].data)
 
-AgiInstruction logicNamesIf[] = {
-	{ "OR", "", NULL },
-	{ "NOT", "", NULL },
-	{ "ELSE", "", NULL },
-	{ "IF", "", NULL }
+const char *logicNamesIf[] = {
+	"OR", "NOT", "ELSE", "IF"
 };
 
 void AgiEngine::debugConsole(int lognum, int mode, const char *str) {
-	AgiInstruction *x;
-	uint8 a, z;
+	AgiOpCodeEntry *curOpCodeTable;
+	uint8 parametersLeft, z;
+	uint8 logicNameIdx;
 	const char *c;
 
 	if (str) {
@@ -52,8 +50,6 @@ void AgiEngine::debugConsole(int lognum, int mode, const char *str) {
 	case 0xFD:
 	case 0xFE:
 	case 0xFF:
-		x = logicNamesIf;
-
 		if (_debug.opcodes) {
 			debugN(0, "%02X %02X %02X %02X %02X %02X %02X %02X %02X\n"
 			       "         ",
@@ -67,12 +63,13 @@ void AgiEngine::debugConsole(int lognum, int mode, const char *str) {
 			       (uint8) * (code + (7 + ip)) & 0xFF,
 			       (uint8) * (code + (8 + ip)) & 0xFF);
 		}
-		debugN(0, "%s ", (x + * (code + ip) - 0xFC)->name);
+		logicNameIdx = (*(code + ip)) - 0xFC;
+		debugN(0, "%s ", logicNamesIf[logicNameIdx]);
 		break;
 	default:
-		x = mode == lCOMMAND_MODE ? logicNamesCmd : logicNamesTest;
-		a = x[*(code + ip)].argumentsLength();
-		c = x[*(code + ip)].args;
+		curOpCodeTable = mode == lCOMMAND_MODE ? _opCodes : _opCodesCond;
+		parametersLeft = curOpCodeTable[*(code + ip)].parameterSize;
+		c = curOpCodeTable[*(code + ip)].parameters;
 
 		if (_debug.opcodes) {
 			debugN(0, "%02X %02X %02X %02X %02X %02X %02X %02X %02X\n"
@@ -87,9 +84,9 @@ void AgiEngine::debugConsole(int lognum, int mode, const char *str) {
 			       (uint8) * (code + (7 + ip)) & 0xFF,
 			       (uint8) * (code + (8 + ip)) & 0xFF);
 		}
-		debugN(0, "%s ", (x + * (code + ip))->name);
+		debugN(0, "%s ", (curOpCodeTable + * (code + ip))->name);
 
-		for (z = 1; a > 0;) {
+		for (z = 1; parametersLeft > 0;) {
 			if (*c == 'n') {
 				debugN(0, "%d", *(code + (ip + z)));
 			} else {
@@ -97,7 +94,7 @@ void AgiEngine::debugConsole(int lognum, int mode, const char *str) {
 			}
 			c++;
 			z++;
-			if (--a > 0)
+			if (--parametersLeft > 0)
 				debugN(0, ",");
 		}
 		break;
diff --git a/engines/agi/op_test.cpp b/engines/agi/op_test.cpp
index 4505668fd1..b4bf38b8a2 100644
--- a/engines/agi/op_test.cpp
+++ b/engines/agi/op_test.cpp
@@ -418,7 +418,7 @@ int AgiEngine::testIfCode(int lognum) {
 
 		default:
 			// Evaluate the command and skip the rest of the instruction
-			_agiCondCommands[op](state, this, p);
+			_opCodesCond[op].functionPtr(state, this, p);
 			skipInstruction(op);
 
 			// NOT mode is enabled only for one instruction
@@ -469,16 +469,26 @@ void AgiEngine::skipInstruction(byte op) {
 		return;
 	if (op == 0x0E && state->_vm->getVersion() >= 0x2000) // said
 		ip += *(code + ip) * 2 + 1;
-	else
-		ip += logicNamesTest[op].argumentsLength();
+	else {
+		ip += _opCodesCond[op].parameterSize;
+	}
 }
 
 void AgiEngine::skipInstructionsUntil(byte v) {
 	AgiGame *state = &_game;
+	int originalIP = state->_curLogic->cIP;
+
 	while (1) {
 		byte op = *(code + ip++);
 		if (op == v)
 			return;
+
+		if (op < 0xFC) {
+			if (!_opCodesCond[op].functionPtr) {
+				// security-check
+				error("illegal opcode %x during skipinstructions in script %d at %d (triggered at %d)", op, state->curLogicNr, ip, originalIP);
+			}
+		}
 		skipInstruction(op);
 	}
 }
diff --git a/engines/agi/opcodes.cpp b/engines/agi/opcodes.cpp
index 472917de77..be6748b40c 100644
--- a/engines/agi/opcodes.cpp
+++ b/engines/agi/opcodes.cpp
@@ -25,10 +25,7 @@
 
 namespace Agi {
 
-AgiInstruction *logicNamesTest;
-AgiInstruction *logicNamesCmd;
-
-AgiInstruction insV1Test[] = {
+const AgiOpCodeDefinitionEntry opCodesV1Cond[] = {
 	{ "",                   "",         &condUnknown },     // 00
 	{ "equaln",             "vn",       &condEqual },       // 01
 	{ "equalv",             "vv",       &condEqualV },      // 02
@@ -48,7 +45,7 @@ AgiInstruction insV1Test[] = {
 	{ "bit",                "nv",       &condBit },         // 10
 };
 
-AgiInstruction insV1[] = {
+const AgiOpCodeDefinitionEntry opCodesV1[] = {
 	{ "return",             "",         NULL },                 // 00
 	{ "increment",          "v",        &cmdIncrement },        // 01
 	{ "decrement",          "v",        &cmdDecrement },        // 02
@@ -149,7 +146,7 @@ AgiInstruction insV1[] = {
 	{ "...",                "nv",       &cmdUnknown },          // 61 # clearbit
 };
 
-AgiInstruction insV2Test[] = {
+AgiOpCodeDefinitionEntry opCodesV2Cond[] = {
 	{ "",                   "",         &condUnknown },         // 00
 	{ "equaln",             "vn",       &condEqual },           // 01
 	{ "equalv",             "vv",       &condEqualV },          // 02
@@ -172,7 +169,7 @@ AgiInstruction insV2Test[] = {
 	{ "in.motion.using.mouse", "",      &condUnknown13 }        // 13
 };
 
-AgiInstruction insV2[] = {
+AgiOpCodeDefinitionEntry opCodesV2[] = {
 	{ "return",             "",         NULL },                 // 00
 	{ "increment",          "v",        &cmdIncrement },        // 01
 	{ "decrement",          "v",        &cmdDecrement },        // 02
@@ -358,36 +355,115 @@ AgiInstruction insV2[] = {
 	{ "adj.ego.move.to.xy", "",         &cmdAdjEgoMoveToXY }    // C6
 };
 
-void AgiEngine::setupOpcodes() {
-	if (getVersion() >= 0x2000) {
-		for (int i = 0; i < ARRAYSIZE(insV2Test); ++i)
-			_agiCondCommands[i] = insV2Test[i].func;
-		for (int i = 0; i < ARRAYSIZE(insV2); ++i)
-			_agiCommands[i] = insV2[i].func;
+//
+// Currently, there is no known difference between v3.002.098 -> v3.002.149
+// So version emulated;
+// 0x0086,
+// 0x0149
+//
 
-		logicNamesTest = insV2Test;
-		logicNamesCmd = insV2;
+void AgiEngine::setupOpCodes(uint16 version) {
+	const AgiOpCodeDefinitionEntry *opCodesTable = nullptr;
+	const AgiOpCodeDefinitionEntry *opCodesCondTable = nullptr;
+	uint16 opCodesTableSize = 0;
+	uint16 opCodesCondTableSize = 0;
+	uint16 opCodesTableMaxSize = sizeof(_opCodes) / sizeof(AgiOpCodeEntry);
+	uint16 opCodesCondTableMaxSize = sizeof(_opCodesCond) / sizeof(AgiOpCodeEntry);
 
-		// Alter opcode parameters for specific games
-		// TODO: This could be either turned into a game feature, or a version
-		// specific check, instead of a game version check
+	debug(0, "Setting up for version 0x%04X", version);
 
-		// The Apple IIGS versions of MH1 and Goldrush both have a parameter for
-		// show.mouse and hide.mouse. Fixes bugs #3577754 and #3426946.
-		if ((getGameID() == GID_MH1 || getGameID() == GID_GOLDRUSH) &&
-		        getPlatform() == Common::kPlatformApple2GS) {
-			logicNamesCmd[176].args = "n";  // hide.mouse
-			logicNamesCmd[178].args = "n";  // show.mouse
-		}
+	if (version >= 0x2000) {
+		opCodesTable = opCodesV2;
+		opCodesCondTable = opCodesV2Cond;
+		opCodesTableSize = ARRAYSIZE(opCodesV2);
+		opCodesCondTableSize = ARRAYSIZE(opCodesV2Cond);
 	} else {
-		for (int i = 0; i < ARRAYSIZE(insV1Test); ++i)
-			_agiCondCommands[i] = insV1Test[i].func;
-		for (int i = 0; i < ARRAYSIZE(insV1); ++i)
-			_agiCommands[i] = insV1[i].func;
+		opCodesTable = opCodesV1;
+		opCodesCondTable = opCodesV1Cond;
+		opCodesTableSize = ARRAYSIZE(opCodesV1);
+		opCodesCondTableSize = ARRAYSIZE(opCodesV1Cond);
+	}
 
-		logicNamesTest = insV1Test;
-		logicNamesCmd = insV1;
+	// copy data over
+	for (int opCodeNr = 0; opCodeNr < opCodesTableSize; opCodeNr++) {
+		_opCodes[opCodeNr].name = opCodesTable[opCodeNr].name;
+		_opCodes[opCodeNr].parameters = opCodesTable[opCodeNr].parameters;
+		_opCodes[opCodeNr].functionPtr = opCodesTable[opCodeNr].functionPtr;
 	}
-}
 
+	for (int opCodeNr = 0; opCodeNr < opCodesCondTableSize; opCodeNr++) {
+		_opCodesCond[opCodeNr].name = opCodesCondTable[opCodeNr].name;
+		_opCodesCond[opCodeNr].parameters = opCodesCondTable[opCodeNr].parameters;
+		_opCodesCond[opCodeNr].functionPtr = opCodesCondTable[opCodeNr].functionPtr;
+	}
+
+	// Alter opcode parameters for specific games
+	if ((version >= 0x2000) && (version < 0x3000)) {
+		// AGI3 adjustments
+
+		// 'quit' takes 0 args for 2.089
+		if (version == 0x2089)
+			_opCodes[0x86].parameters = "";
+
+		// 'print.at' and 'print.at.v' take 3 args before 2.272
+		// This is documented in the specs as only < 2.440, but it seems
+		// that KQ3 (2.272) needs a 'print.at' taking 4 args.
+		if (version < 0x2272) {
+			_opCodes[0x97].parameters = "vvv";
+			_opCodes[0x98].parameters = "vvv";
+		}
+	}
+
+	if (version >= 0x3000) {
+		// AGI3 adjustments
+		// 'unknown176' takes 1 arg for 3.002.086, not 0 args.
+		// 'unknown173' also takes 1 arg for 3.002.068, not 0 args.
+		// Is this actually used anywhere? -- dsymonds
+		if (version == 0x3086) {
+			_opCodes[0xb0].parameters = "n";
+			_opCodes[0xad].parameters = "n";
+		}
+	}
+
+	// TODO: This could be either turned into a game feature, or a version
+	// specific check, instead of a game version check
+	// The Apple IIGS versions of MH1 and Goldrush both have a parameter for
+	// show.mouse and hide.mouse. Fixes bugs #3577754 and #3426946.
+	if ((getGameID() == GID_MH1 || getGameID() == GID_GOLDRUSH) &&
+	        getPlatform() == Common::kPlatformApple2GS) {
+		_opCodes[176].parameters = "n";  // hide.mouse
+		_opCodes[178].parameters = "n";  // show.mouse
+	}
+
+	// FIXME: Apply this fix to other games also that use 2 arguments for command 182.
+	// 'adj.ego.move.to.x.y' (i.e. command 182) takes 2 arguments for at least the
+	// Amiga Gold Rush! (v2.05 1989-03-09) using Amiga AGI 2.316. Amiga's Gold Rush
+	// has been set to use AGI 3.149 in ScummVM so that's why this initialization is
+	// here and not in setupV2Game.
+	if (getGameID() == GID_GOLDRUSH && getPlatform() == Common::kPlatformAmiga)
+		_opCodes[182].parameters = "vv";
+
+	// add invalid entries for every opcode, that is not defined at all
+	for (int opCodeNr = opCodesTableSize; opCodeNr < opCodesTableMaxSize; opCodeNr++) {
+		_opCodes[opCodeNr].name = "illegal";
+		_opCodes[opCodeNr].parameters = "";
+		_opCodes[opCodeNr].functionPtr = nullptr;
+	}
+
+	for (int opCodeNr = opCodesCondTableSize; opCodeNr < opCodesCondTableMaxSize; opCodeNr++) {
+		_opCodesCond[opCodeNr].name = "illegal";
+		_opCodesCond[opCodeNr].parameters = "";
+		_opCodesCond[opCodeNr].functionPtr = nullptr;
+	}
+
+	// calculate parameter size
+	for (int opCodeNr = 0; opCodeNr < opCodesTableSize; opCodeNr++) {
+		_opCodes[opCodeNr].parameterSize = strlen( _opCodes[opCodeNr].parameters);
+	}
+
+	for (int opCodeNr = 0; opCodeNr < opCodesCondTableSize; opCodeNr++) {
+		_opCodesCond[opCodeNr].parameterSize = strlen( _opCodesCond[opCodeNr].parameters);
+	}
 }
+
+} // End of namespace Agi
diff --git a/engines/agi/opcodes.h b/engines/agi/opcodes.h
index d9644bdcda..63c922dc39 100644
--- a/engines/agi/opcodes.h
+++ b/engines/agi/opcodes.h
@@ -25,17 +25,6 @@
 
 namespace Agi {
 
-struct AgiInstruction {
-	const char *name;
-	const char *args;
-	AgiCommand func;
-
-	int argumentsLength() { return strlen(args); }
-};
-
-extern AgiInstruction *logicNamesTest;
-extern AgiInstruction *logicNamesCmd;
-
 void cmdIncrement(AgiGame *state, AgiEngine *vm, uint8 *p);
 void cmdDecrement(AgiGame *state, AgiEngine *vm, uint8 *p);
 void cmdAssignN(AgiGame *state, AgiEngine *vm, uint8 *p);