Merge pull request #3834 from IvanSavenko/mp_fixes

Fixes for multiplayer games

client/CPlayerInterface.cpp

@@ -289,7 +289,12 @@ void CPlayerInterface::yourTurn(QueryID queryID)
 			performAutosave();
 		}
 
-		if (CSH->howManyPlayerInterfaces() > 1) //hot seat message
+		int humanPlayersCount = 0;
+		for(const auto & info : cb->getStartInfo()->playerInfos)
+			if (info.second.isControlledByHuman())
+				humanPlayersCount++;
+
+		if (humanPlayersCount > 1) //hot seat or MP message
 		{
 			adventureInt->onHotseatWaitStarted(playerID);
 

client/lobby/CLobbyScreen.cpp

@@ -47,10 +47,11 @@ CLobbyScreen::CLobbyScreen(ESelectionScreen screenType)
 		tabSel->callOnSelect = std::bind(&IServerAPI::setMapInfo, CSH, _1, nullptr);
 
 		buttonSelect = std::make_shared<CButton>(Point(411, 80), AnimationPath::builtin("GSPBUTT.DEF"), CGI->generaltexth->zelp[45], 0, EShortcut::LOBBY_SELECT_SCENARIO);
-		buttonSelect->addCallback([&]()
+		buttonSelect->addCallback([=]()
 		{
 			toggleTab(tabSel);
-			CSH->setMapInfo(tabSel->getSelectedMapInfo());
+			if (getMapInfo()->isRandomMap)
+				CSH->setMapInfo(tabSel->getSelectedMapInfo());
 		});
 
 		buttonOptions = std::make_shared<CButton>(Point(411, 510), AnimationPath::builtin("GSPBUTT.DEF"), CGI->generaltexth->zelp[46], std::bind(&CLobbyScreen::toggleTab, this, tabOpt), EShortcut::LOBBY_ADDITIONAL_OPTIONS);
@@ -74,10 +75,11 @@ CLobbyScreen::CLobbyScreen(ESelectionScreen screenType)
 		tabRand = std::make_shared<RandomMapTab>();
 		tabRand->mapInfoChanged += std::bind(&IServerAPI::setMapInfo, CSH, _1, _2);
 		buttonRMG = std::make_shared<CButton>(Point(411, 105), AnimationPath::builtin("GSPBUTT.DEF"), CGI->generaltexth->zelp[47], 0, EShortcut::LOBBY_RANDOM_MAP);
-		buttonRMG->addCallback([&]()
+		buttonRMG->addCallback([this]()
 		{
 			toggleTab(tabRand);
-			tabRand->updateMapInfoByHost(); // TODO: This is only needed to force-update mapInfo in CSH when tab is opened
+			if (!getMapInfo()->isRandomMap)
+				tabRand->updateMapInfoByHost();
 		});
 
 		card->iconDifficulty->addCallback(std::bind(&IServerAPI::setDifficulty, CSH, _1));

client/lobby/CLobbyScreen.h

@@ -13,14 +13,14 @@
 
 class CBonusSelection;
 
-class CLobbyScreen : public CSelectionBase
+class CLobbyScreen final : public CSelectionBase
 {
 public:
 	std::shared_ptr<CButton> buttonChat;
 
 	CLobbyScreen(ESelectionScreen type);
 	~CLobbyScreen();
-	void toggleTab(std::shared_ptr<CIntObject> tab) override;
+	void toggleTab(std::shared_ptr<CIntObject> tab) final;
 	void startCampaign();
 	void startScenario(bool allowOnlyAI = false);
 	void toggleMode(bool host);
@@ -28,8 +28,8 @@ public:
 
 	void updateAfterStateChange();
 
-	const CMapInfo * getMapInfo() override;
-	const StartInfo * getStartInfo() override;
+	const CMapInfo * getMapInfo() final;
+	const StartInfo * getStartInfo() final;
 
 	std::shared_ptr<CBonusSelection> bonusSel;
 };

lib/pathfinder/CPathfinder.cpp

@@ -143,13 +143,13 @@ void CPathfinder::calculatePaths()
 		auto * hlp = config->getOrCreatePathfinderHelper(source, gamestate);
 
 		hlp->updateTurnInfo(turn);
-		if(!movement)
+		if(movement == 0)
 		{
 			hlp->updateTurnInfo(++turn);
 			movement = hlp->getMaxMovePoints(source.node->layer);
 			if(!hlp->passOneTurnLimitCheck(source))
 				continue;
-			if(turn >= hlp->options.turnLimit)
+			if(turn > hlp->options.turnLimit)
 				continue;
 		}
 

lib/serializer/ESerializationVersion.h

@@ -39,6 +39,7 @@ enum class ESerializationVersion : int32_t
 	JSON_FLAGS, // 836 json uses new format for flags
 	MANA_LIMIT,	// 837 change MANA_PER_KNOWLEGDE to percentage
 	BONUS_META_STRING,	// 838 bonuses use MetaString instead of std::string for descriptions
+	TURN_TIMERS_STATE, // 839 current state of turn timers is serialized
 
-	CURRENT = BONUS_META_STRING
+	CURRENT = TURN_TIMERS_STATE
 };

server/CGameHandler.cpp

@@ -11,6 +11,7 @@
 #include "CGameHandler.h"
 
 #include "CVCMIServer.h"
+#include "TurnTimerHandler.h"
 #include "ServerNetPackVisitors.h"
 #include "ServerSpellCastEnvironment.h"
 #include "battles/BattleProcessor.h"
@@ -514,7 +515,7 @@ CGameHandler::CGameHandler(CVCMIServer * lobby)
 	, complainNoCreatures("No creatures to split")
 	, complainNotEnoughCreatures("Cannot split that stack, not enough creatures!")
 	, complainInvalidSlot("Invalid slot accessed!")
-	, turnTimerHandler(*this)
+	, turnTimerHandler(std::make_unique<TurnTimerHandler>(*this))
 {
 	QID = 1;
 	applier = std::make_shared<CApplier<CBaseForGHApply>>();
@@ -616,7 +617,7 @@ void CGameHandler::setPortalDwelling(const CGTownInstance * town, bool forced=fa
 void CGameHandler::onPlayerTurnStarted(PlayerColor which)
 {
 	events::PlayerGotTurn::defaultExecute(serverEventBus.get(), which);
-	turnTimerHandler.onPlayerGetTurn(which);
+	turnTimerHandler->onPlayerGetTurn(which);
 }
 
 void CGameHandler::onPlayerTurnEnded(PlayerColor which)
@@ -1009,7 +1010,7 @@ void CGameHandler::start(bool resume)
 		onNewTurn();
 		events::TurnStarted::defaultExecute(serverEventBus.get());
 		for(auto & player : gs->players)
-			turnTimerHandler.onGameplayStart(player.first);
+			turnTimerHandler->onGameplayStart(player.first);
 	}
 	else
 		events::GameResumed::defaultExecute(serverEventBus.get());
@@ -1019,7 +1020,7 @@ void CGameHandler::start(bool resume)
 
 void CGameHandler::tick(int millisecondsPassed)
 {
-	turnTimerHandler.update(millisecondsPassed);
+	turnTimerHandler->update(millisecondsPassed);
 }
 
 void CGameHandler::giveSpells(const CGTownInstance *t, const CGHeroInstance *h)
@@ -1307,7 +1308,7 @@ bool CGameHandler::moveHero(ObjectInstanceID hid, int3 dst, ui8 teleporting, boo
 		if(h->boat && !h->boat->onboardAssaultAllowed)
 			lookForGuards = IGNORE_GUARDS;
 
-		turnTimerHandler.setEndTurnAllowed(h->getOwner(), !movingOntoWater && !movingOntoObstacle);
+		turnTimerHandler->setEndTurnAllowed(h->getOwner(), !movingOntoWater && !movingOntoObstacle);
 		doMove(TryMoveHero::SUCCESS, lookForGuards, visitDest, LEAVING_TILE);
 		return true;
 	}

server/CGameHandler.h

@@ -14,7 +14,6 @@
 #include "../lib/IGameCallback.h"
 #include "../lib/LoadProgress.h"
 #include "../lib/ScriptHandler.h"
-#include "TurnTimerHandler.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -49,6 +48,7 @@ class CBaseForGHApply;
 class PlayerMessageProcessor;
 class BattleProcessor;
 class TurnOrderProcessor;
+class TurnTimerHandler;
 class QueriesProcessor;
 class CObjectVisitQuery;
 
@@ -62,6 +62,7 @@ public:
 	std::unique_ptr<BattleProcessor> battles;
 	std::unique_ptr<QueriesProcessor> queries;
 	std::unique_ptr<TurnOrderProcessor> turnOrder;
+	std::unique_ptr<TurnTimerHandler> turnTimerHandler;
 
 	//use enums as parameters, because doMove(sth, true, false, true) is not readable
 	enum EGuardLook {CHECK_FOR_GUARDS, IGNORE_GUARDS};
@@ -76,8 +77,6 @@ public:
 	ui32 QID;
 
 	SpellCastEnvironment * spellEnv;
-	
-	TurnTimerHandler turnTimerHandler;
 
 	const Services * services() const override;
 	const BattleCb * battle(const BattleID & battleID) const override;
@@ -231,6 +230,9 @@ public:
 		h & *playerMessages;
 		h & *turnOrder;
 
+		if (h.version >= Handler::Version::TURN_TIMERS_STATE)
+			h & *turnTimerHandler;
+
 #if SCRIPTING_ENABLED
 		JsonNode scriptsState;
 		if(h.saving)

server/TurnTimerHandler.h

@@ -25,7 +25,7 @@ class CGameHandler;
 class TurnTimerHandler
 {	
 	CGameHandler & gameHandler;
-	const int turnTimePropagateFrequency = 1000;
+	static constexpr int turnTimePropagateFrequency = 1000;
 	std::map<PlayerColor, TurnTimerInfo> timers;
 	std::map<PlayerColor, int> lastUpdate;
 	std::map<PlayerColor, bool> endTurnAllowed;
@@ -48,4 +48,11 @@ public:
 	void update(int waitTime);
 	void setTimerEnabled(PlayerColor player, bool enabled);
 	void setEndTurnAllowed(PlayerColor player, bool enabled);
+
+	template<typename Handler>
+	void serialize(Handler & h)
+	{
+		h & timers;
+		h & endTurnAllowed;
+	}
 };

server/battles/BattleActionProcessor.cpp

@@ -29,17 +29,12 @@
 #include "../../lib/spells/ISpellMechanics.h"
 #include "../../lib/spells/Problem.h"
 
-BattleActionProcessor::BattleActionProcessor(BattleProcessor * owner)
+BattleActionProcessor::BattleActionProcessor(BattleProcessor * owner, CGameHandler * newGameHandler)
 	: owner(owner)
-	, gameHandler(nullptr)
+	, gameHandler(newGameHandler)
 {
 }
 
-void BattleActionProcessor::setGameHandler(CGameHandler * newGameHandler)
-{
-	gameHandler = newGameHandler;
-}
-
 bool BattleActionProcessor::doEmptyAction(const CBattleInfoCallback & battle, const BattleAction & ba)
 {
 	return true;

server/battles/BattleActionProcessor.h

@@ -78,8 +78,7 @@ class BattleActionProcessor : boost::noncopyable
 	bool makeBattleActionImpl(const CBattleInfoCallback & battle, const BattleAction & ba);
 
 public:
-	explicit BattleActionProcessor(BattleProcessor * owner);
-	void setGameHandler(CGameHandler * newGameHandler);
+	explicit BattleActionProcessor(BattleProcessor * owner, CGameHandler * newGameHandler);
 
 	bool makeAutomaticBattleAction(const CBattleInfoCallback & battle, const BattleAction & ba);
 	bool makePlayerBattleAction(const CBattleInfoCallback & battle, PlayerColor player, const BattleAction & ba);

server/battles/BattleFlowProcessor.cpp

@@ -13,6 +13,7 @@
 #include "BattleProcessor.h"
 
 #include "../CGameHandler.h"
+#include "../TurnTimerHandler.h"
 
 #include "../../lib/CStack.h"
 #include "../../lib/GameSettings.h"
@@ -25,17 +26,12 @@
 #include "../../lib/spells/ISpellMechanics.h"
 #include "../../lib/spells/ObstacleCasterProxy.h"
 
-BattleFlowProcessor::BattleFlowProcessor(BattleProcessor * owner)
+BattleFlowProcessor::BattleFlowProcessor(BattleProcessor * owner, CGameHandler * newGameHandler)
 	: owner(owner)
-	, gameHandler(nullptr)
+	, gameHandler(newGameHandler)
 {
 }
 
-void BattleFlowProcessor::setGameHandler(CGameHandler * newGameHandler)
-{
-	gameHandler = newGameHandler;
-}
-
 void BattleFlowProcessor::summonGuardiansHelper(const CBattleInfoCallback & battle, std::vector<BattleHex> & output, const BattleHex & targetPosition, ui8 side, bool targetIsTwoHex) //return hexes for summoning two hex monsters in output, target = unit to guard
 {
 	int x = targetPosition.getX();
@@ -131,7 +127,7 @@ void BattleFlowProcessor::onBattleStarted(const CBattleInfoCallback & battle)
 {
 	tryPlaceMoats(battle);
 	
-	gameHandler->turnTimerHandler.onBattleStart(battle.getBattle()->getBattleID());
+	gameHandler->turnTimerHandler->onBattleStart(battle.getBattle()->getBattleID());
 
 	if (battle.battleGetTacticDist() == 0)
 		onTacticsEnded(battle);
@@ -324,7 +320,7 @@ void BattleFlowProcessor::activateNextStack(const CBattleInfoCallback & battle)
 		if(!removeGhosts.changedStacks.empty())
 			gameHandler->sendAndApply(&removeGhosts);
 		
-		gameHandler->turnTimerHandler.onBattleNextStack(battle.getBattle()->getBattleID(), *next);
+		gameHandler->turnTimerHandler->onBattleNextStack(battle.getBattle()->getBattleID(), *next);
 
 		if (!tryMakeAutomaticAction(battle, next))
 		{

server/battles/BattleFlowProcessor.h

@@ -51,8 +51,7 @@ class BattleFlowProcessor : boost::noncopyable
 	bool makeAutomaticAction(const CBattleInfoCallback & battle, const CStack * stack, BattleAction & ba); //used when action is taken by stack without volition of player (eg. unguided catapult attack)
 
 public:
-	explicit BattleFlowProcessor(BattleProcessor * owner);
-	void setGameHandler(CGameHandler * newGameHandler);
+	explicit BattleFlowProcessor(BattleProcessor * owner, CGameHandler * newGameHandler);
 
 	void onBattleStarted(const CBattleInfoCallback & battle);
 	void onTacticsEnded(const CBattleInfoCallback & battle);

server/battles/BattleProcessor.cpp

@@ -33,15 +33,9 @@
 
 BattleProcessor::BattleProcessor(CGameHandler * gameHandler)
 	: gameHandler(gameHandler)
-	, flowProcessor(std::make_unique<BattleFlowProcessor>(this))
-	, actionsProcessor(std::make_unique<BattleActionProcessor>(this))
-	, resultProcessor(std::make_unique<BattleResultProcessor>(this))
-{
-	setGameHandler(gameHandler);
-}
-
-BattleProcessor::BattleProcessor():
-	BattleProcessor(nullptr)
+	, flowProcessor(std::make_unique<BattleFlowProcessor>(this, gameHandler))
+	, actionsProcessor(std::make_unique<BattleActionProcessor>(this, gameHandler))
+	, resultProcessor(std::make_unique<BattleResultProcessor>(this, gameHandler))
 {
 }
 
@@ -316,12 +310,3 @@ void BattleProcessor::battleAfterLevelUp(const BattleID & battleID, const Battle
 {
 	resultProcessor->battleAfterLevelUp(battleID, result);
 }
-
-void BattleProcessor::setGameHandler(CGameHandler * newGameHandler)
-{
-	gameHandler = newGameHandler;
-
-	actionsProcessor->setGameHandler(newGameHandler);
-	flowProcessor->setGameHandler(newGameHandler);
-	resultProcessor->setGameHandler(newGameHandler);
-}

server/battles/BattleProcessor.h

@@ -52,11 +52,8 @@ class BattleProcessor : boost::noncopyable
 
 public:
 	explicit BattleProcessor(CGameHandler * gameHandler);
-	BattleProcessor();
 	~BattleProcessor();
 
-	void setGameHandler(CGameHandler * gameHandler);
-
 	/// Starts battle with specified parameters
 	void startBattlePrimary(const CArmedInstance *army1, const CArmedInstance *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool creatureBank = false, const CGTownInstance *town = nullptr);
 	/// Starts battle between two armies (which can also be heroes) at specified tile
@@ -78,6 +75,5 @@ public:
 	{
 
 	}
-
 };
 

server/battles/BattleResultProcessor.cpp

@@ -11,6 +11,7 @@
 #include "BattleResultProcessor.h"
 
 #include "../CGameHandler.h"
+#include "../TurnTimerHandler.h"
 #include "../processors/HeroPoolProcessor.h"
 #include "../queries/QueriesProcessor.h"
 #include "../queries/BattleQueries.h"
@@ -29,17 +30,12 @@
 #include "../../lib/serializer/Cast.h"
 #include "../../lib/spells/CSpellHandler.h"
 
-BattleResultProcessor::BattleResultProcessor(BattleProcessor * owner)
+BattleResultProcessor::BattleResultProcessor(BattleProcessor * owner, CGameHandler * newGameHandler)
 //	: owner(owner)
-	: gameHandler(nullptr)
+	: gameHandler(newGameHandler)
 {
 }
 
-void BattleResultProcessor::setGameHandler(CGameHandler * newGameHandler)
-{
-	gameHandler = newGameHandler;
-}
-
 CasualtiesAfterBattle::CasualtiesAfterBattle(const CBattleInfoCallback & battle, uint8_t sideInBattle):
 	army(battle.battleGetArmyObject(sideInBattle))
 {
@@ -297,7 +293,7 @@ void BattleResultProcessor::endBattle(const CBattleInfoCallback & battle)
 			otherBattleQuery->result = battleQuery->result;
 	}
 
-	gameHandler->turnTimerHandler.onBattleEnd(battle.getBattle()->getBattleID());
+	gameHandler->turnTimerHandler->onBattleEnd(battle.getBattle()->getBattleID());
 	gameHandler->sendAndApply(battleResult);
 
 	if (battleResult->queryID == QueryID::NONE)

server/battles/BattleResultProcessor.h

@@ -70,8 +70,7 @@ class BattleResultProcessor : boost::noncopyable
 	std::map<BattleID, std::unique_ptr<FinishingBattleHelper>> finishingBattles;
 
 public:
-	explicit BattleResultProcessor(BattleProcessor * owner);
-	void setGameHandler(CGameHandler * newGameHandler);
+	explicit BattleResultProcessor(BattleProcessor * owner, CGameHandler * newGameHandler);
 
 	bool battleIsEnding(const CBattleInfoCallback & battle) const;
 

server/processors/HeroPoolProcessor.cpp

@@ -25,11 +25,6 @@
 #include "../../lib/gameState/TavernSlot.h"
 #include "../../lib/GameSettings.h"
 
-HeroPoolProcessor::HeroPoolProcessor()
-	: gameHandler(nullptr)
-{
-}
-
 HeroPoolProcessor::HeroPoolProcessor(CGameHandler * gameHandler)
 	: gameHandler(gameHandler)
 {

server/processors/HeroPoolProcessor.h

@@ -28,6 +28,8 @@ class CGameHandler;
 
 class HeroPoolProcessor : boost::noncopyable
 {
+	CGameHandler * gameHandler;
+
 	/// per-player random generators
 	std::map<PlayerColor, std::unique_ptr<CRandomGenerator>> playerSeed;
 
@@ -49,9 +51,6 @@ class HeroPoolProcessor : boost::noncopyable
 	TavernHeroSlot selectSlotForRole(const PlayerColor & player, TavernSlotRole roleID);
 
 public:
-	CGameHandler * gameHandler;
-
-	HeroPoolProcessor();
 	HeroPoolProcessor(CGameHandler * gameHandler);
 
 	void onHeroSurrendered(const PlayerColor & color, const CGHeroInstance * hero);
@@ -66,7 +65,6 @@ public:
 
 	template <typename Handler> void serialize(Handler &h)
 	{
-		// h & gameHandler; // FIXME: make this work instead of using deserializationFix in gameHandler
 		h & playerSeed;
 		h & heroSeed;
 	}

server/processors/PlayerMessageProcessor.cpp

@@ -29,11 +29,6 @@
 #include "../../lib/networkPacks/StackLocation.h"
 #include "../../lib/serializer/Connection.h"
 
-PlayerMessageProcessor::PlayerMessageProcessor()
-	:gameHandler(nullptr)
-{
-}
-
 PlayerMessageProcessor::PlayerMessageProcessor(CGameHandler * gameHandler)
 	:gameHandler(gameHandler)
 {

server/processors/PlayerMessageProcessor.h

@@ -21,6 +21,8 @@ class CGameHandler;
 
 class PlayerMessageProcessor
 {
+	CGameHandler * gameHandler;
+
 	void executeCheatCode(const std::string & cheatName, PlayerColor player, ObjectInstanceID currObj, const std::vector<std::string> & arguments );
 	bool handleCheatCode(const std::string & cheatFullCommand, PlayerColor player, ObjectInstanceID currObj);
 	bool handleHostCommand(PlayerColor player, const std::string & message);
@@ -43,9 +45,6 @@ class PlayerMessageProcessor
 	void cheatFly(PlayerColor player, const CGHeroInstance * hero);
 
 public:
-	CGameHandler * gameHandler;
-
-	PlayerMessageProcessor();
 	PlayerMessageProcessor(CGameHandler * gameHandler);
 
 	/// incoming NetPack handling

server/processors/TurnOrderProcessor.cpp

@@ -249,10 +249,12 @@ void TurnOrderProcessor::doStartPlayerTurn(PlayerColor which)
 	assert(gameHandler->getPlayerState(which));
 	assert(gameHandler->getPlayerState(which)->status == EPlayerStatus::INGAME);
 
-	//Note: on game load, "actingPlayer" might already contain list of players
+	// Only if player is actually starting his turn (and not loading from save)
+	if (!actingPlayers.count(which))
+		gameHandler->onPlayerTurnStarted(which);
+
 	actingPlayers.insert(which);
 	awaitingPlayers.erase(which);
-	gameHandler->onPlayerTurnStarted(which);
 
 	auto turnQuery = std::make_shared<TimerPauseQuery>(gameHandler, which);
 	gameHandler->queries->addQuery(turnQuery);

server/queries/MapQueries.cpp

@@ -12,6 +12,7 @@
 
 #include "QueriesProcessor.h"
 #include "../CGameHandler.h"
+#include "../TurnTimerHandler.h"
 #include "../../lib/mapObjects/MiscObjects.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
 #include "../../lib/networkPacks/PacksForServer.h"
@@ -30,12 +31,12 @@ bool TimerPauseQuery::blocksPack(const CPack *pack) const
 
 void TimerPauseQuery::onAdding(PlayerColor color)
 {
-	gh->turnTimerHandler.setTimerEnabled(color, false);
+	gh->turnTimerHandler->setTimerEnabled(color, false);
 }
 
 void TimerPauseQuery::onRemoval(PlayerColor color)
 {
-	gh->turnTimerHandler.setTimerEnabled(color, true);
+	gh->turnTimerHandler->setTimerEnabled(color, true);
 }
 
 bool TimerPauseQuery::endsByPlayerAnswer() const