Mostly finished refactoring of BattleActionsController

TODO: test actions
TODO: test casters
TODO: fix random casters

client/battle/BattleActionsController.h

@@ -14,6 +14,9 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 class BattleAction;
+namespace spells {
+class Caster;
+}
 
 VCMI_LIB_NAMESPACE_END
 
@@ -34,65 +37,76 @@ class BattleActionsController
 	/// all actions possible to call at the moment by player
 	std::vector<PossiblePlayerBattleAction> possibleActions;
 
-	/// actions possible to take on hovered hex
-	std::vector<PossiblePlayerBattleAction> localActions;
+	/// spell for which player's hero is choosing destination
+	std::shared_ptr<BattleAction> heroSpellToCast;
 
-	/// these actions display message in case of illegal target
-	std::vector<PossiblePlayerBattleAction> illegalActions;
+	/// cached message that was set by this class in status bar
+	std::string currentConsoleMsg;
 
-	/// action that will be performed on l-click
-	PossiblePlayerBattleAction currentAction;
+	/// if true, active stack could possibly cast some target spell
+	const CSpell * creatureSpellToCast;
 
-	/// last action chosen (and saved) by player
-	PossiblePlayerBattleAction selectedAction;
+	bool isCastingPossibleHere (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);
 
-	/// if there are not possible actions to choose from, this action should be show as "illegal" in UI
-	PossiblePlayerBattleAction illegalAction;
+	bool actionIsLegal(PossiblePlayerBattleAction action, BattleHex hoveredHex);
 
-	/// if true, stack currently aims to cats a spell
-	bool creatureCasting;
+	void actionSetCursor(PossiblePlayerBattleAction action, BattleHex hoveredHex);
+	void actionSetCursorBlocked(PossiblePlayerBattleAction action, BattleHex hoveredHex);
 
-	/// if true, player is choosing destination for his spell - only for GUI / console
-	bool spellDestSelectMode;
+	std::string actionGetStatusMessage(PossiblePlayerBattleAction action, BattleHex hoveredHex);
+	std::string actionGetStatusMessageBlocked(PossiblePlayerBattleAction action, BattleHex hoveredHex);
 
-	/// spell for which player is choosing destination
-	std::shared_ptr<BattleAction> spellToCast;
+	void actionRealize(PossiblePlayerBattleAction action, BattleHex hoveredHex);
 
-	/// spell for which player is choosing destination, pointer for convenience
-	const CSpell *currentSpell;
+	PossiblePlayerBattleAction selectAction(BattleHex myNumber);
 
-	/// cached message that was set by this class in status bar
-	std::string currentConsoleMsg;
+	const CStack * getStackForHex(BattleHex myNumber);
 
-	bool isCastingPossibleHere (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);
+	/// attempts to initialize spellcasting action for stack
+	/// will silently return if stack is not a spellcaster
+	void tryActivateStackSpellcasting(const CStack *casterStack);
 
+	const spells::Caster * getCurrentSpellcaster() const;
 public:
 	BattleActionsController(BattleInterface & owner);
 
 	/// initialize list of potential actions for new active stack
 	void activateStack();
 
-	/// initialize potential actions for spells that can be cast by active stack
+	/// returns true if UI is currently in target selection mode
+	bool spellcastingModeActive() const;
+
+	/// enter targeted spellcasting mode for creature, e.g. via "F" hotkey
 	void enterCreatureCastingMode();
 
-	/// initialize potential actions for selected spell
+	/// initialize hero spellcasting mode, e.g. on selecting spell in spellbook
 	void castThisSpell(SpellID spellID);
 
 	/// ends casting spell (eg. when spell has been cast or canceled)
 	void endCastingSpell();
 
-	/// update UI (e.g. status bar/cursor) according to new active hex
-	void handleHex(BattleHex myNumber, int eventType);
+	/// update cursor and status bar according to new active hex
+	void onHexHovered(BattleHex hoveredHex);
 
-	/// returns currently selected spell or SpellID::NONE on error
-	SpellID selectedSpell() const;
+	/// performs action according to selected hex
+	void onHexClicked(BattleHex clickedHex);
+
+	/// returns spell that is currently being cast by hero or nullptr if none
+	const CSpell * getHeroSpellToCast() const;
+
+	/// if current stack is spellcaster, returns
+	const CSpell * getStackSpellToCast( BattleHex targetHex ) const;
+	const CSpell * getAnySpellToCast( BattleHex targetHex ) const;
+
+	/// returns true if current stack is a spellcaster
+	bool isActiveStackFixedSpellcaster() const;
+
+	/// returns true if current stack is random spellcaster (e.g. Genie)
+	bool isActiveStackRandomSpellcaster() const;
 
-	/// returns true if UI is currently in target selection mode
-	bool spellcastingModeActive() const;
-	
 	/// methods to work with array of possible actions, needed to control special creatures abilities
 	const std::vector<PossiblePlayerBattleAction> & getPossibleActions() const;
 	void removePossibleAction(PossiblePlayerBattleAction);

client/battle/BattleFieldController.cpp

@@ -109,7 +109,7 @@ void BattleFieldController::mouseMoved(const SDL_MouseMotionEvent &event)
 {
 	BattleHex selectedHex = getHoveredHex();
 
-	owner.actionsController->handleHex(selectedHex, MOVE);
+	owner.actionsController->onHexHovered(selectedHex);
 }
 
 
@@ -237,12 +237,12 @@ std::set<BattleHex> BattleFieldController::getHighlightedHexesSpellRange()
 
 	if(owner.actionsController->spellcastingModeActive())//hero casts spell
 	{
-		spell = owner.actionsController->selectedSpell().toSpell();
+		spell = owner.actionsController->getHeroSpellToCast();
 		caster = owner.getActiveHero();
 	}
-	else if(owner.stacksController->activeStackSpellToCast() != SpellID::NONE)//stack casts spell
+	else if(owner.actionsController->getStackSpellToCast(hoveredHex) != nullptr)//stack casts spell
 	{
-		spell = SpellID(owner.stacksController->activeStackSpellToCast()).toSpell();
+		spell = owner.actionsController->getStackSpellToCast(hoveredHex);
 		caster = owner.stacksController->getActiveStack();
 		mode = spells::Mode::CREATURE_ACTIVE;
 	}
@@ -520,7 +520,7 @@ BattleHex BattleFieldController::fromWhichHexAttack(BattleHex attackTarget)
 		}
 		default:
 			assert(0);
-			return attackTarget.cloneInDirection(BattleHex::LEFT);
+			return BattleHex::INVALID;
 		}
 	}
 }

client/battle/BattleStacksController.cpp

@@ -73,8 +73,6 @@ BattleStacksController::BattleStacksController(BattleInterface & owner):
 	activeStack(nullptr),
 	stackToActivate(nullptr),
 	selectedStack(nullptr),
-	stackCanCastSpell(false),
-	creatureSpellToCast(uint32_t(-1)),
 	animIDhelper(0)
 {
 	//preparing graphics for displaying amounts of creatures
@@ -738,25 +736,6 @@ void BattleStacksController::activateStack()
 	const CStack * s = getActiveStack();
 	if(!s)
 		return;
-
-	//set casting flag to true if creature can use it to not check it every time
-	const auto spellcaster = s->getBonusLocalFirst(Selector::type()(Bonus::SPELLCASTER));
-	const auto randomSpellcaster = s->getBonusLocalFirst(Selector::type()(Bonus::RANDOM_SPELLCASTER));
-	if(s->canCast() && (spellcaster || randomSpellcaster))
-	{
-		stackCanCastSpell = true;
-		if(randomSpellcaster)
-			creatureSpellToCast = -1; //spell will be set later on cast
-		else
-			creatureSpellToCast = owner.curInt->cb->battleGetRandomStackSpell(CRandomGenerator::getDefault(), s, CBattleInfoCallback::RANDOM_AIMED); //faerie dragon can cast only one spell until their next move
-		//TODO: what if creature can cast BOTH random genie spell and aimed spell?
-		//TODO: faerie dragon type spell should be selected by server
-	}
-	else
-	{
-		stackCanCastSpell = false;
-		creatureSpellToCast = -1;
-	}
 }
 
 void BattleStacksController::setSelectedStack(const CStack *stack)
@@ -779,18 +758,6 @@ bool BattleStacksController::facingRight(const CStack * stack) const
 	return stackFacingRight.at(stack->ID);
 }
 
-bool BattleStacksController::activeStackSpellcaster()
-{
-	return stackCanCastSpell;
-}
-
-SpellID BattleStacksController::activeStackSpellToCast()
-{
-	if (!stackCanCastSpell)
-		return SpellID::NONE;
-	return SpellID(creatureSpellToCast);
-}
-
 Point BattleStacksController::getStackPositionAtHex(BattleHex hexNum, const CStack * stack) const
 {
 	Point ret(-500, -500); //returned value
@@ -908,12 +875,12 @@ std::vector<const CStack *> BattleStacksController::selectHoveredStacks()
 
 	if(owner.actionsController->spellcastingModeActive())//hero casts spell
 	{
-		spell = owner.actionsController->selectedSpell().toSpell();
+		spell = owner.actionsController->getHeroSpellToCast();
 		caster = owner.getActiveHero();
 	}
-	else if(owner.stacksController->activeStackSpellToCast() != SpellID::NONE)//stack casts spell
+	else if(owner.actionsController->getStackSpellToCast(hoveredHex) != nullptr)//stack casts spell
 	{
-		spell = SpellID(owner.stacksController->activeStackSpellToCast()).toSpell();
+		spell = owner.actionsController->getStackSpellToCast(hoveredHex);
 		caster = owner.stacksController->getActiveStack();
 		mode = spells::Mode::CREATURE_ACTIVE;
 	}

client/battle/BattleStacksController.h

@@ -79,10 +79,6 @@ class BattleStacksController
 	/// stack that was selected for multi-target spells - Teleport / Sacrifice
 	const CStack *selectedStack;
 
-	/// if true, active stack could possibly cast some target spell
-	bool stackCanCastSpell;
-	si32 creatureSpellToCast;
-
 	/// for giving IDs for animations
 	ui32 animIDhelper;
 
@@ -123,9 +119,6 @@ public:
 	void startAction(const BattleAction* action);
 	void endAction(const BattleAction* action);
 
-	bool activeStackSpellcaster();
-	SpellID activeStackSpellToCast();
-
 	void activateStack(); //sets activeStack to stackToActivate etc. //FIXME: No, it's not clear at all
 
 	void setActiveStack(const CStack *stack);