Fix alternative actions to support more than 2 actions + simplify logic

client/battle/BattleActionsController.cpp

@@ -985,26 +985,23 @@ void BattleActionsController::activateStack()
 		std::list<PossiblePlayerBattleAction> actionsToSelect;
 		if(!possibleActions.empty())
 		{
-			switch(possibleActions.front().get())
+			auto primaryAction = possibleActions.front().get();
+
+			if(primaryAction == PossiblePlayerBattleAction::SHOOT || primaryAction == PossiblePlayerBattleAction::AIMED_SPELL_CREATURE
+				|| primaryAction == PossiblePlayerBattleAction::ANY_LOCATION || primaryAction == PossiblePlayerBattleAction::ATTACK_AND_RETURN)
 			{
-				case PossiblePlayerBattleAction::SHOOT:
-					actionsToSelect.push_back(possibleActions.front());
-					actionsToSelect.push_back(PossiblePlayerBattleAction::ATTACK);
-					break;
-					
-				case PossiblePlayerBattleAction::ATTACK_AND_RETURN:
-					actionsToSelect.push_back(possibleActions.front());
-					actionsToSelect.push_back(PossiblePlayerBattleAction::WALK_AND_ATTACK);
-					break;
-					
-				case PossiblePlayerBattleAction::AIMED_SPELL_CREATURE:
-					actionsToSelect.push_back(possibleActions.front());
-					actionsToSelect.push_back(PossiblePlayerBattleAction::ATTACK);
-					break;
-				case PossiblePlayerBattleAction::ANY_LOCATION:
-					actionsToSelect.push_back(possibleActions.front());
-					actionsToSelect.push_back(PossiblePlayerBattleAction::ATTACK);
-					break;
+				actionsToSelect.push_back(possibleActions.front());
+
+				auto shootActionPredicate = [](const PossiblePlayerBattleAction& action)
+				{
+					return action.get() == PossiblePlayerBattleAction::SHOOT;
+				};
+				bool hasShootSecondaryAction = std::any_of(possibleActions.begin() + 1, possibleActions.end(), shootActionPredicate);
+
+				if(hasShootSecondaryAction)
+					actionsToSelect.emplace_back(PossiblePlayerBattleAction::SHOOT);
+
+				actionsToSelect.emplace_back(PossiblePlayerBattleAction::ATTACK); //always allow melee attack as last option
 			}
 		}
 		owner.windowObject->setAlternativeActions(actionsToSelect);
@@ -1071,3 +1068,8 @@ void BattleActionsController::pushFrontPossibleAction(PossiblePlayerBattleAction
 {
 	possibleActions.insert(possibleActions.begin(), action);
 }
+
+void BattleActionsController::resetCurrentStackPossibleActions()
+{
+	possibleActions = getPossibleActionsForStack(owner.stacksController->getActiveStack());
+}

client/battle/BattleActionsController.h

@@ -122,4 +122,7 @@ public:
 	
 	/// inserts possible action in the beggining in order to prioritize it
 	void pushFrontPossibleAction(PossiblePlayerBattleAction);
+
+	/// resets possible actions to original state
+	void resetCurrentStackPossibleActions();
 };

client/battle/BattleWindow.cpp

@@ -48,7 +48,7 @@
 
 BattleWindow::BattleWindow(BattleInterface & owner):
 	owner(owner),
-	defaultAction(PossiblePlayerBattleAction::INVALID)
+	lastAlternativeAction(PossiblePlayerBattleAction::INVALID)
 {
 	OBJ_CONSTRUCTION_CAPTURING_ALL_NO_DISPOSE;
 	pos.w = 800;
@@ -568,14 +568,18 @@ void BattleWindow::showAlternativeActionIcon(PossiblePlayerBattleAction action)
 
 void BattleWindow::setAlternativeActions(const std::list<PossiblePlayerBattleAction> & actions)
 {
+	assert(actions.size() != 1);
+
 	alternativeActions = actions;
-	defaultAction = PossiblePlayerBattleAction::INVALID;
+	lastAlternativeAction = PossiblePlayerBattleAction::INVALID;
+
 	if(alternativeActions.size() > 1)
-		defaultAction = alternativeActions.back();
-	if(!alternativeActions.empty())
+	{
+		lastAlternativeAction = alternativeActions.back();
 		showAlternativeActionIcon(alternativeActions.front());
+	}
 	else
-		showAlternativeActionIcon(defaultAction);
+		showAlternativeActionIcon(PossiblePlayerBattleAction::INVALID);
 }
 
 void BattleWindow::bAutofightf()
@@ -670,23 +674,18 @@ void BattleWindow::bSwitchActionf()
 {
 	if(alternativeActions.empty())
 		return;
-	
-	if(alternativeActions.front() == defaultAction)
-	{
-		alternativeActions.push_back(alternativeActions.front());
-		alternativeActions.pop_front();
-	}
-	
+
 	auto actions = owner.actionsController->getPossibleActions();
-	if(!actions.empty() && actions.front() == alternativeActions.front())
+
+	if(!actions.empty() && actions.front() != lastAlternativeAction)
 	{
 		owner.actionsController->removePossibleAction(alternativeActions.front());
-		showAlternativeActionIcon(defaultAction);
+		showAlternativeActionIcon(*std::next(alternativeActions.begin()));
 	}
 	else
 	{
-		owner.actionsController->pushFrontPossibleAction(alternativeActions.front());
-		showAlternativeActionIcon(alternativeActions.front());
+		owner.actionsController->resetCurrentStackPossibleActions();
+		showAlternativeActionIcon(owner.actionsController->getPossibleActions().front());
 	}
 	
 	alternativeActions.push_back(alternativeActions.front());

client/battle/BattleWindow.h

@@ -65,7 +65,7 @@ class BattleWindow : public InterfaceObjectConfigurable
 	
 	/// management of alternative actions
 	std::list<PossiblePlayerBattleAction> alternativeActions;
-	PossiblePlayerBattleAction defaultAction;
+	PossiblePlayerBattleAction lastAlternativeAction;
 	void showAlternativeActionIcon(PossiblePlayerBattleAction);
 
 	/// flip battle queue visibility to opposite

lib/battle/PossiblePlayerBattleAction.h

@@ -74,6 +74,11 @@ public:
 	{
 		return action == other.action && spellToCast == other.spellToCast;
 	}
+
+	bool operator != (const PossiblePlayerBattleAction & other) const
+	{
+		return action != other.action || spellToCast != other.spellToCast;
+	}
 };
 
 VCMI_LIB_NAMESPACE_END