Battle AI: add some comments + refactoring

AI/BattleAI/AttackPossibility.cpp

@@ -13,6 +13,11 @@
                               // Eventually only IBattleInfoCallback and battle::Unit should be used, 
                               // CUnitState should be private and CStack should be removed completely
 
+uint64_t averageDmg(const TDmgRange & range)
+{
+	return (range.first + range.second) / 2;
+}
+
 AttackPossibility::AttackPossibility(BattleHex from, BattleHex dest, const BattleAttackInfo & attack)
 	: from(from), dest(dest), attack(attack)
 {
@@ -20,7 +25,7 @@ AttackPossibility::AttackPossibility(BattleHex from, BattleHex dest, const Battl
 
 int64_t AttackPossibility::damageDiff() const
 {
-	return damageDealt - damageReceived - collateralDamage + shootersBlockedDmg;
+	return defenderDamageReduce - attackerDamageReduce - collateralDamageReduce + shootersBlockedDmg;
 }
 
 int64_t AttackPossibility::attackValue() const
@@ -28,23 +33,31 @@ int64_t AttackPossibility::attackValue() const
 	return damageDiff();
 }
 
-int64_t AttackPossibility::calculateDpsReduce(
+/// <summary>
+/// How enemy damage will be reduced by this attack
+/// Half bounty for kill, half for making damage equal to enemy health
+/// Bounty - the killed creature average damage calculated against attacker
+/// </summary>
+int64_t AttackPossibility::calculateDamageReduce(
 	const battle::Unit * attacker,
 	const battle::Unit * defender,
 	uint64_t damageDealt,
-	std::shared_ptr<CBattleInfoCallback> cb)
+	const CBattleInfoCallback & cb)
 {
+	const float HEALTH_BOUNTY = 0.5;
+	const float KILL_BOUNTY = 1.0 - HEALTH_BOUNTY;
+
 	vstd::amin(damageDealt, defender->getAvailableHealth());
 
-	auto enemyDamageBeforeAttack = cb->battleEstimateDamage(BattleAttackInfo(defender, attacker, defender->canShoot()));
+	auto enemyDamageBeforeAttack = cb.battleEstimateDamage(BattleAttackInfo(defender, attacker, defender->canShoot()));
 	auto enemiesKilled = damageDealt / defender->MaxHealth() + (damageDealt % defender->MaxHealth() >= defender->getFirstHPleft() ? 1 : 0);
-	auto enemyDps = (enemyDamageBeforeAttack.first + enemyDamageBeforeAttack.second) / 2;
+	auto enemyDamage = averageDmg(enemyDamageBeforeAttack);
-	auto dpsPerEnemy = enemyDps / (double)defender->getCount();
+	auto damagePerEnemy = enemyDamage / (double)defender->getCount();
 
-	return (int64_t)(dpsPerEnemy * (enemiesKilled + damageDealt / (double)defender->MaxHealth()) / 2);
+	return (int64_t)(damagePerEnemy * (enemiesKilled * KILL_BOUNTY + damageDealt * HEALTH_BOUNTY / (double)defender->MaxHealth()));
 }
 
-int64_t AttackPossibility::evaluateBlockedShootersDmg(const BattleAttackInfo & attackInfo, BattleHex hex, const HypotheticBattle * state)
+int64_t AttackPossibility::evaluateBlockedShootersDmg(const BattleAttackInfo & attackInfo, BattleHex hex, const HypotheticBattle & state)
 {
 	int64_t res = 0;
 
@@ -55,10 +68,10 @@ int64_t AttackPossibility::evaluateBlockedShootersDmg(const BattleAttackInfo & a
 	auto hexes = attacker->getSurroundingHexes(hex);
 	for(BattleHex tile : hexes)
 	{
-		auto st = state->battleGetUnitByPos(tile, true);
+		auto st = state.battleGetUnitByPos(tile, true);
-		if(!st || !state->battleMatchOwner(st, attacker))
+		if(!st || !state.battleMatchOwner(st, attacker))
 			continue;
-		if(!state->battleCanShoot(st))
+		if(!state.battleCanShoot(st))
 			continue;
 
 		BattleAttackInfo rangeAttackInfo(st, attacker, true);
@@ -67,23 +80,23 @@ int64_t AttackPossibility::evaluateBlockedShootersDmg(const BattleAttackInfo & a
 		BattleAttackInfo meleeAttackInfo(st, attacker, false);
 		meleeAttackInfo.defenderPos = hex;
 
-		auto rangeDmg = getCbc()->battleEstimateDamage(rangeAttackInfo);
+		auto rangeDmg = state.battleEstimateDamage(rangeAttackInfo);
-		auto meleeDmg = getCbc()->battleEstimateDamage(meleeAttackInfo);
+		auto meleeDmg = state.battleEstimateDamage(meleeAttackInfo);
 
-		int64_t gain = (rangeDmg.first + rangeDmg.second - meleeDmg.first - meleeDmg.second) / 2 + 1;
+		int64_t gain = averageDmg(rangeDmg) - averageDmg(meleeDmg) + 1;
 		res += gain;
 	}
 
 	return res;
 }
 
-AttackPossibility AttackPossibility::evaluate(const BattleAttackInfo & attackInfo, BattleHex hex, const HypotheticBattle * state)
+AttackPossibility AttackPossibility::evaluate(const BattleAttackInfo & attackInfo, BattleHex hex, const HypotheticBattle & state)
 {
 	auto attacker = attackInfo.attacker;
 	auto defender = attackInfo.defender;
 	const std::string cachingStringBlocksRetaliation = "type_BLOCKS_RETALIATION";
 	static const auto selectorBlocksRetaliation = Selector::type()(Bonus::BLOCKS_RETALIATION);
-	const auto attackerSide = getCbc()->playerToSide(getCbc()->battleGetOwner(attacker));
+	const auto attackerSide = state.playerToSide(state.battleGetOwner(attacker));
 	const bool counterAttacksBlocked = attacker->hasBonus(selectorBlocksRetaliation, cachingStringBlocksRetaliation);
 
 	AttackPossibility bestAp(hex, BattleHex::INVALID, attackInfo);
@@ -111,9 +124,9 @@ AttackPossibility AttackPossibility::evaluate(const BattleAttackInfo & attackInf
 		std::vector<const battle::Unit*> units;
 
 		if (attackInfo.shooting)
-			units = state->getAttackedBattleUnits(attacker, defHex, true, BattleHex::INVALID);
+			units = state.getAttackedBattleUnits(attacker, defHex, true, BattleHex::INVALID);
 		else
-			units = state->getAttackedBattleUnits(attacker, defHex, false, hex);
+			units = state.getAttackedBattleUnits(attacker, defHex, false, hex);
 
 		// ensure the defender is also affected
 		bool addDefender = true;
@@ -139,11 +152,11 @@ AttackPossibility AttackPossibility::evaluate(const BattleAttackInfo & attackInf
 
 			for(int i = 0; i < totalAttacks; i++)
 			{
-				int64_t damageDealt, damageReceived, enemyDpsReduce, ourDpsReduce;
+				int64_t damageDealt, damageReceived, defenderDamageReduce, attackerDamageReduce;
 
 				TDmgRange retaliation(0, 0);
-				auto attackDmg = getCbc()->battleEstimateDamage(ap.attack, &retaliation);
+				auto attackDmg = state.battleEstimateDamage(ap.attack, &retaliation);
-				TDmgRange enemyDamageBeforeAttack = getCbc()->battleEstimateDamage(BattleAttackInfo(u, attacker, u->canShoot()));
+				TDmgRange defenderDamageBeforeAttack = state.battleEstimateDamage(BattleAttackInfo(u, attacker, u->canShoot()));
 
 				vstd::amin(attackDmg.first, defenderState->getAvailableHealth());
 				vstd::amin(attackDmg.second, defenderState->getAvailableHealth());
@@ -151,36 +164,36 @@ AttackPossibility AttackPossibility::evaluate(const BattleAttackInfo & attackInf
 				vstd::amin(retaliation.first, ap.attackerState->getAvailableHealth());
 				vstd::amin(retaliation.second, ap.attackerState->getAvailableHealth());
 
-				damageDealt = (attackDmg.first + attackDmg.second) / 2;
+				damageDealt = averageDmg(attackDmg);
-				enemyDpsReduce = calculateDpsReduce(attacker, defender, damageDealt, getCbc());
+				defenderDamageReduce = calculateDamageReduce(attacker, defender, damageDealt, state);
 				ap.attackerState->afterAttack(attackInfo.shooting, false);
 
 				//FIXME: use ranged retaliation
 				damageReceived = 0;
-				ourDpsReduce = 0;
+				attackerDamageReduce = 0;
 
 				if (!attackInfo.shooting && defenderState->ableToRetaliate() && !counterAttacksBlocked)
 				{
-					damageReceived = (retaliation.first + retaliation.second) / 2;
+					damageReceived = averageDmg(retaliation);
-					ourDpsReduce = calculateDpsReduce(defender, attacker, damageReceived, getCbc());
+					attackerDamageReduce = calculateDamageReduce(defender, attacker, damageReceived, state);
 					defenderState->afterAttack(attackInfo.shooting, true);
 				}
 
-				bool isEnemy = state->battleMatchOwner(attacker, u);
+				bool isEnemy = state.battleMatchOwner(attacker, u);
 
 				// this includes enemy units as well as attacker units under enemy's mind control
 				if(isEnemy)
-					ap.damageDealt += enemyDpsReduce;
+					ap.defenderDamageReduce += defenderDamageReduce;
 
 				// damaging attacker's units (even those under enemy's mind control) is considered friendly fire
 				if(attackerSide == u->unitSide())
-					ap.collateralDamage += enemyDpsReduce;
+					ap.collateralDamageReduce += defenderDamageReduce;
 
 				if(u->unitId() == defender->unitId() || 
 					(!attackInfo.shooting && CStack::isMeleeAttackPossible(u, attacker, hex)))
 				{
 					//FIXME: handle RANGED_RETALIATION ?
-					ap.damageReceived += ourDpsReduce;
+					ap.attackerDamageReduce += attackerDamageReduce;
 				}
 
 				ap.attackerState->damage(damageReceived);
@@ -198,11 +211,11 @@ AttackPossibility AttackPossibility::evaluate(const BattleAttackInfo & attackInf
 	// check how much damage we gain from blocking enemy shooters on this hex
 	bestAp.shootersBlockedDmg = evaluateBlockedShootersDmg(attackInfo, hex, state);
 
-	logAi->debug("BattleAI best AP: %s -> %s at %d from %d, affects %d units: %lld %lld %lld %lld",
+	logAi->debug("BattleAI best AP: %s -> %s at %d from %d, affects %d units: d:%lld a:%lld c:%lld s:%lld",
 		attackInfo.attacker->unitType()->identifier,
 		attackInfo.defender->unitType()->identifier,
 		(int)bestAp.dest, (int)bestAp.from, (int)bestAp.affectedUnits.size(),
-		bestAp.damageDealt, bestAp.damageReceived, bestAp.collateralDamage, bestAp.shootersBlockedDmg);
+		bestAp.defenderDamageReduce, bestAp.attackerDamageReduce, bestAp.collateralDamageReduce, bestAp.shootersBlockedDmg);
 
 	//TODO other damage related to attack (eg. fire shield and other abilities)
 	return bestAp;

AI/BattleAI/AttackPossibility.h

@@ -15,6 +15,10 @@
 
 #define BATTLE_TRACE_LEVEL 0
 
+/// <summary>
+/// Evaluate attack value of one particular attack taking into account various effects like
+/// retaliation, 2-hex breath, collateral damage, shooters blocked damage
+/// </summary>
 class AttackPossibility
 {
 public:
@@ -26,9 +30,9 @@ public:
 
 	std::vector<std::shared_ptr<battle::CUnitState>> affectedUnits;
 
-	int64_t damageDealt = 0;
+	int64_t defenderDamageReduce = 0;
-	int64_t damageReceived = 0; //usually by counter-attack
+	int64_t attackerDamageReduce = 0; //usually by counter-attack
-	int64_t collateralDamage = 0; // friendly fire (usually by two-hex attacks)
+	int64_t collateralDamageReduce = 0; // friendly fire (usually by two-hex attacks)
 	int64_t shootersBlockedDmg = 0;
 
 	AttackPossibility(BattleHex from, BattleHex dest, const BattleAttackInfo & attack_);
@@ -36,14 +40,14 @@ public:
 	int64_t damageDiff() const;
 	int64_t attackValue() const;
 
-	static AttackPossibility evaluate(const BattleAttackInfo & attackInfo, BattleHex hex, const HypotheticBattle * state);
+	static AttackPossibility evaluate(const BattleAttackInfo & attackInfo, BattleHex hex, const HypotheticBattle & state);
 
-	static int64_t calculateDpsReduce(
+	static int64_t calculateDamageReduce(
 		const battle::Unit * attacker,
 		const battle::Unit * defender,
 		uint64_t damageDealt,
-		std::shared_ptr<CBattleInfoCallback> cb);
+		const CBattleInfoCallback & cb);
 
 private:
-	static int64_t evaluateBlockedShootersDmg(const BattleAttackInfo & attackInfo, BattleHex hex, const HypotheticBattle * state);
+	static int64_t evaluateBlockedShootersDmg(const BattleAttackInfo & attackInfo, BattleHex hex, const HypotheticBattle & state);
 };

AI/BattleAI/BattleAI.cpp

@@ -161,9 +161,8 @@ BattleAction CBattleAI::activeStack( const CStack * stack )
 		}
 
 		HypotheticBattle hb(env.get(), cb);
-		int turn = 0;
 		
-		PotentialTargets targets(stack, &hb);
+		PotentialTargets targets(stack, hb);
 		BattleExchangeEvaluator scoreEvaluator(cb, env);
 		auto moveTarget = scoreEvaluator.findMoveTowardsUnreachable(stack, targets, hb);
 
@@ -171,7 +170,7 @@ BattleAction CBattleAI::activeStack( const CStack * stack )
 
 		if(!targets.possibleAttacks.empty())
 		{
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
 			logAi->trace("Evaluating attack for %s", stack->getDescription());
 #endif
 
@@ -205,15 +204,15 @@ BattleAction CBattleAI::activeStack( const CStack * stack )
 				else
 				{
 					result = BattleAction::makeMeleeAttack(stack, bestAttack.attack.defender->getPosition(), bestAttack.from);
-					action = "mellee";
+					action = "melee";
 				}
 
-				logAi->debug("BattleAI: %s -> %s x %d, %s, from %d curpos %d dist %d speed %d: %lld %lld %lld",
+				logAi->debug("BattleAI: %s -> %s x %d, %s, from %d curpos %d dist %d speed %d: +%lld -%lld = %lld",
 					bestAttack.attackerState->unitType()->identifier,
 					bestAttack.affectedUnits[0]->unitType()->identifier,
 					(int)bestAttack.affectedUnits[0]->getCount(), action, (int)bestAttack.from, (int)bestAttack.attack.attacker->getPosition().hex,
 					bestAttack.attack.chargedFields, bestAttack.attack.attacker->Speed(0, true),
-					bestAttack.damageDealt, bestAttack.damageReceived, bestAttack.attackValue()
+					bestAttack.defenderDamageReduce, bestAttack.attackerDamageReduce, bestAttack.attackValue()
 				);
 			}
 		}
@@ -323,12 +322,15 @@ BattleAction CBattleAI::goTowardsNearest(const CStack * stack, std::vector<Battl
 		// We just check all available hexes and pick the one closest to the target.
 		auto nearestAvailableHex = vstd::minElementByFun(avHexes, [&](BattleHex hex) -> int
 		{
+			const int MOAT_PENALTY = 100; // avoid landing on moat
+			const int BLOCKED_STACK_PENALTY = 100; // avoid landing on moat
+
 			auto distance = BattleHex::getDistance(bestNeighbor, hex);
 
 			if(vstd::contains(moatHexes, hex))
-				distance += 100;
+				distance += MOAT_PENALTY;
 
-			return scoreEvaluator.checkPositionBlocksOurStacks(hb, stack, hex) ? 100 + distance : distance;
+			return scoreEvaluator.checkPositionBlocksOurStacks(hb, stack, hex) ? BLOCKED_STACK_PENALTY + distance : distance;
 		});
 
 		return BattleAction::makeMove(stack, *nearestAvailableHex);
@@ -444,7 +446,7 @@ void CBattleAI::attemptCastingSpell()
 
 	using ValueMap = PossibleSpellcast::ValueMap;
 
-	auto evaluateQueue = [&](ValueMap & values, const std::vector<battle::Units> & queue, HypotheticBattle * state, size_t minTurnSpan, bool * enemyHadTurnOut) -> bool
+	auto evaluateQueue = [&](ValueMap & values, const std::vector<battle::Units> & queue, HypotheticBattle & state, size_t minTurnSpan, bool * enemyHadTurnOut) -> bool
 	{
 		bool firstRound = true;
 		bool enemyHadTurn = false;
@@ -455,7 +457,7 @@ void CBattleAI::attemptCastingSpell()
 		for(auto & round : queue)
 		{
 			if(!firstRound)
-				state->nextRound(0);//todo: set actual value?
+				state.nextRound(0);//todo: set actual value?
 			for(auto unit : round)
 			{
 				if(!vstd::contains(values, unit->unitId()))
@@ -464,11 +466,11 @@ void CBattleAI::attemptCastingSpell()
 				if(!unit->alive())
 					continue;
 
-				if(state->battleGetOwner(unit) != playerID)
+				if(state.battleGetOwner(unit) != playerID)
 				{
 					enemyHadTurn = true;
 
-					if(!firstRound || state->battleCastSpells(unit->unitSide()) == 0)
+					if(!firstRound || state.battleCastSpells(unit->unitSide()) == 0)
 					{
 						//enemy could counter our spell at this point
 						//anyway, we do not know what enemy will do
@@ -482,7 +484,7 @@ void CBattleAI::attemptCastingSpell()
 					ourTurnSpan++;
 				}
 
-				state->nextTurn(unit->unitId());
+				state.nextTurn(unit->unitId());
 
 				PotentialTargets pt(unit, state);
 
@@ -490,22 +492,22 @@ void CBattleAI::attemptCastingSpell()
 				{
 					AttackPossibility ap = pt.bestAction();
 
-					auto swb = state->getForUpdate(unit->unitId());
+					auto swb = state.getForUpdate(unit->unitId());
 					*swb = *ap.attackerState;
 
-					if(ap.damageDealt > 0)
+					if(ap.defenderDamageReduce > 0)
 						swb->removeUnitBonus(Bonus::UntilAttack);
-					if(ap.damageReceived > 0)
+					if(ap.attackerDamageReduce > 0)
 						swb->removeUnitBonus(Bonus::UntilBeingAttacked);
 
 					for(auto affected : ap.affectedUnits)
 					{
-						swb = state->getForUpdate(affected->unitId());
+						swb = state.getForUpdate(affected->unitId());
 						*swb = *affected;
 
-						if(ap.damageDealt > 0)
+						if(ap.defenderDamageReduce > 0)
 							swb->removeUnitBonus(Bonus::UntilBeingAttacked);
-						if(ap.damageReceived > 0 && ap.attack.defender->unitId() == affected->unitId())
+						if(ap.attackerDamageReduce > 0 && ap.attack.defender->unitId() == affected->unitId())
 							swb->removeUnitBonus(Bonus::UntilAttack);
 					}
 				}
@@ -513,7 +515,7 @@ void CBattleAI::attemptCastingSpell()
 				auto bav = pt.bestActionValue();
 
 				//best action is from effective owner`s point if view, we need to convert to our point if view
-				if(state->battleGetOwner(unit) != playerID)
+				if(state.battleGetOwner(unit) != playerID)
 					bav = -bav;
 				values[unit->unitId()] += bav;
 			}
@@ -566,7 +568,7 @@ void CBattleAI::attemptCastingSpell()
 
 		HypotheticBattle state(env.get(), cb);
 
-		evaluateQueue(valueOfStack, turnOrder, &state, 0, &enemyHadTurn);
+		evaluateQueue(valueOfStack, turnOrder, state, 0, &enemyHadTurn);
 
 		if(!enemyHadTurn)
 		{
@@ -614,7 +616,7 @@ void CBattleAI::attemptCastingSpell()
 
 		state.battleGetTurnOrder(newTurnOrder, amount, 2);
 
-		const bool turnSpanOK = evaluateQueue(newValueOfStack, newTurnOrder, &state, minTurnSpan, nullptr);
+		const bool turnSpanOK = evaluateQueue(newValueOfStack, newTurnOrder, state, minTurnSpan, nullptr);
 
 		if(turnSpanOK || castNow)
 		{

AI/BattleAI/BattleExchangeVariant.cpp

@@ -18,12 +18,12 @@ AttackerValue::AttackerValue()
 }
 
 MoveTarget::MoveTarget()
-	:positions()
+	: positions()
 {
 	score = EvaluationResult::INEFFECTIVE_SCORE;
 }
 
-int64_t BattleExchangeVariant::trackAttack(const AttackPossibility & ap, HypotheticBattle * state)
+int64_t BattleExchangeVariant::trackAttack(const AttackPossibility & ap, HypotheticBattle & state)
 {
 	auto affectedUnits = ap.affectedUnits;
 
@@ -31,7 +31,7 @@ int64_t BattleExchangeVariant::trackAttack(const AttackPossibility & ap, Hypothe
 
 	for(auto affectedUnit : affectedUnits)
 	{
-		auto unitToUpdate = state->getForUpdate(affectedUnit->unitId());
+		auto unitToUpdate = state.getForUpdate(affectedUnit->unitId());
 
 		unitToUpdate->health = affectedUnit->health;
 		unitToUpdate->shots = affectedUnit->shots;
@@ -43,9 +43,9 @@ int64_t BattleExchangeVariant::trackAttack(const AttackPossibility & ap, Hypothe
 
 	dpsScore += attackValue;
 
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
 	logAi->trace(
-		"%s -> %s, ap attack, %s, dps: %d, score: %d",
+		"%s -> %s, ap attack, %s, dps: %lld, score: %lld",
 		ap.attack.attacker->getDescription(),
 		ap.attack.defender->getDescription(),
 		ap.attack.shooting ? "shot" : "mellee",
@@ -61,14 +61,14 @@ int64_t BattleExchangeVariant::trackAttack(
 	std::shared_ptr<StackWithBonuses> defender,
 	bool shooting,
 	bool isOurAttack,
-	std::shared_ptr<CBattleInfoCallback> cb,
+	const CBattleInfoCallback & cb,
 	bool evaluateOnly)
 {
 	const std::string cachingStringBlocksRetaliation = "type_BLOCKS_RETALIATION";
 	static const auto selectorBlocksRetaliation = Selector::type()(Bonus::BLOCKS_RETALIATION);
 	const bool counterAttacksBlocked = attacker->hasBonus(selectorBlocksRetaliation, cachingStringBlocksRetaliation);
 
-	TDmgRange retalitation;
+	TDmgRange retaliation;
 	BattleAttackInfo bai(attacker.get(), defender.get(), shooting);
 
 	if(shooting)
@@ -76,30 +76,30 @@ int64_t BattleExchangeVariant::trackAttack(
 		bai.attackerPos.setXY(8, 5);
 	}
 
-	auto attack = cb->battleEstimateDamage(bai, &retalitation);
+	auto attack = cb.battleEstimateDamage(bai, &retaliation);
 	int64_t attackDamage = (attack.first + attack.second) / 2;
-	int64_t defenderDpsReduce = AttackPossibility::calculateDpsReduce(attacker.get(), defender.get(), attackDamage, cb);
+	int64_t defenderDamageReduce = AttackPossibility::calculateDamageReduce(attacker.get(), defender.get(), attackDamage, cb);
-	int64_t attackerDpsReduce = 0;
+	int64_t attackerDamageReduce = 0;
 
 	if(!evaluateOnly)
 	{
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
 		logAi->trace(
-			"%s -> %s, normal attack, %s, dps: %d, %d",
+			"%s -> %s, normal attack, %s, dps: %lld, %lld",
 			attacker->getDescription(),
 			defender->getDescription(),
 			shooting ? "shot" : "mellee",
 			attackDamage,
-			defenderDpsReduce);
+			defenderDamageReduce);
 #endif
 
 		if(isOurAttack)
 		{
-			dpsScore += defenderDpsReduce;
+			dpsScore += defenderDamageReduce;
-			attackerValue[attacker->unitId()].value += defenderDpsReduce;
+			attackerValue[attacker->unitId()].value += defenderDamageReduce;
 		}
 		else
-			dpsScore -= defenderDpsReduce;
+			dpsScore -= defenderDamageReduce;
 
 		defender->damage(attackDamage);
 		attacker->afterAttack(shooting, false);
@@ -107,45 +107,45 @@ int64_t BattleExchangeVariant::trackAttack(
 
 	if(defender->alive() && defender->ableToRetaliate() && !counterAttacksBlocked && !shooting)
 	{
-		if(retalitation.second != 0)
+		if(retaliation.second != 0)
 		{
-			auto retalitationDamage = (retalitation.first + retalitation.second) / 2;
+			auto retaliationDamage = (retaliation.first + retaliation.second) / 2;
-			attackerDpsReduce = AttackPossibility::calculateDpsReduce(defender.get(), attacker.get(), retalitationDamage, cb);
+			attackerDamageReduce = AttackPossibility::calculateDamageReduce(defender.get(), attacker.get(), retaliationDamage, cb);
 
 			if(!evaluateOnly)
 			{
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
 				logAi->trace(
-					"%s -> %s, retalitation, dps: %d, %d",
+					"%s -> %s, retaliation, dps: %lld, %lld",
 					defender->getDescription(),
 					attacker->getDescription(),
-					retalitationDamage,
+					retaliationDamage,
-					attackerDpsReduce);
+					attackerDamageReduce);
 #endif
 
 				if(isOurAttack)
 				{
-					dpsScore -= attackerDpsReduce;
+					dpsScore -= attackerDamageReduce;
 					attackerValue[attacker->unitId()].isRetalitated = true;
 				}
 				else
 				{
-					dpsScore += attackerDpsReduce;
+					dpsScore += attackerDamageReduce;
-					attackerValue[defender->unitId()].value += attackerDpsReduce;
+					attackerValue[defender->unitId()].value += attackerDamageReduce;
 				}
 
-				attacker->damage(retalitationDamage);
+				attacker->damage(retaliationDamage);
 				defender->afterAttack(false, true);
 			}
 		}
 	}
 
-	auto score = defenderDpsReduce - attackerDpsReduce;
+	auto score = defenderDamageReduce - attackerDamageReduce;
 
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
 	if(!score)
 	{
-		logAi->trace("Zero %d %d", defenderDpsReduce, attackerDpsReduce);
+		logAi->trace("Attack has zero score d:%lld a:%lld", defenderDamageReduce, attackerDamageReduce);
 	}
 #endif
 
@@ -171,7 +171,7 @@ EvaluationResult BattleExchangeEvaluator::findBestTarget(const battle::Unit * ac
 
 	if(!activeStack->waited())
 	{
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
 		logAi->trace("Evaluating waited attack for %s", activeStack->getDescription());
 #endif
 
@@ -233,7 +233,7 @@ MoveTarget BattleExchangeEvaluator::findMoveTowardsUnreachable(const battle::Uni
 		for(auto hex : hexes)
 		{
 			auto bai = BattleAttackInfo(activeStack, closestStack, cb->battleCanShoot(activeStack));
-			auto attack = AttackPossibility::evaluate(bai, hex, &hb);
+			auto attack = AttackPossibility::evaluate(bai, hex, hb);
 
 			attack.shootersBlockedDmg = 0; // we do not want to count on it, it is not for sure
 
@@ -323,7 +323,7 @@ std::vector<const battle::Unit *> BattleExchangeEvaluator::getExchangeUnits(
 
 	if(allReachableUnits.size() < 2)
 	{
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
 		logAi->trace("Reachability map contains only %d stacks", allReachableUnits.size());
 #endif
 
@@ -347,8 +347,8 @@ int64_t BattleExchangeEvaluator::calculateExchange(
 	PotentialTargets & targets,
 	HypotheticBattle & hb)
 {
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
-	logAi->trace("Battle exchange at %d", ap.attack.shooting ? ap.dest : ap.from);
+	logAi->trace("Battle exchange at %lld", ap.attack.shooting ? ap.dest : ap.from);
 #endif
 
 	std::vector<const battle::Unit *> ourStacks;
@@ -396,7 +396,7 @@ int64_t BattleExchangeEvaluator::calculateExchange(
 
 		if(!attacker->alive())
 		{
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
 			logAi->trace(	"Attacker is dead");
 #endif
 
@@ -415,11 +415,11 @@ int64_t BattleExchangeEvaluator::calculateExchange(
 					stackWithBonuses,
 					exchangeBattle.battleCanShoot(stackWithBonuses.get()),
 					isOur,
-					cb,
+					*cb,
 					true);
 
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
-				logAi->trace("Best target selector %s->%s score = %d", attacker->getDescription(), u->getDescription(), score);
+				logAi->trace("Best target selector %s->%s score = %lld", attacker->getDescription(), u->getDescription(), score);
 #endif
 
 				return score;
@@ -448,7 +448,7 @@ int64_t BattleExchangeEvaluator::calculateExchange(
 				}
 				else
 				{
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
 					logAi->trace("Battle queue is empty and no reachable enemy.");
 #endif
 
@@ -463,13 +463,13 @@ int64_t BattleExchangeEvaluator::calculateExchange(
 
 		if(canUseAp && activeUnit == ap.attack.attacker && targetUnit == ap.attack.defender)
 		{
-			v.trackAttack(ap, &exchangeBattle);
+			v.trackAttack(ap, exchangeBattle);
 		}
 		else
 		{
 			for(int i = 0; i < totalAttacks; i++)
 			{
-				v.trackAttack(attacker, defender, shooting, isOur, cb);
+				v.trackAttack(attacker, defender, shooting, isOur, exchangeBattle);
 
 				if(!attacker->alive() || !defender->alive())
 					break;
@@ -489,10 +489,12 @@ int64_t BattleExchangeEvaluator::calculateExchange(
 			});
 	}
 
+	// avoid blocking path for stronger stack by weaker stack
+	// the method checks if all stacks can be placed around enemy
 	v.adjustPositions(melleeAttackers, ap, reachabilityMap);
 
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
-	logAi->trace("Exchange score: %ld", v.getScore());
+	logAi->trace("Exchange score: %lld", v.getScore());
 #endif
 
 	return v.getScore();
@@ -522,7 +524,7 @@ void BattleExchangeVariant::adjustPositions(
 		vstd::erase_if_present(hexes, ap.attack.attacker->occupiedHex(ap.attack.attackerPos));
 	}
 
-	int64_t notRealizedDps = 0;
+	int64_t notRealizedDamage = 0;
 
 	for(auto unit : attackers)
 	{
@@ -534,7 +536,7 @@ void BattleExchangeVariant::adjustPositions(
 				return vstd::contains(reachabilityMap[h], unit);
 			}))
 		{
-			notRealizedDps += attackerValue[unit->unitId()].value;
+			notRealizedDamage += attackerValue[unit->unitId()].value;
 			continue;
 		}
 
@@ -542,7 +544,7 @@ void BattleExchangeVariant::adjustPositions(
 			{
 				auto score = vstd::contains(reachabilityMap[h], unit)
 					? reachabilityMap[h].size()
-					: 1000;
+					: 0;
 
 				if(unit->doubleWide())
 				{
@@ -558,7 +560,7 @@ void BattleExchangeVariant::adjustPositions(
 		hexes.erase(desiredPosition);
 	}
 
-	if(notRealizedDps > ap.attackValue() && notRealizedDps > attackerValue[ap.attack.attacker->unitId()].value)
+	if(notRealizedDamage > ap.attackValue() && notRealizedDamage > attackerValue[ap.attack.attacker->unitId()].value)
 	{
 		dpsScore = EvaluationResult::INEFFECTIVE_SCORE;
 	}
@@ -566,9 +568,11 @@ void BattleExchangeVariant::adjustPositions(
 
 void BattleExchangeEvaluator::updateReachabilityMap(HypotheticBattle & hb)
 {
+	const int TURN_DEPTH = 2;
+
 	turnOrder.clear();
 	
-	hb.battleGetTurnOrder(turnOrder, 1000, 2);
+	hb.battleGetTurnOrder(turnOrder, std::numeric_limits<int>::max(), TURN_DEPTH);
 	reachabilityMap.clear();
 
 	for(int turn = 0; turn < turnOrder.size(); turn++)
@@ -618,9 +622,14 @@ void BattleExchangeEvaluator::updateReachabilityMap(HypotheticBattle & hb)
 	}
 }
 
+// avoid blocking path for stronger stack by weaker stack
 bool BattleExchangeEvaluator::checkPositionBlocksOurStacks(HypotheticBattle & hb, const battle::Unit * activeUnit, BattleHex position)
 {
-	int blockingScore = 0;
+	const int BLOCKING_THRESHOLD = 70;
+	const int BLOCKING_OWN_ATTACK_PENALTY = 100;
+	const int BLOCKING_OWN_MOVE_PENALTY = 1;
+
+	float blockingScore = 0;
 
 	auto activeUnitDamage = activeUnit->getMinDamage(hb.battleCanShoot(activeUnit)) * activeUnit->getCount();
 
@@ -638,9 +647,7 @@ bool BattleExchangeEvaluator::checkPositionBlocksOurStacks(HypotheticBattle & hb
 				continue;
 
 			auto blockedUnitDamage = unit->getMinDamage(hb.battleCanShoot(unit)) * unit->getCount();
-
+			auto ratio = blockedUnitDamage / (blockedUnitDamage + activeUnitDamage);
-			if(blockedUnitDamage < activeUnitDamage)
-				continue;
 
 			auto unitReachability = turnBattle.getReachability(unit);
 
@@ -668,15 +675,15 @@ bool BattleExchangeEvaluator::checkPositionBlocksOurStacks(HypotheticBattle & hb
 
 				if(!reachable && vstd::contains(reachabilityMap[hex], unit))
 				{
-					blockingScore += enemyUnit ? 100 : 1;
+					blockingScore += ratio * (enemyUnit ? BLOCKING_OWN_ATTACK_PENALTY : BLOCKING_OWN_MOVE_PENALTY);
 				}
 			}
 		}
 	}
 
-#if BATTLE_TRACE_LEVEL==1
+#if BATTLE_TRACE_LEVEL>=1
-	logAi->trace("Position %d, blocking score %d", position.hex, blockingScore);
+	logAi->trace("Position %d, blocking score %f", position.hex, blockingScore);
 #endif
 
-	return blockingScore > 50;
+	return blockingScore > BLOCKING_THRESHOLD;
 }

AI/BattleAI/BattleExchangeVariant.h

@@ -47,6 +47,12 @@ struct EvaluationResult
 	}
 };
 
+/// <summary>
+/// The class represents evaluation of attack value
+/// of exchanges between all stacks which can access particular hex
+/// starting from initial attack represented by AttackPossibility and further according turn order.
+/// Negative score value means we get more demage than deal
+/// </summary>
 class BattleExchangeVariant
 {
 public:
@@ -55,14 +61,14 @@ public:
 	{
 	}
 
-	int64_t trackAttack(const AttackPossibility & ap, HypotheticBattle * state);
+	int64_t trackAttack(const AttackPossibility & ap, HypotheticBattle & state);
 
 	int64_t trackAttack(
 		std::shared_ptr<StackWithBonuses> attacker,
 		std::shared_ptr<StackWithBonuses> defender,
 		bool shooting,
 		bool isOurAttack,
-		std::shared_ptr<CBattleInfoCallback> cb,
+		const CBattleInfoCallback & cb,
 		bool evaluateOnly = false);
 
 	int64_t getScore() const { return dpsScore; }

AI/BattleAI/PotentialTargets.cpp

@@ -11,11 +11,11 @@
 #include "PotentialTargets.h"
 #include "../../lib/CStack.h"//todo: remove
 
-PotentialTargets::PotentialTargets(const battle::Unit * attacker, const HypotheticBattle * state)
+PotentialTargets::PotentialTargets(const battle::Unit * attacker, const HypotheticBattle & state)
 {
-	auto attackerInfo = state->battleGetUnitByID(attacker->unitId());
+	auto attackerInfo = state.battleGetUnitByID(attacker->unitId());
-	auto reachability = state->getReachability(attackerInfo);
+	auto reachability = state.getReachability(attackerInfo);
-	auto avHexes = state->battleGetAvailableHexes(reachability, attackerInfo);
+	auto avHexes = state.battleGetAvailableHexes(reachability, attackerInfo);
 
 	//FIXME: this should part of battleGetAvailableHexes
 	bool forceTarget = false;
@@ -25,7 +25,7 @@ PotentialTargets::PotentialTargets(const battle::Unit * attacker, const Hypothet
 	if(attackerInfo->hasBonusOfType(Bonus::ATTACKS_NEAREST_CREATURE))
 	{
 		forceTarget = true;
-		auto nearest = state->getNearestStack(attackerInfo);
+		auto nearest = state.getNearestStack(attackerInfo);
 
 		if(nearest.first != nullptr)
 		{
@@ -34,14 +34,14 @@ PotentialTargets::PotentialTargets(const battle::Unit * attacker, const Hypothet
 		}
 	}
 
-	auto aliveUnits = state->battleGetUnitsIf([=](const battle::Unit * unit)
+	auto aliveUnits = state.battleGetUnitsIf([=](const battle::Unit * unit)
 	{
 		return unit->isValidTarget() && unit->unitId() != attackerInfo->unitId();
 	});
 
 	for(auto defender : aliveUnits)
 	{
-		if(!forceTarget && !state->battleMatchOwner(attackerInfo, defender))
+		if(!forceTarget && !state.battleMatchOwner(attackerInfo, defender))
 			continue;
 
 		auto GenerateAttackInfo = [&](bool shooting, BattleHex hex) -> AttackPossibility
@@ -61,7 +61,7 @@ PotentialTargets::PotentialTargets(const battle::Unit * attacker, const Hypothet
 			else
 				unreachableEnemies.push_back(defender);
 		}
-		else if(state->battleCanShoot(attackerInfo, defender->getPosition()))
+		else if(state.battleCanShoot(attackerInfo, defender->getPosition()))
 		{
 			possibleAttacks.push_back(GenerateAttackInfo(true, BattleHex::INVALID));
 		}
@@ -84,22 +84,18 @@ PotentialTargets::PotentialTargets(const battle::Unit * attacker, const Hypothet
 
 	boost::sort(possibleAttacks, [](const AttackPossibility & lhs, const AttackPossibility & rhs) -> bool
 	{
-		if(lhs.collateralDamage > rhs.collateralDamage)
+		return lhs.damageDiff() > rhs.damageDiff();
-			return false;
-		if(lhs.collateralDamage < rhs.collateralDamage)
-			return true;
-		return (lhs.damageDealt + lhs.shootersBlockedDmg - lhs.damageReceived > rhs.damageDealt + rhs.shootersBlockedDmg - rhs.damageReceived);
 	});
 
 	if (!possibleAttacks.empty())
 	{
 		auto & bestAp = possibleAttacks[0];
 
-		logGlobal->info("Battle AI best: %s -> %s at %d from %d, affects %d units: %lld %lld %lld %lld",
+		logGlobal->info("Battle AI best: %s -> %s at %d from %d, affects %d units: d:%lld a:%lld c:%lld s:%lld",
 			bestAp.attack.attacker->unitType()->identifier,
-			state->battleGetUnitByPos(bestAp.dest)->unitType()->identifier,
+			state.battleGetUnitByPos(bestAp.dest)->unitType()->identifier,
 			(int)bestAp.dest, (int)bestAp.from, (int)bestAp.affectedUnits.size(),
-			bestAp.damageDealt, bestAp.damageReceived, bestAp.collateralDamage, bestAp.shootersBlockedDmg);
+			bestAp.defenderDamageReduce, bestAp.attackerDamageReduce, bestAp.collateralDamageReduce, bestAp.shootersBlockedDmg);
 	}
 }
 
@@ -107,6 +103,7 @@ int64_t PotentialTargets::bestActionValue() const
 {
 	if(possibleAttacks.empty())
 		return 0;
+
 	return bestAction().attackValue();
 }
 
@@ -114,6 +111,6 @@ const AttackPossibility & PotentialTargets::bestAction() const
 {
 	if(possibleAttacks.empty())
 		throw std::runtime_error("No best action, since we don't have any actions");
-	return possibleAttacks.at(0);
+
-	//return *vstd::maxElementByFun(possibleAttacks, [](const AttackPossibility &ap) { return ap.attackValue(); } );
+	return possibleAttacks.front();
 }

AI/BattleAI/PotentialTargets.h

@@ -17,7 +17,7 @@ public:
 	std::vector<const battle::Unit *> unreachableEnemies;
 
 	PotentialTargets(){};
-	PotentialTargets(const battle::Unit * attacker, const HypotheticBattle * state);
+	PotentialTargets(const battle::Unit * attacker, const HypotheticBattle & state);
 
 	const AttackPossibility & bestAction() const;
 	int64_t bestActionValue() const;

AI/Nullkiller/Pathfinding/AINodeStorage.cpp

@@ -180,13 +180,14 @@ std::vector<CGPathNode *> AINodeStorage::getInitialNodes()
 	for(auto actorPtr : actors)
 	{
 		ChainActor * actor = actorPtr.get();
-		AIPathNode * initialNode =
-			getOrCreateNode(actor->initialPosition, actor->layer, actor)
-			.get();
 
-		if(!initialNode)
+		auto allocated = getOrCreateNode(actor->initialPosition, actor->layer, actor);
+
+		if(!allocated)
 			continue;
 
+		AIPathNode * initialNode = allocated.get();
+
 		initialNode->inPQ = false;
 		initialNode->pq = nullptr;
 		initialNode->turns = actor->initialTurn;