Refactoring/cleanup of classes in CBattleAnimations.cpp

client/CPlayerInterface.cpp

@@ -32,7 +32,6 @@
 #include "windows/CTradeWindow.h"
 #include "windows/CSpellWindow.h"
 #include "../lib/CConfigHandler.h"
-#include "battle/CCreatureAnimation.h"
 #include "Graphics.h"
 #include "windows/GUIClasses.h"
 #include "../lib/CArtHandler.h"

client/CPlayerInterface.h

@@ -42,7 +42,6 @@ class CAdvMapInt;
 class CCastleInterface;
 class CBattleInterface;
 class CComponent;
-class CCreatureAnimation;
 class CSelectableComponent;
 class CSlider;
 class CInGameConsole;

client/battle/CBattleAnimations.cpp

@@ -35,78 +35,85 @@
 #include "../../lib/mapObjects/CGTownInstance.h"
 
 CBattleAnimation::CBattleAnimation(CBattleInterface * _owner)
-	: owner(_owner), ID(_owner->stacksController->animIDhelper++)
+	: owner(_owner),
+	  ID(_owner->stacksController->animIDhelper++),
+	  initialized(false)
 {
 	logAnim->trace("Animation #%d created", ID);
 }
 
-CBattleAnimation::~CBattleAnimation()
+bool CBattleAnimation::tryInitialize()
 {
-	logAnim->trace("Animation #%d deleted", ID);
+	assert(!initialized);
+
+	if ( init() )
+	{
+		initialized = true;
+		return true;
+	}
+	return false;
 }
 
-std::list<std::pair<CBattleAnimation *, bool>> & CBattleAnimation::pendingAnimations()
+bool CBattleAnimation::isInitialized()
 {
-	return owner->stacksController->pendingAnims;
+	return initialized;
 }
 
-std::shared_ptr<CCreatureAnimation> CBattleAnimation::stackAnimation(const CStack * stack)
+CBattleAnimation::~CBattleAnimation()
 {
-	return owner->stacksController->creAnims[stack->ID];
+	logAnim->trace("Animation #%d ended, type is %s", ID, typeid(this).name());
+	for(auto & elem : pendingAnimations())
+	{
+		if(elem == this)
+			elem = nullptr;
+	}
+	logAnim->trace("Animation #%d deleted", ID);
 }
 
-bool CBattleAnimation::stackFacingRight(const CStack * stack)
+std::vector<CBattleAnimation *> & CBattleAnimation::pendingAnimations()
 {
-	return owner->stacksController->creDir[stack->ID];
+	return owner->stacksController->currentAnimations;
 }
 
-ui32 CBattleAnimation::maxAnimationID()
+std::shared_ptr<CCreatureAnimation> CBattleAnimation::stackAnimation(const CStack * stack)
 {
-	return owner->stacksController->animIDhelper;
+	return owner->stacksController->stackAnimation[stack->ID];
 }
 
-void CBattleAnimation::setStackFacingRight(const CStack * stack, bool facingRight)
+bool CBattleAnimation::stackFacingRight(const CStack * stack)
 {
-	owner->stacksController->creDir[stack->ID] = facingRight;
+	return owner->stacksController->stackFacingRight[stack->ID];
 }
 
-void CBattleAnimation::endAnim()
+void CBattleAnimation::setStackFacingRight(const CStack * stack, bool facingRight)
 {
-	logAnim->trace("Animation #%d ended, type is %s", ID, typeid(this).name());
-	for(auto & elem : pendingAnimations())
-	{
-		if(elem.first == this)
-		{
-			elem.first = nullptr;
-		}
-	}
+	owner->stacksController->stackFacingRight[stack->ID] = facingRight;
 }
 
-bool CBattleAnimation::isEarliest(bool perStackConcurrency)
+bool CBattleAnimation::checkInitialConditions()
 {
-	int lowestMoveID = maxAnimationID() + 5;//FIXME: why 5?
+	int lowestMoveID = ID;
 	CBattleStackAnimation * thAnim = dynamic_cast<CBattleStackAnimation *>(this);
 	CEffectAnimation * thSen = dynamic_cast<CEffectAnimation *>(this);
 
 	for(auto & elem : pendingAnimations())
 	{
-		CBattleStackAnimation * stAnim = dynamic_cast<CBattleStackAnimation *>(elem.first);
-		CEffectAnimation * sen = dynamic_cast<CEffectAnimation *>(elem.first);
-		if(perStackConcurrency && stAnim && thAnim && stAnim->stack->ID != thAnim->stack->ID)
-			continue;
+		CEffectAnimation * sen = dynamic_cast<CEffectAnimation *>(elem);
 
-		if(perStackConcurrency && sen && thSen && sen != thSen)
+		// all effect animations can play concurrently with each other
+		if(sen && thSen && sen != thSen)
 			continue;
 
-		CReverseAnimation * revAnim = dynamic_cast<CReverseAnimation *>(stAnim);
+		CReverseAnimation * revAnim = dynamic_cast<CReverseAnimation *>(elem);
 
-		if(revAnim && thAnim && stAnim && stAnim->stack->ID == thAnim->stack->ID && revAnim->priority)
+		// 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.first)
-			vstd::amin(lowestMoveID, elem.first->ID);
+		if(elem)
+			vstd::amin(lowestMoveID, elem->ID);
 	}
-	return (ID == lowestMoveID) || (lowestMoveID == (maxAnimationID() + 5));
+	return ID == lowestMoveID;
 }
 
 CBattleStackAnimation::CBattleStackAnimation(CBattleInterface * owner, const CStack * stack)
@@ -128,7 +135,7 @@ void CAttackAnimation::nextFrame()
 	if(myAnim->getType() != group)
 	{
 		myAnim->setType(group);
-		myAnim->onAnimationReset += std::bind(&CAttackAnimation::endAnim, this);
+		myAnim->onAnimationReset += [&](){ delete this; };
 	}
 
 	if(!soundPlayed)
@@ -139,20 +146,18 @@ void CAttackAnimation::nextFrame()
 			CCS->soundh->playSound(battle_sound(getCreature(), attack));
 		soundPlayed = true;
 	}
-	CBattleAnimation::nextFrame();
 }
 
-void CAttackAnimation::endAnim()
+CAttackAnimation::~CAttackAnimation()
 {
 	myAnim->setType(CCreatureAnim::HOLDING);
-	CBattleStackAnimation::endAnim();
 }
 
 bool CAttackAnimation::checkInitialConditions()
 {
 	for(auto & elem : pendingAnimations())
 	{
-		CBattleStackAnimation * stAnim = dynamic_cast<CBattleStackAnimation *>(elem.first);
+		CBattleStackAnimation * stAnim = dynamic_cast<CBattleStackAnimation *>(elem);
 		CReverseAnimation * revAnim = dynamic_cast<CReverseAnimation *>(stAnim);
 
 		if(revAnim && attackedStack) // enemy must be fully reversed
@@ -161,7 +166,7 @@ bool CAttackAnimation::checkInitialConditions()
 				return false;
 		}
 	}
-	return isEarliest(false);
+	return CBattleAnimation::checkInitialConditions();
 }
 
 const CCreature * CAttackAnimation::getCreature()
@@ -192,32 +197,32 @@ killed(_attackedInfo.killed), timeToWait(0)
 
 bool CDefenceAnimation::init()
 {
-	ui32 lowestMoveID = maxAnimationID() + 5;
+	ui32 lowestMoveID = ID;
 	for(auto & elem : pendingAnimations())
 	{
 
-		CDefenceAnimation * defAnim = dynamic_cast<CDefenceAnimation *>(elem.first);
+		CDefenceAnimation * defAnim = dynamic_cast<CDefenceAnimation *>(elem);
 		if(defAnim && defAnim->stack->ID != stack->ID)
 			continue;
 
-		CAttackAnimation * attAnim = dynamic_cast<CAttackAnimation *>(elem.first);
+		CAttackAnimation * attAnim = dynamic_cast<CAttackAnimation *>(elem);
 		if(attAnim && attAnim->stack->ID != stack->ID)
 			continue;
 
-		CEffectAnimation * sen = dynamic_cast<CEffectAnimation *>(elem.first);
+		CEffectAnimation * sen = dynamic_cast<CEffectAnimation *>(elem);
 		if (sen && attacker == nullptr)
 			return false;
 
 		if (sen)
 			continue;
 
-		CReverseAnimation * animAsRev = dynamic_cast<CReverseAnimation *>(elem.first);
+		CReverseAnimation * animAsRev = dynamic_cast<CReverseAnimation *>(elem);
 
 		if(animAsRev)
 			return false;
 
-		if(elem.first)
-			vstd::amin(lowestMoveID, elem.first->ID);
+		if(elem)
+			vstd::amin(lowestMoveID, elem->ID);
 	}
 
 	if(ID > lowestMoveID)
@@ -287,7 +292,7 @@ void CDefenceAnimation::startAnimation()
 {
 	CCS->soundh->playSound(getMySound());
 	myAnim->setType(getMyAnimType());
-	myAnim->onAnimationReset += std::bind(&CDefenceAnimation::endAnim, this);
+	myAnim->onAnimationReset += [&](){ delete this; };
 }
 
 void CDefenceAnimation::nextFrame()
@@ -302,7 +307,7 @@ void CDefenceAnimation::nextFrame()
 	CBattleAnimation::nextFrame();
 }
 
-void CDefenceAnimation::endAnim()
+CDefenceAnimation::~CDefenceAnimation()
 {
 	if(killed)
 	{
@@ -315,11 +320,6 @@ void CDefenceAnimation::endAnim()
 	{
 		myAnim->setType(CCreatureAnim::HOLDING);
 	}
-
-
-	CBattleAnimation::endAnim();
-
-	delete this;
 }
 
 CDummyAnimation::CDummyAnimation(CBattleInterface * _owner, int howManyFrames)
@@ -337,14 +337,7 @@ void CDummyAnimation::nextFrame()
 {
 	counter++;
 	if(counter > howMany)
-		endAnim();
-}
-
-void CDummyAnimation::endAnim()
-{
-	CBattleAnimation::endAnim();
-
-	delete this;
+		delete this;
 }
 
 bool CMeleeAttackAnimation::init()
@@ -354,7 +347,7 @@ bool CMeleeAttackAnimation::init()
 
 	if(!attackingStack || myAnim->isDead())
 	{
-		endAnim();
+		delete this;
 		return false;
 	}
 
@@ -444,20 +437,14 @@ CMeleeAttackAnimation::CMeleeAttackAnimation(CBattleInterface * _owner, const CS
 	logAnim->debug("Created melee attack anim for %s", attacker->getName());
 }
 
-void CMeleeAttackAnimation::endAnim()
-{
-	CAttackAnimation::endAnim();
-	delete this;
-}
-
 bool CMovementAnimation::init()
 {
-	if( !isEarliest(false) )
+	if( !CBattleAnimation::checkInitialConditions() )
 		return false;
 
 	if(!stack || myAnim->isDead())
 	{
-		endAnim();
+		delete this;
 		return false;
 	}
 
@@ -465,7 +452,7 @@ bool CMovementAnimation::init()
 	   stack->hasBonus(Selector::typeSubtype(Bonus::FLYING, 1)))
 	{
 		//no movement or teleport, end immediately
-		endAnim();
+		delete this;
 		return false;
 	}
 
@@ -540,28 +527,19 @@ void CMovementAnimation::nextFrame()
 			oldPos = nextHex;
 			nextHex = destTiles[curentMoveIndex];
 
-			// re-init animation
-			for(auto & elem : pendingAnimations())
-			{
-				if (elem.first == this)
-				{
-					elem.second = false;
-					break;
-				}
-			}
+			// request re-initialization
+			initialized = false;
 		}
 		else
-			endAnim();
+			delete this;
 	}
 }
 
-void CMovementAnimation::endAnim()
+CMovementAnimation::~CMovementAnimation()
 {
 	assert(stack);
 
 	myAnim->pos = owner->stacksController->getStackPositionAtHex(nextHex, stack);
-	CBattleAnimation::endAnim();
-
 	owner->stacksController->addNewAnim(new CMovementEndAnimation(owner, stack, nextHex));
 
 	if(owner->moveSoundHander != -1)
@@ -569,7 +547,6 @@ void CMovementAnimation::endAnim()
 		CCS->soundh->stopSound(owner->moveSoundHander);
 		owner->moveSoundHander = -1;
 	}
-	delete this;
 }
 
 CMovementAnimation::CMovementAnimation(CBattleInterface *_owner, const CStack *_stack, std::vector<BattleHex> _destTiles, int _distance)
@@ -594,14 +571,13 @@ CMovementEndAnimation::CMovementEndAnimation(CBattleInterface * _owner, const CS
 
 bool CMovementEndAnimation::init()
 {
-	if( !isEarliest(true) )
-		return false;
+	//if( !isEarliest(true) )
+	//	return false;
 
 	if(!stack || myAnim->framesInGroup(CCreatureAnim::MOVE_END) == 0 ||
 		myAnim->isDead())
 	{
-		endAnim();
-
+		delete this;
 		return false;
 	}
 
@@ -609,20 +585,17 @@ bool CMovementEndAnimation::init()
 
 	myAnim->setType(CCreatureAnim::MOVE_END);
 
-	myAnim->onAnimationReset += std::bind(&CMovementEndAnimation::endAnim, this);
+	myAnim->onAnimationReset += [&](){ delete this; };
 
 	return true;
 }
 
-void CMovementEndAnimation::endAnim()
+CMovementEndAnimation::~CMovementEndAnimation()
 {
-	CBattleAnimation::endAnim();
-
 	if(myAnim->getType() != CCreatureAnim::DEAD)
 		myAnim->setType(CCreatureAnim::HOLDING); //resetting to default
 
 	CCS->curh->show();
-	delete this;
 }
 
 CMovementStartAnimation::CMovementStartAnimation(CBattleInterface * _owner, const CStack * _stack)
@@ -633,30 +606,23 @@ CMovementStartAnimation::CMovementStartAnimation(CBattleInterface * _owner, cons
 
 bool CMovementStartAnimation::init()
 {
-	if( !isEarliest(false) )
+	if( !CBattleAnimation::checkInitialConditions() )
 		return false;
 
 
 	if(!stack || myAnim->isDead())
 	{
-		CMovementStartAnimation::endAnim();
+		delete this;
 		return false;
 	}
 
 	CCS->soundh->playSound(battle_sound(stack->getCreature(), startMoving));
 	myAnim->setType(CCreatureAnim::MOVE_START);
-	myAnim->onAnimationReset += std::bind(&CMovementStartAnimation::endAnim, this);
+	myAnim->onAnimationReset += [&](){ delete this; };
 
 	return true;
 }
 
-void CMovementStartAnimation::endAnim()
-{
-	CBattleAnimation::endAnim();
-
-	delete this;
-}
-
 CReverseAnimation::CReverseAnimation(CBattleInterface * _owner, const CStack * stack, BattleHex dest, bool _priority)
 : CBattleStackAnimation(_owner, stack), hex(dest), priority(_priority)
 {
@@ -667,11 +633,11 @@ bool CReverseAnimation::init()
 {
 	if(myAnim == nullptr || myAnim->isDead())
 	{
-		endAnim();
+		delete this;
 		return false; //there is no such creature
 	}
 
-	if(!priority && !isEarliest(false))
+	if(!priority && !CBattleAnimation::checkInitialConditions())
 		return false;
 
 	if(myAnim->framesInGroup(CCreatureAnim::TURN_L))
@@ -686,13 +652,10 @@ bool CReverseAnimation::init()
 	return true;
 }
 
-void CReverseAnimation::endAnim()
+CReverseAnimation::~CReverseAnimation()
 {
-	CBattleAnimation::endAnim();
-	if( stack->alive() )//don't do that if stack is dead
+	if( stack && stack->alive() )//don't do that if stack is dead
 		myAnim->setType(CCreatureAnim::HOLDING);
-
-	delete this;
 }
 
 void CBattleStackAnimation::rotateStack(BattleHex hex)
@@ -706,7 +669,7 @@ void CReverseAnimation::setupSecondPart()
 {
 	if(!stack)
 	{
-		endAnim();
+		delete this;
 		return;
 	}
 
@@ -715,10 +678,10 @@ void CReverseAnimation::setupSecondPart()
 	if(myAnim->framesInGroup(CCreatureAnim::TURN_R))
 	{
 		myAnim->setType(CCreatureAnim::TURN_R);
-		myAnim->onAnimationReset += std::bind(&CReverseAnimation::endAnim, this);
+		myAnim->onAnimationReset += [&](){ delete this; };
 	}
 	else
-		endAnim();
+		delete this;
 }
 
 CRangedAttackAnimation::CRangedAttackAnimation(CBattleInterface * owner_, const CStack * attacker, BattleHex dest_, const CStack * defender)
@@ -742,11 +705,14 @@ bool CShootingAnimation::init()
 	if( !CAttackAnimation::checkInitialConditions() )
 		return false;
 
+	assert(attackingStack);
+	assert(!myAnim->isDead());
+
 	if(!attackingStack || myAnim->isDead())
 	{
 		//FIXME: how is this possible?
 		logAnim->warn("Shooting animation has not started yet but attacker is dead! Aborting...");
-		endAnim();
+		delete this;
 		return false;
 	}
 
@@ -757,11 +723,6 @@ bool CShootingAnimation::init()
 		return false;
 	}
 
-	//FIXME: this cause freeze
-	// opponent must face attacker ( = different directions) before he can be attacked
-	//if (attackingStack && attackedStack && owner->creDir[attackingStack->ID] == owner->creDir[attackedStack->ID])
-	//	return false;
-
 	setAnimationGroup();
 	initializeProjectile();
 	shooting = true;
@@ -827,8 +788,8 @@ void CShootingAnimation::nextFrame()
 {
 	for(auto & it : pendingAnimations())
 	{
-		CMovementStartAnimation * anim = dynamic_cast<CMovementStartAnimation *>(it.first);
-		CReverseAnimation * anim2 = dynamic_cast<CReverseAnimation *>(it.first);
+		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 ) )
 			return;
 	}
@@ -889,7 +850,7 @@ void CShootingAnimation::emitExplosion()
 	}
 }
 
-void CShootingAnimation::endAnim()
+CShootingAnimation::~CShootingAnimation()
 {
 	assert(!owner->projectilesController->hasActiveProjectile(attackingStack));
 	assert(projectileEmitted);
@@ -900,8 +861,6 @@ void CShootingAnimation::endAnim()
 		logAnim->warn("Shooting animation has finished but projectile was not emitted! Emitting it now...");
 		emitProjectile();
 	}
-	CAttackAnimation::endAnim();
-	delete this;
 }
 
 CCastAnimation::CCastAnimation(CBattleInterface * owner_, const CStack * attacker, BattleHex dest_, const CStack * defender)
@@ -918,7 +877,7 @@ bool CCastAnimation::init()
 
 	if(!attackingStack || myAnim->isDead())
 	{
-		endAnim();
+		delete this;
 		return false;
 	}
 
@@ -1008,7 +967,7 @@ void CCastAnimation::nextFrame()
 {
 	for(auto & it : pendingAnimations())
 	{
-		CReverseAnimation * anim = dynamic_cast<CReverseAnimation *>(it.first);
+		CReverseAnimation * anim = dynamic_cast<CReverseAnimation *>(it);
 		if(anim && anim->stack->ID == stack->ID && anim->priority)
 			return;
 	}
@@ -1016,20 +975,12 @@ void CCastAnimation::nextFrame()
 	if(myAnim->getType() != group)
 	{
 		myAnim->setType(group);
-		myAnim->onAnimationReset += std::bind(&CAttackAnimation::endAnim, this);
+		myAnim->onAnimationReset += [&](){ delete this; };
 	}
 
 	CBattleAnimation::nextFrame();
 }
 
-
-void CCastAnimation::endAnim()
-{
-	CAttackAnimation::endAnim();
-	delete this;
-}
-
-
 CEffectAnimation::CEffectAnimation(CBattleInterface * _owner, std::string _customAnim, int _x, int _y, int _dx, int _dy, bool _Vflip, bool _alignToBottom)
 	: CBattleAnimation(_owner),
 	destTile(BattleHex::INVALID),
@@ -1076,7 +1027,7 @@ CEffectAnimation::CEffectAnimation(CBattleInterface * _owner, std::string _custo
 
 bool CEffectAnimation::init()
 {
-	if(!isEarliest(true))
+	if(!CBattleAnimation::checkInitialConditions())
 		return false;
 
 	const bool areaEffect = (!destTile.isValid() && x == -1 && y == -1);
@@ -1090,7 +1041,7 @@ bool CEffectAnimation::init()
 	auto first = animation->getImage(0, 0, true);
 	if(!first)
 	{
-		endAnim();
+		delete this;
 		return false;
 	}
 
@@ -1168,8 +1119,8 @@ void CEffectAnimation::nextFrame()
 
 			if(elem.currentFrame >= elem.animation->size())
 			{
-				endAnim();
-				break;
+				delete this;
+				return;
 			}
 			else
 			{
@@ -1180,10 +1131,8 @@ void CEffectAnimation::nextFrame()
 	}
 }
 
-void CEffectAnimation::endAnim()
+CEffectAnimation::~CEffectAnimation()
 {
-	CBattleAnimation::endAnim();
-
 	auto & effects = owner->effectsController->battleEffects;
 
 	for ( auto it = effects.begin(); it != effects.end(); )
@@ -1193,6 +1142,4 @@ void CEffectAnimation::endAnim()
 		else
 			it++;
 	}
-
-	delete this;
 }

client/battle/CBattleAnimations.h

@@ -27,24 +27,27 @@ struct StackAttackedInfo;
 /// Base class of battle animations
 class CBattleAnimation
 {
+
 protected:
 	CBattleInterface * owner;
+	bool initialized;
 
-	std::list<std::pair<CBattleAnimation *, bool>> & pendingAnimations();
+	std::vector<CBattleAnimation *> & pendingAnimations();
 	std::shared_ptr<CCreatureAnimation> stackAnimation(const CStack * stack);
 	bool stackFacingRight(const CStack * stack);
 	void setStackFacingRight(const CStack * stack, bool facingRight);
-	ui32 maxAnimationID();
 
-public:
 	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
+
+	bool isInitialized();
+	bool tryInitialize();
 	virtual void nextFrame() {} //call every new frame
-	virtual void endAnim(); //to be called mostly internally; in this class it removes animation from pendingAnims list
 	virtual ~CBattleAnimation();
 
-	bool isEarliest(bool perStackConcurrency); //determines if this animation is earliest of all
-
-	ui32 ID; //unique identifier
 	CBattleAnimation(CBattleInterface * _owner);
 };
 
@@ -77,10 +80,10 @@ protected:
 	const CCreature * getCreature();
 public:
 	void nextFrame() override;
-	void endAnim() override;
 	bool checkInitialConditions();
 
 	CAttackAnimation(CBattleInterface *_owner, const CStack *attacker, BattleHex _dest, const CStack *defender);
+	~CAttackAnimation();
 };
 
 /// Animation of a defending unit
@@ -99,10 +102,9 @@ class CDefenceAnimation : public CBattleStackAnimation
 public:
 	bool init() override;
 	void nextFrame() override;
-	void endAnim() override;
 
 	CDefenceAnimation(StackAttackedInfo _attackedInfo, CBattleInterface * _owner);
-	virtual ~CDefenceAnimation(){};
+	~CDefenceAnimation();
 };
 
 class CDummyAnimation : public CBattleAnimation
@@ -113,10 +115,8 @@ private:
 public:
 	bool init() override;
 	void nextFrame() override;
-	void endAnim() override;
 
 	CDummyAnimation(CBattleInterface * _owner, int howManyFrames);
-	virtual ~CDummyAnimation(){}
 };
 
 /// Hand-to-hand attack
@@ -124,10 +124,8 @@ class CMeleeAttackAnimation : public CAttackAnimation
 {
 public:
 	bool init() override;
-	void endAnim() override;
 
 	CMeleeAttackAnimation(CBattleInterface * _owner, const CStack * attacker, BattleHex _dest, const CStack * _attacked);
-	virtual ~CMeleeAttackAnimation(){};
 };
 
 /// Move animation of a creature
@@ -150,10 +148,9 @@ public:
 
 	bool init() override;
 	void nextFrame() override;
-	void endAnim() override;
 
 	CMovementAnimation(CBattleInterface *_owner, const CStack *_stack, std::vector<BattleHex> _destTiles, int _distance);
-	virtual ~CMovementAnimation(){};
+	~CMovementAnimation();
 };
 
 /// Move end animation of a creature
@@ -163,10 +160,9 @@ private:
 	BattleHex destinationTile;
 public:
 	bool init() override;
-	void endAnim() override;
 
 	CMovementEndAnimation(CBattleInterface * _owner, const CStack * _stack, BattleHex destTile);
-	virtual ~CMovementEndAnimation(){};
+	~CMovementEndAnimation();
 };
 
 /// Move start animation of a creature
@@ -174,10 +170,8 @@ class CMovementStartAnimation : public CBattleStackAnimation
 {
 public:
 	bool init() override;
-	void endAnim() override;
 
 	CMovementStartAnimation(CBattleInterface * _owner, const CStack * _stack);
-	virtual ~CMovementStartAnimation(){};
 };
 
 /// Class responsible for animation of stack chaning direction (left <-> right)
@@ -188,13 +182,10 @@ public:
 	bool priority; //true - high, false - low
 	bool init() override;
 
-
-
 	void setupSecondPart();
-	void endAnim() override;
 
 	CReverseAnimation(CBattleInterface * _owner, const CStack * stack, BattleHex dest, bool _priority);
-	virtual ~CReverseAnimation(){};
+	~CReverseAnimation();
 };
 
 class CRangedAttackAnimation : public CAttackAnimation
@@ -220,12 +211,11 @@ private:
 public:
 	bool init() override;
 	void nextFrame() override;
-	void endAnim() override;
 
 	//last two params only for catapult attacks
 	CShootingAnimation(CBattleInterface * _owner, const CStack * attacker, BattleHex _dest,
 		const CStack * _attacked, bool _catapult = false, int _catapultDmg = 0);
-	virtual ~CShootingAnimation(){};
+	~CShootingAnimation();
 };
 
 class CCastAnimation : public CRangedAttackAnimation
@@ -235,11 +225,8 @@ public:
 
 	bool init() override;
 	void nextFrame() override;
-	void endAnim() override;
-
 };
 
-
 /// This class manages effect animation
 class CEffectAnimation : public CBattleAnimation
 {
@@ -252,12 +239,11 @@ private:
 public:
 	bool init() override;
 	void nextFrame() override;
-	void endAnim() override;
 
 	CEffectAnimation(CBattleInterface * _owner, std::string _customAnim, int _x, int _y, int _dx = 0, int _dy = 0, bool _Vflip = false, bool _alignToBottom = false);
 
 	CEffectAnimation(CBattleInterface * _owner, std::shared_ptr<CAnimation> _customAnim, int _x, int _y, int _dx = 0, int _dy = 0);
 
 	CEffectAnimation(CBattleInterface * _owner, std::string _customAnim, BattleHex _destTile, bool _Vflip = false, bool _alignToBottom = false);
-	virtual ~CEffectAnimation(){};
+	 ~CEffectAnimation();
 };

client/battle/CBattleInterface.h

@@ -28,9 +28,7 @@ struct CatapultAttack;
 struct BattleTriggerEffect;
 struct BattleHex;
 struct InfoAboutHero;
-//class CBattleGameInterface;
 struct CustomEffectInfo;
-//class CSpell;
 
 VCMI_LIB_NAMESPACE_END
 
@@ -92,7 +90,6 @@ private:
 	void activateStack(); //sets activeStack to stackToActivate etc. //FIXME: No, it's not clear at all
 	void requestAutofightingAIToTakeAction();
 
-
 	void giveCommand(EActionType action, BattleHex tile = BattleHex(), si32 additional = -1);
 	void sendCommand(BattleAction *& command, const CStack * actor = nullptr);
 

client/battle/CBattleStacksController.cpp

@@ -93,13 +93,13 @@ void CBattleStacksController::showBattlefieldObjects(std::shared_ptr<CCanvas> ca
 {
 	auto getCurrentPosition = [&](const CStack *stack) -> BattleHex
 	{
-		for (auto & anim : pendingAnims)
+		for (auto & anim : currentAnimations)
 		{
 			// certainly ugly workaround but fixes quite annoying bug
 			// stack position will be updated only *after* movement is finished
 			// before this - stack is always at its initial position. Thus we need to find
 			// its current position. Which can be found only in this class
-			if (CMovementAnimation *move = dynamic_cast<CMovementAnimation*>(anim.first))
+			if (CMovementAnimation *move = dynamic_cast<CMovementAnimation*>(anim))
 			{
 				if (move->stack == stack)
 					return move->nextHex;
@@ -112,17 +112,17 @@ void CBattleStacksController::showBattlefieldObjects(std::shared_ptr<CCanvas> ca
 
 	for (auto & stack : stacks)
 	{
-		if (creAnims.find(stack->ID) == creAnims.end()) //e.g. for summoned but not yet handled stacks
+		if (stackAnimation.find(stack->ID) == stackAnimation.end()) //e.g. for summoned but not yet handled stacks
 			continue;
 
 		//if (stack->initialPosition < 0) // turret shooters are handled separately
 		//	continue;
 
 		//FIXME: hack to ignore ghost stacks
-		if ((creAnims[stack->ID]->getType() == CCreatureAnim::DEAD || creAnims[stack->ID]->getType() == CCreatureAnim::HOLDING) && stack->isGhost())
+		if ((stackAnimation[stack->ID]->getType() == CCreatureAnim::DEAD || stackAnimation[stack->ID]->getType() == CCreatureAnim::HOLDING) && stack->isGhost())
 			continue;//ignore
 
-		if (creAnims[stack->ID]->isDead())
+		if (stackAnimation[stack->ID]->isDead())
 		{
 			//if ( location == stack->getPosition() )
 			if ( location == BattleHex::HEX_BEFORE_ALL ) //FIXME: any cases when using BEFORE_ALL won't work?
@@ -131,7 +131,7 @@ void CBattleStacksController::showBattlefieldObjects(std::shared_ptr<CCanvas> ca
 		}
 
 		// standing - blit at current position
-		if (!creAnims[stack->ID]->isMoving())
+		if (!stackAnimation[stack->ID]->isMoving())
 		{
 			if ( location == stack->getPosition() )
 				showStack(canvas, stack);
@@ -157,9 +157,9 @@ void CBattleStacksController::showBattlefieldObjects(std::shared_ptr<CCanvas> ca
 
 void CBattleStacksController::stackReset(const CStack * stack)
 {
-	auto iter = creAnims.find(stack->ID);
+	auto iter = stackAnimation.find(stack->ID);
 
-	if(iter == creAnims.end())
+	if(iter == stackAnimation.end())
 	{
 		logGlobal->error("Unit %d have no animation", stack->ID);
 		return;
@@ -181,7 +181,7 @@ void CBattleStacksController::stackReset(const CStack * stack)
 
 void CBattleStacksController::stackAdded(const CStack * stack)
 {
-	creDir[stack->ID] = stack->side == BattleSide::ATTACKER; // must be set before getting stack position
+	stackFacingRight[stack->ID] = stack->side == BattleSide::ATTACKER; // must be set before getting stack position
 
 	Point coords = getStackPositionAtHex(stack->getPosition(), stack);
 
@@ -191,32 +191,32 @@ void CBattleStacksController::stackAdded(const CStack * stack)
 
 		const CCreature *turretCreature = owner->siegeController->getTurretCreature();
 
-		creAnims[stack->ID] = AnimationControls::getAnimation(turretCreature);
-		creAnims[stack->ID]->pos.h = 225;
+		stackAnimation[stack->ID] = AnimationControls::getAnimation(turretCreature);
+		stackAnimation[stack->ID]->pos.h = 225;
 
 		coords = owner->siegeController->getTurretCreaturePosition(stack->initialPosition);
 	}
 	else
 	{
-		creAnims[stack->ID] = AnimationControls::getAnimation(stack->getCreature());
-		creAnims[stack->ID]->onAnimationReset += std::bind(&onAnimationFinished, stack, creAnims[stack->ID]);
-		creAnims[stack->ID]->pos.h = creAnims[stack->ID]->getHeight();
+		stackAnimation[stack->ID] = AnimationControls::getAnimation(stack->getCreature());
+		stackAnimation[stack->ID]->onAnimationReset += std::bind(&onAnimationFinished, stack, stackAnimation[stack->ID]);
+		stackAnimation[stack->ID]->pos.h = stackAnimation[stack->ID]->getHeight();
 	}
-	creAnims[stack->ID]->pos.x = coords.x;
-	creAnims[stack->ID]->pos.y = coords.y;
-	creAnims[stack->ID]->pos.w = creAnims[stack->ID]->getWidth();
-	creAnims[stack->ID]->setType(CCreatureAnim::HOLDING);
+	stackAnimation[stack->ID]->pos.x = coords.x;
+	stackAnimation[stack->ID]->pos.y = coords.y;
+	stackAnimation[stack->ID]->pos.w = stackAnimation[stack->ID]->getWidth();
+	stackAnimation[stack->ID]->setType(CCreatureAnim::HOLDING);
 }
 
 void CBattleStacksController::setActiveStack(const CStack *stack)
 {
 	if (activeStack) // update UI
-		creAnims[activeStack->ID]->setBorderColor(AnimationControls::getNoBorder());
+		stackAnimation[activeStack->ID]->setBorderColor(AnimationControls::getNoBorder());
 
 	activeStack = stack;
 
 	if (activeStack) // update UI
-		creAnims[activeStack->ID]->setBorderColor(AnimationControls::getGoldBorder());
+		stackAnimation[activeStack->ID]->setBorderColor(AnimationControls::getGoldBorder());
 
 	owner->controlPanel->blockUI(activeStack == nullptr);
 }
@@ -227,7 +227,7 @@ void CBattleStacksController::setHoveredStack(const CStack *stack)
 		 return;
 
 	if (mouseHoveredStack)
-		creAnims[mouseHoveredStack->ID]->setBorderColor(AnimationControls::getNoBorder());
+		stackAnimation[mouseHoveredStack->ID]->setBorderColor(AnimationControls::getNoBorder());
 
 	// stack must be alive and not active (which uses gold border instead)
 	if (stack && stack->alive() && stack != activeStack)
@@ -236,9 +236,9 @@ void CBattleStacksController::setHoveredStack(const CStack *stack)
 
 		if (mouseHoveredStack)
 		{
-			creAnims[mouseHoveredStack->ID]->setBorderColor(AnimationControls::getBlueBorder());
-			if (creAnims[mouseHoveredStack->ID]->framesInGroup(CCreatureAnim::MOUSEON) > 0)
-				creAnims[mouseHoveredStack->ID]->playOnce(CCreatureAnim::MOUSEON);
+			stackAnimation[mouseHoveredStack->ID]->setBorderColor(AnimationControls::getBlueBorder());
+			if (stackAnimation[mouseHoveredStack->ID]->framesInGroup(CCreatureAnim::MOUSEON) > 0)
+				stackAnimation[mouseHoveredStack->ID]->playOnce(CCreatureAnim::MOUSEON);
 		}
 	}
 	else
@@ -264,9 +264,9 @@ bool CBattleStacksController::stackNeedsAmountBox(const CStack * stack)
 	if(stack->getCount() == 0) //hide box when target is going to die anyway - do not display "0 creatures"
 		return false;
 
-	for(auto anim : pendingAnims) //no matter what other conditions below are, hide box when creature is playing hit animation
+	for(auto anim : currentAnimations) //no matter what other conditions below are, hide box when creature is playing hit animation
 	{
-		auto hitAnimation = dynamic_cast<CDefenceAnimation*>(anim.first);
+		auto hitAnimation = dynamic_cast<CDefenceAnimation*>(anim);
 		if(hitAnimation && (hitAnimation->stack->ID == stack->ID))
 			return false;
 	}
@@ -325,18 +325,18 @@ void CBattleStacksController::showStackAmountBox(std::shared_ptr<CCanvas> canvas
 			(moveInside ? amountBG->width() + 10 : 0) * reverseSideShift;
 	int yAdd = 260 + ((stack->side == BattleSide::ATTACKER || moveInside) ? 0 : -15);
 
-	canvas->draw(amountBG, creAnims[stack->ID]->pos.topLeft() + Point(xAdd, yAdd));
+	canvas->draw(amountBG, stackAnimation[stack->ID]->pos.topLeft() + Point(xAdd, yAdd));
 
 	//blitting amount
-	Point textPos = creAnims[stack->ID]->pos.topLeft() + amountBG->dimensions()/2 + Point(xAdd, yAdd);
+	Point textPos = stackAnimation[stack->ID]->pos.topLeft() + amountBG->dimensions()/2 + Point(xAdd, yAdd);
 
 	canvas->drawText(textPos, EFonts::FONT_TINY, Colors::WHITE, ETextAlignment::CENTER, makeNumberShort(stack->getCount()));
 }
 
 void CBattleStacksController::showStack(std::shared_ptr<CCanvas> canvas, const CStack * stack)
 {
-	creAnims[stack->ID]->nextFrame(canvas, facingRight(stack)); // do actual blit
-	creAnims[stack->ID]->incrementFrame(float(GH.mainFPSmng->getElapsedMilliseconds()) / 1000);
+	stackAnimation[stack->ID]->nextFrame(canvas, facingRight(stack)); // do actual blit
+	stackAnimation[stack->ID]->incrementFrame(float(GH.mainFPSmng->getElapsedMilliseconds()) / 1000);
 
 	if (stackNeedsAmountBox(stack))
 		showStackAmountBox(canvas, stack);
@@ -344,44 +344,37 @@ void CBattleStacksController::showStack(std::shared_ptr<CCanvas> canvas, const C
 
 void CBattleStacksController::updateBattleAnimations()
 {
-	//handle animations
-	for (auto & elem : pendingAnims)
+	for (auto & elem : currentAnimations)
 	{
-		if (!elem.first) //this animation should be deleted
+		if (!elem)
 			continue;
 
-		if (!elem.second)
-		{
-			elem.second = elem.first->init();
-		}
-		if (elem.second && elem.first)
-			elem.first->nextFrame();
+		if (elem->isInitialized())
+			elem->nextFrame();
+		else
+			elem->tryInitialize();
 	}
 
-	//delete anims
-	int preSize = static_cast<int>(pendingAnims.size());
-	for (auto it = pendingAnims.begin(); it != pendingAnims.end(); ++it)
+	bool hadAnimations = !currentAnimations.empty();
+	for (auto it = currentAnimations.begin(); it != currentAnimations.end();)
 	{
-		if (it->first == nullptr)
-		{
-			pendingAnims.erase(it);
-			it = pendingAnims.begin();
-			break;
-		}
+		if (*it == nullptr)
+			it = currentAnimations.erase(it);
+		else
+			++it;
 	}
 
-	if (preSize > 0 && pendingAnims.empty())
+	if (hadAnimations && currentAnimations.empty())
 	{
 		//anims ended
 		owner->controlPanel->blockUI(activeStack == nullptr);
-
 		owner->animsAreDisplayed.setn(false);
 	}
 }
 
 void CBattleStacksController::addNewAnim(CBattleAnimation *anim)
 {
-	pendingAnims.push_back( std::make_pair(anim, false) );
+	currentAnimations.push_back(anim);
 	owner->animsAreDisplayed.setn(true);
 }
 
@@ -427,7 +420,7 @@ void CBattleStacksController::stacksAreAttacked(std::vector<StackAttackedInfo> a
 	for (auto & attackedInfo : attackedInfos)
 	{
 		if (attackedInfo.rebirth)
-			creAnims[attackedInfo.defender->ID]->setType(CCreatureAnim::HOLDING);
+			stackAnimation[attackedInfo.defender->ID]->setType(CCreatureAnim::HOLDING);
 		if (attackedInfo.cloneKilled)
 			stackRemoved(attackedInfo.defender->ID);
 	}
@@ -476,7 +469,7 @@ void CBattleStacksController::endAction(const BattleAction* action)
 	{
 		bool shouldFaceRight  = s && s->side == BattleSide::ATTACKER;
 
-		if (s && facingRight(s) != shouldFaceRight && s->alive() && creAnims[s->ID]->isIdle())
+		if (s && facingRight(s) != shouldFaceRight && s->alive() && stackAnimation[s->ID]->isIdle())
 		{
 			addNewAnim(new CReverseAnimation(owner, s, s->getPosition(), false));
 		}
@@ -495,7 +488,7 @@ void CBattleStacksController::startAction(const BattleAction* action)
 	{
 		assert(stack);
 		owner->moveStarted = true;
-		if (creAnims[action->stackNumber]->framesInGroup(CCreatureAnim::MOVE_START))
+		if (stackAnimation[action->stackNumber]->framesInGroup(CCreatureAnim::MOVE_START))
 			addNewAnim(new CMovementStartAnimation(owner, stack));
 
 		//if(shouldRotate(stack, stack->getPosition(), actionTarget.at(0).hexValue))
@@ -505,7 +498,7 @@ void CBattleStacksController::startAction(const BattleAction* action)
 
 void CBattleStacksController::activateStack()
 {
-	if ( !pendingAnims.empty())
+	if ( !currentAnimations.empty())
 		return;
 
 	if ( !stackToActivate)
@@ -557,7 +550,7 @@ const CStack* CBattleStacksController::getActiveStack()
 
 bool CBattleStacksController::facingRight(const CStack * stack)
 {
-	return creDir[stack->ID];
+	return stackFacingRight[stack->ID];
 }
 
 bool CBattleStacksController::activeStackSpellcaster()

client/battle/CBattleStacksController.h

@@ -35,9 +35,9 @@ class CBattleStacksController
 	std::shared_ptr<IImage> amountPositive;
 	std::shared_ptr<IImage> amountEffNeutral;
 
-	std::list<std::pair<CBattleAnimation *, bool>> pendingAnims; //currently displayed animations <anim, initialized>
-	std::map<int32_t, std::shared_ptr<CCreatureAnimation>> creAnims; //animations of creatures from fighting armies (order by BattleInfo's stacks' ID)
-	std::map<int, bool> creDir; // <creatureID, if false reverse creature's animation> //TODO: move it to battle callback
+	std::vector<CBattleAnimation *> currentAnimations; //currently displayed animations <anim, initialized>
+	std::map<int32_t, std::shared_ptr<CCreatureAnimation>> stackAnimation; //animations of creatures from fighting armies (order by BattleInfo's stacks' ID)
+	std::map<int, bool> stackFacingRight; // <creatureID, if false reverse creature's animation> //TODO: move it to battle callback
 
 	const CStack *activeStack; //number of active stack; nullptr - no one
 	const CStack *mouseHoveredStack; // stack below mouse pointer, used for border animation

client/windows/GUIClasses.cpp

@@ -28,8 +28,8 @@
 
 #include "../battle/CBattleInterfaceClasses.h"
 #include "../battle/CBattleInterface.h"
-#include "../battle/CCreatureAnimation.h"
 
+#include "../gui/CAnimation.h"
 #include "../gui/CGuiHandler.h"
 #include "../gui/SDL_Extensions.h"
 #include "../gui/CCursorHandler.h"