Moved all animation ordering logic to callers

Previously, CBattleAnimation & inheritors were controlling animation
ordering - e.g. which animations should play after which.
Now, this is controlled by caller, e.g. BattleInterface & its
controllers.
H3 animations are fairly linear and can be split in stages which are
already somewhat implemented via waitForAnims

AI/BattleAI/BattleAI.h

@@ -80,7 +80,7 @@ public:
 	//void actionFinished(const BattleAction &action) override;//occurs AFTER every action taken by any stack or by the hero
 	//void actionStarted(const BattleAction &action) override;//occurs BEFORE every action taken by any stack or by the hero
 	//void battleAttack(const BattleAttack *ba) override; //called when stack is performing attack
-	//void battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa) override; //called when stack receives damage (after battleAttack())
+	//void battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa, bool ranged) override; //called when stack receives damage (after battleAttack())
 	//void battleEnd(const BattleResult *br) override;
 	//void battleResultsApplied() override; //called when all effects of last battle are applied
 	//void battleNewRoundFirst(int round) override; //called at the beginning of each turn before changes are applied;

AI/StupidAI/StupidAI.cpp

@@ -177,7 +177,7 @@ void CStupidAI::battleAttack(const BattleAttack *ba)
 	print("battleAttack called");
 }
 
-void CStupidAI::battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa)
+void CStupidAI::battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa, bool ranged)
 {
 	print("battleStacksAttacked called");
 }

AI/StupidAI/StupidAI.h

@@ -31,7 +31,7 @@ public:
 	BattleAction activeStack(const CStack * stack) override; //called when it's turn of that stack
 
 	void battleAttack(const BattleAttack *ba) override; //called when stack is performing attack
-	void battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa) override; //called when stack receives damage (after battleAttack())
+	void battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa, bool ranged) override; //called when stack receives damage (after battleAttack())
 	void battleEnd(const BattleResult *br) override;
 	//void battleResultsApplied() override; //called when all effects of last battle are applied
 	void battleNewRoundFirst(int round) override; //called at the beginning of each turn before changes are applied;

client/CPlayerInterface.cpp

@@ -944,7 +944,7 @@ void CPlayerInterface::battleTriggerEffect (const BattleTriggerEffect & bte)
 	RETURN_IF_QUICK_COMBAT;
 	battleInt->effectsController->battleTriggerEffect(bte);
 }
-void CPlayerInterface::battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa)
+void CPlayerInterface::battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa, bool ranged)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
 	BATTLE_EVENT_POSSIBLE_RETURN;
@@ -954,24 +954,28 @@ void CPlayerInterface::battleStacksAttacked(const std::vector<BattleStackAttacke
 	{
 		const CStack * defender = cb->battleGetStackByID(elem.stackAttacked, false);
 		const CStack * attacker = cb->battleGetStackByID(elem.attackerID, false);
-		if(elem.isEffect())
-		{
-			if(defender && !elem.isSecondary())
-				battleInt->effectsController->displayEffect(EBattleEffect::EBattleEffect(elem.effect), defender->getPosition());
-		}
-		if(elem.isSpell())
-		{
-			if(defender)
-				battleInt->displaySpellEffect(elem.spellID, defender->getPosition());
-		}
-		//FIXME: why action is deleted during enchanter cast?
-		bool remoteAttack = false;
 
-		if(LOCPLINT->curAction)
-			remoteAttack |= LOCPLINT->curAction->actionType != EActionType::WALK_AND_ATTACK;
+		assert(defender);
 
-		StackAttackedInfo to_put = {defender, elem.damageAmount, elem.killedAmount, attacker, remoteAttack, elem.killed(), elem.willRebirth(), elem.cloneKilled()};
-		arg.push_back(to_put);
+		StackAttackedInfo     info;
+		info.defender       = defender;
+		info.attacker       = attacker;
+		info.damageDealt    = elem.damageAmount;
+		info.amountKilled   = elem.killedAmount;
+		info.battleEffect   = EBattleEffect::INVALID;
+		info.spellEffect    = SpellID::NONE;
+		info.indirectAttack = ranged;
+		info.killed         = elem.killed();
+		info.rebirth        = elem.willRebirth();
+		info.cloneKilled    = elem.cloneKilled();
+
+		if(elem.isEffect() && !elem.isSecondary())
+			info.battleEffect = EBattleEffect::EBattleEffect(elem.effect);
+
+		if (elem.isSpell())
+			info.spellEffect = elem.spellID;
+
+		arg.push_back(info);
 	}
 	battleInt->stacksAreAttacked(arg);
 }

client/CPlayerInterface.h

@@ -197,7 +197,7 @@ public:
 	void battleSpellCast(const BattleSpellCast *sc) override;
 	void battleStacksEffectsSet(const SetStackEffect & sse) override; //called when a specific effect is set to stacks
 	void battleTriggerEffect(const BattleTriggerEffect & bte) override; //various one-shot effect
-	void battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa) override;
+	void battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa, bool ranged) override;
 	void battleStartBefore(const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2) override; //called by engine just before battle starts; side=0 - left, side=1 - right
 	void battleStart(const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool side) override; //called by engine when battle starts; side=0 - left, side=1 - right
 	void battleUnitsChanged(const std::vector<UnitChanges> & units, const std::vector<CustomEffectInfo> & customEffects) override;

client/NetPacksClient.cpp

@@ -748,7 +748,7 @@ void BattleAttack::applyFirstCl(CClient *cl)
 
 void BattleAttack::applyCl(CClient *cl)
 {
-	callBattleInterfaceIfPresentForBothSides(cl, &IBattleEventsReceiver::battleStacksAttacked, bsa);
+	callBattleInterfaceIfPresentForBothSides(cl, &IBattleEventsReceiver::battleStacksAttacked, bsa, shot());
 }
 
 void StartAction::applyFirstCl(CClient *cl)
@@ -770,7 +770,7 @@ void SetStackEffect::applyCl(CClient *cl)
 
 void StacksInjured::applyCl(CClient *cl)
 {
-	callBattleInterfaceIfPresentForBothSides(cl, &IBattleEventsReceiver::battleStacksAttacked, stacks);
+	callBattleInterfaceIfPresentForBothSides(cl, &IBattleEventsReceiver::battleStacksAttacked, stacks, false);
 }
 
 void BattleResultsApplied::applyCl(CClient *cl)

client/battle/BattleAnimationClasses.cpp

@@ -90,32 +90,6 @@ void CBattleAnimation::setStackFacingRight(const CStack * stack, bool facingRigh
 	owner.stacksController->stackFacingRight[stack->ID] = facingRight;
 }
 
-bool CBattleAnimation::checkInitialConditions()
-{
-	int lowestMoveID = ID;
-	auto * thAnim = dynamic_cast<CBattleStackAnimation *>(this);
-	auto * thSen = dynamic_cast<CPointEffectAnimation *>(this);
-
-	for(auto & elem : pendingAnimations())
-	{
-		auto * sen = dynamic_cast<CPointEffectAnimation *>(elem);
-
-		// all effect animations can play concurrently with each other
-		if(sen && thSen && sen != thSen)
-			continue;
-
-		auto * revAnim = dynamic_cast<CReverseAnimation *>(elem);
-
-		// if there is high-priority reverse animation affecting our stack then this animation will wait
-		if(revAnim && thAnim && revAnim && revAnim->stack->ID == thAnim->stack->ID && revAnim->priority)
-			return false;
-
-		if(elem)
-			vstd::amin(lowestMoveID, elem->ID);
-	}
-	return ID == lowestMoveID;
-}
-
 CBattleStackAnimation::CBattleStackAnimation(BattleInterface & owner, const CStack * stack)
 	: CBattleAnimation(owner),
 	  myAnim(stackAnimation(stack)),
@@ -153,22 +127,6 @@ CAttackAnimation::~CAttackAnimation()
 	myAnim->setType(ECreatureAnimType::HOLDING);
 }
 
-bool CAttackAnimation::checkInitialConditions()
-{
-	for(auto & elem : pendingAnimations())
-	{
-		CBattleStackAnimation * stAnim = dynamic_cast<CBattleStackAnimation *>(elem);
-		CReverseAnimation * revAnim = dynamic_cast<CReverseAnimation *>(stAnim);
-
-		if(revAnim && attackedStack) // enemy must be fully reversed
-		{
-			if (revAnim->stack->ID == attackedStack->ID)
-				return false;
-		}
-	}
-	return CBattleAnimation::checkInitialConditions();
-}
-
 const CCreature * CAttackAnimation::getCreature() const
 {
 	if (attackingStack->getCreature()->idNumber == CreatureID::ARROW_TOWERS)
@@ -202,46 +160,6 @@ CDefenceAnimation::CDefenceAnimation(StackAttackedInfo _attackedInfo, BattleInte
 
 bool CDefenceAnimation::init()
 {
-	ui32 lowestMoveID = ID;
-	for(auto & elem : pendingAnimations())
-	{
-
-		auto * defAnim = dynamic_cast<CDefenceAnimation *>(elem);
-		if(defAnim && defAnim->stack->ID != stack->ID)
-			continue;
-
-		auto * attAnim = dynamic_cast<CAttackAnimation *>(elem);
-		if(attAnim && attAnim->stack->ID != stack->ID)
-			continue;
-
-		auto * sen = dynamic_cast<CPointEffectAnimation *>(elem);
-		if (sen && attacker == nullptr)
-			return false;
-
-		if (sen)
-			continue;
-
-		CReverseAnimation * animAsRev = dynamic_cast<CReverseAnimation *>(elem);
-
-		if(animAsRev)
-			return false;
-
-		if(elem)
-			vstd::amin(lowestMoveID, elem->ID);
-	}
-
-	if(ID > lowestMoveID)
-		return false;
-
-
-	//reverse unit if necessary
-	if(attacker && owner.getCurrentPlayerInterface()->cb->isToReverse(stack->getPosition(), attacker->getPosition(), stackFacingRight(stack), attacker->doubleWide(), stackFacingRight(attacker)))
-	{
-		owner.stacksController->addNewAnim(new CReverseAnimation(owner, stack, stack->getPosition(), true));
-		return false;
-	}
-	//unit reversed
-
 	if(rangedAttack && attacker != nullptr && owner.projectilesController->hasActiveProjectile(attacker)) //delay hit animation
 	{
 		return false;
@@ -256,6 +174,7 @@ bool CDefenceAnimation::init()
 
 		timeToWait = myAnim->framesInGroup(getMyAnimType()) * frameLength / 2;
 
+		//FIXME: perhaps this should be pause instead?
 		myAnim->setType(ECreatureAnimType::HOLDING);
 	}
 	else
@@ -349,9 +268,6 @@ void CDummyAnimation::nextFrame()
 
 bool CMeleeAttackAnimation::init()
 {
-	if(!CAttackAnimation::checkInitialConditions())
-		return false;
-
 	if(!attackingStack || myAnim->isDeadOrDying())
 	{
 		delete this;
@@ -362,7 +278,7 @@ bool CMeleeAttackAnimation::init()
 
 	if(toReverse)
 	{
-		owner.stacksController->addNewAnim(new CReverseAnimation(owner, stack, attackingStackPosBeforeReturn, true));
+		owner.stacksController->addNewAnim(new CReverseAnimation(owner, stack, attackingStackPosBeforeReturn));
 		return false;
 	}
 
@@ -453,8 +369,8 @@ CStackMoveAnimation::CStackMoveAnimation(BattleInterface & owner, const CStack *
 
 bool CMovementAnimation::init()
 {
-	if( !CBattleAnimation::checkInitialConditions() )
-		return false;
+	assert(stack);
+	assert(!myAnim->isDeadOrDying());
 
 	if(!stack || myAnim->isDeadOrDying())
 	{
@@ -473,17 +389,9 @@ bool CMovementAnimation::init()
 	//reverse unit if necessary
 	if(owner.stacksController->shouldRotate(stack, oldPos, currentHex))
 	{
-		// it seems that H3 does NOT plays full rotation animation here in most situations
+		// it seems that H3 does NOT plays full rotation animation during movement
 		// Logical since it takes quite a lot of time
-		if (curentMoveIndex == 0) // full rotation only for moving towards first tile.
-		{
-			owner.stacksController->addNewAnim(new CReverseAnimation(owner, stack, oldPos, true));
-			return false;
-		}
-		else
-		{
-			rotateStack(oldPos);
-		}
+		rotateStack(oldPos);
 	}
 
 	if(myAnim->getType() != ECreatureAnimType::MOVING)
@@ -554,7 +462,6 @@ CMovementAnimation::~CMovementAnimation()
 	assert(stack);
 
 	myAnim->pos = owner.stacksController->getStackPositionAtHex(currentHex, stack);
-	owner.stacksController->addNewAnim(new CMovementEndAnimation(owner, stack, currentHex));
 
 	if(owner.moveSoundHander != -1)
 	{
@@ -584,11 +491,10 @@ CMovementEndAnimation::CMovementEndAnimation(BattleInterface & owner, const CSta
 
 bool CMovementEndAnimation::init()
 {
-	//if( !isEarliest(true) )
-	//	return false;
+	assert(stack);
+	assert(!myAnim->isDeadOrDying());
 
-	if(!stack || myAnim->framesInGroup(ECreatureAnimType::MOVE_END) == 0 ||
-		myAnim->isDeadOrDying())
+	if(!stack || myAnim->isDeadOrDying())
 	{
 		delete this;
 		return false;
@@ -596,8 +502,13 @@ bool CMovementEndAnimation::init()
 
 	CCS->soundh->playSound(battle_sound(stack->getCreature(), endMoving));
 
-	myAnim->setType(ECreatureAnimType::MOVE_END);
+	if(!myAnim->framesInGroup(ECreatureAnimType::MOVE_END))
+	{
+		delete this;
+		return false;
+	}
 
+	myAnim->setType(ECreatureAnimType::MOVE_END);
 	myAnim->onAnimationReset += [&](){ delete this; };
 
 	return true;
@@ -619,8 +530,8 @@ CMovementStartAnimation::CMovementStartAnimation(BattleInterface & owner, const
 
 bool CMovementStartAnimation::init()
 {
-	if( !CBattleAnimation::checkInitialConditions() )
-		return false;
+	assert(stack);
+	assert(!myAnim->isDeadOrDying());
 
 	if(!stack || myAnim->isDeadOrDying())
 	{
@@ -629,15 +540,20 @@ bool CMovementStartAnimation::init()
 	}
 
 	CCS->soundh->playSound(battle_sound(stack->getCreature(), startMoving));
+
+	if(!myAnim->framesInGroup(ECreatureAnimType::MOVE_START))
+	{
+		delete this;
+		return false;
+	}
+
 	myAnim->setType(ECreatureAnimType::MOVE_START);
 	myAnim->onAnimationReset += [&](){ delete this; };
-
 	return true;
 }
 
-CReverseAnimation::CReverseAnimation(BattleInterface & owner, const CStack * stack, BattleHex dest, bool _priority)
-	: CStackMoveAnimation(owner, stack, dest),
-	  priority(_priority)
+CReverseAnimation::CReverseAnimation(BattleInterface & owner, const CStack * stack, BattleHex dest)
+	: CStackMoveAnimation(owner, stack, dest)
 {
 	logAnim->debug("Created reverse anim for %s", stack->getName());
 }
@@ -650,9 +566,6 @@ bool CReverseAnimation::init()
 		return false; //there is no such creature
 	}
 
-	if(!priority && !CBattleAnimation::checkInitialConditions())
-		return false;
-
 	if(myAnim->framesInGroup(ECreatureAnimType::TURN_L))
 	{
 		myAnim->setType(ECreatureAnimType::TURN_L);
@@ -699,9 +612,6 @@ void CReverseAnimation::setupSecondPart()
 
 bool CResurrectionAnimation::init()
 {
-	if( !CBattleAnimation::checkInitialConditions() )
-		return false;
-
 	if(!stack)
 	{
 		delete this;
@@ -734,9 +644,6 @@ CRangedAttackAnimation::CRangedAttackAnimation(BattleInterface & owner, const CS
 
 bool CRangedAttackAnimation::init()
 {
-	if( !CAttackAnimation::checkInitialConditions() )
-		return false;
-
 	assert(attackingStack);
 	assert(!myAnim->isDeadOrDying());
 
@@ -748,13 +655,6 @@ bool CRangedAttackAnimation::init()
 		return false;
 	}
 
-	//reverse unit if necessary
-	if (attackingStack && attackedStack && owner.getCurrentPlayerInterface()->cb->isToReverse(attackingStack->getPosition(), attackedStack->getPosition(), stackFacingRight(attackingStack), attackingStack->doubleWide(), stackFacingRight(attackedStack)))
-	{
-		owner.stacksController->addNewAnim(new CReverseAnimation(owner, attackingStack, attackingStack->getPosition(), true));
-		return false;
-	}
-
 	logAnim->info("Ranged attack animation initialized");
 	setAnimationGroup();
 	initializeProjectile();
@@ -820,17 +720,6 @@ void CRangedAttackAnimation::emitProjectile()
 
 void CRangedAttackAnimation::nextFrame()
 {
-	for(auto & it : pendingAnimations())
-	{
-		CMovementStartAnimation * anim = dynamic_cast<CMovementStartAnimation *>(it);
-		CReverseAnimation * anim2 = dynamic_cast<CReverseAnimation *>(it);
-		if( (anim && anim->stack->ID == stack->ID) || (anim2 && anim2->stack->ID == stack->ID && anim2->priority ) )
-		{
-			assert(0); // FIXME: our stack started to move even though we are playing shooting animation? How?
-			return;
-		}
-	}
-
 	// animation should be paused if there is an active projectile
 	if (projectileEmitted)
 	{
@@ -1056,9 +945,6 @@ CPointEffectAnimation::CPointEffectAnimation(BattleInterface & owner, soundBase:
 
 bool CPointEffectAnimation::init()
 {
-	if(!CBattleAnimation::checkInitialConditions())
-		return false;
-
 	animation->preload();
 
 	auto first = animation->getImage(0, 0, true);

client/battle/BattleAnimationClasses.h

@@ -44,7 +44,6 @@ protected:
 	void setStackFacingRight(const CStack * stack, bool facingRight);
 
 	virtual bool init() = 0; //to be called - if returned false, call again until returns true
-	bool checkInitialConditions(); //determines if this animation is earliest of all
 
 public:
 	ui32 ID; //unique identifier
@@ -61,7 +60,7 @@ public:
 class CBattleStackAnimation : public CBattleAnimation
 {
 public:
-	std::shared_ptr<CreatureAnimation> myAnim; //animation for our stack, managed by CBattleInterface
+	std::shared_ptr<CreatureAnimation> myAnim; //animation for our stack, managed by BattleInterface
 	const CStack * stack; //id of stack whose animation it is
 
 	CBattleStackAnimation(BattleInterface & owner, const CStack * _stack);
@@ -86,7 +85,6 @@ protected:
 	const CCreature * getCreature() const;
 public:
 	void nextFrame() override;
-	bool checkInitialConditions();
 
 	CAttackAnimation(BattleInterface & owner, const CStack *attacker, BattleHex _dest, const CStack *defender);
 	~CAttackAnimation();
@@ -190,12 +188,11 @@ public:
 class CReverseAnimation : public CStackMoveAnimation
 {
 public:
-	bool priority; //true - high, false - low
 	bool init() override;
 
 	void setupSecondPart();
 
-	CReverseAnimation(BattleInterface & owner, const CStack * stack, BattleHex dest, bool _priority);
+	CReverseAnimation(BattleInterface & owner, const CStack * stack, BattleHex dest);
 	~CReverseAnimation();
 };
 

client/battle/BattleConstants.h

@@ -9,6 +9,27 @@
  */
 #pragma once
 
+namespace EBattleEffect
+{
+	enum EBattleEffect
+	{
+		// list of battle effects that have hardcoded triggers
+		FEAR         = 15,
+		GOOD_LUCK    = 18,
+		GOOD_MORALE  = 20,
+		BAD_MORALE   = 30,
+		BAD_LUCK     = 48,
+		RESURRECT    = 50,
+		DRAIN_LIFE   = 52, // hardcoded constant in CGameHandler
+		POISON       = 67,
+		DEATH_BLOW   = 73,
+		REGENERATION = 74,
+		MANA_DRAIN   = 77,
+
+		INVALID      = -1,
+	};
+}
+
 namespace EHeroAnimType
 {
 enum Type

client/battle/BattleEffectsController.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include "../../lib/battle/BattleHex.h"
+#include "BattleConstants.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -25,27 +26,6 @@ class BattleInterface;
 class BattleRenderer;
 class CPointEffectAnimation;
 
-namespace EBattleEffect
-{
-	enum EBattleEffect
-	{
-		// list of battle effects that have hardcoded triggers
-		FEAR         = 15,
-		GOOD_LUCK    = 18,
-		GOOD_MORALE  = 20,
-		BAD_MORALE   = 30,
-		BAD_LUCK     = 48,
-		RESURRECT    = 50,
-		DRAIN_LIFE   = 52, // hardcoded constant in CGameHandler
-		POISON       = 67,
-		DEATH_BLOW   = 73,
-		REGENERATION = 74,
-		MANA_DRAIN   = 77,
-
-		INVALID      = -1,
-	};
-}
-
 /// Struct for battle effect animation e.g. morale, prayer, armageddon, bless,...
 struct BattleEffect
 {
@@ -74,7 +54,6 @@ public:
 	//displays custom effect on the battlefield
 	void displayEffect(EBattleEffect::EBattleEffect effect, const BattleHex & destTile);
 	void displayEffect(EBattleEffect::EBattleEffect effect, uint32_t soundID, const BattleHex & destTile);
-	//void displayEffects(EBattleEffect::EBattleEffect effect, uint32_t soundID, const std::vector<BattleHex> & destTiles);
 
 	void battleTriggerEffect(const BattleTriggerEffect & bte);
 

client/battle/BattleInterface.cpp

@@ -51,7 +51,7 @@ BattleInterface::BattleInterface(const CCreatureSet *army1, const CCreatureSet *
 		std::shared_ptr<CPlayerInterface> att, std::shared_ptr<CPlayerInterface> defen, std::shared_ptr<CPlayerInterface> spectatorInt)
 	: attackingHeroInstance(hero1), defendingHeroInstance(hero2), animCount(0),
 	attackerInt(att), defenderInt(defen), curInt(att),
-	myTurn(false), moveStarted(false), moveSoundHander(-1), bresult(nullptr), battleActionsStarted(false)
+	myTurn(false), moveSoundHander(-1), bresult(nullptr), battleActionsStarted(false)
 {
 	OBJ_CONSTRUCTION;
 
@@ -548,6 +548,7 @@ void BattleInterface::spellCast(const BattleSpellCast * sc)
 		stacksController->addNewAnim(new CPointEffectAnimation(*this, soundBase::invalid, sc->side ? "SP07_A.DEF" : "SP07_B.DEF", leftHero));
 		stacksController->addNewAnim(new CPointEffectAnimation(*this, soundBase::invalid, sc->side ? "SP07_B.DEF" : "SP07_A.DEF", rightHero));
 	}
+	waitForAnims();
 }
 
 void BattleInterface::battleStacksEffectsSet(const SetStackEffect & sse)
@@ -908,10 +909,6 @@ void BattleInterface::show(SDL_Surface *to)
 	SDL_SetClipRect(to, &buf); //restoring previous clip_rect
 
 	showInterface(to);
-
-	//activation of next stack, if any
-	//TODO: should be moved to the very start of this method?
-	//activateStack();
 }
 
 void BattleInterface::collectRenderableObjects(BattleRenderer & renderer)

client/battle/BattleInterface.h

@@ -9,6 +9,7 @@
  */
 #pragma once
 
+#include "BattleConstants.h"
 #include "../gui/CIntObject.h"
 #include "../../lib/spells/CSpellHandler.h" //CSpell::TAnimation
 
@@ -56,10 +57,15 @@ class BattleEffectsController;
 /// Small struct which contains information about the id of the attacked stack, the damage dealt,...
 struct StackAttackedInfo
 {
-	const CStack *defender; //attacked stack
-	int64_t dmg; //damage dealt
-	unsigned int amountKilled; //how many creatures in stack has been killed
-	const CStack *attacker; //attacking stack
+	const CStack *defender;
+	const CStack *attacker;
+
+	int64_t  damageDealt;
+	uint32_t amountKilled;
+
+	EBattleEffect::EBattleEffect battleEffect;
+	SpellID spellEffect;
+
 	bool indirectAttack; //if true, stack was attacked indirectly - spell or ranged attack
 	bool killed; //if true, stack has been killed
 	bool rebirth; //if true, play rebirth animation after all
@@ -114,7 +120,6 @@ public:
 	static CondSh<BattleAction *> givenCommand; //data != nullptr if we have i.e. moved current unit
 
 	bool myTurn; //if true, interface is active (commands can be ordered)
-	bool moveStarted; //if true, the creature that is already moving is going to make its first step
 	int moveSoundHander; // sound handler used when moving a unit
 
 	const BattleResult *bresult; //result of a battle; if non-zero then display when all animations end

client/battle/BattleStacksController.cpp

@@ -147,6 +147,10 @@ void BattleStacksController::collectRenderableObjects(BattleRenderer & renderer)
 
 void BattleStacksController::stackReset(const CStack * stack)
 {
+	//FIXME: there should be no more ongoing animations. If not - then some other method created animations but did not wait for them to end
+	assert(!owner.animsAreDisplayed.get());
+	owner.waitForAnims();
+
 	auto iter = stackAnimation.find(stack->ID);
 
 	if(iter == stackAnimation.end())
@@ -170,8 +174,6 @@ void BattleStacksController::stackReset(const CStack * stack)
 	}
 
 	owner.waitForAnims();
-
-	//TODO: handle more cases
 }
 
 void BattleStacksController::stackAdded(const CStack * stack)
@@ -395,20 +397,41 @@ void BattleStacksController::stacksAreAttacked(std::vector<StackAttackedInfo> at
 {
 	for(auto & attackedInfo : attackedInfos)
 	{
-		//if (!attackedInfo.cloneKilled) //FIXME: play dead animation for cloned creature before it vanishes
-			addNewAnim(new CDefenceAnimation(attackedInfo, owner));
+		if (!attackedInfo.attacker)
+			continue;
 
-		if(attackedInfo.rebirth)
-		{
-			owner.effectsController->displayEffect(EBattleEffect::RESURRECT, soundBase::RESURECT, attackedInfo.defender->getPosition());
-		}
+		bool needsReverse =
+				owner.curInt->cb->isToReverse(
+					attackedInfo.defender->getPosition(),
+					attackedInfo.attacker->getPosition(),
+					facingRight(attackedInfo.defender),
+					attackedInfo.attacker->doubleWide(),
+					facingRight(attackedInfo.attacker));
+
+		if (needsReverse)
+			addNewAnim(new CReverseAnimation(owner, attackedInfo.defender, attackedInfo.defender->getPosition()));
+	}
+
+	for(auto & attackedInfo : attackedInfos)
+	{
+		addNewAnim(new CDefenceAnimation(attackedInfo, owner));
+
+		if (attackedInfo.battleEffect != EBattleEffect::INVALID)
+			owner.effectsController->displayEffect(EBattleEffect::EBattleEffect(attackedInfo.battleEffect), attackedInfo.defender->getPosition());
+
+		if (attackedInfo.spellEffect != SpellID::NONE)
+			owner.displaySpellEffect(attackedInfo.spellEffect, attackedInfo.defender->getPosition());
 	}
 	owner.waitForAnims();
 
 	for (auto & attackedInfo : attackedInfos)
 	{
 		if (attackedInfo.rebirth)
+		{
+			owner.effectsController->displayEffect(EBattleEffect::RESURRECT, soundBase::RESURECT, attackedInfo.defender->getPosition());
 			addNewAnim(new CResurrectionAnimation(owner, attackedInfo.defender));
+		}
+
 		if (attackedInfo.cloneKilled)
 			stackRemoved(attackedInfo.defender->ID);
 	}
@@ -417,21 +440,50 @@ void BattleStacksController::stacksAreAttacked(std::vector<StackAttackedInfo> at
 
 void BattleStacksController::stackMoved(const CStack *stack, std::vector<BattleHex> destHex, int distance)
 {
+	assert(destHex.size() > 0);
+
+	//FIXME: there should be no more ongoing animations. If not - then some other method created animations but did not wait for them to end
+	assert(!owner.animsAreDisplayed.get());
+
+	if(shouldRotate(stack, stack->getPosition(), destHex[0]))
+		addNewAnim(new CReverseAnimation(owner, stack, destHex[0]));
+
+	addNewAnim(new CMovementStartAnimation(owner, stack));
+	owner.waitForAnims();
+
 	addNewAnim(new CMovementAnimation(owner, stack, destHex, distance));
 	owner.waitForAnims();
+
+	addNewAnim(new CMovementEndAnimation(owner, stack, destHex.back()));
+	owner.waitForAnims();
 }
 
-void BattleStacksController::stackAttacking( const CStack *attacker, BattleHex dest, const CStack *attacked, bool shooting )
+void BattleStacksController::stackAttacking( const CStack *attacker, BattleHex dest, const CStack *defender, bool shooting )
 {
+	bool needsReverse =
+			owner.curInt->cb->isToReverse(
+				attacker->getPosition(),
+				defender->getPosition(),
+				facingRight(attacker),
+				attacker->doubleWide(),
+				facingRight(defender));
+
+	if (needsReverse)
+		addNewAnim(new CReverseAnimation(owner, attacker, attacker->getPosition()));
+
+	owner.waitForAnims();
+
 	if (shooting)
 	{
-		addNewAnim(new CShootingAnimation(owner, attacker, dest, attacked));
+		addNewAnim(new CShootingAnimation(owner, attacker, dest, defender));
 	}
 	else
 	{
-		addNewAnim(new CMeleeAttackAnimation(owner, attacker, dest, attacked));
+		addNewAnim(new CMeleeAttackAnimation(owner, attacker, dest, defender));
 	}
-	//waitForAnims();
+
+	// do not wait - waiting will be done at stacksAreAttacked
+	// waitForAnims();
 }
 
 bool BattleStacksController::shouldRotate(const CStack * stack, const BattleHex & oldPos, const BattleHex & nextHex) const
@@ -450,8 +502,11 @@ bool BattleStacksController::shouldRotate(const CStack * stack, const BattleHex
 
 void BattleStacksController::endAction(const BattleAction* action)
 {
+	//FIXME: there should be no more ongoing animations. If not - then some other method created animations but did not wait for them to end
+	assert(!owner.animsAreDisplayed.get());
+	owner.waitForAnims();
+
 	//check if we should reverse stacks
-	//for some strange reason, it's not enough
 	TStacks stacks = owner.curInt->cb->battleGetStacks(CBattleCallback::MINE_AND_ENEMY);
 
 	for (const CStack *s : stacks)
@@ -460,29 +515,15 @@ void BattleStacksController::endAction(const BattleAction* action)
 
 		if (s && facingRight(s) != shouldFaceRight && s->alive() && stackAnimation[s->ID]->isIdle())
 		{
-			addNewAnim(new CReverseAnimation(owner, s, s->getPosition(), false));
+			addNewAnim(new CReverseAnimation(owner, s, s->getPosition()));
 		}
 	}
+	owner.waitForAnims();
 }
 
 void BattleStacksController::startAction(const BattleAction* action)
 {
-	const CStack *stack = owner.curInt->cb->battleGetStackByID(action->stackNumber);
 	setHoveredStack(nullptr);
-
-	auto actionTarget = action->getTarget(owner.curInt->cb.get());
-
-	if(action->actionType == EActionType::WALK
-		|| (action->actionType == EActionType::WALK_AND_ATTACK && actionTarget.at(0).hexValue != stack->getPosition()))
-	{
-		assert(stack);
-		owner.moveStarted = true;
-		if (stackAnimation[action->stackNumber]->framesInGroup(ECreatureAnimType::MOVE_START))
-			addNewAnim(new CMovementStartAnimation(owner, stack));
-
-		//if(shouldRotate(stack, stack->getPosition(), actionTarget.at(0).hexValue))
-		//	addNewAnim(new CReverseAnimation(owner, stack, stack->getPosition(), true));
-	}
 }
 
 void BattleStacksController::activateStack()

client/gui/CGuiHandler.cpp

@@ -238,7 +238,7 @@ void CGuiHandler::handleCurrentEvent()
 				break;
 
 			case SDLK_F9:
-				//not working yet since CClient::run remain locked after CBattleInterface removal
+				//not working yet since CClient::run remain locked after BattleInterface removal
 //				if(LOCPLINT->battleInt)
 //				{
 //					GH.popInts(1);

lib/CGameInterface.cpp

@@ -174,9 +174,9 @@ void CAdventureAI::battleStart(const CCreatureSet * army1, const CCreatureSet *
 	battleAI->battleStart(army1, army2, tile, hero1, hero2, side);
 }
 
-void CAdventureAI::battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa)
+void CAdventureAI::battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa, bool ranged)
 {
-	battleAI->battleStacksAttacked(bsa);
+	battleAI->battleStacksAttacked(bsa, ranged);
 }
 
 void CAdventureAI::actionStarted(const BattleAction & action)

lib/CGameInterface.h

@@ -154,7 +154,7 @@ public:
 	virtual void battleNewRound(int round) override;
 	virtual void battleCatapultAttacked(const CatapultAttack & ca) override;
 	virtual void battleStart(const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool side) override;
-	virtual void battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa) override;
+	virtual void battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa, bool ranged) override;
 	virtual void actionStarted(const BattleAction &action) override;
 	virtual void battleNewRoundFirst(int round) override;
 	virtual void actionFinished(const BattleAction &action) override;

lib/GameConstants.h

@@ -911,7 +911,8 @@ enum class EActionType : int32_t
 	INVALID = -1,
 	NO_ACTION = 0,
 	HERO_SPELL,
-	WALK, DEFEND,
+	WALK,
+	DEFEND,
 	RETREAT,
 	SURRENDER,
 	WALK_AND_ATTACK,

lib/IGameEventsReceiver.h

@@ -59,7 +59,7 @@ public:
 	virtual void actionFinished(const BattleAction &action){};//occurs AFTER every action taken by any stack or by the hero
 	virtual void actionStarted(const BattleAction &action){};//occurs BEFORE every action taken by any stack or by the hero
 	virtual void battleAttack(const BattleAttack *ba){}; //called when stack is performing attack
-	virtual void battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa){}; //called when stack receives damage (after battleAttack())
+	virtual void battleStacksAttacked(const std::vector<BattleStackAttacked> & bsa, bool ranged){}; //called when stack receives damage (after battleAttack())
 	virtual void battleEnd(const BattleResult *br){};
 	virtual void battleNewRoundFirst(int round){}; //called at the beginning of each turn before changes are applied;
 	virtual void battleNewRound(int round){}; //called at the beginning of each turn, round=-1 is the tactic phase, round=0 is the first "normal" turn

lib/NetPacks.h

@@ -1569,10 +1569,14 @@ struct BattleUnitsChanged : public CPackForClient
 struct BattleStackAttacked
 {
 	BattleStackAttacked():
-		stackAttacked(0), attackerID(0),
-		killedAmount(0), damageAmount(0),
+		stackAttacked(0),
+		attackerID(0),
+		killedAmount(0),
+		damageAmount(0),
 		newState(),
-		flags(0), effect(0), spellID(SpellID::NONE)
+		flags(0),
+		effect(0),
+		spellID(SpellID::NONE)
 	{};
 
 	DLL_LINKAGE void applyGs(CGameState *gs);

server/CGameHandler.cpp

@@ -1221,6 +1221,7 @@ int64_t CGameHandler::applyBattleEffects(BattleAttack & bat, std::shared_ptr<bat
 	BattleStackAttacked bsa;
 	if(secondary)
 		bsa.flags |= BattleStackAttacked::SECONDARY; //all other targets do not suffer from spells & spell-like abilities
+
 	bsa.attackerID = attackerState->unitId();
 	bsa.stackAttacked = def->unitId();
 	{