Remove serialization of raw pointers from serialization

client/windows/CMapOverview.cpp

@@ -163,7 +163,7 @@ CMapOverviewWidget::CMapOverviewWidget(CMapOverview& parent):
 			lf.checkMagicBytes(SAVEGAME_MAGIC);
 
 			auto mapHeader = std::make_unique<CMapHeader>();
-			StartInfo * startInfo;
+			std::unique_ptr<StartInfo> startInfo;
 			lf >> *(mapHeader) >> startInfo;
 
 			if(startInfo->campState)

lib/IGameCallback.cpp

@@ -182,6 +182,7 @@ void CPrivilegedInfoCallback::loadCommonState(CLoadFile & in)
 	CMapHeader dum;
 	StartInfo si;
 	ActiveModsInSaveList activeMods;
+	CGameState & gs = *gameState();
 
 	logGlobal->info("\tReading header");
 	in.serializer & dum;
@@ -193,23 +194,24 @@ void CPrivilegedInfoCallback::loadCommonState(CLoadFile & in)
 	in.serializer & activeMods;
 
 	logGlobal->info("\tReading gamestate");
-	in.serializer & *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&>(gameState()->getMap());
+	out.serializer & static_cast<const CMapHeader&>(gs.getMap());
 	logGlobal->info("\tSaving options");
-	out.serializer & *gameState()->getStartInfo();
+	out.serializer & *gs.getStartInfo();
 	logGlobal->info("\tSaving mod list");
 	out.serializer & activeMods;
 	logGlobal->info("\tSaving gamestate");
-	out.serializer & *gameState();
+	out.serializer & gs;
 }
 
 TerrainTile * CNonConstInfoCallback::getTile(const int3 & pos)

lib/gameState/CGameState.cpp

@@ -1577,6 +1577,9 @@ void CGameState::deserializationFix()
 
 	buildGlobalTeamPlayerTree();
 	attachArmedObjects();
+
+	if (campaign)
+		campaign->setGamestate(this);
 }
 
 void CGameState::buildGlobalTeamPlayerTree()

lib/gameState/CGameState.h

@@ -181,8 +181,7 @@ public:
 		h & *heroesPool;
 		h & globalEffects;
 		h & currentRumor;
-		if (campaign)
-			h & *campaign;
+		h & campaign;
 		h & allocatedArtifacts;
 		h & statistic;
 

lib/gameState/CGameStateCampaign.cpp

@@ -40,6 +40,8 @@ CampaignHeroReplacement::CampaignHeroReplacement(std::shared_ptr<CGHeroInstance>
 {
 }
 
+CGameStateCampaign::CGameStateCampaign() = default;
+
 CGameStateCampaign::CGameStateCampaign(CGameState * owner):
 	gameState(owner)
 {
@@ -47,6 +49,11 @@ CGameStateCampaign::CGameStateCampaign(CGameState * owner):
 	assert(gameState->scenarioOps->campState != nullptr);
 }
 
+void CGameStateCampaign::setGamestate(CGameState * owner)
+{
+	gameState = owner;
+}
+
 std::optional<CampaignBonus> CGameStateCampaign::currentBonus() const
 {
 	auto campaignState = gameState->scenarioOps->campState;

lib/gameState/CGameStateCampaign.h

@@ -31,7 +31,7 @@ struct CampaignHeroReplacement
 
 class CGameStateCampaign : public Serializeable
 {
-	CGameState * gameState;
+	CGameState * gameState = nullptr;
 
 	/// Contains list of heroes that may be available in this scenario
 	/// temporary helper for game initialization, not serialized
@@ -54,7 +54,9 @@ class CGameStateCampaign : public Serializeable
 	void giveCampaignBonusToHero(CGHeroInstance * hero);
 
 public:
+	CGameStateCampaign();
 	CGameStateCampaign(CGameState * owner);
+	void setGamestate(CGameState * owner);
 
 	void placeCampaignHeroes();
 	void initStartingResources();
@@ -66,7 +68,7 @@ public:
 
 	template <typename Handler> void serialize(Handler &h)
 	{
-		h & gameState;
+		// no-op, but needed to auto-create this class if gamestate had it during serialization
 	}
 };
 

lib/mapObjects/CGTownInstance.cpp

@@ -276,11 +276,7 @@ CGTownInstance::CGTownInstance(IGameCallback *cb):
 	this->setNodeType(CBonusSystemNode::TOWN);
 }
 
-CGTownInstance::~CGTownInstance()
-{
-	for (auto & elem : rewardableBuildings)
-		delete elem.second;
-}
+CGTownInstance::~CGTownInstance() = default;
 
 int CGTownInstance::spellsAtLevel(int level, bool checkGuild) const
 {
@@ -398,7 +394,7 @@ void CGTownInstance::initializeConfigurableBuildings(vstd::RNG & rand)
 			continue;
 
 		try {
-			rewardableBuildings[kvp.first] = new TownRewardableBuildingInstance(this, kvp.second->bid, rand);
+			rewardableBuildings[kvp.first] = std::make_unique<TownRewardableBuildingInstance>(this, kvp.second->bid, rand);
 		}
 		catch (std::runtime_error & e)
 		{

lib/mapObjects/CGTownInstance.h

@@ -67,7 +67,7 @@ public:
 	ui32 identifier; //special identifier from h3m (only > RoE maps)
 	PlayerColor alignmentToPlayer; // if set to non-neutral, random town will have same faction as specified player
 	std::set<BuildingID> forbiddenBuildings;
-	std::map<BuildingID, TownRewardableBuildingInstance*> rewardableBuildings;
+	std::map<BuildingID, std::unique_ptr<TownRewardableBuildingInstance>> rewardableBuildings;
 	std::vector<SpellID> possibleSpells, obligatorySpells;
 	std::vector<std::vector<SpellID> > spells; //spells[level] -> vector of spells, first will be available in guild
 	std::vector<CCastleEvent> events;

lib/networkPacks/NetPacksLib.cpp

@@ -2452,7 +2452,7 @@ void SetRewardableConfiguration::applyGs(CGameState *gs)
 
 		for (auto & building : townPtr->rewardableBuildings)
 			if (building.second->getBuildingType() == buildingID)
-				buildingPtr = building.second;
+				buildingPtr = building.second.get();
 
 		auto * rewardablePtr = dynamic_cast<TownRewardableBuildingInstance *>(buildingPtr);
 		assert(rewardablePtr);

lib/serializer/BinaryDeserializer.h

@@ -191,8 +191,8 @@ public:
 			load(data[i]);
 	}
 
-	template < typename T, typename std::enable_if_t < std::is_pointer_v<T>, int  > = 0 >
-	void load(T &data)
+	template<typename T>
+	void loadRawPointer(T & data)
 	{
 		bool isNull;
 		load( isNull );
@@ -256,7 +256,7 @@ public:
 	{
 		typedef typename std::remove_const_t<T> NonConstT;
 		NonConstT *internalPtr;
-		load(internalPtr);
+		loadRawPointer(internalPtr);
 
 		const auto * internalPtrDerived = static_cast<Serializeable*>(internalPtr);
 
@@ -299,7 +299,7 @@ public:
 	void load(std::unique_ptr<T> &data)
 	{
 		T *internalPtr;
-		load( internalPtr );
+		loadRawPointer( internalPtr );
 		data.reset(internalPtr);
 	}
 	template <typename T, size_t N>

lib/serializer/BinarySerializer.h

@@ -144,8 +144,8 @@ public:
 			*this & data[i];
 	}
 
-	template < typename T, typename std::enable_if_t < std::is_pointer_v<T>, int  > = 0 >
-	void save(const T &data)
+	template<typename T>
+	void saveRawPointer(const T & data)
 	{
 		//write if pointer is not nullptr
 		bool isNull = (data == nullptr);
@@ -199,19 +199,19 @@ public:
 	void save(const std::shared_ptr<T> &data)
 	{
 		T *internalPtr = data.get();
-		save(internalPtr);
+		saveRawPointer(internalPtr);
 	}
 	template <typename T>
 	void save(const std::shared_ptr<const T> &data)
 	{
 		const T *internalPtr = data.get();
-		save(internalPtr);
+		saveRawPointer(internalPtr);
 	}
 	template <typename T>
 	void save(const std::unique_ptr<T> &data)
 	{
 		T *internalPtr = data.get();
-		save(internalPtr);
+		saveRawPointer(internalPtr);
 	}
 	template <typename T, typename std::enable_if_t < !std::is_same_v<T, bool >, int  > = 0>
 	void save(const std::vector<T> &data)

lib/serializer/CMemorySerializer.h

@@ -34,7 +34,7 @@ public:
 	static std::unique_ptr<T> deepCopy(const T &data)
 	{
 		CMemorySerializer mem;
-		mem.oser & &data;
+		mem.oser.saveRawPointer(&data);
 
 		std::unique_ptr<T> ret;
 		mem.iser & ret;
@@ -45,7 +45,7 @@ public:
 	static std::shared_ptr<T> deepCopyShared(const T &data)
 	{
 		CMemorySerializer mem;
-		mem.oser & &data;
+		mem.oser.saveRawPointer(&data);
 
 		std::shared_ptr<T> ret;
 		mem.iser & ret;

lib/serializer/Connection.cpp

@@ -78,7 +78,7 @@ void CConnection::sendPack(const CPack & pack)
 		throw std::runtime_error("Attempt to send packet on a closed connection!");
 
 	packWriter->buffer.clear();
-	(*serializer) & (&pack);
+	serializer->saveRawPointer(&pack);
 
 	logNetwork->trace("Sending a pack of type %s", typeid(pack).name());
 

server/queries/VisitQueries.cpp

@@ -86,7 +86,7 @@ void TownBuildingVisitQuery::onAdded(PlayerColor color)
 	while (!visitedBuilding.empty() && owner->topQuery(color).get() == this)
 	{
 		visitingHero = visitedBuilding.back().hero->id;
-		const auto * building = visitedTown->rewardableBuildings.at(visitedBuilding.back().building);
+		const auto & building = visitedTown->rewardableBuildings.at(visitedBuilding.back().building);
 		building->onHeroVisit(visitedBuilding.back().hero);
 		visitedBuilding.pop_back();
 	}