Fixes for several discovered edge cases

lib/NetPacksLib.cpp

@@ -2284,14 +2284,12 @@ void StartAction::applyGs(CGameState *gs)
 		return;
 	}
 
-	if(ba.actionType != EActionType::HERO_SPELL) //don't check for stack if it's custom action by hero
-	{
-		assert(st);
-	}
-	else
-	{
+	[[maybe_unused]] bool heroAction = ba.actionType == EActionType::HERO_SPELL || ba.actionType ==EActionType::SURRENDER || ba.actionType ==EActionType::RETREAT || ba.actionType == EActionType::END_TACTIC_PHASE;
+
+	assert(st || heroAction); // stack must exists for all non-hero actions
+
+	if(ba.actionType == EActionType::HERO_SPELL)
 		gs->curB->sides[ba.side].usedSpellsHistory.emplace_back(ba.actionSubtype);
-	}
 
 	switch(ba.actionType)
 	{

server/NetPacksServer.cpp

@@ -285,7 +285,7 @@ void ApplyGhNetPackVisitor::visitMakeAction(MakeAction & pack)
 	if (!gh.hasPlayerAt(pack.player, pack.c))
 		gh.throwAndComplain(&pack, "No such pack.player!");
 
-	result = gh.battles->makeBattleAction(pack.player, pack.ba);
+	result = gh.battles->makePlayerBattleAction(pack.player, pack.ba);
 }
 
 void ApplyGhNetPackVisitor::visitDigWithHero(DigWithHero & pack)

server/battles/BattleFlowProcessor.cpp

@@ -520,9 +520,8 @@ void BattleFlowProcessor::onActionMade(const BattleAction &ba)
 {
 	const auto & battle = gameHandler->gameState()->curB;
 
-	const CStack * actedStack = battle->battleGetStackByID(ba.stackNumber);
-	const CStack * activeStack = battle->battleGetStackByID(battle->getActiveStackID());
-	assert(activeStack != nullptr);
+	const CStack * actedStack = battle->battleGetStackByID(ba.stackNumber, false);
+	const CStack * activeStack = battle->battleGetStackByID(battle->getActiveStackID(), false);
 
 	//we're after action, all results applied
 
@@ -530,11 +529,17 @@ void BattleFlowProcessor::onActionMade(const BattleAction &ba)
 	if(owner->checkBattleStateChanges())
 		return;
 
-	bool heroAction = ba.actionType == EActionType::HERO_SPELL || ba.actionType ==EActionType::SURRENDER || ba.actionType ==EActionType::RETREAT || ba.actionType ==EActionType::END_TACTIC_PHASE;
+	bool heroAction = ba.actionType == EActionType::HERO_SPELL || ba.actionType ==EActionType::SURRENDER || ba.actionType ==EActionType::RETREAT;
+	bool tacticsAction = ba.actionType == EActionType::END_TACTIC_PHASE;
 
-	if (heroAction)
+	if (activeStack == nullptr && !tacticsAction)
 	{
-		if (activeStack->alive())
+		throw std::runtime_error("Unexpected action - no active stack!");
+	}
+
+	if (heroAction || tacticsAction)
+	{
+		if (!tacticsAction && activeStack->alive())
 		{
 			// this is action made by hero AND unit is alive (e.g. not killed by casted spell)
 			// keep current active stack for next action
@@ -578,7 +583,7 @@ bool BattleFlowProcessor::makeAutomaticAction(const CStack *stack, BattleAction
 	bsa.askPlayerInterface = false;
 	gameHandler->sendAndApply(&bsa);
 
-	bool ret = owner->makeBattleAction(ba);
+	bool ret = owner->makeAutomaticBattleAction(ba);
 	return ret;
 }
 

server/battles/BattleProcessor.cpp

@@ -236,7 +236,7 @@ void BattleProcessor::updateGateState()
 		gameHandler->sendAndApply(&db);
 }
 
-bool BattleProcessor::makeBattleAction(PlayerColor player, BattleAction &ba)
+bool BattleProcessor::makePlayerBattleAction(PlayerColor player, BattleAction &ba)
 {
 	const BattleInfo * b = gameHandler->gameState()->curB;
 
@@ -263,6 +263,14 @@ bool BattleProcessor::makeBattleAction(PlayerColor player, BattleAction &ba)
 	}
 	else
 	{
+		bool heroAction = ba.actionType == EActionType::HERO_SPELL || ba.actionType ==EActionType::SURRENDER || ba.actionType ==EActionType::RETREAT || ba.actionType == EActionType::END_TACTIC_PHASE;
+
+		if (ba.stackNumber != b->getActiveStackID() && heroAction == false)
+		{
+			gameHandler->complain("Can not make actions - stack is not active!");
+			return false;
+		}
+
 		auto active = b->battleActiveUnit();
 		if(!active && gameHandler->complain("No active unit in battle!"))
 			return false;
@@ -273,7 +281,9 @@ bool BattleProcessor::makeBattleAction(PlayerColor player, BattleAction &ba)
 			return false;
 	}
 
-	return makeBattleAction(ba);
+	bool result = actionsProcessor->makeBattleAction(ba);
+	flowProcessor->onActionMade(ba);
+	return result;
 }
 
 void BattleProcessor::setBattleResult(EBattleResult resultType, int victoriusSide)
@@ -282,11 +292,9 @@ void BattleProcessor::setBattleResult(EBattleResult resultType, int victoriusSid
 	resultProcessor->endBattle(gameHandler->gameState()->curB->tile, gameHandler->gameState()->curB->battleGetFightingHero(0), gameHandler->gameState()->curB->battleGetFightingHero(1));
 }
 
-bool BattleProcessor::makeBattleAction(const BattleAction &ba)
+bool BattleProcessor::makeAutomaticBattleAction(const BattleAction &ba)
 {
-	bool result = actionsProcessor->makeBattleAction(ba);
-	flowProcessor->onActionMade(ba);
-	return result;
+	return actionsProcessor->makeBattleAction(ba);
 }
 
 void BattleProcessor::endBattleConfirm(const BattleInfo * battleInfo)

server/battles/BattleProcessor.h

@@ -45,7 +45,7 @@ class BattleProcessor : boost::noncopyable
 	bool checkBattleStateChanges();
 	void setupBattle(int3 tile, const CArmedInstance *armies[2], const CGHeroInstance *heroes[2], bool creatureBank, const CGTownInstance *town);
 
-	bool makeBattleAction(const BattleAction & ba);
+	bool makeAutomaticBattleAction(const BattleAction & ba);
 
 	void setBattleResult(EBattleResult resultType, int victoriusSide);
 
@@ -64,7 +64,7 @@ public:
 	void startBattleI(const CArmedInstance *army1, const CArmedInstance *army2, bool creatureBank = false);
 
 	/// Processing of incoming battle action netpack
-	bool makeBattleAction(PlayerColor player, BattleAction & ba);
+	bool makePlayerBattleAction(PlayerColor player, BattleAction & ba);
 
 	/// Applies results of a battle once player agrees to them
 	void endBattleConfirm(const BattleInfo * battleInfo);