Significantly simplify packs applying

Apparently our logic for packs applying with types registration is
overcomplicated and by now completely unnecessary - it became redundant
after introduction of visitor pattern.

AI/Nullkiller/AIGateway.cpp

@@ -20,7 +20,6 @@
 #include "../../lib/CHeroHandler.h"
 #include "../../lib/GameSettings.h"
 #include "../../lib/gameState/CGameState.h"
-#include "../../lib/serializer/CTypeList.h"
 #include "../../lib/serializer/BinarySerializer.h"
 #include "../../lib/serializer/BinaryDeserializer.h"
 #include "../../lib/networkPacks/PacksForClient.h"

AI/VCAI/VCAI.cpp

@@ -30,7 +30,6 @@
 #include "../../lib/networkPacks/PacksForClient.h"
 #include "../../lib/networkPacks/PacksForClientBattle.h"
 #include "../../lib/networkPacks/PacksForServer.h"
-#include "../../lib/serializer/CTypeList.h"
 #include "../../lib/serializer/BinarySerializer.h"
 #include "../../lib/serializer/BinaryDeserializer.h"
 

client/CPlayerInterface.cpp

@@ -102,7 +102,6 @@
 
 #include "../lib/serializer/BinaryDeserializer.h"
 #include "../lib/serializer/BinarySerializer.h"
-#include "../lib/serializer/CTypeList.h"
 
 #include "../lib/spells/CSpellHandler.h"
 

client/CServerHandler.cpp

@@ -35,6 +35,7 @@
 #include "../lib/TurnTimerInfo.h"
 #include "../lib/VCMIDirs.h"
 #include "../lib/campaign/CampaignState.h"
+#include "../lib/gameState/CGameState.h"
 #include "../lib/gameState/HighScore.h"
 #include "../lib/CPlayerState.h"
 #include "../lib/mapping/CMapInfo.h"
@@ -44,7 +45,6 @@
 #include "../lib/rmg/CMapGenOptions.h"
 #include "../lib/serializer/Connection.h"
 #include "../lib/filesystem/Filesystem.h"
-#include "../lib/registerTypes/RegisterTypesLobbyPacks.h"
 #include "../lib/serializer/CMemorySerializer.h"
 #include "../lib/UnlockGuard.h"
 
@@ -56,61 +56,6 @@
 
 #include <vcmi/events/EventBus.h>
 
-template<typename T> class CApplyOnLobby;
-
-class CBaseForLobbyApply
-{
-public:
-	virtual bool applyOnLobbyHandler(CServerHandler * handler, CPackForLobby & pack) const = 0;
-	virtual void applyOnLobbyScreen(CLobbyScreen * lobby, CServerHandler * handler, CPackForLobby & pack) const = 0;
-	virtual ~CBaseForLobbyApply(){};
-	template<typename U> static CBaseForLobbyApply * getApplier(const U * t = nullptr)
-	{
-		return new CApplyOnLobby<U>();
-	}
-};
-
-template<typename T> class CApplyOnLobby : public CBaseForLobbyApply
-{
-public:
-	bool applyOnLobbyHandler(CServerHandler * handler, CPackForLobby & pack) const override
-	{
-		auto & ref = static_cast<T&>(pack);
-		ApplyOnLobbyHandlerNetPackVisitor visitor(*handler);
-
-		logNetwork->trace("\tImmediately apply on lobby: %s", typeid(ref).name());
-		ref.visit(visitor);
-
-		return visitor.getResult();
-	}
-
-	void applyOnLobbyScreen(CLobbyScreen * lobby, CServerHandler * handler, CPackForLobby & pack) const override
-	{
-		auto & ref = static_cast<T &>(pack);
-		ApplyOnLobbyScreenNetPackVisitor visitor(*handler, lobby);
-
-		logNetwork->trace("\tApply on lobby from queue: %s", typeid(ref).name());
-		ref.visit(visitor);
-	}
-};
-
-template<> class CApplyOnLobby<CPack>: public CBaseForLobbyApply
-{
-public:
-	bool applyOnLobbyHandler(CServerHandler * handler, CPackForLobby & pack) const override
-	{
-		logGlobal->error("Cannot apply plain CPack!");
-		assert(0);
-		return false;
-	}
-
-	void applyOnLobbyScreen(CLobbyScreen * lobby, CServerHandler * handler, CPackForLobby & pack) const override
-	{
-		logGlobal->error("Cannot apply plain CPack!");
-		assert(0);
-	}
-};
-
 CServerHandler::~CServerHandler()
 {
 	if (serverRunner)
@@ -148,7 +93,6 @@ CServerHandler::CServerHandler()
 	: networkHandler(INetworkHandler::createHandler())
 	, lobbyClient(std::make_unique<GlobalLobbyClient>())
 	, gameChat(std::make_unique<GameChatHandler>())
-	, applier(std::make_unique<CApplier<CBaseForLobbyApply>>())
 	, threadNetwork(&CServerHandler::threadRunNetwork, this)
 	, state(EClientState::NONE)
 	, serverPort(0)
@@ -159,7 +103,6 @@ CServerHandler::CServerHandler()
 	, client(nullptr)
 {
 	uuid = boost::uuids::to_string(boost::uuids::random_generator()());
-	registerTypesLobbyPacks(*applier);
 }
 
 void CServerHandler::threadRunNetwork()
@@ -320,8 +263,8 @@ void CServerHandler::onConnectionEstablished(const NetworkConnectionPtr & netCon
 
 void CServerHandler::applyPackOnLobbyScreen(CPackForLobby & pack)
 {
-	const CBaseForLobbyApply * apply = applier->getApplier(CTypeList::getInstance().getTypeID(&pack)); //find the applier
-	apply->applyOnLobbyScreen(dynamic_cast<CLobbyScreen *>(SEL), this, pack);
+	ApplyOnLobbyScreenNetPackVisitor visitor(*this, dynamic_cast<CLobbyScreen *>(SEL));
+	pack.visit(visitor);
 	GH.windows().totalRedraw();
 }
 
@@ -960,7 +903,10 @@ void CServerHandler::waitForServerShutdown()
 
 void CServerHandler::visitForLobby(CPackForLobby & lobbyPack)
 {
-	if(applier->getApplier(CTypeList::getInstance().getTypeID(&lobbyPack))->applyOnLobbyHandler(this, lobbyPack))
+	ApplyOnLobbyHandlerNetPackVisitor visitor(*this);
+	lobbyPack.visit(visitor);
+
+	if(visitor.getResult())
 	{
 		if(!settings["session"]["headless"].Bool())
 			applyPackOnLobbyScreen(lobbyPack);

client/CServerHandler.h

@@ -102,7 +102,6 @@ class CServerHandler final : public IServerAPI, public LobbyInfo, public INetwor
 	std::shared_ptr<INetworkConnection> networkConnection;
 	std::unique_ptr<GlobalLobbyClient> lobbyClient;
 	std::unique_ptr<GameChatHandler> gameChat;
-	std::unique_ptr<CApplier<CBaseForLobbyApply>> applier;
 	std::unique_ptr<IServerRunner> serverRunner;
 	std::shared_ptr<CMapInfo> mapToStart;
 	std::vector<std::string> localPlayerNames;

client/Client.cpp

@@ -35,7 +35,6 @@
 #include "../lib/mapping/CMapService.h"
 #include "../lib/pathfinder/CGPathNode.h"
 #include "../lib/filesystem/Filesystem.h"
-#include "../lib/registerTypes/RegisterTypesClientPacks.h"
 
 #include <memory>
 #include <vcmi/events/EventBus.h>
@@ -50,53 +49,6 @@
 
 ThreadSafeVector<int> CClient::waitingRequest;
 
-template<typename T> class CApplyOnCL;
-
-class CBaseForCLApply
-{
-public:
-	virtual void applyOnClAfter(CClient * cl, CPack * pack) const =0;
-	virtual void applyOnClBefore(CClient * cl, CPack * pack) const =0;
-	virtual ~CBaseForCLApply(){}
-
-	template<typename U> static CBaseForCLApply * getApplier(const U * t = nullptr)
-	{
-		return new CApplyOnCL<U>();
-	}
-};
-
-template<typename T> class CApplyOnCL : public CBaseForCLApply
-{
-public:
-	void applyOnClAfter(CClient * cl, CPack * pack) const override
-	{
-		T * ptr = static_cast<T *>(pack);
-		ApplyClientNetPackVisitor visitor(*cl, *cl->gameState());
-		ptr->visit(visitor);
-	}
-	void applyOnClBefore(CClient * cl, CPack * pack) const override
-	{
-		T * ptr = static_cast<T *>(pack);
-		ApplyFirstClientNetPackVisitor visitor(*cl, *cl->gameState());
-		ptr->visit(visitor);
-	}
-};
-
-template<> class CApplyOnCL<CPack>: public CBaseForCLApply
-{
-public:
-	void applyOnClAfter(CClient * cl, CPack * pack) const override
-	{
-		logGlobal->error("Cannot apply on CL plain CPack!");
-		assert(0);
-	}
-	void applyOnClBefore(CClient * cl, CPack * pack) const override
-	{
-		logGlobal->error("Cannot apply on CL plain CPack!");
-		assert(0);
-	}
-};
-
 CPlayerEnvironment::CPlayerEnvironment(PlayerColor player_, CClient * cl_, std::shared_ptr<CCallback> mainCallback_)
 	: player(player_),
 	cl(cl_),
@@ -130,12 +82,9 @@ const CPlayerEnvironment::GameCb * CPlayerEnvironment::game() const
 	return mainCallback.get();
 }
 
-
 CClient::CClient()
 {
 	waitingRequest.clear();
-	applier = std::make_shared<CApplier<CBaseForCLApply>>();
-	registerTypesClientPacks(*applier);
 	gs = nullptr;
 }
 
@@ -402,23 +351,19 @@ void CClient::installNewBattleInterface(std::shared_ptr<CBattleGameInterface> ba
 
 void CClient::handlePack(CPack * pack)
 {
-	CBaseForCLApply * apply = applier->getApplier(CTypeList::getInstance().getTypeID(pack)); //find the applier
-	if(apply)
-	{
-		apply->applyOnClBefore(this, pack);
-		logNetwork->trace("\tMade first apply on cl: %s", typeid(*pack).name());
-		{
-			boost::unique_lock lock(CGameState::mutex);
-			gs->apply(pack);
-		}
-		logNetwork->trace("\tApplied on gs: %s", typeid(*pack).name());
-		apply->applyOnClAfter(this, pack);
-		logNetwork->trace("\tMade second apply on cl: %s", typeid(*pack).name());
-	}
-	else
+	ApplyClientNetPackVisitor afterVisitor(*this, *gameState());
+	ApplyFirstClientNetPackVisitor beforeVisitor(*this, *gameState());
+
+	pack->visit(beforeVisitor);
+	logNetwork->trace("\tMade first apply on cl: %s", typeid(*pack).name());
 	{
-		logNetwork->error("Message %s cannot be applied, cannot find applier!", typeid(*pack).name());
+		boost::unique_lock lock(CGameState::mutex);
+		gs->apply(pack);
 	}
+	logNetwork->trace("\tApplied on gs: %s", typeid(*pack).name());
+	pack->visit(afterVisitor);
+	logNetwork->trace("\tMade second apply on cl: %s", typeid(*pack).name());
+
 	delete pack;
 }
 

client/Client.h

@@ -237,8 +237,6 @@ private:
 #endif
 	std::unique_ptr<events::EventBus> clientEventBus;
 
-	std::shared_ptr<CApplier<CBaseForCLApply>> applier;
-
 	mutable boost::mutex pathCacheMutex;
 	std::map<const CGHeroInstance *, std::shared_ptr<CPathsInfo>> pathCache;
 

client/mainmenu/CMainMenu.cpp

@@ -47,7 +47,6 @@
 
 #include "../../lib/texts/CGeneralTextHandler.h"
 #include "../../lib/campaign/CampaignHandler.h"
-#include "../../lib/serializer/CTypeList.h"
 #include "../../lib/filesystem/Filesystem.h"
 #include "../../lib/filesystem/CCompressedStream.h"
 #include "../../lib/mapping/CMapInfo.h"

server/CGameHandler.cpp

@@ -71,8 +71,6 @@
 #include "../lib/pathfinder/PathfinderOptions.h"
 #include "../lib/pathfinder/TurnInfo.h"
 
-#include "../lib/registerTypes/RegisterTypesServerPacks.h"
-
 #include "../lib/rmg/CMapGenOptions.h"
 
 #include "../lib/serializer/CSaveFile.h"
@@ -92,53 +90,6 @@
 #define COMPLAIN_RET(txt) {complain(txt); return false;}
 #define COMPLAIN_RETF(txt, FORMAT) {complain(boost::str(boost::format(txt) % FORMAT)); return false;}
 
-template <typename T> class CApplyOnGH;
-
-class CBaseForGHApply
-{
-public:
-	virtual bool applyOnGH(CGameHandler * gh, CGameState * gs, CPack * pack) const =0;
-	virtual ~CBaseForGHApply(){}
-	template<typename U> static CBaseForGHApply *getApplier(const U * t=nullptr)
-	{
-		return new CApplyOnGH<U>();
-	}
-};
-
-template <typename T> class CApplyOnGH : public CBaseForGHApply
-{
-public:
-	bool applyOnGH(CGameHandler * gh, CGameState * gs, CPack * pack) const override
-	{
-		T *ptr = static_cast<T*>(pack);
-		try
-		{
-			ApplyGhNetPackVisitor applier(*gh);
-
-			ptr->visit(applier);
-
-			return applier.getResult();
-		}
-		catch(ExceptionNotAllowedAction & e)
-		{
-            (void)e;
-			return false;
-		}
-	}
-};
-
-template <>
-class CApplyOnGH<CPack> : public CBaseForGHApply
-{
-public:
-	bool applyOnGH(CGameHandler * gh, CGameState * gs, CPack * pack) const override
-	{
-		logGlobal->error("Cannot apply on GH plain CPack!");
-		assert(0);
-		return false;
-	}
-};
-
 static inline double distance(int3 a, int3 b)
 {
 	return std::sqrt((double)(a.x-b.x)*(a.x-b.x) + (a.y-b.y)*(a.y-b.y));
@@ -499,28 +450,30 @@ void CGameHandler::handleReceivedPack(CPackForServer * pack)
 		pack->c->sendPack(&applied);
 	};
 
-	CBaseForGHApply * apply = applier->getApplier(CTypeList::getInstance().getTypeID(pack)); //and appropriate applier object
 	if(isBlockedByQueries(pack, pack->player))
 	{
 		sendPackageResponse(false);
 	}
-	else if(apply)
-	{
-		const bool result = apply->applyOnGH(this, this->gs, pack);
-		if(result)
-			logGlobal->trace("Message %s successfully applied!", typeid(*pack).name());
-		else
-			complain((boost::format("Got false in applying %s... that request must have been fishy!")
-				% typeid(*pack).name()).str());
 
-		sendPackageResponse(true);
+	bool result;
+	try
+	{
+		ApplyGhNetPackVisitor applier(*this);
+		pack->visit(applier);
+		result = applier.getResult();
 	}
-	else
+	catch(ExceptionNotAllowedAction &)
 	{
-		logGlobal->error("Message cannot be applied, cannot find applier (unregistered type)!");
-		sendPackageResponse(false);
+		result = false;
 	}
 
+	if(result)
+		logGlobal->trace("Message %s successfully applied!", typeid(*pack).name());
+	else
+		complain((boost::format("Got false in applying %s... that request must have been fishy!")
+			% typeid(*pack).name()).str());
+
+	sendPackageResponse(true);
 	vstd::clear_pointer(pack);
 }
 
@@ -538,8 +491,6 @@ CGameHandler::CGameHandler(CVCMIServer * lobby)
 	, turnTimerHandler(std::make_unique<TurnTimerHandler>(*this))
 {
 	QID = 1;
-	applier = std::make_shared<CApplier<CBaseForGHApply>>();
-	registerTypesServerPacks(*applier);
 
 	spellEnv = new ServerSpellCastEnvironment(this);
 }

server/CGameHandler.h

@@ -57,7 +57,6 @@ class CObjectVisitQuery;
 class CGameHandler : public IGameCallback, public Environment
 {
 	CVCMIServer * lobby;
-	std::shared_ptr<CApplier<CBaseForGHApply>> applier;
 
 public:
 	std::unique_ptr<HeroPoolProcessor> heroPool;

server/CVCMIServer.cpp

@@ -17,7 +17,12 @@
 
 #include "../lib/CHeroHandler.h"
 #include "../lib/CPlayerState.h"
-#include "../lib/registerTypes/RegisterTypesLobbyPacks.h"
+#include "../lib/campaign/CampaignState.h"
+#include "../lib/gameState/CGameState.h"
+#include "../lib/mapping/CMapDefines.h"
+#include "../lib/mapping/CMapInfo.h"
+#include "../lib/mapping/CMapHeader.h"
+#include "../lib/rmg/CMapGenOptions.h"
 #include "../lib/serializer/CMemorySerializer.h"
 #include "../lib/serializer/Connection.h"
 #include "../lib/texts/CGeneralTextHandler.h"
@@ -28,64 +33,6 @@
 #include <boost/uuid/uuid_generators.hpp>
 #include <boost/program_options.hpp>
 
-template<typename T> class CApplyOnServer;
-
-class CBaseForServerApply
-{
-public:
-	virtual bool applyOnServerBefore(CVCMIServer * srv, CPack * pack) const =0;
-	virtual void applyOnServerAfter(CVCMIServer * srv, CPack * pack) const =0;
-	virtual ~CBaseForServerApply() {}
-	template<typename U> static CBaseForServerApply * getApplier(const U * t = nullptr)
-	{
-		return new CApplyOnServer<U>();
-	}
-};
-
-template <typename T> class CApplyOnServer : public CBaseForServerApply
-{
-public:
-	bool applyOnServerBefore(CVCMIServer * srv, CPack * pack) const override
-	{
-		T * ptr = static_cast<T *>(pack);
-		ClientPermissionsCheckerNetPackVisitor checker(*srv);
-		ptr->visit(checker);
-
-		if(checker.getResult())
-		{
-			ApplyOnServerNetPackVisitor applier(*srv);
-			ptr->visit(applier);
-			return applier.getResult();
-		}
-		else
-			return false;
-	}
-
-	void applyOnServerAfter(CVCMIServer * srv, CPack * pack) const override
-	{
-		T * ptr = static_cast<T *>(pack);
-		ApplyOnServerAfterAnnounceNetPackVisitor applier(*srv);
-		ptr->visit(applier);
-	}
-};
-
-template <>
-class CApplyOnServer<CPack> : public CBaseForServerApply
-{
-public:
-	bool applyOnServerBefore(CVCMIServer * srv, CPack * pack) const override
-	{
-		logGlobal->error("Cannot apply plain CPack!");
-		assert(0);
-		return false;
-	}
-	void applyOnServerAfter(CVCMIServer * srv, CPack * pack) const override
-	{
-		logGlobal->error("Cannot apply plain CPack!");
-		assert(0);
-	}
-};
-
 class CVCMIServerPackVisitor : public VCMI_LIB_WRAP_NAMESPACE(ICPackVisitor)
 {
 private:
@@ -126,8 +73,6 @@ CVCMIServer::CVCMIServer(uint16_t port, bool runByClient)
 {
 	uuid = boost::uuids::to_string(boost::uuids::random_generator()());
 	logNetwork->trace("CVCMIServer created! UUID: %s", uuid);
-	applier = std::make_shared<CApplier<CBaseForServerApply>>();
-	registerTypesLobbyPacks(*applier);
 
 	networkHandler = INetworkHandler::createHandler();
 }
@@ -376,9 +321,16 @@ void CVCMIServer::onDisconnected(const std::shared_ptr<INetworkConnection> & con
 
 void CVCMIServer::handleReceivedPack(std::unique_ptr<CPackForLobby> pack)
 {
-	CBaseForServerApply * apply = applier->getApplier(CTypeList::getInstance().getTypeID(pack.get()));
-	if(apply->applyOnServerBefore(this, pack.get()))
-		announcePack(std::move(pack));
+	ClientPermissionsCheckerNetPackVisitor checker(*this);
+	pack->visit(checker);
+
+	if(checker.getResult())
+	{
+		ApplyOnServerNetPackVisitor applier(*this);
+		pack->visit(applier);
+		if (applier.getResult())
+			announcePack(std::move(pack));
+	}
 }
 
 void CVCMIServer::announcePack(std::unique_ptr<CPackForLobby> pack)
@@ -392,7 +344,8 @@ void CVCMIServer::announcePack(std::unique_ptr<CPackForLobby> pack)
 		activeConnection->sendPack(pack.get());
 	}
 
-	applier->getApplier(CTypeList::getInstance().getTypeID(pack.get()))->applyOnServerAfter(this, pack.get());
+	ApplyOnServerAfterAnnounceNetPackVisitor applier(*this);
+	pack->visit(applier);
 }
 
 void CVCMIServer::announceMessage(const MetaString & txt)

server/CVCMIServer.h

@@ -52,7 +52,6 @@ class CVCMIServer : public LobbyInfo, public INetworkServerListener, public INet
 
 	std::unique_ptr<INetworkHandler> networkHandler;
 
-	std::shared_ptr<CApplier<CBaseForServerApply>> applier;
 	EServerState state = EServerState::LOBBY;
 
 	std::shared_ptr<CConnection> findConnection(const std::shared_ptr<INetworkConnection> &);