Move rest of commonly-accessed UnitState queries to bonus cache

client/NetPacksClient.cpp

@@ -821,7 +821,7 @@ void ApplyClientNetPackVisitor::visitBattleSetActiveStack(BattleSetActiveStack &
 
 	const CStack *activated = gs.getBattle(pack.battleID)->battleGetStackByID(pack.stack);
 	PlayerColor playerToCall; //pack.player that will move activated stack
-	if(activated->hasBonusOfType(BonusType::HYPNOTIZED))
+	if(activated->isHypnotized())
 	{
 		playerToCall = gs.getBattle(pack.battleID)->getSide(BattleSide::ATTACKER).color == activated->unitOwner()
 			? gs.getBattle(pack.battleID)->getSide(BattleSide::DEFENDER).color

lib/battle/CBattleInfoCallback.cpp

@@ -714,18 +714,7 @@ bool CBattleInfoCallback::battleCanShoot(const battle::Unit * attacker) const
 	if (!attacker->canShoot())
 		return false;
 
-	//forgetfulness
-	TConstBonusListPtr forgetfulList = attacker->getBonusesOfType(BonusType::FORGETFULL);
-	if(!forgetfulList->empty())
-	{
-		int forgetful = forgetfulList->totalValue();
-
-		//advanced+ level
-		if(forgetful > 1)
-			return false;
-	}
-
-	return !battleIsUnitBlocked(attacker) || attacker->hasBonusOfType(BonusType::FREE_SHOOTING);
+	return attacker->canShootBlocked() || !battleIsUnitBlocked(attacker);
 }
 
 bool CBattleInfoCallback::battleCanTargetEmptyHex(const battle::Unit * attacker) const
@@ -1732,9 +1721,6 @@ bool CBattleInfoCallback::battleIsUnitBlocked(const battle::Unit * unit) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 
-	if(unit->hasBonusOfType(BonusType::SIEGE_WEAPON)) //siege weapons cannot be blocked
-		return false;
-
 	for(const auto * adjacent : battleAdjacentUnits(unit))
 	{
 		if(adjacent->unitOwner() != unit->unitOwner()) //blocked by enemy stack

lib/battle/CBattleInfoEssentials.cpp

@@ -404,7 +404,7 @@ PlayerColor CBattleInfoEssentials::battleGetOwner(const battle::Unit * unit) con
 
 	PlayerColor initialOwner = getBattle()->getSidePlayer(unit->unitSide());
 
-	if(unit->hasBonusOfType(BonusType::HYPNOTIZED))
+	if(unit->isHypnotized())
 		return otherPlayer(initialOwner);
 	else
 		return initialOwner;

lib/battle/CUnitState.cpp

@@ -526,9 +526,16 @@ bool CUnitState::isCaster() const
 	return casts.total() > 0;//do not check specific cast abilities here
 }
 
+bool CUnitState::canShootBlocked() const
+{
+	return bonusCache.cache.getBonusValue(UnitBonusValuesProxy::HAS_FREE_SHOOTING);
+}
+
 bool CUnitState::canShoot() const
 {
-	return shots.canUse(1);
+	return
+		shots.canUse(1) &&
+		bonusCache.cache.getBonusValue(UnitBonusValuesProxy::FORGETFULL) <= 1; //advanced+ level
 }
 
 bool CUnitState::isShooter() const
@@ -563,6 +570,11 @@ int64_t CUnitState::getTotalHealth() const
 	return health.total();
 }
 
+int64_t CUnitState::getMaxHealth() const
+{
+	return std::max(1, bonusCache.cache.getBonusValue(UnitBonusValuesProxy::STACK_HEALTH));
+}
+
 BattleHex CUnitState::getPosition() const
 {
 	return position;
@@ -686,6 +698,11 @@ BattlePhases::Type CUnitState::battleQueuePhase(int turn) const
 	}
 }
 
+bool CUnitState::isHypnotized() const
+{
+	return bonusCache.cache.getBonusValue(UnitBonusValuesProxy::HYPNOTIZED);
+}
+
 int CUnitState::getTotalAttacks(bool ranged) const
 {
 	return 1 + (ranged ?
@@ -943,6 +960,10 @@ const UnitBonusValuesProxy::SelectorsArray * CUnitState::generateBonusSelectors(
 		defence.And(selectorRanged),//DEFENCE_MELEE,
 		defence.And(selectorRanged),//DEFENCE_RANGED,
 		Selector::type()(BonusType::IN_FRENZY),//IN_FRENZY,
+		Selector::type()(BonusType::FORGETFULL),//FORGETFULL,
+		Selector::type()(BonusType::HYPNOTIZED),//HYPNOTIZED,
+		Selector::type()(BonusType::FREE_SHOOTING).Or(Selector::type()(BonusType::SIEGE_WEAPON)),//HAS_FREE_SHOOTING,
+		Selector::type()(BonusType::STACK_HEALTH),//STACK_HEALTH,
 	};
 
 	return &selectors;

lib/battle/CUnitState.h

@@ -145,6 +145,10 @@ public:
 		DEFENCE_RANGED,
 
 		IN_FRENZY,
+		HYPNOTIZED,
+		FORGETFULL,
+		HAS_FREE_SHOOTING,
+		STACK_HEALTH,
 
 		TOTAL_KEYS,
 	};
@@ -228,11 +232,14 @@ public:
 	bool isFrozen() const override;
 	bool isValidTarget(bool allowDead = false) const override;
 
+	bool isHypnotized() const override;
+
 	bool isClone() const override;
 	bool hasClone() const override;
 
 	bool canCast() const override;
 	bool isCaster() const override;
+	bool canShootBlocked() const override;
 	bool canShoot() const override;
 	bool isShooter() const override;
 
@@ -241,6 +248,7 @@ public:
 	int32_t getFirstHPleft() const override;
 	int64_t getAvailableHealth() const override;
 	int64_t getTotalHealth() const override;
+	int64_t getMaxHealth() const override;
 
 	BattleHex getPosition() const override;
 	void setPosition(BattleHex hex) override;

lib/battle/Unit.h

@@ -84,11 +84,14 @@ public:
 	bool isTurret() const;
 	virtual bool isValidTarget(bool allowDead = false) const = 0; //non-turret non-ghost stacks (can be attacked or be object of magic effect)
 
+	virtual bool isHypnotized() const = 0;
+
 	virtual bool isClone() const = 0;
 	virtual bool hasClone() const = 0;
 
 	virtual bool canCast() const = 0;
 	virtual bool isCaster() const = 0;
+	virtual bool canShootBlocked() const = 0;
 	virtual bool canShoot() const = 0;
 	virtual bool isShooter() const = 0;
 

lib/bonuses/BonusCache.cpp

@@ -18,7 +18,7 @@
 #include "../VCMI_Lib.h"
 #include "../IGameSettings.h"
 
-int BonusCacheBase::getBonusValueImpl(BonusCacheEntry & currentValue, const CSelector & selector) const
+int BonusCacheBase::getBonusValueImpl(BonusCacheEntry & currentValue, const CSelector & selector, BonusCacheMode mode) const
 {
 	if (target->getTreeVersion() == currentValue.version)
 	{
@@ -28,7 +28,12 @@ int BonusCacheBase::getBonusValueImpl(BonusCacheEntry & currentValue, const CSel
 	{
 		// NOTE: following code theoretically can fail if bonus tree was changed by another thread between two following lines
 		// However, this situation should not be possible - gamestate modification should only happen in single-treaded mode with locked gamestate mutex
-		int newValue = target->valOfBonuses(selector);
+		int newValue;
+
+		if (mode == BonusCacheMode::VALUE)
+			newValue = target->valOfBonuses(selector);
+		else
+			newValue = target->hasBonus(selector);
 		currentValue.value = newValue;
 		currentValue.version = target->getTreeVersion();
 
@@ -42,7 +47,7 @@ BonusValueCache::BonusValueCache(const IBonusBearer * target, const CSelector se
 
 int BonusValueCache::getValue() const
 {
-	return getBonusValueImpl(value, selector);
+	return getBonusValueImpl(value, selector, BonusCacheMode::VALUE);
 }
 
 PrimarySkillsCache::PrimarySkillsCache(const IBonusBearer * target)

lib/bonuses/BonusCache.h

@@ -12,7 +12,7 @@
 
 #include "BonusSelector.h"
 
-enum class BonusCacheMode
+enum class BonusCacheMode : int8_t
 {
 	VALUE, // total value of bonus will be cached
 	PRESENCE, // presence of bonus will be cached
@@ -34,7 +34,7 @@ protected:
 		std::atomic<int64_t> value = 0;
 	};
 
-	int getBonusValueImpl(BonusCacheEntry & currentValue, const CSelector & selector) const;
+	int getBonusValueImpl(BonusCacheEntry & currentValue, const CSelector & selector, BonusCacheMode) const;
 };
 
 /// Cache that tracks a single query to bonus system
@@ -62,7 +62,13 @@ public:
 	int getBonusValue(EnumType which) const
 	{
 		auto index = static_cast<size_t>(which);
-		return getBonusValueImpl(cache[index], (*selectors)[index]);
+		return getBonusValueImpl(cache[index], (*selectors)[index], BonusCacheMode::VALUE);
+	}
+
+	int hasBonus(EnumType which) const
+	{
+		auto index = static_cast<size_t>(which);
+		return getBonusValueImpl(cache[index], (*selectors)[index], BonusCacheMode::PRESENCE);
 	}
 
 private:

test/mock/mock_battle_Unit.h

@@ -57,10 +57,12 @@ public:
 	MOCK_CONST_METHOD0(isFrozen, bool());
 	MOCK_CONST_METHOD1(isValidTarget, bool(bool));
 
+	MOCK_CONST_METHOD0(isHypnotized, bool());
 	MOCK_CONST_METHOD0(isClone, bool());
 	MOCK_CONST_METHOD0(hasClone, bool());
 	MOCK_CONST_METHOD0(canCast, bool());
 	MOCK_CONST_METHOD0(isCaster, bool());
+	MOCK_CONST_METHOD0(canShootBlocked, bool());
 	MOCK_CONST_METHOD0(canShoot, bool());
 	MOCK_CONST_METHOD0(isShooter, bool());