Move visit query to a separate class, remove no longer needed Cast.h

client/CServerHandler.cpp

@@ -51,7 +51,6 @@
 #include <boost/uuid/uuid.hpp>
 #include <boost/uuid/uuid_io.hpp>
 #include <boost/uuid/uuid_generators.hpp>
-#include "../lib/serializer/Cast.h"
 #include "LobbyClientNetPackVisitors.h"
 
 #include <vcmi/events/EventBus.h>

lib/CMakeLists.txt

@@ -620,7 +620,6 @@ set(lib_MAIN_HEADERS
 	serializer/JsonSerializeFormat.h
 	serializer/JsonSerializer.h
 	serializer/JsonUpdater.h
-	serializer/Cast.h
 	serializer/ESerializationVersion.h
 	serializer/RegisterTypes.h
 	serializer/Serializeable.h

lib/serializer/Cast.h

@@ -1,26 +0,0 @@
-/*
- * Cast.h, part of VCMI engine
- *
- * Authors: listed in file AUTHORS in main folder
- *
- * License: GNU General Public License v2.0 or later
- * Full text of license available in license.txt file, in main folder
- *
- */
-#pragma once
-
-VCMI_LIB_NAMESPACE_BEGIN
-
-template<class T, class F>
-inline const T * dynamic_ptr_cast(const F * ptr)
-{
-	return dynamic_cast<const T *>(ptr);
-}
-
-template<class T, class F>
-inline T * dynamic_ptr_cast(F * ptr)
-{
-	return dynamic_cast<T *>(ptr);
-}
-
-VCMI_LIB_NAMESPACE_END

server/CGameHandler.cpp

@@ -21,6 +21,7 @@
 #include "processors/TurnOrderProcessor.h"
 #include "queries/QueriesProcessor.h"
 #include "queries/MapQueries.h"
+#include "queries/VisitQueries.h"
 
 #include "../lib/ArtifactUtils.h"
 #include "../lib/CArtHandler.h"
@@ -2178,7 +2179,7 @@ bool CGameHandler::visitTownBuilding(ObjectInstanceID tid, BuildingID bid)
 		if (hero && t->town->buildings.at(bid)->manualHeroVisit)
 		{
 			// FIXME: query might produce unintended side effects, double check
-			auto visitQuery = std::make_shared<CObjectVisitQuery>(this, t, hero, t->visitablePos());
+			auto visitQuery = std::make_shared<CObjectVisitQuery>(this, t, hero);
 			queries->addQuery(visitQuery);
 			building->onHeroVisit(hero);
 			queries->popIfTop(visitQuery);
@@ -3226,7 +3227,7 @@ void CGameHandler::objectVisited(const CGObjectInstance * obj, const CGHeroInsta
 					visitedObject = visitedTown;
 			}
 		}
-		visitQuery = std::make_shared<CObjectVisitQuery>(this, visitedObject, h, visitedObject->visitablePos());
+		visitQuery = std::make_shared<CObjectVisitQuery>(this, visitedObject, h);
 		queries->addQuery(visitQuery); //TODO real visit pos
 
 		HeroVisit hv;

server/CMakeLists.txt

@@ -9,6 +9,7 @@ set(vcmiservercommon_SRCS
 		queries/BattleQueries.cpp
 		queries/CQuery.cpp
 		queries/MapQueries.cpp
+		queries/VisitQueries.cpp
 		queries/QueriesProcessor.cpp
 
 		processors/HeroPoolProcessor.cpp
@@ -36,6 +37,7 @@ set(vcmiservercommon_HEADERS
 		queries/BattleQueries.h
 		queries/CQuery.h
 		queries/MapQueries.h
+		queries/VisitQueries.h
 		queries/QueriesProcessor.h
 
 		processors/HeroPoolProcessor.h

server/NetPacksServer.cpp

@@ -27,7 +27,6 @@
 #include "../lib/battle/Unit.h"
 #include "../lib/spells/CSpellHandler.h"
 #include "../lib/spells/ISpellMechanics.h"
-#include "../lib/serializer/Cast.h"
 
 void ApplyGhNetPackVisitor::visitSaveGame(SaveGame & pack)
 {

server/battles/BattleResultProcessor.cpp

@@ -27,7 +27,6 @@
 #include "../../lib/mapObjects/CGTownInstance.h"
 #include "../../lib/networkPacks/PacksForClientBattle.h"
 #include "../../lib/networkPacks/PacksForClient.h"
-#include "../../lib/serializer/Cast.h"
 #include "../../lib/spells/CSpellHandler.h"
 
 #include <vstd/RNG.h>
@@ -81,7 +80,7 @@ CasualtiesAfterBattle::CasualtiesAfterBattle(const CBattleInfoCallback & battle,
 			else if(warMachine != ArtifactID::CATAPULT && st->getCount() <= 0)
 			{
 				logGlobal->debug("War machine has been destroyed");
-				auto hero = dynamic_ptr_cast<CGHeroInstance> (army);
+				auto hero = dynamic_cast<const CGHeroInstance*> (army);
 				if (hero)
 					removedWarMachines.push_back (ArtifactLocation(hero->id, hero->getArtPos(warMachine, true)));
 				else

server/queries/BattleQueries.cpp

@@ -21,7 +21,6 @@
 #include "../../lib/mapObjects/CGObjectInstance.h"
 #include "../../lib/mapObjects/CGTownInstance.h"
 #include "../../lib/networkPacks/PacksForServer.h"
-#include "../../lib/serializer/Cast.h"
 
 void CBattleQuery::notifyObjectAboutRemoval(const CGObjectInstance * visitedObject, const CGHeroInstance * visitingHero) const
 {
@@ -51,10 +50,10 @@ CBattleQuery::CBattleQuery(CGameHandler * owner):
 
 bool CBattleQuery::blocksPack(const CPack * pack) const
 {
-	if(dynamic_ptr_cast<MakeAction>(pack) != nullptr)
+	if(dynamic_cast<const MakeAction*>(pack) != nullptr)
 		return false;
 
-	if(dynamic_ptr_cast<GamePause>(pack) != nullptr)
+	if(dynamic_cast<const GamePause*>(pack) != nullptr)
 		return false;
 
 	return true;

server/queries/CQuery.cpp

@@ -14,7 +14,6 @@
 
 #include "../CGameHandler.h"
 
-#include "../../lib/serializer/Cast.h"
 #include "../../lib/networkPacks/PacksForServer.h"
 
 std::ostream & operator<<(std::ostream & out, const CQuery & query)
@@ -116,7 +115,7 @@ void CQuery::setReply(std::optional<int32_t> reply)
 bool CQuery::blockAllButReply(const CPack * pack) const
 {
 	//We accept only query replies from correct player
-	if(auto reply = dynamic_ptr_cast<QueryReply>(pack))
+	if(auto reply = dynamic_cast<const QueryReply*>(pack))
 		return !vstd::contains(players, reply->player);
 
 	return true;

server/queries/MapQueries.cpp

@@ -13,10 +13,9 @@
 #include "QueriesProcessor.h"
 #include "../CGameHandler.h"
 #include "../TurnTimerHandler.h"
-#include "../../lib/mapObjects/MiscObjects.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
+#include "../../lib/mapObjects/MiscObjects.h"
 #include "../../lib/networkPacks/PacksForServer.h"
-#include "../../lib/serializer/Cast.h"
 
 TimerPauseQuery::TimerPauseQuery(CGameHandler * owner, PlayerColor player):
 	CQuery(owner)
@@ -44,38 +43,6 @@ bool TimerPauseQuery::endsByPlayerAnswer() const
 	return true;
 }
 
-CObjectVisitQuery::CObjectVisitQuery(CGameHandler * owner, const CGObjectInstance * Obj, const CGHeroInstance * Hero, int3 Tile):
-	CQuery(owner), visitedObject(Obj), visitingHero(Hero), tile(Tile), removeObjectAfterVisit(false)
-{
-	addPlayer(Hero->tempOwner);
-}
-
-bool CObjectVisitQuery::blocksPack(const CPack *pack) const
-{
-	//During the visit itself ALL actions are blocked.
-	//(However, the visit may trigger a query above that'll pass some.)
-	return true;
-}
-
-void CObjectVisitQuery::onRemoval(PlayerColor color)
-{
-	gh->objectVisitEnded(*this);
-
-	//TODO or should it be destructor?
-	//Can object visit affect 2 players and what would be desired behavior?
-	if(removeObjectAfterVisit)
-		gh->removeObject(visitedObject, color);
-}
-
-void CObjectVisitQuery::onExposure(QueryPtr topQuery)
-{
-	//Object may have been removed and deleted.
-	if(gh->isValidObject(visitedObject))
-		topQuery->notifyObjectAboutRemoval(visitedObject, visitingHero);
-
-	owner->popIfTop(*this);
-}
-
 void CGarrisonDialogQuery::notifyObjectAboutRemoval(const CGObjectInstance * visitedObject, const CGHeroInstance * visitingHero) const
 {
 	visitedObject->garrisonDialogClosed(visitingHero);
@@ -97,22 +64,22 @@ bool CGarrisonDialogQuery::blocksPack(const CPack * pack) const
 	ourIds.insert(this->exchangingArmies[0]->id);
 	ourIds.insert(this->exchangingArmies[1]->id);
 
-	if(auto stacks = dynamic_ptr_cast<ArrangeStacks>(pack))
+	if(auto stacks = dynamic_cast<const ArrangeStacks*>(pack))
 		return !vstd::contains(ourIds, stacks->id1) || !vstd::contains(ourIds, stacks->id2);
 
-	if(auto stacks = dynamic_ptr_cast<BulkSplitStack>(pack))
+	if(auto stacks = dynamic_cast<const BulkSplitStack*>(pack))
 		return !vstd::contains(ourIds, stacks->srcOwner);
 
-	if(auto stacks = dynamic_ptr_cast<BulkMergeStacks>(pack))
+	if(auto stacks = dynamic_cast<const BulkMergeStacks*>(pack))
 		return !vstd::contains(ourIds, stacks->srcOwner);
 
-	if(auto stacks = dynamic_ptr_cast<BulkSmartSplitStack>(pack))
+	if(auto stacks = dynamic_cast<const BulkSmartSplitStack*>(pack))
 		return !vstd::contains(ourIds, stacks->srcOwner);
 
-	if(auto stacks = dynamic_ptr_cast<BulkMoveArmy>(pack))
+	if(auto stacks = dynamic_cast<const BulkMoveArmy*>(pack))
 		return !vstd::contains(ourIds, stacks->srcArmy) || !vstd::contains(ourIds, stacks->destArmy);
 
-	if(auto arts = dynamic_ptr_cast<ExchangeArtifacts>(pack))
+	if(auto arts = dynamic_cast<const ExchangeArtifacts*>(pack))
 	{
 		if(auto id1 = arts->src.artHolder)
 			if(!vstd::contains(ourIds, id1))
@@ -123,28 +90,28 @@ bool CGarrisonDialogQuery::blocksPack(const CPack * pack) const
 				return true;
 		return false;
 	}
-	if(auto dismiss = dynamic_ptr_cast<DisbandCreature>(pack))
+	if(auto dismiss = dynamic_cast<const DisbandCreature*>(pack))
 		return !vstd::contains(ourIds, dismiss->id);
 
-	if(auto arts = dynamic_ptr_cast<BulkExchangeArtifacts>(pack))
+	if(auto arts = dynamic_cast<const BulkExchangeArtifacts*>(pack))
 		return !vstd::contains(ourIds, arts->srcHero) || !vstd::contains(ourIds, arts->dstHero);
 
-	if(auto arts = dynamic_ptr_cast<ManageBackpackArtifacts>(pack))
+	if(auto arts = dynamic_cast<const ManageBackpackArtifacts*>(pack))
 		return !vstd::contains(ourIds, arts->artHolder);
 
-	if(auto art = dynamic_ptr_cast<EraseArtifactByClient>(pack))
+	if(auto art = dynamic_cast<const EraseArtifactByClient*>(pack))
 	{
 		if(auto id = art->al.artHolder)
 			return !vstd::contains(ourIds, id);
 	}
 
-	if(auto dismiss = dynamic_ptr_cast<AssembleArtifacts>(pack))
+	if(auto dismiss = dynamic_cast<const AssembleArtifacts*>(pack))
 		return !vstd::contains(ourIds, dismiss->heroID);
 
-	if(auto upgrade = dynamic_ptr_cast<UpgradeCreature>(pack))
+	if(auto upgrade = dynamic_cast<const UpgradeCreature*>(pack))
 		return !vstd::contains(ourIds, upgrade->id);
 
-	if(auto formation = dynamic_ptr_cast<SetFormation>(pack))
+	if(auto formation = dynamic_cast<const SetFormation*>(pack))
 		return !vstd::contains(ourIds, formation->hid);
 
 	return CDialogQuery::blocksPack(pack);
@@ -179,44 +146,44 @@ bool OpenWindowQuery::blocksPack(const CPack *pack) const
 {
 	if (mode == EOpenWindowMode::RECRUITMENT_FIRST || mode == EOpenWindowMode::RECRUITMENT_ALL)
 	{
-		if(dynamic_ptr_cast<RecruitCreatures>(pack) != nullptr)
+		if(dynamic_cast<const RecruitCreatures*>(pack) != nullptr)
 			return false;
 
 		// If hero has no free slots, he might get some stacks merged automatically
-		if(dynamic_ptr_cast<ArrangeStacks>(pack) != nullptr)
+		if(dynamic_cast<const ArrangeStacks*>(pack) != nullptr)
 			return false;
 	}
 
 	if (mode == EOpenWindowMode::TAVERN_WINDOW)
 	{
-		if(dynamic_ptr_cast<HireHero>(pack) != nullptr)
+		if(dynamic_cast<const HireHero*>(pack) != nullptr)
 			return false;
 	}
 
 	if (mode == EOpenWindowMode::UNIVERSITY_WINDOW)
 	{
-		if(dynamic_ptr_cast<TradeOnMarketplace>(pack) != nullptr)
+		if(dynamic_cast<const TradeOnMarketplace*>(pack) != nullptr)
 			return false;
 	}
 
 	if (mode == EOpenWindowMode::MARKET_WINDOW)
 	{
-		if(dynamic_ptr_cast<ExchangeArtifacts>(pack) != nullptr)
+		if(dynamic_cast<const ExchangeArtifacts*>(pack) != nullptr)
 			return false;
 
-		if(dynamic_ptr_cast<BulkExchangeArtifacts>(pack) != nullptr)
+		if(dynamic_cast<const BulkExchangeArtifacts*>(pack) != nullptr)
 			return false;
 
-		if(dynamic_ptr_cast<ManageBackpackArtifacts>(pack) != nullptr)
+		if(dynamic_cast<const ManageBackpackArtifacts*>(pack) != nullptr)
 			return false;
 
-		if(dynamic_ptr_cast<AssembleArtifacts>(pack))
+		if(dynamic_cast<const AssembleArtifacts*>(pack))
 			return false;
 
-		if(dynamic_ptr_cast<EraseArtifactByClient>(pack))
+		if(dynamic_cast<const EraseArtifactByClient*>(pack))
 			return false;
 
-		if(dynamic_ptr_cast<TradeOnMarketplace>(pack) != nullptr)
+		if(dynamic_cast<const TradeOnMarketplace*>(pack) != nullptr)
 			return false;
 	}
 
@@ -225,7 +192,6 @@ bool OpenWindowQuery::blocksPack(const CPack *pack) const
 
 void CTeleportDialogQuery::notifyObjectAboutRemoval(const CGObjectInstance * visitedObject, const CGHeroInstance * visitingHero) const
 {
-	// do not change to dynamic_ptr_cast - SIGSEGV!
 	auto obj = dynamic_cast<const CGTeleport*>(visitedObject);
 	if(obj)
 		obj->teleportDialogAnswered(visitingHero, *answer, td.exits);

server/queries/MapQueries.h

@@ -15,12 +15,8 @@
 VCMI_LIB_NAMESPACE_BEGIN
 class CGHeroInstance;
 class CGObjectInstance;
-class int3;
 VCMI_LIB_NAMESPACE_END
 
-
-class TurnTimerHandler;
-
 //Created when player starts turn or when player puts game on [ause
 //Removed when player accepts a turn or continur play
 class TimerPauseQuery : public CQuery
@@ -34,23 +30,6 @@ public:
 	bool endsByPlayerAnswer() const override;
 };
 
-//Created when hero visits object.
-//Removed when query above is resolved (or immediately after visit if no queries were created)
-class CObjectVisitQuery : public CQuery
-{
-public:
-	const CGObjectInstance *visitedObject;
-	const CGHeroInstance *visitingHero;
-	int3 tile; //may be different than hero pos -> eg. visit via teleport
-	bool removeObjectAfterVisit;
-
-	CObjectVisitQuery(CGameHandler * owner, const CGObjectInstance *Obj, const CGHeroInstance *Hero, int3 Tile);
-
-	bool blocksPack(const CPack *pack) const override;
-	void onRemoval(PlayerColor color) override;
-	void onExposure(QueryPtr topQuery) override;
-};
-
 //Created when hero attempts move and something happens
 //(not necessarily position change, could be just an object interaction).
 class CHeroMovementQuery : public CQuery

server/queries/VisitQueries.cpp

@@ -0,0 +1,47 @@
+/*
+ * VisitQueries.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#include "StdInc.h"
+#include "VisitQueries.h"
+
+#include "QueriesProcessor.h"
+#include "../CGameHandler.h"
+#include "../../lib/mapObjects/CGHeroInstance.h"
+
+CObjectVisitQuery::CObjectVisitQuery(CGameHandler * owner, const CGObjectInstance * Obj, const CGHeroInstance * Hero):
+	CQuery(owner), visitedObject(Obj), visitingHero(Hero), removeObjectAfterVisit(false)
+{
+	addPlayer(Hero->tempOwner);
+}
+
+bool CObjectVisitQuery::blocksPack(const CPack *pack) const
+{
+	//During the visit itself ALL actions are blocked.
+	//(However, the visit may trigger a query above that'll pass some.)
+	return true;
+}
+
+void CObjectVisitQuery::onRemoval(PlayerColor color)
+{
+	gh->objectVisitEnded(*this);
+
+	//TODO or should it be destructor?
+	//Can object visit affect 2 players and what would be desired behavior?
+	if(removeObjectAfterVisit)
+		gh->removeObject(visitedObject, color);
+}
+
+void CObjectVisitQuery::onExposure(QueryPtr topQuery)
+{
+	//Object may have been removed and deleted.
+	if(gh->isValidObject(visitedObject))
+		topQuery->notifyObjectAboutRemoval(visitedObject, visitingHero);
+
+	owner->popIfTop(*this);
+}

server/queries/VisitQueries.h

@@ -0,0 +1,29 @@
+/*
+ * VisitQueries.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#pragma once
+
+#include "CQuery.h"
+
+//Created when hero visits object.
+//Removed when query above is resolved (or immediately after visit if no queries were created)
+class CObjectVisitQuery : public CQuery
+{
+public:
+	const CGObjectInstance *visitedObject;
+	const CGHeroInstance *visitingHero;
+
+	bool removeObjectAfterVisit;
+
+	CObjectVisitQuery(CGameHandler * owner, const CGObjectInstance *Obj, const CGHeroInstance *Hero);
+
+	bool blocksPack(const CPack *pack) const override;
+	void onRemoval(PlayerColor color) override;
+	void onExposure(QueryPtr topQuery) override;
+};