SCI: Change workaround for PalVary / Animate race condition

The new approach is to delay kAnimate briefly (with an 68ms timeout)
while there is a zero-tick PalVary running, so that it has time to
trigger.

The previous workaround would immediately process a zero-tick
PalVaryInit/PalVaryReverse. This caused problems in QfG3 (bug #10304)
where it interfered with PalVaryPause.

The previous workaround could also be modified to handle pause/resume,
but this new approach should be closer to SSCI's behaviour, which used a
timer for a zero-tick PalVary too.

This fixes bug #10304, and keeps #5298 fixed too.

3 files changed, 49 insertions, 18 deletions

diff --git a/engines/sci/graphics/animate.cpp b/engines/sci/graphics/animate.cpp
index 317e98feab..8875162394 100644
--- a/engines/sci/graphics/animate.cpp
+++ b/engines/sci/graphics/animate.cpp
@@ -650,6 +650,11 @@ void GfxAnimate::animateShowPic() {
 }
 
 void GfxAnimate::kernelAnimate(reg_t listReference, bool cycle, int argc, reg_t *argv) {
+	// If necessary, delay this kAnimate for a running PalVary.
+	// See delayForPalVaryWorkaround() for details.
+	if (_screen->_picNotValid)
+		_palette->delayForPalVaryWorkaround();
+
 	byte old_picNotValid = _screen->_picNotValid;
 
 	if (getSciVersion() >= SCI_VERSION_1_1)
diff --git a/engines/sci/graphics/palette.cpp b/engines/sci/graphics/palette.cpp
index e36028d984..bc0b8485d2 100644
--- a/engines/sci/graphics/palette.cpp
+++ b/engines/sci/graphics/palette.cpp
@@ -716,6 +716,7 @@ void GfxPalette::palVaryInit() {
 	_palVaryStepStop = 0;
 	_palVaryDirection = 0;
 	_palVaryTicks = 0;
+	_palVaryZeroTick = false;
 }
 
 bool GfxPalette::palVaryLoadTargetPalette(GuiResourceId resourceId) {
@@ -759,19 +760,13 @@ bool GfxPalette::kernelPalVaryInit(GuiResourceId resourceId, uint16 ticks, uint1
 		_palVaryStep = 1;
 		_palVaryStepStop = stepStop;
 		_palVaryDirection = direction;
+
 		// if no ticks are given, jump directly to destination
-		if (!_palVaryTicks) {
+		if (!_palVaryTicks)
 			_palVaryDirection = stepStop;
-			// sierra sci set the timer to 1 tick instead of calling it directly
-			//  we have to change this to prevent a race condition to happen in
-			//  at least freddy pharkas during nighttime. In that case kPalVary is
-			//  called right before a transition and because we load pictures much
-			//  faster, the 1 tick won't pass sometimes resulting in the palette
-			//  being daytime instead of nighttime during the transition.
-			palVaryProcess(1, true);
-		} else {
-			palVaryInstallTimer();
-		}
+		_palVaryZeroTick = (_palVaryTicks == 0); //see delayForPalVaryWorkaround()
+
+		palVaryInstallTimer();
 		return true;
 	}
 	return false;
@@ -788,14 +783,13 @@ int16 GfxPalette::kernelPalVaryReverse(int16 ticks, uint16 stepStop, int16 direc
 	_palVaryStepStop = stepStop;
 	_palVaryDirection = direction != -1 ? -direction : -_palVaryDirection;
 
-	if (!_palVaryTicks) {
+	// if no ticks are given, jump directly to destination
+	if (!_palVaryTicks)
 		_palVaryDirection = _palVaryStepStop - _palVaryStep;
-		// see palVaryInit above, we fix the code here as well
-		//  just in case
-		palVaryProcess(1, true);
-	} else {
-		palVaryInstallTimer();
-	}
+	_palVaryZeroTick = (_palVaryTicks == 0); // see delayForPalVaryWorkaround()
+
+	palVaryInstallTimer();
+
 	return kernelPalVaryGetCurrentStep();
 }
 
@@ -855,6 +849,7 @@ void GfxPalette::palVaryIncreaseSignal() {
 	// FIXME: increments from another thread aren't guaranteed to be atomic
 	if (!_palVaryPaused)
 		_palVarySignal++;
+	_palVaryZeroTick = false;
 }
 
 // Actually do the pal vary processing
@@ -865,6 +860,34 @@ void GfxPalette::palVaryUpdate() {
 	}
 }
 
+void GfxPalette::delayForPalVaryWorkaround() {
+	if (_palVaryResourceId == -1)
+		return;
+	if (_palVaryPaused)
+		return;
+
+	// This gets called at the very beginning of kAnimate.
+	// If a zero-tick palVary is running, we delay briefly to give the
+	// palVary time to trigger. In theory there should be no reason for this
+	// to have to wait more than a tick, but we time-out after 4 ticks
+	// to be on the safe side.
+	//
+	// This prevents a race condition in Freddy Pharkas during nighttime,
+	// since we load pictures much faster than on original hardware (bug #5298).
+
+	if (_palVaryZeroTick) {
+		int i;
+		for (i = 0; i < 4; ++i) {
+			g_sci->sleep(17);
+			if (!_palVaryZeroTick)
+				break;
+		}
+		debugC(kDebugLevelGraphics, "Delayed kAnimate for kPalVary, %d times", i+1);
+		if (_palVaryZeroTick)
+			warning("Delayed kAnimate for kPalVary timed out");
+	}
+}
+
 void GfxPalette::palVaryPrepareForTransition() {
 	if (_palVaryResourceId != -1) {
 		// Before doing transitions, we have to prepare palette
diff --git a/engines/sci/graphics/palette.h b/engines/sci/graphics/palette.h
index af74169976..2923df9220 100644
--- a/engines/sci/graphics/palette.h
+++ b/engines/sci/graphics/palette.h
@@ -88,6 +88,8 @@ public:
 	void palVaryPrepareForTransition();
 	void palVaryProcess(int signal, bool setPalette);
 
+	void delayForPalVaryWorkaround();
+
 	Palette _sysPalette;
 
 	void saveLoadWithSerializer(Common::Serializer &s);
@@ -122,6 +124,7 @@ protected:
 	uint16 _palVaryTicks;
 	int _palVaryPaused;
 	int _palVarySignal;
+	bool _palVaryZeroTick;
 	uint16 _totalScreenColors;
 
 	void loadMacIconBarPalette();