Creature spellcast refactor (#569)

* Move some logic to lib
* Mouse action priority queue enhancement
* Get rid of siegehandler dependency
* Improve AI offensive spellcasting
* CBattleInterface cleanup

AI/BattleAI/BattleAI.cpp

@@ -14,7 +14,6 @@
 
 #include "StackWithBonuses.h"
 #include "EnemyInfo.h"
-#include "PossibleSpellcast.h"
 #include "../../lib/CStopWatch.h"
 #include "../../lib/CThreadHelper.h"
 #include "../../lib/spells/CSpellHandler.h"
@@ -122,16 +121,54 @@ BattleAction CBattleAI::activeStack( const CStack * stack )
 			return *action;
 		//best action is from effective owner point if view, we are effective owner as we received "activeStack"
 
+	
+		//evaluate casting spell for spellcasting stack
+		boost::optional<PossibleSpellcast> bestSpellcast(boost::none);
+		//TODO: faerie dragon type spell should be selected by server
+		SpellID creatureSpellToCast = cb->battleGetRandomStackSpell(CRandomGenerator::getDefault(), stack, CBattleInfoCallback::RANDOM_AIMED);
+		if(stack->hasBonusOfType(Bonus::SPELLCASTER) && stack->canCast() && creatureSpellToCast != SpellID::NONE)
+		{
+			const CSpell * spell = creatureSpellToCast.toSpell();
+
+			if(spell->canBeCast(getCbc().get(), spells::Mode::CREATURE_ACTIVE, stack))
+			{
+				std::vector<PossibleSpellcast> possibleCasts;
+				spells::BattleCast temp(getCbc().get(), stack, spells::Mode::CREATURE_ACTIVE, spell);
+				for(auto & target : temp.findPotentialTargets())
+				{
+					PossibleSpellcast ps;
+					ps.dest = target;
+					ps.spell = spell;
+					evaluateCreatureSpellcast(stack, ps);
+					possibleCasts.push_back(ps);
+				}
+
+				std::sort(possibleCasts.begin(), possibleCasts.end(), [&](const PossibleSpellcast & lhs, const PossibleSpellcast & rhs) { return lhs.value > rhs.value; });
+				if(!possibleCasts.empty() && possibleCasts.front().value > 0)
+				{
+					bestSpellcast = boost::optional<PossibleSpellcast>(possibleCasts.front());
+				}
+			}
+		}
+
 		HypotheticBattle hb(getCbc());
 
 		PotentialTargets targets(stack, &hb);
 		if(targets.possibleAttacks.size())
 		{
-			auto hlp = targets.bestAction();
-			if(hlp.attack.shooting)
-				return BattleAction::makeShotAttack(stack, hlp.attack.defender);
+			AttackPossibility bestAttack = targets.bestAction();
+
+			//TODO: consider more complex spellcast evaluation, f.e. because "re-retaliation" during enemy move in same turn for melee attack etc.
+			if(bestSpellcast.is_initialized() && bestSpellcast->value > bestAttack.damageDiff())
+				return BattleAction::makeCreatureSpellcast(stack, bestSpellcast->dest, bestSpellcast->spell->id);
+			else if(bestAttack.attack.shooting)
+				return BattleAction::makeShotAttack(stack, bestAttack.attack.defender);
 			else
-				return BattleAction::makeMeleeAttack(stack, hlp.attack.defender->getPosition(), hlp.tile);
+				return BattleAction::makeMeleeAttack(stack, bestAttack.attack.defender->getPosition(), bestAttack.tile);
+		}
+		else if(bestSpellcast.is_initialized())
+		{
+			return BattleAction::makeCreatureSpellcast(stack, bestSpellcast->dest, bestSpellcast->spell->id);
 		}
 		else
 		{
@@ -521,6 +558,58 @@ void CBattleAI::attemptCastingSpell()
 	}
 }
 
+//Below method works only for offensive spells
+void CBattleAI::evaluateCreatureSpellcast(const CStack * stack, PossibleSpellcast & ps)
+{
+	using ValueMap = PossibleSpellcast::ValueMap;
+
+	RNGStub rngStub;
+	HypotheticBattle state(getCbc());
+	TStacks all = getCbc()->battleGetAllStacks(false);
+	
+	ValueMap healthOfStack;
+	ValueMap newHealthOfStack;
+
+	for(auto unit : all)
+	{
+		healthOfStack[unit->unitId()] = unit->getAvailableHealth();
+	}
+
+	spells::BattleCast cast(&state, stack, spells::Mode::CREATURE_ACTIVE, ps.spell);
+	cast.target = ps.dest;
+	cast.cast(&state, rngStub);
+
+	for(auto unit : all)
+	{
+		auto unitId = unit->unitId();
+		auto localUnit = state.battleGetUnitByID(unitId);
+		newHealthOfStack[unitId] = localUnit->getAvailableHealth();
+	}
+
+	int64_t totalGain = 0;
+
+	for(auto unit : all)
+	{
+		auto unitId = unit->unitId();
+		auto localUnit = state.battleGetUnitByID(unitId);
+
+		auto healthDiff = newHealthOfStack[unitId] - healthOfStack[unitId];
+
+		if(localUnit->unitOwner() != getCbc()->getPlayerID())
+			healthDiff = -healthDiff;
+
+		if(healthDiff < 0)
+		{
+			ps.value = -1;
+			return; //do not damage own units at all
+		}
+
+		totalGain += healthDiff;
+	}
+
+	ps.value = totalGain;
+};
+
 int CBattleAI::distToNearestNeighbour(BattleHex hex, const ReachabilityInfo::TDistances &dists, BattleHex *chosenHex)
 {
 	int ret = 1000000;

AI/BattleAI/BattleAI.h

@@ -9,6 +9,7 @@
  */
 #pragma once
 #include "../../lib/AI_Base.h"
+#include "PossibleSpellcast.h"
 #include "PotentialTargets.h"
 
 class CSpell;
@@ -60,6 +61,8 @@ public:
 	void init(std::shared_ptr<CBattleCallback> CB) override;
 	void attemptCastingSpell();
 
+	void evaluateCreatureSpellcast(const CStack * stack, PossibleSpellcast & ps); //for offensive damaging spells only
+
 	BattleAction activeStack(const CStack * stack) override; //called when it's turn of that stack
 	BattleAction goTowards(const CStack * stack, BattleHex hex );
 

client/battle/CBattleInterface.cpp

@@ -415,8 +415,8 @@ CBattleInterface::CBattleInterface(const CCreatureSet *army1, const CCreatureSet
 	CCS->soundh->setCallback(battleIntroSoundChannel, onIntroPlayed);
 	memset(stackCountOutsideHexes, 1, GameConstants::BFIELD_SIZE *sizeof(bool)); //initialize array with trues
 
-	currentAction = INVALID;
-	selectedAction = INVALID;
+	currentAction = PossiblePlayerBattleAction::INVALID;
+	selectedAction = PossiblePlayerBattleAction::INVALID;
 	addUsedEvents(RCLICK | MOVE | KEYBOARD);
 	blockUI(true);
 }
@@ -1395,24 +1395,6 @@ void CBattleInterface::battleStacksEffectsSet(const SetStackEffect & sse)
 		redrawBackgroundWithHexes(activeStack);
 }
 
-CBattleInterface::PossibleActions CBattleInterface::getCasterAction(const CSpell * spell, const spells::Caster * caster, spells::Mode mode) const
-{
-	PossibleActions spellSelMode = ANY_LOCATION;
-
-	const CSpell::TargetInfo ti(spell, caster->getSpellSchoolLevel(spell), mode);
-
-	if(ti.massive || ti.type == spells::AimType::NO_TARGET)
-		spellSelMode = NO_LOCATION;
-	else if(ti.type == spells::AimType::LOCATION && ti.clearAffected)
-		spellSelMode = FREE_LOCATION;
-	else if(ti.type == spells::AimType::CREATURE)
-		spellSelMode = AIMED_SPELL_CREATURE;
-	else if(ti.type == spells::AimType::OBSTACLE)
-		spellSelMode = OBSTACLE;
-
-	return spellSelMode;
-}
-
 void CBattleInterface::setHeroAnimation(ui8 side, int phase)
 {
 	if(side == BattleSide::ATTACKER)
@@ -1441,9 +1423,9 @@ void CBattleInterface::castThisSpell(SpellID spellID)
 	const CGHeroInstance *castingHero = (attackingHeroInstance->tempOwner == curInt->playerID) ? attackingHeroInstance : defendingHeroInstance;
 	assert(castingHero); // code below assumes non-null hero
 	sp = spellID.toSpell();
-	PossibleActions spellSelMode = getCasterAction(sp, castingHero, spells::Mode::HERO);
+	PossiblePlayerBattleAction spellSelMode = curInt->cb->getCasterAction(sp, castingHero, spells::Mode::HERO);
 
-	if (spellSelMode == NO_LOCATION) //user does not have to select location
+	if (spellSelMode == PossiblePlayerBattleAction::NO_LOCATION) //user does not have to select location
 	{
 		spellToCast->aimToHex(BattleHex::INVALID);
 		curInt->cb->battleMakeAction(spellToCast.get());
@@ -1669,7 +1651,7 @@ void CBattleInterface::activateStack()
 		creatureSpellToCast = -1;
 	}
 
-	getPossibleActionsForStack(s, false);
+	possibleActions = getPossibleActionsForStack(s);
 
 	GH.fakeMouseMove();
 }
@@ -1686,7 +1668,7 @@ void CBattleInterface::endCastingSpell()
 
 		if(activeStack)
 		{
-			getPossibleActionsForStack(activeStack, false); //restore actions after they were cleared
+			possibleActions = getPossibleActionsForStack(activeStack); //restore actions after they were cleared
 			myTurn = true;
 		}
 	}
@@ -1694,7 +1676,7 @@ void CBattleInterface::endCastingSpell()
 	{
 		if(activeStack)
 		{
-			getPossibleActionsForStack(activeStack, false);
+			possibleActions = getPossibleActionsForStack(activeStack);
 			GH.fakeMouseMove();
 		}
 	}
@@ -1723,7 +1705,7 @@ void CBattleInterface::enterCreatureCastingMode()
 	if (creatureSpellToCast == -1)
 		return;
 
-	if (vstd::contains(possibleActions, NO_LOCATION))
+	if (vstd::contains(possibleActions, PossiblePlayerBattleAction::NO_LOCATION))
 	{
 		const spells::Caster *caster = activeStack;
 		const CSpell *spell = SpellID(creatureSpellToCast).toSpell();
@@ -1740,67 +1722,77 @@ void CBattleInterface::enterCreatureCastingMode()
 	}
 	else
 	{
-		getPossibleActionsForStack(activeStack, true);
+		possibleActions = getPossibleActionsForStack(activeStack);
+
+		auto actionFilterPredicate = [](const PossiblePlayerBattleAction x)
+		{
+			return (x != PossiblePlayerBattleAction::ANY_LOCATION) && (x != PossiblePlayerBattleAction::NO_LOCATION) &&
+				(x != PossiblePlayerBattleAction::FREE_LOCATION) && (x != PossiblePlayerBattleAction::AIMED_SPELL_CREATURE) && 
+				(x != PossiblePlayerBattleAction::OBSTACLE);
+		};
+
+		vstd::erase_if(possibleActions, actionFilterPredicate);
 		GH.fakeMouseMove();
 	}
 }
 
-void CBattleInterface::getPossibleActionsForStack(const CStack *stack, const bool forceCast)
+std::vector<PossiblePlayerBattleAction> CBattleInterface::getPossibleActionsForStack(const CStack *stack)
 {
-	possibleActions.clear();
-	if (tacticsMode)
-	{
-		possibleActions.push_back(MOVE_TACTICS);
-		possibleActions.push_back(CHOOSE_TACTICS_STACK);
-	}
-	else
-	{
-		PossibleActions notPriority = INVALID;
-		//first action will be prioritized over later ones
-		if(stack->canCast()) //TODO: check for battlefield effects that prevent casting?
-		{
-			if(stack->hasBonusOfType (Bonus::SPELLCASTER))
-			{
-				if(creatureSpellToCast != -1)
-				{
-					const CSpell *spell = SpellID(creatureSpellToCast).toSpell();
-					PossibleActions act = getCasterAction(spell, stack, spells::Mode::CREATURE_ACTIVE);
+	BattleClientInterfaceData data; //hard to get rid of these things so for now they're required data to pass
+	data.creatureSpellToCast = creatureSpellToCast;
+	data.tacticsMode = tacticsMode;
+	auto allActions = curInt->cb->getClientActionsForStack(stack, data);
 
-					if(forceCast)
-					{
-						//forced action to be only one possible
-						possibleActions.push_back(act);
-						return;
-					}
-					else
-						//if cast is not forced, cast action will have lowest priority
-						notPriority = act;
-				}
-			}
-			if (stack->hasBonusOfType (Bonus::RANDOM_SPELLCASTER))
-				possibleActions.push_back (RANDOM_GENIE_SPELL);
-			if (stack->hasBonusOfType (Bonus::DAEMON_SUMMONING))
-				possibleActions.push_back (RISE_DEMONS);
-		}
-		if(stack->canShoot())
-			possibleActions.push_back(SHOOT);
-		if(stack->hasBonusOfType(Bonus::RETURN_AFTER_STRIKE))
-			possibleActions.push_back(ATTACK_AND_RETURN);
+	return std::vector<PossiblePlayerBattleAction>(allActions);
+}
 
-		possibleActions.push_back(ATTACK); //all active stacks can attack
-		possibleActions.push_back(WALK_AND_ATTACK); //not all stacks can always walk, but we will check this elsewhere
+void CBattleInterface::reorderPossibleActionsPriority(const CStack * stack, MouseHoveredHexContext context)
+{
+	if(tacticsMode || possibleActions.empty()) return; //this function is not supposed to be called in tactics mode or before getPossibleActionsForStack
 
-		if (stack->canMove() && stack->Speed(0, true)) //probably no reason to try move war machines or bound stacks
-			possibleActions.push_back (MOVE_STACK); //all active stacks can attack
+	auto assignPriority = [&](PossiblePlayerBattleAction const & item) -> uint8_t //large lambda assigning priority which would have to be part of possibleActions without it
+	{
+		switch(item)
+		{
+		case PossiblePlayerBattleAction::AIMED_SPELL_CREATURE:
+		case PossiblePlayerBattleAction::ANY_LOCATION:
+		case PossiblePlayerBattleAction::NO_LOCATION:
+		case PossiblePlayerBattleAction::FREE_LOCATION:
+		case PossiblePlayerBattleAction::OBSTACLE:
+			if(!stack->hasBonusOfType(Bonus::NO_SPELLCAST_BY_DEFAULT) && context == MouseHoveredHexContext::OCCUPIED_HEX)
+				return 1;
+			else
+				return 100;//bottom priority
+			break;
+		case PossiblePlayerBattleAction::RANDOM_GENIE_SPELL:
+			return 2; break;
+		case PossiblePlayerBattleAction::RISE_DEMONS:
+			return 3; break;
+		case PossiblePlayerBattleAction::SHOOT:
+			return 4; break;
+		case PossiblePlayerBattleAction::ATTACK_AND_RETURN:
+			return 5; break;
+		case PossiblePlayerBattleAction::ATTACK:
+			return 6; break;
+		case PossiblePlayerBattleAction::WALK_AND_ATTACK:
+			return 7; break;
+		case PossiblePlayerBattleAction::MOVE_STACK:
+			return 8; break;
+		case PossiblePlayerBattleAction::CATAPULT:
+			return 9; break;		
+		case PossiblePlayerBattleAction::HEAL:
+			return 10; break;	
+		default:
+			return 200; break;
+		}
+	};
 
-		if (siegeH && stack->hasBonusOfType (Bonus::CATAPULT)) //TODO: check shots
-			possibleActions.push_back (CATAPULT);
-		if (stack->hasBonusOfType (Bonus::HEALER))
-			possibleActions.push_back (HEAL);
+	auto comparer = [&](PossiblePlayerBattleAction const & lhs, PossiblePlayerBattleAction const & rhs)
+	{
+		return assignPriority(lhs) > assignPriority(rhs);
+	};
 
-		if (notPriority != INVALID)
-			possibleActions.push_back(notPriority);
-	}
+	std::make_heap(possibleActions.begin(), possibleActions.end(), comparer);
 }
 
 void CBattleInterface::printConsoleAttacked(const CStack * defender, int dmg, int killed, const CStack * attacker, bool multiple)
@@ -2127,21 +2119,22 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 	localActions.clear();
 	illegalActions.clear();
 
+	reorderPossibleActionsPriority(activeStack, shere ? MouseHoveredHexContext::OCCUPIED_HEX : MouseHoveredHexContext::UNOCCUPIED_HEX);
 	const bool forcedAction = possibleActions.size() == 1;
 
-	for (PossibleActions action : possibleActions)
+	for (PossiblePlayerBattleAction action : possibleActions)
 	{
 		bool legalAction = false; //this action is legal and can be performed
 		bool notLegal = false; //this action is not legal and should display message
 
 		switch (action)
 		{
-			case CHOOSE_TACTICS_STACK:
+			case PossiblePlayerBattleAction::CHOOSE_TACTICS_STACK:
 				if (shere && ourStack)
 					legalAction = true;
 				break;
-			case MOVE_TACTICS:
-			case MOVE_STACK:
+			case PossiblePlayerBattleAction::MOVE_TACTICS:
+			case PossiblePlayerBattleAction::MOVE_STACK:
 			{
 				if (!(shere && shere->alive())) //we can walk on dead stacks
 				{
@@ -2150,9 +2143,9 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 				}
 				break;
 			}
-			case ATTACK:
-			case WALK_AND_ATTACK:
-			case ATTACK_AND_RETURN:
+			case PossiblePlayerBattleAction::ATTACK:
+			case PossiblePlayerBattleAction::WALK_AND_ATTACK:
+			case PossiblePlayerBattleAction::ATTACK_AND_RETURN:
 			{
 				if(curInt->cb->battleCanAttack(activeStack, shere, myNumber))
 				{
@@ -2167,22 +2160,22 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 				}
 			}
 				break;
-			case SHOOT:
+			case PossiblePlayerBattleAction::SHOOT:
 				if(curInt->cb->battleCanShoot(activeStack, myNumber))
 					legalAction = true;
 				break;
-			case ANY_LOCATION:
+			case PossiblePlayerBattleAction::ANY_LOCATION:
 				if (myNumber > -1) //TODO: this should be checked for all actions
 				{
 					if(isCastingPossibleHere(activeStack, shere, myNumber))
 						legalAction = true;
 				}
 				break;
-			case AIMED_SPELL_CREATURE:
+			case PossiblePlayerBattleAction::AIMED_SPELL_CREATURE:
 				if(shere && isCastingPossibleHere(activeStack, shere, myNumber))
 					legalAction = true;
 				break;
-			case RANDOM_GENIE_SPELL:
+			case PossiblePlayerBattleAction::RANDOM_GENIE_SPELL:
 			{
 				if(shere && ourStack && shere != activeStack && shere->alive()) //only positive spells for other allied creatures
 				{
@@ -2194,11 +2187,11 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 				}
 			}
 				break;
-			case OBSTACLE:
+			case PossiblePlayerBattleAction::OBSTACLE:
 				if(isCastingPossibleHere(activeStack, shere, myNumber))
 					legalAction = true;
 				break;
-			case TELEPORT:
+			case PossiblePlayerBattleAction::TELEPORT:
 			{
 				//todo: move to mechanics
 				ui8 skill = 0;
@@ -2213,13 +2206,13 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 					notLegal = true;
 			}
 				break;
-			case SACRIFICE: //choose our living stack to sacrifice
+			case PossiblePlayerBattleAction::SACRIFICE: //choose our living stack to sacrifice
 				if (shere && shere != selectedStack && ourStack && shere->alive())
 					legalAction = true;
 				else
 					notLegal = true;
 				break;
-			case FREE_LOCATION:
+			case PossiblePlayerBattleAction::FREE_LOCATION:
 				legalAction = true;
 				if(!isCastingPossibleHere(activeStack, shere, myNumber))
 				{
@@ -2227,15 +2220,15 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 					notLegal = true;
 				}
 				break;
-			case CATAPULT:
+			case PossiblePlayerBattleAction::CATAPULT:
 				if (isCatapultAttackable(myNumber))
 					legalAction = true;
 				break;
-			case HEAL:
+			case PossiblePlayerBattleAction::HEAL:
 				if (shere && ourStack && shere->canBeHealed())
 					legalAction = true;
 				break;
-			case RISE_DEMONS:
+			case PossiblePlayerBattleAction::RISE_DEMONS:
 				if (shere && ourStack && !shere->alive())
 				{
 					if (!(shere->hasBonusOfType(Bonus::UNDEAD)
@@ -2253,7 +2246,7 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 		else if (notLegal || forcedAction)
 			illegalActions.push_back (action);
 	}
-	illegalAction = INVALID; //clear it in first place
+	illegalAction = PossiblePlayerBattleAction::INVALID; //clear it in first place
 
 	if (vstd::contains(localActions, selectedAction)) //try to use last selected action by default
 		currentAction = selectedAction;
@@ -2261,7 +2254,7 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 		currentAction = localActions.front();
 	else //no legal action possible
 	{
-		currentAction = INVALID; //don't allow to do anything
+		currentAction = PossiblePlayerBattleAction::INVALID; //don't allow to do anything
 
 		if (vstd::contains(illegalActions, selectedAction))
 			illegalAction = selectedAction;
@@ -2269,25 +2262,25 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 			illegalAction = illegalActions.front();
 		else if (shere && ourStack && shere->alive()) //last possibility - display info about our creature
 		{
-			currentAction = CREATURE_INFO;
+			currentAction = PossiblePlayerBattleAction::CREATURE_INFO;
 		}
 		else
-			illegalAction = INVALID; //we should never be here
+			illegalAction = PossiblePlayerBattleAction::INVALID; //we should never be here
 	}
 
 	bool isCastingPossible = false;
 	bool secondaryTarget = false;
 
-	if (currentAction > INVALID)
+	if (currentAction > PossiblePlayerBattleAction::INVALID)
 	{
 		switch (currentAction) //display console message, realize selected action
 		{
-			case CHOOSE_TACTICS_STACK:
+			case PossiblePlayerBattleAction::CHOOSE_TACTICS_STACK:
 				consoleMsg = (boost::format(CGI->generaltexth->allTexts[481]) % shere->getName()).str(); //Select %s
 				realizeAction = [=](){ stackActivated(shere); };
 				break;
-			case MOVE_TACTICS:
-			case MOVE_STACK:
+			case PossiblePlayerBattleAction::MOVE_TACTICS:
+			case PossiblePlayerBattleAction::MOVE_STACK:
 				if (activeStack->hasBonusOfType(Bonus::FLYING))
 				{
 					cursorFrame = ECursor::COMBAT_FLY;
@@ -2316,14 +2309,14 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 					}
 				};
 				break;
-			case ATTACK:
-			case WALK_AND_ATTACK:
-			case ATTACK_AND_RETURN: //TODO: allow to disable return
+			case PossiblePlayerBattleAction::ATTACK:
+			case PossiblePlayerBattleAction::WALK_AND_ATTACK:
+			case PossiblePlayerBattleAction::ATTACK_AND_RETURN: //TODO: allow to disable return
 				{
 					setBattleCursor(myNumber); //handle direction of cursor and attackable tile
 					setCursor = false; //don't overwrite settings from the call above //TODO: what does it mean?
 
-					bool returnAfterAttack = currentAction == ATTACK_AND_RETURN;
+					bool returnAfterAttack = currentAction == PossiblePlayerBattleAction::ATTACK_AND_RETURN;
 
 					realizeAction = [=]()
 					{
@@ -2339,7 +2332,7 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 					consoleMsg = (boost::format(CGI->generaltexth->allTexts[36]) % shere->getName() % estDmgText).str(); //Attack %s (%s damage)
 				}
 				break;
-			case SHOOT:
+			case PossiblePlayerBattleAction::SHOOT:
 			{
 				if (curInt->cb->battleHasShootingPenalty(activeStack, myNumber))
 					cursorFrame = ECursor::COMBAT_SHOOT_PENALTY;
@@ -2352,7 +2345,7 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 				consoleMsg = (boost::format(CGI->generaltexth->allTexts[296]) % shere->getName() % activeStack->shots.available() % estDmgText).str();
 			}
 				break;
-			case AIMED_SPELL_CREATURE:
+			case PossiblePlayerBattleAction::AIMED_SPELL_CREATURE:
 				sp = CGI->spellh->objects[creatureCasting ? creatureSpellToCast : spellToCast->actionSubtype]; //necessary if creature has random Genie spell at same time
 				consoleMsg = boost::str(boost::format(CGI->generaltexth->allTexts[27]) % sp->name % shere->getName()); //Cast %s on %s
 				switch (sp->id)
@@ -2365,53 +2358,53 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 				}
 				isCastingPossible = true;
 				break;
-			case ANY_LOCATION:
+			case PossiblePlayerBattleAction::ANY_LOCATION:
 				sp = CGI->spellh->objects[creatureCasting ? creatureSpellToCast : spellToCast->actionSubtype]; //necessary if creature has random Genie spell at same time
 				consoleMsg = boost::str(boost::format(CGI->generaltexth->allTexts[26]) % sp->name); //Cast %s
 				isCastingPossible = true;
 				break;
-			case RANDOM_GENIE_SPELL: //we assume that teleport / sacrifice will never be available as random spell
+			case PossiblePlayerBattleAction::RANDOM_GENIE_SPELL: //we assume that teleport / sacrifice will never be available as random spell
 				sp = nullptr;
 				consoleMsg = boost::str(boost::format(CGI->generaltexth->allTexts[301]) % shere->getName()); //Cast a spell on %
 				creatureCasting = true;
 				isCastingPossible = true;
 				break;
-			case TELEPORT:
+			case PossiblePlayerBattleAction::TELEPORT:
 				consoleMsg = CGI->generaltexth->allTexts[25]; //Teleport Here
 				cursorFrame = ECursor::COMBAT_TELEPORT;
 				isCastingPossible = true;
 				break;
-			case OBSTACLE:
+			case PossiblePlayerBattleAction::OBSTACLE:
 				consoleMsg = CGI->generaltexth->allTexts[550];
 				//TODO: remove obstacle cursor
 				isCastingPossible = true;
 				break;
-			case SACRIFICE:
+			case PossiblePlayerBattleAction::SACRIFICE:
 				consoleMsg = (boost::format(CGI->generaltexth->allTexts[549]) % shere->getName()).str(); //sacrifice the %s
 				cursorFrame = ECursor::COMBAT_SACRIFICE;
 				isCastingPossible = true;
 				break;
-			case FREE_LOCATION:
+			case PossiblePlayerBattleAction::FREE_LOCATION:
 				consoleMsg = boost::str(boost::format(CGI->generaltexth->allTexts[26]) % sp->name); //Cast %s
 				isCastingPossible = true;
 				break;
-			case HEAL:
+			case PossiblePlayerBattleAction::HEAL:
 				cursorFrame = ECursor::COMBAT_HEAL;
 				consoleMsg = (boost::format(CGI->generaltexth->allTexts[419]) % shere->getName()).str(); //Apply first aid to the %s
 				realizeAction = [=](){ giveCommand(EActionType::STACK_HEAL, myNumber); }; //command healing
 				break;
-			case RISE_DEMONS:
+			case PossiblePlayerBattleAction::RISE_DEMONS:
 				cursorType = ECursor::SPELLBOOK;
 				realizeAction = [=]()
 				{
 					giveCommand(EActionType::DAEMON_SUMMONING, myNumber);
 				};
 				break;
-			case CATAPULT:
+			case PossiblePlayerBattleAction::CATAPULT:
 				cursorFrame = ECursor::COMBAT_SHOOT_CATAPULT;
 				realizeAction = [=](){ giveCommand(EActionType::CATAPULT, myNumber); };
 				break;
-			case CREATURE_INFO:
+			case PossiblePlayerBattleAction::CREATURE_INFO:
 			{
 				cursorFrame = ECursor::COMBAT_QUERY;
 				consoleMsg = (boost::format(CGI->generaltexth->allTexts[297]) % shere->getName()).str();
@@ -2424,19 +2417,19 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 	{
 		switch (illegalAction)
 		{
-			case AIMED_SPELL_CREATURE:
-			case RANDOM_GENIE_SPELL:
+			case PossiblePlayerBattleAction::AIMED_SPELL_CREATURE:
+			case PossiblePlayerBattleAction::RANDOM_GENIE_SPELL:
 				cursorFrame = ECursor::COMBAT_BLOCKED;
 				consoleMsg = CGI->generaltexth->allTexts[23];
 				break;
-			case TELEPORT:
+			case PossiblePlayerBattleAction::TELEPORT:
 				cursorFrame = ECursor::COMBAT_BLOCKED;
 				consoleMsg = CGI->generaltexth->allTexts[24]; //Invalid Teleport Destination
 				break;
-			case SACRIFICE:
+			case PossiblePlayerBattleAction::SACRIFICE:
 				consoleMsg = CGI->generaltexth->allTexts[543]; //choose army to sacrifice
 				break;
-			case FREE_LOCATION:
+			case PossiblePlayerBattleAction::FREE_LOCATION:
 				cursorFrame = ECursor::COMBAT_BLOCKED;
 				consoleMsg = boost::str(boost::format(CGI->generaltexth->allTexts[181]) % sp->name); //No room to place %s here
 				break;
@@ -2453,8 +2446,8 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 	{
 		switch (currentAction) //don't use that with teleport / sacrifice
 		{
-			case TELEPORT: //FIXME: more generic solution?
-			case SACRIFICE:
+			case PossiblePlayerBattleAction::TELEPORT: //FIXME: more generic solution?
+			case PossiblePlayerBattleAction::SACRIFICE:
 				break;
 			default:
 				cursorType = ECursor::SPELLBOOK;
@@ -2474,11 +2467,11 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 				{
 					case SpellID::TELEPORT: //don't cast spell yet, only select target
 						spellToCast->aimToUnit(shere);
-						possibleActions.push_back(TELEPORT);
+						possibleActions.push_back(PossiblePlayerBattleAction::TELEPORT);
 						break;
 					case SpellID::SACRIFICE:
 						spellToCast->aimToHex(myNumber);
-						possibleActions.push_back(SACRIFICE);
+						possibleActions.push_back(PossiblePlayerBattleAction::SACRIFICE);
 						break;
 				}
 			}
@@ -2526,7 +2519,7 @@ void CBattleInterface::handleHex(BattleHex myNumber, int eventType)
 		if (eventType == LCLICK && realizeAction)
 		{
 			//opening creature window shouldn't affect myTurn...
-			if ((currentAction != CREATURE_INFO) && !secondaryTarget)
+			if ((currentAction != PossiblePlayerBattleAction::CREATURE_INFO) && !secondaryTarget)
 			{
 				myTurn = false; //tends to crash with empty calls
 			}

client/battle/CBattleInterface.h

@@ -15,6 +15,7 @@
 #include "CBattleAnimations.h"
 
 #include "../../lib/spells/CSpellHandler.h" //CSpell::TAnimation
+#include "../../lib/battle/CBattleInfoCallback.h"
 
 class CLabel;
 class CCreatureSet;
@@ -104,20 +105,16 @@ struct CatapultProjectileInfo
 	double calculateY(double x);
 };
 
+enum class MouseHoveredHexContext
+{
+	UNOCCUPIED_HEX,
+	OCCUPIED_HEX
+};
+
 /// Big class which handles the overall battle interface actions and it is also responsible for
 /// drawing everything correctly.
 class CBattleInterface : public WindowBase
 {
-	enum PossibleActions // actions performed at l-click
-	{
-		INVALID = -1, CREATURE_INFO,
-		MOVE_TACTICS, CHOOSE_TACTICS_STACK,
-		MOVE_STACK, ATTACK, WALK_AND_ATTACK, ATTACK_AND_RETURN, SHOOT, //OPEN_GATE, //we can open castle gate during siege
-		NO_LOCATION, ANY_LOCATION, OBSTACLE, TELEPORT, SACRIFICE, RANDOM_GENIE_SPELL,
-		FREE_LOCATION, //used with Force Field and Fire Wall - all tiles affected by spell must be free
-		CATAPULT, HEAL, RISE_DEMONS,
-		AIMED_SPELL_CREATURE
-	};
 private:
 	SDL_Surface *background, *menu, *amountNormal, *amountNegative, *amountPositive, *amountEffNeutral, *cellBorders, *backgroundWithHexes;
 
@@ -169,12 +166,12 @@ private:
 	std::shared_ptr<BattleAction> spellToCast; //spell for which player is choosing destination
 	const CSpell *sp; //spell pointer for convenience
 	si32 creatureSpellToCast;
-	std::vector<PossibleActions> possibleActions; //all actions possible to call at the moment by player
-	std::vector<PossibleActions> localActions; //actions possible to take on hovered hex
-	std::vector<PossibleActions> illegalActions; //these actions display message in case of illegal target
-	PossibleActions currentAction; //action that will be performed on l-click
-	PossibleActions selectedAction; //last action chosen (and saved) by player
-	PossibleActions illegalAction; //most likely action that can't be performed here
+	std::vector<PossiblePlayerBattleAction> possibleActions; //all actions possible to call at the moment by player
+	std::vector<PossiblePlayerBattleAction> localActions; //actions possible to take on hovered hex
+	std::vector<PossiblePlayerBattleAction> illegalActions; //these actions display message in case of illegal target
+	PossiblePlayerBattleAction currentAction; //action that will be performed on l-click
+	PossiblePlayerBattleAction selectedAction; //last action chosen (and saved) by player
+	PossiblePlayerBattleAction illegalAction; //most likely action that can't be performed here
 	bool battleActionsStarted; //used for delaying battle actions until intro sound stops
 	int battleIntroSoundChannel; //required as variable for disabling it via ESC key
 
@@ -183,8 +180,9 @@ private:
 
 	void requestAutofightingAIToTakeAction();
 
-	void getPossibleActionsForStack (const CStack *stack, const bool forceCast); //called when stack gets its turn
+	std::vector<PossiblePlayerBattleAction> getPossibleActionsForStack (const CStack *stack); //called when stack gets its turn
 	void endCastingSpell(); //ends casting spell (eg. when spell has been cast or canceled)
+	void reorderPossibleActionsPriority(const CStack * stack, MouseHoveredHexContext context);
 
 	//force active stack to cast a spell if possible
 	void enterCreatureCastingMode();
@@ -275,8 +273,6 @@ private:
 	void redrawBackgroundWithHexes(const CStack *activeStack);
 	/** End of battle screen blitting methods */
 
-	PossibleActions getCasterAction(const CSpell *spell, const spells::Caster *caster, spells::Mode mode) const;
-
 	void setHeroAnimation(ui8 side, int phase);
 public:
 	static CondSh<bool> animsAreDisplayed; //for waiting with the end of battle for end of anims

lib/HeroBonus.h

@@ -270,6 +270,7 @@ public:
 	BONUS_NAME(BLOCK_MAGIC_BELOW) /*blocks casting spells of the level < value */ \
 	BONUS_NAME(DESTRUCTION) /*kills extra units after hit, subtype = 0 - kill percentage of units, 1 - kill amount, val = chance in percent to trigger, additional info - amount/percentage to kill*/ \
 	BONUS_NAME(SPECIAL_CRYSTAL_GENERATION) /*crystal dragon crystal generation*/ \
+	BONUS_NAME(NO_SPELLCAST_BY_DEFAULT) /*spellcast will not be default attack option for this creature*/ \
 
 	/* end of list */
 

lib/battle/BattleAction.cpp

@@ -74,6 +74,17 @@ BattleAction BattleAction::makeShotAttack(const battle::Unit * shooter, const ba
 	return ba;
 }
 
+BattleAction BattleAction::makeCreatureSpellcast(const battle::Unit * stack, const battle::Target & target, SpellID spellID)
+{
+	BattleAction ba;
+	ba.actionType = EActionType::MONSTER_SPELL;
+	ba.actionSubtype = spellID;
+	ba.setTarget(target);
+	ba.side = stack->unitSide();
+	ba.stackNumber = stack->unitId();
+	return ba;
+}
+
 BattleAction BattleAction::makeMove(const battle::Unit * stack, BattleHex dest)
 {
 	BattleAction ba;

lib/battle/BattleAction.h

@@ -35,6 +35,7 @@ public:
 	static BattleAction makeWait(const battle::Unit * stack);
 	static BattleAction makeMeleeAttack(const battle::Unit * stack, BattleHex destination, BattleHex attackFrom, bool returnAfterAttack = true);
 	static BattleAction makeShotAttack(const battle::Unit * shooter, const battle::Unit * target);
+	static BattleAction makeCreatureSpellcast(const battle::Unit * stack, const battle::Target & target, SpellID spellID);
 	static BattleAction makeMove(const battle::Unit * stack, BattleHex dest);
 	static BattleAction makeEndOFTacticPhase(ui8 side);
 

lib/battle/CBattleInfoCallback.cpp

@@ -203,6 +203,70 @@ si8 CBattleInfoCallback::battleCanTeleportTo(const battle::Unit * stack, BattleH
 	return true;
 }
 
+std::vector<PossiblePlayerBattleAction> CBattleInfoCallback::getClientActionsForStack(const CStack * stack, const BattleClientInterfaceData & data)
+{
+	RETURN_IF_NOT_BATTLE(std::vector<PossiblePlayerBattleAction>());
+	std::vector<PossiblePlayerBattleAction> allowedActionList;
+	if(data.tacticsMode) //would "if(battleGetTacticDist() > 0)" work?
+	{
+		allowedActionList.push_back(PossiblePlayerBattleAction::MOVE_TACTICS);
+		allowedActionList.push_back(PossiblePlayerBattleAction::CHOOSE_TACTICS_STACK);
+	}
+	else
+	{
+		if(stack->canCast()) //TODO: check for battlefield effects that prevent casting?
+		{
+			if(stack->hasBonusOfType(Bonus::SPELLCASTER) && data.creatureSpellToCast != -1)
+			{
+				const CSpell *spell = SpellID(data.creatureSpellToCast).toSpell();
+				PossiblePlayerBattleAction act = getCasterAction(spell, stack, spells::Mode::CREATURE_ACTIVE);
+				allowedActionList.push_back(act);
+			}
+			if(stack->hasBonusOfType(Bonus::RANDOM_SPELLCASTER))
+				allowedActionList.push_back(PossiblePlayerBattleAction::RANDOM_GENIE_SPELL);
+			if(stack->hasBonusOfType(Bonus::DAEMON_SUMMONING))
+				allowedActionList.push_back(PossiblePlayerBattleAction::RISE_DEMONS);
+		}
+		if(stack->canShoot())
+			allowedActionList.push_back(PossiblePlayerBattleAction::SHOOT);
+		if(stack->hasBonusOfType(Bonus::RETURN_AFTER_STRIKE))
+			allowedActionList.push_back(PossiblePlayerBattleAction::ATTACK_AND_RETURN);
+
+		allowedActionList.push_back(PossiblePlayerBattleAction::ATTACK); //all active stacks can attack
+		allowedActionList.push_back(PossiblePlayerBattleAction::WALK_AND_ATTACK); //not all stacks can always walk, but we will check this elsewhere
+
+		if(stack->canMove() && stack->Speed(0, true)) //probably no reason to try move war machines or bound stacks
+			allowedActionList.push_back(PossiblePlayerBattleAction::MOVE_STACK);
+
+		auto siegedTown = battleGetDefendedTown();
+		if(siegedTown && siegedTown->hasFort() && stack->hasBonusOfType(Bonus::CATAPULT)) //TODO: check shots
+			allowedActionList.push_back(PossiblePlayerBattleAction::CATAPULT);
+		if(stack->hasBonusOfType(Bonus::HEALER))
+			allowedActionList.push_back(PossiblePlayerBattleAction::HEAL);
+	}
+
+	return allowedActionList;
+}
+
+PossiblePlayerBattleAction CBattleInfoCallback::getCasterAction(const CSpell * spell, const spells::Caster * caster, spells::Mode mode) const
+{
+	RETURN_IF_NOT_BATTLE(PossiblePlayerBattleAction::INVALID);
+	PossiblePlayerBattleAction spellSelMode = PossiblePlayerBattleAction::ANY_LOCATION;
+
+	const CSpell::TargetInfo ti(spell, caster->getSpellSchoolLevel(spell), mode);
+
+	if(ti.massive || ti.type == spells::AimType::NO_TARGET)
+		spellSelMode = PossiblePlayerBattleAction::NO_LOCATION;
+	else if(ti.type == spells::AimType::LOCATION && ti.clearAffected)
+		spellSelMode = PossiblePlayerBattleAction::FREE_LOCATION;
+	else if(ti.type == spells::AimType::CREATURE)
+		spellSelMode = PossiblePlayerBattleAction::AIMED_SPELL_CREATURE;
+	else if(ti.type == spells::AimType::OBSTACLE)
+		spellSelMode = PossiblePlayerBattleAction::OBSTACLE;
+
+	return spellSelMode;
+}
+
 std::set<BattleHex> CBattleInfoCallback::battleGetAttackedHexes(const CStack* attacker, BattleHex destinationTile, BattleHex attackerPos) const
 {
 	std::set<BattleHex> attackedHexes;

lib/battle/CBattleInfoCallback.h

@@ -32,6 +32,23 @@ struct DLL_LINKAGE AttackableTiles
 	}
 };
 
+enum class PossiblePlayerBattleAction // actions performed at l-click
+{
+	INVALID = -1, CREATURE_INFO,
+	MOVE_TACTICS, CHOOSE_TACTICS_STACK,
+	MOVE_STACK, ATTACK, WALK_AND_ATTACK, ATTACK_AND_RETURN, SHOOT, //OPEN_GATE, //we can open castle gate during siege
+	NO_LOCATION, ANY_LOCATION, OBSTACLE, TELEPORT, SACRIFICE, RANDOM_GENIE_SPELL,
+	FREE_LOCATION, //used with Force Field and Fire Wall - all tiles affected by spell must be free
+	CATAPULT, HEAL, RISE_DEMONS,
+	AIMED_SPELL_CREATURE
+};
+
+struct DLL_LINKAGE BattleClientInterfaceData
+{
+	si32 creatureSpellToCast;
+	ui8 tacticsMode;
+};
+
 class DLL_LINKAGE CBattleInfoCallback : public virtual CBattleInfoEssentials
 {
 public:
@@ -99,6 +116,8 @@ public:
 	SpellID getRandomCastedSpell(CRandomGenerator & rand, const CStack * caster) const; //called at the beginning of turn for Faerie Dragon
 
 	si8 battleCanTeleportTo(const battle::Unit * stack, BattleHex destHex, int telportLevel) const; //checks if teleportation of given stack to given position can take place
+	std::vector<PossiblePlayerBattleAction> getClientActionsForStack(const CStack * stack, const BattleClientInterfaceData & data);
+	PossiblePlayerBattleAction getCasterAction(const CSpell * spell, const spells::Caster * caster, spells::Mode mode) const;
 
 	//convenience methods using the ones above
 	bool isInTacticRange(BattleHex dest) const;