Merge pull request #873 from wjp/sci_save_palvary

SCI: Unconditionally save palvary state

3 files changed, 23 insertions, 5 deletions

diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 720f6783ee..5184874064 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -764,17 +764,33 @@ void GfxPalette::saveLoadWithSerializer(Common::Serializer &s) {
 			palVaryRemoveTimer();
 
 		s.syncAsSint32LE(_palVaryResourceId);
-		if (_palVaryResourceId != -1) {
-			palVarySaveLoadPalette(s, &_palVaryOriginPalette);
-			palVarySaveLoadPalette(s, &_palVaryTargetPalette);
+		if (_palVaryResourceId != -1 || s.getVersion() >= 40) {
+			if (_palVaryResourceId != -1) {
+				palVarySaveLoadPalette(s, &_palVaryOriginPalette);
+				palVarySaveLoadPalette(s, &_palVaryTargetPalette);
+			}
 			s.syncAsSint16LE(_palVaryStep);
 			s.syncAsSint16LE(_palVaryStepStop);
 			s.syncAsSint16LE(_palVaryDirection);
 			s.syncAsUint16LE(_palVaryTicks);
 			s.syncAsSint32LE(_palVaryPaused);
+			if (s.getVersion() >= 40)
+				s.syncAsSint32LE(_palVarySignal);
 		}
 
-		_palVarySignal = 0;
+		if (s.isLoading() && s.getVersion() < 40) {
+			// Reset _palVaryPaused to 0 when loading an old savegame.
+			// Before version 40, we didn't restore or reset _palVaryPaused.
+			// In QfG3 this could get it stuck at positive values (bug #9674).
+			//
+			// Other SCI11 games don't appear to use palVaryPaused at all.
+			// (Looked at eq2, freddy, kq6, lb2, mgoose11, pq1, qg1, sq4, sq5)
+			_palVaryPaused = 0;
+
+			// Clear any pending updates, since _palVarySignal also wasn't saved
+			// before version 40.
+			_palVarySignal = 0;
+		}
 
 		if (s.isLoading() && _palVaryResourceId != -1) {
 			palVaryInstallTimer();
diff --git a/engines/sci/engine/savegame.h b/engines/sci/engine/savegame.h
index 873394aebb..a75db25bf9 100644
--- a/engines/sci/engine/savegame.h
+++ b/engines/sci/engine/savegame.h
@@ -37,6 +37,7 @@ struct EngineState;
  *
  * Version - new/changed feature
  * =============================
+ *      40 - always store palvary variables
  *      39 - Accurate SCI32 arrays/strings, score metadata, avatar metadata
  *      38 - SCI32 cursor
  *      37 - Segment entry data changed to pointers
@@ -64,7 +65,7 @@ struct EngineState;
  */
 
 enum {
-	CURRENT_SAVEGAME_VERSION = 39,
+	CURRENT_SAVEGAME_VERSION = 40,
 	MINIMUM_SAVEGAME_VERSION = 14
 };
 
diff --git a/engines/sci/graphics/palette.cpp b/engines/sci/graphics/palette.cpp
index 1514ad838f..6fe1fb1b49 100644
--- a/engines/sci/graphics/palette.cpp
+++ b/engines/sci/graphics/palette.cpp
@@ -852,6 +852,7 @@ void GfxPalette::palVaryCallback(void *refCon) {
 }
 
 void GfxPalette::palVaryIncreaseSignal() {
+	// FIXME: increments from another thread aren't guaranteed to be atomic
 	if (!_palVaryPaused)
 		_palVarySignal++;
 }