Merge pull request #5498 from IvanSavenko/netlag_prep

Minor refactoring of GameHandler to help with antilag support

AI/Nullkiller/AIGateway.cpp

@@ -645,7 +645,7 @@ void AIGateway::showBlockingDialog(const std::string & text, const std::vector<C
 			bool answer = true;
 			auto objects = cb->getVisitableObjs(target);
 
-			if(hero.validAndSet() && target.valid() && objects.size())
+			if(hero.validAndSet() && target.isValid() && objects.size())
 			{
 				auto topObj = objects.front()->id == hero->id ? objects.back() : objects.front();
 				auto objType = topObj->ID; // top object should be our hero
@@ -722,7 +722,7 @@ void AIGateway::showTeleportDialog(const CGHeroInstance * hero, TeleportChannelI
 	{
 		nullkiller->memory->knownTeleportChannels[channel]->passability = TeleportChannel::IMPASSABLE;
 	}
-	else if(destinationTeleport != ObjectInstanceID() && destinationTeleportPos.valid())
+	else if(destinationTeleport != ObjectInstanceID() && destinationTeleportPos.isValid())
 	{
 		auto neededExit = std::make_pair(destinationTeleport, destinationTeleportPos);
 		if(destinationTeleport != ObjectInstanceID() && vstd::contains(exits, neededExit))
@@ -1352,7 +1352,7 @@ bool AIGateway::moveHeroToTile(int3 dst, HeroPtr h)
 			}
 
 			destinationTeleport = exitId;
-			if(exitPos.valid())
+			if(exitPos.isValid())
 				destinationTeleportPos = exitPos;
 			cb->moveHero(*h, h->pos, false);
 			destinationTeleport = ObjectInstanceID();

AI/Nullkiller/Analyzers/ObjectClusterizer.cpp

@@ -100,7 +100,7 @@ std::optional<const CGObjectInstance *> ObjectClusterizer::getBlocker(const AIPa
 		if (ai->cb->isVisible(node.coord))
 			blockers = ai->cb->getVisitableObjs(node.coord);
 
-		if(guardPos.valid() && ai->cb->isVisible(guardPos))
+		if(guardPos.isValid() && ai->cb->isVisible(guardPos))
 		{
 			auto guard = ai->cb->getTopObj(ai->cb->getGuardingCreaturePosition(node.coord));
 

AI/Nullkiller/Goals/ExploreNeighbourTile.cpp

@@ -54,7 +54,7 @@ void ExploreNeighbourTile::accept(AIGateway * ai)
 				}
 			});
 
-		if(!target.valid())
+		if(!target.isValid())
 		{
 			return;
 		}

AI/Nullkiller/Pathfinding/GraphPaths.h

@@ -41,7 +41,7 @@ struct GraphPathNodePointer
 
 	bool valid() const
 	{
-		return coord.valid();
+		return coord.isValid();
 	}
 };
 

AI/Nullkiller/Pathfinding/ObjectGraphCalculator.cpp

@@ -97,7 +97,7 @@ void ObjectGraphCalculator::addMinimalDistanceJunctions()
 			if(target->hasNodeAt(pos))
 				return;
 
-			if(ai->cb->getGuardingCreaturePosition(pos).valid())
+			if(ai->cb->getGuardingCreaturePosition(pos).isValid())
 				return;
 
 			ConnectionCostInfo currentCost = getConnectionsCost(paths);
@@ -200,7 +200,7 @@ void ObjectGraphCalculator::calculateConnections(const int3 & pos, std::vector<A
 			auto pos1 = path1.targetHero->visitablePos();
 			auto pos2 = path2.targetHero->visitablePos();
 
-			if(guardPos.valid() && guardPos != pos1 && guardPos != pos2)
+			if(guardPos.isValid() && guardPos != pos1 && guardPos != pos2)
 				continue;
 
 			auto obj1 = actorObjectMap[path1.targetHero];
@@ -310,7 +310,7 @@ void ObjectGraphCalculator::addObjectActor(const CGObjectInstance * obj)
 
 	auto shipyard = dynamic_cast<const IShipyard *>(obj);
 		
-	if(shipyard && shipyard->bestLocation().valid())
+	if(shipyard && shipyard->bestLocation().isValid())
 	{
 		int3 virtualBoat = shipyard->bestLocation();
 			

AI/VCAI/Pathfinding/PathfindingManager.cpp

@@ -130,7 +130,7 @@ Goals::TGoalVec PathfindingManager::findPath(
 #ifdef VCMI_TRACE_PATHFINDER
 		logAi->trace("Path found size=%i, first tile=%s", path.nodes.size(), firstTileToGet.toString());
 #endif
-		if(firstTileToGet.valid() && ai->isTileNotReserved(hero.get(), firstTileToGet))
+		if(firstTileToGet.isValid() && ai->isTileNotReserved(hero.get(), firstTileToGet))
 		{
 			danger = path.getTotalDanger(hero);
 

AI/VCAI/VCAI.cpp

@@ -699,7 +699,7 @@ void VCAI::showTeleportDialog(const CGHeroInstance * hero, TeleportChannelID cha
 	{
 		knownTeleportChannels[channel]->passability = TeleportChannel::IMPASSABLE;
 	}
-	else if(destinationTeleport != ObjectInstanceID() && destinationTeleportPos.valid())
+	else if(destinationTeleport != ObjectInstanceID() && destinationTeleportPos.isValid())
 	{
 		auto neededExit = std::make_pair(destinationTeleport, destinationTeleportPos);
 		if(destinationTeleport != ObjectInstanceID() && vstd::contains(exits, neededExit))
@@ -1291,7 +1291,7 @@ bool VCAI::isGoodForVisit(const CGObjectInstance * obj, HeroPtr h, const AIPath
 {
 	const int3 pos = obj->visitablePos();
 	const int3 targetPos = path.firstTileToGet();
-	if (!targetPos.valid())
+	if (!targetPos.isValid())
 		return false;
 	if (!isTileNotReserved(h.get(), targetPos))
 		return false;
@@ -1320,7 +1320,7 @@ bool VCAI::isGoodForVisit(const CGObjectInstance * obj, HeroPtr h, const AIPath
 
 bool VCAI::isTileNotReserved(const CGHeroInstance * h, int3 t) const
 {
-	if(t.valid())
+	if(t.isValid())
 	{
 		auto obj = cb->getTopObj(t);
 		if(obj && vstd::contains(ai->reservedObjs, obj)
@@ -1900,7 +1900,7 @@ bool VCAI::moveHeroToTile(int3 dst, HeroPtr h)
 		auto doTeleportMovement = [&](ObjectInstanceID exitId, int3 exitPos)
 		{
 			destinationTeleport = exitId;
-			if(exitPos.valid())
+			if(exitPos.isValid())
 				destinationTeleportPos = exitPos;
 			cb->moveHero(*h, h->pos, false);
 			destinationTeleport = ObjectInstanceID();

client/HeroMovementController.cpp

@@ -120,7 +120,7 @@ void HeroMovementController::updatePath(const CGHeroInstance * hero, const TryMo
 
 	assert(GAME->interface()->makingTurn);
 
-	bool directlyAttackingCreature = details.attackedFrom.has_value() && GAME->interface()->localState->getPath(hero).lastNode().coord == details.attackedFrom;
+	bool directlyAttackingCreature = details.attackedFrom.isValid() && GAME->interface()->localState->getPath(hero).lastNode().coord == details.attackedFrom;
 
 	int3 desiredTarget = GAME->interface()->localState->getPath(hero).nextNode().coord;
 	int3 actualTarget = hero->convertToVisitablePos(details.end);
@@ -163,7 +163,7 @@ void HeroMovementController::onTryMoveHero(const CGHeroInstance * hero, const Tr
 	}
 
 	bool directlyAttackingCreature =
-		details.attackedFrom.has_value() &&
+		details.attackedFrom.isValid() &&
 		GAME->interface()->localState->hasPath(hero) &&
 		GAME->interface()->localState->getPath(hero).lastNode().coord == details.attackedFrom;
 
@@ -194,7 +194,7 @@ void HeroMovementController::onTryMoveHero(const CGHeroInstance * hero, const Tr
 	if(directlyAttackingCreature)
 	{
 		// Get direction to attacker.
-		int3 posOffset = *details.attackedFrom - details.end + int3(2, 1, 0);
+		int3 posOffset = details.attackedFrom - details.end + int3(2, 1, 0);
 		static const ui8 dirLookup[3][3] =
 		{
 			{ 1, 2, 3 },

client/windows/CCastleInterface.cpp

@@ -675,7 +675,7 @@ void CCastleBuildings::recreate()
 	if(town->hasBuilt(BuildingID::SHIPYARD))
 	{
 		auto bayPos = town->bestLocation();
-		if(!bayPos.valid())
+		if(!bayPos.isValid())
 			logGlobal->warn("Shipyard in non-coastal town!");
 		std::vector <const CGObjectInstance *> vobjs = GAME->interface()->cb->getVisitableObjs(bayPos, false);
 		//there is visitable obj at shipyard output tile and it's a boat or hero (on boat)

lib/constants/EntityIdentifiers.cpp

@@ -628,6 +628,22 @@ std::string GameResID::entityType()
 	return "resource";
 }
 
+const std::array<PlayerColor, PlayerColor::PLAYER_LIMIT_I> & PlayerColor::ALL_PLAYERS()
+{
+	static const std::array allPlayers = {
+		PlayerColor(0),
+		PlayerColor(1),
+		PlayerColor(2),
+		PlayerColor(3),
+		PlayerColor(4),
+		PlayerColor(5),
+		PlayerColor(6),
+		PlayerColor(7)
+	};
+
+	return allPlayers;
+}
+
 const std::array<PrimarySkill, 4> & PrimarySkill::ALL_SKILLS()
 {
 	static const std::array allSkills = {

lib/constants/EntityIdentifiers.h

@@ -144,6 +144,8 @@ public:
 	static const PlayerColor NEUTRAL; //255
 	static const PlayerColor PLAYER_LIMIT; //player limit per map
 
+	static const std::array<PlayerColor, PLAYER_LIMIT_I> & ALL_PLAYERS();
+
 	bool isValidPlayer() const; //valid means < PLAYER_LIMIT (especially non-neutral)
 	bool isSpectator() const;
 

lib/gameState/CGameState.cpp

@@ -1051,9 +1051,9 @@ BattleInfo * CGameState::getBattle(const BattleID & battle)
 
 BattleField CGameState::battleGetBattlefieldType(int3 tile, vstd::RNG & rand)
 {
-	assert(tile.valid());
+	assert(tile.isValid());
 
-	if(!tile.valid())
+	if(!tile.isValid())
 		return BattleField::NONE;
 
 	const TerrainTile &t = map->getTile(tile);

lib/int3.h

@@ -158,7 +158,7 @@ public:
 		return result;
 	}
 
-	constexpr bool valid() const //Should be named "isValid"?
+	constexpr bool isValid() const //Should be named "isValid"?
 	{
 		return z >= 0; //minimal condition that needs to be fulfilled for tiles in the map
 	}

lib/mapObjects/IObjectInterface.cpp

@@ -112,7 +112,7 @@ IBoatGenerator::EGeneratorState IBoatGenerator::shipyardStatus() const
 {
 	int3 tile = bestLocation();
 
-	if(!tile.valid())
+	if(!tile.isValid())
 		return TILE_BLOCKED; //no available water
 
 	const TerrainTile *t = getObject()->cb->getTile(tile);

lib/mapping/MapFormatH3M.cpp

@@ -2749,7 +2749,7 @@ void CMapLoaderH3M::afterRead()
 	for(auto & p : map->players)
 	{
 		int3 posOfMainTown = p.posOfMainTown;
-		if(posOfMainTown.valid() && map->isInTheMap(posOfMainTown))
+		if(posOfMainTown.isValid() && map->isInTheMap(posOfMainTown))
 		{
 			const TerrainTile & t = map->getTile(posOfMainTown);
 

lib/mapping/MapFormatJson.cpp

@@ -401,7 +401,7 @@ void CMapFormatJson::serializePlayerInfo(JsonSerializeFormat & handler)
 		}
 
 		//saving whole structure only if position is valid
-		if(!handler.saving || info.posOfMainTown.valid())
+		if(!handler.saving || info.posOfMainTown.isValid())
 		{
 			auto mainTown = handler.enterStruct("mainTown");
 			handler.serializeBool("generateHero", info.generateHeroAtMainTown);
@@ -411,7 +411,7 @@ void CMapFormatJson::serializePlayerInfo(JsonSerializeFormat & handler)
 		}
 		if(!handler.saving)
 		{
-			info.hasMainTown = info.posOfMainTown.valid();
+			info.hasMainTown = info.posOfMainTown.isValid();
 		}
 
 		handler.serializeString("mainHero", info.mainHeroInstance);//must be before "heroes"
@@ -1315,7 +1315,7 @@ void CMapSaverJson::writeObjects()
 		obj->serializeJson(handler);
 	}
 
-	if(map->grailPos.valid())
+	if(map->grailPos.isValid())
 	{
 		JsonNode grail;
 		grail["type"].String() = "grail";

lib/mapping/ObstacleProxy.cpp

@@ -34,7 +34,7 @@ void ObstacleProxy::collectPossibleObstacles(TerrainId terrain)
 			{
 				for(const auto & temp : handler->getTemplates())
 				{
-					if(temp->canBePlacedAt(terrain) && temp->getBlockMapOffset().valid())
+					if(temp->canBePlacedAt(terrain) && temp->getBlockMapOffset().isValid())
 						obstaclesBySize[temp->getBlockedOffsets().size()].push_back(temp);
 				}
 			}
@@ -193,7 +193,7 @@ bool ObstacleProxy::prepareBiome(const ObstacleSetFilter & filter, vstd::RNG & r
 		{
 			for (const auto & temp : os->getObstacles())
 			{
-				if(temp->getBlockMapOffset().valid())
+				if(temp->getBlockMapOffset().isValid())
 				{
 					obstaclesBySize[temp->getBlockedOffsets().size()].push_back(temp);
 				}

lib/networkPacks/NetPacksBase.h

@@ -21,10 +21,6 @@ class ICPackVisitor;
 
 struct DLL_LINKAGE CPack : public Serializeable
 {
-	/// Pointer to connection that pack received from
-	/// Only set & used on server
-	std::shared_ptr<CConnection> c;
-
 	CPack() = default;
 	virtual ~CPack() = default;
 

lib/networkPacks/PacksForClient.h

@@ -640,13 +640,20 @@ struct DLL_LINKAGE TryMoveHero : public CPackForClient
 		DISEMBARK
 	};
 
+	/// ID of moved hero
 	ObjectInstanceID id;
+	/// Movement points that hero will have after movement
 	ui32 movePoints = 0;
-	EResult result = FAILED; //uses EResult
-	int3 start; //h3m format
+	/// Result of movement attempt. FAILED should generally never happen unless client requested invalid operation
+	EResult result = FAILED;
+	/// Hero anchor position from which hero moves
+	int3 start;
+	/// Hero anchor position to which hero moves
 	int3 end;
-	std::unordered_set<int3> fowRevealed; //revealed tiles
-	std::optional<int3> attackedFrom; // Set when stepping into endangered tile.
+	/// Tiles that were revealed by this move
+	std::unordered_set<int3> fowRevealed;
+	/// If hero moves on guarded tile, this field will be set to visitable pos of attacked wandering monster
+	int3 attackedFrom;
 
 	void visitTyped(ICPackVisitor & visitor) override;
 

lib/pathfinder/CGPathNode.cpp

@@ -106,7 +106,7 @@ void PathNodeInfo::setNode(CGameState * gs, CGPathNode * n)
 
 	if(coord != node->coord)
 	{
-		assert(node->coord.valid());
+		assert(node->coord.isValid());
 
 		coord = node->coord;
 		tile = gs->getTile(coord);
@@ -131,7 +131,7 @@ void PathNodeInfo::setNode(CGameState * gs, CGPathNode * n)
 
 void PathNodeInfo::updateInfo(CPathfinderHelper * hlp, CGameState * gs)
 {
-	if(gs->guardingCreaturePosition(node->coord).valid() && !isInitialPosition)
+	if(gs->guardingCreaturePosition(node->coord).isValid() && !isInitialPosition)
 	{
 		guarded = true;
 	}

lib/pathfinder/NodeStorage.cpp

@@ -100,7 +100,7 @@ std::vector<CGPathNode *> NodeStorage::calculateTeleportations(
 	{
 		auto * node = getNode(neighbour, source.node->layer);
 
-		if(!node->coord.valid())
+		if(!node->coord.isValid())
 		{
 			logAi->debug("Teleportation exit is blocked " + neighbour.toString());
 			continue;
@@ -132,7 +132,7 @@ std::vector<CGPathNode *> NodeStorage::getInitialNodes()
 	initialNode->moveRemains = out.hero->movementPointsRemaining();
 	initialNode->setCost(0.0);
 
-	if(!initialNode->coord.valid())
+	if(!initialNode->coord.isValid())
 	{
 		initialNode->coord = out.hpos;
 	}

lib/pathfinder/PathfinderUtil.h

@@ -63,7 +63,7 @@ namespace PathfinderUtil
 			{
 				return EPathAccessibility::BLOCKED;
 			}
-			else if(gs->guardingCreaturePosition(pos).valid())
+			else if(gs->guardingCreaturePosition(pos).isValid())
 			{
 				// Monster close by; blocked visit for battle
 				return EPathAccessibility::GUARDED;

lib/rmg/RmgObject.cpp

@@ -96,7 +96,7 @@ void Object::Instance::setPosition(const int3 & position)
 
 void Object::Instance::setPositionRaw(const int3 & position)
 {
-	if(!dObject.anchorPos().valid())
+	if(!dObject.anchorPos().isValid())
 	{
 		dObject.setAnchorPos(dPosition + dParent.getPosition());
 		dBlockedAreaCache.clear();

lib/rmg/RmgPath.cpp

@@ -99,7 +99,7 @@ Path Path::search(const Tileset & dst, bool straight, std::function<float(const
 		{
 			// Trace the path using the saved parent information and return path
 			int3 backTracking = currentNode;
-			while (cameFrom[backTracking].valid())
+			while (cameFrom[backTracking].isValid())
 			{
 				result.dPath.add(backTracking);
 				backTracking = cameFrom[backTracking];

lib/rmg/Zone.cpp

@@ -347,7 +347,7 @@ void Zone::fractalize()
 			}
 			
 			possibleTiles.subtract(tilesToIgnore);
-			if(!nodeFound.valid()) //nothing else can be done (?)
+			if(!nodeFound.isValid()) //nothing else can be done (?)
 				break;
 			tilesToIgnore.clear();
 		}

lib/rmg/modificators/ConnectionsPlacer.cpp

@@ -273,7 +273,7 @@ void ConnectionsPlacer::selfSideDirectConnection(const rmg::ZoneConnection & con
 			}
 		}
 		
-		if(guardPos.valid())
+		if(guardPos.isValid())
 		{
 			assert(zone.getModificator<ObjectManager>());
 			auto & manager = *zone.getModificator<ObjectManager>();

lib/rmg/modificators/ObjectManager.cpp

@@ -242,7 +242,7 @@ int3 ObjectManager::findPlaceForObject(const rmg::Area & searchArea, rmg::Object
 		}
 	}
 	
-	if(result.valid())
+	if(result.isValid())
 		obj.setPosition(result);
 	return result;
 }
@@ -348,7 +348,7 @@ rmg::Path ObjectManager::placeAndConnectObject(const rmg::Area & searchArea, rmg
 	while(true)
 	{
 		pos = findPlaceForObject(possibleArea, obj, weightFunction, optimizer);
-		if(!pos.valid())
+		if(!pos.isValid())
 		{
 			return rmg::Path::invalid();
 		}

lib/spells/AdventureSpellMechanics.cpp

@@ -603,7 +603,7 @@ ESpellCastResult TownPortalMechanics::beginCast(SpellCastEnvironment * env, cons
 		return ESpellCastResult::CANCEL;
 	}
 
-	if(!parameters.pos.valid() && parameters.caster->getSpellSchoolLevel(owner) >= 2)
+	if(!parameters.pos.isValid() && parameters.caster->getSpellSchoolLevel(owner) >= 2)
 	{
 		auto queryCallback = [this, env, parameters](std::optional<int32_t> reply) -> void
 		{

server/CGameHandler.cpp

@@ -129,9 +129,9 @@ events::EventBus * CGameHandler::eventBus() const
 	return serverEventBus.get();
 }
 
-CVCMIServer * CGameHandler::gameLobby() const
+CVCMIServer & CGameHandler::gameLobby() const
 {
-	return lobby;
+	return *lobby;
 }
 
 void CGameHandler::levelUpHero(const CGHeroInstance * hero, SecondarySkill skill)
@@ -421,7 +421,7 @@ void CGameHandler::changeSecSkill(const CGHeroInstance * hero, SecondarySkill wh
 
 void CGameHandler::handleClientDisconnection(std::shared_ptr<CConnection> c)
 {
-	if(lobby->getState() == EServerState::SHUTDOWN || !gs || !gs->scenarioOps)
+	if(gameLobby().getState() == EServerState::SHUTDOWN || !gs || !gs->scenarioOps)
 	{
 		assert(0); // game should have shut down before reaching this point!
 		return;
@@ -461,7 +461,7 @@ void CGameHandler::handleClientDisconnection(std::shared_ptr<CConnection> c)
 	}
 }
 
-void CGameHandler::handleReceivedPack(CPackForServer & pack)
+void CGameHandler::handleReceivedPack(std::shared_ptr<CConnection> connection, CPackForServer & pack)
 {
 	//prepare struct informing that action was applied
 	auto sendPackageResponse = [&](bool successfullyApplied)
@@ -471,7 +471,7 @@ void CGameHandler::handleReceivedPack(CPackForServer & pack)
 		applied.result = successfullyApplied;
 		applied.packType = CTypeList::getInstance().getTypeID(&pack);
 		applied.requestID = pack.requestID;
-		pack.c->sendPack(applied);
+		connection->sendPack(applied);
 	};
 
 	if(isBlockedByQueries(&pack, pack.player))
@@ -483,7 +483,7 @@ void CGameHandler::handleReceivedPack(CPackForServer & pack)
 		bool result;
 		try
 		{
-			ApplyGhNetPackVisitor applier(*this);
+			ApplyGhNetPackVisitor applier(*this, connection);
 			pack.visit(applier);
 			result = applier.getResult();
 		}
@@ -709,9 +709,9 @@ void CGameHandler::start(bool resume)
 {
 	LOG_TRACE_PARAMS(logGlobal, "resume=%d", resume);
 
-	for (auto cc : lobby->activeConnections)
+	for (auto cc : gameLobby().activeConnections)
 	{
-		auto players = lobby->getAllClientPlayers(cc->connectionID);
+		auto players = gameLobby().getAllClientPlayers(cc->connectionID);
 		std::stringstream sbuffer;
 		sbuffer << "Connection " << cc->connectionID << " will handle " << players.size() << " player: ";
 		for (PlayerColor color : players)
@@ -924,7 +924,7 @@ bool CGameHandler::moveHero(ObjectInstanceID hid, int3 dst, EMovementMode moveme
 			leaveTile();
 
 		if (lookForGuards == CHECK_FOR_GUARDS && isInTheMap(guardPos))
-			tmh.attackedFrom = std::make_optional(guardPos);
+			tmh.attackedFrom = guardPos;
 
 		tmh.result = result;
 		sendAndApply(tmh);
@@ -1439,7 +1439,7 @@ void CGameHandler::heroExchange(ObjectInstanceID hero1, ObjectInstanceID hero2)
 void CGameHandler::sendToAllClients(CPackForClient & pack)
 {
 	logNetwork->trace("\tSending to all clients: %s", typeid(pack).name());
-	for (auto c : lobby->activeConnections)
+	for (auto c : gameLobby().activeConnections)
 		c->sendPack(pack);
 }
 
@@ -1468,64 +1468,62 @@ void CGameHandler::sendAndApply(NewStructures & pack)
 	checkVictoryLossConditionsForPlayer(getTown(pack.tid)->tempOwner);
 }
 
-bool CGameHandler::isPlayerOwns(CPackForServer * pack, ObjectInstanceID id)
+bool CGameHandler::isPlayerOwns(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack, ObjectInstanceID id)
 {
-	return pack->player == getOwner(id) && hasPlayerAt(getOwner(id), pack->c);
+	return pack->player == getOwner(id) && hasPlayerAt(getOwner(id), connection);
 }
 
-void CGameHandler::throwNotAllowedAction(CPackForServer * pack)
+void CGameHandler::throwNotAllowedAction(const std::shared_ptr<CConnection> & connection)
 {
-	if(pack->c)
-		playerMessages->sendSystemMessage(pack->c, MetaString::createFromTextID("vcmi.server.errors.notAllowed"));
+	playerMessages->sendSystemMessage(connection, MetaString::createFromTextID("vcmi.server.errors.notAllowed"));
 
 	logNetwork->error("Player is not allowed to perform this action!");
 	throw ExceptionNotAllowedAction();
 }
 
-void CGameHandler::wrongPlayerMessage(CPackForServer * pack, PlayerColor expectedplayer)
+void CGameHandler::wrongPlayerMessage(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack, PlayerColor expectedplayer)
 {
 	auto str = MetaString::createFromTextID("vcmi.server.errors.wrongIdentified");
 	str.appendName(pack->player);
 	str.appendName(expectedplayer);
 	logNetwork->error(str.toString());
 
-	if(pack->c)
-		playerMessages->sendSystemMessage(pack->c, str);
+	playerMessages->sendSystemMessage(connection, str);
 }
 
-void CGameHandler::throwIfWrongOwner(CPackForServer * pack, ObjectInstanceID id)
+void CGameHandler::throwIfWrongOwner(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack, ObjectInstanceID id)
 {
-	if(!isPlayerOwns(pack, id))
+	if(!isPlayerOwns(connection, pack, id))
 	{
-		wrongPlayerMessage(pack, getOwner(id));
-		throwNotAllowedAction(pack);
+		wrongPlayerMessage(connection, pack, getOwner(id));
+		throwNotAllowedAction(connection);
 	}
 }
 
-void CGameHandler::throwIfPlayerNotActive(CPackForServer * pack)
+void CGameHandler::throwIfPlayerNotActive(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack)
 {
 	if (!turnOrder->isPlayerMakingTurn(pack->player))
-		throwNotAllowedAction(pack);
+		throwNotAllowedAction(connection);
 }
 
-void CGameHandler::throwIfWrongPlayer(CPackForServer * pack)
+void CGameHandler::throwIfWrongPlayer(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack)
 {
-	throwIfWrongPlayer(pack, pack->player);
+	throwIfWrongPlayer(connection, pack, pack->player);
 }
 
-void CGameHandler::throwIfWrongPlayer(CPackForServer * pack, PlayerColor player)
+void CGameHandler::throwIfWrongPlayer(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack, PlayerColor player)
 {
-	if(!hasPlayerAt(player, pack->c) || pack->player != player)
+	if(!hasPlayerAt(player, connection) || pack->player != player)
 	{
-		wrongPlayerMessage(pack, player);
-		throwNotAllowedAction(pack);
+		wrongPlayerMessage(connection, pack, player);
+		throwNotAllowedAction(connection);
 	}
 }
 
-void CGameHandler::throwAndComplain(CPackForServer * pack, std::string txt)
+void CGameHandler::throwAndComplain(const std::shared_ptr<CConnection> & connection, std::string txt)
 {
 	complain(txt);
-	throwNotAllowedAction(pack);
+	throwNotAllowedAction(connection);
 }
 
 void CGameHandler::save(const std::string & filename)
@@ -1586,7 +1584,7 @@ bool CGameHandler::load(const std::string & filename)
 			errorMsg.appendRawString("\n");
 			errorMsg.appendRawString(e.whatExcessive());
 		}
-		lobby->announceMessage(errorMsg);
+		gameLobby().announceMessage(errorMsg);
 		return false;
 	}
 	catch(const IdentifierResolutionException & e)
@@ -1595,7 +1593,7 @@ bool CGameHandler::load(const std::string & filename)
 		MetaString errorMsg;
 		errorMsg.appendTextID("vcmi.server.errors.unknownEntity");
 		errorMsg.replaceRawString(e.identifierName);
-		lobby->announceMessage(errorMsg);
+		gameLobby().announceMessage(errorMsg);
 		return false;
 	}
 
@@ -1605,11 +1603,11 @@ bool CGameHandler::load(const std::string & filename)
 		auto str = MetaString::createFromTextID("vcmi.broadcast.failedLoadGame");
 		str.appendRawString(": ");
 		str.appendRawString(e.what());
-		lobby->announceMessage(str);
+		gameLobby().announceMessage(str);
 		return false;
 	}
 	gs->preInit(LIBRARY, this);
-	gs->updateOnLoad(lobby->si.get());
+	gs->updateOnLoad(gameLobby().si.get());
 	return true;
 }
 
@@ -3542,7 +3540,7 @@ void CGameHandler::checkVictoryLossConditionsForPlayer(PlayerColor player)
 
 			if(p->human)
 			{
-				lobby->setState(EServerState::SHUTDOWN);
+				gameLobby().setState(EServerState::SHUTDOWN);
 			}
 		}
 		else

server/CGameHandler.h

@@ -85,7 +85,7 @@ public:
 	const GameCb * game() const override;
 	vstd::CLoggerBase * logger() const override;
 	events::EventBus * eventBus() const override;
-	CVCMIServer * gameLobby() const;
+	CVCMIServer & gameLobby() const;
 
 	bool isValidObject(const CGObjectInstance *obj) const;
 	bool isBlockedByQueries(const CPackForServer *pack, PlayerColor player);
@@ -194,7 +194,7 @@ public:
 
 	void init(StartInfo *si, Load::ProgressAccumulator & progressTracking);
 	void handleClientDisconnection(std::shared_ptr<CConnection> c);
-	void handleReceivedPack(CPackForServer & pack);
+	void handleReceivedPack(std::shared_ptr<CConnection> c, CPackForServer & pack);
 	bool hasPlayerAt(PlayerColor player, std::shared_ptr<CConnection> c) const;
 	bool hasBothPlayersAtSameConnection(PlayerColor left, PlayerColor right) const;
 
@@ -266,19 +266,19 @@ public:
 	void sendAndApply(SetResources & pack);
 	void sendAndApply(NewStructures & pack);
 
-	void wrongPlayerMessage(CPackForServer * pack, PlayerColor expectedplayer);
+	void wrongPlayerMessage(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack, PlayerColor expectedplayer);
 	/// Unconditionally throws with "Action not allowed" message
-	[[noreturn]] void throwNotAllowedAction(CPackForServer * pack);
+	[[noreturn]] void throwNotAllowedAction(const std::shared_ptr<CConnection> & connection);
 	/// Throws if player stated in pack is not making turn right now
-	void throwIfPlayerNotActive(CPackForServer * pack);
+	void throwIfPlayerNotActive(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack);
 	/// Throws if object is not owned by pack sender
-	void throwIfWrongOwner(CPackForServer * pack, ObjectInstanceID id);
+	void throwIfWrongOwner(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack, ObjectInstanceID id);
 	/// Throws if player is not present on connection of this pack
-	void throwIfWrongPlayer(CPackForServer * pack, PlayerColor player);
-	void throwIfWrongPlayer(CPackForServer * pack);
-	[[noreturn]] void throwAndComplain(CPackForServer * pack, std::string txt);
+	void throwIfWrongPlayer(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack, PlayerColor player);
+	void throwIfWrongPlayer(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack);
+	[[noreturn]] void throwAndComplain(const std::shared_ptr<CConnection> & connection, std::string txt);
 
-	bool isPlayerOwns(CPackForServer * pack, ObjectInstanceID id);
+	bool isPlayerOwns(const std::shared_ptr<CConnection> & connection, const CPackForServer * pack, ObjectInstanceID id);
 
 	void start(bool resume);
 	void tick(int millisecondsPassed);

server/CVCMIServer.cpp

@@ -39,10 +39,13 @@ class CVCMIServerPackVisitor : public VCMI_LIB_WRAP_NAMESPACE(ICPackVisitor)
 private:
 	CVCMIServer & handler;
 	std::shared_ptr<CGameHandler> gh;
+	std::shared_ptr<CConnection> connection;
 
 public:
-	CVCMIServerPackVisitor(CVCMIServer & handler, std::shared_ptr<CGameHandler> gh)
-			:handler(handler), gh(gh)
+	CVCMIServerPackVisitor(CVCMIServer & handler, const std::shared_ptr<CGameHandler> & gh, const std::shared_ptr<CConnection> & connection)
+		: handler(handler)
+		, gh(gh)
+		, connection(connection)
 	{
 	}
 
@@ -50,13 +53,13 @@ public:
 
 	void visitForLobby(CPackForLobby & packForLobby) override
 	{
-		handler.handleReceivedPack(packForLobby);
+		handler.handleReceivedPack(connection, packForLobby);
 	}
 
 	void visitForServer(CPackForServer & serverPack) override
 	{
 		if (gh)
-			gh->handleReceivedPack(serverPack);
+			gh->handleReceivedPack(connection, serverPack);
 		else
 			logNetwork->error("Received pack for game server while in lobby!");
 	}
@@ -130,8 +133,7 @@ void CVCMIServer::onPacketReceived(const std::shared_ptr<INetworkConnection> & c
 		throw std::out_of_range("Unknown connection received in CVCMIServer::findConnection");
 
 	auto pack = c->retrievePack(message);
-	pack->c = c;
-	CVCMIServerPackVisitor visitor(*this, this->gh);
+	CVCMIServerPackVisitor visitor(*this, this->gh, c);
 	pack->visit(visitor);
 }
 
@@ -310,20 +312,19 @@ void CVCMIServer::onDisconnected(const std::shared_ptr<INetworkConnection> & con
 	if (c)
 	{
 		LobbyClientDisconnected lcd;
-		lcd.c = c;
 		lcd.clientId = c->connectionID;
-		handleReceivedPack(lcd);
+		handleReceivedPack(c, lcd);
 	}
 }
 
-void CVCMIServer::handleReceivedPack(CPackForLobby & pack)
+void CVCMIServer::handleReceivedPack(std::shared_ptr<CConnection> connection, CPackForLobby & pack)
 {
-	ClientPermissionsCheckerNetPackVisitor checker(*this);
+	ClientPermissionsCheckerNetPackVisitor checker(*this, connection);
 	pack.visit(checker);
 
 	if(checker.getResult())
 	{
-		ApplyOnServerNetPackVisitor applier(*this);
+		ApplyOnServerNetPackVisitor applier(*this, connection);
 		pack.visit(applier);
 		if (applier.getResult())
 			announcePack(pack);
@@ -397,6 +398,7 @@ void CVCMIServer::clientConnected(std::shared_ptr<CConnection> c, std::vector<st
 	assert(getState() == EServerState::LOBBY);
 
 	c->connectionID = currentClientId++;
+	c->uuid = uuid;
 
 	if(hostClientId == -1)
 	{
@@ -445,41 +447,6 @@ void CVCMIServer::clientDisconnected(std::shared_ptr<CConnection> connection)
 	{
 		gh->handleClientDisconnection(connection);
 	}
-
-//	PlayerReinitInterface startAiPack;
-//	startAiPack.playerConnectionId = PlayerSettings::PLAYER_AI;
-//
-//	for(auto it = playerNames.begin(); it != playerNames.end();)
-//	{
-//		if(it->second.connection != c->connectionID)
-//		{
-//			++it;
-//			continue;
-//		}
-//
-//		int id = it->first;
-//		std::string playerLeftMsgText = boost::str(boost::format("%s (pid %d cid %d) left the game") % id % playerNames[id].name % c->connectionID);
-//		announceTxt(playerLeftMsgText); //send lobby text, it will be ignored for non-lobby clients
-//		auto * playerSettings = si->getPlayersSettings(id);
-//		if(!playerSettings)
-//		{
-//			++it;
-//			continue;
-//		}
-//
-//		it = playerNames.erase(it);
-//		setPlayerConnectedId(*playerSettings, PlayerSettings::PLAYER_AI);
-//
-//		if(gh && si && state == EServerState::GAMEPLAY)
-//		{
-//			gh->playerMessages->broadcastMessage(playerSettings->color, playerLeftMsgText);
-//	//		gh->connections[playerSettings->color].insert(hostClient);
-//			startAiPack.players.push_back(playerSettings->color);
-//		}
-//	}
-//
-//	if(!startAiPack.players.empty())
-//		gh->sendAndApply(startAiPack);
 }
 
 void CVCMIServer::reconnectPlayer(int connId)

server/CVCMIServer.h

@@ -102,7 +102,7 @@ public:
 	void announceMessage(const MetaString & txt);
 	void announceMessage(const std::string & txt);
 
-	void handleReceivedPack(CPackForLobby & pack);
+	void handleReceivedPack(std::shared_ptr<CConnection> connection, CPackForLobby & pack);
 
 	void updateAndPropagateLobbyState();
 

server/LobbyNetPackVisitors.h

@@ -14,12 +14,15 @@
 class ClientPermissionsCheckerNetPackVisitor : public VCMI_LIB_WRAP_NAMESPACE(ICPackVisitor)
 {
 private:
-	bool result;
+	std::shared_ptr<CConnection> connection;
 	CVCMIServer & srv;
+	bool result;
 
 public:
-	ClientPermissionsCheckerNetPackVisitor(CVCMIServer & srv)
-		:srv(srv), result(false)
+	ClientPermissionsCheckerNetPackVisitor(CVCMIServer & srv, const std::shared_ptr<CConnection> & connection)
+		: connection(connection)
+		, srv(srv)
+		, result(false)
 	{
 	}
 
@@ -49,7 +52,7 @@ private:
 
 public:
 	ApplyOnServerAfterAnnounceNetPackVisitor(CVCMIServer & srv)
-		:srv(srv)
+		: srv(srv)
 	{
 	}
 
@@ -64,12 +67,15 @@ public:
 class ApplyOnServerNetPackVisitor : public VCMI_LIB_WRAP_NAMESPACE(ICPackVisitor)
 {
 private:
-	bool result;
+	std::shared_ptr<CConnection> connection;
 	CVCMIServer & srv;
+	bool result;
 
 public:
-	ApplyOnServerNetPackVisitor(CVCMIServer & srv)
-		:srv(srv), result(true)
+	ApplyOnServerNetPackVisitor(CVCMIServer & srv, const std::shared_ptr<CConnection> & connection)
+		: connection(connection)
+		, srv(srv)
+		, result(true)
 	{
 	}
 

server/NetPacksLobbyServer.cpp

@@ -28,7 +28,7 @@ void ClientPermissionsCheckerNetPackVisitor::visitForLobby(CPackForLobby & pack)
 {
 	if(pack.isForServer())
 	{
-		result = srv.isClientHost(pack.c->connectionID);
+		result = srv.isClientHost(connection->connectionID);
 	}
 }
 
@@ -49,12 +49,12 @@ void ClientPermissionsCheckerNetPackVisitor::visitLobbyClientConnected(LobbyClie
 void ApplyOnServerNetPackVisitor::visitLobbyClientConnected(LobbyClientConnected & pack)
 {
 	auto compatibleVersion = std::min(pack.version, ESerializationVersion::CURRENT);
-	pack.c->setSerializationVersion(compatibleVersion);
+	connection->setSerializationVersion(compatibleVersion);
 
-	srv.clientConnected(pack.c, pack.names, pack.uuid, pack.mode);
+	srv.clientConnected(connection, pack.names, pack.uuid, pack.mode);
 
 	// Server need to pass some data to newly connected client
-	pack.clientId = pack.c->connectionID;
+	pack.clientId = connection->connectionID;
 	pack.mode = srv.si->mode;
 	pack.hostClientId = srv.hostClientId;
 	pack.version = compatibleVersion;
@@ -64,9 +64,6 @@ void ApplyOnServerNetPackVisitor::visitLobbyClientConnected(LobbyClientConnected
 
 void ApplyOnServerAfterAnnounceNetPackVisitor::visitLobbyClientConnected(LobbyClientConnected & pack)
 {
-	// FIXME: we need to avoid senting something to client that not yet get answer for LobbyClientConnected
-	// Until UUID set we only pass LobbyClientConnected to this client
-	pack.c->uuid = pack.uuid;
 	srv.updateAndPropagateLobbyState();
 
 // FIXME: what is this??? We do NOT support reconnection into ongoing game - at the very least queries and battles are NOT serialized
@@ -83,7 +80,7 @@ void ApplyOnServerAfterAnnounceNetPackVisitor::visitLobbyClientConnected(LobbyCl
 
 void ClientPermissionsCheckerNetPackVisitor::visitLobbyClientDisconnected(LobbyClientDisconnected & pack)
 {
-	if(pack.clientId != pack.c->connectionID)
+	if(pack.clientId != connection->connectionID)
 	{
 		result = false;
 		return;
@@ -97,7 +94,7 @@ void ClientPermissionsCheckerNetPackVisitor::visitLobbyClientDisconnected(LobbyC
 			return;
 		}
 
-		if(pack.c->connectionID != srv.hostClientId)
+		if(connection->connectionID != srv.hostClientId)
 		{
 			result = false;
 			return;
@@ -109,38 +106,14 @@ void ClientPermissionsCheckerNetPackVisitor::visitLobbyClientDisconnected(LobbyC
 
 void ApplyOnServerNetPackVisitor::visitLobbyClientDisconnected(LobbyClientDisconnected & pack)
 {
-	pack.c->getConnection()->close();
-	srv.clientDisconnected(pack.c);
+	connection->getConnection()->close();
+	srv.clientDisconnected(connection);
 	result = true;
 }
 
 void ApplyOnServerAfterAnnounceNetPackVisitor::visitLobbyClientDisconnected(LobbyClientDisconnected & pack)
 {
-	if(pack.shutdownServer)
-	{
-		logNetwork->info("Client requested shutdown, server will close itself...");
-		srv.setState(EServerState::SHUTDOWN);
-		return;
-	}
-	else if(srv.activeConnections.empty())
-	{
-		logNetwork->error("Last connection lost, server will close itself...");
-		srv.setState(EServerState::SHUTDOWN);
-	}
-	else if(pack.c->connectionID == srv.hostClientId)
-	{
-		LobbyChangeHost ph;
-		auto newHost = srv.activeConnections.front();
-		ph.newHostConnectionId = newHost->connectionID;
-		srv.announcePack(ph);
-	}
 	srv.updateAndPropagateLobbyState();
-	
-//	if(srv.getState() != EServerState::SHUTDOWN && srv.remoteConnections.count(pack.c))
-//	{
-//		srv.remoteConnections -= pack.c;
-//		srv.connectToRemote();
-//	}
 }
 
 void ClientPermissionsCheckerNetPackVisitor::visitLobbyChatMessage(LobbyChatMessage & pack)
@@ -202,12 +175,12 @@ void ApplyOnServerNetPackVisitor::visitLobbySetCampaignBonus(LobbySetCampaignBon
 
 void ClientPermissionsCheckerNetPackVisitor::visitLobbyGuiAction(LobbyGuiAction & pack)
 {
-	result = srv.isClientHost(pack.c->connectionID);
+	result = srv.isClientHost(connection->connectionID);
 }
 
 void ClientPermissionsCheckerNetPackVisitor::visitLobbyRestartGame(LobbyRestartGame & pack)
 {
-	result = srv.isClientHost(pack.c->connectionID);
+	result = srv.isClientHost(connection->connectionID);
 }
 
 void ApplyOnServerNetPackVisitor::visitLobbyRestartGame(LobbyRestartGame & pack)
@@ -225,12 +198,12 @@ void ApplyOnServerAfterAnnounceNetPackVisitor::visitLobbyRestartGame(LobbyRestar
 
 void ClientPermissionsCheckerNetPackVisitor::visitLobbyPrepareStartGame(LobbyPrepareStartGame & pack)
 {
-	result = srv.isClientHost(pack.c->connectionID);
+	result = srv.isClientHost(connection->connectionID);
 }
 
 void ClientPermissionsCheckerNetPackVisitor::visitLobbyStartGame(LobbyStartGame & pack)
 {
-	result = srv.isClientHost(pack.c->connectionID);
+	result = srv.isClientHost(connection->connectionID);
 }
 
 void ApplyOnServerNetPackVisitor::visitLobbyStartGame(LobbyStartGame & pack)
@@ -276,7 +249,7 @@ void ApplyOnServerAfterAnnounceNetPackVisitor::visitLobbyStartGame(LobbyStartGam
 
 void ClientPermissionsCheckerNetPackVisitor::visitLobbyChangeHost(LobbyChangeHost & pack)
 {
-	result = srv.isClientHost(pack.c->connectionID);
+	result = srv.isClientHost(connection->connectionID);
 }
 
 void ApplyOnServerNetPackVisitor::visitLobbyChangeHost(LobbyChangeHost & pack)
@@ -296,13 +269,13 @@ void ApplyOnServerAfterAnnounceNetPackVisitor::visitLobbyChangeHost(LobbyChangeH
 
 void ClientPermissionsCheckerNetPackVisitor::visitLobbyChangePlayerOption(LobbyChangePlayerOption & pack)
 {
-	if(srv.isClientHost(pack.c->connectionID))
+	if(srv.isClientHost(connection->connectionID))
 	{
 		result = true;
 		return;
 	}
 
-	if(vstd::contains(srv.getAllClientPlayers(pack.c->connectionID), pack.color))
+	if(vstd::contains(srv.getAllClientPlayers(connection->connectionID), pack.color))
 	{
 		result = true;
 		return;
@@ -439,7 +412,7 @@ void ApplyOnServerNetPackVisitor::visitLobbyPvPAction(LobbyPvPAction & pack)
 
 void ClientPermissionsCheckerNetPackVisitor::visitLobbyDelete(LobbyDelete & pack)
 {
-	result = srv.isClientHost(pack.c->connectionID);
+	result = srv.isClientHost(connection->connectionID);
 }
 
 void ApplyOnServerNetPackVisitor::visitLobbyDelete(LobbyDelete & pack)

server/NetPacksServer.cpp

@@ -46,22 +46,22 @@ void ApplyGhNetPackVisitor::visitGamePause(GamePause & pack)
 
 void ApplyGhNetPackVisitor::visitEndTurn(EndTurn & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 	result = gh.turnOrder->onPlayerEndsTurn(pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitDismissHero(DismissHero & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.hid);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.hid);
+	gh.throwIfPlayerNotActive(connection, &pack);
 	result = gh.removeObject(gh.getObj(pack.hid), pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitMoveHero(MoveHero & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.hid);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.hid);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	for (auto const & dest : pack.path)
 	{
@@ -77,96 +77,96 @@ void ApplyGhNetPackVisitor::visitMoveHero(MoveHero & pack)
 
 void ApplyGhNetPackVisitor::visitCastleTeleportHero(CastleTeleportHero & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.hid);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.hid);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.teleportHero(pack.hid, pack.dest, pack.source, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitArrangeStacks(ArrangeStacks & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.arrangeStacks(pack.id1, pack.id2, pack.what, pack.p1, pack.p2, pack.val, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitBulkMoveArmy(BulkMoveArmy & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.srcArmy);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.srcArmy);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.bulkMoveArmy(pack.srcArmy, pack.destArmy, pack.srcSlot);
 }
 
 void ApplyGhNetPackVisitor::visitBulkSplitStack(BulkSplitStack & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.bulkSplitStack(pack.src, pack.srcOwner, pack.amount);
 }
 
 void ApplyGhNetPackVisitor::visitBulkMergeStacks(BulkMergeStacks & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.bulkMergeStacks(pack.src, pack.srcOwner);
 }
 
 void ApplyGhNetPackVisitor::visitBulkSmartSplitStack(BulkSmartSplitStack & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.bulkSmartSplitStack(pack.src, pack.srcOwner);
 }
 
 void ApplyGhNetPackVisitor::visitDisbandCreature(DisbandCreature & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.id);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.id);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.disbandCreature(pack.id, pack.pos);
 }
 
 void ApplyGhNetPackVisitor::visitBuildStructure(BuildStructure & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.tid);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.tid);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.buildStructure(pack.tid, pack.bid);
 }
 
 void ApplyGhNetPackVisitor::visitSpellResearch(SpellResearch & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.tid);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.tid);
+	gh.throwIfPlayerNotActive(connection, &pack);
 	
 	result = gh.spellResearch(pack.tid, pack.spellAtSlot, pack.accepted);
 }
 
 void ApplyGhNetPackVisitor::visitVisitTownBuilding(VisitTownBuilding & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.tid);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.tid);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.visitTownBuilding(pack.tid, pack.bid);
 }
 
 void ApplyGhNetPackVisitor::visitRecruitCreatures(RecruitCreatures & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 	// ownership checks are inside recruitCreatures
 	result = gh.recruitCreatures(pack.tid, pack.dst, pack.crid, pack.amount, pack.level, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitUpgradeCreature(UpgradeCreature & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.id);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.id);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.upgradeCreature(pack.id, pack.pos, pack.cid);
 }
@@ -174,9 +174,9 @@ void ApplyGhNetPackVisitor::visitUpgradeCreature(UpgradeCreature & pack)
 void ApplyGhNetPackVisitor::visitGarrisonHeroSwap(GarrisonHeroSwap & pack)
 {
 	const CGTownInstance * town = gh.getTown(pack.tid);
-	if(!gh.isPlayerOwns(&pack, pack.tid) && !(town->garrisonHero && gh.isPlayerOwns(&pack, town->garrisonHero->id)))
-		gh.throwNotAllowedAction(&pack); //neither town nor garrisoned hero (if present) is ours
-	gh.throwIfPlayerNotActive(&pack);
+	if(!gh.isPlayerOwns(connection, &pack, pack.tid) && !(town->garrisonHero && gh.isPlayerOwns(connection, &pack, town->garrisonHero->id)))
+		gh.throwNotAllowedAction(connection); //neither town nor garrisoned hero (if present) is ours
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.garrisonSwap(pack.tid);
 }
@@ -184,8 +184,8 @@ void ApplyGhNetPackVisitor::visitGarrisonHeroSwap(GarrisonHeroSwap & pack)
 void ApplyGhNetPackVisitor::visitExchangeArtifacts(ExchangeArtifacts & pack)
 {
 	if(gh.getHero(pack.src.artHolder))
-		gh.throwIfWrongPlayer(&pack, gh.getOwner(pack.src.artHolder)); //second hero can be ally
-	gh.throwIfPlayerNotActive(&pack);
+		gh.throwIfWrongPlayer(connection, &pack, gh.getOwner(pack.src.artHolder)); //second hero can be ally
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.moveArtifact(pack.player, pack.src, pack.dst);
 }
@@ -193,17 +193,17 @@ void ApplyGhNetPackVisitor::visitExchangeArtifacts(ExchangeArtifacts & pack)
 void ApplyGhNetPackVisitor::visitBulkExchangeArtifacts(BulkExchangeArtifacts & pack)
 {
 	if(gh.getMarket(pack.srcHero) == nullptr)
-		gh.throwIfWrongOwner(&pack, pack.srcHero);
+		gh.throwIfWrongOwner(connection, &pack, pack.srcHero);
 	if(pack.swap)
-		gh.throwIfWrongOwner(&pack, pack.dstHero);
+		gh.throwIfWrongOwner(connection, &pack, pack.dstHero);
 
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 	result = gh.bulkMoveArtifacts(pack.player, pack.srcHero, pack.dstHero, pack.swap, pack.equipped, pack.backpack);
 }
 
 void ApplyGhNetPackVisitor::visitManageBackpackArtifacts(ManageBackpackArtifacts & pack)
 {
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	if(gh.getPlayerRelations(pack.player, gh.getOwner(pack.artHolder)) != PlayerRelations::ENEMIES)
 		result = gh.manageBackpackArtifacts(pack.player, pack.artHolder, pack.cmd);
@@ -211,7 +211,7 @@ void ApplyGhNetPackVisitor::visitManageBackpackArtifacts(ManageBackpackArtifacts
 
 void ApplyGhNetPackVisitor::visitManageEquippedArtifacts(ManageEquippedArtifacts & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.artHolder);
+	gh.throwIfWrongOwner(connection, &pack, pack.artHolder);
 	if(pack.saveCostume)
 		result = gh.saveArtifactsCostume(pack.player, pack.artHolder, pack.costumeIdx);
 	else
@@ -220,22 +220,22 @@ void ApplyGhNetPackVisitor::visitManageEquippedArtifacts(ManageEquippedArtifacts
 
 void ApplyGhNetPackVisitor::visitAssembleArtifacts(AssembleArtifacts & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.heroID);
+	gh.throwIfWrongOwner(connection, &pack, pack.heroID);
 	// gh.throwIfPlayerNotActive(&pack); // Might happen when player captures artifacts in battle?
 	result = gh.assembleArtifacts(pack.heroID, pack.artifactSlot, pack.assemble, pack.assembleTo);
 }
 
 void ApplyGhNetPackVisitor::visitEraseArtifactByClient(EraseArtifactByClient & pack)
 {
-	gh.throwIfWrongPlayer(&pack, gh.getOwner(pack.al.artHolder));
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongPlayer(connection, &pack, gh.getOwner(pack.al.artHolder));
+	gh.throwIfPlayerNotActive(connection, &pack);
 	result = gh.eraseArtifactByClient(pack.al);
 }
 
 void ApplyGhNetPackVisitor::visitBuyArtifact(BuyArtifact & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.hid);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.hid);
+	gh.throwIfPlayerNotActive(connection, &pack);
 	result = gh.buyArtifact(pack.hid, pack.aid);
 }
 
@@ -245,14 +245,14 @@ void ApplyGhNetPackVisitor::visitTradeOnMarketplace(TradeOnMarketplace & pack)
 	const CGHeroInstance * hero = gh.getHero(pack.heroId);
 	const auto * market = gh.getMarket(pack.marketId);
 
-	gh.throwIfWrongPlayer(&pack);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	if(!object)
-		gh.throwAndComplain(&pack, "Invalid market object");
+		gh.throwAndComplain(connection, "Invalid market object");
 
 	if(!market)
-		gh.throwAndComplain(&pack, "market is not-a-market! :/");
+		gh.throwAndComplain(connection, "market is not-a-market! :/");
 
 	bool heroCanBeInvalid = false;
 
@@ -261,7 +261,7 @@ void ApplyGhNetPackVisitor::visitTradeOnMarketplace(TradeOnMarketplace & pack)
 		// For resource exchange we must use our own market or visit neutral market
 		if (object->getOwner().isValidPlayer())
 		{
-			gh.throwIfWrongOwner(&pack, pack.marketId);
+			gh.throwIfWrongOwner(connection, &pack, pack.marketId);
 			heroCanBeInvalid = true;
 		}
 	}
@@ -271,24 +271,24 @@ void ApplyGhNetPackVisitor::visitTradeOnMarketplace(TradeOnMarketplace & pack)
 		// For skeleton transformer, if hero is null then object must be owned
 		if (!hero)
 		{
-			gh.throwIfWrongOwner(&pack, pack.marketId);
+			gh.throwIfWrongOwner(connection, &pack, pack.marketId);
 			heroCanBeInvalid = true;
 		}
 	}
 
 	if (!heroCanBeInvalid)
 	{
-		gh.throwIfWrongOwner(&pack, pack.heroId);
+		gh.throwIfWrongOwner(connection, &pack, pack.heroId);
 
 		if (!hero)
-			gh.throwAndComplain(&pack, "Can not trade - no hero!");
+			gh.throwAndComplain(connection, "Can not trade - no hero!");
 
 		// TODO: check that object is actually being visited (e.g. Query exists)
 		if (!object->visitableAt(hero->visitablePos()))
-			gh.throwAndComplain(&pack, "Can not trade - object not visited!");
+			gh.throwAndComplain(connection, "Can not trade - object not visited!");
 
 		if (object->getOwner().isValidPlayer() && gh.getPlayerRelations(object->getOwner(), hero->getOwner()) == PlayerRelations::ENEMIES)
-			gh.throwAndComplain(&pack, "Can not trade - market not owned!");
+			gh.throwAndComplain(connection, "Can not trade - market not owned!");
 	}
 
 	result = true;
@@ -344,62 +344,57 @@ void ApplyGhNetPackVisitor::visitTradeOnMarketplace(TradeOnMarketplace & pack)
 		return;
 	}
 	default:
-		gh.throwAndComplain(&pack, "Unknown exchange pack.mode!");
+		gh.throwAndComplain(connection, "Unknown exchange pack.mode!");
 	}
 }
 
 void ApplyGhNetPackVisitor::visitSetFormation(SetFormation & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.hid);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.hid);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.setFormation(pack.hid, pack.formation);
 }
 
 void ApplyGhNetPackVisitor::visitHireHero(HireHero & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.heroPool->hireHero(pack.tid, pack.hid, pack.player, pack.nhid);
 }
 
 void ApplyGhNetPackVisitor::visitBuildBoat(BuildBoat & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	if(gh.getPlayerRelations(gh.getOwner(pack.objid), pack.player) == PlayerRelations::ENEMIES)
-		gh.throwAndComplain(&pack, "Can't build boat at enemy shipyard");
+		gh.throwAndComplain(connection, "Can't build boat at enemy shipyard");
 
 	result = gh.buildBoat(pack.objid, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitQueryReply(QueryReply & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
 
-	auto playerToConnection = gh.connections.find(pack.player);
-	if(playerToConnection == gh.connections.end())
-		gh.throwAndComplain(&pack, "No such pack.player!");
-	if(!vstd::contains(playerToConnection->second, pack.c))
-		gh.throwAndComplain(&pack, "Message came from wrong connection!");
 	if(pack.qid == QueryID(-1))
-		gh.throwAndComplain(&pack, "Cannot answer the query with pack.id -1!");
+		gh.throwAndComplain(connection, "Cannot answer the query with pack.id -1!");
 
 	result = gh.queryReply(pack.qid, pack.reply, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitSaveLocalState(SaveLocalState & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
 	*gh.gameState()->getPlayerState(pack.player)->playerLocalSettings = pack.data;
 	result = true;
 }
 
 void ApplyGhNetPackVisitor::visitMakeAction(MakeAction & pack)
 {
-	gh.throwIfWrongPlayer(&pack);
+	gh.throwIfWrongPlayer(connection, &pack);
 	// allowed even if it is not our turn - will be filtered by battle sides
 
 	result = gh.battles->makePlayerBattleAction(pack.battleID, pack.player, pack.ba);
@@ -407,23 +402,23 @@ void ApplyGhNetPackVisitor::visitMakeAction(MakeAction & pack)
 
 void ApplyGhNetPackVisitor::visitDigWithHero(DigWithHero & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.id);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.id);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	result = gh.dig(gh.getHero(pack.id));
 }
 
 void ApplyGhNetPackVisitor::visitCastAdvSpell(CastAdvSpell & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.hid);
-	gh.throwIfPlayerNotActive(&pack);
+	gh.throwIfWrongOwner(connection, &pack, pack.hid);
+	gh.throwIfPlayerNotActive(connection, &pack);
 
 	if (!pack.sid.hasValue())
-		gh.throwNotAllowedAction(&pack);
+		gh.throwNotAllowedAction(connection);
 
 	const CGHeroInstance * h = gh.getHero(pack.hid);
 	if(!h)
-		gh.throwNotAllowedAction(&pack);
+		gh.throwNotAllowedAction(connection);
 
 	AdventureSpellCastParameters p;
 	p.caster = h;
@@ -436,7 +431,7 @@ void ApplyGhNetPackVisitor::visitCastAdvSpell(CastAdvSpell & pack)
 void ApplyGhNetPackVisitor::visitPlayerMessage(PlayerMessage & pack)
 {
 	if(!pack.player.isSpectator()) // TODO: clearly not a great way to verify permissions
-		gh.throwIfWrongPlayer(&pack, pack.player);
+		gh.throwIfWrongPlayer(connection, &pack, pack.player);
 	
 	gh.playerMessages->playerMessage(pack.player, pack.text, pack.currObj);
 	result = true;

server/ServerNetPackVisitors.h

@@ -14,12 +14,15 @@
 class ApplyGhNetPackVisitor : public VCMI_LIB_WRAP_NAMESPACE(ICPackVisitor)
 {
 private:
-	bool result;
+	std::shared_ptr<CConnection> connection;
 	CGameHandler & gh;
+	bool result;
 
 public:
-	ApplyGhNetPackVisitor(CGameHandler & gh)
-		:gh(gh), result(false)
+	ApplyGhNetPackVisitor(CGameHandler & gh, const std::shared_ptr<CConnection> & connection)
+		: connection(connection)
+		, gh(gh)
+		, result(false)
 	{
 	}
 

server/processors/NewTurnProcessor.cpp

@@ -436,7 +436,7 @@ RumorState NewTurnProcessor::pickNewRumor()
 	static const std::vector<RumorState::ERumorType> rumorTypes = {RumorState::TYPE_MAP, RumorState::TYPE_SPECIAL, RumorState::TYPE_RAND, RumorState::TYPE_RAND};
 	std::vector<RumorState::ERumorTypeSpecial> sRumorTypes = {
 															  RumorState::RUMOR_OBELISKS, RumorState::RUMOR_ARTIFACTS, RumorState::RUMOR_ARMY, RumorState::RUMOR_INCOME};
-	if(gameHandler->gameState()->map->grailPos.valid()) // Grail should always be on map, but I had related crash I didn't manage to reproduce
+	if(gameHandler->gameState()->map->grailPos.isValid()) // Grail should always be on map, but I had related crash I didn't manage to reproduce
 		sRumorTypes.push_back(RumorState::RUMOR_GRAIL);
 
 	int rumorId = -1;

server/processors/PlayerMessageProcessor.cpp

@@ -66,17 +66,17 @@ void PlayerMessageProcessor::playerMessage(PlayerColor player, const std::string
 
 void PlayerMessageProcessor::commandExit(PlayerColor player, const std::vector<std::string> & words)
 {
-	bool isHost = gameHandler->gameLobby()->isPlayerHost(player);
+	bool isHost = gameHandler->gameLobby().isPlayerHost(player);
 	if(!isHost)
 		return;
 
 	broadcastSystemMessage(MetaString::createFromTextID("vcmi.broadcast.gameTerminated"));
-	gameHandler->gameLobby()->setState(EServerState::SHUTDOWN);
+	gameHandler->gameLobby().setState(EServerState::SHUTDOWN);
 }
 
 void PlayerMessageProcessor::commandKick(PlayerColor player, const std::vector<std::string> & words)
 {
-	bool isHost = gameHandler->gameLobby()->isPlayerHost(player);
+	bool isHost = gameHandler->gameLobby().isPlayerHost(player);
 	if(!isHost)
 		return;
 
@@ -88,10 +88,10 @@ void PlayerMessageProcessor::commandKick(PlayerColor player, const std::vector<s
 			playerToKick = PlayerColor(std::stoi(playername));
 		else
 		{
-			for(auto & c : gameHandler->connections)
+			for (PlayerColor color : PlayerColor::ALL_PLAYERS())
 			{
-				if(c.first.toString() == playername)
-					playerToKick = c.first;
+				if(color.toString() == playername)
+					playerToKick = color;
 			}
 		}
 
@@ -108,7 +108,7 @@ void PlayerMessageProcessor::commandKick(PlayerColor player, const std::vector<s
 
 void PlayerMessageProcessor::commandSave(PlayerColor player, const std::vector<std::string> & words)
 {
-	bool isHost = gameHandler->gameLobby()->isPlayerHost(player);
+	bool isHost = gameHandler->gameLobby().isPlayerHost(player);
 	if(!isHost)
 		return;
 
@@ -143,7 +143,7 @@ void PlayerMessageProcessor::commandCheaters(PlayerColor player, const std::vect
 
 void PlayerMessageProcessor::commandStatistic(PlayerColor player, const std::vector<std::string> & words)
 {
-	bool isHost = gameHandler->gameLobby()->isPlayerHost(player);
+	bool isHost = gameHandler->gameLobby().isPlayerHost(player);
 	if(!isHost)
 		return;
 

server/processors/TurnOrderProcessor.cpp

@@ -260,7 +260,7 @@ void TurnOrderProcessor::doStartNewDay()
 
 	if(!activePlayer)
 	{
-		gameHandler->gameLobby()->setState(EServerState::SHUTDOWN);
+		gameHandler->gameLobby().setState(EServerState::SHUTDOWN);
 		return;
 	}