Merge pull request #6296 from mrhaandi/fix-unit-surrounded

Fix violated assert(direction != BattleHex::NONE)

client/battle/BattleActionsController.cpp

@@ -653,10 +653,11 @@ bool BattleActionsController::actionIsLegal(PossiblePlayerBattleAction action, c
 		case PossiblePlayerBattleAction::WALK_AND_ATTACK:
 		case PossiblePlayerBattleAction::ATTACK_AND_RETURN:
 			{
-				if (owner.fieldController->isTileAttackable(targetHex)) // move isTileAttackable to be part of battleCanAttack?
+				auto activeStack = owner.stacksController->getActiveStack();
+				if (targetStack && targetStack != activeStack && owner.fieldController->isTileAttackable(targetHex)) // move isTileAttackable to be part of battleCanAttack?
 				{
 					BattleHex attackFromHex = owner.fieldController->fromWhichHexAttack(targetHex);
-					if(owner.getBattle()->battleCanAttack(owner.stacksController->getActiveStack(), targetStack, attackFromHex))
+					if(owner.getBattle()->battleCanAttack(activeStack, targetStack, attackFromHex))
 						return true;
 				}
 				return false;

client/battle/BattleFieldController.cpp

@@ -819,7 +819,7 @@ bool BattleFieldController::isTileAttackable(const BattleHex & number) const
 
 	for (auto & elem : occupiableHexes)
 	{
-		if (BattleHex::mutualPosition(elem, number) != BattleHex::EDir::NONE || elem == number)
+		if (BattleHex::mutualPosition(elem, number) != BattleHex::EDir::NONE)
 			return true;
 	}
 	return false;

lib/battle/CBattleInfoCallback.cpp

@@ -280,8 +280,11 @@ std::vector<PossiblePlayerBattleAction> CBattleInfoCallback::getClientActionsFor
 		if(stack->hasBonusOfType(BonusType::RETURN_AFTER_STRIKE))
 			allowedActionList.push_back(PossiblePlayerBattleAction::ATTACK_AND_RETURN);
 
-		allowedActionList.push_back(PossiblePlayerBattleAction::ATTACK); //all active stacks can attack
-		allowedActionList.push_back(PossiblePlayerBattleAction::WALK_AND_ATTACK); //not all stacks can always walk, but we will check this elsewhere
+		if (stack->isMeleeAttacker()) //not all stacks can actually attack or walk and attack, check this elsewhere
+		{
+			allowedActionList.push_back(PossiblePlayerBattleAction::ATTACK);
+			allowedActionList.push_back(PossiblePlayerBattleAction::WALK_AND_ATTACK);
+		}
 
 		if(stack->canMove() && stack->getMovementRange(0)) //probably no reason to try move war machines or bound stacks
 			allowedActionList.push_back(PossiblePlayerBattleAction::MOVE_STACK);
@@ -683,6 +686,9 @@ bool CBattleInfoCallback::battleCanAttack(const battle::Unit * stack, const batt
 	if(!battleMatchOwner(stack, target))
 		return false;
 
+	if (!stack->isMeleeAttacker())
+		return false;
+
 	if (stack->getPosition() != dest)
 	{
 		for (const auto & obstacle : battleGetAllObstacles())
@@ -695,10 +701,6 @@ bool CBattleInfoCallback::battleCanAttack(const battle::Unit * stack, const batt
 		}
 	}
 
-	auto id = stack->unitType()->getId();
-	if (id == CreatureID::FIRST_AID_TENT || id == CreatureID::CATAPULT)
-		return false;
-
 	return target->alive();
 }
 

lib/battle/Unit.cpp

@@ -38,6 +38,16 @@ bool Unit::isTurret() const
 	return creatureIndex() == CreatureID::ARROW_TOWERS;
 }
 
+bool Unit::isMeleeAttacker() const
+{
+	//exclude war machines
+	if (hasBonusOfType(BonusType::SIEGE_WEAPON))
+		return false;
+
+	//TODO consider that a mod may introduce a melee war machine. Possibly a new bonus type NO_MELEE_ATTACK is needed.
+	return true;
+}
+
 std::string Unit::getDescription() const
 {
 	boost::format fmt("Unit %d of side %d");

lib/battle/Unit.h

@@ -97,6 +97,7 @@ public:
 	virtual bool canShootBlocked() const = 0;
 	virtual bool canShoot() const = 0;
 	virtual bool isShooter() const = 0;
+	bool isMeleeAttacker() const;
 
 	/// returns initial size of this unit
 	virtual int32_t getCount() const = 0;