Changes according to review

Global.h

@@ -543,6 +543,12 @@ namespace vstd
 		return *itr;
 	}
 
+	template <typename Container, typename Item>
+	bool erase(Container &c, const Item &item)
+	{
+		c.erase(boost::remove(c, item), c.end());
+	}
+
 	template<typename Range, typename Predicate>
 	void erase_if(Range &vec, Predicate pred)
 	{
@@ -704,12 +710,6 @@ namespace vstd
 		return false;
 	}
 
-	template <typename Container, typename Pred>
-	void erase(Container &c, Pred pred)
-	{
-		c.erase(boost::remove_if(c, pred), c.end());
-	}
-
 	template<typename T>
 	void removeDuplicates(std::vector<T> &vec)
 	{

client/battle/BattleAnimationClasses.cpp

@@ -1176,13 +1176,9 @@ void CPointEffectAnimation::clearEffect()
 {
 	auto & effects = owner.effectsController->battleEffects;
 
-	for ( auto it = effects.begin(); it != effects.end(); )
-	{
-		if (it->effectID == ID)
-			it = effects.erase(it);
-		else
-			it++;
-	}
+	vstd::erase_if(effects, [&](const BattleEffect & effect){
+		return effect.effectID == ID;
+	});
 }
 
 CPointEffectAnimation::~CPointEffectAnimation()

client/battle/BattleEffectsController.cpp

@@ -127,7 +127,7 @@ void BattleEffectsController::collectRenderableObjects(BattleRenderer & renderer
 {
 	for (auto & elem : battleEffects)
 	{
-		renderer.insert( EBattleFieldLayer::EFFECTS, elem.position, [&elem](BattleRenderer::RendererPtr canvas)
+		renderer.insert( EBattleFieldLayer::EFFECTS, elem.position, [&elem](BattleRenderer::RendererRef canvas)
 		{
 			int currentFrame = static_cast<int>(floor(elem.currentFrame));
 			currentFrame %= elem.animation->size();

client/battle/BattleInterface.cpp

@@ -918,14 +918,14 @@ void BattleInterface::collectRenderableObjects(BattleRenderer & renderer)
 {
 	if (attackingHero)
 	{
-		renderer.insert(EBattleFieldLayer::HEROES, BattleHex(0),[this](BattleRenderer::RendererPtr canvas)
+		renderer.insert(EBattleFieldLayer::HEROES, BattleHex(0),[this](BattleRenderer::RendererRef canvas)
 		{
 			attackingHero->render(canvas);
 		});
 	}
 	if (defendingHero)
 	{
-		renderer.insert(EBattleFieldLayer::HEROES, BattleHex(GameConstants::BFIELD_WIDTH-1),[this](BattleRenderer::RendererPtr canvas)
+		renderer.insert(EBattleFieldLayer::HEROES, BattleHex(GameConstants::BFIELD_WIDTH-1),[this](BattleRenderer::RendererRef canvas)
 		{
 			defendingHero->render(canvas);
 		});

client/battle/BattleObstacleController.cpp

@@ -138,7 +138,7 @@ void BattleObstacleController::collectRenderableObjects(BattleRenderer & rendere
 		if (obstacle->obstacleType == CObstacleInstance::MOAT)
 			continue;
 
-		renderer.insert(EBattleFieldLayer::OBSTACLES, obstacle->pos, [this, obstacle]( BattleRenderer::RendererPtr canvas ){
+		renderer.insert(EBattleFieldLayer::OBSTACLES, obstacle->pos, [this, obstacle]( BattleRenderer::RendererRef canvas ){
 			auto img = getObstacleImage(*obstacle);
 			if(img)
 			{

client/battle/BattleProjectileController.cpp

@@ -230,13 +230,11 @@ void BattleProjectileController::showProjectiles(Canvas & canvas)
 		auto projectile = *it;
 		if ( projectile->playing )
 			projectile->show(canvas);
-
-		// finished flying
-		if ( projectile->step > projectile->steps)
-			it = projectiles.erase(it);
-		else
-			it++;
 	}
+
+	vstd::erase_if(projectiles, [&](const std::shared_ptr<ProjectileBase> & projectile){
+		return projectile->step > projectile->steps;
+	});
 }
 
 bool BattleProjectileController::hasActiveProjectile(const CStack * stack) const

client/battle/BattleRenderer.cpp

@@ -47,7 +47,7 @@ void BattleRenderer::sortObjects()
 	});
 }
 
-void BattleRenderer::renderObjects(BattleRenderer::RendererPtr targetCanvas)
+void BattleRenderer::renderObjects(BattleRenderer::RendererRef targetCanvas)
 {
 	for (auto const & object : objects)
 		object.functor(targetCanvas);
@@ -63,7 +63,7 @@ void BattleRenderer::insert(EBattleFieldLayer layer, BattleHex tile, BattleRende
 	objects.push_back({functor, layer, tile});
 }
 
-void BattleRenderer::execute(BattleRenderer::RendererPtr targetCanvas)
+void BattleRenderer::execute(BattleRenderer::RendererRef targetCanvas)
 {
 	collectObjects();
 	sortObjects();

client/battle/BattleRenderer.h

@@ -29,8 +29,8 @@ enum class EBattleFieldLayer {
 class BattleRenderer
 {
 public:
-	using RendererPtr = Canvas &;
-	using RenderFunctor = std::function<void(RendererPtr)>;
+	using RendererRef = Canvas &;
+	using RenderFunctor = std::function<void(RendererRef)>;
 
 private:
 	BattleInterface & owner;
@@ -45,10 +45,10 @@ private:
 
 	void collectObjects();
 	void sortObjects();
-	void renderObjects(RendererPtr targetCanvas);
+	void renderObjects(RendererRef targetCanvas);
 public:
 	BattleRenderer(BattleInterface & owner);
 
 	void insert(EBattleFieldLayer layer, BattleHex tile, RenderFunctor functor);
-	void execute(RendererPtr targetCanvas);
+	void execute(RendererRef targetCanvas);
 };

client/battle/BattleSiegeController.cpp

@@ -297,14 +297,14 @@ void BattleSiegeController::collectRenderableObjects(BattleRenderer & renderer)
 			wallPiece == EWallVisual::BOTTOM_BATTLEMENT ||
 			wallPiece == EWallVisual::UPPER_BATTLEMENT)
 		{
-			renderer.insert( EBattleFieldLayer::STACKS, getWallPiecePosition(wallPiece), [this, wallPiece](BattleRenderer::RendererPtr canvas){
+			renderer.insert( EBattleFieldLayer::STACKS, getWallPiecePosition(wallPiece), [this, wallPiece](BattleRenderer::RendererRef canvas){
 				owner.stacksController->showStack(canvas, getTurretStack(wallPiece));
 			});
-			renderer.insert( EBattleFieldLayer::BATTLEMENTS, getWallPiecePosition(wallPiece), [this, wallPiece](BattleRenderer::RendererPtr canvas){
+			renderer.insert( EBattleFieldLayer::BATTLEMENTS, getWallPiecePosition(wallPiece), [this, wallPiece](BattleRenderer::RendererRef canvas){
 				showWallPiece(canvas, wallPiece, owner.pos.topLeft());
 			});
 		}
-		renderer.insert( EBattleFieldLayer::WALLS, getWallPiecePosition(wallPiece), [this, wallPiece](BattleRenderer::RendererPtr canvas){
+		renderer.insert( EBattleFieldLayer::WALLS, getWallPiecePosition(wallPiece), [this, wallPiece](BattleRenderer::RendererRef canvas){
 			showWallPiece(canvas, wallPiece, owner.pos.topLeft());
 		});
 

client/battle/BattleStacksController.cpp

@@ -93,9 +93,9 @@ BattleStacksController::BattleStacksController(BattleInterface & owner):
 	}
 }
 
-BattleHex BattleStacksController::getStackCurrentPosition(const CStack * stack)
+BattleHex BattleStacksController::getStackCurrentPosition(const CStack * stack) const
 {
-	if ( !stackAnimation[stack->ID]->isMoving())
+	if ( !stackAnimation.at(stack->ID)->isMoving())
 		return stack->getPosition();
 
 	if (stack->hasBonusOfType(Bonus::FLYING))
@@ -132,13 +132,13 @@ void BattleStacksController::collectRenderableObjects(BattleRenderer & renderer)
 		auto layer = stackAnimation[stack->ID]->isDead() ? EBattleFieldLayer::CORPSES : EBattleFieldLayer::STACKS;
 		auto location = getStackCurrentPosition(stack);
 
-		renderer.insert(layer, location, [this, stack]( BattleRenderer::RendererPtr renderer ){
+		renderer.insert(layer, location, [this, stack]( BattleRenderer::RendererRef renderer ){
 			showStack(renderer, stack);
 		});
 
 		if (stackNeedsAmountBox(stack))
 		{
-			renderer.insert(EBattleFieldLayer::STACK_AMOUNTS, location, [this, stack]( BattleRenderer::RendererPtr renderer ){
+			renderer.insert(EBattleFieldLayer::STACK_AMOUNTS, location, [this, stack]( BattleRenderer::RendererRef renderer ){
 				showStackAmountBox(renderer, stack);
 			});
 		}
@@ -172,6 +172,9 @@ void BattleStacksController::stackReset(const CStack * stack)
 
 void BattleStacksController::stackAdded(const CStack * stack)
 {
+	// Tower shooters have only their upper half visible
+	static const int turretCreatureAnimationHeight = 235;
+
 	stackFacingRight[stack->ID] = stack->side == BattleSide::ATTACKER; // must be set before getting stack position
 
 	Point coords = getStackPositionAtHex(stack->getPosition(), stack);
@@ -183,7 +186,7 @@ void BattleStacksController::stackAdded(const CStack * stack)
 		const CCreature *turretCreature = owner.siegeController->getTurretCreature();
 
 		stackAnimation[stack->ID] = AnimationControls::getAnimation(turretCreature);
-		stackAnimation[stack->ID]->pos.h = 235;
+		stackAnimation[stack->ID]->pos.h = turretCreatureAnimationHeight;
 
 		coords = owner.siegeController->getTurretCreaturePosition(stack->initialPosition);
 	}
@@ -236,7 +239,7 @@ void BattleStacksController::setHoveredStack(const CStack *stack)
 		mouseHoveredStack = nullptr;
 }
 
-bool BattleStacksController::stackNeedsAmountBox(const CStack * stack)
+bool BattleStacksController::stackNeedsAmountBox(const CStack * stack) const
 {
 	BattleHex currentActionTarget;
 	if(owner.curInt->curAction)
@@ -347,13 +350,7 @@ void BattleStacksController::updateBattleAnimations()
 	}
 
 	bool hadAnimations = !currentAnimations.empty();
-	for (auto it = currentAnimations.begin(); it != currentAnimations.end();)
-	{
-		if (*it == nullptr)
-			it = currentAnimations.erase(it);
-		else
-			++it;
-	}
+	vstd::erase(currentAnimations, nullptr);
 
 	if (hadAnimations && currentAnimations.empty())
 	{
@@ -379,19 +376,15 @@ void BattleStacksController::stackActivated(const CStack *stack) //TODO: check i
 
 void BattleStacksController::stackRemoved(uint32_t stackID)
 {
-	if (getActiveStack() != nullptr)
+	if (getActiveStack() && getActiveStack()->ID == stackID)
 	{
-		if (getActiveStack()->ID == stackID)
-		{
-			BattleAction *action = new BattleAction();
-			action->side = owner.defendingHeroInstance ? (owner.curInt->playerID == owner.defendingHeroInstance->tempOwner) : false;
-			action->actionType = EActionType::CANCEL;
-			action->stackNumber = getActiveStack()->ID;
-			owner.givenCommand.setn(action);
-			setActiveStack(nullptr);
-		}
+		BattleAction *action = new BattleAction();
+		action->side = owner.defendingHeroInstance ? (owner.curInt->playerID == owner.defendingHeroInstance->tempOwner) : false;
+		action->actionType = EActionType::CANCEL;
+		action->stackNumber = getActiveStack()->ID;
+		owner.givenCommand.setn(action);
+		setActiveStack(nullptr);
 	}
-	//todo: ensure that ghost stack animation has fadeout effect
 }
 
 void BattleStacksController::stacksAreAttacked(std::vector<StackAttackedInfo> attackedInfos)

client/battle/BattleStacksController.h

@@ -71,9 +71,9 @@ class BattleStacksController
 	/// for giving IDs for animations
 	ui32 animIDhelper;
 
-	bool stackNeedsAmountBox(const CStack * stack);
+	bool stackNeedsAmountBox(const CStack * stack) const;
 	void showStackAmountBox(Canvas & canvas, const CStack * stack);
-	BattleHex getStackCurrentPosition(const CStack * stack);
+	BattleHex getStackCurrentPosition(const CStack * stack) const;
 
 	std::shared_ptr<IImage> getStackAmountBox(const CStack * stack);