Get rid of some old serialization workaround code that is not needed because serializer was improved in the meantime.

Fixing #1254.
Fixed possible race causing corruption of the server savegame.

client/CMT.cpp

@@ -34,6 +34,7 @@
 #include "../lib/NetPacks.h"
 #include "CMessage.h"
 #include "../lib/CModHandler.h"
+#include "../lib/CTownHandler.h"
 #include "../lib/CObjectHandler.h"
 #include "../lib/CArtHandler.h"
 #include "../lib/CScriptingModule.h"
@@ -201,7 +202,6 @@ int main(int argc, char** argv)
     }
     fclose(check);
 #endif
-    
     std::cout << "Starting... " << std::endl;
 	po::options_description opts("Allowed options");
 	opts.add_options()

client/NetPacksClient.cpp

@@ -783,9 +783,6 @@ void SaveGame::applyCl(CClient *cl)
 	CFileInfo info(fname);
 	CResourceHandler::get()->createResource(info.getStem() + ".vcgm1");
 
-	//FIXME: Workaround for a file that was created by server and in future should be used only by server
-	CResourceHandler::get()->createResource(info.getStem() + ".vsgm1");
-
 	try
 	{
 		CSaveFile save(CResourceHandler::get()->getResourceName(ResourceID(info.getStem(), EResType::CLIENT_SAVEGAME)));

lib/CObjectHandler.cpp

@@ -2174,7 +2174,7 @@ void CGTownInstance::onHeroVisit(const CGHeroInstance * h) const
 			cb->heroVisitCastle(this, h);
 		}
 	}
-	else
+	else if(h->visitablePos() == visitablePos())
 	{
 		if (h->commander && !h->commander->alive) //rise commander. TODO: interactive script
 		{
@@ -2186,6 +2186,10 @@ void CGTownInstance::onHeroVisit(const CGHeroInstance * h) const
 		}
 		cb->heroVisitCastle(this, h);
 	}
+	else
+	{
+		logGlobal->errorStream() << h->name << " visits allied town of " << name << " from different pos?";
+	}
 }
 
 void CGTownInstance::onHeroLeave(const CGHeroInstance * h) const
@@ -2433,10 +2437,13 @@ std::string CGTownInstance::nodeName() const
 void CGTownInstance::deserializationFix()
 {
 	attachTo(&townAndVis);
-	if(visitingHero)
-		visitingHero->attachTo(&townAndVis);
-	if(garrisonHero)
-		garrisonHero->attachTo(this);
+
+	//Hero is already handled by CGameState::attachArmedObjects
+
+// 	if(visitingHero)
+// 		visitingHero->attachTo(&townAndVis);
+// 	if(garrisonHero)
+// 		garrisonHero->attachTo(this);
 }
 
 void CGTownInstance::recreateBuildingsBonuses()

lib/CObjectHandler.h

@@ -354,8 +354,8 @@ public:
 		h & static_cast<CArmedInstance&>(*this);
 		h & static_cast<CArtifactSet&>(*this);
 		h & exp & level & name & biography & portrait & mana & secSkills & movement
-			& sex & inTownGarrison & /*artifacts & artifWorn & */spells & patrol & moveDir;
-
+			& sex & inTownGarrison & spells & patrol & moveDir;
+		h & visitedTown & boat;
 		h & type & specialty & commander;
 		BONUS_TREE_DESERIALIZATION_FIX
 		//visitied town pointer will be restored by map serialization method
@@ -389,11 +389,6 @@ public:
 	int maxMovePoints(bool onLand) const;
 	int movementPointsAfterEmbark(int MPsBefore, int basicCost, bool disembark = false) const;
 
-// 	const CArtifact* getArtAtPos(ui16 pos) const; //NULL - no artifact
-// 	const CArtifact * getArt(int pos) const;
-// 	si32 getArtPos(int aid) const; //looks for equipped artifact with given ID and returns its slot ID or -1 if none(if more than one such artifact lower ID is returned)
-// 	bool hasArt(ui32 aid) const; //checks if hero possess artifact of given id (either in backack or worn)
-
 	//int getSpellSecLevel(int spell) const; //returns level of secondary ability (fire, water, earth, air magic) known to this hero and applicable to given spell; -1 if error
 	static int3 convertPosition(int3 src, bool toh3m); //toh3m=true: manifest->h3m; toh3m=false: h3m->manifest
 	double getHeroStrength() const;
@@ -610,7 +605,9 @@ public:
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & static_cast<CGDwelling&>(*this);
-		h & name & builded & destroyed & identifier & alignment & forbiddenBuildings & builtBuildings & bonusValue
+		h & name & builded & destroyed & identifier;
+		h & garrisonHero & visitingHero;
+		h & alignment & forbiddenBuildings & builtBuildings & bonusValue
 			& possibleSpells & obligatorySpells & spells & /*strInfo & */events & bonusingBuildings;
 
 		for (std::vector<CGTownBuilding*>::iterator i = bonusingBuildings.begin(); i!=bonusingBuildings.end(); i++)
@@ -618,7 +615,6 @@ public:
 
 		h & town & townAndVis;
 		BONUS_TREE_DESERIALIZATION_FIX
-		//garrison/visiting hero pointers will be restored in the map serialization
 	}
 	//////////////////////////////////////////////////////////////////////////
 
@@ -1185,7 +1181,7 @@ public:
 	}
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
-		h & static_cast<CGObjectInstance&>(*this) & direction;
+		h & static_cast<CGObjectInstance&>(*this) & direction & hero;
 	}
 };
 

lib/mapping/CMap.h

@@ -268,13 +268,9 @@ struct DLL_LINKAGE TerrainTile
 	template <typename Handler>
 	void serialize(Handler & h, const int version)
 	{
-		h & terType & terView & riverType & riverDir & roadType &roadDir & extTileFlags & blocked;
-
-		if(!h.saving)
-		{
-			visitable = false;
-			//these flags (and obj vectors) will be restored in map serialization
-		}
+		h & terType & terView & riverType & riverDir & roadType &roadDir & extTileFlags;
+		h & visitable & blocked;
+		h & visitableObjects & blockingObjects;
 	}
 };
 
@@ -424,6 +420,7 @@ public:
 		}
 
 		h & customDefs & objects;
+		h & heroes & towns & artInstances;
 
 		// static members
 		h & CGTeleport::objs;
@@ -433,69 +430,5 @@ public:
 		h & CGObelisk::obeliskCount & CGObelisk::visited;
 		h & CGTownInstance::merchantArtifacts;
 		h & CGTownInstance::universitySkills;
-
-		if(!h.saving)
-		{
-			for(ui32 i = 0; i < objects.size(); ++i)
-			{
-				if(!objects[i]) continue;
-
-				switch (objects[i]->ID)
-				{
-				case Obj::HERO:
-					heroes.push_back (static_cast<CGHeroInstance*>(+objects[i]));
-					break;
-				case Obj::TOWN:
-					towns.push_back (static_cast<CGTownInstance*>(+objects[i]));
-					break;
-				}
-
-				// recreate blockvis map
-				addBlockVisTiles(objects[i]);
-			}
-
-			// if hero is visiting/garrisoned in town set appropriate pointers
-			for(ui32 i = 0; i < heroes.size(); ++i)
-			{
-				int3 vistile = heroes[i]->pos;
-				vistile.x++;
-				for(ui32 j = 0; j < towns.size(); ++j)
-				{
-					// hero stands on the town entrance
-					if(vistile == towns[j]->pos)
-					{
-						if(heroes[i]->inTownGarrison)
-						{
-							towns[j]->garrisonHero = heroes[i];
-							removeBlockVisTiles(heroes[i]);
-						}
-						else
-						{
-							towns[j]->visitingHero = heroes[i];
-						}
-
-						heroes[i]->visitedTown = towns[j];
-						break;
-					}
-				}
-
-				vistile.x -= 2; //manifest pos
-				const TerrainTile & t = getTile(vistile);
-				if(t.terType != ETerrainType::WATER) continue;
-
-				//hero stands on the water - he must be in the boat
-				for(ui32 j = 0; j < t.visitableObjects.size(); ++j)
-				{
-					if(t.visitableObjects[j]->ID == Obj::BOAT)
-					{
-						CGBoat * b = static_cast<CGBoat *>(t.visitableObjects[j]);
-						heroes[i]->boat = b;
-						b->hero = heroes[i];
-						removeBlockVisTiles(b);
-						break;
-					}
-				}
-			}
-		}
 	}
 };

server/CGameHandler.cpp

@@ -2186,7 +2186,7 @@ void CGameHandler::save(const std::string & filename )
 {
     logGlobal->errorStream() << "Saving to " << filename;
 	CFileInfo info(filename);
-	CResourceHandler::get()->createResource(info.getStem() + ".vlgm1");
+	//CResourceHandler::get()->createResource(info.getStem() + ".vlgm1");
 	CResourceHandler::get()->createResource(info.getStem() + ".vsgm1");
 
 	{