- Partially fixed mantis #1065 (Gate with hex 95 can't be attacked)

- Fixed 'catapult tried to attack non-catapultable hex!' problem, now catapult attacks attackable wall parts only - Fixed problem that the server performed applying damage on a wall part twice - Added methods for checking what wall parts are attackable and if a wall part is potentially attackable - Added functionality to trace net packages - Added functionality to trace std::vectors - Added tracing for CatapultAttack(CPack) - Updated various toString methods to use {} instead of [] - Refactoring
2025-10-31 00:07:39 +02:00 · 2013-12-08 17:54:13 +00:00
parent 5de70ba235
commit 69eee05ccc
13 changed files with 156 additions and 87 deletions
--- a/Global.h
+++ b/Global.h
@@ -226,14 +226,23 @@ template<typename T, size_t N> char (&_ArrayCountObj(const T (&)[N]))[N];
 /* VCMI standard library */
 /* ---------------------------------------------------------------------------- */
 template<typename T>
-std::ostream &operator<<(std::ostream &out, const boost::optional<T> &opt)
+std::ostream & operator<<(std::ostream & out, const boost::optional<T> & opt)
 {
-	if(opt)
-		return out << *opt;
-	else
-		return out<< "empty";
+	if(opt) return out << *opt;
+	else return out << "empty";
 }

+template<typename T>
+std::ostream & operator<<(std::ostream & out, const std::vector<T> & container)
+{
+	out << "[";
+	for(auto it = container.begin(); it != container.end(); ++it)
+	{
+		out << *it;
+		if(std::prev(container.end()) != it) out << ", ";
+	}
+	return out << "]";
+}

 namespace vstd
 {
--- a/client/battle/CBattleInterface.cpp
+++ b/client/battle/CBattleInterface.cpp
@@ -1156,14 +1156,12 @@ bool CBattleInterface::isTileAttackable(const BattleHex & number) const

 bool CBattleInterface::isCatapultAttackable(BattleHex hex) const
 {
-	if(!siegeH  ||  tacticsMode)
-		return false;
+	if(!siegeH || tacticsMode) return false;

-	int wallUnder = curInt->cb->battleHexToWallPart(hex);
-	if(wallUnder < 0) //invalid or indestructible
-		return false;
+	auto wallPart = curInt->cb->battleHexToWallPart(hex);
+	if(!curInt->cb->isWallPartPotentiallyAttackable(wallPart)) return false;

-	auto state = curInt->cb->battleGetWallState(wallUnder);
+	auto state = curInt->cb->battleGetWallState(static_cast<int>(wallPart));
 	return state != EWallState::DESTROYED && state != EWallState::NONE;
 }

--- a/lib/BattleAction.cpp
+++ b/lib/BattleAction.cpp
@@ -98,6 +98,6 @@ std::ostream & operator<<(std::ostream & os, const BattleAction & ba)
 	std::stringstream actionTypeStream;
 	actionTypeStream << ba.actionType;

-	return os << boost::str(boost::format("[BattleAction: side '%d', stackNumber '%d', actionType '%s', destinationTile '%s', additionalInfo '%d', selectedStack '%d']")
+	return os << boost::str(boost::format("{BattleAction: side '%d', stackNumber '%d', actionType '%s', destinationTile '%s', additionalInfo '%d', selectedStack '%d'}")
 			% static_cast<int>(ba.side) % ba.stackNumber % actionTypeStream.str() % ba.destinationTile % ba.additionalInfo % ba.selectedStack);
 }
--- a/lib/BattleHex.cpp
+++ b/lib/BattleHex.cpp
@@ -161,5 +161,5 @@ BattleHex BattleHex::getClosestTile(bool attackerOwned, BattleHex initialPos, st

 std::ostream & operator<<(std::ostream & os, const BattleHex & hex)
 {
-	return os << boost::str(boost::format("[BattleHex: x '%d', y '%d', hex '%d']") % hex.getX() % hex.getY() % hex.hex);
+	return os << boost::str(boost::format("{BattleHex: x '%d', y '%d', hex '%d'}") % hex.getX() % hex.getY() % hex.hex);
 }
--- a/lib/BattleState.cpp
+++ b/lib/BattleState.cpp
@@ -386,13 +386,13 @@ BattleInfo * BattleInfo::setupBattle( int3 tile, ETerrainType terrain, BFieldTyp

 		if (!town->hasBuilt(BuildingID::CITADEL))
 		{
-			curB->si.wallState[EWallParts::KEEP] = EWallState::NONE;
+			curB->si.wallState[EWallPart::KEEP] = EWallState::NONE;
 		}

 		if (!town->hasBuilt(BuildingID::CASTLE))
 		{
-			curB->si.wallState[EWallParts::UPPER_TOWER] = EWallState::NONE;
-			curB->si.wallState[EWallParts::BOTTOM_TOWER] = EWallState::NONE;
+			curB->si.wallState[EWallPart::UPPER_TOWER] = EWallState::NONE;
+			curB->si.wallState[EWallPart::BOTTOM_TOWER] = EWallState::NONE;
 		}
 	}

--- a/lib/BattleState.h
+++ b/lib/BattleState.h
@@ -33,7 +33,7 @@ struct BattleStackAttacked;
 //only for use in BattleInfo
 struct DLL_LINKAGE SiegeInfo
 {
-	std::array<si8, EWallParts::PARTS_COUNT> wallState;
+	std::array<si8, EWallPart::PARTS_COUNT> wallState;

 	// return EWallState decreased by value of damage points
 	static EWallState::EWallState applyDamage(EWallState::EWallState state, unsigned int value)
--- a/lib/CBattleCallback.cpp
+++ b/lib/CBattleCallback.cpp
@@ -54,38 +54,38 @@ namespace SiegeStuffThatShouldBeMovedToHandlers //  <=== TODO
 		return stackLeft != destLeft;
 	}

-	//potentially attackable parts of wall
-	static const std::pair<int, EWallParts::EWallParts> attackable[] =
+	// parts of wall
+	static const std::pair<int, EWallPart::EWallPart> wallParts[] =
 	{
-		std::make_pair(50,  EWallParts::KEEP),
-		std::make_pair(183, EWallParts::BOTTOM_TOWER),
-		std::make_pair(182, EWallParts::BOTTOM_WALL),
-		std::make_pair(130, EWallParts::BELOW_GATE),
-		std::make_pair(62,  EWallParts::OVER_GATE),
-		std::make_pair(29,  EWallParts::UPPER_WALL),
-		std::make_pair(12,  EWallParts::UPPER_TOWER),
-		std::make_pair(95,  EWallParts::GATE),
-		std::make_pair(96,  EWallParts::GATE),
-		std::make_pair(45,  EWallParts::INDESTRUCTIBLE_PART),
-		std::make_pair(78,  EWallParts::INDESTRUCTIBLE_PART),
-		std::make_pair(112, EWallParts::INDESTRUCTIBLE_PART),
-		std::make_pair(147, EWallParts::INDESTRUCTIBLE_PART)
+		std::make_pair(50,  EWallPart::KEEP),
+		std::make_pair(183, EWallPart::BOTTOM_TOWER),
+		std::make_pair(182, EWallPart::BOTTOM_WALL),
+		std::make_pair(130, EWallPart::BELOW_GATE),
+		std::make_pair(62,  EWallPart::OVER_GATE),
+		std::make_pair(29,  EWallPart::UPPER_WALL),
+		std::make_pair(12,  EWallPart::UPPER_TOWER),
+		std::make_pair(95,  EWallPart::INDESTRUCTIBLE_PART_OF_GATE),
+		std::make_pair(96,  EWallPart::GATE),
+		std::make_pair(45,  EWallPart::INDESTRUCTIBLE_PART),
+		std::make_pair(78,  EWallPart::INDESTRUCTIBLE_PART),
+		std::make_pair(112, EWallPart::INDESTRUCTIBLE_PART),
+		std::make_pair(147, EWallPart::INDESTRUCTIBLE_PART)
 	};

-	static EWallParts::EWallParts hexToWallPart(BattleHex hex)
+	static EWallPart::EWallPart hexToWallPart(BattleHex hex)
 	{
-		for(auto & elem : attackable)
+		for(auto & elem : wallParts)
 		{
 			if(elem.first == hex)
 				return elem.second;
 		}

-		return EWallParts::INVALID; //not found!
+		return EWallPart::INVALID; //not found!
 	}

-	static BattleHex WallPartToHex(EWallParts::EWallParts part)
+	static BattleHex WallPartToHex(EWallPart::EWallPart part)
 	{
-		for(auto & elem : attackable)
+		for(auto & elem : wallParts)
 		{
 			if(elem.second == part)
 				return elem.first;
@@ -425,7 +425,7 @@ si8 CBattleInfoEssentials::battleGetWallState(int partOfWall) const
 	if(getBattle()->siege == CGTownInstance::NONE)
 		return EWallState::NONE;

-	assert(partOfWall >= 0 && partOfWall < EWallParts::PARTS_COUNT);
+	assert(partOfWall >= 0 && partOfWall < EWallPart::PARTS_COUNT);
 	return getBattle()->si.wallState[partOfWall];
 }

@@ -452,8 +452,7 @@ si8 CBattleInfoCallback::battleHasWallPenalty(const IBonusBearer *bonusBearer, B
 		if (shooterPosition > destHex && ((destHex % GameConstants::BFIELD_WIDTH - shooterPosition % GameConstants::BFIELD_WIDTH) < 2)) //shooting up high
 			row -= 2;
 		const int wallPos = lineToWallHex(row);
-		if (battleHexToWallPart(wallPos) < 0) //wall still exists or is indestructible
-			return true;
+		if (!isWallPartPotentiallyAttackable(battleHexToWallPart(wallPos))) return true;
 	}

 	return false;
@@ -1094,7 +1093,7 @@ AccessibilityInfo CBattleInfoCallback::getAccesibility() const
 	}

 	//gate -> should be before stacks
-	if(battleGetSiegeLevel() > 0 && battleGetWallState(EWallParts::GATE) != EWallState::DESTROYED)
+	if(battleGetSiegeLevel() > 0 && battleGetWallState(EWallPart::GATE) != EWallState::DESTROYED)
 	{
 		ret[95] = ret[96] = EAccessibility::GATE; //block gate's hexes
 	}
@@ -1514,18 +1513,45 @@ si8 CBattleInfoCallback::battleHasDistancePenalty(const IBonusBearer *bonusBeare
 	return true;
 }

-BattleHex CBattleInfoCallback::wallPartToBattleHex(EWallParts::EWallParts part) const
+BattleHex CBattleInfoCallback::wallPartToBattleHex(EWallPart::EWallPart part) const
 {
 	RETURN_IF_NOT_BATTLE(BattleHex::INVALID);
 	return WallPartToHex(part);
 }

-EWallParts::EWallParts CBattleInfoCallback::battleHexToWallPart(BattleHex hex) const
+EWallPart::EWallPart CBattleInfoCallback::battleHexToWallPart(BattleHex hex) const
 {
-	RETURN_IF_NOT_BATTLE(EWallParts::INVALID);
+	RETURN_IF_NOT_BATTLE(EWallPart::INVALID);
 	return hexToWallPart(hex);
 }

+bool CBattleInfoCallback::isWallPartPotentiallyAttackable(EWallPart::EWallPart wallPart) const
+{
+	RETURN_IF_NOT_BATTLE(false);
+	return wallPart != EWallPart::INDESTRUCTIBLE_PART && wallPart != EWallPart::INDESTRUCTIBLE_PART_OF_GATE &&
+			wallPart != EWallPart::INVALID;
+}
+
+std::vector<BattleHex> CBattleInfoCallback::getAttackableBattleHexes() const
+{
+	std::vector<BattleHex> attackableBattleHexes;
+	RETURN_IF_NOT_BATTLE(attackableBattleHexes);
+
+	for(auto & wallPartPair : wallParts)
+	{
+		if(isWallPartPotentiallyAttackable(wallPartPair.second))
+		{
+			auto wallState = static_cast<EWallState::EWallState>(battleGetWallState(static_cast<int>(wallPartPair.second)));
+			if(wallState == EWallState::INTACT || wallState == EWallState::DAMAGED)
+			{
+				attackableBattleHexes.push_back(BattleHex(wallPartPair.first));
+			}
+		}
+	}
+
+	return attackableBattleHexes;
+}
+
 ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleIsImmune(const CGHeroInstance * caster, const CSpell * spell, ECastingMode::ECastingMode mode, BattleHex dest) const
 {
 	RETURN_IF_NOT_BATTLE(ESpellCastProblem::INVALID);
--- a/lib/CBattleCallback.h
+++ b/lib/CBattleCallback.h
@@ -170,7 +170,6 @@ public:
 	std::vector<shared_ptr<const CObstacleInstance> > battleGetAllObstacles(boost::optional<BattlePerspective::BattlePerspective> perspective = boost::none) const; //returns all obstacles on the battlefield
 	TStacks battleGetAllStacks(bool includeTurrets = false) const; //returns all stacks, alive or dead or undead or mechanical :)
 	bool battleHasNativeStack(ui8 side) const;
-	si8 battleGetWallState(int partOfWall) const; //for determining state of a part of the wall; format: parameter [0] - keep, [1] - bottom tower, [2] - bottom wall, [3] - below gate, [4] - over gate, [5] - upper wall, [6] - uppert tower, [7] - gate; returned value: 1 - intact, 2 - damaged, 3 - destroyed; 0 - no battle
 	int battleGetMoatDmg() const; //what dmg unit will suffer if ending turn in the moat
 	const CGTownInstance * battleGetDefendedTown() const; //returns defended town if current battle is a siege, nullptr instead
 	const CStack *battleActiveStack() const;
@@ -186,6 +185,10 @@ public:
 	const CArmedInstance * battleGetArmyObject(ui8 side) const; 
 	InfoAboutHero battleGetHeroInfo(ui8 side) const;

+	// for determining state of a part of the wall; format: parameter [0] - keep, [1] - bottom tower, [2] - bottom wall,
+	// [3] - below gate, [4] - over gate, [5] - upper wall, [6] - uppert tower, [7] - gate; returned value: 1 - intact, 2 - damaged, 3 - destroyed; 0 - no battle
+	si8 battleGetWallState(int partOfWall) const;
+
 	//helpers
 	TStacks battleAliveStacks() const;
 	TStacks battleAliveStacks(ui8 side) const;
@@ -251,8 +254,10 @@ public:
 	si8 battleHasWallPenalty(const CStack * stack, BattleHex destHex) const; //checks if given stack has wall penalty
 	si8 battleHasWallPenalty(const IBonusBearer *bonusBearer, BattleHex shooterPosition, BattleHex destHex) const; //checks if given stack has wall penalty

-	BattleHex wallPartToBattleHex(EWallParts::EWallParts part) const;
-	EWallParts::EWallParts battleHexToWallPart(BattleHex hex) const; //returns part of destructible wall / gate / keep under given hex or -1 if not found
+	BattleHex wallPartToBattleHex(EWallPart::EWallPart part) const;
+	EWallPart::EWallPart battleHexToWallPart(BattleHex hex) const; //returns part of destructible wall / gate / keep under given hex or -1 if not found
+	bool isWallPartPotentiallyAttackable(EWallPart::EWallPart wallPart) const; // returns true if the wall part is potentially attackable (independent of wall state), false if not
+	std::vector<BattleHex> getAttackableBattleHexes() const;

 	//*** MAGIC 
 	si8 battleMaxSpellLevel() const; //calculates minimum spell level possible to be cast on battlefield - takes into account artifacts of both heroes; if no effects are set, 0 is returned
--- a/lib/Connection.cpp
+++ b/lib/Connection.cpp
@@ -224,7 +224,7 @@ CPack * CConnection::retreivePack()
 	boost::unique_lock<boost::mutex> lock(*rmx);
    logNetwork->traceStream() << "Listening... ";
 	*this >> ret;
-    logNetwork->traceStream() << "\treceived server message of type " << typeid(*ret).name();
+	logNetwork->traceStream() << "\treceived server message of type " << typeid(*ret).name() << ", data: " << ret;
 	return ret;
 }

--- a/lib/GameConstants.h
+++ b/lib/GameConstants.h
@@ -439,13 +439,13 @@ namespace ECommander
 	const int MAX_SKILL_LEVEL = 5;
 }

-namespace EWallParts
+namespace EWallPart
 {
-	enum EWallParts
+	enum EWallPart
 	{
-		INDESTRUCTIBLE_PART = -2, INVALID = -1,
+		INDESTRUCTIBLE_PART_OF_GATE = -3, INDESTRUCTIBLE_PART = -2, INVALID = -1,
 		KEEP = 0, BOTTOM_TOWER, BOTTOM_WALL, BELOW_GATE, OVER_GATE, UPPER_WALL, UPPER_TOWER, GATE,
-		PARTS_COUNT
+		PARTS_COUNT /* This constant SHOULD always stay as the last item in the enum. */
 	};
 }

--- a/lib/NetPacks.h
+++ b/lib/NetPacks.h
@@ -48,10 +48,12 @@ struct CPack
 	{
        logNetwork->errorStream() << "CPack serialized... this should not happen!";
 	}
-	void applyGs(CGameState *gs)
-	{};
+	void applyGs(CGameState *gs) { }
+	virtual std::string toString() const { return boost::str(boost::format("{CPack: type '%d'}") % type); }
 };

+std::ostream & operator<<(std::ostream & out, const CPack * pack);
+
 struct CPackForClient : public CPack
 {
 	CPackForClient(){type = 1;};
@@ -1678,6 +1680,8 @@ struct CatapultAttack : public CPackForClient //3015
 		ui8 attackedPart;
 		ui8 damageDealt;

+		DLL_LINKAGE std::string toString() const;
+
 		template <typename Handler> void serialize(Handler &h, const int version)
 		{
 			h & destinationTile & attackedPart & damageDealt;
@@ -1688,9 +1692,9 @@ struct CatapultAttack : public CPackForClient //3015

 	DLL_LINKAGE void applyGs(CGameState *gs);
 	void applyCl(CClient *cl);
+	DLL_LINKAGE std::string toString() const override;

 	std::vector< AttackInfo > attackedParts;
-
 	int attacker; //if -1, then a spell caused this

 	template <typename Handler> void serialize(Handler &h, const int version)
@@ -1699,6 +1703,8 @@ struct CatapultAttack : public CPackForClient //3015
 	}
 };

+DLL_LINKAGE std::ostream & operator<<(std::ostream & out, const CatapultAttack::AttackInfo & attackInfo);
+
 struct BattleStacksRemoved : public CPackForClient //3016
 {
 	BattleStacksRemoved(){type = 3016;}
--- a/lib/NetPacksLib.cpp
+++ b/lib/NetPacksLib.cpp
@@ -35,6 +35,11 @@
 #undef max
 #endif

+std::ostream & operator<<(std::ostream & out, const CPack * pack)
+{
+	return out << pack->toString();
+}
+
 DLL_LINKAGE void SetResource::applyGs( CGameState *gs )
 {
 	assert(player < PlayerColor::PLAYER_LIMIT);
@@ -1508,6 +1513,22 @@ DLL_LINKAGE void CatapultAttack::applyGs( CGameState *gs )
 	}
 }

+DLL_LINKAGE std::string CatapultAttack::AttackInfo::toString() const
+{
+	return boost::str(boost::format("{AttackInfo: destinationTile '%d', attackedPart '%d', damageDealt '%d'}")
+					  % destinationTile % static_cast<int>(attackedPart) % static_cast<int>(damageDealt));
+}
+
+DLL_LINKAGE std::ostream & operator<<(std::ostream & out, const CatapultAttack::AttackInfo & attackInfo)
+{
+	return out << attackInfo.toString();
+}
+
+DLL_LINKAGE std::string CatapultAttack::toString() const
+{
+	return boost::str(boost::format("{CatapultAttack: attackedParts '%s', attacker '%d'}") % attackedParts % attacker);
+}
+
 DLL_LINKAGE void BattleStacksRemoved::applyGs( CGameState *gs )
 {
 	if(!gs->curB)
--- a/server/CGameHandler.cpp
+++ b/server/CGameHandler.cpp
@@ -3476,21 +3476,21 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 		}
 	case Battle::CATAPULT:
 		{
-			auto getCatapultHitChance = [&](EWallParts::EWallParts part, const CHeroHandler::SBallisticsLevelInfo & sbi) -> int
+			auto getCatapultHitChance = [&](EWallPart::EWallPart part, const CHeroHandler::SBallisticsLevelInfo & sbi) -> int
 			{
 				switch(part)
 				{
-				case EWallParts::GATE:
+				case EWallPart::GATE:
 					return sbi.gate;
-				case EWallParts::KEEP:
+				case EWallPart::KEEP:
 					return sbi.keep;
-				case EWallParts::BOTTOM_TOWER:
-				case EWallParts::UPPER_TOWER:
+				case EWallPart::BOTTOM_TOWER:
+				case EWallPart::UPPER_TOWER:
 					return sbi.tower;
-				case EWallParts::BOTTOM_WALL:
-				case EWallParts::BELOW_GATE:
-				case EWallParts::OVER_GATE:
-				case EWallParts::UPPER_WALL:
+				case EWallPart::BOTTOM_WALL:
+				case EWallPart::BELOW_GATE:
+				case EWallPart::OVER_GATE:
+				case EWallPart::UPPER_WALL:
 					return sbi.wall;
 				default:
 					return 0;
@@ -3504,8 +3504,8 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 			const CGHeroInstance * attackingHero = gs->curB->battleGetFightingHero(ba.side);
 			CHeroHandler::SBallisticsLevelInfo sbi = VLC->heroh->ballistics.at(attackingHero->getSecSkillLevel(SecondarySkill::BALLISTICS));

-			EWallParts::EWallParts desiredTarget = gs->curB->battleHexToWallPart(ba.destinationTile);
-			if(desiredTarget < 0)
+			auto wallPart = gs->curB->battleHexToWallPart(ba.destinationTile);
+			if(!gs->curB->isWallPartPotentiallyAttackable(wallPart))
 			{
 				complain("catapult tried to attack non-catapultable hex!");
 				break;
@@ -3514,7 +3514,7 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 			//in successive iterations damage is dealt but not yet subtracted from wall's HPs
 			auto &currentHP = gs->curB->si.wallState;

-			if (currentHP.at(desiredTarget) == EWallState::DESTROYED  ||  currentHP.at(desiredTarget) == EWallState::NONE)
+			if (currentHP.at(wallPart) == EWallState::DESTROYED  ||  currentHP.at(wallPart) == EWallState::NONE)
 			{
 				complain("catapult tried to attack already destroyed wall part!");
 				break;
@@ -3523,7 +3523,7 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 			for(int g=0; g<sbi.shots; ++g)
 			{
 				bool hitSuccessfull = false;
-				EWallParts::EWallParts attackedPart = desiredTarget;
+				auto attackedPart = wallPart;

 				do // catapult has chance to attack desired target. Othervice - attacks randomly
 				{
@@ -3535,12 +3535,12 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 					}
 					else // select new target
 					{
-						std::vector<EWallParts::EWallParts> allowedTargets;
+						std::vector<EWallPart::EWallPart> allowedTargets;
 						for (size_t i=0; i< currentHP.size(); i++)
 						{
 							if (currentHP.at(i) != EWallState::DESTROYED &&
 							    currentHP.at(i) != EWallState::NONE)
-								allowedTargets.push_back(EWallParts::EWallParts(i));
+								allowedTargets.push_back(EWallPart::EWallPart(i));
 						}
 						if (allowedTargets.empty())
 							break;
@@ -3569,32 +3569,30 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 				{
 					if(dmgRand <= dmgChance[damage])
 					{
-						currentHP[attackedPart] = SiegeInfo::applyDamage(EWallState::EWallState(currentHP.at(attackedPart)), damage);
-
 						attack.damageDealt = damage;
 						break;
 					}
 				}
 				// attacked tile may have changed - update destination
-				attack.destinationTile = gs->curB->wallPartToBattleHex(EWallParts::EWallParts(attack.attackedPart));
+				attack.destinationTile = gs->curB->wallPartToBattleHex(EWallPart::EWallPart(attack.attackedPart));

 				logGlobal->traceStream() << "Catapult attacks " << (int)attack.attackedPart
 				                         << " dealing " << (int)attack.damageDealt << " damage";

 				//removing creatures in turrets / keep if one is destroyed
-				if(attack.damageDealt > 0 && (attackedPart == EWallParts::KEEP ||
-					attackedPart == EWallParts::BOTTOM_TOWER || attackedPart == EWallParts::UPPER_TOWER))
+				if(attack.damageDealt > 0 && (attackedPart == EWallPart::KEEP ||
+					attackedPart == EWallPart::BOTTOM_TOWER || attackedPart == EWallPart::UPPER_TOWER))
 				{
 					int posRemove = -1;
 					switch(attackedPart)
 					{
-					case EWallParts::KEEP:
+					case EWallPart::KEEP:
 						posRemove = -2;
 						break;
-					case EWallParts::BOTTOM_TOWER:
+					case EWallPart::BOTTOM_TOWER:
 						posRemove = -3;
 						break;
-					case EWallParts::UPPER_TOWER:
+					case EWallPart::UPPER_TOWER:
 						posRemove = -4;
 						break;
 					}
@@ -5975,20 +5973,26 @@ void CGameHandler::runBattle()

 			if(next->getCreature()->idNumber == CreatureID::CATAPULT && (!curOwner || curOwner->getSecSkillLevel(SecondarySkill::BALLISTICS) == 0)) //catapult, hero has no ballistics
 			{
-				BattleAction attack;
-				static const int wallHexes[] = {50, 183, 182, 130, 62, 29, 12, 95};
+				const auto & attackableBattleHexes = curB.getAttackableBattleHexes();

-				attack.destinationTile = wallHexes[ rand()%ARRAY_COUNT(wallHexes) ];
-				attack.actionType = Battle::CATAPULT;
-				attack.additionalInfo = 0;
-				attack.side = !next->attackerOwned;
-				attack.stackNumber = next->ID;
+				if(!attackableBattleHexes.empty())
+				{
+					BattleAction attack;
+					attack.destinationTile = attackableBattleHexes[rand() % attackableBattleHexes.size()];
+					attack.actionType = Battle::CATAPULT;
+					attack.additionalInfo = 0;
+					attack.side = !next->attackerOwned;
+					attack.stackNumber = next->ID;

-				makeAutomaticAction(next, attack);
+					makeAutomaticAction(next, attack);
+				}
+				else
+				{
+					makeStackDoNothing(next);
+				}
 				continue;
 			}

-
 			if(next->getCreature()->idNumber == CreatureID::FIRST_AID_TENT)
 			{
 				std::vector< const CStack * > possibleStacks;