[Scummvm-git-logs] scummvm master -> 438bdce4542332e8abaf01e160aeea72e26781e4

[AndywinXp]

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

Summary:
438bdce454 SCUMM: Implement proper v4 palette cycling code


Commit: 438bdce4542332e8abaf01e160aeea72e26781e4
    https://github.com/scummvm/scummvm/commit/438bdce4542332e8abaf01e160aeea72e26781e4
Author: AndywinXp (andywinxp at gmail.com)
Date: 2023-06-12T00:26:02+02:00

Commit Message:
SCUMM: Implement proper v4 palette cycling code

This commit properly takes care of #10854 and #1016;
the previous code was retrofitted from v5 and thus potentially
presented inaccuracies (e.g. room 1 in LOOM CD shouldn't
cycle colors).

A post-load fix was also necessary for older savegames,
otherwise color cycling would not occur at all until the room
was exited and then re-entered. Being that the cycling data
was being mangled by the previous implementation, there was
no choice but to introduce a new save version to detect this.

Changed paths:
    engines/scumm/palette.cpp
    engines/scumm/saveload.cpp


diff --git a/engines/scumm/palette.cpp b/engines/scumm/palette.cpp
index b8fc627cb0d..50c230f91a5 100644
--- a/engines/scumm/palette.cpp
+++ b/engines/scumm/palette.cpp
@@ -590,29 +590,20 @@ void ScummEngine::initCycl(const byte *ptr) {
 	memset(_colorCycle, 0, sizeof(_colorCycle));
 
 	if (_game.features & GF_SMALL_HEADER) {
+		// Code verified from LOOM CD and MI1 VGA disasms, which
+		// are the only executables which contain said code at all
+
 		cycl = _colorCycle;
 		for (j = 0; j < 16; ++j, ++cycl) {
-			uint16 delay = READ_BE_UINT16(ptr);
+			cycl->delay = READ_BE_UINT16(ptr);
 			ptr += 2;
-			byte start = *ptr++;
-			byte end = *ptr++;
-
-			if (!delay || delay == 0x0aaa || start >= end)
-				continue;
 
 			cycl->counter = 0;
-			cycl->delay = 16384 / delay;
-			cycl->flags = 2;
-			// FIXME bug #10854: lava flows up instead of down in the floppy VGA version
-			// of Monkey1 if we don't do this. It's fine in the original interpreter and
-			// in the VGA CD version (which doesn't use GF_SMALL_HEADER). This is maybe
-			// meant to be always 0 for GF_SMALL_HEADER games, but until disasm confirms
-			// or disproves this, we limit this change to the lava cave (where we can see
-			// it's wrong), just in case.
-			if (_game.id == GID_MONKEY_VGA && (_roomResource == 39 || _roomResource == 65))
-				cycl->flags = 0;
-			cycl->start = start;
-			cycl->end = end;
+			cycl->start = *ptr++;
+			cycl->end = *ptr++;
+
+			if (cycl->delay && cycl->delay != 0x0AAA)
+				cycl->counter = cycl->start;
 		}
 	} else {
 		memset(_colorUsedByCycle, 0, sizeof(_colorUsedByCycle));
@@ -734,6 +725,35 @@ void ScummEngine::cyclePalette() {
 	int valueToAdd;
 	int i, j;
 
+	if (_game.features & GF_SMALL_HEADER) {
+		// Code verified from LOOM CD and MI1 VGA disasms, which
+		// are the only executables which contain said code at all
+
+		for (i = 0; i < 16; i++) {
+			if (_colorCycle[i].counter) {
+				_colorCycle[i].counter++;
+				if (_colorCycle[i].counter > _colorCycle[i].end)
+					_colorCycle[i].counter = _colorCycle[i].start;
+
+				byte start = _colorCycle[i].start;
+				byte end = _colorCycle[i].end;
+
+				if (start <= end) {
+					byte cycleVal = _colorCycle[i].counter;
+					for (j = start; j <= end; j++) {
+						_shadowPalette[j] = cycleVal--;
+						if (cycleVal < start)
+							cycleVal = end;
+					}
+				}
+
+				setDirtyColors(_colorCycle[i].start, _colorCycle[i].end);
+				moveMemInPalRes(_colorCycle[i].start, _colorCycle[i].end, 0);
+			}
+		}
+		return;
+	}
+
 #ifndef DISABLE_TOWNS_DUAL_LAYER_MODE
 	if (_game.platform == Common::kPlatformFMTowns && !(_townsPaletteFlags & 1))
 		return;
diff --git a/engines/scumm/saveload.cpp b/engines/scumm/saveload.cpp
index ca42ec95a28..4fd9794af48 100644
--- a/engines/scumm/saveload.cpp
+++ b/engines/scumm/saveload.cpp
@@ -68,7 +68,7 @@ struct SaveInfoSection {
 
 #define SaveInfoSectionSize (4+4+4 + 4+4 + 4+2)
 
-#define CURRENT_VER 108
+#define CURRENT_VER 109
 #define INFOSECTION_VERSION 2
 
 #pragma mark -
@@ -1810,6 +1810,23 @@ void ScummEngine::saveLoadWithSerializer(Common::Serializer &s) {
 		}
 	}
 
+	// Before version 109, palette cycling for v4 games was handled in a different
+	// way (which is, by retrofitting v5 code, which caused a class of bugs like #10854).
+	// The proper v4 code has now been implemented from disasm (specifically, only the
+	// LOOM CD and MI1 VGA executables have said code).
+	//
+	// Given that the previous implementation mangled the cycling data during the init
+	// phase, we have to resort to the following post-load fix, otherwise the color
+	// cycling will not occur on game load.
+	if (_game.version == 4 && (_game.id == GID_LOOM || _game.id == GID_MONKEY_VGA) &&
+		s.getVersion() < VER(109)) {
+		byte *roomptr = getResourceAddress(rtRoom, _roomResource);
+		const byte *ptr = findResourceData(MKTAG('C', 'Y', 'C', 'L'), roomptr);
+		if (ptr) {
+			initCycl(ptr);
+		}
+	}
+
 	//
 	// Save/load more global object state
 	//