SHERLOCK: Refactoring, cleanup, and fixes for savegame code

22 files changed, 103 insertions, 43 deletions

diff --git a/engines/sherlock/inventory.cpp b/engines/sherlock/inventory.cpp
index a8ecb64102..7fedab8d37 100644
--- a/engines/sherlock/inventory.cpp
+++ b/engines/sherlock/inventory.cpp
@@ -32,7 +32,7 @@ InventoryItem::InventoryItem(int requiredFlag, const Common::String &name,
 		_examine(examine), _lookFlag(0) {
 }
 
-void InventoryItem::synchronize(Common::Serializer &s) {
+void InventoryItem::synchronize(Serializer &s) {
 	s.syncAsSint16LE(_requiredFlag);
 	s.syncAsSint16LE(_lookFlag);
 	s.syncString(_name);
@@ -450,7 +450,7 @@ int Inventory::deleteItemFromInventory(const Common::String &name) {
 	return 1;
 }
 
-void Inventory::synchronize(Common::Serializer &s) {
+void Inventory::synchronize(Serializer &s) {
 	s.syncAsSint16LE(_holdings);
 
 	uint count = size();
diff --git a/engines/sherlock/inventory.h b/engines/sherlock/inventory.h
index 02f570f5da..a2c317f2a8 100644
--- a/engines/sherlock/inventory.h
+++ b/engines/sherlock/inventory.h
@@ -25,10 +25,10 @@
 
 #include "common/scummsys.h"
 #include "common/array.h"
-#include "common/serializer.h"
 #include "common/str-array.h"
 #include "sherlock/objects.h"
 #include "sherlock/resources.h"
+#include "sherlock/saveload.h"
 
 namespace Sherlock {
 
@@ -69,7 +69,7 @@ struct InventoryItem {
 	/**
 	 * Synchronize the data for an inventory item
 	 */
-	void synchronize(Common::Serializer &s);
+	void synchronize(Serializer &s);
 };
 
 class Inventory : public Common::Array<InventoryItem> {
@@ -163,7 +163,7 @@ public:
 	/**
 	 * Synchronize the data for a savegame
 	 */
-	void synchronize(Common::Serializer &s);
+	void synchronize(Serializer &s);
 };
 
 } // End of namespace Sherlock
diff --git a/engines/sherlock/journal.cpp b/engines/sherlock/journal.cpp
index ab61cd6677..80365a2f50 100644
--- a/engines/sherlock/journal.cpp
+++ b/engines/sherlock/journal.cpp
@@ -1294,7 +1294,7 @@ void Journal::resetPosition() {
 	_page = 1;
 }
 
-void Journal::synchronize(Common::Serializer &s) {
+void Journal::synchronize(Serializer &s) {
 	s.syncAsSint16LE(_index);
 	s.syncAsSint16LE(_sub);
 	s.syncAsSint16LE(_page);
diff --git a/engines/sherlock/journal.h b/engines/sherlock/journal.h
index d62b8338c0..f164562d4f 100644
--- a/engines/sherlock/journal.h
+++ b/engines/sherlock/journal.h
@@ -26,9 +26,9 @@
 #include "common/scummsys.h"
 #include "common/array.h"
 #include "common/rect.h"
-#include "common/serializer.h"
 #include "common/str-array.h"
 #include "common/stream.h"
+#include "sherlock/saveload.h"
 
 namespace Sherlock {
 
@@ -131,7 +131,7 @@ public:
 	/**
 	 * Synchronize the data for a savegame
 	 */
-	void synchronize(Common::Serializer &s);
+	void synchronize(Serializer &s);
 };
 
 } // End of namespace Sherlock
diff --git a/engines/sherlock/map.cpp b/engines/sherlock/map.cpp
index 4e8ac3ecc6..028b387c2f 100644
--- a/engines/sherlock/map.cpp
+++ b/engines/sherlock/map.cpp
@@ -553,18 +553,11 @@ void Map::highlightIcon(const Common::Point &pt) {
 	}
 }
 
-void Map::synchronize(Common::Serializer &s) {
-	s.syncAsSint16LE(_bigPos.x);
-	s.syncAsSint16LE(_bigPos.y);
-
-	Point32 overPos(_overPos.x / FIXED_INT_MULTIPLIER, _overPos.y / FIXED_INT_MULTIPLIER);
-	s.syncAsSint16LE(overPos.x);
-	s.syncAsSint16LE(overPos.y);
-	if (s.isLoading()) {
-		_overPos.x = overPos.x * FIXED_INT_MULTIPLIER;
-		_overPos.y = overPos.y * FIXED_INT_MULTIPLIER;
-	}
-
+void Map::synchronize(Serializer &s) {
+	s.syncAsSint32LE(_bigPos.x);
+	s.syncAsSint32LE(_bigPos.y);
+	s.syncAsSint32LE(_overPos.x);
+	s.syncAsSint16LE(_overPos.y);
 	s.syncAsSint16LE(_oldCharPoint);
 }
 
diff --git a/engines/sherlock/map.h b/engines/sherlock/map.h
index e0c7d038c4..2f789d284a 100644
--- a/engines/sherlock/map.h
+++ b/engines/sherlock/map.h
@@ -26,11 +26,11 @@
 #include "common/scummsys.h"
 #include "common/array.h"
 #include "common/rect.h"
-#include "common/serializer.h"
 #include "common/str.h"
 #include "common/str-array.h"
 #include "sherlock/surface.h"
 #include "sherlock/objects.h"
+#include "sherlock/saveload.h"
 
 namespace Sherlock {
 
@@ -172,7 +172,7 @@ public:
 	/**
 	 * Synchronize the data for a savegame
 	 */
-	void synchronize(Common::Serializer &s);
+	void synchronize(Serializer &s);
 };
 
 } // End of namespace Sherlock
diff --git a/engines/sherlock/people.cpp b/engines/sherlock/people.cpp
index f9e9724302..65b52c55d5 100644
--- a/engines/sherlock/people.cpp
+++ b/engines/sherlock/people.cpp
@@ -662,7 +662,7 @@ void People::clearTalking() {
 	}
 }
 
-void People::synchronize(Common::Serializer &s) {
+void People::synchronize(Serializer &s) {
 	s.syncAsByte(_holmesOn);
 
 	if (IS_SERRATED_SCALPEL) {
diff --git a/engines/sherlock/people.h b/engines/sherlock/people.h
index 4b19d44c7c..c4abf397b2 100644
--- a/engines/sherlock/people.h
+++ b/engines/sherlock/people.h
@@ -24,9 +24,9 @@
 #define SHERLOCK_PEOPLE_H
 
 #include "common/scummsys.h"
-#include "common/serializer.h"
 #include "common/queue.h"
 #include "sherlock/objects.h"
+#include "sherlock/saveload.h"
 
 namespace Sherlock {
 
@@ -189,7 +189,7 @@ public:
 	/**
 	 * Synchronize the data for a savegame
 	 */
-	void synchronize(Common::Serializer &s);
+	virtual void synchronize(Serializer &s) = 0;
 
 	/**
 	 * Change the sequence of the scene background object associated with the current speaker.
diff --git a/engines/sherlock/saveload.cpp b/engines/sherlock/saveload.cpp
index ddf4917a34..c0f1bf1da1 100644
--- a/engines/sherlock/saveload.cpp
+++ b/engines/sherlock/saveload.cpp
@@ -153,7 +153,9 @@ SaveStateList SaveManager::getSavegameList(const Common::String &target) {
 			Common::InSaveFile *in = g_system->getSavefileManager()->openForLoading(*file);
 
 			if (in) {
-				readSavegameHeader(in, header);
+				if (!readSavegameHeader(in, header))
+					continue;
+
 				saveList.push_back(SaveStateDescriptor(slot, header._saveName));
 
 				header._thumbnail->free();
@@ -176,7 +178,7 @@ bool SaveManager::readSavegameHeader(Common::InSaveFile *in, SherlockSavegameHea
 		return false;
 
 	header._version = in->readByte();
-	if (header._version > SHERLOCK_SAVEGAME_VERSION)
+	if (header._version < MINIMUM_SAVEGAME_VERSION || header._version > CURRENT_SAVEGAME_VERSION)
 		return false;
 
 	// Read in the string
@@ -204,7 +206,7 @@ void SaveManager::writeSavegameHeader(Common::OutSaveFile *out, SherlockSavegame
 	// Write out a savegame header
 	out->write(SAVEGAME_STR, SAVEGAME_STR_SIZE + 1);
 
-	out->writeByte(SHERLOCK_SAVEGAME_VERSION);
+	out->writeByte(CURRENT_SAVEGAME_VERSION);
 
 	// Write savegame name
 	out->write(header._saveName.c_str(), header._saveName.size());
@@ -302,7 +304,8 @@ void SaveManager::loadGame(int slot) {
 	}
 
 	// Synchronize the savegame data
-	Common::Serializer s(saveFile, nullptr);
+	Serializer s(saveFile, nullptr);
+	s.setSaveVersion(header._version);
 	synchronize(s);
 
 	delete saveFile;
@@ -317,7 +320,8 @@ void SaveManager::saveGame(int slot, const Common::String &name) {
 	writeSavegameHeader(out, header);
 
 	// Synchronize the savegame data
-	Common::Serializer s(nullptr, out);
+	Serializer s(nullptr, out);
+	s.setSaveVersion(CURRENT_SAVEGAME_VERSION);
 	synchronize(s);
 
 	out->finalize();
@@ -328,7 +332,7 @@ Common::String SaveManager::generateSaveName(int slot) {
 	return Common::String::format("%s.%03d", _target.c_str(), slot);
 }
 
-void SaveManager::synchronize(Common::Serializer &s) {
+void SaveManager::synchronize(Serializer &s) {
 	Inventory &inv = *_vm->_inventory;
 	Journal &journal = *_vm->_journal;
 	Map &map = *_vm->_map;
diff --git a/engines/sherlock/saveload.h b/engines/sherlock/saveload.h
index a7ed852a5f..49ccc508ef 100644
--- a/engines/sherlock/saveload.h
+++ b/engines/sherlock/saveload.h
@@ -34,7 +34,11 @@ namespace Sherlock {
 
 #define MAX_SAVEGAME_SLOTS 99
 #define ONSCREEN_FILES_COUNT 5
-#define SHERLOCK_SAVEGAME_VERSION 1
+
+enum {
+	CURRENT_SAVEGAME_VERSION = 2,
+	MINIMUM_SAVEGAME_VERSION = 2
+};
 
 enum SaveMode { SAVEMODE_NONE = 0, SAVEMODE_LOAD = 1, SAVEMODE_SAVE = 2 };
 
@@ -51,6 +55,20 @@ struct SherlockSavegameHeader {
 
 class SherlockEngine;
 
+
+/**
+ * Derived serializer class with extra synchronization types
+ */
+class Serializer : public Common::Serializer {
+public:
+	Serializer(Common::SeekableReadStream *in, Common::WriteStream *out) : Common::Serializer(in, out) {}
+
+	/**
+	 * New method to allow setting the version
+	 */
+	void setSaveVersion(byte version) { _version = version; }
+};
+
 class SaveManager {
 private:
 	SherlockEngine *_vm;
@@ -65,7 +83,7 @@ private:
 	/**
 	 * Synchronize the data for a savegame
 	 */
-	void synchronize(Common::Serializer &s);
+	void synchronize(Serializer &s);
 public:
 	Common::StringArray _savegames;
 	int _savegameIndex;
diff --git a/engines/sherlock/scalpel/scalpel_people.cpp b/engines/sherlock/scalpel/scalpel_people.cpp
index 08100fef11..2b76eea55e 100644
--- a/engines/sherlock/scalpel/scalpel_people.cpp
+++ b/engines/sherlock/scalpel/scalpel_people.cpp
@@ -86,6 +86,20 @@ void ScalpelPeople::setTalking(int speaker) {
 	}
 }
 
+
+void ScalpelPeople::synchronize(Serializer &s) {
+	s.syncAsByte(_holmesOn);
+	s.syncAsSint32LE(_player._position.x);
+	s.syncAsSint32LE(_player._position.y);
+	s.syncAsSint16LE(_player._sequenceNumber);
+	s.syncAsSint16LE(_holmesQuotient);
+
+	if (s.isLoading()) {
+		_hSavedPos = _player._position;
+		_hSavedFacing = _player._sequenceNumber;
+	}
+}
+
 void ScalpelPeople::setTalkSequence(int speaker, int sequenceNum) {
 	People &people = *_vm->_people;
 	Scene &scene = *_vm->_scene;
diff --git a/engines/sherlock/scalpel/scalpel_people.h b/engines/sherlock/scalpel/scalpel_people.h
index b2267790e0..6981157103 100644
--- a/engines/sherlock/scalpel/scalpel_people.h
+++ b/engines/sherlock/scalpel/scalpel_people.h
@@ -52,6 +52,11 @@ public:
 	void setTalking(int speaker);
 
 	/**
+	 * Synchronize the data for a savegame
+	 */
+	virtual void synchronize(Serializer &s);
+
+	/**
 	 * Change the sequence of the scene background object associated with the specified speaker.
 	 */
 	virtual void setTalkSequence(int speaker, int sequenceNum = 1);
diff --git a/engines/sherlock/scene.cpp b/engines/sherlock/scene.cpp
index 1b0d89be54..c43887438e 100644
--- a/engines/sherlock/scene.cpp
+++ b/engines/sherlock/scene.cpp
@@ -1193,7 +1193,7 @@ int Scene::closestZone(const Common::Point &pt) {
 	return zone;
 }
 
-void Scene::synchronize(Common::Serializer &s) {
+void Scene::synchronize(Serializer &s) {
 	if (s.isSaving())
 		saveSceneStatus();
 
diff --git a/engines/sherlock/scene.h b/engines/sherlock/scene.h
index 70531a7173..11fd1da149 100644
--- a/engines/sherlock/scene.h
+++ b/engines/sherlock/scene.h
@@ -300,7 +300,7 @@ public:
 	/**
 	 * Synchronize the data for a savegame
 	 */
-	void synchronize(Common::Serializer &s);
+	void synchronize(Serializer &s);
 
 	/**
 	 * Resets the NPC path information when entering a new scene.
diff --git a/engines/sherlock/screen.cpp b/engines/sherlock/screen.cpp
index f0c993a708..de93d0aec3 100644
--- a/engines/sherlock/screen.cpp
+++ b/engines/sherlock/screen.cpp
@@ -485,7 +485,7 @@ Common::Rect Screen::getDisplayBounds() {
 		Common::Rect(0, 0, this->w(), this->h());
 }
 
-void Screen::synchronize(Common::Serializer &s) {
+void Screen::synchronize(Serializer &s) {
 	int fontNumb = _fontNumber;
 	s.syncAsByte(fontNumb);
 	if (s.isLoading())
diff --git a/engines/sherlock/screen.h b/engines/sherlock/screen.h
index 75df51f13d..9d394da3c8 100644
--- a/engines/sherlock/screen.h
+++ b/engines/sherlock/screen.h
@@ -25,9 +25,9 @@
 
 #include "common/list.h"
 #include "common/rect.h"
-#include "common/serializer.h"
 #include "sherlock/surface.h"
 #include "sherlock/resources.h"
+#include "sherlock/saveload.h"
 
 namespace Sherlock {
 
@@ -265,7 +265,7 @@ public:
 	/**
 	 * Synchronize the data for a savegame
 	 */
-	void synchronize(Common::Serializer &s);
+	void synchronize(Serializer &s);
 
 	// Rose Tattoo specific methods
 	void initPaletteFade(int bytesToRead);
diff --git a/engines/sherlock/sherlock.cpp b/engines/sherlock/sherlock.cpp
index cbfc2f1864..27778db699 100644
--- a/engines/sherlock/sherlock.cpp
+++ b/engines/sherlock/sherlock.cpp
@@ -244,7 +244,7 @@ void SherlockEngine::syncSoundSettings() {
 	_music->syncMusicSettings();
 }
 
-void SherlockEngine::synchronize(Common::Serializer &s) {
+void SherlockEngine::synchronize(Serializer &s) {
 	for (uint idx = 0; idx < _flags.size(); ++idx)
 		s.syncAsByte(_flags[idx]);
 }
diff --git a/engines/sherlock/sherlock.h b/engines/sherlock/sherlock.h
index 66b0313fb3..6ed1ef579f 100644
--- a/engines/sherlock/sherlock.h
+++ b/engines/sherlock/sherlock.h
@@ -207,7 +207,7 @@ public:
 	/**
 	 * Synchronize the data for a savegame
 	 */
-	void synchronize(Common::Serializer &s);
+	void synchronize(Serializer &s);
 };
 
 #define IS_ROSE_TATTOO (_vm->getGameID() == GType_RoseTattoo)
diff --git a/engines/sherlock/talk.cpp b/engines/sherlock/talk.cpp
index 3a009e9160..dff1250475 100644
--- a/engines/sherlock/talk.cpp
+++ b/engines/sherlock/talk.cpp
@@ -1184,7 +1184,7 @@ void Talk::popStack() {
 	}
 }
 
-void Talk::synchronize(Common::Serializer &s) {
+void Talk::synchronize(Serializer &s) {
 	for (uint idx = 0; idx < _talkHistory.size(); ++idx) {
 		TalkHistoryEntry &he = _talkHistory[idx];
 
diff --git a/engines/sherlock/talk.h b/engines/sherlock/talk.h
index c6fda0b5d7..bbe3e86a48 100644
--- a/engines/sherlock/talk.h
+++ b/engines/sherlock/talk.h
@@ -26,10 +26,10 @@
 #include "common/scummsys.h"
 #include "common/array.h"
 #include "common/rect.h"
-#include "common/serializer.h"
 #include "common/stream.h"
 #include "common/stack.h"
 #include "sherlock/objects.h"
+#include "sherlock/saveload.h"
 
 namespace Sherlock {
 
@@ -359,7 +359,7 @@ public:
 	/**
 	 * Synchronize the data for a savegame
 	 */
-	void synchronize(Common::Serializer &s);
+	void synchronize(Serializer &s);
 };
 
 } // End of namespace Sherlock
diff --git a/engines/sherlock/tattoo/tattoo_people.cpp b/engines/sherlock/tattoo/tattoo_people.cpp
index 4e4f11b983..6952c876c0 100644
--- a/engines/sherlock/tattoo/tattoo_people.cpp
+++ b/engines/sherlock/tattoo/tattoo_people.cpp
@@ -200,6 +200,28 @@ void TattooPeople::setTalkSequence(int speaker, int sequenceNum) {
 	}
 }
 
+void TattooPeople::synchronize(Serializer &s) {
+	s.syncAsByte(_holmesOn);
+
+	for (int idx = 0; idx < MAX_CHARACTERS; ++idx) {
+		Person &p = _data[idx];
+		s.syncAsSint32LE(p._position.x);
+		s.syncAsSint32LE(p._position.y);
+		s.syncAsSint16LE(p._sequenceNumber);
+		s.syncAsSint16LE(p._type);
+		s.syncString(p._walkVGSName);
+		s.syncString(p._description);
+		s.syncString(p._examine);
+	}
+
+	s.syncAsSint16LE(_holmesQuotient);
+
+	if (s.isLoading()) {
+		_hSavedPos = _player._position;
+		_hSavedFacing = _player._sequenceNumber;
+	}
+}
+
 } // End of namespace Tattoo
 
 } // End of namespace Sherlock
diff --git a/engines/sherlock/tattoo/tattoo_people.h b/engines/sherlock/tattoo/tattoo_people.h
index 481ce7804a..c4f0bfd3ed 100644
--- a/engines/sherlock/tattoo/tattoo_people.h
+++ b/engines/sherlock/tattoo/tattoo_people.h
@@ -87,6 +87,10 @@ public:
 	 */
 	void setListenSequence(int speaker, int sequenceNum);
 
+	/**
+	 * Synchronize the data for a savegame
+	 */
+	virtual void synchronize(Serializer &s);
 
 	/**
 	 * Change the sequence of the scene background object associated with the specified speaker.