Merge pull request #1764 from IvanSavenko/selectable_spellcaster

Implemented support for multi-spell casters

client/battle/BattleActionsController.cpp

@@ -113,8 +113,7 @@ static std::string formatRangedAttack(const DamageEstimation & estimation, const
 
 BattleActionsController::BattleActionsController(BattleInterface & owner):
 	owner(owner),
-	heroSpellToCast(nullptr),
-	creatureSpellToCast(nullptr)
+	heroSpellToCast(nullptr)
 {}
 
 void BattleActionsController::endCastingSpell()
@@ -134,8 +133,8 @@ bool BattleActionsController::isActiveStackSpellcaster() const
 	if (!casterStack)
 		return false;
 
-	const auto randomSpellcaster = casterStack->getBonusLocalFirst(Selector::type()(Bonus::SPELLCASTER));
-	return (randomSpellcaster && casterStack->canCast());
+	bool spellcaster = casterStack->hasBonusOfType(Bonus::SPELLCASTER);
+	return (spellcaster && casterStack->canCast());
 }
 
 void BattleActionsController::enterCreatureCastingMode()
@@ -154,10 +153,13 @@ void BattleActionsController::enterCreatureCastingMode()
 	if (!isActiveStackSpellcaster())
 		return;
 
-	if (vstd::contains(possibleActions, PossiblePlayerBattleAction::NO_LOCATION))
+	for (auto const & action : possibleActions)
 	{
+		if (action.get() != PossiblePlayerBattleAction::NO_LOCATION)
+			continue;
+
 		const spells::Caster * caster = owner.stacksController->getActiveStack();
-		const CSpell * spell = getStackSpellToCast();
+		const CSpell * spell = action.spell().toSpell();
 
 		spells::Target target;
 		target.emplace_back();
@@ -176,31 +178,26 @@ void BattleActionsController::enterCreatureCastingMode()
 
 			CCS->curh->set(Cursor::Combat::POINTER);
 		}
+		return;
 	}
-	else
-	{
-		possibleActions = getPossibleActionsForStack(owner.stacksController->getActiveStack());
 
-		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);
-		};
+	possibleActions = getPossibleActionsForStack(owner.stacksController->getActiveStack());
 
-		vstd::erase_if(possibleActions, actionFilterPredicate);
-		GH.fakeMouseMove();
-	}
+	auto actionFilterPredicate = [](const PossiblePlayerBattleAction x)
+	{
+		return !x.spellcast();
+	};
+
+	vstd::erase_if(possibleActions, actionFilterPredicate);
+	GH.fakeMouseMove();
 }
 
 std::vector<PossiblePlayerBattleAction> BattleActionsController::getPossibleActionsForStack(const CStack *stack) const
 {
 	BattleClientInterfaceData data; //hard to get rid of these things so for now they're required data to pass
 
-	if (getStackSpellToCast())
-		data.creatureSpellToCast = getStackSpellToCast()->getId();
-	else
-		data.creatureSpellToCast = SpellID::NONE;
+	for (auto const & spell : creatureSpells)
+		data.creatureSpellsToCast.push_back(spell->id);
 
 	data.tacticsMode = owner.tacticsMode;
 	auto allActions = owner.curInt->cb->getClientActionsForStack(stack, data);
@@ -217,7 +214,7 @@ void BattleActionsController::reorderPossibleActionsPriority(const CStack * stac
 
 	auto assignPriority = [&](PossiblePlayerBattleAction const & item) -> uint8_t //large lambda assigning priority which would have to be part of possibleActions without it
 	{
-		switch(item)
+		switch(item.get())
 		{
 		case PossiblePlayerBattleAction::AIMED_SPELL_CREATURE:
 		case PossiblePlayerBattleAction::ANY_LOCATION:
@@ -278,7 +275,7 @@ void BattleActionsController::castThisSpell(SpellID spellID)
 	assert(castingHero); // code below assumes non-null hero
 	PossiblePlayerBattleAction spellSelMode = owner.curInt->cb->getCasterAction(spellID.toSpell(), castingHero, spells::Mode::HERO);
 
-	if (spellSelMode == PossiblePlayerBattleAction::NO_LOCATION) //user does not have to select location
+	if (spellSelMode.get() == PossiblePlayerBattleAction::NO_LOCATION) //user does not have to select location
 	{
 		heroSpellToCast->aimToHex(BattleHex::INVALID);
 		owner.curInt->cb->battleMakeAction(heroSpellToCast.get());
@@ -299,19 +296,30 @@ const CSpell * BattleActionsController::getHeroSpellToCast( ) const
 	return nullptr;
 }
 
-const CSpell * BattleActionsController::getStackSpellToCast( ) const
+const CSpell * BattleActionsController::getStackSpellToCast(BattleHex hoveredHex)
 {
-	if (isActiveStackSpellcaster())
-		return creatureSpellToCast;
+	if (heroSpellToCast)
+		return nullptr;
 
-	return nullptr;
+	if (!owner.stacksController->getActiveStack())
+		return nullptr;
+
+	if (!hoveredHex.isValid())
+		return nullptr;
+
+	auto action = selectAction(hoveredHex);
+
+	if (action.spell() == SpellID::NONE)
+		return nullptr;
+
+	return action.spell().toSpell();
 }
 
-const CSpell * BattleActionsController::getCurrentSpell( ) const
+const CSpell * BattleActionsController::getCurrentSpell(BattleHex hoveredHex)
 {
 	if (getHeroSpellToCast())
 		return getHeroSpellToCast();
-	return getStackSpellToCast();
+	return getStackSpellToCast(hoveredHex);
 }
 
 const CStack * BattleActionsController::getStackForHex(BattleHex hoveredHex)
@@ -324,7 +332,7 @@ const CStack * BattleActionsController::getStackForHex(BattleHex hoveredHex)
 
 void BattleActionsController::actionSetCursor(PossiblePlayerBattleAction action, BattleHex targetHex)
 {
-	switch (action)
+	switch (action.get())
 	{
 		case PossiblePlayerBattleAction::CHOOSE_TACTICS_STACK:
 			CCS->curh->set(Cursor::Combat::POINTER);
@@ -387,7 +395,7 @@ void BattleActionsController::actionSetCursor(PossiblePlayerBattleAction action,
 
 void BattleActionsController::actionSetCursorBlocked(PossiblePlayerBattleAction action, BattleHex targetHex)
 {
-	switch (action)
+	switch (action.get())
 	{
 		case PossiblePlayerBattleAction::AIMED_SPELL_CREATURE:
 		case PossiblePlayerBattleAction::RANDOM_GENIE_SPELL:
@@ -410,7 +418,7 @@ std::string BattleActionsController::actionGetStatusMessage(PossiblePlayerBattle
 {
 	const CStack * targetStack = getStackForHex(targetHex);
 
-	switch (action) //display console message, realize selected action
+	switch (action.get()) //display console message, realize selected action
 	{
 		case PossiblePlayerBattleAction::CHOOSE_TACTICS_STACK:
 			return (boost::format(CGI->generaltexth->allTexts[481]) % targetStack->getName()).str(); //Select %s
@@ -441,10 +449,10 @@ std::string BattleActionsController::actionGetStatusMessage(PossiblePlayerBattle
 		}
 
 		case PossiblePlayerBattleAction::AIMED_SPELL_CREATURE:
-			return boost::str(boost::format(CGI->generaltexth->allTexts[27]) % getCurrentSpell()->getNameTranslated() % targetStack->getName()); //Cast %s on %s
+			return boost::str(boost::format(CGI->generaltexth->allTexts[27]) % action.spell().toSpell()->getNameTranslated() % targetStack->getName()); //Cast %s on %s
 
 		case PossiblePlayerBattleAction::ANY_LOCATION:
-			return boost::str(boost::format(CGI->generaltexth->allTexts[26]) % getCurrentSpell()->getNameTranslated()); //Cast %s
+			return boost::str(boost::format(CGI->generaltexth->allTexts[26]) % action.spell().toSpell()->getNameTranslated()); //Cast %s
 
 		case PossiblePlayerBattleAction::RANDOM_GENIE_SPELL: //we assume that teleport / sacrifice will never be available as random spell
 			return boost::str(boost::format(CGI->generaltexth->allTexts[301]) % targetStack->getName()); //Cast a spell on %
@@ -459,7 +467,7 @@ std::string BattleActionsController::actionGetStatusMessage(PossiblePlayerBattle
 			return (boost::format(CGI->generaltexth->allTexts[549]) % targetStack->getName()).str(); //sacrifice the %s
 
 		case PossiblePlayerBattleAction::FREE_LOCATION:
-			return boost::str(boost::format(CGI->generaltexth->allTexts[26]) % getCurrentSpell()->getNameTranslated()); //Cast %s
+			return boost::str(boost::format(CGI->generaltexth->allTexts[26]) % action.spell().toSpell()->getNameTranslated()); //Cast %s
 
 		case PossiblePlayerBattleAction::HEAL:
 			return (boost::format(CGI->generaltexth->allTexts[419]) % targetStack->getName()).str(); //Apply first aid to the %s
@@ -479,7 +487,7 @@ std::string BattleActionsController::actionGetStatusMessage(PossiblePlayerBattle
 
 std::string BattleActionsController::actionGetStatusMessageBlocked(PossiblePlayerBattleAction action, BattleHex targetHex)
 {
-	switch (action)
+	switch (action.get())
 	{
 		case PossiblePlayerBattleAction::AIMED_SPELL_CREATURE:
 		case PossiblePlayerBattleAction::RANDOM_GENIE_SPELL:
@@ -492,7 +500,7 @@ std::string BattleActionsController::actionGetStatusMessageBlocked(PossiblePlaye
 			return CGI->generaltexth->allTexts[543]; //choose army to sacrifice
 			break;
 		case PossiblePlayerBattleAction::FREE_LOCATION:
-			return boost::str(boost::format(CGI->generaltexth->allTexts[181]) % getCurrentSpell()->getNameTranslated()); //No room to place %s here
+			return boost::str(boost::format(CGI->generaltexth->allTexts[181]) % action.spell().toSpell()->getNameTranslated()); //No room to place %s here
 			break;
 		default:
 			return "";
@@ -504,7 +512,7 @@ bool BattleActionsController::actionIsLegal(PossiblePlayerBattleAction action, B
 	const CStack * targetStack = getStackForHex(targetHex);
 	bool targetStackOwned = targetStack && targetStack->owner == owner.curInt->playerID;
 
-	switch (action)
+	switch (action.get())
 	{
 		case PossiblePlayerBattleAction::CHOOSE_TACTICS_STACK:
 		case PossiblePlayerBattleAction::CREATURE_INFO:
@@ -541,11 +549,14 @@ bool BattleActionsController::actionIsLegal(PossiblePlayerBattleAction action, B
 		case PossiblePlayerBattleAction::SHOOT:
 			return owner.curInt->cb->battleCanShoot(owner.stacksController->getActiveStack(), targetHex);
 
+		case PossiblePlayerBattleAction::NO_LOCATION:
+			return false;
+
 		case PossiblePlayerBattleAction::ANY_LOCATION:
-			return isCastingPossibleHere(owner.stacksController->getActiveStack(), targetStack, targetHex);
+			return isCastingPossibleHere(action.spell().toSpell(), owner.stacksController->getActiveStack(), targetStack, targetHex);
 
 		case PossiblePlayerBattleAction::AIMED_SPELL_CREATURE:
-			return targetStack && isCastingPossibleHere(owner.stacksController->getActiveStack(), targetStack, targetHex);
+			return targetStack && isCastingPossibleHere(action.spell().toSpell(), owner.stacksController->getActiveStack(), targetStack, targetHex);
 
 		case PossiblePlayerBattleAction::RANDOM_GENIE_SPELL:
 			if(targetStack && targetStackOwned && targetStack != owner.stacksController->getActiveStack() && targetStack->alive()) //only positive spells for other allied creatures
@@ -566,8 +577,8 @@ bool BattleActionsController::actionIsLegal(PossiblePlayerBattleAction action, B
 
 		case PossiblePlayerBattleAction::OBSTACLE:
 		case PossiblePlayerBattleAction::FREE_LOCATION:
-			return isCastingPossibleHere(owner.stacksController->getActiveStack(), targetStack, targetHex);
-			return isCastingPossibleHere(owner.stacksController->getActiveStack(), targetStack, targetHex);
+			return isCastingPossibleHere(action.spell().toSpell(), owner.stacksController->getActiveStack(), targetStack, targetHex);
+			return isCastingPossibleHere(action.spell().toSpell(), owner.stacksController->getActiveStack(), targetStack, targetHex);
 
 		case PossiblePlayerBattleAction::CATAPULT:
 			return owner.siegeController && owner.siegeController->isAttackableByCatapult(targetHex);
@@ -584,7 +595,7 @@ void BattleActionsController::actionRealize(PossiblePlayerBattleAction action, B
 {
 	const CStack * targetStack = getStackForHex(targetHex);
 
-	switch (action) //display console message, realize selected action
+	switch (action.get()) //display console message, realize selected action
 	{
 		case PossiblePlayerBattleAction::CHOOSE_TACTICS_STACK:
 		{
@@ -615,7 +626,7 @@ void BattleActionsController::actionRealize(PossiblePlayerBattleAction action, B
 		case PossiblePlayerBattleAction::WALK_AND_ATTACK:
 		case PossiblePlayerBattleAction::ATTACK_AND_RETURN: //TODO: allow to disable return
 		{
-			bool returnAfterAttack = action == PossiblePlayerBattleAction::ATTACK_AND_RETURN;
+			bool returnAfterAttack = action.get() == PossiblePlayerBattleAction::ATTACK_AND_RETURN;
 			BattleHex attackFromHex = owner.fieldController->fromWhichHexAttack(targetHex);
 			if(attackFromHex.isValid()) //we can be in this line when unreachable creature is L - clicked (as of revision 1308)
 			{
@@ -668,16 +679,16 @@ void BattleActionsController::actionRealize(PossiblePlayerBattleAction action, B
 		case PossiblePlayerBattleAction::SACRIFICE:
 		case PossiblePlayerBattleAction::FREE_LOCATION:
 		{
-			if (action == PossiblePlayerBattleAction::AIMED_SPELL_CREATURE )
+			if (action.get() == PossiblePlayerBattleAction::AIMED_SPELL_CREATURE )
 			{
-				if (getCurrentSpell()->id == SpellID::SACRIFICE)
+				if (action.spell() == SpellID::SACRIFICE)
 				{
 					heroSpellToCast->aimToHex(targetHex);
 					possibleActions.push_back(PossiblePlayerBattleAction::SACRIFICE);
 					owner.stacksController->setSelectedStack(targetStack);
 					return;
 				}
-				if (getCurrentSpell()->id == SpellID::TELEPORT)
+				if (action.spell() == SpellID::TELEPORT)
 				{
 					heroSpellToCast->aimToUnit(targetStack);
 					possibleActions.push_back(PossiblePlayerBattleAction::TELEPORT);
@@ -688,9 +699,9 @@ void BattleActionsController::actionRealize(PossiblePlayerBattleAction action, B
 
 			if (!spellcastingModeActive())
 			{
-				if (getStackSpellToCast())
+				if (action.spell().toSpell())
 				{
-					owner.giveCommand(EActionType::MONSTER_SPELL, targetHex, getStackSpellToCast()->getId());
+					owner.giveCommand(EActionType::MONSTER_SPELL, targetHex, action.spell());
 				}
 				else //unknown random spell
 				{
@@ -819,12 +830,25 @@ void BattleActionsController::onHexLeftClicked(BattleHex clickedHex)
 
 void BattleActionsController::tryActivateStackSpellcasting(const CStack *casterStack)
 {
-	const auto spellcaster = casterStack->getBonusLocalFirst(Selector::type()(Bonus::SPELLCASTER));
+	creatureSpells.clear();
+
+	bool spellcaster = casterStack->hasBonusOfType(Bonus::SPELLCASTER);
 	if(casterStack->canCast() && spellcaster)
 	{
 		// faerie dragon can cast only one, randomly selected spell until their next move
 		//TODO: faerie dragon type spell should be selected by server
-		creatureSpellToCast = owner.curInt->cb->battleGetRandomStackSpell(CRandomGenerator::getDefault(), casterStack, CBattleInfoCallback::RANDOM_AIMED).toSpell();
+		const auto * spellToCast = owner.curInt->cb->battleGetRandomStackSpell(CRandomGenerator::getDefault(), casterStack, CBattleInfoCallback::RANDOM_AIMED).toSpell();
+
+		if (spellToCast)
+			creatureSpells.push_back(spellToCast);
+	}
+
+	TConstBonusListPtr bl = casterStack->getBonuses(Selector::type()(Bonus::SPELLCASTER));
+
+	for (auto const & bonus : *bl)
+	{
+		if (bonus->additionalInfo[0] <= 0)
+			creatureSpells.push_back(SpellID(bonus->subtype).toSpell());
 	}
 }
 
@@ -845,11 +869,9 @@ spells::Mode BattleActionsController::getCurrentCastMode() const
 
 }
 
-bool BattleActionsController::isCastingPossibleHere(const CStack *casterStack, const CStack *targetStack, BattleHex targetHex)
+bool BattleActionsController::isCastingPossibleHere(const CSpell * currentSpell, const CStack *casterStack, const CStack *targetStack, BattleHex targetHex)
 {
-	auto currentSpell = getCurrentSpell();
 	assert(currentSpell);
-
 	if (!currentSpell)
 		return false;
 
@@ -892,7 +914,7 @@ void BattleActionsController::activateStack()
 		std::list<PossiblePlayerBattleAction> actionsToSelect;
 		if(!possibleActions.empty())
 		{
-			switch(possibleActions.front())
+			switch(possibleActions.front().get())
 			{
 				case PossiblePlayerBattleAction::SHOOT:
 					actionsToSelect.push_back(possibleActions.front());
@@ -942,12 +964,7 @@ bool BattleActionsController::currentActionSpellcasting(BattleHex hoveredHex)
 
 	auto action = selectAction(hoveredHex);
 
-	return
-		action == PossiblePlayerBattleAction::ANY_LOCATION ||
-		action == PossiblePlayerBattleAction::NO_LOCATION ||
-		action == PossiblePlayerBattleAction::FREE_LOCATION ||
-		action == PossiblePlayerBattleAction::AIMED_SPELL_CREATURE ||
-		action == PossiblePlayerBattleAction::OBSTACLE;
+	return action.spellcast();
 }
 
 const std::vector<PossiblePlayerBattleAction> & BattleActionsController::getPossibleActions() const

client/battle/BattleActionsController.h

@@ -45,9 +45,9 @@ class BattleActionsController
 	std::string currentConsoleMsg;
 
 	/// if true, active stack could possibly cast some target spell
-	const CSpell * creatureSpellToCast;
+	std::vector<const CSpell *> creatureSpells;
 
-	bool isCastingPossibleHere (const CStack *sactive, const CStack *shere, BattleHex myNumber);
+	bool isCastingPossibleHere (const CSpell * spell, const CStack *sactive, const CStack *shere, BattleHex myNumber);
 	bool canStackMoveHere (const CStack *sactive, BattleHex MyNumber) const; //TODO: move to BattleState / callback
 	std::vector<PossiblePlayerBattleAction> getPossibleActionsForStack (const CStack *stack) const; //called when stack gets its turn
 	void reorderPossibleActionsPriority(const CStack * stack, MouseHoveredHexContext context);
@@ -74,7 +74,7 @@ class BattleActionsController
 	const CSpell * getHeroSpellToCast() const;
 
 	/// if current stack is spellcaster, returns spell being cast, or null othervice
-	const CSpell * getStackSpellToCast( ) const;
+	const CSpell * getStackSpellToCast(BattleHex hoveredHex);
 
 	/// returns true if current stack is a spellcaster
 	bool isActiveStackSpellcaster() const;
@@ -116,7 +116,7 @@ public:
 	void onHexRightClicked(BattleHex clickedHex);
 
 	const spells::Caster * getCurrentSpellcaster() const;
-	const CSpell * getCurrentSpell() const;
+	const CSpell * getCurrentSpell(BattleHex hoveredHex);
 	spells::Mode getCurrentCastMode() const;
 
 	/// methods to work with array of possible actions, needed to control special creatures abilities

client/battle/BattleFieldController.cpp

@@ -263,7 +263,7 @@ std::set<BattleHex> BattleFieldController::getHighlightedHexesSpellRange()
 	const CSpell *spell = nullptr;
 
 	spells::Mode mode = owner.actionsController->getCurrentCastMode();
-	spell = owner.actionsController->getCurrentSpell();
+	spell = owner.actionsController->getCurrentSpell(hoveredHex);
 	caster = owner.actionsController->getCurrentSpellcaster();
 
 	if(caster && spell) //when casting spell

client/battle/BattleStacksController.cpp

@@ -849,7 +849,7 @@ std::vector<const CStack *> BattleStacksController::selectHoveredStacks()
 	const CSpell *spell = nullptr;
 
 	spells::Mode mode = owner.actionsController->getCurrentCastMode();
-	spell = owner.actionsController->getCurrentSpell();
+	spell = owner.actionsController->getCurrentSpell(hoveredHex);
 	caster = owner.actionsController->getCurrentSpellcaster();
 
 	if(caster && spell && owner.actionsController->currentActionSpellcasting(hoveredHex) ) //when casting spell

client/battle/BattleWindow.cpp

@@ -38,7 +38,8 @@
 #include "../windows/settings/SettingsMainWindow.h"
 
 BattleWindow::BattleWindow(BattleInterface & owner):
-	owner(owner)
+	owner(owner),
+	defaultAction(PossiblePlayerBattleAction::INVALID)
 {
 	OBJ_CONSTRUCTION_CAPTURING_ALL_NO_DISPOSE;
 	pos.w = 800;
@@ -326,7 +327,7 @@ void BattleWindow::showAlternativeActionIcon(PossiblePlayerBattleAction action)
 		return;
 	
 	std::string iconName = variables["actionIconDefault"].String();
-	switch(action)
+	switch(action.get())
 	{
 		case PossiblePlayerBattleAction::ATTACK:
 			iconName = variables["actionIconAttack"].String();

client/battle/BattleWindow.h

@@ -12,6 +12,7 @@
 #include "../gui/CIntObject.h"
 #include "../gui/InterfaceObjectConfigurable.h"
 #include "../../lib/battle/CBattleInfoCallback.h"
+#include "../../lib/battle/PossiblePlayerBattleAction.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 class CStack;

cmake_modules/VCMI_lib.cmake

@@ -258,6 +258,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/battle/IBattleInfoCallback.h
 		${MAIN_LIB_DIR}/battle/IBattleState.h
 		${MAIN_LIB_DIR}/battle/IUnitInfo.h
+		${MAIN_LIB_DIR}/battle/PossiblePlayerBattleAction.h
 		${MAIN_LIB_DIR}/battle/ReachabilityInfo.h
 		${MAIN_LIB_DIR}/battle/SideInBattle.h
 		${MAIN_LIB_DIR}/battle/SiegeInfo.h

lib/battle/CBattleInfoCallback.cpp

@@ -15,6 +15,7 @@
 #include "../CStack.h"
 #include "BattleInfo.h"
 #include "DamageCalculator.h"
+#include "PossiblePlayerBattleAction.h"
 #include "../NetPacks.h"
 #include "../spells/CSpellHandler.h"
 #include "../mapObjects/CGTownInstance.h"
@@ -218,11 +219,14 @@ std::vector<PossiblePlayerBattleAction> CBattleInfoCallback::getClientActionsFor
 	{
 		if(stack->canCast()) //TODO: check for battlefield effects that prevent casting?
 		{
-			if(stack->hasBonusOfType(Bonus::SPELLCASTER) && data.creatureSpellToCast != -1)
+			if(stack->hasBonusOfType(Bonus::SPELLCASTER))
 			{
-				const CSpell *spell = SpellID(data.creatureSpellToCast).toSpell();
-				PossiblePlayerBattleAction act = getCasterAction(spell, stack, spells::Mode::CREATURE_ACTIVE);
-				allowedActionList.push_back(act);
+				for (auto const & spellID : data.creatureSpellsToCast)
+				{
+					const CSpell *spell = spellID.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);
@@ -251,7 +255,7 @@ std::vector<PossiblePlayerBattleAction> CBattleInfoCallback::getClientActionsFor
 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;
+	auto spellSelMode = PossiblePlayerBattleAction::ANY_LOCATION;
 
 	const CSpell::TargetInfo ti(spell, caster->getSpellSchoolLevel(spell), mode);
 
@@ -264,7 +268,7 @@ PossiblePlayerBattleAction CBattleInfoCallback::getCasterAction(const CSpell * s
 	else if(ti.type == spells::AimType::OBSTACLE)
 		spellSelMode = PossiblePlayerBattleAction::OBSTACLE;
 
-	return spellSelMode;
+	return PossiblePlayerBattleAction(spellSelMode, spell->id);
 }
 
 std::set<BattleHex> CBattleInfoCallback::battleGetAttackedHexes(const CStack* attacker, BattleHex destinationTile, BattleHex attackerPos) const
@@ -1653,12 +1657,16 @@ SpellID CBattleInfoCallback::getRandomCastedSpell(CRandomGenerator & rand,const
 	int totalWeight = 0;
 	for(const auto & b : *bl)
 	{
-		totalWeight += std::max(b->additionalInfo[0], 1); //minimal chance to cast is 1
+		totalWeight += std::max(b->additionalInfo[0], 0); //spells with 0 weight are non-random, exclude them
 	}
+
+	if (totalWeight == 0)
+		return SpellID::NONE;
+
 	int randomPos = rand.nextInt(totalWeight - 1);
 	for(const auto & b : *bl)
 	{
-		randomPos -= std::max(b->additionalInfo[0], 1);
+		randomPos -= std::max(b->additionalInfo[0], 0);
 		if(randomPos < 0)
 		{
 			return SpellID(b->subtype);

lib/battle/CBattleInfoCallback.h

@@ -24,6 +24,7 @@ class CSpell;
 struct CObstacleInstance;
 class IBonusBearer;
 class CRandomGenerator;
+class PossiblePlayerBattleAction;
 
 namespace spells
 {
@@ -42,35 +43,9 @@ struct DLL_LINKAGE AttackableTiles
 	}
 };
 
-enum class PossiblePlayerBattleAction // actions performed at l-click
-{
-	INVALID = -1,
-	CREATURE_INFO,
-	HERO_INFO,
-	MOVE_TACTICS,
-	CHOOSE_TACTICS_STACK,
-
-	MOVE_STACK,
-	ATTACK,
-	WALK_AND_ATTACK,
-	ATTACK_AND_RETURN,
-	SHOOT,
-	CATAPULT,
-	HEAL,
-
-	NO_LOCATION,          // massive spells that affect every possible target, automatic casts
-	ANY_LOCATION,
-	OBSTACLE,
-	TELEPORT,
-	SACRIFICE,
-	RANDOM_GENIE_SPELL,   // random spell on a friendly creature
-	FREE_LOCATION,        // used with Force Field and Fire Wall - all tiles affected by spell must be free
-	AIMED_SPELL_CREATURE, // spell targeted at creature
-};
-
 struct DLL_LINKAGE BattleClientInterfaceData
 {
-	si32 creatureSpellToCast;
+	std::vector<SpellID> creatureSpellsToCast;
 	ui8 tacticsMode;
 };
 

lib/battle/PossiblePlayerBattleAction.h

@@ -0,0 +1,78 @@
+/*
+ * CBattleInfoCallback.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#include "../GameConstants.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+class PossiblePlayerBattleAction // actions performed at l-click
+{
+public:
+	enum Actions {
+		INVALID = -1,
+		CREATURE_INFO,
+		HERO_INFO,
+		MOVE_TACTICS,
+		CHOOSE_TACTICS_STACK,
+
+		MOVE_STACK,
+		ATTACK,
+		WALK_AND_ATTACK,
+		ATTACK_AND_RETURN,
+		SHOOT,
+		CATAPULT,
+		HEAL,
+
+		RANDOM_GENIE_SPELL,   // random spell on a friendly creature
+
+		NO_LOCATION,          // massive spells that affect every possible target, automatic casts
+		ANY_LOCATION,
+		OBSTACLE,
+		TELEPORT,
+		SACRIFICE,
+		FREE_LOCATION,        // used with Force Field and Fire Wall - all tiles affected by spell must be free
+		AIMED_SPELL_CREATURE, // spell targeted at creature
+	};
+
+private:
+	Actions action;
+	SpellID spellToCast;
+
+public:
+	bool spellcast() const
+	{
+		return action == ANY_LOCATION || action == NO_LOCATION || action == OBSTACLE || action == TELEPORT ||
+			   action == SACRIFICE || action == FREE_LOCATION || action == AIMED_SPELL_CREATURE;
+	}
+
+	Actions get() const
+	{
+		return action;
+	}
+
+	SpellID spell() const
+	{
+		return spellToCast;
+	}
+
+	PossiblePlayerBattleAction(Actions action, SpellID spellToCast = SpellID::NONE):
+		action(static_cast<Actions>(action)),
+		spellToCast(spellToCast)
+	{
+		assert((spellToCast != SpellID::NONE) == spellcast());
+	}
+
+	bool operator == (const PossiblePlayerBattleAction & other) const
+	{
+		return action == other.action && spellToCast == other.spellToCast;
+	}
+};
+
+VCMI_LIB_NAMESPACE_END

server/CGameHandler.cpp

@@ -4910,8 +4910,8 @@ bool CGameHandler::makeBattleAction(BattleAction &ba)
 			const CStack * stack = gs->curB->battleGetStackByID(ba.stackNumber);
 			SpellID spellID = SpellID(ba.actionSubtype);
 
-			std::shared_ptr<const Bonus> randSpellcaster = stack->getBonusLocalFirst(Selector::type()(Bonus::RANDOM_SPELLCASTER));
-			std::shared_ptr<const Bonus> spellcaster = stack->getBonusLocalFirst(Selector::typeSubtype(Bonus::SPELLCASTER, spellID));
+			std::shared_ptr<const Bonus> randSpellcaster = stack->getBonus(Selector::type()(Bonus::RANDOM_SPELLCASTER));
+			std::shared_ptr<const Bonus> spellcaster = stack->getBonus(Selector::typeSubtype(Bonus::SPELLCASTER, spellID));
 
 			//TODO special bonus for genies ability
 			if (randSpellcaster && battleGetRandomStackSpell(getRandomGenerator(), stack, CBattleInfoCallback::RANDOM_AIMED) < 0)