[Scummvm-git-logs] scummvm master -> bc8550ce02a036e3757d817e963e038463844412

sluicebox noreply at scummvm.org
Tue Jan 23 01:13:12 UTC 2024


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

Summary:
6ef496532e AGI: Use parameter names in function declarations
c0433ab956 AGI: Remove unused unload code
8c5b6c032e AGI: Rewrite logic decoding for clarity, OOB fixes
be698249f1 AGI: Cleanup loader code
8f2127e418 AGI: Cleanup graphics code
bc8550ce02 AGI: Allow message box to be drawn over menu


Commit: 6ef496532e1daf29ddeb80610afa0d0e15c9d216
    https://github.com/scummvm/scummvm/commit/6ef496532e1daf29ddeb80610afa0d0e15c9d216
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-01-22T17:32:45-07: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 2dae7fad816..93fdf9f3bfe 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 c52e14e4418..1cced30f22b 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: c0433ab956cda4c06d217d0ca0537e1e6db97c61
    https://github.com/scummvm/scummvm/commit/c0433ab956cda4c06d217d0ca0537e1e6db97c61
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-01-22T17:32:45-07: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 93fdf9f3bfe..fb0e32767a3 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 65c7cc9bb3c..37b72bca9f0 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 33c278479a4..f9bc3aa6d8b 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: 8c5b6c032edbed4eea65e1bdc0df6d7cf1cba5ac
    https://github.com/scummvm/scummvm/commit/8c5b6c032edbed4eea65e1bdc0df6d7cf1cba5ac
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-01-22T17:32:45-07: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: be698249f1650a2b9632e39b8ccbfe03d8ab6e70
    https://github.com/scummvm/scummvm/commit/be698249f1650a2b9632e39b8ccbfe03d8ab6e70
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-01-22T17:32:46-07: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.

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..c6441a4d416 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.toString().c_str());
+			return nullptr;
+		}
 		offset -= IMAGE_SIZE;
 	} else {
-		fp.open(_filenameDisk0);
+		if (!fp.open(_filenameDisk0)) {
+			warning("AgiLoader_v1::loadVolRes: could not open %s", _filenameDisk0.toString().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 37b72bca9f0..7b3e40d204f 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::Path path(Common::String::format("vol.%i", agid->volume));
 
 	debugC(3, kDebugLevelResources, "Vol res: path = %s", path.toString().c_str());
@@ -182,19 +175,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 f9bc3aa6d8b..b45ca86d12b 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.getPath("path"));
 
 	if (!dir.getChildren(fslist, Common::FSNode::kListFilesOnly)) {
-		warning("AgiEngine: invalid game path '%s'", dir.getPath().toString(Common::Path::kNativeSeparator).c_str());
+		warning("AgiLoader_v3: invalid game path '%s'", dir.getPath().toString(Common::Path::kNativeSeparator).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::Path 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::Path 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.toString().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: 8f2127e4184eb871192c68d02fa66880b8180dde
    https://github.com/scummvm/scummvm/commit/8f2127e4184eb871192c68d02fa66880b8180dde
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-01-22T17:32:46-07: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 a5a0cfe4a01..1e0b760c4c0 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: bc8550ce02a036e3757d817e963e038463844412
    https://github.com/scummvm/scummvm/commit/bc8550ce02a036e3757d817e963e038463844412
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-01-22T18:06:40-07: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 1e0b760c4c0..10afc08d3c0 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();




More information about the Scummvm-git-logs mailing list