Fix and simplify game saving / loading

Global.h

@@ -247,8 +247,6 @@ using TLockGuardRec = std::lock_guard<std::recursive_mutex>;
 #  define DLL_LINKAGE DLL_IMPORT
 #endif
 
-#define THROW_FORMAT(message, formatting_elems)  throw std::runtime_error(boost::str(boost::format(message) % formatting_elems))
-
 // old iOS SDKs compatibility
 #ifdef VCMI_IOS
 #include <AvailabilityVersions.h>

client/windows/CMapOverview.cpp

@@ -160,16 +160,15 @@ CMapOverviewWidget::CMapOverviewWidget(CMapOverview& parent):
 		std::unique_ptr<CMap> campaignMap = nullptr;
 		if(p.tabType != ESelectionScreen::newGame && config["variables"]["mapPreviewForSaves"].Bool())
 		{
-			CLoadFile lf(*CResourceHandler::get()->getResourceName(ResourcePath(p.resource.getName(), EResType::SAVEGAME)), ESerializationVersion::MINIMAL);
-			lf.checkMagicBytes(SAVEGAME_MAGIC);
-
-			auto mapHeader = std::make_unique<CMapHeader>();
-			std::unique_ptr<StartInfo> startInfo;
-			lf >> *(mapHeader) >> startInfo;
-
-			if(startInfo->campState)
-				campaignMap = startInfo->campState->getMap(*startInfo->campState->currentScenario(), nullptr);
-			res = ResourcePath(startInfo->fileURI, EResType::MAP);
+			CLoadFile lf(*CResourceHandler::get()->getResourceName(ResourcePath(p.resource.getName(), EResType::SAVEGAME)), nullptr);
+			CMapHeader mapHeader;
+			StartInfo startInfo;
+			lf.load(mapHeader);
+			lf.load(startInfo);
+
+			if(startInfo.campState)
+				campaignMap = startInfo.campState->getMap(*startInfo.campState->currentScenario(), nullptr);
+			res = ResourcePath(startInfo.fileURI, EResType::MAP);
 		}
 		if(!campaignMap)
 			minimaps = createMinimaps(res);

lib/CGameInterface.h

@@ -51,8 +51,6 @@ class CStackInstance;
 class CCommanderInstance;
 class CStack;
 class CCreature;
-class CLoadFile;
-class CSaveFile;
 class BattleStateInfo;
 struct ArtifactLocation;
 class BattleStateInfoForRetreat;

lib/IGameCallback.cpp

@@ -22,7 +22,6 @@
 #include "entities/hero/CHero.h"
 #include "networkPacks/ArtifactLocation.h"
 #include "serializer/CLoadFile.h"
-#include "serializer/CSaveFile.h"
 #include "rmg/CMapGenOptions.h"
 #include "mapObjectConstructors/AObjectTypeHandler.h"
 #include "mapObjectConstructors/CObjectClassesHandler.h"
@@ -175,46 +174,6 @@ void CPrivilegedInfoCallback::getAllowedSpells(std::vector<SpellID> & out, std::
 	}
 }
 
-void CPrivilegedInfoCallback::loadCommonState(CLoadFile & in)
-{
-	logGlobal->info("Loading lib part of game...");
-	in.checkMagicBytes(SAVEGAME_MAGIC);
-
-	CMapHeader dum;
-	StartInfo si;
-	ActiveModsInSaveList activeMods;
-	CGameState & gs = *gameState();
-
-	logGlobal->info("\tReading header");
-	in.serializer & dum;
-
-	logGlobal->info("\tReading options");
-	in.serializer & si;
-
-	logGlobal->info("\tReading mod list");
-	in.serializer & activeMods;
-
-	logGlobal->info("\tReading gamestate");
-	in.serializer & gs;
-}
-
-void CPrivilegedInfoCallback::saveCommonState(CSaveFile & out) const
-{
-	ActiveModsInSaveList activeMods;
-	const CGameState & gs = *gameState();
-
-	logGlobal->info("Saving lib part of game...");
-	out.putMagicBytes(SAVEGAME_MAGIC);
-	logGlobal->info("\tSaving header");
-	out.serializer & static_cast<const CMapHeader&>(gs.getMap());
-	logGlobal->info("\tSaving options");
-	out.serializer & *gs.getStartInfo();
-	logGlobal->info("\tSaving mod list");
-	out.serializer & activeMods;
-	logGlobal->info("\tSaving gamestate");
-	out.serializer & gs;
-}
-
 TerrainTile * CNonConstInfoCallback::getTile(const int3 & pos)
 {
 	if(!gameState()->getMap().isInTheMap(pos))

lib/IGameCallback.h

@@ -32,8 +32,6 @@ struct BattleLayout;
 class CCreatureSet;
 class CStackBasicDescriptor;
 class CGCreature;
-class CSaveFile;
-class CLoadFile;
 class IObjectInterface;
 enum class EOpenWindowMode : uint8_t;
 
@@ -77,9 +75,6 @@ public:
 	//gives 3 treasures, 3 minors, 1 major -> used by Black Market and Artifact Merchant
 	void pickAllowedArtsSet(std::vector<ArtifactID> & out, vstd::RNG & rand);
 	void getAllowedSpells(std::vector<SpellID> &out, std::optional<ui16> level = std::nullopt);
-
-	void saveCommonState(CSaveFile &out) const; //stores GS
-	void loadCommonState(CLoadFile &in); //loads GS
 };
 
 class DLL_LINKAGE IGameEventCallback

lib/gameState/CGameState.cpp

@@ -15,8 +15,10 @@
 #include "TavernHeroesPool.h"
 #include "CGameStateCampaign.h"
 #include "SThievesGuildInfo.h"
+#include "QuestInfo.h"
 
 #include "../ArtifactUtils.h"
+#include "../GameSettings.h"
 #include "../texts/CGeneralTextHandler.h"
 #include "../CPlayerState.h"
 #include "../CStopWatch.h"
@@ -25,6 +27,9 @@
 #include "../TerrainHandler.h"
 #include "../VCMIDirs.h"
 #include "../GameLibrary.h"
+#include "../bonuses/Limiters.h"
+#include "../bonuses/Propagators.h"
+#include "../bonuses/Updaters.h"
 #include "../battle/BattleInfo.h"
 #include "../campaign/CampaignState.h"
 #include "../constants/StringConstants.h"
@@ -44,6 +49,7 @@
 #include "../mapping/CMap.h"
 #include "../mapping/CMapEditManager.h"
 #include "../mapping/CMapService.h"
+#include "../modding/ActiveModsInSaveList.h"
 #include "../modding/IdentifierStorage.h"
 #include "../modding/ModScope.h"
 #include "../networkPacks/NetPacksBase.h"
@@ -51,6 +57,8 @@
 #include "../pathfinder/PathfinderOptions.h"
 #include "../rmg/CMapGenerator.h"
 #include "../serializer/CMemorySerializer.h"
+#include "../serializer/CLoadFile.h"
+#include "../serializer/CSaveFile.h"
 #include "../spells/CSpellHandler.h"
 #include "UpgradeInfo.h"
 
@@ -1737,4 +1745,28 @@ CArtifactInstance * CGameState::createArtifact(const ArtifactID & artID, const S
 	return map->createArtifact(artID, spellId);
 }
 
+void CGameState::saveGame(CSaveFile & file) const
+{
+	ActiveModsInSaveList activeModsDummy;
+	logGlobal->info("Saving game state");
+	file.save(*getMapHeader());
+	file.save(*getStartInfo());
+	file.save(activeModsDummy);
+	file.save(*this);
+}
+
+void CGameState::loadGame(CLoadFile & file)
+{
+	logGlobal->info("Loading game state...");
+
+	CMapHeader dummyHeader;
+	StartInfo dummyStartInfo;
+	ActiveModsInSaveList dummyActiveMods;
+
+	file.load(dummyHeader);
+	file.load(dummyStartInfo);
+	file.load(dummyActiveMods);
+	file.load(*this);
+}
+
 VCMI_LIB_NAMESPACE_END

lib/gameState/CGameState.h

@@ -28,6 +28,8 @@ class EVictoryLossCheckResult;
 class Services;
 class IMapService;
 class CMap;
+class CSaveFile;
+class CLoadFile;
 struct CPack;
 class CHeroClass;
 struct EventCondition;
@@ -168,6 +170,9 @@ public:
 	/// Any server-side code outside of GH must use vstd::RNG::getDefault
 	vstd::RNG & getRandomGenerator();
 
+	void saveGame(CSaveFile & file) const;
+	void loadGame(CLoadFile & file);
+
 	template <typename Handler> void serialize(Handler &h)
 	{
 		h & scenarioOps;

lib/mapping/CMapInfo.cpp

@@ -59,11 +59,12 @@ void CMapInfo::mapInit(const std::string & fname)
 
 void CMapInfo::saveInit(const ResourcePath & file)
 {
-	CLoadFile lf(*CResourceHandler::get()->getResourceName(file), ESerializationVersion::MINIMAL);
-	lf.checkMagicBytes(SAVEGAME_MAGIC);
+	CLoadFile lf(*CResourceHandler::get()->getResourceName(file), nullptr);
 
 	mapHeader = std::make_unique<CMapHeader>();
-	lf >> *(mapHeader) >> scenarioOptionsOfSave;
+	scenarioOptionsOfSave = std::make_unique<StartInfo>();
+	lf.load(*mapHeader);
+	lf.load(*scenarioOptionsOfSave);
 	fileURI = file.getName();
 	originalFileURI = file.getOriginalName();
 	fullFileURI = getFullFileURI(file);

lib/serializer/BinaryDeserializer.h

@@ -62,7 +62,6 @@ private:
 		if(length > 1000000)
 		{
 			logGlobal->warn("Warning: very big length: %d", length);
-			reader->reportState(logGlobal);
 		};
 		return length;
 	}

lib/serializer/CLoadFile.cpp

@@ -12,90 +12,58 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-CLoadFile::CLoadFile(const boost::filesystem::path & fname, ESerializationVersion minimalVersion)
+CLoadFile::CLoadFile(const boost::filesystem::path & fname, IGameCallback * cb)
 	: serializer(this)
+	, fName(fname.string())
+	, sfile(fname.c_str(), std::ios::in | std::ios::binary)
 {
-	openNextFile(fname, minimalVersion);
-}
-
-//must be instantiated in .cpp file for access to complete types of all member fields
-CLoadFile::~CLoadFile() = default;
-
-int CLoadFile::read(std::byte * data, unsigned size)
-{
-	sfile->read(reinterpret_cast<char *>(data), size);
-	return size;
-}
-
-void CLoadFile::openNextFile(const boost::filesystem::path & fname, ESerializationVersion minimalVersion)
-{
+	serializer.cb = cb;
 	serializer.loadingGamestate = true;
 	assert(!serializer.reverseEndianness);
-	assert(minimalVersion <= ESerializationVersion::CURRENT);
 
-	try
-	{
-		fName = fname.string();
-		sfile = std::make_unique<std::fstream>(fname.c_str(), std::ios::in | std::ios::binary);
-		sfile->exceptions(std::ifstream::failbit | std::ifstream::badbit); //we throw a lot anyway
+	fName = fname.string();
+	sfile.exceptions(std::ifstream::failbit | std::ifstream::badbit); //we throw a lot anyway
 
-		if(!(*sfile))
-			THROW_FORMAT("Error: cannot open to read %s!", fName);
+	if(!sfile)
+		throw std::runtime_error("Error: cannot open file '" + fName + "' for reading!");
 
-		//we can read
-		char buffer[4];
-		sfile->read(buffer, 4);
-		if(std::memcmp(buffer, "VCMI", 4) != 0)
-			THROW_FORMAT("Error: not a VCMI file(%s)!", fName);
+	//we can read
+	char buffer[4];
+	sfile.read(buffer, 4);
+	if(std::memcmp(buffer, "VCMI", 4) != 0)
+		throw std::runtime_error("Error: '" + fName + "' is not a VCMI file!");
 
-		serializer & serializer.version;
-		if(serializer.version < minimalVersion)
-			THROW_FORMAT("Error: too old file format (%s)!", fName);
+	serializer & serializer.version;
+	if(serializer.version < ESerializationVersion::MINIMAL)
+		throw std::runtime_error("Error: too old file format detected in '" + fName + "'!");
 
-		if(serializer.version > ESerializationVersion::CURRENT)
-		{
-			logGlobal->warn("Warning format version mismatch: found %d when current is %d! (file %s)\n", vstd::to_underlying(serializer.version), vstd::to_underlying(ESerializationVersion::CURRENT), fName);
+	if(serializer.version > ESerializationVersion::CURRENT)
+	{
+		logGlobal->warn("Warning format version mismatch: found %d when current is %d! (file %s)\n", vstd::to_underlying(serializer.version), vstd::to_underlying(ESerializationVersion::CURRENT), fName);
 
-			auto * versionptr = reinterpret_cast<char *>(&serializer.version);
-			std::reverse(versionptr, versionptr + 4);
-			logGlobal->warn("Version number reversed is %x, checking...", vstd::to_underlying(serializer.version));
+		auto * versionptr = reinterpret_cast<char *>(&serializer.version);
+		std::reverse(versionptr, versionptr + 4);
+		logGlobal->warn("Version number reversed is %x, checking...", vstd::to_underlying(serializer.version));
 
-			if(serializer.version == ESerializationVersion::CURRENT)
-			{
-				logGlobal->warn("%s seems to have different endianness! Entering reversing mode.", fname.string());
-				serializer.reverseEndianness = true;
-			}
-			else
-				THROW_FORMAT("Error: too new file format (%s)!", fName);
+		if(serializer.version == ESerializationVersion::CURRENT)
+		{
+			logGlobal->warn("%s seems to have different endianness! Entering reversing mode.", fname.string());
+			serializer.reverseEndianness = true;
 		}
+		else
+			throw std::runtime_error("Error: too new file format detected in '" + fName + "'!");
 	}
-	catch(...)
-	{
-		clear(); //if anything went wrong, we delete file and rethrow
-		throw;
-	}
-}
-
-void CLoadFile::reportState(vstd::CLoggerBase * out)
-{
-	out->debug("CLoadFile");
-	if(!!sfile && *sfile)
-		out->debug("\tOpened %s Position: %d", fName, sfile->tellg());
-}
 
-void CLoadFile::clear()
-{
-	sfile = nullptr;
-	fName.clear();
-	serializer.version = ESerializationVersion::NONE;
+	std::string loaded = SAVEGAME_MAGIC;
+	sfile.read(loaded.data(), SAVEGAME_MAGIC.length());
+	if(loaded != SAVEGAME_MAGIC)
+		throw std::runtime_error("Magic bytes doesn't match!");
 }
 
-void CLoadFile::checkMagicBytes(const std::string &text)
+int CLoadFile::read(std::byte * data, unsigned size)
 {
-	std::string loaded = text;
-	read(reinterpret_cast<std::byte*>(loaded.data()), text.length());
-	if(loaded != text)
-		throw std::runtime_error("Magic bytes doesn't match!");
+	sfile.read(reinterpret_cast<char *>(data), size);
+	return size;
 }
 
 VCMI_LIB_NAMESPACE_END

lib/serializer/CLoadFile.h

@@ -15,27 +15,21 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 class DLL_LINKAGE CLoadFile : public IBinaryReader
 {
-public:
 	BinaryDeserializer serializer;
 
 	std::string fName;
-	std::unique_ptr<std::fstream> sfile;
+	std::fstream sfile;
 
-	CLoadFile(const boost::filesystem::path & fname, ESerializationVersion minimalVersion = ESerializationVersion::CURRENT); //throws!
-	virtual ~CLoadFile();
 	int read(std::byte * data, unsigned size) override; //throws!
 
-	void openNextFile(const boost::filesystem::path & fname, ESerializationVersion minimalVersion); //throws!
-	void clear();
-	void reportState(vstd::CLoggerBase * out) override;
-
-	void checkMagicBytes(const std::string & text);
+public:
+	CLoadFile(const boost::filesystem::path & fname, IGameCallback * cb); //throws!
 
 	template<class T>
-	CLoadFile & operator>>(T &t)
+	void load(T & data)
 	{
-		serializer & t;
-		return * this;
+		static_assert(is_serializeable<BinaryDeserializer, T>::value, "This class can't be deserialized (possible pointer?)");
+		serializer & data;
 	}
 };
 

lib/serializer/CSaveFile.cpp

@@ -14,50 +14,23 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 CSaveFile::CSaveFile(const boost::filesystem::path &fname)
 	: serializer(this)
+	, sfile(fname.c_str(), std::ios::out | std::ios::binary)
+	, fName(fname)
 {
-	openNextFile(fname);
-}
-
-//must be instantiated in .cpp file for access to complete types of all member fields
-CSaveFile::~CSaveFile() = default;
-
-int CSaveFile::write(const std::byte * data, unsigned size)
-{
-	sfile->write(reinterpret_cast<const char *>(data), size);
-	return size;
-}
+	sfile.exceptions(std::ifstream::failbit | std::ifstream::badbit); //we throw a lot anyway
 
-void CSaveFile::openNextFile(const boost::filesystem::path &fname)
-{
-	fName = fname;
-	try
-	{
-		sfile = std::make_unique<std::fstream>(fname.c_str(), std::ios::out | std::ios::binary);
-		sfile->exceptions(std::ifstream::failbit | std::ifstream::badbit); //we throw a lot anyway
-
-		if(!(*sfile))
-			THROW_FORMAT("Error: cannot open to write %s!", fname);
+	if(!sfile)
+		throw std::runtime_error("Error: cannot open file '" + fName.string() + "' for writing!");
 
-		sfile->write("VCMI",4); //write magic identifier
-		serializer & ESerializationVersion::CURRENT; //write format version
-	}
-	catch(...)
-	{
-		logGlobal->error("Failed to save to %s", fname.string());
-		clear();
-		throw;
-	}
+	sfile.write("VCMI", 4); //write magic identifier
+	serializer & ESerializationVersion::CURRENT; //write format version
+	sfile.write(SAVEGAME_MAGIC.c_str(), SAVEGAME_MAGIC.length());
 }
 
-void CSaveFile::clear()
-{
-	fName.clear();
-	sfile = nullptr;
-}
-
-void CSaveFile::putMagicBytes(const std::string &text)
+int CSaveFile::write(const std::byte * data, unsigned size)
 {
-	write(reinterpret_cast<const std::byte*>(text.c_str()), text.length());
+	sfile.write(reinterpret_cast<const char *>(data), size);
+	return size;
 }
 
 VCMI_LIB_NAMESPACE_END

lib/serializer/CSaveFile.h

@@ -10,31 +10,27 @@
 #pragma once
 
 #include "BinarySerializer.h"
+#include "CSerializer.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-class DLL_LINKAGE CSaveFile : public IBinaryWriter
+class DLL_LINKAGE CSaveFile final : public IBinaryWriter
 {
-public:
 	BinarySerializer serializer;
 
 	boost::filesystem::path fName;
-	std::unique_ptr<std::fstream> sfile;
-
-	CSaveFile(const boost::filesystem::path &fname); //throws!
-	~CSaveFile();
-	int write(const std::byte * data, unsigned size) override;
+	std::fstream sfile;
 
-	void openNextFile(const boost::filesystem::path &fname); //throws!
-	void clear();
+	int write(const std::byte * data, unsigned size) final;
 
-	void putMagicBytes(const std::string &text);
+public:
+	explicit CSaveFile(const boost::filesystem::path & fname); //throws!
 
 	template<class T>
-	CSaveFile & operator<<(const T &t)
+	void save(const T & data)
 	{
-		serializer & t;
-		return * this;
+		static_assert(is_serializeable<BinarySerializer, T>::value, "This class can't be serialized (possible pointer?)");
+		serializer & data;
 	}
 };
 

lib/serializer/CSerializer.h

@@ -32,7 +32,6 @@ class IBinaryReader
 public:
 	virtual ~IBinaryReader() = default;
 	virtual int read(std::byte * data, unsigned size) = 0;
-	virtual void reportState(vstd::CLoggerBase * out){};
 };
 
 /// Base class for serializers

lib/serializer/ESerializationVersion.h

@@ -32,9 +32,12 @@ enum class ESerializationVersion : int32_t
 	NONE = 0,
 
 	RELEASE_160 = 873,
-	MINIMAL = RELEASE_160,
 
 	MAP_HEADER_DISPOSED_HEROES, // map header contains disposed heroes list
+	NO_RAW_POINTERS_IN_SERIALIZER, // large rework that removed all non-owning pointers from serializer
 	
-	CURRENT = MAP_HEADER_DISPOSED_HEROES
+	CURRENT = NO_RAW_POINTERS_IN_SERIALIZER,
+	MINIMAL = CURRENT,
 };
+
+static_assert(ESerializationVersion::MINIMAL <= ESerializationVersion::CURRENT, "Invalid serialization version definition!");

server/CGameHandler.cpp

@@ -1544,12 +1544,10 @@ void CGameHandler::save(const std::string & filename)
 
 	try
 	{
-		{
-			CSaveFile save(*CResourceHandler::get("local")->getResourceName(savePath));
-			saveCommonState(save);
-			logGlobal->info("Saving server state");
-			save << *this;
-		}
+		CSaveFile save(*CResourceHandler::get("local")->getResourceName(savePath));
+		gameState()->saveGame(save);
+		logGlobal->info("Saving server state");
+		save.save(*this);
 		logGlobal->info("Game has been successfully saved!");
 	}
 	catch(std::exception &e)
@@ -1567,13 +1565,11 @@ bool CGameHandler::load(const std::string & filename)
 
 	try
 	{
-		{
-			CLoadFile lf(*CResourceHandler::get()->getResourceName(ResourcePath(stem.to_string(), EResType::SAVEGAME)), ESerializationVersion::MINIMAL);
-			lf.serializer.cb = this;
-			loadCommonState(lf);
-			logGlobal->info("Loading server state");
-			lf >> *this;
-		}
+		CLoadFile lf(*CResourceHandler::get()->getResourceName(ResourcePath(stem.to_string(), EResType::SAVEGAME)), this);
+		gs = std::make_shared<CGameState>(this);
+		gs->loadGame(lf);
+		logGlobal->info("Loading server state");
+		lf.load(*this);
 		logGlobal->info("Game has been successfully loaded!");
 	}
 	catch(const ModIncompatibility & e)