Replace "currentPlayer" from gamestate with "activePlayers"

- Allows multiple active players at once, e.g. simturns
- Cleared up validation of netpacks by server, e.g. always check for
pack sender

client/CPlayerInterface.cpp

@@ -1469,7 +1469,7 @@ void CPlayerInterface::centerView (int3 pos, int focusTime)
 void CPlayerInterface::objectRemoved(const CGObjectInstance * obj)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
-	if(LOCPLINT->cb->getCurrentPlayer() == playerID && obj->getRemovalSound())
+	if(LOCPLINT->cb->isPlayerMakingTurn(playerID) && obj->getRemovalSound())
 	{
 		waitWhileDialog();
 		CCS->soundh->playSound(obj->getRemovalSound().value());

client/Client.cpp

@@ -693,13 +693,6 @@ std::shared_ptr<const CPathsInfo> CClient::getPathsInfo(const CGHeroInstance * h
 	}
 }
 
-PlayerColor CClient::getLocalPlayer() const
-{
-	if(LOCPLINT)
-		return LOCPLINT->playerID;
-	return getCurrentPlayer();
-}
-
 #if SCRIPTING_ENABLED
 scripting::Pool * CClient::getGlobalContextPool() const
 {

client/Client.h

@@ -156,7 +156,6 @@ public:
 
 	void invalidatePaths();
 	std::shared_ptr<const CPathsInfo> getPathsInfo(const CGHeroInstance * h);
-	virtual PlayerColor getLocalPlayer() const override;
 
 	friend class CCallback; //handling players actions
 	friend class CBattleCallback; //handling players actions

client/NetPacksClient.cpp

@@ -301,9 +301,15 @@ void ApplyClientNetPackVisitor::visitBulkMoveArtifacts(BulkMoveArtifacts & pack)
 		}
 	};
 
+	ArtifactLocation srcLoc(pack.srcArtHolder, pack.artsPack0.front().srcPos);
+	ArtifactLocation dstLoc(pack.dstArtHolder, pack.artsPack0.front().dstPos);
+
 	// Begin a session of bulk movement of arts. It is not necessary but useful for the client optimization.
-	callInterfaceIfPresent(cl, cl.getCurrentPlayer(), &IGameEventsReceiver::bulkArtMovementStart, 
-		pack.artsPack0.size() + pack.artsPack1.size());
+	callInterfaceIfPresent(cl, srcLoc.owningPlayer(), &IGameEventsReceiver::bulkArtMovementStart, pack.artsPack0.size() + pack.artsPack1.size());
+
+	if (srcLoc.owningPlayer() != dstLoc.owningPlayer())
+		callInterfaceIfPresent(cl, dstLoc.owningPlayer(), &IGameEventsReceiver::bulkArtMovementStart, pack.artsPack0.size() + pack.artsPack1.size());
+
 	applyMove(pack.artsPack0);
 	if(pack.swap)
 		applyMove(pack.artsPack1);
@@ -386,9 +392,15 @@ void ApplyClientNetPackVisitor::visitPlayerReinitInterface(PlayerReinitInterface
 	auto initInterfaces = [this]()
 	{
 		cl.initPlayerInterfaces();
-		auto currentPlayer = cl.gameState()->currentPlayer;
-		callAllInterfaces(cl, &IGameEventsReceiver::playerStartsTurn, currentPlayer);
-		callOnlyThatInterface(cl, currentPlayer, &CGameInterface::yourTurn);
+
+		for (PlayerColor player(0); player < PlayerColor::PLAYER_LIMIT; ++player)
+		{
+			if (cl.gameState()->isPlayerMakingTurn(player))
+			{
+				callAllInterfaces(cl, &IGameEventsReceiver::playerStartsTurn, player);
+				callOnlyThatInterface(cl, player, &CGameInterface::yourTurn);
+			}
+		}
 	};
 	
 	for(auto player : pack.players)

client/adventureMap/AdventureMapInterface.cpp

@@ -327,7 +327,7 @@ void AdventureMapInterface::onEnemyTurnStarted(PlayerColor playerID, bool isHuma
 
 	mapAudio->onEnemyTurnStarted();
 	widget->getMinimap()->setAIRadar(!isHuman);
-	widget->getInfoBar()->startEnemyTurn(LOCPLINT->cb->getCurrentPlayer());
+	widget->getInfoBar()->startEnemyTurn(playerID);
 	setState(isHuman ? EAdventureState::OTHER_HUMAN_PLAYER_TURN : EAdventureState::AI_PLAYER_TURN);
 }
 
@@ -363,8 +363,7 @@ void AdventureMapInterface::onPlayerTurnStarted(PlayerColor playerID)
 	onCurrentPlayerChanged(playerID);
 
 	setState(EAdventureState::MAKING_TURN);
-	if(LOCPLINT->cb->getCurrentPlayer() == LOCPLINT->playerID
-		|| settings["session"]["spectate"].Bool())
+	if(playerID == LOCPLINT->playerID || settings["session"]["spectate"].Bool())
 	{
 		widget->getMinimap()->setAIRadar(false);
 		widget->getInfoBar()->showSelection();

client/adventureMap/TurnTimerWidget.cpp

@@ -67,14 +67,18 @@ void TurnTimerWidget::show(Canvas & to)
 	showAll(to);
 }
 
-void TurnTimerWidget::setTime(int time)
+void TurnTimerWidget::setTime(PlayerColor player, int time)
 {
 	int newTime = time / 1000;
-	if((LOCPLINT->cb->getCurrentPlayer() == LOCPLINT->playerID)
+	if((LOCPLINT->cb->isPlayerMakingTurn(LOCPLINT->playerID))
 	   && (newTime != turnTime)
 	   && notifications.count(newTime))
+	{
 		CCS->soundh->playSound(variables["notificationSound"].String());
+	}
+
 	turnTime = newTime;
+
 	if(auto w = widget<CLabel>("timer"))
 	{
 		std::ostringstream oss;
@@ -83,18 +87,23 @@ void TurnTimerWidget::setTime(int time)
 		
 		if(graphics && LOCPLINT && LOCPLINT->cb
 		   && variables["textColorFromPlayerColor"].Bool()
-		   && LOCPLINT->cb->getCurrentPlayer().isValidPlayer())
+		   && player.isValidPlayer())
 		{
-			w->setColor(graphics->playerColors[LOCPLINT->cb->getCurrentPlayer()]);
+			w->setColor(graphics->playerColors[player]);
 		}
 	}
 }
 
 void TurnTimerWidget::tick(uint32_t msPassed)
 {
-	if(LOCPLINT && LOCPLINT->cb)
+	if(!LOCPLINT || !LOCPLINT->cb)
+		return;
+
+	for (PlayerColor player(0); player < PlayerColor::PLAYER_LIMIT; ++player)
 	{
-		auto player = LOCPLINT->cb->getCurrentPlayer();
+		if (!LOCPLINT->cb->isPlayerMakingTurn(player))
+			continue;
+
 		auto time = LOCPLINT->cb->getPlayerTurnTime(player);
 		cachedTurnTime -= msPassed;
 		if(cachedTurnTime < 0) cachedTurnTime = 0; //do not go below zero
@@ -107,7 +116,8 @@ void TurnTimerWidget::tick(uint32_t msPassed)
 				lastTurnTime = time;
 				cachedTurnTime = time;
 			}
-			else setTime(cachedTurnTime);
+			else
+				setTime(player, cachedTurnTime);
 		};
 		
 		auto * playerInfo = LOCPLINT->cb->getPlayer(player);

client/adventureMap/TurnTimerWidget.h

@@ -39,13 +39,13 @@ private:
 	std::set<int> notifications;
 	
 	std::shared_ptr<DrawRect> buildDrawRect(const JsonNode & config) const;
-	
+
 public:
 
 	void show(Canvas & to) override;
 	void tick(uint32_t msPassed) override;
 	
-	void setTime(int time);
+	void setTime(PlayerColor player, int time);
 
 	TurnTimerWidget();
 };

lib/CGameInfoCallback.cpp

@@ -320,8 +320,7 @@ bool CGameInfoCallback::getHeroInfo(const CGObjectInstance * hero, InfoAboutHero
 	dest.initFromHero(h, infoLevel);
 
 	//DISGUISED bonus implementation
-
-	if(getPlayerRelations(getLocalPlayer(), hero->tempOwner) == PlayerRelations::ENEMIES)
+	if(getPlayerRelations(*player, hero->tempOwner) == PlayerRelations::ENEMIES)
 	{
 		//todo: bonus cashing
 		int disguiseLevel = h->valOfBonuses(Selector::typeSubtype(BonusType::DISGUISED, 0));
@@ -705,9 +704,9 @@ bool CGameInfoCallback::isOwnedOrVisited(const CGObjectInstance *obj) const
 	return visitor->ID == Obj::HERO && canGetFullInfo(visitor); //owned or allied hero is a visitor
 }
 
-PlayerColor CGameInfoCallback::getCurrentPlayer() const
+bool CGameInfoCallback::isPlayerMakingTurn(PlayerColor player) const
 {
-	return gs->currentPlayer;
+	return gs->actingPlayers.count(player);
 }
 
 CGameInfoCallback::CGameInfoCallback(CGameState * GS, std::optional<PlayerColor> Player):
@@ -932,11 +931,6 @@ const CGHeroInstance * CGameInfoCallback::getHeroWithSubid( int subid ) const
 	return gs->map->allHeroes.at(subid).get();
 }
 
-PlayerColor CGameInfoCallback::getLocalPlayer() const
-{
-	return getCurrentPlayer();
-}
-
 bool CGameInfoCallback::isInTheMap(const int3 &pos) const
 {
 	return gs->map->isInTheMap(pos);
@@ -944,7 +938,7 @@ bool CGameInfoCallback::isInTheMap(const int3 &pos) const
 
 void CGameInfoCallback::getVisibleTilesInRange(std::unordered_set<int3> &tiles, int3 pos, int radious, int3::EDistanceFormula distanceFormula) const
 {
-	gs->getTilesInRange(tiles, pos, radious, getLocalPlayer(), -1, distanceFormula);
+	gs->getTilesInRange(tiles, pos, radious, *player, -1, distanceFormula);
 }
 
 void CGameInfoCallback::calculatePaths(const std::shared_ptr<PathfinderConfig> & config)

lib/CGameInfoCallback.h

@@ -63,8 +63,6 @@ public:
 //	PlayerRelations getPlayerRelations(PlayerColor color1, PlayerColor color2) const;
 //	void getThievesGuildInfo(SThievesGuildInfo & thi, const CGObjectInstance * obj); //get thieves' guild info obtainable while visiting given object
 //	EPlayerStatus getPlayerStatus(PlayerColor player, bool verbose = true) const; //-1 if no such player
-//	PlayerColor getCurrentPlayer() const; //player that currently makes move // TODO synchronous turns
-	virtual PlayerColor getLocalPlayer() const = 0; //player that is currently owning given client (if not a client, then returns current player)
 //	const PlayerSettings * getPlayerSettings(PlayerColor color) const;
 
 
@@ -99,7 +97,6 @@ public:
 //	const TerrainTile * getTile(int3 tile, bool verbose = true) const;
 //	std::shared_ptr<boost::multi_array<TerrainTile*, 3>> getAllVisibleTiles() const;
 //	bool isInTheMap(const int3 &pos) const;
-//	void getVisibleTilesInRange(std::unordered_set<int3> &tiles, int3 pos, int radious, int3::EDistanceFormula distanceFormula = int3::DIST_2D) const;
 
 	//town
 //	const CGTownInstance* getTown(ObjectInstanceID objid) const;
@@ -151,8 +148,7 @@ public:
 	virtual PlayerRelations getPlayerRelations(PlayerColor color1, PlayerColor color2) const;
 	virtual void getThievesGuildInfo(SThievesGuildInfo & thi, const CGObjectInstance * obj); //get thieves' guild info obtainable while visiting given object
 	virtual EPlayerStatus getPlayerStatus(PlayerColor player, bool verbose = true) const; //-1 if no such player
-	virtual PlayerColor getCurrentPlayer() const; //player that currently makes move // TODO synchronous turns
-	PlayerColor getLocalPlayer() const override; //player that is currently owning given client (if not a client, then returns current player)
+	virtual bool isPlayerMakingTurn(PlayerColor player) const; //player that currently makes move // TODO synchronous turns
 	virtual const PlayerSettings * getPlayerSettings(PlayerColor color) const;
 	virtual TurnTimerInfo getPlayerTurnTime(PlayerColor color) const;
 

lib/NetPacksLib.cpp

@@ -2501,7 +2501,8 @@ void PlayerCheated::applyGs(CGameState * gs) const
 
 void YourTurn::applyGs(CGameState * gs) const
 {
-	gs->currentPlayer = player;
+	gs->actingPlayers.clear();
+	gs->actingPlayers.insert(player);
 }
 
 void DaysWithoutTown::applyGs(CGameState * gs) const

lib/gameState/CGameState.h

@@ -86,6 +86,9 @@ public:
 	//we have here all heroes available on this map that are not hired
 	std::unique_ptr<TavernHeroesPool> heroesPool;
 
+	/// list of players currently making turn. Usually - just one, except for simturns
+	std::set<PlayerColor> actingPlayers;
+
 	CGameState();
 	virtual ~CGameState();
 
@@ -95,7 +98,6 @@ public:
 	void updateOnLoad(StartInfo * si);
 
 	ConstTransitivePtr<StartInfo> scenarioOps, initialOpts; //second one is a copy of settings received from pregame (not randomized)
-	PlayerColor currentPlayer; //ID of player currently having turn
 	ConstTransitivePtr<BattleInfo> curB; //current battle
 	ui32 day; //total number of days in game
 	ConstTransitivePtr<CMap> map;
@@ -151,7 +153,7 @@ public:
 	{
 		h & scenarioOps;
 		h & initialOpts;
-		h & currentPlayer;
+		h & actingPlayers;
 		h & day;
 		h & map;
 		h & players;

scripting/lua/api/GameCb.cpp

@@ -30,7 +30,6 @@ const std::vector<GameCbProxy::CustomRegType> GameCbProxy::REGISTER_CUSTOM =
 {
 	{"getDate", LuaMethodWrapper<GameCb, decltype(&GameCb::getDate), &GameCb::getDate>::invoke, false},
 	{"isAllowed", LuaMethodWrapper<GameCb, decltype(&GameCb::isAllowed), &GameCb::isAllowed>::invoke, false},
-	{"getCurrentPlayer", LuaMethodWrapper<GameCb, decltype(&GameCb::getLocalPlayer), &GameCb::getLocalPlayer>::invoke, false},
 	{"getPlayer", LuaMethodWrapper<GameCb, decltype(&GameCb::getPlayer), &GameCb::getPlayer>::invoke, false},
 
 	{"getHero", LuaMethodWrapper<GameCb, decltype(&GameCb::getHero), &GameCb::getHero>::invoke, false},

server/CGameHandler.cpp

@@ -1003,7 +1003,10 @@ void CGameHandler::run(bool resume)
 	{
 		const int waitTime = 100; //ms
 
-		turnTimerHandler.onPlayerMakingTurn(gs->players[gs->getCurrentPlayer()], waitTime);
+		for(auto & player : gs->players)
+			if (gs->isPlayerMakingTurn(player.first))
+				turnTimerHandler.onPlayerMakingTurn(player.second, waitTime);
+
 		if(gs->curB)
 			turnTimerHandler.onBattleLoop(waitTime);
 
@@ -1064,7 +1067,7 @@ bool CGameHandler::moveHero(ObjectInstanceID hid, int3 dst, ui8 teleporting, boo
 {
 	const CGHeroInstance *h = getHero(hid);
 	// not turn of that hero or player can't simply teleport hero (at least not with this function)
-	if (!h  || (asker != PlayerColor::NEUTRAL && (teleporting || h->getOwner() != gs->currentPlayer)))
+	if(!h || (asker != PlayerColor::NEUTRAL && teleporting))
 	{
 		if(h && getStartInfo()->turnTimerInfo.isEnabled() && gs->players[h->getOwner()].turnTimer.turnTimer == 0)
 			return true; //timer expired, no error
@@ -1278,7 +1281,7 @@ bool CGameHandler::teleportHero(ObjectInstanceID hid, ObjectInstanceID dstid, ui
 	const CGHeroInstance *h = getHero(hid);
 	const CGTownInstance *t = getTown(dstid);
 
-	if (!h || !t || h->getOwner() != gs->currentPlayer)
+	if (!h || !t)
 		COMPLAIN_RET("Invalid call to teleportHero!");
 
 	const CGTownInstance *from = h->visitedTown;
@@ -1646,12 +1649,6 @@ void CGameHandler::sendAndApply(CPackForClient * pack)
 	logNetwork->trace("\tApplied on gs: %s", typeid(*pack).name());
 }
 
-void CGameHandler::applyAndSend(CPackForClient * pack)
-{
-	gs->apply(pack);
-	sendToAllClients(pack);
-}
-
 void CGameHandler::sendAndApply(CGarrisonOperationPack * pack)
 {
 	sendAndApply(static_cast<CPackForClient *>(pack));
@@ -1672,7 +1669,7 @@ void CGameHandler::sendAndApply(NewStructures * pack)
 
 bool CGameHandler::isPlayerOwns(CPackForServer * pack, ObjectInstanceID id)
 {
-	return getPlayerAt(pack->c) == getOwner(id);
+	return pack->player == getOwner(id) && hasPlayerAt(getOwner(id), pack->c);
 }
 
 void CGameHandler::throwNotAllowedAction(CPackForServer * pack)
@@ -1687,14 +1684,14 @@ void CGameHandler::throwNotAllowedAction(CPackForServer * pack)
 void CGameHandler::wrongPlayerMessage(CPackForServer * pack, PlayerColor expectedplayer)
 {
 	std::ostringstream oss;
-	oss << "You were identified as player " << getPlayerAt(pack->c) << " while expecting " << expectedplayer;
+	oss << "You were identified as player " << pack->player << " while expecting " << expectedplayer;
 	logNetwork->error(oss.str());
 
 	if(pack->c)
 		playerMessages->sendSystemMessage(pack->c, oss.str());
 }
 
-void CGameHandler::throwOnWrongOwner(CPackForServer * pack, ObjectInstanceID id)
+void CGameHandler::throwIfWrongOwner(CPackForServer * pack, ObjectInstanceID id)
 {
 	if(!isPlayerOwns(pack, id))
 	{
@@ -1703,9 +1700,14 @@ void CGameHandler::throwOnWrongOwner(CPackForServer * pack, ObjectInstanceID id)
 	}
 }
 
-void CGameHandler::throwOnWrongPlayer(CPackForServer * pack, PlayerColor player)
+void CGameHandler::throwIfWrongPlayer(CPackForServer * pack)
+{
+	throwIfWrongPlayer(pack, pack->player);
+}
+
+void CGameHandler::throwIfWrongPlayer(CPackForServer * pack, PlayerColor player)
 {
-	if(!hasPlayerAt(player, pack->c) && player != getPlayerAt(pack->c))
+	if(!hasPlayerAt(player, pack->c) || pack->player != player)
 	{
 		wrongPlayerMessage(pack, player);
 		throwNotAllowedAction(pack);
@@ -2173,30 +2175,6 @@ bool CGameHandler::hasPlayerAt(PlayerColor player, std::shared_ptr<CConnection>
 	return connections.at(player).count(c);
 }
 
-PlayerColor CGameHandler::getPlayerAt(std::shared_ptr<CConnection> c) const
-{
-	std::set<PlayerColor> all;
-	for (auto i=connections.cbegin(); i!=connections.cend(); i++)
-		if(vstd::contains(i->second, c))
-			all.insert(i->first);
-
-	switch(all.size())
-	{
-	case 0:
-		return PlayerColor::NEUTRAL;
-	case 1:
-		return *all.begin();
-	default:
-		{
-			//if we have more than one player at this connection, try to pick active one
-			if (vstd::contains(all, gs->currentPlayer))
-				return gs->currentPlayer;
-			else
-				return PlayerColor::CANNOT_DETERMINE; //cannot say which player is it
-		}
-	}
-}
-
 bool CGameHandler::disbandCreature(ObjectInstanceID id, SlotID pos)
 {
 	const CArmedInstance * s1 = static_cast<const CArmedInstance *>(getObjInstance(id));

server/CGameHandler.h

@@ -178,7 +178,6 @@ public:
 	void init(StartInfo *si, Load::ProgressAccumulator & progressTracking);
 	void handleClientDisconnection(std::shared_ptr<CConnection> c);
 	void handleReceivedPack(CPackForServer * pack);
-	PlayerColor getPlayerAt(std::shared_ptr<CConnection> c) const;
 	bool hasPlayerAt(PlayerColor player, std::shared_ptr<CConnection> c) const;
 
 	bool queryReply( QueryID qid, const JsonNode & answer, PlayerColor player );
@@ -243,16 +242,22 @@ public:
 
 	void sendToAllClients(CPackForClient * pack);
 	void sendAndApply(CPackForClient * pack) override;
-	void applyAndSend(CPackForClient * pack);
 	void sendAndApply(CGarrisonOperationPack * pack);
 	void sendAndApply(SetResources * pack);
 	void sendAndApply(NewStructures * pack);
 
 	void wrongPlayerMessage(CPackForServer * pack, PlayerColor expectedplayer);
+	/// Unconditionally throws with "Action not allowed" message
 	void throwNotAllowedAction(CPackForServer * pack);
-	void throwOnWrongOwner(CPackForServer * pack, ObjectInstanceID id);
-	void throwOnWrongPlayer(CPackForServer * pack, PlayerColor player);
+	/// Throws if player stated in pack is not making turn right now
+	void throwIfPlayerNotActive(CPackForServer * pack);
+	/// Throws if object is not owned by pack sender
+	void throwIfWrongOwner(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);
 	void throwAndComplain(CPackForServer * pack, std::string txt);
+
 	bool isPlayerOwns(CPackForServer * pack, ObjectInstanceID id);
 
 	void run(bool resume);

server/NetPacksServer.cpp

@@ -37,76 +37,80 @@ void ApplyGhNetPackVisitor::visitSaveGame(SaveGame & pack)
 
 void ApplyGhNetPackVisitor::visitEndTurn(EndTurn & pack)
 {
-	if (!gh.hasPlayerAt(pack.player, pack.c))
-		gh.throwAndComplain(&pack, "No such pack.player!");
-
+	gh.throwIfWrongPlayer(&pack);
 	result = gh.turnOrder->onPlayerEndsTurn(pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitDismissHero(DismissHero & pack)
 {
-	gh.throwOnWrongOwner(&pack, pack.hid);
+	gh.throwIfWrongOwner(&pack, pack.hid);
 	result = gh.removeObject(gh.getObj(pack.hid));
 }
 
 void ApplyGhNetPackVisitor::visitMoveHero(MoveHero & pack)
 {
-	result = gh.moveHero(pack.hid, pack.dest, 0, pack.transit, gh.getPlayerAt(pack.c));
+	gh.throwIfWrongOwner(&pack, pack.hid);
+	result = gh.moveHero(pack.hid, pack.dest, 0, pack.transit, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitCastleTeleportHero(CastleTeleportHero & pack)
 {
-	gh.throwOnWrongOwner(&pack, pack.hid);
+	gh.throwIfWrongOwner(&pack, pack.hid);
 
-	result = gh.teleportHero(pack.hid, pack.dest, pack.source, gh.getPlayerAt(pack.c));
+	result = gh.teleportHero(pack.hid, pack.dest, pack.source, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitArrangeStacks(ArrangeStacks & pack)
 {
-	//checks for owning in the gh func
-	result = gh.arrangeStacks(pack.id1, pack.id2, pack.what, pack.p1, pack.p2, pack.val, gh.getPlayerAt(pack.c));
+	gh.throwIfWrongPlayer(&pack);
+	result = gh.arrangeStacks(pack.id1, pack.id2, pack.what, pack.p1, pack.p2, pack.val, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitBulkMoveArmy(BulkMoveArmy & pack)
 {
+	gh.throwIfWrongPlayer(&pack);
 	result = gh.bulkMoveArmy(pack.srcArmy, pack.destArmy, pack.srcSlot);
 }
 
 void ApplyGhNetPackVisitor::visitBulkSplitStack(BulkSplitStack & pack)
 {
+	gh.throwIfWrongPlayer(&pack);
 	result = gh.bulkSplitStack(pack.src, pack.srcOwner, pack.amount);
 }
 
 void ApplyGhNetPackVisitor::visitBulkMergeStacks(BulkMergeStacks & pack)
 {
+	gh.throwIfWrongPlayer(&pack);
 	result = gh.bulkMergeStacks(pack.src, pack.srcOwner);
 }
 
 void ApplyGhNetPackVisitor::visitBulkSmartSplitStack(BulkSmartSplitStack & pack)
 {
+	gh.throwIfWrongPlayer(&pack);
 	result = gh.bulkSmartSplitStack(pack.src, pack.srcOwner);
 }
 
 void ApplyGhNetPackVisitor::visitDisbandCreature(DisbandCreature & pack)
 {
-	gh.throwOnWrongOwner(&pack, pack.id);
+	gh.throwIfWrongOwner(&pack, pack.id);
 	result = gh.disbandCreature(pack.id, pack.pos);
 }
 
 void ApplyGhNetPackVisitor::visitBuildStructure(BuildStructure & pack)
 {
-	gh.throwOnWrongOwner(&pack, pack.tid);
+	gh.throwIfWrongOwner(&pack, pack.tid);
 	result = gh.buildStructure(pack.tid, pack.bid);
 }
 
 void ApplyGhNetPackVisitor::visitRecruitCreatures(RecruitCreatures & pack)
 {
+	gh.throwIfWrongOwner(&pack, pack.tid);
 	result = gh.recruitCreatures(pack.tid, pack.dst, pack.crid, pack.amount, pack.level);
 }
 
 void ApplyGhNetPackVisitor::visitUpgradeCreature(UpgradeCreature & pack)
 {
-	gh.throwOnWrongOwner(&pack, pack.id);
+	gh.throwIfWrongOwner(&pack, pack.id);
 	result = gh.upgradeCreature(pack.id, pack.pos, pack.cid);
 }
 
@@ -120,37 +124,38 @@ void ApplyGhNetPackVisitor::visitGarrisonHeroSwap(GarrisonHeroSwap & pack)
 
 void ApplyGhNetPackVisitor::visitExchangeArtifacts(ExchangeArtifacts & pack)
 {
-	gh.throwOnWrongPlayer(&pack, pack.src.owningPlayer()); //second hero can be ally
+	gh.throwIfWrongPlayer(&pack, pack.src.owningPlayer()); //second hero can be ally
 	result = gh.moveArtifact(pack.src, pack.dst);
 }
 
 void ApplyGhNetPackVisitor::visitBulkExchangeArtifacts(BulkExchangeArtifacts & pack)
 {
-	const CGHeroInstance * pSrcHero = gh.getHero(pack.srcHero);
-	gh.throwOnWrongPlayer(&pack, pSrcHero->getOwner());
+	gh.throwIfWrongOwner(&pack, pack.srcHero);
 	result = gh.bulkMoveArtifacts(pack.srcHero, pack.dstHero, pack.swap);
 }
 
 void ApplyGhNetPackVisitor::visitAssembleArtifacts(AssembleArtifacts & pack)
 {
-	gh.throwOnWrongOwner(&pack, pack.heroID);
+	gh.throwIfWrongOwner(&pack, pack.heroID);
 	result = gh.assembleArtifacts(pack.heroID, pack.artifactSlot, pack.assemble, pack.assembleTo);
 }
 
 void ApplyGhNetPackVisitor::visitEraseArtifactByClient(EraseArtifactByClient & pack)
 {
-	gh.throwOnWrongPlayer(&pack, pack.al.owningPlayer());
+	gh.throwIfWrongPlayer(&pack, pack.al.owningPlayer());
 	result = gh.eraseArtifactByClient(pack.al);
 }
 
 void ApplyGhNetPackVisitor::visitBuyArtifact(BuyArtifact & pack)
 {
-	gh.throwOnWrongOwner(&pack, pack.hid);
+	gh.throwIfWrongOwner(&pack, pack.hid);
 	result = gh.buyArtifact(pack.hid, pack.aid);
 }
 
 void ApplyGhNetPackVisitor::visitTradeOnMarketplace(TradeOnMarketplace & pack)
 {
+	gh.throwIfWrongPlayer(&pack);
+
 	const CGObjectInstance * market = gh.getObj(pack.marketId);
 	if(!market)
 		gh.throwAndComplain(&pack, "Invalid market object");
@@ -177,7 +182,7 @@ void ApplyGhNetPackVisitor::visitTradeOnMarketplace(TradeOnMarketplace & pack)
 		gh.throwAndComplain(&pack, "This hero can't use this marketplace!");
 
 	if(!allyTownSkillTrade)
-		gh.throwOnWrongPlayer(&pack, player);
+		gh.throwIfWrongPlayer(&pack, player);
 
 	result = true;
 
@@ -231,28 +236,31 @@ void ApplyGhNetPackVisitor::visitTradeOnMarketplace(TradeOnMarketplace & pack)
 
 void ApplyGhNetPackVisitor::visitSetFormation(SetFormation & pack)
 {
-	gh.throwOnWrongOwner(&pack, pack.hid);
+	gh.throwIfWrongOwner(&pack, pack.hid);
 	result = gh.setFormation(pack.hid, pack.formation);
 }
 
 void ApplyGhNetPackVisitor::visitHireHero(HireHero & pack)
 {
-	if (!gh.hasPlayerAt(pack.player, pack.c))
-		gh.throwAndComplain(&pack, "No such pack.player!");
+	gh.throwIfWrongPlayer(&pack);
 
 	result = gh.heroPool->hireHero(pack.tid, pack.hid, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitBuildBoat(BuildBoat & pack)
 {
-	if(gh.getPlayerRelations(gh.getOwner(pack.objid), gh.getPlayerAt(pack.c)) == PlayerRelations::ENEMIES)
+	gh.throwIfWrongPlayer(&pack);
+
+	if(gh.getPlayerRelations(gh.getOwner(pack.objid), pack.player) == PlayerRelations::ENEMIES)
 		gh.throwAndComplain(&pack, "Can't build boat at enemy shipyard");
 
-	result = gh.buildBoat(pack.objid, gh.getPlayerAt(pack.c));
+	result = gh.buildBoat(pack.objid, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitQueryReply(QueryReply & pack)
 {
+	gh.throwIfWrongPlayer(&pack);
+
 	auto playerToConnection = gh.connections.find(pack.player);
 	if(playerToConnection == gh.connections.end())
 		gh.throwAndComplain(&pack, "No such pack.player!");
@@ -266,21 +274,20 @@ void ApplyGhNetPackVisitor::visitQueryReply(QueryReply & pack)
 
 void ApplyGhNetPackVisitor::visitMakeAction(MakeAction & pack)
 {
-	if (!gh.hasPlayerAt(pack.player, pack.c))
-		gh.throwAndComplain(&pack, "No such pack.player!");
+	gh.throwIfWrongPlayer(&pack);
 
 	result = gh.battles->makePlayerBattleAction(pack.player, pack.ba);
 }
 
 void ApplyGhNetPackVisitor::visitDigWithHero(DigWithHero & pack)
 {
-	gh.throwOnWrongOwner(&pack, pack.id);
+	gh.throwIfWrongOwner(&pack, pack.id);
 	result = gh.dig(gh.getHero(pack.id));
 }
 
 void ApplyGhNetPackVisitor::visitCastAdvSpell(CastAdvSpell & pack)
 {
-	gh.throwOnWrongOwner(&pack, pack.hid);
+	gh.throwIfWrongOwner(&pack, pack.hid);
 
 	const CSpell * s = pack.sid.toSpell();
 	if(!s)
@@ -299,7 +306,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.throwOnWrongPlayer(&pack, pack.player);
+		gh.throwIfWrongPlayer(&pack, pack.player);
 	
 	gh.playerMessages->playerMessage(pack.player, pack.text, pack.currObj);
 	result = true;

server/processors/TurnOrderProcessor.cpp

@@ -103,7 +103,7 @@ void TurnOrderProcessor::doStartPlayerTurn(PlayerColor which)
 	gameHandler->sendAndApply(&yt);
 
 	assert(actingPlayers.size() == 1); // No simturns yet :(
-	assert(gameHandler->getCurrentPlayer() == *actingPlayers.begin());
+	assert(gameHandler->isPlayerMakingTurn(*actingPlayers.begin()));
 }
 
 void TurnOrderProcessor::doEndPlayerTurn(PlayerColor which)
@@ -122,7 +122,7 @@ void TurnOrderProcessor::doEndPlayerTurn(PlayerColor which)
 
 	assert(!actingPlayers.empty());
 	assert(actingPlayers.size() == 1); // No simturns yet :(
-	assert(gameHandler->getCurrentPlayer() == *actingPlayers.begin());
+	assert(gameHandler->isPlayerMakingTurn(*actingPlayers.begin()));
 }
 
 void TurnOrderProcessor::addPlayer(PlayerColor which)
@@ -144,7 +144,7 @@ void TurnOrderProcessor::onPlayerEndsGame(PlayerColor which)
 
 	assert(!actingPlayers.empty());
 	assert(actingPlayers.size() == 1); // No simturns yet :(
-	assert(gameHandler->getCurrentPlayer() == *actingPlayers.begin());
+	assert(gameHandler->isPlayerMakingTurn(*actingPlayers.begin()));
 }
 
 bool TurnOrderProcessor::onPlayerEndsTurn(PlayerColor which)