From 99e5569ab5d6597e78a05d671688cd421da035c3 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Tue, 13 Dec 2022 15:10:31 +0200 Subject: [PATCH] Changes according to review --- Global.h | 12 +++--- client/battle/BattleAnimationClasses.cpp | 10 ++--- client/battle/BattleEffectsController.cpp | 2 +- client/battle/BattleInterface.cpp | 4 +- client/battle/BattleObstacleController.cpp | 2 +- client/battle/BattleProjectileController.cpp | 10 ++--- client/battle/BattleRenderer.cpp | 4 +- client/battle/BattleRenderer.h | 8 ++-- client/battle/BattleSiegeController.cpp | 6 +-- client/battle/BattleStacksController.cpp | 41 ++++++++------------ client/battle/BattleStacksController.h | 4 +- 11 files changed, 45 insertions(+), 58 deletions(-) diff --git a/Global.h b/Global.h index 0270d6942..e06f6fcfe 100644 --- a/Global.h +++ b/Global.h @@ -543,6 +543,12 @@ namespace vstd return *itr; } + template + bool erase(Container &c, const Item &item) + { + c.erase(boost::remove(c, item), c.end()); + } + template void erase_if(Range &vec, Predicate pred) { @@ -704,12 +710,6 @@ namespace vstd return false; } - template - void erase(Container &c, Pred pred) - { - c.erase(boost::remove_if(c, pred), c.end()); - } - template void removeDuplicates(std::vector &vec) { diff --git a/client/battle/BattleAnimationClasses.cpp b/client/battle/BattleAnimationClasses.cpp index d0894d447..3c41ffdca 100644 --- a/client/battle/BattleAnimationClasses.cpp +++ b/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() diff --git a/client/battle/BattleEffectsController.cpp b/client/battle/BattleEffectsController.cpp index e9e8e9336..3493ddc14 100644 --- a/client/battle/BattleEffectsController.cpp +++ b/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(floor(elem.currentFrame)); currentFrame %= elem.animation->size(); diff --git a/client/battle/BattleInterface.cpp b/client/battle/BattleInterface.cpp index 65d5e26dd..16dcec2a9 100644 --- a/client/battle/BattleInterface.cpp +++ b/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); }); diff --git a/client/battle/BattleObstacleController.cpp b/client/battle/BattleObstacleController.cpp index e1d459008..292ee393b 100644 --- a/client/battle/BattleObstacleController.cpp +++ b/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) { diff --git a/client/battle/BattleProjectileController.cpp b/client/battle/BattleProjectileController.cpp index be7f42a4b..45bf477de 100644 --- a/client/battle/BattleProjectileController.cpp +++ b/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 & projectile){ + return projectile->step > projectile->steps; + }); } bool BattleProjectileController::hasActiveProjectile(const CStack * stack) const diff --git a/client/battle/BattleRenderer.cpp b/client/battle/BattleRenderer.cpp index 3162dedab..9679d6a40 100644 --- a/client/battle/BattleRenderer.cpp +++ b/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(); diff --git a/client/battle/BattleRenderer.h b/client/battle/BattleRenderer.h index 929cf49f4..28dbee1b9 100644 --- a/client/battle/BattleRenderer.h +++ b/client/battle/BattleRenderer.h @@ -29,8 +29,8 @@ enum class EBattleFieldLayer { class BattleRenderer { public: - using RendererPtr = Canvas &; - using RenderFunctor = std::function; + using RendererRef = Canvas &; + using RenderFunctor = std::function; 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); }; diff --git a/client/battle/BattleSiegeController.cpp b/client/battle/BattleSiegeController.cpp index 1fab6209b..436cec9f2 100644 --- a/client/battle/BattleSiegeController.cpp +++ b/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()); }); diff --git a/client/battle/BattleStacksController.cpp b/client/battle/BattleStacksController.cpp index 303364f0c..0b3b15ff3 100644 --- a/client/battle/BattleStacksController.cpp +++ b/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 attackedInfos) diff --git a/client/battle/BattleStacksController.h b/client/battle/BattleStacksController.h index c1599dd89..9c14c227b 100644 --- a/client/battle/BattleStacksController.h +++ b/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 getStackAmountBox(const CStack * stack);