Use bonus system cache whenever possible

AI/Nullkiller/Analyzers/HeroManager.cpp

@@ -72,8 +72,8 @@ float HeroManager::evaluateSpeciality(const CGHeroInstance * hero) const
 {
 	auto heroSpecial = Selector::source(BonusSource::HERO_SPECIAL, BonusSourceID(hero->getHeroTypeID()));
 	auto secondarySkillBonus = Selector::targetSourceType()(BonusSource::SECONDARY_SKILL);
-	auto specialSecondarySkillBonuses = hero->getBonuses(heroSpecial.And(secondarySkillBonus));
-	auto secondarySkillBonuses = hero->getBonuses(Selector::sourceTypeSel(BonusSource::SECONDARY_SKILL));
+	auto specialSecondarySkillBonuses = hero->getBonuses(heroSpecial.And(secondarySkillBonus), "HeroManager::evaluateSpeciality");
+	auto secondarySkillBonuses = hero->getBonusesFrom(BonusSource::SECONDARY_SKILL);
 	float specialityScore = 0.0f;
 
 	for(auto bonus : *secondarySkillBonuses)

client/battle/BattleActionsController.cpp

@@ -957,7 +957,7 @@ void BattleActionsController::tryActivateStackSpellcasting(const CStack *casterS
 			creatureSpells.push_back(spellToCast.toSpell());
 	}
 
-	TConstBonusListPtr bl = casterStack->getBonuses(Selector::type()(BonusType::SPELLCASTER));
+	TConstBonusListPtr bl = casterStack->getBonusesOfType(BonusType::SPELLCASTER);
 
 	for(const auto & bonus : *bl)
 	{

client/windows/CHeroWindow.cpp

@@ -284,7 +284,7 @@ void CHeroWindow::update()
 
 	dismissButton->block(noDismiss);
 
-	if(curHero->valOfBonuses(Selector::type()(BonusType::BEFORE_BATTLE_REPOSITION)) == 0)
+	if(curHero->valOfBonuses(BonusType::BEFORE_BATTLE_REPOSITION) == 0)
 	{
 		tacticsButton->block(true);
 	}

lib/BasicTypes.cpp

@@ -43,7 +43,7 @@ TerrainId AFactionMember::getNativeTerrain() const
 
 int32_t AFactionMember::magicResistance() const
 {
-	si32 val = getBonusBearer()->valOfBonuses(Selector::type()(BonusType::MAGIC_RESISTANCE));
+	si32 val = getBonusBearer()->valOfBonuses(BonusType::MAGIC_RESISTANCE);
 	vstd::amin (val, 100);
 	return val;
 }
@@ -114,9 +114,7 @@ int AFactionMember::moraleValAndBonusList(TConstBonusListPtr & bonusList) const
 		return 0;
 	}
 
-	static const auto moraleSelector = Selector::type()(BonusType::MORALE);
-	static const std::string cachingStrMor = "type_MORALE";
-	bonusList = getBonusBearer()->getBonuses(moraleSelector, cachingStrMor);
+	bonusList = getBonusBearer()->getBonusesOfType(BonusType::MORALE);
 
 	return std::clamp(bonusList->totalValue(), maxBadMorale, maxGoodMorale);
 }
@@ -140,9 +138,7 @@ int AFactionMember::luckValAndBonusList(TConstBonusListPtr & bonusList) const
 		return 0;
 	}
 
-	static const auto luckSelector = Selector::type()(BonusType::LUCK);
-	static const std::string cachingStrLuck = "type_LUCK";
-	bonusList = getBonusBearer()->getBonuses(luckSelector, cachingStrLuck);
+	bonusList = getBonusBearer()->getBonusesOfType(BonusType::LUCK);
 
 	return std::clamp(bonusList->totalValue(), maxBadLuck, maxGoodLuck);
 }

lib/CBonusTypeHandler.cpp

@@ -76,7 +76,7 @@ std::string CBonusTypeHandler::bonusToString(const std::shared_ptr<Bonus> & bonu
 	std::string text = VLC->generaltexth->translate(textID);
 
 	if (text.find("${val}") != std::string::npos)
-		boost::algorithm::replace_all(text, "${val}", std::to_string(bearer->valOfBonuses(Selector::typeSubtype(bonus->type, bonus->subtype))));
+		boost::algorithm::replace_all(text, "${val}", std::to_string(bearer->valOfBonuses(bonus->type, bonus->subtype)));
 
 	if (text.find("${subtype.creature}") != std::string::npos && bonus->subtype.as<CreatureID>().hasValue())
 		boost::algorithm::replace_all(text, "${subtype.creature}", bonus->subtype.as<CreatureID>().toCreature()->getNamePluralTranslated());

lib/battle/BattleInfo.cpp

@@ -399,8 +399,8 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 	{
 		if(heroes[i])
 		{
-			battleRepositionHex[i] += heroes[i]->valOfBonuses(Selector::type()(BonusType::BEFORE_BATTLE_REPOSITION));
-			battleRepositionHexBlock[i] += heroes[i]->valOfBonuses(Selector::type()(BonusType::BEFORE_BATTLE_REPOSITION_BLOCK));
+			battleRepositionHex[i] += heroes[i]->valOfBonuses(BonusType::BEFORE_BATTLE_REPOSITION);
+			battleRepositionHexBlock[i] += heroes[i]->valOfBonuses(BonusType::BEFORE_BATTLE_REPOSITION_BLOCK);
 		}
 	}
 	int tacticsSkillDiffAttacker = battleRepositionHex[BattleSide::ATTACKER] - battleRepositionHexBlock[BattleSide::DEFENDER];

lib/battle/CBattleInfoCallback.cpp

@@ -715,10 +715,10 @@ bool CBattleInfoCallback::battleCanShoot(const battle::Unit * attacker) const
 		return false;
 
 	//forgetfulness
-	TConstBonusListPtr forgetfulList = attacker->getBonuses(Selector::type()(BonusType::FORGETFULL));
+	TConstBonusListPtr forgetfulList = attacker->getBonusesOfType(BonusType::FORGETFULL);
 	if(!forgetfulList->empty())
 	{
-		int forgetful = forgetfulList->valOfBonuses(Selector::type()(BonusType::FORGETFULL));
+		int forgetful = forgetfulList->totalValue();
 
 		//advanced+ level
 		if(forgetful > 1)
@@ -1880,9 +1880,7 @@ SpellID CBattleInfoCallback::getRandomBeneficialSpell(vstd::RNG & rand, const ba
 		{
 			const auto * kingMonster = getAliveEnemy([&](const CStack * stack) -> bool //look for enemy, non-shooting stack
 			{
-				const auto isKing = Selector::type()(BonusType::KING);
-
-				return stack->hasBonus(isKing);
+				return stack->hasBonusOfType(BonusType::KING);
 			});
 
 			if (!kingMonster)
@@ -1907,7 +1905,7 @@ SpellID CBattleInfoCallback::getRandomCastedSpell(vstd::RNG & rand,const CStack
 {
 	RETURN_IF_NOT_BATTLE(SpellID::NONE);
 
-	TConstBonusListPtr bl = caster->getBonuses(Selector::type()(BonusType::SPELLCASTER));
+	TConstBonusListPtr bl = caster->getBonusesOfType(BonusType::SPELLCASTER);
 	if (!bl->size())
 		return SpellID::NONE;
 
@@ -1971,7 +1969,7 @@ si8 CBattleInfoCallback::battleMinSpellLevel(BattleSide side) const
 	if(!node)
 		return 0;
 
-	auto b = node->getBonuses(Selector::type()(BonusType::BLOCK_MAGIC_BELOW));
+	auto b = node->getBonusesOfType(BonusType::BLOCK_MAGIC_BELOW);
 	if(b->size())
 		return b->totalValue();
 
@@ -1990,7 +1988,7 @@ si8 CBattleInfoCallback::battleMaxSpellLevel(BattleSide side) const
 		return GameConstants::SPELL_LEVELS;
 
 	//We can't "just get value" - it'd be 0 if there are bonuses (and all would be blocked)
-	auto b = node->getBonuses(Selector::type()(BonusType::BLOCK_MAGIC_ABOVE));
+	auto b = node->getBonusesOfType(BonusType::BLOCK_MAGIC_ABOVE);
 	if(b->size())
 		return b->totalValue();
 

lib/battle/CBattleInfoEssentials.cpp

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

lib/battle/CUnitState.cpp

@@ -85,7 +85,7 @@ void CAmmo::serializeJson(JsonSerializeFormat & handler)
 ///CShots
 CShots::CShots(const battle::Unit * Owner)
 	: CAmmo(Owner, Selector::type()(BonusType::SHOTS)),
-	shooter(Owner, Selector::type()(BonusType::SHOOTER))
+	shooter(Owner, BonusType::SHOOTER)
 {
 }
 
@@ -124,8 +124,8 @@ CCasts::CCasts(const battle::Unit * Owner):
 CRetaliations::CRetaliations(const battle::Unit * Owner)
 	: CAmmo(Owner, Selector::type()(BonusType::ADDITIONAL_RETALIATION)),
 	totalCache(0),
-	noRetaliation(Owner, Selector::type()(BonusType::SIEGE_WEAPON).Or(Selector::type()(BonusType::HYPNOTIZED)).Or(Selector::type()(BonusType::NO_RETALIATION))),
-	unlimited(Owner, Selector::type()(BonusType::UNLIMITED_RETALIATIONS))
+	noRetaliation(Owner, Selector::type()(BonusType::SIEGE_WEAPON).Or(Selector::type()(BonusType::HYPNOTIZED)).Or(Selector::type()(BonusType::NO_RETALIATION)), "CRetaliations::noRetaliation"),
+	unlimited(Owner, BonusType::UNLIMITED_RETALIATIONS)
 {
 }
 
@@ -347,7 +347,7 @@ CUnitState::CUnitState():
 	attack(this, Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::ATTACK)), 0),
 	defence(this, Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::DEFENSE)), 0),
 	inFrenzy(this, Selector::type()(BonusType::IN_FRENZY)),
-	cloneLifetimeMarker(this, Selector::type()(BonusType::NONE).And(Selector::source(BonusSource::SPELL_EFFECT, BonusSourceID(SpellID(SpellID::CLONE))))),
+	cloneLifetimeMarker(this, Selector::type()(BonusType::NONE).And(Selector::source(BonusSource::SPELL_EFFECT, BonusSourceID(SpellID(SpellID::CLONE)))), "CUnitState::cloneLifetimeMarker"),
 	cloneID(-1)
 {
 
@@ -591,7 +591,11 @@ void CUnitState::setPosition(BattleHex hex)
 
 int32_t CUnitState::getInitiative(int turn) const
 {
-	return valOfBonuses(Selector::type()(BonusType::STACKS_SPEED).And(Selector::turns(turn)));
+	if (turn == 0)
+		return valOfBonuses(BonusType::STACKS_SPEED);
+
+	std::string cachingStr = "type_STACKS_SPEED_turns_" + std::to_string(turn);
+	return valOfBonuses(Selector::type()(BonusType::STACKS_SPEED).And(Selector::turns(turn)), cachingStr);
 }
 
 uint8_t CUnitState::getRangedFullDamageDistance() const
@@ -602,7 +606,7 @@ uint8_t CUnitState::getRangedFullDamageDistance() const
 	uint8_t rangedFullDamageDistance = GameConstants::BATTLE_SHOOTING_PENALTY_DISTANCE;
 
 	// overwrite full ranged damage distance with the value set in Additional info field of LIMITED_SHOOTING_RANGE bonus
-	if(this->hasBonus(Selector::type()(BonusType::LIMITED_SHOOTING_RANGE)))
+	if(hasBonusOfType(BonusType::LIMITED_SHOOTING_RANGE))
 	{
 		auto bonus = this->getBonus(Selector::type()(BonusType::LIMITED_SHOOTING_RANGE));
 		if(bonus != nullptr && bonus->additionalInfo != CAddInfo::NONE)
@@ -620,7 +624,7 @@ uint8_t CUnitState::getShootingRangeDistance() const
 	uint8_t shootingRangeDistance = GameConstants::BATTLE_SHOOTING_RANGE_DISTANCE;
 
 	// overwrite full ranged damage distance with the value set in Additional info field of LIMITED_SHOOTING_RANGE bonus
-	if(this->hasBonus(Selector::type()(BonusType::LIMITED_SHOOTING_RANGE)))
+	if(hasBonusOfType(BonusType::LIMITED_SHOOTING_RANGE))
 	{
 		auto bonus = this->getBonus(Selector::type()(BonusType::LIMITED_SHOOTING_RANGE));
 		if(bonus != nullptr)
@@ -632,7 +636,14 @@ uint8_t CUnitState::getShootingRangeDistance() const
 
 bool CUnitState::canMove(int turn) const
 {
-	return alive() && !hasBonus(Selector::type()(BonusType::NOT_ACTIVE).And(Selector::turns(turn))); //eg. Ammo Cart or blinded creature
+	if (!alive())
+		return false;
+
+	if (turn == 0)
+		return valOfBonuses(BonusType::NOT_ACTIVE);
+
+	std::string cachingStr = "type_NOT_ACTIVE_turns_" + std::to_string(turn);
+	return valOfBonuses(Selector::type()(BonusType::NOT_ACTIVE).And(Selector::turns(turn)), cachingStr); //eg. Ammo Cart or blinded creature
 }
 
 bool CUnitState::defended(int turn) const

lib/battle/DamageCalculator.cpp

@@ -161,7 +161,7 @@ int DamageCalculator::getActorAttackSlayer() const
 		return 0;
 
 	auto slayerEffects = info.attacker->getBonuses(selectorSlayer, cachingStrSlayer);
-	auto slayerAffected = info.defender->unitType()->valOfBonuses(Selector::type()(BonusType::KING));
+	auto slayerAffected = info.defender->unitType()->valOfBonuses(BonusType::KING);
 
 	if(std::shared_ptr<const Bonus> slayerEffect = slayerEffects->getFirst(Selector::all))
 	{
@@ -269,26 +269,16 @@ double DamageCalculator::getAttackDoubleDamageFactor() const
 
 double DamageCalculator::getAttackJoustingFactor() const
 {
-	const std::string cachingStrJousting = "type_JOUSTING";
-	static const auto selectorJousting = Selector::type()(BonusType::JOUSTING);
-
-	const std::string cachingStrChargeImmunity = "type_CHARGE_IMMUNITY";
-	static const auto selectorChargeImmunity = Selector::type()(BonusType::CHARGE_IMMUNITY);
-
 	//applying jousting bonus
-	if(info.chargeDistance > 0 && info.attacker->hasBonus(selectorJousting, cachingStrJousting) && !info.defender->hasBonus(selectorChargeImmunity, cachingStrChargeImmunity))
-		return info.chargeDistance * (info.attacker->valOfBonuses(selectorJousting))/100.0;
+	if(info.chargeDistance > 0 && info.attacker->hasBonusOfType(BonusType::JOUSTING) && !info.defender->hasBonusOfType(BonusType::CHARGE_IMMUNITY))
+		return info.chargeDistance * (info.attacker->valOfBonuses(BonusType::JOUSTING))/100.0;
 	return 0.0;
 }
 
 double DamageCalculator::getAttackHateFactor() const
 {
 	//assume that unit have only few HATE features and cache them all
-	const std::string cachingStrHate = "type_HATE";
-	static const auto selectorHate = Selector::type()(BonusType::HATE);
-
-	auto allHateEffects = info.attacker->getBonuses(selectorHate, cachingStrHate);
-
+	auto allHateEffects = info.attacker->getBonusesOfType(BonusType::HATE);
 	return allHateEffects->valOfBonuses(Selector::subtype()(BonusSubtypeID(info.defender->creatureId()))) / 100.0;
 }
 
@@ -411,7 +401,7 @@ double DamageCalculator::getDefenseForgetfulnessFactor() const
 	{
 		//todo: set actual percentage in spell bonus configuration instead of just level; requires non trivial backward compatibility handling
 		//get list first, total value of 0 also counts
-		TConstBonusListPtr forgetfulList = info.attacker->getBonuses(Selector::type()(BonusType::FORGETFULL),"type_FORGETFULL");
+		TConstBonusListPtr forgetfulList = info.attacker->getBonusesOfType(BonusType::FORGETFULL);
 
 		if(!forgetfulList->empty())
 		{

lib/bonuses/CBonusProxy.cpp

@@ -158,7 +158,7 @@ int CTotalsProxy::getMeleeValue() const
 
 	if(treeVersion != meleeCachedLast)
 	{
-		auto bonuses = target->getBonuses(selector, limit);
+		auto bonuses = target->getBonuses(selector, limit, "CTotalsProxy::getMeleeValue");
 		meleeValue = initialValue + bonuses->totalValue();
 		meleeCachedLast = treeVersion;
 	}
@@ -174,7 +174,7 @@ int CTotalsProxy::getRangedValue() const
 
 	if(treeVersion != rangedCachedLast)
 	{
-		auto bonuses = target->getBonuses(selector, limit);
+		auto bonuses = target->getBonuses(selector, limit, "CTotalsProxy::getRangedValue");
 		rangedValue = initialValue + bonuses->totalValue();
 		rangedCachedLast = treeVersion;
 	}
@@ -183,10 +183,21 @@ int CTotalsProxy::getRangedValue() const
 }
 
 ///CCheckProxy
-CCheckProxy::CCheckProxy(const IBonusBearer * Target, CSelector Selector):
+CCheckProxy::CCheckProxy(const IBonusBearer * Target, BonusType bonusType):
+	target(Target),
+	selector(Selector::type()(bonusType)),
+	cachingStr("type_" + std::to_string(static_cast<int>(bonusType))),
+	cachedLast(0),
+	hasBonus(false)
+{
+
+}
+
+CCheckProxy::CCheckProxy(const IBonusBearer * Target, CSelector Selector, const std::string & cachingStr):
 	target(Target),
 	selector(std::move(Selector)),
 	cachedLast(0),
+	cachingStr(cachingStr),
 	hasBonus(false)
 {
 }
@@ -200,11 +211,11 @@ bool CCheckProxy::getHasBonus() const
 
 	if(treeVersion != cachedLast)
 	{
-		hasBonus = target->hasBonus(selector);
+		hasBonus = target->hasBonus(selector, cachingStr);
 		cachedLast = treeVersion;
 	}
 
 	return hasBonus;
 }
 
-VCMI_LIB_NAMESPACE_END
+VCMI_LIB_NAMESPACE_END

lib/bonuses/CBonusProxy.h

@@ -73,7 +73,8 @@ private:
 class DLL_LINKAGE CCheckProxy
 {
 public:
-	CCheckProxy(const IBonusBearer * Target, CSelector Selector);
+	CCheckProxy(const IBonusBearer * Target, CSelector Selector, const std::string & cachingStr);
+	CCheckProxy(const IBonusBearer * Target, BonusType bonusType);
 	CCheckProxy(const CCheckProxy & other);
 	CCheckProxy& operator= (const CCheckProxy & other) = default;
 
@@ -81,10 +82,11 @@ public:
 
 private:
 	const IBonusBearer * target;
+	std::string cachingStr;
 	CSelector selector;
 
 	mutable int64_t cachedLast;
 	mutable bool hasBonus;
 };
 
-VCMI_LIB_NAMESPACE_END
+VCMI_LIB_NAMESPACE_END

lib/bonuses/IBonusBearer.cpp

@@ -42,6 +42,27 @@ TConstBonusListPtr IBonusBearer::getBonuses(const CSelector &selector, const CSe
 	return getAllBonuses(selector, limit, cachingStr);
 }
 
+TConstBonusListPtr IBonusBearer::getBonusesFrom(BonusSource source) const
+{
+	std::string cachingStr = "source_" + std::to_string(static_cast<int>(source));
+	CSelector s = Selector::sourceTypeSel(source);
+	return getBonuses(s, cachingStr);
+}
+
+TConstBonusListPtr IBonusBearer::getBonusesOfType(BonusType type) const
+{
+	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type));
+	CSelector s = Selector::type()(type);
+	return getBonuses(s, cachingStr);
+}
+
+TConstBonusListPtr IBonusBearer::getBonusesOfType(BonusType type, BonusSubtypeID subtype) const
+{
+	std::string cachingStr = "type_" + std::to_string(static_cast<int>(type)) + "_" + subtype.toString();
+	CSelector s = Selector::type()(type);
+	return getBonuses(s, cachingStr);
+}
+
 int IBonusBearer::valOfBonuses(BonusType type) const
 {
 	//This part is performance-critical
@@ -84,7 +105,14 @@ bool IBonusBearer::hasBonusOfType(BonusType type, BonusSubtypeID subtype) const
 
 bool IBonusBearer::hasBonusFrom(BonusSource source, BonusSourceID sourceID) const
 {
-	return hasBonus(Selector::source(source,sourceID));
+	std::string cachingStr = "source_" + std::to_string(static_cast<int>(source)) + "_" + sourceID.toString();
+	return hasBonus(Selector::source(source,sourceID), cachingStr);
+}
+
+bool IBonusBearer::hasBonusFrom(BonusSource source) const
+{
+	std::string cachingStr = "source_" + std::to_string(static_cast<int>(source));
+	return hasBonus((Selector::sourceTypeSel(source)), cachingStr);
 }
 
 std::shared_ptr<const Bonus> IBonusBearer::getBonus(const CSelector &selector) const

lib/bonuses/IBonusBearer.h

@@ -20,12 +20,12 @@ public:
 	// * selector is predicate that tests if Bonus matches our criteria
 	IBonusBearer() = default;
 	virtual ~IBonusBearer() = default;
-	virtual TConstBonusListPtr getAllBonuses(const CSelector &selector, const CSelector &limit, const std::string &cachingStr = "") const = 0;
-	int valOfBonuses(const CSelector &selector, const std::string &cachingStr = "") const;
-	bool hasBonus(const CSelector &selector, const std::string &cachingStr = "") const;
-	bool hasBonus(const CSelector &selector, const CSelector &limit, const std::string &cachingStr = "") const;
-	TConstBonusListPtr getBonuses(const CSelector &selector, const CSelector &limit, const std::string &cachingStr = "") const;
-	TConstBonusListPtr getBonuses(const CSelector &selector, const std::string &cachingStr = "") const;
+	virtual TConstBonusListPtr getAllBonuses(const CSelector &selector, const CSelector &limit, const std::string &cachingStr = {}) const = 0;
+	int valOfBonuses(const CSelector &selector, const std::string &cachingStr = {}) const;
+	bool hasBonus(const CSelector &selector, const std::string &cachingStr = {}) const;
+	bool hasBonus(const CSelector &selector, const CSelector &limit, const std::string &cachingStr = {}) const;
+	TConstBonusListPtr getBonuses(const CSelector &selector, const CSelector &limit, const std::string &cachingStr = {}) const;
+	TConstBonusListPtr getBonuses(const CSelector &selector, const std::string &cachingStr = {}) const;
 
 	std::shared_ptr<const Bonus> getBonus(const CSelector &selector) const; //returns any bonus visible on node that matches (or nullptr if none matches)
 
@@ -34,8 +34,13 @@ public:
 	bool hasBonusOfType(BonusType type) const;//determines if hero has a bonus of given type (and optionally subtype)
 	int valOfBonuses(BonusType type, BonusSubtypeID subtype) const; //subtype -> subtype of bonus;
 	bool hasBonusOfType(BonusType type, BonusSubtypeID subtype) const;//determines if hero has a bonus of given type (and optionally subtype)
+	bool hasBonusFrom(BonusSource source) const;
 	bool hasBonusFrom(BonusSource source, BonusSourceID sourceID) const;
 
+	TConstBonusListPtr getBonusesFrom(BonusSource source) const;
+	TConstBonusListPtr getBonusesOfType(BonusType type) const;
+	TConstBonusListPtr getBonusesOfType(BonusType type, BonusSubtypeID subtype) const;
+
 	virtual int64_t getTreeVersion() const = 0;
 };
 

lib/mapObjects/CArmedInstance.cpp

@@ -38,9 +38,6 @@ void CArmedInstance::randomizeArmy(FactionID type)
 	}
 }
 
-// Take Angelic Alliance troop-mixing freedom of non-evil units into account.
-CSelector CArmedInstance::nonEvilAlignmentMixSelector = Selector::type()(BonusType::NONEVIL_ALIGNMENT_MIX);
-
 CArmedInstance::CArmedInstance(IGameCallback *cb)
 	:CArmedInstance(cb, false)
 {
@@ -49,7 +46,7 @@ CArmedInstance::CArmedInstance(IGameCallback *cb)
 CArmedInstance::CArmedInstance(IGameCallback *cb, bool isHypothetic):
 	CGObjectInstance(cb),
 	CBonusSystemNode(isHypothetic),
-	nonEvilAlignmentMix(this, nonEvilAlignmentMixSelector),
+	nonEvilAlignmentMix(this, BonusType::NONEVIL_ALIGNMENT_MIX), // Take Angelic Alliance troop-mixing freedom of non-evil units into account.
 	battle(nullptr)
 {
 }

lib/mapObjects/CGHeroInstance.cpp

@@ -257,8 +257,7 @@ void CGHeroInstance::setMovementPoints(int points)
 
 int CGHeroInstance::movementPointsLimit(bool onLand) const
 {
-	TurnInfo ti(this);
-	return movementPointsLimitCached(onLand, &ti);
+	return valOfBonuses(BonusType::MOVEMENT, onLand ? BonusCustomSubtype::heroMovementLand : BonusCustomSubtype::heroMovementSea);
 }
 
 int CGHeroInstance::getLowestCreatureSpeed() const
@@ -274,7 +273,7 @@ void CGHeroInstance::updateArmyMovementBonus(bool onLand, const TurnInfo * ti) c
 		lowestCreatureSpeed = realLowestSpeed;
 		//Let updaters run again
 		treeHasChanged();
-		ti->updateHeroBonuses(BonusType::MOVEMENT, Selector::subtype()(onLand ? BonusCustomSubtype::heroMovementLand : BonusCustomSubtype::heroMovementSea));
+		ti->updateHeroBonuses(BonusType::MOVEMENT);
 	}
 }
 
@@ -406,7 +405,7 @@ void CGHeroInstance::initHero(vstd::RNG & rand)
 		putArtifact(ArtifactPosition::MACH4, artifact); //everyone has a catapult
 	}
 
-	if(!hasBonus(Selector::sourceType()(BonusSource::HERO_BASE_SKILL)))
+	if(!hasBonusFrom(BonusSource::HERO_BASE_SKILL))
 	{
 		for(int g=0; g<GameConstants::PRIMARY_SKILLS; ++g)
 		{
@@ -682,7 +681,7 @@ void CGHeroInstance::pickRandomObject(vstd::RNG & rand)
 
 void CGHeroInstance::recreateSecondarySkillsBonuses()
 {
-	auto secondarySkillsBonuses = getBonuses(Selector::sourceType()(BonusSource::SECONDARY_SKILL));
+	auto secondarySkillsBonuses = getBonusesFrom(BonusSource::SECONDARY_SKILL);
 	for(const auto & bonus : *secondarySkillsBonuses)
 		removeBonus(bonus);
 
@@ -973,7 +972,7 @@ CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &b
 		// figure out what to raise - pick strongest creature meeting requirements
 		CreatureID creatureTypeRaised = CreatureID::NONE; //now we always have IMPROVED_NECROMANCY, no need for hardcode
 		int requiredCasualtyLevel = 1;
-		TConstBonusListPtr improvedNecromancy = getBonuses(Selector::type()(BonusType::IMPROVED_NECROMANCY));
+		TConstBonusListPtr improvedNecromancy = getBonusesOfType(BonusType::IMPROVED_NECROMANCY);
 		if(!improvedNecromancy->empty())
 		{
 			int maxCasualtyLevel = 1;
@@ -1141,9 +1140,8 @@ void CGHeroInstance::pushPrimSkill( PrimarySkill which, int val )
 {
 	auto sel = Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(which))
 		.And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL));
-	if(hasBonus(sel))
-		removeBonuses(sel);
-		
+
+	removeBonuses(sel);
 	addNewBonus(std::make_shared<Bonus>(BonusDuration::PERMANENT, BonusType::PRIMARY_SKILL, BonusSource::HERO_BASE_SKILL, val, BonusSourceID(id), BonusSubtypeID(which)));
 }
 
@@ -1281,7 +1279,7 @@ const std::set<SpellID> & CGHeroInstance::getSpellsInSpellbook() const
 
 int CGHeroInstance::maxSpellLevel() const
 {
-	return std::min(GameConstants::SPELL_LEVELS, valOfBonuses(Selector::type()(BonusType::MAX_LEARNABLE_SPELL_LEVEL)));
+	return std::min(GameConstants::SPELL_LEVELS, valOfBonuses(BonusType::MAX_LEARNABLE_SPELL_LEVEL));
 }
 
 void CGHeroInstance::attachToBoat(CGBoat* newBoat)
@@ -1850,7 +1848,7 @@ bool CGHeroInstance::isMissionCritical() const
 
 void CGHeroInstance::fillUpgradeInfo(UpgradeInfo & info, const CStackInstance & stack) const
 {
-	TConstBonusListPtr lista = getBonuses(Selector::typeSubtype(BonusType::SPECIAL_UPGRADE, BonusSubtypeID(stack.getId())));
+	TConstBonusListPtr lista = getBonusesOfType(BonusType::SPECIAL_UPGRADE, BonusSubtypeID(stack.getId()));
 	for(const auto & it : *lista)
 	{
 		auto nid = CreatureID(it->additionalInfo[0]);
@@ -1921,7 +1919,7 @@ const IOwnableObject * CGHeroInstance::asOwnable() const
 
 int CGHeroInstance::getBasePrimarySkillValue(PrimarySkill which) const
 {
-	std::string cachingStr = "type_PRIMARY_SKILL_base_" + std::to_string(static_cast<int>(which));
+	std::string cachingStr = "CGHeroInstance::getBasePrimarySkillValue" + std::to_string(static_cast<int>(which));
 	auto selector = Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(which)).And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL));
 	auto minSkillValue = VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, which.getNum());
 	return std::max(valOfBonuses(selector, cachingStr), minSkillValue);

lib/mapObjects/CGTownInstance.cpp

@@ -161,7 +161,7 @@ GrowthInfo CGTownInstance::getGrowthInfo(int level) const
 			ret.entries.emplace_back(subID, BuildingID::HORDE_2, creature->getHorde());
 
 	//statue-of-legion-like bonus: % to base+castle
-	TConstBonusListPtr bonuses2 = getBonuses(Selector::type()(BonusType::CREATURE_GROWTH_PERCENT));
+	TConstBonusListPtr bonuses2 = getBonusesOfType(BonusType::CREATURE_GROWTH_PERCENT);
 	for(const auto & b : *bonuses2)
 	{
 		const auto growth = b->val * (base + castleBonus) / 100;
@@ -173,7 +173,7 @@ GrowthInfo CGTownInstance::getGrowthInfo(int level) const
 
 	//other *-of-legion-like bonuses (%d to growth cumulative with grail)
 	// Note: bonus uses 1-based levels (Pikeman is level 1), town list uses 0-based (Pikeman in 0-th creatures entry)
-	TConstBonusListPtr bonuses = getBonuses(Selector::typeSubtype(BonusType::CREATURE_GROWTH, BonusCustomSubtype::creatureLevel(level+1)));
+	TConstBonusListPtr bonuses = getBonusesOfType(BonusType::CREATURE_GROWTH, BonusCustomSubtype::creatureLevel(level+1));
 	for(const auto & b : *bonuses)
 		ret.entries.emplace_back(b->val, b->Description(cb));
 

lib/pathfinder/TurnInfo.cpp

@@ -41,7 +41,7 @@ TurnInfo::TurnInfo(const CGHeroInstance * Hero, const int turn):
 	maxMovePointsWater(-1),
 	turn(turn)
 {
-	bonuses = hero->getAllBonuses(Selector::days(turn), Selector::all, "");
+	bonuses = hero->getAllBonuses(Selector::days(turn), Selector::all, "all_days" + std::to_string(turn));
 	bonusCache = std::make_unique<BonusCache>(bonuses);
 	nativeTerrain = hero->getNativeTerrain();
 }
@@ -125,7 +125,7 @@ int TurnInfo::getMaxMovePoints(const EPathfindingLayer & layer) const
 	return layer == EPathfindingLayer::SAIL ? maxMovePointsWater : maxMovePointsLand;
 }
 
-void TurnInfo::updateHeroBonuses(BonusType type, const CSelector& sel) const
+void TurnInfo::updateHeroBonuses(BonusType type) const
 {
 	switch(type)
 	{
@@ -144,7 +144,7 @@ void TurnInfo::updateHeroBonuses(BonusType type, const CSelector& sel) const
 		bonusCache->pathfindingVal = bonuses->valOfBonuses(Selector::type()(BonusType::ROUGH_TERRAIN_DISCOUNT));
 		break;
 	default:
-		bonuses = hero->getAllBonuses(Selector::days(turn), Selector::all, "");
+		bonuses = hero->getAllBonuses(Selector::days(turn), Selector::all, "all_days" + std::to_string(turn));
 	}
 }
 

lib/pathfinder/TurnInfo.h

@@ -46,7 +46,7 @@ struct DLL_LINKAGE TurnInfo
 	bool hasBonusOfType(const BonusType type, const BonusSubtypeID subtype) const;
 	int valOfBonuses(const BonusType type) const;
 	int valOfBonuses(const BonusType type, const BonusSubtypeID subtype) const;
-	void updateHeroBonuses(BonusType type, const CSelector& sel) const;
+	void updateHeroBonuses(BonusType type) const;
 	int getMaxMovePoints(const EPathfindingLayer & layer) const;
 };
 

lib/spells/TargetCondition.cpp

@@ -212,7 +212,7 @@ protected:
 	{
 		if(!m->isMagicalEffect()) //Always pass on non-magical
 			return true;
-		TConstBonusListPtr levelImmunities = target->getBonuses(Selector::type()(BonusType::LEVEL_SPELL_IMMUNITY));
+		TConstBonusListPtr levelImmunities = target->getBonusesOfType(BonusType::LEVEL_SPELL_IMMUNITY);
 		return levelImmunities->size() == 0 ||
 		levelImmunities->totalValue() < m->getSpellLevel() ||
 		m->getSpellLevel() <= 0;

server/battles/BattleFlowProcessor.cpp

@@ -198,7 +198,7 @@ void BattleFlowProcessor::castOpeningSpells(const CBattleInfoCallback & battle)
 		if (!h)
 			continue;
 
-		TConstBonusListPtr bl = h->getBonuses(Selector::type()(BonusType::OPENING_BATTLE_SPELL));
+		TConstBonusListPtr bl = h->getBonusesOfType(BonusType::OPENING_BATTLE_SPELL);
 
 		for (auto b : *bl)
 		{
@@ -629,7 +629,7 @@ bool BattleFlowProcessor::makeAutomaticAction(const CBattleInfoCallback & battle
 
 void BattleFlowProcessor::stackEnchantedTrigger(const CBattleInfoCallback & battle, const CStack * st)
 {
-	auto bl = *(st->getBonuses(Selector::type()(BonusType::ENCHANTED)));
+	auto bl = *(st->getBonusesOfType(BonusType::ENCHANTED));
 	for(auto b : bl)
 	{
 		if (!b->subtype.as<SpellID>().hasValue())
@@ -678,10 +678,10 @@ void BattleFlowProcessor::stackTurnTrigger(const CBattleInfoCallback & battle, c
 	if (st->alive())
 	{
 		//unbind
-		if (st->hasBonus(Selector::type()(BonusType::BIND_EFFECT)))
+		if (st->hasBonusOfType(BonusType::BIND_EFFECT))
 		{
 			bool unbind = true;
-			BonusList bl = *(st->getBonuses(Selector::type()(BonusType::BIND_EFFECT)));
+			BonusList bl = *(st->getBonusesOfType(BonusType::BIND_EFFECT));
 			auto adjacent = battle.battleAdjacentUnits(st);
 
 			for (auto b : bl)