[Scummvm-git-logs] scummvm branch-2-8 -> 6848ce8ad2b3165dfccfe7e586f470b83a637f43

sluicebox noreply at scummvm.org
Fri Feb 16 02:55:12 UTC 2024


This automated email contains information about 21 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
7241b9feae AGI: Fix Cmd_RunOpcode output
f81324d82c AGI: Detect V3 volume format in V2 games
312b421662 AGI: Remove unused function declarations
2a35eb308c AGI: Add error check when loading sounds
87f982d806 AGI: Add more error checks when loading sounds
f878d20614 AGI: Use parameter names in function declarations
249d4d0e30 AGI: Remove unused unload code
c6dc29d64d AGI: Rewrite logic decoding for clarity, OOB fixes
b21288d43e AGI: Cleanup loader code
9f4049a678 AGI: Cleanup graphics code
f17b812710 SCI: Fix HOYLE4 unexpected discard in Euchre
4671c96143 AGI: Allow message box to be drawn over menu
b5458e3ad5 AGI: Fix Mixed-Up Mother Goose Amiga emulated version
6f346f9608 AGI: Fix Manhunter 2 Amiga emulated version
f2221d9b1e AGI: Use correct Amiga platform value
8cf76f893e AGI: Fix opcode table version detection
9667ee52c3 AGI: Get rid of the older game flags
d71734fff0 AGI: Remove superfluous AGI version setup for AGDS games
9aa94960bd AGI: Document CoCo3 games in detection table
45bbc029b0 AGI: Handle compressed V3 volumes in V2 games (CoCo3)
6848ce8ad2 SCI32: Fix LIGHTHOUSE uninit variable read on VMD playback


Commit: 7241b9feae48947a13f47ad01916041d2b1e5d46
    https://github.com/scummvm/scummvm/commit/7241b9feae48947a13f47ad01916041d2b1e5d46
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:26:36-05:00

Commit Message:
AGI: Fix Cmd_RunOpcode output

Changed paths:
    engines/agi/console.cpp


diff --git a/engines/agi/console.cpp b/engines/agi/console.cpp
index ec244dd2719..cdbbd0fd50e 100644
--- a/engines/agi/console.cpp
+++ b/engines/agi/console.cpp
@@ -113,7 +113,7 @@ bool Console::Cmd_RunOpcode(int argc, const char **argv) {
 			p[3] = argv[5] ? (char)strtoul(argv[5], nullptr, 0) : 0;
 			p[4] = argv[6] ? (char)strtoul(argv[6], nullptr, 0) : 0;
 
-			debugC(5, kDebugLevelMain, "Opcode: %s %s %s %s", opCodes[i].name, argv[1], argv[2], argv[3]);
+			debugC(5, kDebugLevelMain, "Opcode: %s %d %d %d %d %d", opCodes[i].name, p[0], p[1], p[2], p[3], p[4]);
 
 			_vm->executeAgiCommand(i, p);
 


Commit: f81324d82c69477840adb8f10014ef1df23a64bb
    https://github.com/scummvm/scummvm/commit/f81324d82c69477840adb8f10014ef1df23a64bb
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:26:36-05:00

Commit Message:
AGI: Detect V3 volume format in V2 games

The CoCo3 version of Xmas Card 86 has a volume file with a V3 format.

The V2 loader now detects this and ignores the extra header bytes.

Fixes bug #14699

Changed paths:
    engines/agi/agi.h
    engines/agi/loader_v2.cpp
    engines/agi/loader_v3.cpp


diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index 33f2f90cb42..64893e7cdb0 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -639,14 +639,17 @@ public:
 class AgiLoader_v2 : public AgiLoader {
 private:
 	AgiEngine *_vm;
+	bool _hasV3VolumeFormat;
 
 	int loadDir(AgiDir *agid, const char *fname);
 	uint8 *loadVolRes(AgiDir *agid);
+	bool detectV3VolumeFormat();
 
 public:
 
 	AgiLoader_v2(AgiEngine *vm) {
 		_vm = vm;
+		_hasV3VolumeFormat = false;
 	}
 
 	int init() override;
diff --git a/engines/agi/loader_v2.cpp b/engines/agi/loader_v2.cpp
index 5121711febe..85b26a1fae1 100644
--- a/engines/agi/loader_v2.cpp
+++ b/engines/agi/loader_v2.cpp
@@ -83,6 +83,51 @@ int AgiLoader_v2::loadDir(AgiDir *agid, const char *fname) {
 	return errOK;
 }
 
+/**
+ * Detects if the volume format is really V3.
+ *
+ * The volume format for a V2 game should have 5 byte headers.
+ * The CoCo3 version of Xmas Card 86 has 7 byte headers.
+ * The resource length repeats as if it were a V3 volume with no compression.
+ *
+ * This function detects if a volume has this unusual structure so that
+ * loadVolRes() can ignore the two extra header bytes.
+ */
+bool AgiLoader_v2::detectV3VolumeFormat() {
+	uint8 volume = _vm->_game.dirLogic[0].volume;
+	Common::Path path(Common::String::format("vol.%i", volume));
+	Common::File volumeFile;
+	if (!volumeFile.open(path)) {
+		return false;
+	}
+
+	// read the first few entries and see if they match the 7 byte header
+	uint8 volumeHeader[7];
+	for (int i = 0; i < 5; i++) {
+		if (volumeFile.read(&volumeHeader, 7) != 7) {
+			return false;
+		}
+		// signature
+		if (READ_BE_UINT16(volumeHeader) != 0x1234) {
+			return false;
+		}
+		// volume number
+		if (volumeHeader[2] != volume) {
+			return false;
+		}
+		// duplicate resource lengths
+		uint16 resourceLength1 = READ_LE_UINT16(volumeHeader + 3);
+		uint16 resourceLength2 = READ_LE_UINT16(volumeHeader + 5);
+		if (resourceLength1 != resourceLength2) {
+			return false;
+		}
+		if (!volumeFile.seek(resourceLength1, SEEK_CUR)) {
+			return false;
+		}
+	}
+	return true;
+}
+
 int AgiLoader_v2::init() {
 	int ec = errOK;
 
@@ -94,6 +139,8 @@ int AgiLoader_v2::init() {
 		ec = loadDir(_vm->_game.dirView, VIEWDIR);
 	if (ec == errOK)
 		ec = loadDir(_vm->_game.dirSound, SNDDIR);
+	if (ec == errOK)
+		_hasV3VolumeFormat = detectV3VolumeFormat();
 
 	return ec;
 }
@@ -138,11 +185,11 @@ int AgiLoader_v2::unloadResource(int16 resourceType, int16 resourceNr) {
 /**
  * This function loads a raw resource into memory,
  * if further decoding is required, it must be done by another
- * routine. NULL is returned if unsucsessfull.
+ * routine. NULL is returned if unsuccessful.
  */
 uint8 *AgiLoader_v2::loadVolRes(struct AgiDir *agid) {
 	uint8 *data = nullptr;
-	char x[6];
+	uint8 volumeHeader[7];
 	Common::File fp;
 	unsigned int sig;
 	Common::String path;
@@ -153,9 +200,9 @@ uint8 *AgiLoader_v2::loadVolRes(struct AgiDir *agid) {
 	if (agid->offset != _EMPTY && fp.open(path)) {
 		debugC(3, kDebugLevelResources, "loading resource at offset %d", agid->offset);
 		fp.seek(agid->offset, SEEK_SET);
-		fp.read(&x, 5);
-		if ((sig = READ_BE_UINT16((uint8 *) x)) == 0x1234) {
-			agid->len = READ_LE_UINT16((uint8 *) x + 3);
+		fp.read(&volumeHeader, _hasV3VolumeFormat ? 7 : 5);
+		if ((sig = READ_BE_UINT16(volumeHeader)) == 0x1234) {
+			agid->len = READ_LE_UINT16(volumeHeader + 3);
 			data = (uint8 *)calloc(1, agid->len + 32);
 			if (data != nullptr) {
 				fp.read(data, agid->len);
diff --git a/engines/agi/loader_v3.cpp b/engines/agi/loader_v3.cpp
index 4684864d530..58a0c0505eb 100644
--- a/engines/agi/loader_v3.cpp
+++ b/engines/agi/loader_v3.cpp
@@ -194,7 +194,7 @@ int AgiLoader_v3::unloadResource(int16 resourceType, int16 resourceNr) {
  * If further decoding is required, it must be done by another
  * routine.
  *
- * NULL is returned if unsucsessful.
+ * NULL is returned if unsuccessful.
  */
 uint8 *AgiLoader_v3::loadVolRes(AgiDir *agid) {
 	char x[8];


Commit: 312b42166206c6b100aaf7caaac2408e7bbd6ce2
    https://github.com/scummvm/scummvm/commit/312b42166206c6b100aaf7caaac2408e7bbd6ce2
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:26:37-05:00

Commit Message:
AGI: Remove unused function declarations

Changed paths:
    engines/agi/agi.h
    engines/agi/graphics.h
    engines/agi/loader_v3.cpp
    engines/agi/picture.h
    engines/agi/sound.h
    engines/agi/sprite.h


diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index 64893e7cdb0..8edec43ae49 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -935,14 +935,8 @@ public:
 
 public:
 	void decrypt(uint8 *mem, int len);
-	void releaseSprites();
 	uint16 processAGIEvents();
-	int viewPictures();
 	int runGame();
-	int getAppDir(char *appDir, unsigned int size);
-
-	int setupV2Game(int ver);
-	int setupV3Game(int ver);
 
 	void newRoom(int16 newRoomNr);
 	void resetControllers();
@@ -957,7 +951,6 @@ public:
 
 	// Objects
 public:
-	int showObjects();
 	int loadObjects(const char *fname);
 	int loadObjects(Common::File &fp);
 	const char *objectName(uint16 objectNr);
@@ -1005,8 +998,6 @@ public:
 
 	// View
 private:
-
-	void lSetLoop(ScreenObjEntry *screenObj, int16 loopNr);
 	void updateView(ScreenObjEntry *screenObj);
 
 public:
@@ -1027,8 +1018,6 @@ private:
 	void unpackViewCelDataAGI256(AgiViewCel *celData, byte *compressedData, uint16 compressedSize);
 
 public:
-	void addToPic(int, int, int, int, int, int, int);
-	void drawObj(int);
 	bool isEgoView(const ScreenObjEntry *screenObj);
 
 	// Motion
@@ -1058,14 +1047,12 @@ public:
 
 	// Keyboard
 	int doPollKeyboard();
-	void cleanKeyboard();
 
 	bool handleMouseClicks(uint16 &key);
 	bool handleController(uint16 key);
 
 	bool showPredictiveDialog();
 
-	uint16 agiGetKeypress();
 	int waitKey();
 	int waitAnyKey();
 
diff --git a/engines/agi/graphics.h b/engines/agi/graphics.h
index 726fecf1214..ab9464a2fe0 100644
--- a/engines/agi/graphics.h
+++ b/engines/agi/graphics.h
@@ -92,7 +92,6 @@ public:
 
 	void translateGameRectToDisplayScreen(int16 &x, int16 &y, int16 &width, int16 &height);
 	void translateVisualRectToDisplayScreen(int16 &x, int16 &y, int16 &width, int16 &height);
-	void translateDisplayRectToVisualScreen(int16 &x, int16 &y, int16 &width, int16 &height);
 
 	uint32 getDisplayOffsetToGameScreenPos(int16 x, int16 y);
 	uint32 getDisplayOffsetToVisualScreenPos(int16 x, int16 y);
diff --git a/engines/agi/loader_v3.cpp b/engines/agi/loader_v3.cpp
index 58a0c0505eb..7f3275056ff 100644
--- a/engines/agi/loader_v3.cpp
+++ b/engines/agi/loader_v3.cpp
@@ -232,8 +232,6 @@ uint8 *AgiLoader_v3::loadVolRes(AgiDir *agid) {
 			// Manhunter 2 uses such pictures
 			data = compBuffer;
 			agid->flags |= RES_PICTURE_V3_NIBBLE_PARM;
-			//data = _vm->_picture->convertV3Pic(compBuffer, agid->clen);
-			// compBuffer has been freed inside convertV3Pic()
 		} else if (agid->len == agid->clen) {
 			// do not decompress
 			data = compBuffer;
diff --git a/engines/agi/picture.h b/engines/agi/picture.h
index 4954c3b7189..82231719fdc 100644
--- a/engines/agi/picture.h
+++ b/engines/agi/picture.h
@@ -75,7 +75,6 @@ public:
 private:
 	void draw_xCorner(bool skipOtherCoords = false);
 	void yCorner(bool skipOtherCoords = false);
-	int plotPatternPoint(int x, int y, int bitpos);
 	void plotBrush();
 
 	byte getNextByte();
@@ -112,7 +111,6 @@ public:
 	void showPic(); // <-- for regular AGI games
 	void showPic(int16 x, int16 y, int16 pic_width, int16 pic_height); // <-- for preAGI games
 	void showPicWithTransition();
-	uint8 *convertV3Pic(uint8 *src, uint32 len);
 
 	void plotPattern(int x, int y);     // public because it's used directly by preagi
 
diff --git a/engines/agi/sound.h b/engines/agi/sound.h
index b823b280382..ecc19a7c34e 100644
--- a/engines/agi/sound.h
+++ b/engines/agi/sound.h
@@ -137,7 +137,6 @@ public:
 	void setVolume(uint8 volume);
 
 	void unloadSound(int);
-	void playSound();
 	void startSound(int, int);
 	void stopSound();
 
diff --git a/engines/agi/sprite.h b/engines/agi/sprite.h
index 0c67f772630..96107ed2d22 100644
--- a/engines/agi/sprite.h
+++ b/engines/agi/sprite.h
@@ -94,8 +94,6 @@ public:
 	SpritesMgr(AgiEngine *agi, GfxMgr *gfx);
 	~SpritesMgr();
 
-	int initSprites();
-	void deinitSprites();
 	void addToPic(int16 viewNr, int16 loopNr, int16 celNr, int16 xPos, int16 yPos, int16 priority, int16 border);
 	void addToPicDrawPriorityBox(ScreenObjEntry *screenObj, int16 border);
 };


Commit: 2a35eb308ca03e441f3a8f6a82589ca603983cfd
    https://github.com/scummvm/scummvm/commit/2a35eb308ca03e441f3a8f6a82589ca603983cfd
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:26:37-05:00

Commit Message:
AGI: Add error check when loading sounds

Fixes the CoCo3 version of Xmas Card 86 crashing on startup.

The sound format isn't supported yet but the return value from
AgiSound::createFromRawResource wasn't tested.

Changed paths:
    engines/agi/loader_v2.cpp


diff --git a/engines/agi/loader_v2.cpp b/engines/agi/loader_v2.cpp
index 85b26a1fae1..884daf64a79 100644
--- a/engines/agi/loader_v2.cpp
+++ b/engines/agi/loader_v2.cpp
@@ -282,11 +282,12 @@ int AgiLoader_v2::loadResource(int16 resourceType, int16 resourceNr) {
 
 		data = loadVolRes(&_vm->_game.dirSound[resourceNr]);
 
-		if (data != nullptr) {
-			// Freeing of the raw resource from memory is delegated to the createFromRawResource-function
-			_vm->_game.sounds[resourceNr] = AgiSound::createFromRawResource(data, _vm->_game.dirSound[resourceNr].len, resourceNr, _vm->_soundemu);
+		// "data" is freed by objects created by createFromRawResource on success
+		_vm->_game.sounds[resourceNr] = AgiSound::createFromRawResource(data, _vm->_game.dirSound[resourceNr].len, resourceNr, _vm->_soundemu);
+		if (_vm->_game.sounds[resourceNr] != nullptr) {
 			_vm->_game.dirSound[resourceNr].flags |= RES_LOADED;
 		} else {
+			free(data);
 			ec = errBadResource;
 		}
 		break;


Commit: 87f982d80610af73bb4f35fc4112540705536102
    https://github.com/scummvm/scummvm/commit/87f982d80610af73bb4f35fc4112540705536102
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:26:37-05:00

Commit Message:
AGI: Add more error checks when loading sounds

Applies the previous commit to V1 and V3 loaders

Changed paths:
    engines/agi/loader_v1.cpp
    engines/agi/loader_v3.cpp


diff --git a/engines/agi/loader_v1.cpp b/engines/agi/loader_v1.cpp
index ea192142ec9..71af5156014 100644
--- a/engines/agi/loader_v1.cpp
+++ b/engines/agi/loader_v1.cpp
@@ -257,11 +257,12 @@ int AgiLoader_v1::loadResource(int16 resourceType, int16 resourceNr) {
 
 		data = loadVolRes(&_vm->_game.dirSound[resourceNr]);
 
-		if (data != nullptr) {
-			// Freeing of the raw resource from memory is delegated to the createFromRawResource-function
-			_vm->_game.sounds[resourceNr] = AgiSound::createFromRawResource(data, _vm->_game.dirSound[resourceNr].len, resourceNr, _vm->_soundemu);
+		// "data" is freed by objects created by createFromRawResource on success
+		_vm->_game.sounds[resourceNr] = AgiSound::createFromRawResource(data, _vm->_game.dirSound[resourceNr].len, resourceNr, _vm->_soundemu);
+		if (_vm->_game.sounds[resourceNr] != nullptr) {
 			_vm->_game.dirSound[resourceNr].flags |= RES_LOADED;
 		} else {
+			free(data);
 			ec = errBadResource;
 		}
 		break;
diff --git a/engines/agi/loader_v3.cpp b/engines/agi/loader_v3.cpp
index 7f3275056ff..24092f9deb6 100644
--- a/engines/agi/loader_v3.cpp
+++ b/engines/agi/loader_v3.cpp
@@ -316,11 +316,13 @@ int AgiLoader_v3::loadResource(int16 resourceType, int16 resourceNr) {
 			break;
 
 		data = loadVolRes(&_vm->_game.dirSound[resourceNr]);
-		if (data != nullptr) {
-			// Freeing of the raw resource from memory is delegated to the createFromRawResource-function
-			_vm->_game.sounds[resourceNr] = AgiSound::createFromRawResource(data, _vm->_game.dirSound[resourceNr].len, resourceNr, _vm->_soundemu);
+
+		// "data" is freed by objects created by createFromRawResource on success
+		_vm->_game.sounds[resourceNr] = AgiSound::createFromRawResource(data, _vm->_game.dirSound[resourceNr].len, resourceNr, _vm->_soundemu);
+		if (_vm->_game.sounds[resourceNr] != nullptr) {
 			_vm->_game.dirSound[resourceNr].flags |= RES_LOADED;
 		} else {
+			free(data);
 			ec = errBadResource;
 		}
 		break;


Commit: f878d20614c8d1b2526eace3beb27dea4dbebbb4
    https://github.com/scummvm/scummvm/commit/f878d20614c8d1b2526eace3beb27dea4dbebbb4
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:26:37-05:00

Commit Message:
AGI: Use parameter names in function declarations

Changed paths:
    engines/agi/agi.h
    engines/agi/lzw.h
    engines/agi/objects.cpp
    engines/agi/picture.cpp
    engines/agi/picture.h
    engines/agi/sound.h
    engines/agi/sound_2gs.h
    engines/agi/sound_coco3.h
    engines/agi/sound_pcjr.cpp
    engines/agi/sound_pcjr.h
    engines/agi/sound_sarien.h
    engines/agi/systemui.h
    engines/agi/view.cpp


diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index 8edec43ae49..e9c59144433 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -610,8 +610,8 @@ public:
 	virtual int detectGame() = 0;
 	virtual int loadResource(int16 resourceType, int16 resourceNr) = 0;
 	virtual int unloadResource(int16 resourceType, int16 resourceNr) = 0;
-	virtual int loadObjects(const char *) = 0;
-	virtual int loadWords(const char *) = 0;
+	virtual int loadObjects(const char *fname) = 0;
+	virtual int loadWords(const char *fname) = 0;
 };
 
 class AgiLoader_v1 : public AgiLoader {
@@ -632,8 +632,8 @@ public:
 	int detectGame() override;
 	int loadResource(int16 resourceType, int16 resourceNr) override;
 	int unloadResource(int16 resourceType, int16 resourceNr) override;
-	int loadObjects(const char *) override;
-	int loadWords(const char *) override;
+	int loadObjects(const char *fname) override;
+	int loadWords(const char *fname) override;
 };
 
 class AgiLoader_v2 : public AgiLoader {
@@ -657,8 +657,8 @@ public:
 	int detectGame() override;
 	int loadResource(int16 resourceType, int16 resourceNr) override;
 	int unloadResource(int16 resourceType, int16 resourceNr) override;
-	int loadObjects(const char *) override;
-	int loadWords(const char *) override;
+	int loadObjects(const char *fname) override;
+	int loadWords(const char *fname) override;
 };
 
 class AgiLoader_v3 : public AgiLoader {
@@ -679,8 +679,8 @@ public:
 	int detectGame() override;
 	int loadResource(int16 resourceType, int16 resourceNr) override;
 	int unloadResource(int16 resourceType, int16 resourceNr) override;
-	int loadObjects(const char *) override;
-	int loadWords(const char *) override;
+	int loadObjects(const char *fname) override;
+	int loadWords(const char *fname) override;
 };
 
 class GfxFont;
@@ -955,7 +955,7 @@ public:
 	int loadObjects(Common::File &fp);
 	const char *objectName(uint16 objectNr);
 	int objectGetLocation(uint16 objectNr);
-	void objectSetLocation(uint16 objectNr, int);
+	void objectSetLocation(uint16 objectNr, int location);
 private:
 	int decodeObjects(uint8 *mem, uint32 flen);
 	int readObjects(Common::File &fp, int flen);
@@ -965,9 +965,9 @@ public:
 	int decodeLogic(int16 logicNr);
 	void unloadLogic(int16 logicNr);
 	int runLogic(int16 logicNr);
-	void debugConsole(int, int, const char *);
+	void debugConsole(int lognum, int mode, const char *str);
 	bool testIfCode(int16 logicNr);
-	void executeAgiCommand(uint8, uint8 *);
+	void executeAgiCommand(uint8 op, uint8 *p);
 
 private:
 	bool _veryFirstInitialCycle; /**< signals, that currently the very first cycle is executed (restarts, etc. do not count!) */
@@ -988,13 +988,13 @@ public:
 	// Some submethods of testIfCode
 	void skipInstruction(byte op);
 	void skipInstructionsUntil(byte v);
-	uint8 testObjRight(uint8, uint8, uint8, uint8, uint8);
-	uint8 testObjCenter(uint8, uint8, uint8, uint8, uint8);
-	uint8 testObjInBox(uint8, uint8, uint8, uint8, uint8);
-	uint8 testPosn(uint8, uint8, uint8, uint8, uint8);
-	uint8 testSaid(uint8, uint8 *);
-	uint8 testController(uint8);
-	uint8 testCompareStrings(uint8, uint8);
+	uint8 testObjRight(uint8 n, uint8 x1, uint8 y1, uint8 x2, uint8 y2);
+	uint8 testObjCenter(uint8 n, uint8 x1, uint8 y1, uint8 x2, uint8 y2);
+	uint8 testObjInBox(uint8 n, uint8 x1, uint8 y1, uint8 x2, uint8 y2);
+	uint8 testPosn(uint8 n, uint8 x1, uint8 y1, uint8 x2, uint8 y2);
+	uint8 testSaid(uint8 nwords, uint8 *cc);
+	uint8 testController(uint8 cont);
+	uint8 testCompareStrings(uint8 s1, uint8 s2);
 
 	// View
 private:
@@ -1007,8 +1007,8 @@ public:
 
 	void clipViewCoordinates(ScreenObjEntry *screenObj);
 
-	void startUpdate(ScreenObjEntry *);
-	void stopUpdate(ScreenObjEntry *);
+	void startUpdate(ScreenObjEntry *viewPtr);
+	void stopUpdate(ScreenObjEntry *viewPtr);
 	void updateScreenObjTable();
 	void unloadView(int16 viewNr);
 	int decodeView(byte *resourceData, uint16 resourceSize, int16 viewNr);
diff --git a/engines/agi/lzw.h b/engines/agi/lzw.h
index 1d4f635b3a5..a2540119612 100644
--- a/engines/agi/lzw.h
+++ b/engines/agi/lzw.h
@@ -24,7 +24,7 @@
 
 namespace Agi {
 
-void lzwExpand(uint8 *, uint8 *, int32);
+void lzwExpand(uint8 *in, uint8 *out, int32 len);
 
 } // End of namespace Agi
 
diff --git a/engines/agi/objects.cpp b/engines/agi/objects.cpp
index 2bbe73ee533..d5964bbc4c5 100644
--- a/engines/agi/objects.cpp
+++ b/engines/agi/objects.cpp
@@ -121,12 +121,12 @@ int AgiEngine::readObjects(Common::File &fp, int flen) {
 	return errOK;
 }
 
-void AgiEngine::objectSetLocation(uint16 objectNr, int i) {
+void AgiEngine::objectSetLocation(uint16 objectNr, int location) {
 	if (objectNr >= _game.numObjects) {
 		warning("AgiEngine::objectSetLocation: Can't access object %d.\n", objectNr);
 		return;
 	}
-	_objects[objectNr].location = i;
+	_objects[objectNr].location = location;
 }
 
 int AgiEngine::objectGetLocation(uint16 objectNr) {
diff --git a/engines/agi/picture.cpp b/engines/agi/picture.cpp
index 64b7c5b9a08..b6c16ab3a25 100644
--- a/engines/agi/picture.cpp
+++ b/engines/agi/picture.cpp
@@ -983,13 +983,13 @@ int PictureMgr::decodePicture(byte *data, uint32 length, int clr, int pic_width,
  * Unload an AGI picture resource.
  * This function unloads an AGI picture resource and deallocates
  * resource data.
- * @param n AGI picture resource number
+ * @param picNr AGI picture resource number
  */
-int PictureMgr::unloadPicture(int n) {
+int PictureMgr::unloadPicture(int picNr) {
 	// remove visual buffer & priority buffer if they exist
-	if (_vm->_game.dirPic[n].flags & RES_LOADED) {
-		free(_vm->_game.pictures[n].rdata);
-		_vm->_game.dirPic[n].flags &= ~RES_LOADED;
+	if (_vm->_game.dirPic[picNr].flags & RES_LOADED) {
+		free(_vm->_game.pictures[picNr].rdata);
+		_vm->_game.dirPic[picNr].flags &= ~RES_LOADED;
 	}
 
 	return errOK;
diff --git a/engines/agi/picture.h b/engines/agi/picture.h
index 82231719fdc..b259dd38ebe 100644
--- a/engines/agi/picture.h
+++ b/engines/agi/picture.h
@@ -85,7 +85,7 @@ public:
 
 	int decodePicture(int16 resourceNr, bool clearScreen, bool agi256 = false, int16 pic_width = _DEFAULT_WIDTH, int16 pic_height = _DEFAULT_HEIGHT);
 	int decodePicture(byte *data, uint32 length, int clear, int pic_width = _DEFAULT_WIDTH, int pic_height = _DEFAULT_HEIGHT);
-	int unloadPicture(int);
+	int unloadPicture(int picNr);
 	void drawPicture();
 private:
 	void drawPictureC64();
diff --git a/engines/agi/sound.h b/engines/agi/sound.h
index ecc19a7c34e..5acbe467606 100644
--- a/engines/agi/sound.h
+++ b/engines/agi/sound.h
@@ -77,7 +77,7 @@ public:
 	virtual ~SoundGen();
 
 	virtual void play(int resnum) = 0;
-	virtual void stop(void) = 0;
+	virtual void stop() = 0;
 
 	AgiBase *_vm;
 
diff --git a/engines/agi/sound_2gs.h b/engines/agi/sound_2gs.h
index b6413b05fc9..edaab6b51dc 100644
--- a/engines/agi/sound_2gs.h
+++ b/engines/agi/sound_2gs.h
@@ -222,7 +222,7 @@ public:
 	~SoundGen2GS() override;
 
 	void play(int resnum) override;
-	void stop(void) override;
+	void stop() override;
 
 	int readBuffer(int16 *buffer, const int numSamples) override;
 
diff --git a/engines/agi/sound_coco3.h b/engines/agi/sound_coco3.h
index aca1261b549..b8e3c3b273c 100644
--- a/engines/agi/sound_coco3.h
+++ b/engines/agi/sound_coco3.h
@@ -45,7 +45,7 @@ public:
 	~SoundGenCoCo3() override;
 
 	void play(int resnum) override;
-	void stop(void) override;
+	void stop() override;
 
 	// AudioStream API
 	int readBuffer(int16 *buffer, const int numSamples) override;
diff --git a/engines/agi/sound_pcjr.cpp b/engines/agi/sound_pcjr.cpp
index d256911a937..efba2b4b788 100644
--- a/engines/agi/sound_pcjr.cpp
+++ b/engines/agi/sound_pcjr.cpp
@@ -159,7 +159,7 @@ void SoundGenPCJr::play(int resnum) {
 	_v1size = pcjrSound->getLength() - 1;
 }
 
-void SoundGenPCJr::stop(void) {
+void SoundGenPCJr::stop() {
 	int i;
 
 	for (i = 0; i < CHAN_MAX; i++) {
diff --git a/engines/agi/sound_pcjr.h b/engines/agi/sound_pcjr.h
index e426c4a26b7..b236c266ef9 100644
--- a/engines/agi/sound_pcjr.h
+++ b/engines/agi/sound_pcjr.h
@@ -76,7 +76,7 @@ public:
 	~SoundGenPCJr() override;
 
 	void play(int resnum) override;
-	void stop(void) override;
+	void stop() override;
 
 	// AudioStream API
 	int readBuffer(int16 *buffer, const int numSamples) override;
diff --git a/engines/agi/sound_sarien.h b/engines/agi/sound_sarien.h
index d09f8447fbd..e76071b63e0 100644
--- a/engines/agi/sound_sarien.h
+++ b/engines/agi/sound_sarien.h
@@ -71,7 +71,7 @@ public:
 	~SoundGenSarien() override;
 
 	void play(int resnum) override;
-	void stop(void) override;
+	void stop() override;
 
 	// AudioStream API
 	int readBuffer(int16 *buffer, const int numSamples) override;
diff --git a/engines/agi/systemui.h b/engines/agi/systemui.h
index 7ec6504e77f..62a137f1423 100644
--- a/engines/agi/systemui.h
+++ b/engines/agi/systemui.h
@@ -109,7 +109,6 @@ private:
 	SystemUIButtonArray _buttonArray;
 
 	Common::Rect createRect(int16 x, int16 adjX, int16 y, int16 adjY, int16 width, int16 adjWidth, int16 height, int16 adjHeight);
-	//void moveRect(int16 x, int16 adjX, int16 y, int16 adjY);
 
 	void drawButton(SystemUIButtonEntry *button);
 	void drawButtonAppleIIgs(SystemUIButtonEntry *buttonEntry);
diff --git a/engines/agi/view.cpp b/engines/agi/view.cpp
index 4cfbc810a2f..6e52e936cf6 100644
--- a/engines/agi/view.cpp
+++ b/engines/agi/view.cpp
@@ -587,12 +587,12 @@ void AgiEngine::clipViewCoordinates(ScreenObjEntry *screenObj) {
 
 /**
  * Set the view table entry as updating.
- * @param v pointer to view table entry
+ * @param viewPtr pointer to view table entry
  */
-void AgiEngine::startUpdate(ScreenObjEntry *v) {
-	if (~v->flags & fUpdate) {
+void AgiEngine::startUpdate(ScreenObjEntry *viewPtr) {
+	if (~viewPtr->flags & fUpdate) {
 		_sprites->eraseSprites();
-		v->flags |= fUpdate;
+		viewPtr->flags |= fUpdate;
 		_sprites->buildAllSpriteLists();
 		_sprites->drawAllSpriteLists();
 	}
@@ -600,7 +600,7 @@ void AgiEngine::startUpdate(ScreenObjEntry *v) {
 
 /**
  * Set the view table entry as non-updating.
- * @param v pointer to view table entry
+ * @param viewPtr pointer to view table entry
  */
 void AgiEngine::stopUpdate(ScreenObjEntry *viewPtr) {
 	if (viewPtr->flags & fUpdate) {


Commit: 249d4d0e30535137cc22993f69a8f30416adf50a
    https://github.com/scummvm/scummvm/commit/249d4d0e30535137cc22993f69a8f30416adf50a
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:26:38-05:00

Commit Message:
AGI: Remove unused unload code

Changed paths:
    engines/agi/agi.cpp
    engines/agi/agi.h
    engines/agi/loader_v1.cpp
    engines/agi/loader_v2.cpp
    engines/agi/loader_v3.cpp


diff --git a/engines/agi/agi.cpp b/engines/agi/agi.cpp
index fc34349f9ef..23e4cc22df1 100644
--- a/engines/agi/agi.cpp
+++ b/engines/agi/agi.cpp
@@ -185,35 +185,28 @@ int AgiEngine::agiInit() {
  */
 
 void AgiEngine::agiUnloadResources() {
-	int i;
-
 	// Make sure logic 0 is always loaded
-	for (i = 1; i < MAX_DIRECTORY_ENTRIES; i++) {
+	for (int i = 1; i < MAX_DIRECTORY_ENTRIES; i++) {
 		_loader->unloadResource(RESOURCETYPE_LOGIC, i);
 	}
-	for (i = 0; i < MAX_DIRECTORY_ENTRIES; i++) {
+	for (int i = 0; i < MAX_DIRECTORY_ENTRIES; i++) {
 		_loader->unloadResource(RESOURCETYPE_VIEW, i);
 		_loader->unloadResource(RESOURCETYPE_PICTURE, i);
 		_loader->unloadResource(RESOURCETYPE_SOUND, i);
 	}
 }
 
-int AgiEngine::agiDeinit() {
-	int ec;
-
+void AgiEngine::agiDeinit() {
 	if (!_loader)
-		return errOK;
+		return;
 
 	_words->clearEgoWords(); // remove all words from memory
 	agiUnloadResources();    // unload resources in memory
 	_loader->unloadResource(RESOURCETYPE_LOGIC, 0);
-	ec = _loader->deinit();
 	_objects.clear();
 	_words->unloadDictionary();
 
 	clearImageStack();
-
-	return ec;
 }
 
 int AgiEngine::agiLoadResource(int16 resourceType, int16 resourceNr) {
@@ -240,8 +233,8 @@ int AgiEngine::agiLoadResource(int16 resourceType, int16 resourceNr) {
 	return i;
 }
 
-int AgiEngine::agiUnloadResource(int16 resourceType, int16 resourceNr) {
-	return _loader->unloadResource(resourceType, resourceNr);
+void AgiEngine::agiUnloadResource(int16 resourceType, int16 resourceNr) {
+	_loader->unloadResource(resourceType, resourceNr);
 }
 
 struct GameSettings {
diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index e9c59144433..1138e3bdc56 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -606,10 +606,9 @@ public:
 	virtual ~AgiLoader() {}
 
 	virtual int init() = 0;
-	virtual int deinit() = 0;
 	virtual int detectGame() = 0;
 	virtual int loadResource(int16 resourceType, int16 resourceNr) = 0;
-	virtual int unloadResource(int16 resourceType, int16 resourceNr) = 0;
+	virtual void unloadResource(int16 resourceType, int16 resourceNr) = 0;
 	virtual int loadObjects(const char *fname) = 0;
 	virtual int loadWords(const char *fname) = 0;
 };
@@ -628,10 +627,9 @@ public:
 	AgiLoader_v1(AgiEngine *vm);
 
 	int init() override;
-	int deinit() override;
 	int detectGame() override;
 	int loadResource(int16 resourceType, int16 resourceNr) override;
-	int unloadResource(int16 resourceType, int16 resourceNr) override;
+	void unloadResource(int16 resourceType, int16 resourceNr) override;
 	int loadObjects(const char *fname) override;
 	int loadWords(const char *fname) override;
 };
@@ -653,10 +651,9 @@ public:
 	}
 
 	int init() override;
-	int deinit() override;
 	int detectGame() override;
 	int loadResource(int16 resourceType, int16 resourceNr) override;
-	int unloadResource(int16 resourceType, int16 resourceNr) override;
+	void unloadResource(int16 resourceType, int16 resourceNr) override;
 	int loadObjects(const char *fname) override;
 	int loadWords(const char *fname) override;
 };
@@ -675,10 +672,9 @@ public:
 	}
 
 	int init() override;
-	int deinit() override;
 	int detectGame() override;
 	int loadResource(int16 resourceType, int16 resourceNr) override;
-	int unloadResource(int16 resourceType, int16 resourceNr) override;
+	void unloadResource(int16 resourceType, int16 resourceNr) override;
 	int loadObjects(const char *fname) override;
 	int loadWords(const char *fname) override;
 };
@@ -914,10 +910,10 @@ public:
 	void wait(uint32 msec, bool busy = false);
 
 	int agiInit();
-	int agiDeinit();
+	void agiDeinit();
 	int agiDetectGame();
 	int agiLoadResource(int16 resourceType, int16 resourceNr);
-	int agiUnloadResource(int16 resourceType, int16 resourceNr);
+	void agiUnloadResource(int16 resourceType, int16 resourceNr);
 	void agiUnloadResources();
 
 	int getKeypress() override;
diff --git a/engines/agi/loader_v1.cpp b/engines/agi/loader_v1.cpp
index 71af5156014..b55d78b3f9d 100644
--- a/engines/agi/loader_v1.cpp
+++ b/engines/agi/loader_v1.cpp
@@ -164,11 +164,6 @@ int AgiLoader_v1::init() {
 	return ec;
 }
 
-int AgiLoader_v1::deinit() {
-	int ec = errOK;
-	return ec;
-}
-
 uint8 *AgiLoader_v1::loadVolRes(struct AgiDir *agid) {
 	uint8 *data = nullptr;
 	Common::File fp;
@@ -293,7 +288,7 @@ int AgiLoader_v1::loadResource(int16 resourceType, int16 resourceNr) {
 	return ec;
 }
 
-int AgiLoader_v1::unloadResource(int16 resourceType, int16 resourceNr) {
+void AgiLoader_v1::unloadResource(int16 resourceType, int16 resourceNr) {
 	switch (resourceType) {
 	case RESOURCETYPE_LOGIC:
 		_vm->unloadLogic(resourceNr);
@@ -310,8 +305,6 @@ int AgiLoader_v1::unloadResource(int16 resourceType, int16 resourceNr) {
 	default:
 		break;
 	}
-
-	return errOK;
 }
 
 int AgiLoader_v1::loadObjects(const char *fname) {
diff --git a/engines/agi/loader_v2.cpp b/engines/agi/loader_v2.cpp
index 884daf64a79..1a96ed10777 100644
--- a/engines/agi/loader_v2.cpp
+++ b/engines/agi/loader_v2.cpp
@@ -145,23 +145,7 @@ int AgiLoader_v2::init() {
 	return ec;
 }
 
-int AgiLoader_v2::deinit() {
-	int ec = errOK;
-
-#if 0
-	// unload words
-	agiV2UnloadWords();
-
-	// unload objects
-	agiV2UnloadObjects();
-#endif
-
-	return ec;
-}
-
-int AgiLoader_v2::unloadResource(int16 resourceType, int16 resourceNr) {
-	debugC(3, kDebugLevelResources, "unload resource");
-
+void AgiLoader_v2::unloadResource(int16 resourceType, int16 resourceNr) {
 	switch (resourceType) {
 	case RESOURCETYPE_LOGIC:
 		_vm->unloadLogic(resourceNr);
@@ -178,8 +162,6 @@ int AgiLoader_v2::unloadResource(int16 resourceType, int16 resourceNr) {
 	default:
 		break;
 	}
-
-	return errOK;
 }
 
 /**
diff --git a/engines/agi/loader_v3.cpp b/engines/agi/loader_v3.cpp
index 24092f9deb6..d7f418ab767 100644
--- a/engines/agi/loader_v3.cpp
+++ b/engines/agi/loader_v3.cpp
@@ -154,21 +154,7 @@ int AgiLoader_v3::init() {
 	return ec;
 }
 
-int AgiLoader_v3::deinit() {
-	int ec = errOK;
-
-#if 0
-	// unload words
-	agiV3UnloadWords();
-
-	// unload objects
-	agiV3UnloadObjects();
-#endif
-
-	return ec;
-}
-
-int AgiLoader_v3::unloadResource(int16 resourceType, int16 resourceNr) {
+void AgiLoader_v3::unloadResource(int16 resourceType, int16 resourceNr) {
 	switch (resourceType) {
 	case RESOURCETYPE_LOGIC:
 		_vm->unloadLogic(resourceNr);
@@ -185,8 +171,6 @@ int AgiLoader_v3::unloadResource(int16 resourceType, int16 resourceNr) {
 	default:
 		break;
 	}
-
-	return errOK;
 }
 
 /**


Commit: c6dc29d64dd685ce7f835aa104ce2351f9c5b929
    https://github.com/scummvm/scummvm/commit/c6dc29d64dd685ce7f835aa104ce2351f9c5b929
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:26:38-05:00

Commit Message:
AGI: Rewrite logic decoding for clarity, OOB fixes

Changed paths:
    engines/agi/logic.cpp
    engines/agi/logic.h


diff --git a/engines/agi/logic.cpp b/engines/agi/logic.cpp
index dfcd261e823..04b6b018d55 100644
--- a/engines/agi/logic.cpp
+++ b/engines/agi/logic.cpp
@@ -27,82 +27,89 @@ namespace Agi {
  * Decode logic resource
  * This function decodes messages from the specified raw logic resource
  * into a message list.
- * @param n  The number of the logic resource to decode.
+ * @param logicNr  The number of the logic resource to decode.
  */
 int AgiEngine::decodeLogic(int16 logicNr) {
-	int ec = errOK;
-	int mstart, mend, mc;
-	uint8 *m0;
-	AgiLogic *curLogic = &_game.logics[logicNr];
-
-	// decrypt messages at end of logic + build message list
-
-	// report ("decoding logic #%d\n", n);
-	m0 = curLogic->data;
-
-	mstart = READ_LE_UINT16(m0) + 2;
-	mc = *(m0 + mstart);
-	mend = READ_LE_UINT16(m0 + mstart + 1);
-	m0 += mstart + 3;   // cover header info
-	mstart = mc << 1;
-
-	// if the logic was not compressed, decrypt the text messages
-	// only if there are more than 0 messages
-	if ((~_game.dirLogic[logicNr].flags & RES_COMPRESSED) && mc > 0)
-		decrypt(m0 + mstart, mend - mstart);    // decrypt messages
-
-	// build message list
-	m0 = curLogic->data;
-	mstart = READ_LE_UINT16(m0) + 2;    // +2 covers pointer
-	_game.logics[logicNr].numTexts = *(m0 + mstart);
-
-	// resetp logic pointers
-	curLogic->sIP = 2;
-	curLogic->cIP = 2;
-	curLogic->size = READ_LE_UINT16(m0) + 2;    // logic end pointer
-
-	// allocate list of pointers to point into our data
-
-	curLogic->texts = (const char **)calloc(1 + curLogic->numTexts, sizeof(char *));
+	AgiLogic &logic = _game.logics[logicNr];
+	AgiDir &dirLogic = _game.dirLogic[logicNr];
+
+	// bytecode section:
+	// u16  bytecode size
+	// u8[] bytecode
+	uint16 bytecodeSize = READ_LE_UINT16(logic.data);
+
+	// message section:
+	// u8       message count
+	// u16      messages size (2 + offsets + strings)
+	// u16[]    string offsets (relative to message section + 1)
+	// string[] strings (null terminated, possibly encrypted)
+	int messageSectionPos = 2 + bytecodeSize;
+	uint8 messageCount = logic.data[messageSectionPos];
+	uint16 messagesSize = READ_LE_UINT16(logic.data + messageSectionPos + 1);
+	int stringOffsetsPos = messageSectionPos + 3;
+	int stringsPos = stringOffsetsPos + (2 * messageCount);
+	int stringsSize = messagesSize - 2 - (2 * messageCount);
+
+	// decrypt the message strings if the logic was not compressed
+	// and the logic has messages.
+	if ((~dirLogic.flags & RES_COMPRESSED) && messageCount > 0) {
+		decrypt(logic.data + stringsPos, stringsSize);
+	}
 
-	// cover header info
-	m0 += mstart + 3;
+	// reset logic pointers
+	logic.sIP = 2;
+	logic.cIP = 2;
+	logic.size = messageSectionPos; // exclude messages from logic size
+
+	// allocate list of pointers to message texts. last entry is null.
+	logic.numTexts = messageCount;
+	logic.texts = (const char **)calloc(1 + logic.numTexts, sizeof(char *));
+	if (logic.texts == nullptr) {
+		free(logic.data);
+		logic.data = nullptr;
+		logic.numTexts = 0;
+		return errNotEnoughMemory;
+	}
 
-	if (curLogic->texts != nullptr) {
-		// move list of strings into list to make real pointers
-		for (mc = 0; mc < curLogic->numTexts; mc++) {
-			mend = READ_LE_UINT16(m0 + mc * 2);
-			_game.logics[logicNr].texts[mc] = mend ? (const char *)m0 + mend - 2 : (const char *)"";
+	// populate list of pointers to message texts
+	for (int i = 0; i < messageCount; i++) {
+		int stringOffset = READ_LE_UINT16(logic.data + stringOffsetsPos + (i * 2));
+		if (stringOffset != 0) {
+			// offset is relative to the message section + 1
+			stringOffset += messageSectionPos + 1;
+			logic.texts[i] = (const char *)(logic.data + stringOffset);
+		} else {
+			// TODO: does this happen? when is a string offset zero?
+			logic.texts[i] = "";
 		}
-		// set loaded flag now its all completly loaded
-		_game.dirLogic[logicNr].flags |= RES_LOADED;
-	} else {
-		// unload data
-		// Note that not every logic has text
-		free(curLogic->data);
-		ec = errNotEnoughMemory;
 	}
 
-	return ec;
+	// set loaded flag
+	dirLogic.flags |= RES_LOADED;
+	return errOK;
 }
 
 /**
  * Unload logic resource
  * This function unloads the specified logic resource, freeing any
  * memory chunks allocated for this resource.
- * @param n  The number of the logic resource to unload
+ * @param logicNr  The number of the logic resource to unload
  */
 void AgiEngine::unloadLogic(int16 logicNr) {
-	if (_game.dirLogic[logicNr].flags & RES_LOADED) {
-		free(_game.logics[logicNr].data);
-		free(_game.logics[logicNr].texts);
-		_game.logics[logicNr].numTexts = 0;
-		_game.dirLogic[logicNr].flags &= ~RES_LOADED;
+	AgiLogic &logic = _game.logics[logicNr];
+	AgiDir &dirLogic = _game.dirLogic[logicNr];
+
+	if (dirLogic.flags & RES_LOADED) {
+		free(logic.data);
+		logic.data = nullptr;
+		free(logic.texts);
+		logic.texts = nullptr;
+		logic.numTexts = 0;
+		dirLogic.flags &= ~RES_LOADED;
 	}
 
-	// if cached, we end up here
-	_game.logics[logicNr].sIP = 2;
-	_game.logics[logicNr].cIP = 2;
+	logic.sIP = 2;
+	logic.cIP = 2;
 }
 
 } // End of namespace Agi
diff --git a/engines/agi/logic.h b/engines/agi/logic.h
index f24ade0349a..11ed02a7b79 100644
--- a/engines/agi/logic.h
+++ b/engines/agi/logic.h
@@ -29,7 +29,7 @@ namespace Agi {
  */
 struct AgiLogic {
 	uint8 *data;        /**< raw resource data */
-	int size;           /**< size of data */
+	int size;           /**< size of data (excluding message section) */
 	int sIP;            /**< saved IP */
 	int cIP;            /**< current IP */
 	int numTexts;       /**< number of messages */


Commit: b21288d43e58978ed1e60c59672715ba66433710
    https://github.com/scummvm/scummvm/commit/b21288d43e58978ed1e60c59672715ba66433710
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:38:44-05:00

Commit Message:
AGI: Cleanup loader code

Most of these changes resolve inconsistencies between the three loaders.
Formatting, error handling, logging, etc.

V2 and V3 parsers no longer assume that directory files are multiples
of three. Some Mac games have padding. This caused uninitialized memory
to be parsed and stored in the directory table.

Cherry picked and adapted from be698249f1650a2b9632e39b8ccbfe03d8ab6e70

Changed paths:
    engines/agi/loader_v1.cpp
    engines/agi/loader_v2.cpp
    engines/agi/loader_v3.cpp


diff --git a/engines/agi/loader_v1.cpp b/engines/agi/loader_v1.cpp
index b55d78b3f9d..870eeb3b3e8 100644
--- a/engines/agi/loader_v1.cpp
+++ b/engines/agi/loader_v1.cpp
@@ -69,7 +69,7 @@ int AgiLoader_v1::loadDir_DDP(AgiDir *agid, int offset, int max) {
 	if (!fp.open(_filenameDisk0))
 		return errBadFileOpen;
 
-	// Cleanup
+	// initialize directory entries to empty
 	for (int i = 0; i < MAX_DIRECTORY_ENTRIES; i++) {
 		agid[i].volume = 0xFF;
 		agid[i].offset = _EMPTY;
@@ -92,8 +92,6 @@ int AgiLoader_v1::loadDir_DDP(AgiDir *agid, int offset, int max) {
 		}
 	}
 
-	fp.close();
-
 	return errOK;
 }
 
@@ -103,7 +101,7 @@ int AgiLoader_v1::loadDir_BC(AgiDir *agid, int offset, int max) {
 	if (!fp.open(_filenameDisk0))
 		return errBadFileOpen;
 
-	// Cleanup
+	// initialize directory entries to empty
 	for (int i = 0; i < MAX_DIRECTORY_ENTRIES; i++) {
 		agid[i].volume = 0xFF;
 		agid[i].offset = _EMPTY;
@@ -127,8 +125,6 @@ int AgiLoader_v1::loadDir_BC(AgiDir *agid, int offset, int max) {
 		}
 	}
 
-	fp.close();
-
 	return errOK;
 }
 
@@ -165,7 +161,6 @@ int AgiLoader_v1::init() {
 }
 
 uint8 *AgiLoader_v1::loadVolRes(struct AgiDir *agid) {
-	uint8 *data = nullptr;
 	Common::File fp;
 	int offset = agid->offset;
 
@@ -173,27 +168,31 @@ uint8 *AgiLoader_v1::loadVolRes(struct AgiDir *agid) {
 		return nullptr;
 
 	if (offset > IMAGE_SIZE) {
-		fp.open(_filenameDisk1);
+		if (!fp.open(_filenameDisk1)) {
+			warning("AgiLoader_v1::loadVolRes: could not open %s", _filenameDisk1.c_str());
+			return nullptr;
+		}
 		offset -= IMAGE_SIZE;
 	} else {
-		fp.open(_filenameDisk0);
+		if (!fp.open(_filenameDisk0)) {
+			warning("AgiLoader_v1::loadVolRes: could not open %s", _filenameDisk0.c_str());
+			return nullptr;
+		}
 	}
 
 	fp.seek(offset, SEEK_SET);
 
-	int signature = fp.readUint16BE();
+	uint16 signature = fp.readUint16BE();
 	if (signature != 0x1234) {
 		warning("AgiLoader_v1::loadVolRes: bad signature %04x", signature);
 		return nullptr;
 	}
 
-	fp.readByte();
+	fp.readByte(); // volume number
 	agid->len = fp.readUint16LE();
-	data = (uint8 *)calloc(1, agid->len + 32);
+	uint8 *data = (uint8 *)calloc(1, agid->len + 32); // why the extra 32 bytes?
 	fp.read(data, agid->len);
 
-	fp.close();
-
 	return data;
 }
 
@@ -220,9 +219,7 @@ int AgiLoader_v1::loadResource(int16 resourceType, int16 resourceNr) {
 			_vm->_game.logics[resourceNr].sIP = 2;
 		}
 
-		// if logic was cached, we get here
-		// reset code pointers incase it was cached
-
+		// reset code pointer in case logic was cached
 		_vm->_game.logics[resourceNr].cIP = _vm->_game.logics[resourceNr].sIP;
 		break;
 	case RESOURCETYPE_PICTURE:
diff --git a/engines/agi/loader_v2.cpp b/engines/agi/loader_v2.cpp
index 1a96ed10777..2407dfbe00b 100644
--- a/engines/agi/loader_v2.cpp
+++ b/engines/agi/loader_v2.cpp
@@ -28,9 +28,9 @@ namespace Agi {
 
 int AgiLoader_v2::detectGame() {
 	if (!Common::File::exists(LOGDIR) ||
-	        !Common::File::exists(PICDIR) ||
-	        !Common::File::exists(SNDDIR) ||
-	        !Common::File::exists(VIEWDIR))
+	    !Common::File::exists(PICDIR) ||
+	    !Common::File::exists(SNDDIR) ||
+	    !Common::File::exists(VIEWDIR))
 		return errInvalidAGIFile;
 
 	// Should this go above the previous lines, so we can force emulation versions
@@ -42,44 +42,38 @@ int AgiLoader_v2::detectGame() {
 }
 
 int AgiLoader_v2::loadDir(AgiDir *agid, const char *fname) {
-	Common::File fp;
-	uint8 *mem;
-	uint32 flen;
-	uint i;
-
 	debug(0, "Loading directory: %s", fname);
 
+	Common::File fp;
 	if (!fp.open(fname)) {
 		return errBadFileOpen;
 	}
 
 	fp.seek(0, SEEK_END);
-	flen = fp.pos();
+	uint32 flen = fp.pos();
 	fp.seek(0, SEEK_SET);
 
-	if ((mem = (uint8 *)malloc(flen + 32)) == nullptr) {
-		fp.close();
+	uint8 *mem = (uint8 *)malloc(flen);
+	if (mem == nullptr) {
 		return errNotEnoughMemory;
 	}
 
 	fp.read(mem, flen);
 
-	// set all directory resources to gone
-	for (i = 0; i < MAX_DIRECTORY_ENTRIES; i++) {
+	// initialize directory entries to empty
+	for (int i = 0; i < MAX_DIRECTORY_ENTRIES; i++) {
 		agid[i].volume = 0xff;
 		agid[i].offset = _EMPTY;
 	}
 
-	// build directory entries
-	for (i = 0; i < flen; i += 3) {
+	// read directory entries
+	for (uint32 i = 0; i + 2 < flen; i += 3) {
 		agid[i / 3].volume = *(mem + i) >> 4;
 		agid[i / 3].offset = READ_BE_UINT24(mem + i) & (uint32) _EMPTY;
 		debugC(3, kDebugLevelResources, "%d: volume %d, offset 0x%05x", i / 3, agid[i / 3].volume, agid[i / 3].offset);
 	}
 
 	free(mem);
-	fp.close();
-
 	return errOK;
 }
 
@@ -173,7 +167,6 @@ uint8 *AgiLoader_v2::loadVolRes(struct AgiDir *agid) {
 	uint8 *data = nullptr;
 	uint8 volumeHeader[7];
 	Common::File fp;
-	unsigned int sig;
 	Common::String path;
 
 	path = Common::String::format("vol.%i", agid->volume);
@@ -183,19 +176,19 @@ uint8 *AgiLoader_v2::loadVolRes(struct AgiDir *agid) {
 		debugC(3, kDebugLevelResources, "loading resource at offset %d", agid->offset);
 		fp.seek(agid->offset, SEEK_SET);
 		fp.read(&volumeHeader, _hasV3VolumeFormat ? 7 : 5);
-		if ((sig = READ_BE_UINT16(volumeHeader)) == 0x1234) {
+		uint16 signature = READ_BE_UINT16(volumeHeader);
+		if (signature == 0x1234) {
 			agid->len = READ_LE_UINT16(volumeHeader + 3);
-			data = (uint8 *)calloc(1, agid->len + 32);
+			data = (uint8 *)calloc(1, agid->len + 32); // why the extra 32 bytes?
 			if (data != nullptr) {
 				fp.read(data, agid->len);
 			} else {
 				error("AgiLoader_v2::loadVolRes out of memory");
 			}
 		} else {
-			warning("AgiLoader_v2::loadVolRes: bad signature %04x", sig);
+			warning("AgiLoader_v2::loadVolRes: bad signature %04x", signature);
 			return nullptr;
 		}
-		fp.close();
 	} else {
 		// we have a bad volume resource
 		// set that resource to NA
diff --git a/engines/agi/loader_v3.cpp b/engines/agi/loader_v3.cpp
index d7f418ab767..5cee09d5fb7 100644
--- a/engines/agi/loader_v3.cpp
+++ b/engines/agi/loader_v3.cpp
@@ -37,12 +37,11 @@ int AgiLoader_v3::detectGame() {
 	Common::FSNode dir(ConfMan.get("path"));
 
 	if (!dir.getChildren(fslist, Common::FSNode::kListFilesOnly)) {
-		warning("AgiEngine: invalid game path '%s'", dir.getPath().c_str());
+		warning("AgiLoader_v3: invalid game path '%s'", dir.getPath().c_str());
 		return errInvalidAGIFile;
 	}
 
-	for (Common::FSList::const_iterator file = fslist.begin();
-	        file != fslist.end() && !found; ++file) {
+	for (Common::FSList::const_iterator file = fslist.begin(); file != fslist.end() && !found; ++file) {
 		Common::String f = file->getName();
 		f.toLowercase();
 
@@ -58,53 +57,42 @@ int AgiLoader_v3::detectGame() {
 	}
 
 	if (!found) {
-		debugC(3, kDebugLevelMain, "not found");
+		debugC(3, kDebugLevelMain, "directory file not found");
 		ec = errInvalidAGIFile;
 	}
 
 	return ec;
 }
 
-int AgiLoader_v3::loadDir(struct AgiDir *agid, Common::File *fp,
-						  uint32 offs, uint32 len) {
-	int ec = errOK;
-	uint8 *mem;
-	unsigned int i;
-
+int AgiLoader_v3::loadDir(AgiDir *agid, Common::File *fp, uint32 offs, uint32 len) {
 	fp->seek(offs, SEEK_SET);
-	if ((mem = (uint8 *)malloc(len + 32)) != nullptr) {
-		fp->read(mem, len);
-
-		// set all directory resources to gone
-		for (i = 0; i < MAX_DIRECTORY_ENTRIES; i++) {
-			agid[i].volume = 0xff;
-			agid[i].offset = _EMPTY;
-		}
+	uint8 *mem = (uint8 *)malloc(len);
+	if (mem == nullptr) {
+		return errNotEnoughMemory;
+	}
 
-		// build directory entries
-		for (i = 0; i < len; i += 3) {
-			agid[i / 3].volume = *(mem + i) >> 4;
-			agid[i / 3].offset = READ_BE_UINT24(mem + i) & (uint32) _EMPTY;
-		}
+	fp->read(mem, len);
 
-		free(mem);
-	} else {
-		ec = errNotEnoughMemory;
+	// initialize directory entries to empty
+	for (int i = 0; i < MAX_DIRECTORY_ENTRIES; i++) {
+		agid[i].volume = 0xff;
+		agid[i].offset = _EMPTY;
 	}
 
-	return ec;
-}
+	// read directory entries
+	for (uint32 i = 0; i + 2 < len; i += 3) {
+		agid[i / 3].volume = *(mem + i) >> 4;
+		agid[i / 3].offset = READ_BE_UINT24(mem + i) & (uint32) _EMPTY;
+		debugC(3, kDebugLevelResources, "%d: volume %d, offset 0x%05x", i / 3, agid[i / 3].volume, agid[i / 3].offset);
+	}	
 
-struct agi3vol {
-	uint32 sddr;
-	uint32 len;
-};
+	free(mem);
+	return errOK;
+}
 
 int AgiLoader_v3::init() {
 	int ec = errOK;
-	struct agi3vol agiVol3[4];
-	int i;
-	uint16 xd[4];
+	uint8 fileHeader[8];
 	Common::File fp;
 	Common::String path;
 
@@ -120,36 +108,32 @@ int AgiLoader_v3::init() {
 		return errBadFileOpen;
 	}
 	// build offset table for v3 directory format
-	fp.read(&xd, 8);
+	fp.read(&fileHeader, 8);
 	fp.seek(0, SEEK_END);
 
-	for (i = 0; i < 4; i++)
-		agiVol3[i].sddr = READ_LE_UINT16((uint8 *) & xd[i]);
+	uint16 dirOffsets[4];
+	for (int i = 0; i < 4; i++)
+		dirOffsets[i] = READ_LE_UINT16(&fileHeader[i * 2]);
 
-	agiVol3[0].len = agiVol3[1].sddr - agiVol3[0].sddr;
-	agiVol3[1].len = agiVol3[2].sddr - agiVol3[1].sddr;
-	agiVol3[2].len = agiVol3[3].sddr - agiVol3[2].sddr;
-	agiVol3[3].len = fp.pos() - agiVol3[3].sddr;
+	uint32 dirLengths[4];
+	dirLengths[0] = dirOffsets[1] - dirOffsets[0];
+	dirLengths[1] = dirOffsets[2] - dirOffsets[1];
+	dirLengths[2] = dirOffsets[3] - dirOffsets[2];
+	dirLengths[3] = fp.pos() - dirOffsets[3];
 
-	if (agiVol3[3].len > 256 * 3)
-		agiVol3[3].len = 256 * 3;
+	if (dirLengths[3] > 256 * 3)
+		dirLengths[3] = 256 * 3;
 
 	fp.seek(0, SEEK_SET);
 
 	// read in directory files
-	ec = loadDir(_vm->_game.dirLogic, &fp, agiVol3[0].sddr, agiVol3[0].len);
-
-	if (ec == errOK) {
-		ec = loadDir(_vm->_game.dirPic, &fp, agiVol3[1].sddr, agiVol3[1].len);
-	}
-
-	if (ec == errOK) {
-		ec = loadDir(_vm->_game.dirView, &fp, agiVol3[2].sddr, agiVol3[2].len);
-	}
-
-	if (ec == errOK) {
-		ec = loadDir(_vm->_game.dirSound, &fp, agiVol3[3].sddr, agiVol3[3].len);
-	}
+	ec = loadDir(_vm->_game.dirLogic, &fp, dirOffsets[0], dirLengths[0]);
+	if (ec == errOK)
+		ec = loadDir(_vm->_game.dirPic, &fp, dirOffsets[1], dirLengths[1]);
+	if (ec == errOK)
+		ec = loadDir(_vm->_game.dirView, &fp, dirOffsets[2], dirLengths[2]);
+	if (ec == errOK)
+		ec = loadDir(_vm->_game.dirSound, &fp, dirOffsets[3], dirLengths[3]);
 
 	return ec;
 }
@@ -181,8 +165,8 @@ void AgiLoader_v3::unloadResource(int16 resourceType, int16 resourceNr) {
  * NULL is returned if unsuccessful.
  */
 uint8 *AgiLoader_v3::loadVolRes(AgiDir *agid) {
-	char x[8];
-	uint8 *data = nullptr, *compBuffer;
+	uint8 volumeHeader[7];
+	uint8 *data = nullptr;
 	Common::File fp;
 	Common::String path;
 
@@ -195,23 +179,21 @@ uint8 *AgiLoader_v3::loadVolRes(AgiDir *agid) {
 
 	if (agid->offset != _EMPTY && fp.open(path)) {
 		fp.seek(agid->offset, SEEK_SET);
-		fp.read(&x, 7);
-
-		if (READ_BE_UINT16((uint8 *) x) != 0x1234) {
-			debugC(3, kDebugLevelResources, "path = %s", path.c_str());
-			debugC(3, kDebugLevelResources, "offset = %d", agid->offset);
-			debugC(3, kDebugLevelResources, "x = %x %x", x[0], x[1]);
-			error("ACK! BAD RESOURCE");
-			_vm->quitGame();    // for compilers that don't support NORETURN
+		fp.read(&volumeHeader, 7);
+
+		uint16 signature = READ_BE_UINT16(volumeHeader);
+		if (signature != 0x1234) {
+			warning("AgiLoader_v3::loadVolRes: bad signature %04x", signature);
+			return nullptr;
 		}
 
-		agid->len = READ_LE_UINT16((uint8 *) x + 3);    // uncompressed size
-		agid->clen = READ_LE_UINT16((uint8 *) x + 5);   // compressed len
+		agid->len = READ_LE_UINT16(volumeHeader + 3);    // uncompressed size
+		agid->clen = READ_LE_UINT16(volumeHeader + 5);   // compressed len
 
-		compBuffer = (uint8 *)calloc(1, agid->clen + 32);
+		uint8 *compBuffer = (uint8 *)calloc(1, agid->clen + 32); // why the extra 32 bytes?
 		fp.read(compBuffer, agid->clen);
 
-		if (x[2] & 0x80) { // compressed pic
+		if (volumeHeader[2] & 0x80) { // compressed pic
 			// effectively uncompressed, but having only 4-bit parameters for F0 / F2 commands
 			// Manhunter 2 uses such pictures
 			data = compBuffer;
@@ -221,13 +203,11 @@ uint8 *AgiLoader_v3::loadVolRes(AgiDir *agid) {
 			data = compBuffer;
 		} else {
 			// it is compressed
-			data = (uint8 *)calloc(1, agid->len + 32);
+			data = (uint8 *)calloc(1, agid->len + 32); // why the extra 32 bytes?
 			lzwExpand(compBuffer, data, agid->len);
 			free(compBuffer);
 			agid->flags |= RES_COMPRESSED;
 		}
-
-		fp.close();
 	} else {
 		// we have a bad volume resource
 		// set that resource to NA
@@ -262,23 +242,16 @@ int AgiLoader_v3::loadResource(int16 resourceType, int16 resourceNr) {
 
 			// uncompressed logic files need to be decrypted
 			if (data != nullptr) {
-				// resloaded flag gets set by decode logic
+				// RES_LOADED flag gets set by decode logic
 				// needed to build string table
 				ec = _vm->decodeLogic(resourceNr);
 				_vm->_game.logics[resourceNr].sIP = 2;
 			} else {
 				ec = errBadResource;
 			}
-
-			// logics[n].sIP=2; // saved IP = 2
-			// logics[n].cIP=2; // current IP = 2
-
-			_vm->_game.logics[resourceNr].cIP = _vm->_game.logics[resourceNr].sIP;
 		}
 
-		// if logic was cached, we get here
-		// reset code pointers incase it was cached
-
+		// reset code pointers in case logic was cached
 		_vm->_game.logics[resourceNr].cIP = _vm->_game.logics[resourceNr].sIP;
 		break;
 	case RESOURCETYPE_PICTURE:


Commit: 9f4049a678909dbdf136873efd8d6f3230f92fd8
    https://github.com/scummvm/scummvm/commit/9f4049a678909dbdf136873efd8d6f3230f92fd8
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:39:59-05:00

Commit Message:
AGI: Cleanup graphics code

Changed paths:
    engines/agi/console.cpp
    engines/agi/cycle.cpp
    engines/agi/graphics.cpp
    engines/agi/graphics.h
    engines/agi/op_cmd.cpp
    engines/agi/opcodes.cpp
    engines/agi/view.cpp
    engines/agi/view.h


diff --git a/engines/agi/console.cpp b/engines/agi/console.cpp
index cdbbd0fd50e..7d4619759ce 100644
--- a/engines/agi/console.cpp
+++ b/engines/agi/console.cpp
@@ -488,7 +488,7 @@ bool Console::Cmd_ScreenObj(int argc, const char **argv) {
 			flagsString += "UpdatePos ";
 		if (screenObj->flags & fOnLand)
 			flagsString += "OnLand ";
-		if (screenObj->flags & fDontupdate)
+		if (screenObj->flags & fDontUpdate)
 			flagsString += "DontUpdate ";
 		if (screenObj->flags & fFixLoop)
 			flagsString += "FixLoop ";
diff --git a/engines/agi/cycle.cpp b/engines/agi/cycle.cpp
index 43d2f48c91c..f8927c404b4 100644
--- a/engines/agi/cycle.cpp
+++ b/engines/agi/cycle.cpp
@@ -39,9 +39,7 @@ namespace Agi {
  * @param n room number
  */
 void AgiEngine::newRoom(int16 newRoomNr) {
-	ScreenObjEntry *screenObj;
 	ScreenObjEntry *screenObjEgo = &_game.screenObjTable[SCREENOBJECTS_EGO_ENTRY];
-	int i;
 
 	// Loading trigger
 	artificialDelayTrigger_NewRoom(newRoomNr);
@@ -49,16 +47,16 @@ void AgiEngine::newRoom(int16 newRoomNr) {
 	debugC(4, kDebugLevelMain, "*** room %d ***", newRoomNr);
 	_sound->stopSound();
 
-	i = 0;
-	for (screenObj = _game.screenObjTable; screenObj < &_game.screenObjTable[SCREENOBJECTS_MAX]; screenObj++) {
-		screenObj->objectNr = i++;
-		screenObj->flags &= ~(fAnimated | fDrawn);
-		screenObj->flags |= fUpdate;
-		screenObj->stepTime = 1;
-		screenObj->stepTimeCount = 1;
-		screenObj->cycleTime = 1;
-		screenObj->cycleTimeCount = 1;
-		screenObj->stepSize = 1;
+	for (int i = 0; i < SCREENOBJECTS_MAX; i++) {
+		ScreenObjEntry &screenObj = _game.screenObjTable[i];
+		screenObj.objectNr = i;
+		screenObj.flags &= ~(fAnimated | fDrawn);
+		screenObj.flags |= fUpdate;
+		screenObj.stepTime = 1;
+		screenObj.stepTimeCount = 1;
+		screenObj.cycleTime = 1;
+		screenObj.cycleTimeCount = 1;
+		screenObj.stepSize = 1;
 	}
 	agiUnloadResources();
 
@@ -121,17 +119,13 @@ void AgiEngine::newRoom(int16 newRoomNr) {
 }
 
 void AgiEngine::resetControllers() {
-	int i;
-
-	for (i = 0; i < MAX_CONTROLLERS; i++) {
+	for (int i = 0; i < MAX_CONTROLLERS; i++) {
 		_game.controllerOccurred[i] = false;
 	}
 }
 
 void AgiEngine::interpretCycle() {
 	ScreenObjEntry *screenObjEgo = &_game.screenObjTable[SCREENOBJECTS_EGO_ENTRY];
-	bool oldSound;
-	byte oldScore;
 
 	if (!_game.playerControl)
 		setVar(VM_VAR_EGO_DIRECTION, screenObjEgo->direction);
@@ -140,8 +134,8 @@ void AgiEngine::interpretCycle() {
 
 	checkAllMotions();
 
-	oldScore = getVar(VM_VAR_SCORE);
-	oldSound = getFlag(VM_FLAG_SOUND_ON);
+	byte oldScore = getVar(VM_VAR_SCORE);
+	bool oldSound = getFlag(VM_FLAG_SOUND_ON);
 
 	// Reset script heuristic here
 	resetGetVarSecondsHeuristic();
@@ -182,11 +176,10 @@ void AgiEngine::interpretCycle() {
 
 // We return the current key, or 0 if no key was pressed
 uint16 AgiEngine::processAGIEvents() {
-	uint16 key;
 	ScreenObjEntry *screenObjEgo = &_game.screenObjTable[SCREENOBJECTS_EGO_ENTRY];
 
 	wait(10);
-	key = doPollKeyboard();
+	uint16 key = doPollKeyboard();
 
 	// In AGI Mouse emulation mode we must update the mouse-related
 	// vars in every interpreter cycle.
diff --git a/engines/agi/graphics.cpp b/engines/agi/graphics.cpp
index ad4b373ac6a..3cabdc63ef5 100644
--- a/engines/agi/graphics.cpp
+++ b/engines/agi/graphics.cpp
@@ -72,10 +72,8 @@ GfxMgr::GfxMgr(AgiBase *vm, GfxFont *font) : _vm(vm), _font(font) {
 
 /**
  * Initialize graphics device.
- *
- * @see deinit_video()
  */
-int GfxMgr::initVideo() {
+void GfxMgr::initVideo() {
 	bool forceHires = false;
 
 	// Set up palettes
@@ -148,12 +146,10 @@ int GfxMgr::initVideo() {
 		}
 		break;
 	default:
-		error("initVideo: unsupported render mode");
+		error("initVideo: unsupported render mode: %d", _vm->_renderMode);
 		break;
 	}
 
-	//bool forcedUpscale = true;
-
 	if (_font->isFontHires() || forceHires) {
 		// Upscaling enable
 		_upscaledHires = DISPLAY_UPSCALED_640x400;
@@ -196,7 +192,7 @@ int GfxMgr::initVideo() {
 		initMouseCursor(&_mouseCursorBusy, MOUSECURSOR_MACINTOSH_BUSY, 10, 14, 7, 8);
 		break;
 	default:
-		error("initVideo: unsupported render mode");
+		error("initVideo: unsupported render mode: %d", _vm->_renderMode);
 		break;
 	}
 
@@ -216,27 +212,19 @@ int GfxMgr::initVideo() {
 	// set up mouse cursor palette
 	CursorMan.replaceCursorPalette(MOUSECURSOR_PALETTE, 1, ARRAYSIZE(MOUSECURSOR_PALETTE) / 3);
 	setMouseCursor();
-
-	return errOK;
 }
 
 /**
  * Deinitialize graphics device.
- *
- * @see init_video()
  */
-int GfxMgr::deinitVideo() {
+void GfxMgr::deinitVideo() {
 	// Free mouse cursors in case they were allocated
-	if (_mouseCursor.bitmapDataAllocated)
-		free(_mouseCursor.bitmapDataAllocated);
-	if (_mouseCursorBusy.bitmapDataAllocated)
-		free(_mouseCursorBusy.bitmapDataAllocated);
+	free(_mouseCursor.bitmapDataAllocated);
+	free(_mouseCursorBusy.bitmapDataAllocated);
 
 	free(_displayScreen);
 	free(_gameScreen);
 	free(_priorityScreen);
-
-	return errOK;
 }
 
 void GfxMgr::setRenderStartOffset(uint16 offsetY) {
@@ -528,14 +516,14 @@ void GfxMgr::render_Block(int16 x, int16 y, int16 width, int16 height, bool copy
 	switch (_vm->_renderMode) {
 	case Common::kRenderHercG:
 	case Common::kRenderHercA:
-		render_BlockHercules(x, y, width, height, copyToScreen);
+		render_BlockHercules(x, y, width, height);
 		break;
 	case Common::kRenderCGA:
-		render_BlockCGA(x, y, width, height, copyToScreen);
+		render_BlockCGA(x, y, width, height);
 		break;
 	case Common::kRenderEGA:
 	default:
-		render_BlockEGA(x, y, width, height, copyToScreen);
+		render_BlockEGA(x, y, width, height);
 		break;
 	}
 
@@ -580,7 +568,7 @@ bool GfxMgr::render_Clip(int16 &x, int16 &y, int16 &width, int16 &height, int16
 	return true;
 }
 
-void GfxMgr::render_BlockEGA(int16 x, int16 y, int16 width, int16 height, bool copyToScreen) {
+void GfxMgr::render_BlockEGA(int16 x, int16 y, int16 width, int16 height) {
 	uint32 offsetVisual = SCRIPT_WIDTH * y + x;
 	uint32 offsetDisplay = getDisplayOffsetToGameScreenPos(x, y);
 	int16 remainingWidth = width;
@@ -629,7 +617,7 @@ void GfxMgr::render_BlockEGA(int16 x, int16 y, int16 width, int16 height, bool c
 	}
 }
 
-void GfxMgr::render_BlockCGA(int16 x, int16 y, int16 width, int16 height, bool copyToScreen) {
+void GfxMgr::render_BlockCGA(int16 x, int16 y, int16 width, int16 height) {
 	uint32 offsetVisual = SCRIPT_WIDTH * y + x;
 	uint32 offsetDisplay = getDisplayOffsetToGameScreenPos(x, y);
 	int16 remainingWidth = width;
@@ -704,7 +692,7 @@ static const uint8 herculesColorMapping[] = {
 };
 
 // Sierra actually seems to have rendered the whole screen all the time
-void GfxMgr::render_BlockHercules(int16 x, int16 y, int16 width, int16 height, bool copyToScreen) {
+void GfxMgr::render_BlockHercules(int16 x, int16 y, int16 width, int16 height) {
 	uint32 offsetVisual = SCRIPT_WIDTH * y + x;
 	uint32 offsetDisplay = getDisplayOffsetToGameScreenPos(x, y);
 	int16 remainingWidth = width;
@@ -1197,15 +1185,14 @@ void GfxMgr::drawCharacterOnDisplay(int16 x, int16 y, const byte character, byte
 // - Fanmade "Enclosure" right during the intro
 // - Space Quest 2 almost right at the start when getting captured (after walking into the space ship)
 void GfxMgr::shakeScreen(int16 repeatCount) {
-	int shakeNr, shakeCount;
 	int16 shakeHorizontalPixels = SHAKE_HORIZONTAL_PIXELS * (2 + _displayWidthMulAdjust);
 	int16 shakeVerticalPixels = SHAKE_VERTICAL_PIXELS * (1 + _displayHeightMulAdjust);
 
-	shakeCount = repeatCount * 8; // effectively 4 shakes per repeat
+	int shakeCount = repeatCount * 8; // effectively 4 shakes per repeat
 
 	// it's 4 pixels down and 8 pixels to the right
 	// and it's also filling the remaining space with black
-	for (shakeNr = 0; shakeNr < shakeCount; shakeNr++) {
+	for (int shakeNr = 0; shakeNr < shakeCount; shakeNr++) {
 		if (shakeNr & 1) {
 			// move back
 			g_system->setShakePos(0, 0);
@@ -1228,24 +1215,21 @@ void GfxMgr::initPriorityTable() {
 }
 
 void GfxMgr::createDefaultPriorityTable(uint8 *priorityTable) {
-	int16 priority, step;
 	int16 yPos = 0;
 
-	for (priority = 1; priority < 15; priority++) {
-		for (step = 0; step < 12; step++) {
+	for (int16 priority = 1; priority < 15; priority++) {
+		for (int16 step = 0; step < 12; step++) {
 			priorityTable[yPos++] = priority < 4 ? 4 : priority;
 		}
 	}
 }
 
 void GfxMgr::setPriorityTable(int16 priorityBase) {
-	int16 x, priorityY, priority;
-
 	_priorityTableSet = true;
-	x = (SCRIPT_HEIGHT - priorityBase) * SCRIPT_HEIGHT / 10;
+	int16 x = (SCRIPT_HEIGHT - priorityBase) * SCRIPT_HEIGHT / 10;
 
-	for (priorityY = 0; priorityY < SCRIPT_HEIGHT; priorityY++) {
-		priority = (priorityY - priorityBase) < 0 ? 4 : (priorityY - priorityBase) * SCRIPT_HEIGHT / x + 5;
+	for (int16 priorityY = 0; priorityY < SCRIPT_HEIGHT; priorityY++) {
+		int16 priority = (priorityY - priorityBase) < 0 ? 4 : (priorityY - priorityBase) * SCRIPT_HEIGHT / x + 5;
 		if (priority > 15)
 			priority = 15;
 		_priorityTable[priorityY] = priority;
@@ -1286,8 +1270,6 @@ void GfxMgr::saveLoadFigureOutPriorityTableModifiedBool() {
  * Convert sprite priority to y value.
  */
 int16 GfxMgr::priorityToY(int16 priority) {
-	int16 currentY;
-
 	if (!_priorityTableSet) {
 		// priority table wasn't set by scripts? calculate directly
 		return (priority - 5) * 12 + 48;
@@ -1314,7 +1296,7 @@ int16 GfxMgr::priorityToY(int16 priority) {
 		return 168; // Buggy behavior, see above
 	}
 
-	currentY = 167;
+	int16 currentY = 167;
 	while (_priorityTable[currentY] >= priority) {
 		currentY--;
 		if (currentY < 0) // Original AGI didn't do this, we abort in that case and return -1
@@ -1467,17 +1449,9 @@ void GfxMgr::initMouseCursor(MouseCursorData *mouseCursor, const byte *bitmapDat
 }
 
 void GfxMgr::setMouseCursor(bool busy) {
-	MouseCursorData *mouseCursor = nullptr;
+	MouseCursorData &mouseCursor = busy ? _mouseCursorBusy : _mouseCursor;
 
-	if (!busy) {
-		mouseCursor = &_mouseCursor;
-	} else {
-		mouseCursor = &_mouseCursorBusy;
-	}
-
-	if (mouseCursor) {
-		CursorMan.replaceCursor(mouseCursor->bitmapData, mouseCursor->width, mouseCursor->height, mouseCursor->hotspotX, mouseCursor->hotspotY, 0);
-	}
+	CursorMan.replaceCursor(mouseCursor.bitmapData, mouseCursor.width, mouseCursor.height, mouseCursor.hotspotX, mouseCursor.hotspotY, 0);
 }
 
 #if 0
diff --git a/engines/agi/graphics.h b/engines/agi/graphics.h
index ab9464a2fe0..8ed36ce5ebf 100644
--- a/engines/agi/graphics.h
+++ b/engines/agi/graphics.h
@@ -38,7 +38,7 @@ enum GfxScreenUpscaledMode {
 	DISPLAY_UPSCALED_640x400  = 1
 };
 
-class AgiEngine;
+class AgiBase;
 
 enum GfxScreenMasks {
 	GFX_SCREEN_MASK_VISUAL      = 1,
@@ -69,8 +69,8 @@ private:
 public:
 	GfxMgr(AgiBase *vm, GfxFont *font);
 
-	int initVideo();
-	int deinitVideo();
+	void initVideo();
+	void deinitVideo();
 	void initPalette(uint8 *destPalette, const uint8 *paletteData, uint colorCount = 16, uint fromBits = 6, uint toBits = 8);
 	void initPaletteCLUT(uint8 *destPalette, const uint16 *paletteCLUTData, uint colorCount = 16);
 	void setAGIPal(int);
@@ -176,9 +176,9 @@ public:
 	bool render_Clip(int16 &x, int16 &y, int16 &width, int16 &height, int16 clipAgainstWidth = SCRIPT_WIDTH, int16 clipAgainstHeight = SCRIPT_HEIGHT);
 
 private:
-	void render_BlockEGA(int16 x, int16 y, int16 width, int16 height, bool copyToScreen);
-	void render_BlockCGA(int16 x, int16 y, int16 width, int16 height, bool copyToScreen);
-	void render_BlockHercules(int16 x, int16 y, int16 width, int16 height, bool copyToScreen);
+	void render_BlockEGA(int16 x, int16 y, int16 width, int16 height);
+	void render_BlockCGA(int16 x, int16 y, int16 width, int16 height);
+	void render_BlockHercules(int16 x, int16 y, int16 width, int16 height);
 
 public:
 	void transition_Amiga();
diff --git a/engines/agi/op_cmd.cpp b/engines/agi/op_cmd.cpp
index 555871b5284..1b95d654f37 100644
--- a/engines/agi/op_cmd.cpp
+++ b/engines/agi/op_cmd.cpp
@@ -487,7 +487,7 @@ void cmdSetCel(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 
 	vm->setCel(screenObj, celNr);
 	if (vm->getVersion() >= 0x2000) {
-		screenObj->flags &= ~fDontupdate;
+		screenObj->flags &= ~fDontUpdate;
 	}
 }
 
@@ -498,7 +498,7 @@ void cmdSetCelF(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	byte   value = vm->getVar(varNr);
 
 	vm->setCel(screenObj, value);
-	screenObj->flags &= ~fDontupdate;
+	screenObj->flags &= ~fDontUpdate;
 }
 
 void cmdSetView(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
@@ -1358,7 +1358,7 @@ void cmdDraw(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	sprites->buildRegularSpriteList();
 	sprites->drawRegularSpriteList();
 	sprites->showSprite(screenObj);
-	screenObj->flags &= ~fDontupdate;
+	screenObj->flags &= ~fDontUpdate;
 
 	debugC(4, kDebugLevelScripts, "vt entry #%d flags = %02x", objectNr, screenObj->flags);
 }
@@ -1553,7 +1553,7 @@ void cmdReverseLoop(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 
 	debugC(4, kDebugLevelScripts, "o%d, f%d", objectNr, loopFlag);
 	screenObj->cycle = kCycleRevLoop;
-	screenObj->flags |= (fDontupdate | fUpdate | fCycling);
+	screenObj->flags |= (fDontUpdate | fUpdate | fCycling);
 	screenObj->loop_flag = loopFlag;
 	state->_vm->setFlag(screenObj->loop_flag, false);
 
@@ -1568,7 +1568,7 @@ void cmdReverseLoopV1(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	debugC(4, kDebugLevelScripts, "o%d, f%d", objectNr, loopFlag);
 	screenObj->cycle = kCycleRevLoop;
 	state->_vm->setCel(screenObj, 0);
-	screenObj->flags |= (fDontupdate | fUpdate | fCycling);
+	screenObj->flags |= (fDontUpdate | fUpdate | fCycling);
 	screenObj->loop_flag = loopFlag;
 	//screenObj->parm3 = 0;
 }
@@ -1580,7 +1580,7 @@ void cmdEndOfLoop(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 
 	debugC(4, kDebugLevelScripts, "o%d, f%d", objectNr, loopFlag);
 	screenObj->cycle = kCycleEndOfLoop;
-	screenObj->flags |= (fDontupdate | fUpdate | fCycling);
+	screenObj->flags |= (fDontUpdate | fUpdate | fCycling);
 	screenObj->loop_flag = loopFlag;
 	vm->setFlag(screenObj->loop_flag, false);
 
@@ -1595,7 +1595,7 @@ void cmdEndOfLoopV1(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	debugC(4, kDebugLevelScripts, "o%d, f%d", objectNr, loopFlag);
 	screenObj->cycle = kCycleEndOfLoop;
 	state->_vm->setCel(screenObj, 0);
-	screenObj->flags |= (fDontupdate | fUpdate | fCycling);
+	screenObj->flags |= (fDontUpdate | fUpdate | fCycling);
 	screenObj->loop_flag = loopFlag;
 	//screenObj->parm3 = 0;
 }
@@ -1708,7 +1708,6 @@ void cmdMoveObj(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	uint16 stepSize = parameter[3];
 	uint16 moveFlag = parameter[4];
 	ScreenObjEntry *screenObj = &state->screenObjTable[objectNr];
-	// _D (_D_WARN "o=%d, x=%d, y=%d, s=%d, f=%d", p0, p1, p2, p3, p4);
 
 	screenObj->motionType = kMotionMoveObj;
 	screenObj->move_x = moveX;
@@ -1904,7 +1903,6 @@ void cmdStatus(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 
 void cmdQuit(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	uint16 withoutPrompt = parameter[0];
-//	const char *buttons[] = { "Quit", "Continue", NULL };
 
 	state->_vm->_sound->stopSound();
 	if (withoutPrompt) {
diff --git a/engines/agi/opcodes.cpp b/engines/agi/opcodes.cpp
index f391dcb2e5b..43c8a17b295 100644
--- a/engines/agi/opcodes.cpp
+++ b/engines/agi/opcodes.cpp
@@ -396,7 +396,7 @@ void AgiEngine::setupOpCodes(uint16 version) {
 
 	// Alter opcode parameters for specific games
 	if ((version >= 0x2000) && (version < 0x3000)) {
-		// AGI3 adjustments
+		// AGI2 adjustments
 
 		// 'quit' takes 0 args for 2.089
 		if (version == 0x2089)
diff --git a/engines/agi/view.cpp b/engines/agi/view.cpp
index 6e52e936cf6..9952b47ae34 100644
--- a/engines/agi/view.cpp
+++ b/engines/agi/view.cpp
@@ -28,8 +28,8 @@ namespace Agi {
 void AgiEngine::updateView(ScreenObjEntry *screenObj) {
 	int16 celNr, lastCelNr;
 
-	if (screenObj->flags & fDontupdate) {
-		screenObj->flags &= ~fDontupdate;
+	if (screenObj->flags & fDontUpdate) {
+		screenObj->flags &= ~fDontUpdate;
 		return;
 	}
 
@@ -385,7 +385,7 @@ void AgiEngine::unpackViewCelDataAGI256(AgiViewCel *celData, byte *compressedDat
 
 /**
  * Unloads all data in a view resource
- * @param n number of view resource
+ * @param viewNr number of view resource
  */
 void AgiEngine::unloadView(int16 viewNr) {
 	AgiView *viewData = &_game.views[viewNr];
@@ -580,9 +580,8 @@ void AgiEngine::clipViewCoordinates(ScreenObjEntry *screenObj) {
 	}
 
 	if (getVersion() < 0x2000) {
-		screenObj->flags |= fDontupdate;
+		screenObj->flags |= fDontUpdate;
 	}
-
 }
 
 /**
diff --git a/engines/agi/view.h b/engines/agi/view.h
index 7d73789ca01..26af4c9d85d 100644
--- a/engines/agi/view.h
+++ b/engines/agi/view.h
@@ -86,7 +86,7 @@ enum ViewFlags {
 	fIgnoreObjects  = (1 << 9),     // 0x0200
 	fUpdatePos      = (1 << 10),    // 0x0400
 	fOnLand         = (1 << 11),    // 0x0800
-	fDontupdate     = (1 << 12),    // 0x1000
+	fDontUpdate     = (1 << 12),    // 0x1000
 	fFixLoop        = (1 << 13),    // 0x2000
 	fDidntMove      = (1 << 14),    // 0x4000
 	fAdjEgoXY       = (1 << 15)     // 0x8000


Commit: f17b8127105fab06b3f8ec838e544ad03e9deafb
    https://github.com/scummvm/scummvm/commit/f17b8127105fab06b3f8ec838e544ad03e9deafb
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:39:59-05:00

Commit Message:
SCI: Fix HOYLE4 unexpected discard in Euchre

Fixes bug #14874

Thanks to @Karunamon for reporting this

Changed paths:
    engines/sci/engine/script_patches.cpp


diff --git a/engines/sci/engine/script_patches.cpp b/engines/sci/engine/script_patches.cpp
index 48fc70b1098..8b78a0b2dbc 100644
--- a/engines/sci/engine/script_patches.cpp
+++ b/engines/sci/engine/script_patches.cpp
@@ -2436,11 +2436,39 @@ static const uint16 hoyle4PatchGinUndercutSound[] = {
 	PATCH_END
 };
 
+// In Euchre, when discarding the first card after selecting the "Take it up"
+//  button, a second card can be unexpectedly discarded and break the game.
+//  The script is missing a HandsOff call. This allows two right clicks to
+//  trigger a second discard.
+//
+// We fix this by adding a HandsOff call to the code that handles the first
+//  discard. We make room by jumping to a codepath that does the same work.
+//
+// Applies to: All versions
+// Responsible method: EuchreHand:enterKey
+// Fixes bug: #14874
+static const uint16 hoyle4SignatureEuchreHandsOff[] = {
+	SIG_MAGICDWORD,
+	0x45, 0x01, 0x00,                  // callb 01 00 [ RedrawCast ]
+	0x38, SIG_SELECTOR16(setCursor),   // pushi setCursor
+	SIG_ADDTOOFFSET(+0x2b),
+	0x76,                              // push0
+	0x45, 0x01, 0x00,                  // callb 01 00 [ RedrawCast ]
+	SIG_END
+};
+
+static const uint16 hoyle4PatchEuchreHandsOff[] = {
+	0x45, 0x04, 0x00,                  // callb 04 00 [ HandsOff ]
+	0x32, PATCH_UINT16(0x002b),        // jmp 002b    [ RedrawCast, ... ]
+	PATCH_END
+};
+
 //          script, description,                                      signature                         patch
 static const SciScriptPatcherEntry hoyle4Signatures[] = {
 	{  true,   100, "crazy eights sound",                          1, hoyle4SignatureCrazyEightsSound,  hoyle4PatchCrazyEightsSound },
 	{  true,   400, "gin undercut sound",                          1, hoyle4SignatureGinUndercutSound,  hoyle4PatchGinUndercutSound },
 	{  true,   733, "bridge arithmetic against object",            1, hoyle4SignatureBridgeArithmetic,  hoyle4PatchBridgeArithmetic },
+	{  true,   800, "euchre handsoff",                             1, hoyle4SignatureEuchreHandsOff,    hoyle4PatchEuchreHandsOff },
 	SCI_SIGNATUREENTRY_TERMINATOR
 };
 


Commit: 4671c96143c7f8af0f7dd5cb72664b29bd6f4438
    https://github.com/scummvm/scummvm/commit/4671c96143c7f8af0f7dd5cb72664b29bd6f4438
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:39:59-05:00

Commit Message:
AGI: Allow message box to be drawn over menu

Fixes five Mixed-Up Mother Goose nursery rhymes. Bug #13820

Changed paths:
    engines/agi/graphics.cpp
    engines/agi/op_cmd.cpp
    engines/agi/text.cpp
    engines/agi/text.h


diff --git a/engines/agi/graphics.cpp b/engines/agi/graphics.cpp
index 3cabdc63ef5..caf4841e6af 100644
--- a/engines/agi/graphics.cpp
+++ b/engines/agi/graphics.cpp
@@ -510,8 +510,10 @@ byte GfxMgr::getCGAMixtureColor(byte color) {
 // Attention: in our implementation, y-coordinate is upper left.
 // Sierra passed the lower left instead. We changed it to make upscaling easier.
 void GfxMgr::render_Block(int16 x, int16 y, int16 width, int16 height, bool copyToScreen) {
-	if (!render_Clip(x, y, width, height))
+	if (!render_Clip(x, y, width, height)) {
+		warning("render_Block ignored by clipping. x: %d, y: %d, w: %d, h: %d", x, y, width, height);
 		return;
+	}
 
 	switch (_vm->_renderMode) {
 	case Common::kRenderHercG:
@@ -532,21 +534,33 @@ void GfxMgr::render_Block(int16 x, int16 y, int16 width, int16 height, bool copy
 	}
 }
 
+// FIXME: This function needs clarification and cleanup. Half of the code is
+// logically dead or disabled. Is the purpose to adjust out-of-bounds coordinates
+// so that they fit within boundaries, or is it to identify out-of-bounds
+// coordinates so that the entire drawing operation can be rejected?
 bool GfxMgr::render_Clip(int16 &x, int16 &y, int16 &width, int16 &height, int16 clipAgainstWidth, int16 clipAgainstHeight) {
+	// Allow rendering all the way to the top of the visual screen, even if there
+	// is a menu bar. MMMG nursery rhyme message boxes appear over the menu bar
+	// when the scripts pass a y-coordinate of zero to print.at(). Bug #13820
+	int16 minY = 0 - _renderStartDisplayOffsetY;
+
 	if ((x >= clipAgainstWidth) || ((x + width - 1) < 0) ||
-	        (y < 0) || ((y + (height - 1)) >= clipAgainstHeight)) {
+	        (y < minY) || ((y + (height - 1)) >= clipAgainstHeight)) {
 		return false;
 	}
 
-	if (y < 0) {
+	// FIXME: this check is always false, see above
+	if (y < minY) {
 		height += y;
-		y = 0;
+		y = minY;
 	}
 
+	// FIXME: this check is always false, see above
 	if ((y + height - 1) >= clipAgainstHeight) {
 		height = clipAgainstHeight - y;
 	}
 
+	// FIXME: why is this disabled?
 #if 0
 	if ((y - height + 1) < 0)
 		height = y + 1;
@@ -966,8 +980,10 @@ void GfxMgr::block_restore(int16 x, int16 y, int16 width, int16 height, byte *bu
 //            Going beyond 160x168 will result in messageboxes not getting fully removed
 //            In KQ4's case, the scripts clear the screen that's why it works.
 void GfxMgr::drawBox(int16 x, int16 y, int16 width, int16 height, byte backgroundColor, byte lineColor) {
-	if (!render_Clip(x, y, width, height, VISUAL_WIDTH, VISUAL_HEIGHT - _renderStartVisualOffsetY))
+	if (!render_Clip(x, y, width, height, VISUAL_WIDTH, VISUAL_HEIGHT - _renderStartVisualOffsetY)) {
+		warning("drawBox ignored by clipping. x: %d, y: %d, w: %d, h: %d", x, y, width, height);
 		return;
+	}
 
 	// coordinate translation: visual-screen -> display-screen
 	translateVisualRectToDisplayScreen(x, y, width, height);
diff --git a/engines/agi/op_cmd.cpp b/engines/agi/op_cmd.cpp
index 1b95d654f37..3f9b09462fc 100644
--- a/engines/agi/op_cmd.cpp
+++ b/engines/agi/op_cmd.cpp
@@ -1850,11 +1850,11 @@ void cmdVersion(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 
 void cmdConfigureScreen(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	TextMgr *textMgr = state->_vm->_text;
-	uint16 lineMinPrint = parameter[0];
+	uint16 gameRow = parameter[0];
 	uint16 promptRow = parameter[1];
 	uint16 statusRow = parameter[2];
 
-	textMgr->configureScreen(lineMinPrint);
+	textMgr->configureScreen(gameRow);
 	textMgr->statusRow_Set(statusRow);
 	textMgr->promptRow_Set(promptRow);
 }
diff --git a/engines/agi/text.cpp b/engines/agi/text.cpp
index d0eb753514f..10334a4db39 100644
--- a/engines/agi/text.cpp
+++ b/engines/agi/text.cpp
@@ -89,12 +89,12 @@ void TextMgr::init(SystemUI *systemUI) {
 	_systemUI = systemUI;
 }
 
-void TextMgr::configureScreen(uint16 row_Min) {
-	_window_Row_Min = row_Min;
-	_window_Row_Max = row_Min + 21;
+void TextMgr::configureScreen(uint16 gameRow) {
+	_window_Row_Min = gameRow;
+	_window_Row_Max = gameRow + 21;
 
 	// forward data to GfxMgr as well
-	_gfx->setRenderStartOffset(row_Min * FONT_VISUAL_HEIGHT);
+	_gfx->setRenderStartOffset(gameRow * FONT_VISUAL_HEIGHT);
 }
 uint16 TextMgr::getWindowRowMin() {
 	return _window_Row_Min;
diff --git a/engines/agi/text.h b/engines/agi/text.h
index e0d04ea808b..c5b5d724b52 100644
--- a/engines/agi/text.h
+++ b/engines/agi/text.h
@@ -98,7 +98,7 @@ public:
 	uint16 _window_Row_Max;
 	int16 _reset_Column;
 
-	void configureScreen(uint16 row_Min);
+	void configureScreen(uint16 gameRow);
 	uint16 getWindowRowMin();
 
 	void dialogueOpen();


Commit: b5458e3ad58605c4431293e5efc4b96847711467
    https://github.com/scummvm/scummvm/commit/b5458e3ad58605c4431293e5efc4b96847711467
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:39:59-05:00

Commit Message:
AGI: Fix Mixed-Up Mother Goose Amiga emulated version

Fixes the wrong graphics in the opening credits and throughout
the game. Fixes using the mouse to walk to room edges.

Changed paths:
    engines/agi/agi.h
    engines/agi/detection_tables.h


diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index 1138e3bdc56..c350d49c4ad 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -294,6 +294,7 @@ enum AgiMonitorType {
  * 2.202 (Space Quest II v2.0F)
  * 2.310 (Police Quest I v2.0B 1989-02-22)
  * 2.316 (Gold Rush! v2.05 1989-03-09)
+ * 2.328 (Mixed-Up Mother Mother Goose v1.1)
  * 2.333 (King's Quest III v2.15 1989-11-15)
  *
  * At least these Amiga AGI versions use value 20:
diff --git a/engines/agi/detection_tables.h b/engines/agi/detection_tables.h
index f5b746c5682..207c39e98a3 100644
--- a/engines/agi/detection_tables.h
+++ b/engines/agi/detection_tables.h
@@ -422,7 +422,7 @@ static const AGIGameDescription gameDescriptions[] = {
 	// King's Quest 2 (IIgs) 2.0A 6/16/88 (CE)
 	GAME_PO("kq2", "2.0A 1988-06-16 (CE)", "5203c8b95250a2ecfee93ddb99414753", 0x2917, GID_KQ2, Common::kPlatformApple2GS, GAMEOPTIONS_APPLE2GS),
 
-	// King's Quest 2 (Amiga) 2.0J
+	// King's Quest 2 (Amiga) 2.0J # 2.107
 	GAME_PO("kq2", "2.0J 1987-01-29", "b866f0fab2fad91433a637a828cfa410", 0x2440, GID_KQ2, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
 
 	// King's Quest 2 (Mac) 2.0R 3/23/88
@@ -613,7 +613,7 @@ static const AGIGameDescription gameDescriptions[] = {
 	// Manhunter NY (IIgs) 2.0E 10/05/88 (CE)
 	GAME3_P("mh1", "2.0E 1988-10-05 (CE)", "mhdir", "2f1509f76f24e6e7d213f2dadebbf156", 0x3149, 0, GID_MH1, Common::kPlatformApple2GS),
 
-	// Manhunter NY (Amiga) 1.06 3/18/89
+	// Manhunter NY (Amiga) 1.06 3/18/89 # 2.328
 	GAME3_PO("mh1", "1.06 1989-03-18", "dirs", "92c6183042d1c2bb76236236a7d7a847", 0x3149, GF_OLDAMIGAV20, GID_MH1, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
 
 	// reported by Filippos (thebluegr) in bugreport #3048
@@ -691,10 +691,9 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAMEpre_P("mickey", "", "1.pic", "b6ec04c91a05df374792872c4d4ce66d", 2489,
 							"OBJ/FLASHLIT.OOO", "d60a7b6ff96720037f7e88863f48c5d4", 97, 0x0000, GID_MICKEY, Common::kPlatformDOS),
 
-	// Mixed-Up Mother Goose (Amiga) 1.1
-	// Problematic: crashes
-	// Menus not tested
-	GAME3_PSO("mixedup", "1.1 1986-12-10", "dirs", "5c1295fe6daaf95831195ba12894dbd9", 2021, 0x3086, 0, GID_MIXEDUP, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
+	// Mixed-Up Mother Goose (Amiga) 1.1 # 2.328
+	// Files are timestamped 1986-12-10, but this is a 1989 AGI 3 interpreter.
+	GAME3_PSO("mixedup", "1.1", "dirs", "5c1295fe6daaf95831195ba12894dbd9", 2021, 0x3149, 0, GID_MIXEDUP, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
 
 	// Mixed Up Mother Goose (IIgs)
 	GAME_PO("mixedup", "1987", "3541954a7303467c6df87665312ffb6a", 0x2917, GID_MIXEDUP, Common::kPlatformApple2GS, GAMEOPTIONS_APPLE2GS),


Commit: 6f346f96084b24fbb1f09b69d16357fab1003083
    https://github.com/scummvm/scummvm/commit/6f346f96084b24fbb1f09b69d16357fab1003083
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:40:00-05:00

Commit Message:
AGI: Fix Manhunter 2 Amiga emulated version

Fixes the hold.key opcode being parsed incorrectly

Changed paths:
    engines/agi/detection_tables.h


diff --git a/engines/agi/detection_tables.h b/engines/agi/detection_tables.h
index 207c39e98a3..b0c3bcd7188 100644
--- a/engines/agi/detection_tables.h
+++ b/engines/agi/detection_tables.h
@@ -651,7 +651,7 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME3_P("mh2", "1.0 1989-07-29", "mh2dir", "5e3581495708b952fea24438a6c7e040", 0x3149, 0, GID_MH1, Common::kPlatformAtariST),
 
 	// Manhunter SF (Amiga) 3.06 8/17/89        # 2.333
-	GAME3_PSO("mh2", "3.06 1989-08-17", "dirs", "b412e8a126368b76696696f7632d4c16", 2573, 0x3086, GF_OLDAMIGAV20, GID_MH2, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
+	GAME3_PSO("mh2", "3.06 1989-08-17", "dirs", "b412e8a126368b76696696f7632d4c16", 2573, 0x3149, GF_OLDAMIGAV20, GID_MH2, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
 
 	// Manhunter SF (PC 5.25") 3.02 5.25\"" [AGI 3.002.149]
 	GAME3("mh2", "3.02 1989-07-26 5.25\"", "mh2dir", "bbb2c2f88d5740f7437fb7aa6f080b7b", 0x3149, GID_MH2),


Commit: f2221d9b1e2e15a864dc08d38140b5b04688b470
    https://github.com/scummvm/scummvm/commit/f2221d9b1e2e15a864dc08d38140b5b04688b470
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:40:00-05:00

Commit Message:
AGI: Use correct Amiga platform value

The correct value for variable 20 on Amiga interpreters is always 5.

Confirmed in the Amiga scripts for SQ1, MH1, and MH2. These games all
expect variable 20 to have a value of 5, just like all Amiga games.

Fixes Amiga menus and help text not appearing, and other details.

Note that this fix exposes another bug which now prevents the Amiga
Manhunter games from starting: the opcode table is wrong for these
versions. This also prevents the Atari ST version of MH1 from starting.
Fixed in next commit.

Changed paths:
    engines/agi/agi.h
    engines/agi/cycle.cpp
    engines/agi/detection_tables.h


diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index c350d49c4ad..c6889280997 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -115,18 +115,13 @@ enum AgiGameType {
 	GType_V3 = 3
 };
 
-//
-// GF_OLDAMIGAV20 means that the interpreter is an old Amiga AGI interpreter that
-// uses value 20 for the computer type (v20 i.e. vComputer) rather than the usual value 5.
-//
 enum AgiGameFeatures {
 	GF_AGIMOUSE    = (1 << 0), // this disables "Click-to-walk mouse interface"
 	GF_AGDS        = (1 << 1),
 	GF_AGI256      = (1 << 2), // marks fanmade AGI-256 games
 	GF_FANMADE     = (1 << 3), // marks fanmade games
-	GF_OLDAMIGAV20 = (1 << 4),
 	GF_2GSOLDSOUND = (1 << 5),
-	GF_EXTCHAR = (1 << 6) // use WORDS.TOK.EXTENDED
+	GF_EXTCHAR     = (1 << 6)  // use WORDS.TOK.EXTENDED
 };
 
 enum BooterDisks {
@@ -284,31 +279,12 @@ enum AgiMonitorType {
 /**
  * Different computer types.
  * Used with AGI variable 20 i.e. vComputer.
- *
- * At least these Amiga AGI versions use value 5:
- * 2.082 (King's Quest I v1.0U 1986)
- * 2.090 (King's Quest III v1.01 1986-11-08)
- * x.yyy (Black Cauldron v2.00 1987-06-14)
- * x.yyy (Larry I v1.05 1987-06-26)
- * 2.107 (King's Quest II v2.0J. Date is probably 1987-01-29)
- * 2.202 (Space Quest II v2.0F)
- * 2.310 (Police Quest I v2.0B 1989-02-22)
- * 2.316 (Gold Rush! v2.05 1989-03-09)
- * 2.328 (Mixed-Up Mother Mother Goose v1.1)
- * 2.333 (King's Quest III v2.15 1989-11-15)
- *
- * At least these Amiga AGI versions use value 20:
- * 2.082 (Space Quest I v1.2 1986)
- * x.yyy (Manhunter NY 1.06 3/18/89)
- * 2.333 (Manhunter SF 3.06 8/17/89)
- *
  */
 enum AgiComputerType {
 	kAgiComputerPC = 0,
 	kAgiComputerAtariST = 4,
-	kAgiComputerAmiga = 5, // Newer Amiga AGI interpreters' value (Commonly used)
-	kAgiComputerApple2GS = 7,
-	kAgiComputerAmigaOld = 20 // Older Amiga AGI interpreters' value (Seldom used)
+	kAgiComputerAmiga = 5,
+	kAgiComputerApple2GS = 7
 };
 
 enum AgiSoundType {
diff --git a/engines/agi/cycle.cpp b/engines/agi/cycle.cpp
index f8927c404b4..78a72beca8e 100644
--- a/engines/agi/cycle.cpp
+++ b/engines/agi/cycle.cpp
@@ -488,10 +488,7 @@ int AgiEngine::runGame() {
 			setVar(VM_VAR_SOUNDGENERATOR, kAgiSoundPC);
 			break;
 		case Common::kPlatformAmiga:
-			if (getFeatures() & GF_OLDAMIGAV20)
-				setVar(VM_VAR_COMPUTER, kAgiComputerAmigaOld);
-			else
-				setVar(VM_VAR_COMPUTER, kAgiComputerAmiga);
+			setVar(VM_VAR_COMPUTER, kAgiComputerAmiga);
 			setVar(VM_VAR_SOUNDGENERATOR, kAgiSoundTandy);
 			break;
 		case Common::kPlatformApple2GS:
diff --git a/engines/agi/detection_tables.h b/engines/agi/detection_tables.h
index b0c3bcd7188..ab31c82eeb6 100644
--- a/engines/agi/detection_tables.h
+++ b/engines/agi/detection_tables.h
@@ -614,7 +614,7 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME3_P("mh1", "2.0E 1988-10-05 (CE)", "mhdir", "2f1509f76f24e6e7d213f2dadebbf156", 0x3149, 0, GID_MH1, Common::kPlatformApple2GS),
 
 	// Manhunter NY (Amiga) 1.06 3/18/89 # 2.328
-	GAME3_PO("mh1", "1.06 1989-03-18", "dirs", "92c6183042d1c2bb76236236a7d7a847", 0x3149, GF_OLDAMIGAV20, GID_MH1, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
+	GAME3_PO("mh1", "1.06 1989-03-18", "dirs", "92c6183042d1c2bb76236236a7d7a847", 0x3149, 0, GID_MH1, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
 
 	// reported by Filippos (thebluegr) in bugreport #3048
 	// Manhunter NY (PC 5.25") 1.22 8/31/88 [AGI 3.002.107]
@@ -651,7 +651,7 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME3_P("mh2", "1.0 1989-07-29", "mh2dir", "5e3581495708b952fea24438a6c7e040", 0x3149, 0, GID_MH1, Common::kPlatformAtariST),
 
 	// Manhunter SF (Amiga) 3.06 8/17/89        # 2.333
-	GAME3_PSO("mh2", "3.06 1989-08-17", "dirs", "b412e8a126368b76696696f7632d4c16", 2573, 0x3149, GF_OLDAMIGAV20, GID_MH2, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
+	GAME3_PSO("mh2", "3.06 1989-08-17", "dirs", "b412e8a126368b76696696f7632d4c16", 2573, 0x3149, 0, GID_MH2, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
 
 	// Manhunter SF (PC 5.25") 3.02 5.25\"" [AGI 3.002.149]
 	GAME3("mh2", "3.02 1989-07-26 5.25\"", "mh2dir", "bbb2c2f88d5740f7437fb7aa6f080b7b", 0x3149, GID_MH2),
@@ -758,7 +758,7 @@ static const AGIGameDescription gameDescriptions[] = {
 
 	// Space Quest 1 (Amiga) 1.2            # 2.082
 	// The original game did not have menus, they are enabled under ScummVM
-	GAME_FPO("sq1", "1.2 1986", "0b216d931e95750f1f4837d6a4b821e5", 0x2440, GF_OLDAMIGAV20, GID_SQ1, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
+	GAME_FPO("sq1", "1.2 1986", "0b216d931e95750f1f4837d6a4b821e5", 0x2440, 0, GID_SQ1, Common::kPlatformAmiga, GAMEOPTIONS_AMIGA),
 
 	// Space Quest 1 (Mac) 1.5D
 	GAME_P("sq1", "1.5D 1987-04-02", "ce88419aadd073d1c6682d859b3d8aa2", 0x2440, GID_SQ1, Common::kPlatformMacintosh),


Commit: 8cf76f893ee0ff2d3a306c2767372fca2090862b
    https://github.com/scummvm/scummvm/commit/8cf76f893ee0ff2d3a306c2767372fca2090862b
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:40:02-05:00

Commit Message:
AGI: Fix opcode table version detection

- Fixes Manhunter 1 Amiga / Atari ST not starting
- Fixes Manhunter 2 Amiga not starting
- Fixes Gold Rush Atari ST
- Fixes King's Quest IV Apple IIGS
- Fixes Black Cauldron Apple IIGS
- Uses original opcode name: get.mse.posn (from GR Amiga)

Changed paths:
    engines/agi/op_cmd.cpp
    engines/agi/opcodes.cpp
    engines/agi/opcodes.h


diff --git a/engines/agi/op_cmd.cpp b/engines/agi/op_cmd.cpp
index 3f9b09462fc..01211ca3d6f 100644
--- a/engines/agi/op_cmd.cpp
+++ b/engines/agi/op_cmd.cpp
@@ -1115,9 +1115,10 @@ void cmdAdjEgoMoveToXY(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	int8 x, y;
 
 	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)).
+	// The 2 parameter version is used in:
+	// Amiga/Atari ST Gold Rush!   - Logic 130, 150
+	// Amiga/Atari ST Manhunter 1  - Logic 0
+	// Amiga Manhunter 2           - Logic 0
 	case 2:
 		// Both arguments are signed 8-bit (i.e. in range -128 to +127).
 		x = (int8) parameter[0];
@@ -2272,7 +2273,7 @@ void cmdSetPriBase(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	state->_vm->_gfx->setPriorityTable(priorityBase);
 }
 
-void cmdMousePosn(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
+void cmdGetMousePosn(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	uint16 destVarNr1 = parameter[0];
 	uint16 destVarNr2 = parameter[1];
 	int16 mouseX = vm->_mouse.pos.x;
diff --git a/engines/agi/opcodes.cpp b/engines/agi/opcodes.cpp
index 43c8a17b295..7814b87836b 100644
--- a/engines/agi/opcodes.cpp
+++ b/engines/agi/opcodes.cpp
@@ -24,6 +24,13 @@
 
 namespace Agi {
 
+// FIXME: The parameter strings in the opcode table have mistakes.
+// Nothing depends on the values of the individual characters.
+// Some are out of sync with how the opcode function interprets
+// the parameter. Only the string lengths are used to indicate
+// the parameter count for parsing.
+// Consult the opcode functions for the real parameter types.
+
 const AgiOpCodeDefinitionEntry opCodesV1Cond[] = {
 	{ "",                   "",         &condUnknown },     // 00
 	{ "equaln",             "vn",       &condEqual },       // 01
@@ -345,13 +352,13 @@ AgiOpCodeDefinitionEntry opCodesV2[] = {
 	{ "hold.key",           "",         &cmdHoldKey },          // AD
 	{ "set.pri.base",       "n",        &cmdSetPriBase },       // AE AGI2.936+ *AND* also inside AGI2.425
 	{ "discard.sound",      "n",        &cmdDiscardSound },     // AF was skip for PC
-	{ "hide.mouse",         "",         &cmdHideMouse },        // B0 1 arg for AGI version 3.002.086 AGI3+ only starts here
+	{ "hide.mouse",         "",         &cmdHideMouse },        // B0 1 arg for AGI3 Apple IIGS and AGI 3.002.086. AGI3+ only starts here
 	{ "allow.menu",         "n",        &cmdAllowMenu },        // B1
-	{ "show.mouse",         "",         &cmdShowMouse },        // B2
+	{ "show.mouse",         "",         &cmdShowMouse },        // B2 1 arg for AGI3 Apple IIGS
 	{ "fence.mouse",        "nnnn",     &cmdFenceMouse },       // B3
-	{ "mouse.posn",         "vv",       &cmdMousePosn },        // B4
-	{ "release.key",        "",         &cmdReleaseKey },       // B5 2 args for at least the Amiga GR (v2.05 1989-03-09) using AGI 2.316
-	{ "adj.ego.move.to.xy", "",         &cmdAdjEgoMoveToXY }    // B6
+	{ "get.mse.posn",       "vv",       &cmdGetMousePosn },     // B4
+	{ "release.key",        "",         &cmdReleaseKey },       // B5
+	{ "adj.ego.move.to.x.y","",         &cmdAdjEgoMoveToXY }    // B6 2 args for Amiga/Atari ST GR, MH1, MH2
 };
 
 //
@@ -402,9 +409,10 @@ void AgiEngine::setupOpCodes(uint16 version) {
 		if (version == 0x2089)
 			_opCodes[0x86].parameters = "";
 
-		// 'print.at' and 'print.at.v' take 3 args before 2.089.
-		// This is documented in the specs as only < 2.440, but
-		// SQ1 1.0X (2.089) and KQ3 (2.272) take 4 args. Bug #10872
+		// 'print.at' and 'print.at.v' take three parameters before 2.089.
+		// This is documented in the specs as only < 2.440, but SQ1 1.0X (2.089)
+		// and KQ3 (2.272) take four. Bug #10872. No game scripts have been
+		// discovered that call either opcode with only three parameters.
 		if (version < 0x2089) {
 			_opCodes[0x97].parameters = "vvv";
 			_opCodes[0x98].parameters = "vvv";
@@ -413,32 +421,36 @@ void AgiEngine::setupOpCodes(uint16 version) {
 
 	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
+
+		// hide.mouse and hide.key take 1 parameter for 3.002.086.
+		// KQ4 is the only known game with this interpreter and
+		// its scripts do not call either opcode. no game scripts
+		// have been discovered that call hold.key with 1 parameter.
 		if (version == 0x3086) {
-			_opCodes[0xb0].parameters = "n";
-			_opCodes[0xad].parameters = "n";
+			_opCodes[0xb0].parameters = "n"; // hide.mouse
+			_opCodes[0xad].parameters = "n"; // hold.key
 		}
-	}
 
-	// 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 #6161 and #5885.
-	if ((getGameID() == GID_MH1 || getGameID() == GID_GOLDRUSH) &&
-	        getPlatform() == Common::kPlatformApple2GS) {
-		_opCodes[176].parameters = "n";  // hide.mouse
-		_opCodes[178].parameters = "n";  // show.mouse
-	}
+		// hide.mouse and show.mouse take 1 parameter on Apple IIGS.
+		// Used by Black Cauldron, Gold Rush, King's Quest IV, and Manhunter 1.
+		// Fixes bugs #6161 and #5885.
+		if (getPlatform() == Common::kPlatformApple2GS) {
+			_opCodes[0xb0].parameters = "n";  // hide.mouse
+			_opCodes[0xb2].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";
+		// adj.ego.move.to.x.y takes two parameters for Amiga/Atari ST
+		// versions of Gold Rush, Manhunter 1, and Manhunter 2.
+		// No scripts have been discovered that call adj.ego.move.to.x.y
+		// with zero parameters.
+		if ((getGameID() == GID_GOLDRUSH ||
+			 getGameID() == GID_MH1 ||
+			 getGameID() == GID_MH2) &&
+			(getPlatform() == Common::kPlatformAmiga ||
+			 getPlatform() == Common::kPlatformAtariST)) {
+			_opCodes[0xb6].parameters = "vv";
+		}
+	}
 
 	// add invalid entries for every opcode, that is not defined at all
 	for (int opCodeNr = opCodesTableSize; opCodeNr < ARRAYSIZE(_opCodes); opCodeNr++) {
diff --git a/engines/agi/opcodes.h b/engines/agi/opcodes.h
index 9ea5e5d3291..af1d616e1a8 100644
--- a/engines/agi/opcodes.h
+++ b/engines/agi/opcodes.h
@@ -216,7 +216,7 @@ void cmdHideMouse(AgiGame *state, AgiEngine *vm, uint8 *p); // 0xb0
 void cmdAllowMenu(AgiGame *state, AgiEngine *vm, uint8 *p);
 void cmdShowMouse(AgiGame *state, AgiEngine *vm, uint8 *p);
 void cmdFenceMouse(AgiGame *state, AgiEngine *vm, uint8 *p);
-void cmdMousePosn(AgiGame *state, AgiEngine *vm, uint8 *p);
+void cmdGetMousePosn(AgiGame *state, AgiEngine *vm, uint8 *p);
 void cmdReleaseKey(AgiGame *state, AgiEngine *vm, uint8 *p);
 void cmdAdjEgoMoveToXY(AgiGame *state, AgiEngine *vm, uint8 *p);
 


Commit: 9667ee52c31ca5fce81c7c9a69c39d5beefc2993
    https://github.com/scummvm/scummvm/commit/9667ee52c31ca5fce81c7c9a69c39d5beefc2993
Author: Filippos Karapetis (bluegr at gmail.com)
Date: 2024-02-15T21:40:09-05:00

Commit Message:
AGI: Get rid of the older game flags

These are leftovers from Sarien, and are no longer needed

Changed paths:
    engines/agi/agi.cpp
    engines/agi/agi.h
    engines/agi/objects.cpp
    engines/agi/preagi/preagi.cpp
    engines/agi/saveload.cpp


diff --git a/engines/agi/agi.cpp b/engines/agi/agi.cpp
index 23e4cc22df1..a62ae803f45 100644
--- a/engines/agi/agi.cpp
+++ b/engines/agi/agi.cpp
@@ -143,15 +143,9 @@ int AgiEngine::agiInit() {
 	}
 
 	if (getPlatform() == Common::kPlatformAmiga)
-		_game.gameFlags |= ID_AMIGA;
-
-	if (getFeatures() & GF_AGDS)
-		_game.gameFlags |= ID_AGDS;
-
-	if (_game.gameFlags & ID_AMIGA)
 		debug(1, "Amiga padded game detected.");
 
-	if (_game.gameFlags & ID_AGDS)
+	if (getFeatures() & GF_AGDS)
 		debug(1, "AGDS mode enabled.");
 
 	ec = _loader->init();   // load vol files, etc
@@ -450,8 +444,6 @@ void AgiEngine::initialize() {
 
 	_text->init(_systemUI);
 
-	_game.gameFlags = 0;
-
 	_text->charAttrib_Set(15, 0);
 
 	_game.name[0] = '\0';
diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index c6889280997..6155f419cb0 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -413,9 +413,6 @@ struct AgiGame {
 	bool playerControl; /**< player is in control */
 	bool exitAllLogics; /**< break cycle after new.room */
 	bool pictureShown;  /**< show.pic has been issued */
-#define ID_AGDS     0x00000001
-#define ID_AMIGA    0x00000002
-	int gameFlags;      /**< agi options flags */
 
 	// windows
 	AgiBlock block;
@@ -512,7 +509,6 @@ struct AgiGame {
 		playerControl = false;
 		exitAllLogics = false;
 		pictureShown = false;
-		gameFlags = 0;
 
 		// block defaulted by AgiBlock constructor
 
diff --git a/engines/agi/objects.cpp b/engines/agi/objects.cpp
index d5964bbc4c5..b014eaee1bb 100644
--- a/engines/agi/objects.cpp
+++ b/engines/agi/objects.cpp
@@ -28,7 +28,7 @@ namespace Agi {
 int AgiEngine::decodeObjects(uint8 *mem, uint32 flen) {
 	unsigned int i, so, padsize, spos;
 
-	padsize = _game.gameFlags & ID_AMIGA ? 4 : 3;
+	padsize = getPlatform() == Common::kPlatformAmiga ? 4 : 3;
 
 	_game.numObjects = 0;
 
diff --git a/engines/agi/preagi/preagi.cpp b/engines/agi/preagi/preagi.cpp
index 134435a9afa..dcad8a2d2a9 100644
--- a/engines/agi/preagi/preagi.cpp
+++ b/engines/agi/preagi/preagi.cpp
@@ -50,8 +50,6 @@ void PreAgiEngine::initialize() {
 
 	_font->init();
 
-	_game.gameFlags = 0;
-
 	//_game._vm->_text->charAttrib_Set(15, 0);
 
 	_defaultColor = 0xF;
diff --git a/engines/agi/saveload.cpp b/engines/agi/saveload.cpp
index 70ee112ec25..d45c107876a 100644
--- a/engines/agi/saveload.cpp
+++ b/engines/agi/saveload.cpp
@@ -175,7 +175,7 @@ int AgiEngine::saveGame(const Common::String &fileName, const Common::String &de
 	out->writeSint16BE((int16)_game.exitAllLogics);
 	out->writeSint16BE((int16)_game.pictureShown);
 	out->writeSint16BE((int16)_text->promptIsEnabled()); // was "_game.hasPrompt", no longer needed
-	out->writeSint16BE((int16)_game.gameFlags);
+	out->writeSint16BE(0);	// was _game.gameFlags, no longer needed
 
 	if (_text->promptIsEnabled()) {
 		out->writeSint16BE(0x7FFF);
@@ -504,7 +504,7 @@ int AgiEngine::loadGame(const Common::String &fileName, bool checkId) {
 	_game.exitAllLogics = in->readSint16BE();
 	in->readSint16BE(); // was _game.pictureShown
 	in->readSint16BE(); // was _game.hasPrompt, no longer needed
-	_game.gameFlags = in->readSint16BE();
+	in->readSint16BE();	// was _game.gameFlags, no longer needed
 	if (in->readSint16BE()) {
 		_text->promptEnable();
 	} else {


Commit: d71734fff0bb4e7a9fd65d72057180b04ec62cb6
    https://github.com/scummvm/scummvm/commit/d71734fff0bb4e7a9fd65d72057180b04ec62cb6
Author: Filippos Karapetis (bluegr at gmail.com)
Date: 2024-02-15T21:40:09-05:00

Commit Message:
AGI: Remove superfluous AGI version setup for AGDS games

Changed paths:
    engines/agi/agi.h
    engines/agi/loader_v2.cpp
    engines/agi/metaengine.cpp


diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index 6155f419cb0..24606adb150 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -117,7 +117,7 @@ enum AgiGameType {
 
 enum AgiGameFeatures {
 	GF_AGIMOUSE    = (1 << 0), // this disables "Click-to-walk mouse interface"
-	GF_AGDS        = (1 << 1),
+	GF_AGDS        = (1 << 1), // marks games created with AGDS - all using AGI version 2.440
 	GF_AGI256      = (1 << 2), // marks fanmade AGI-256 games
 	GF_FANMADE     = (1 << 3), // marks fanmade games
 	GF_2GSOLDSOUND = (1 << 5),
@@ -761,7 +761,6 @@ public:
 	void initFeatures();
 	void setFeature(uint32 feature);
 	void initVersion();
-	void setVersion(uint16 version);
 
 	const char *getDiskName(uint16 id);
 
diff --git a/engines/agi/loader_v2.cpp b/engines/agi/loader_v2.cpp
index 2407dfbe00b..bcc2e6bac89 100644
--- a/engines/agi/loader_v2.cpp
+++ b/engines/agi/loader_v2.cpp
@@ -33,11 +33,6 @@ int AgiLoader_v2::detectGame() {
 	    !Common::File::exists(VIEWDIR))
 		return errInvalidAGIFile;
 
-	// 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;
 }
 
diff --git a/engines/agi/metaengine.cpp b/engines/agi/metaengine.cpp
index 4459bd99fb7..10fd89b9d21 100644
--- a/engines/agi/metaengine.cpp
+++ b/engines/agi/metaengine.cpp
@@ -87,10 +87,6 @@ void AgiBase::setFeature(uint32 feature) {
 	_gameFeatures |= feature;
 }
 
-void AgiBase::setVersion(uint16 version) {
-	_gameVersion = version;
-}
-
 void AgiBase::initVersion() {
 	_gameVersion = _gameDescription->version;
 }


Commit: 9aa94960bd0eac074b8c4e5e3bb0f7b060d53bb4
    https://github.com/scummvm/scummvm/commit/9aa94960bd0eac074b8c4e5e3bb0f7b060d53bb4
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:40:14-05:00

Commit Message:
AGI: Document CoCo3 games in detection table

Changed paths:
    engines/agi/detection_tables.h


diff --git a/engines/agi/detection_tables.h b/engines/agi/detection_tables.h
index ab31c82eeb6..72987d9a555 100644
--- a/engines/agi/detection_tables.h
+++ b/engines/agi/detection_tables.h
@@ -291,9 +291,11 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME3("bc", "2.10 1988-11-10 3.5\"", "bcdir", "0de3953c9225009dc91e5b0d1692967b", 0x3149, GID_BC),
 
 	// Black Cauldron (CoCo3 360k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("bc", "", "51212c54808ade96176f201ae0ac7a6f", 357, 0x2440, GID_BC, Common::kPlatformCoCo3),
 
 	// Black Cauldron (CoCo3 360k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("bc", "updated", "c4e1937f74e8100cd0152b904434d8b4", 357, 0x2440, GID_BC, Common::kPlatformCoCo3),
 
 	// Donald Duck's Playground (PC Booter) 1.0Q
@@ -373,9 +375,11 @@ static const AGIGameDescription gameDescriptions[] = {
 	},
 
 	// Gold Rush! (CoCo3 720k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("goldrush", "", "0a41b65efc0cd6c4271e957e6ffbbd8e", 744, 0x2440, GID_GOLDRUSH, Common::kPlatformCoCo3),
 
 	// Gold Rush! (CoCo3 360k/720k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("goldrush", "updated", "c49bf56bf91e31a4601a604e51ef8bfb", 744, 0x2440, GID_GOLDRUSH, Common::kPlatformCoCo3),
 
 	// King's Quest 1 (DOS) 1.0U [AGI 2.272]
@@ -408,12 +412,15 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME("kq1", "2.0F 1987-05-05 5.25\"/3.5\"", "10ad66e2ecbd66951534a50aedcd0128", 0x2917, GID_KQ1),
 
 	// King's Quest 1 (CoCo3 360k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("kq1", "", "10ad66e2ecbd66951534a50aedcd0128", 315, 0x2440, GID_KQ1, Common::kPlatformCoCo3),
 
 	// King's Quest 1 (CoCo3 360k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("kq1", "fixed", "4c8ef8b5d2f1b6c1a93e456d1f1ffc74", 768, 0x2440, GID_KQ1, Common::kPlatformCoCo3),
 
 	// King's Quest 1 (CoCo3 360k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("kq1", "updated", "94087178c78933a4af3cd24d1c8dd7b2", 315, 0x2440, GID_KQ1, Common::kPlatformCoCo3),
 
 	// King's Quest 1 (Russian)
@@ -485,15 +492,19 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME_LPS("kq2", "", "35211c574ececebdc723b23e35f99275", 543, Common::RU_RUS, 0x2917, GID_KQ2, Common::kPlatformDOS),
 
 	// King's Quest 2 (CoCo3 360k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("kq2", "", "b944c4ff18fb8867362dc21cc688a283", 543, 0x2440, GID_KQ2, Common::kPlatformCoCo3),
 
 	// King's Quest 2 (CoCo3 360k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("kq2", "updated", "f64a606de740a5348f3d125c03e989fe", 543, 0x2440, GID_KQ2, Common::kPlatformCoCo3),
 
 	// King's Quest 2 (CoCo3 360k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("kq2", "fixed", "fb33ac2768a94a89117a270771db465c", 768, 0x2440, GID_KQ2, Common::kPlatformCoCo3),
 
 	// King's Quest 2 (CoCo3 720k)
+	// Unofficial port by Guillaume Major
 	GAME_PS("kq2", "", "bdb10876cd4cb20eabd88778c40b4075", 543, 0x2440, GID_KQ2, Common::kPlatformCoCo3),
 
 	// King's Quest 3 (Amiga) 1.01 11/8/86
@@ -541,6 +552,7 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME_LPS("kq3", "2.14 1988-03-15 3.5\"", "87956c92d23f53d81bf2ee9e08fdc64c", 390, Common::ES_ESP, 0x2936, GID_KQ3, Common::kPlatformDOS),
 
 	// King's Quest 3 (CoCo3 158k/360k) 1.0C [AGI 2.023]
+	// Official port by Sierra
 	GAME_PS("kq3", "", "5a6be7d16b1c742c369ef5cc64fefdd2", 429, 0x2440, GID_KQ3, Common::kPlatformCoCo3),
 
 	// King's Quest 4 (PC 5.25") 2.0 7/27/88 [AGI 3.002.086]
@@ -575,12 +587,15 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME3("kq4", "Demo 1988-12-20", "dmdir", "a3332d70170a878469d870b14863d0bf", 0x3149, GID_KQ4),
 
 	// King's Quest 4 (CoCo3 720k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("kq4", "", "9e7729a28e749ca241d2bf71b9b2dbde", 741, 0x2440, GID_KQ4, Common::kPlatformCoCo3),
 
 	// King's Quest 4 (CoCo3 360k/720k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("kq4", "updated", "1959ca10739edb34069bb504dbd74805", 741, 0x2440, GID_KQ4, Common::kPlatformCoCo3),
 
-	// King's Quest 4 (CoCo3 360k/720k) update by Guillaume Major (2021)
+	// King's Quest 4 (CoCo3 360k/720k)
+	// Unofficial port by Guillaume Major
 	GAME_PS("kq4", "", "ef1c0d2d14fe643929a092621c7459cc", 741, 0x2440, GID_KQ4, Common::kPlatformCoCo3),
 
 	// Leisure Suit Larry 1 (PC 5.25"/3.5") 1.00 6/1/87 [AGI 2.440]
@@ -605,6 +620,7 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME_P("lsl1", "1.05 1987-06-26", "8a0076429890531832f0dc113285e31e", 0x2440, GID_LSL1, Common::kPlatformMacintosh),
 
 	// Leisure Suit Larry 1 (CoCo3 158k/360k) [AGI 2.072]
+	// Official port by Sierra
 	GAME_PS("lsl1", "", "a2de1fe76565c3e8b40c9d036b5e5612", 198, 0x2440, GID_LSL1, Common::kPlatformCoCo3),
 
 	// Manhunter NY (ST) 1.03 10/20/88
@@ -624,9 +640,11 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME3_PS("mh1", "1.22 1988-08-31", "mhdir", "5b625329021ad49fd0c1d6f2d6f54bba", 2141, 0x3149, 0, GID_MH1, Common::kPlatformDOS),
 
 	// Manhunter NY (CoCo3 720k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("mh1", "", "b968285caf2f591c78dd9c9e26ab8974", 495, 0x2440, GID_MH1, Common::kPlatformCoCo3),
 
 	// Manhunter NY (CoCo3 360k/720k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("mh1", "updated", "d47da950c62289f8d4ccf36af73365f2", 495, 0x2440, GID_MH1, Common::kPlatformCoCo3),
 
 	{
@@ -681,9 +699,11 @@ static const AGIGameDescription gameDescriptions[] = {
 	},
 
 	// Manhunter SF (CoCo3 720k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("mh2", "", "acaaa577e10d1753c5a74f6ae1d858d4", 591, 0x2440, GID_MH2, Common::kPlatformCoCo3),
 
 	// Manhunter SF (CoCo3 720k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("mh2", "updated", "c64875766700196e72a92359f70f45a9", 591, 0x2440, GID_MH2, Common::kPlatformCoCo3),
 
 	// Mickey's Space Adventure
@@ -702,6 +722,7 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME("mixedup", "1987-11-10", "e524655abf9b96a3b179ffcd1d0f79af", 0x2917, GID_MIXEDUP),
 
 	// Mixed-Up Mother Goose (CoCo3 360k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("mixedup", "", "44e63e9b4d4822a31edea0e8a7e7eac4", 606, 0x2440, GID_MIXEDUP, Common::kPlatformCoCo3),
 
 	// Police Quest 1 (PC) 2.0E 11/17/87 [AGI 2.915]
@@ -739,9 +760,11 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME_LPS("pq1", "2.0G 1987-12-03", "5d151f2f4c4e0675534d49b13529da3f", 360, Common::ES_ESP, 0x2917, GID_PQ1, Common::kPlatformDOS),
 
 	// Police Quest 1 (CoCo3 360k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("pq1", "", "28a077041f75aab78f66804800940085", 375, 0x2440, GID_PQ1, Common::kPlatformCoCo3),
 
 	// Police Quest 1 (CoCo3 360k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("pq1", "updated", "63b9a9c6eec154751dd446cd3693e0e2", 768, 0x2440, GID_PQ1, Common::kPlatformCoCo3),
 
 	// Space Quest 1 (ST) 1.1A
@@ -780,12 +803,15 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME_LVFPN("sq1", "2.2 1987-05-07 5.25\"/3.5\"", "words.tok.extended", "3f1730f3c9d4622a986f735af0f8734a", 12665, Common::FR_FRA, 0x2917, GF_EXTCHAR, GID_SQ1, Common::kPlatformDOS, GType_V2, GAMEOPTIONS_DEFAULT),
 
 	// Space Quest 1 (CoCo3 360k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("sq1", "", "5d67630aba008ec5f7f9a6d0a00582f4", 372, 0x2440, GID_SQ1, Common::kPlatformCoCo3),
 
 	// Space Quest 1 (CoCo3 360k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("sq1", "fixed", "ca822b768b6462e410423ea7f498daee", 768, 0x2440, GID_SQ1, Common::kPlatformCoCo3),
 
 	// Space Quest 1 (CoCo3 360k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("sq1", "updated", "7fa54e6bb7ffeb4cf20eca39d86f5fb2", 387, 0x2440, GID_SQ1, Common::kPlatformCoCo3),
 
 	// Space Quest 2 (PC 3.5") 2.0D [AGI 2.936]
@@ -859,9 +885,11 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME("sq2", "2.0F 1989-01-05 5.25\"", "bb5a44d0bea416f2cd4c3385eaa21af4", 0x2936, GID_SQ2),
 
 	// Space Quest 2 (CoCo3 360k) [AGI 2.023]
+	// Unofficial port by Guillaume Major
 	GAME_PS("sq2", "", "12973d39b892dc9d280257fd271e9597", 768, 0x2440, GID_SQ2, Common::kPlatformCoCo3),
 
 	// Space Quest 2 (CoCo3 360k) [AGI 2.072]
+	// Unofficial port by Guillaume Major
 	GAME_PS("sq2", "updated", "d24f19b047e65e1763eff4b46f3d50df", 768, 0x2440, GID_SQ2, Common::kPlatformCoCo3),
 
 	// Troll's Tale (PC Booter)
@@ -903,6 +931,7 @@ static const AGIGameDescription gameDescriptions[] = {
 	GAME("xmascard", "1986-11-13 [version 1]", "3067b8d5957e2861e069c3c0011bd43d", 0x2272, GID_XMASCARD),
 
 	// Xmas Card 1986 (CoCo3 360k) [AGI 2.072]
+	// Appears to be an unofficial port
 	GAME_PS("xmascard", "", "25ad35e9628fc77e5e0dd35852a272b6", 768, 0x2440, GID_XMASCARD, Common::kPlatformCoCo3),
 
 	FANMADE_FO("2 Player Demo", "4279f46b3cebd855132496476b1d2cca", GF_AGIMOUSE, GAMEOPTIONS_FANMADE_MOUSE),


Commit: 45bbc029b0b05c6fb27f69536d7398a084a36626
    https://github.com/scummvm/scummvm/commit/45bbc029b0b05c6fb27f69536d7398a084a36626
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:40:22-05:00

Commit Message:
AGI: Handle compressed V3 volumes in V2 games (CoCo3)

CoCo3 games use V3 volumes, and most use V3 compression, even though
they are V2 games with V2 directory files. Only KQ3 uses V2 volumes.

LSL1 CoCo3 now starts, along with the many CoCo3 fan ports.

Big thanks to @MusicallyInspired,  @EpicPotatoFiend, and the
SQHistorian discord for teaching me Tandy!

Changed paths:
    engines/agi/loader_v2.cpp


diff --git a/engines/agi/loader_v2.cpp b/engines/agi/loader_v2.cpp
index bcc2e6bac89..4582067c26d 100644
--- a/engines/agi/loader_v2.cpp
+++ b/engines/agi/loader_v2.cpp
@@ -22,6 +22,7 @@
 #include "common/textconsole.h"
 
 #include "agi/agi.h"
+#include "agi/lzw.h"
 #include "agi/words.h"
 
 namespace Agi {
@@ -75,12 +76,13 @@ int AgiLoader_v2::loadDir(AgiDir *agid, const char *fname) {
 /**
  * Detects if the volume format is really V3.
  *
- * The volume format for a V2 game should have 5 byte headers.
- * The CoCo3 version of Xmas Card 86 has 7 byte headers.
- * The resource length repeats as if it were a V3 volume with no compression.
- *
- * This function detects if a volume has this unusual structure so that
- * loadVolRes() can ignore the two extra header bytes.
+ * The CoCo3 version of Leisure Suit Larry uses a V3 volume, even
+ * though it is a V2 game with V2 directory files. Sierra's other
+ * CoCo3 release, King's Quest III, uses regular V2 volumes.
+ * Fan ports of DOS games to CoCo3 use V3 volumes; presumably they
+ * used the Leisure Suit Larry interpreter.
+ * 
+ * Returns true if Logic 0's volume matches the V3 format.
  */
 bool AgiLoader_v2::detectV3VolumeFormat() {
 	uint8 volume = _vm->_game.dirLogic[0].volume;
@@ -92,25 +94,30 @@ bool AgiLoader_v2::detectV3VolumeFormat() {
 
 	// read the first few entries and see if they match the 7 byte header
 	uint8 volumeHeader[7];
-	for (int i = 0; i < 5; i++) {
+	for (int i = 0; i < 10; i++) {
 		if (volumeFile.read(&volumeHeader, 7) != 7) {
 			return false;
 		}
+
 		// signature
 		if (READ_BE_UINT16(volumeHeader) != 0x1234) {
 			return false;
 		}
-		// volume number
-		if (volumeHeader[2] != volume) {
-			return false;
-		}
-		// duplicate resource lengths
-		uint16 resourceLength1 = READ_LE_UINT16(volumeHeader + 3);
-		uint16 resourceLength2 = READ_LE_UINT16(volumeHeader + 5);
-		if (resourceLength1 != resourceLength2) {
+
+		// volume number (high bit == pic compression)
+		if ((volumeHeader[2] & 0x7f) != volume) {
 			return false;
 		}
-		if (!volumeFile.seek(resourceLength1, SEEK_CUR)) {
+
+		// uncompressed and compressed resource length.
+		// we can't validate these values against each other.
+		// uncompressed should always be greater than or equal
+		// to compressed, but fan tools compressed small resources
+		// even when the result was larger than uncompressed.
+		// (Coco3 fan ports of KQ4, MH1, MH2)
+		uint16 compressedResourceLength = READ_LE_UINT16(volumeHeader + 5);
+
+		if (!volumeFile.seek(compressedResourceLength, SEEK_CUR)) {
 			return false;
 		}
 	}
@@ -172,18 +179,35 @@ uint8 *AgiLoader_v2::loadVolRes(struct AgiDir *agid) {
 		fp.seek(agid->offset, SEEK_SET);
 		fp.read(&volumeHeader, _hasV3VolumeFormat ? 7 : 5);
 		uint16 signature = READ_BE_UINT16(volumeHeader);
-		if (signature == 0x1234) {
-			agid->len = READ_LE_UINT16(volumeHeader + 3);
-			data = (uint8 *)calloc(1, agid->len + 32); // why the extra 32 bytes?
-			if (data != nullptr) {
-				fp.read(data, agid->len);
-			} else {
-				error("AgiLoader_v2::loadVolRes out of memory");
-			}
-		} else {
+		if (signature != 0x1234) {
 			warning("AgiLoader_v2::loadVolRes: bad signature %04x", signature);
 			return nullptr;
 		}
+
+		agid->len = READ_LE_UINT16(volumeHeader + 3);
+		if (_hasV3VolumeFormat) {
+			agid->clen = READ_LE_UINT16(volumeHeader + 5);
+		} else {
+			agid->clen = agid->len;
+		}
+
+		uint8 *compBuffer = (uint8 *)calloc(1, agid->clen + 32); // why the extra 32 bytes?
+		fp.read(compBuffer, agid->clen);
+
+		if ((volumeHeader[2] & 0x80) && _hasV3VolumeFormat) { // compressed pic
+			// effectively uncompressed, but having only 4-bit parameters for F0 / F2 commands
+			data = compBuffer;
+			agid->flags |= RES_PICTURE_V3_NIBBLE_PARM;
+		} else if (agid->len == agid->clen) {
+			// do not decompress
+			data = compBuffer;
+		} else {
+			// it is compressed
+			data = (uint8 *)calloc(1, agid->len + 32); // why the extra 32 bytes?
+			lzwExpand(compBuffer, data, agid->len);
+			free(compBuffer);
+			agid->flags |= RES_COMPRESSED;
+		}
 	} else {
 		// we have a bad volume resource
 		// set that resource to NA


Commit: 6848ce8ad2b3165dfccfe7e586f470b83a637f43
    https://github.com/scummvm/scummvm/commit/6848ce8ad2b3165dfccfe7e586f470b83a637f43
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-02-15T21:40:30-05:00

Commit Message:
SCI32: Fix LIGHTHOUSE uninit variable read on VMD playback

Fixes bug #14924

Changed paths:
    engines/sci/engine/workarounds.cpp


diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 2461b2a9566..f28b88e412e 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -423,6 +423,7 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_LAURABOW2,      -1,    90,  1,        "MuseumActor", "init",                         nullptr,     6,     6, { WORKAROUND_FAKE,   0 } }, // Random actors in museum - bug #5197
 	{ GID_LAURABOW2,     240,   240,  0,     "sSteveAnimates", "changeState",                  nullptr,     0,     0, { WORKAROUND_FAKE,   0 } }, // Steve Dorian's idle animation at the docks - bug #5028
 	{ GID_LAURABOW2,      -1,   928,  0,              nullptr, "startText",                    nullptr,     0,     0, { WORKAROUND_FAKE,   0 } }, // gets caused by Text+Audio support (see script patcher)
+	{ GID_LIGHTHOUSE,     -1,     0,  0,          "globalVMD", "play",                         nullptr,     0,     0, { WORKAROUND_FAKE,   0 } }, // when playing a VMD while inventory is disabled (opening a portal in room 360) - bug #14924
 	{ GID_LIGHTHOUSE,     -1,    17,  0,              nullptr, "handleEvent",                  nullptr,     0,     0, { WORKAROUND_FAKE,   0 } }, // when operating the joystick in the puzzle to lower the bridge at the entrance to the workshop, or the joystick that moves the robotic arm in the mini-sub
 	{ GID_LONGBOW,        -1,     0,  0,            "Longbow", "restart",                      nullptr,     0,     0, { WORKAROUND_FAKE,   0 } }, // When canceling a restart game - bug #5244
 	{ GID_LONGBOW,        -1,   213,  0,              "clear", "handleEvent",                  nullptr,     0,     0, { WORKAROUND_FAKE,   0 } }, // When giving an answer using the druid hand sign code in any room




More information about the Scummvm-git-logs mailing list