Merge pull request #2638 from IvanSavenko/battle_processor_regression_fix

Battle processor regression fix

client/CPlayerInterface.cpp

@@ -654,6 +654,11 @@ void CPlayerInterface::buildChanged(const CGTownInstance *town, BuildingID build
 
 void CPlayerInterface::battleStartBefore(const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2)
 {
+	// when battle starts, game will send battleStart pack *before* movement confirmation
+	// and since network thread wait for battle intro to play, movement confirmation will only happen after intro
+	// leading to several bugs, such as blocked input during intro
+	stillMoveHero.setn(STOP_MOVE);
+
 	//Don't wait for dialogs when we are non-active hot-seat player
 	if (LOCPLINT == this)
 		waitForAllDialogs();

client/battle/BattleInterface.cpp

@@ -325,7 +325,7 @@ void BattleInterface::battleFinished(const BattleResult& br, QueryID queryID)
 		curInt->cb->selectionMade(selection, queryID);
 	};
 	GH.windows().pushWindow(wnd);
-	
+
 	curInt->waitWhileDialog(); // Avoid freeze when AI end turn after battle. Check bug #1897
 	CPlayerInterface::battleInt = nullptr;
 }
@@ -601,28 +601,13 @@ void BattleInterface::startAction(const BattleAction & action)
 		return;
 	}
 
-	const CStack *stack = curInt->cb->battleGetStackByID(action.stackNumber);
-
-	if (stack)
-	{
-		windowObject->updateQueue();
-	}
-	else
-	{
-		assert(action.actionType == EActionType::HERO_SPELL); //only cast spell is valid action without acting stack number
-	}
-
 	stacksController->startAction(action);
 
-	if(action.actionType == EActionType::HERO_SPELL) //when hero casts spell
+	if (!action.isUnitAction())
 		return;
 
-	if (!stack)
-	{
-		logGlobal->error("Something wrong with stackNumber in actionStarted. Stack number: %d", action.stackNumber);
-		return;
-	}
-
+	assert(curInt->cb->battleGetStackByID(action.stackNumber));
+	windowObject->updateQueue();
 	effectsController->startAction(action);
 }
 

lib/battle/BattleAction.cpp

@@ -194,7 +194,8 @@ void BattleAction::setTarget(const battle::Target & target_)
 
 bool BattleAction::isUnitAction() const
 {
-	static const std::array<EActionType, 9> actions = {
+	static const std::array<EActionType, 109> actions = {
+		EActionType::NO_ACTION,
 		EActionType::WALK,
 		EActionType::WAIT,
 		EActionType::DEFEND,
@@ -205,7 +206,6 @@ bool BattleAction::isUnitAction() const
 		EActionType::BAD_MORALE,
 		EActionType::STACK_HEAL
 	};
-
 	return vstd::contains(actions, actionType);
 }
 
@@ -215,7 +215,15 @@ bool BattleAction::isSpellAction() const
 		EActionType::HERO_SPELL,
 		EActionType::MONSTER_SPELL
 	};
+	return vstd::contains(actions, actionType);
+}
 
+bool BattleAction::isBattleEndAction() const
+{
+	static const std::array<EActionType, 2> actions = {
+		EActionType::RETREAT,
+		EActionType::SURRENDER
+	};
 	return vstd::contains(actions, actionType);
 }
 
@@ -227,7 +235,6 @@ bool BattleAction::isTacticsAction() const
 		EActionType::RETREAT,
 		EActionType::SURRENDER
 	};
-
 	return vstd::contains(actions, actionType);
 }
 

lib/battle/BattleAction.h

@@ -46,6 +46,7 @@ public:
 	bool isTacticsAction() const;
 	bool isUnitAction() const;
 	bool isSpellAction() const;
+	bool isBattleEndAction() const;
 	std::string toString() const;
 
 	void aimToHex(const BattleHex & destination);

server/battles/BattleActionProcessor.cpp

@@ -535,13 +535,20 @@ bool BattleActionProcessor::makeBattleActionImpl(const BattleAction &ba)
 	logGlobal->trace("Making action: %s", ba.toString());
 	const CStack * stack = gameHandler->gameState()->curB->battleGetStackByID(ba.stackNumber);
 
-	StartAction startAction(ba);
-	gameHandler->sendAndApply(&startAction);
+	// for these events client does not expects StartAction/EndAction wrapper
+	if (!ba.isBattleEndAction())
+	{
+		StartAction startAction(ba);
+		gameHandler->sendAndApply(&startAction);
+	}
 
 	bool result = dispatchBattleAction(ba);
 
-	EndAction endAction;
-	gameHandler->sendAndApply(&endAction);
+	if (!ba.isBattleEndAction())
+	{
+		EndAction endAction;
+		gameHandler->sendAndApply(&endAction);
+	}
 
 	if(ba.actionType == EActionType::WAIT || ba.actionType == EActionType::DEFEND || ba.actionType == EActionType::SHOOT || ba.actionType == EActionType::MONSTER_SPELL)
 		gameHandler->handleObstacleTriggersForUnit(*gameHandler->spellEnv, *stack);

server/battles/BattleProcessor.cpp

@@ -147,6 +147,9 @@ bool BattleProcessor::checkBattleStateChanges()
 	if (gameHandler->battleGetSiegeLevel() > 0)
 		updateGateState();
 
+	if (resultProcessor->battleIsEnding())
+		return true;
+
 	//check if battle ended
 	if (auto result = gameHandler->battleIsFinished())
 	{

server/battles/BattleResultProcessor.cpp

@@ -528,6 +528,7 @@ void BattleResultProcessor::battleAfterLevelUp(const BattleResult &result)
 	}
 
 	finishingBattle.reset();
+	battleResult.reset();
 }
 
 void BattleResultProcessor::setBattleResult(EBattleResult resultType, int victoriusSide)
@@ -537,3 +538,8 @@ void BattleResultProcessor::setBattleResult(EBattleResult resultType, int victor
 	battleResult->winner = victoriusSide; //surrendering side loses
 	gameHandler->gameState()->curB->calculateCasualties(battleResult->casualties);
 }
+
+bool BattleResultProcessor::battleIsEnding() const
+{
+	return battleResult != nullptr;
+}

server/battles/BattleResultProcessor.h

@@ -71,6 +71,8 @@ public:
 	explicit BattleResultProcessor(BattleProcessor * owner);
 	void setGameHandler(CGameHandler * newGameHandler);
 
+	bool battleIsEnding() const;
+
 	void setBattleResult(EBattleResult resultType, int victoriusSide);
 	void endBattle(int3 tile, const CGHeroInstance * hero1, const CGHeroInstance * hero2); //ends battle
 	void endBattleConfirm(const BattleInfo * battleInfo);