Merge pull request #5261 from IvanSavenko/bonus_optimize

Bonus system optimization - per-node invalidation

AI/BattleAI/StackWithBonuses.cpp

@@ -170,7 +170,7 @@ TConstBonusListPtr StackWithBonuses::getAllBonuses(const CSelector & selector, c
 	return ret;
 }
 
-int64_t StackWithBonuses::getTreeVersion() const
+int32_t StackWithBonuses::getTreeVersion() const
 {
 	auto result = owner->getTreeVersion();
 
@@ -485,7 +485,7 @@ BattleLayout HypotheticBattle::getLayout() const
 	return subject->getBattle()->getLayout();
 }
 
-int64_t HypotheticBattle::getTreeVersion() const
+int32_t HypotheticBattle::getTreeVersion() const
 {
 	return getBonusBearer()->getTreeVersion() + bonusTreeVersion;
 }

AI/BattleAI/StackWithBonuses.h

@@ -93,7 +93,7 @@ public:
 	TConstBonusListPtr getAllBonuses(const CSelector & selector, const CSelector & limit,
 		const std::string & cachingStr = "") const override;
 
-	int64_t getTreeVersion() const override;
+	int32_t getTreeVersion() const override;
 
 	void addUnitBonus(const std::vector<Bonus> & bonus);
 	void updateUnitBonus(const std::vector<Bonus> & bonus);
@@ -162,7 +162,7 @@ public:
 	int3 getLocation() const override;
 	BattleLayout getLayout() const override;
 
-	int64_t getTreeVersion() const;
+	int32_t getTreeVersion() const;
 
 	void makeWait(const battle::Unit * activeStack);
 

AI/Nullkiller/Analyzers/BuildAnalyzer.cpp

@@ -20,7 +20,9 @@ void BuildAnalyzer::updateTownDwellings(TownDevelopmentInfo & developmentInfo)
 {
 	for(int level = 0; level < developmentInfo.town->getTown()->creatures.size(); level++)
 	{
+#if NKAI_TRACE_LEVEL >= 1
 		logAi->trace("Checking dwelling level %d", level);
+#endif
 		std::vector<BuildingID> dwellingsInTown;
 
 		for(BuildingID buildID = BuildingID::getDwellingFromLevel(level, 0); buildID.hasValue(); BuildingID::advanceDwelling(buildID))
@@ -143,9 +145,9 @@ void BuildAnalyzer::update()
 	{
 		if(town->built >= cb->getSettings().getInteger(EGameSettings::TOWNS_BUILDINGS_PER_TURN_CAP))
 			continue; // Not much point in trying anything - can't built in this town anymore today
-
+#if NKAI_TRACE_LEVEL >= 1
 		logAi->trace("Checking town %s", town->getNameTranslated());
-
+#endif
 		developmentInfos.push_back(TownDevelopmentInfo(town));
 		TownDevelopmentInfo & developmentInfo = developmentInfos.back();
 
@@ -161,10 +163,10 @@ void BuildAnalyzer::update()
 		}
 		armyCost += developmentInfo.armyCost;
 
+#if NKAI_TRACE_LEVEL >= 1
 		for(auto bi : developmentInfo.toBuild)
-		{
 			logAi->trace("Building preferences %s", bi.toString());
-		}
+#endif
 	}
 
 	std::sort(developmentInfos.begin(), developmentInfos.end(), [](const TownDevelopmentInfo & t1, const TownDevelopmentInfo & t2) -> bool
@@ -179,7 +181,9 @@ void BuildAnalyzer::update()
 
 	goldPressure = (ai->getLockedResources()[EGameResID::GOLD] + (float)armyCost[EGameResID::GOLD] + economyDevelopmentCost) / (1 + 2 * ai->getFreeGold() + (float)dailyIncome[EGameResID::GOLD] * 7.0f);
 
+#if NKAI_TRACE_LEVEL >= 1
 	logAi->trace("Gold pressure: %f", goldPressure);
+#endif
 }
 
 void BuildAnalyzer::reset()
@@ -268,12 +272,15 @@ BuildingInfo BuildAnalyzer::getBuildingOrPrerequisite(
 
 			if(vstd::contains_if(missingBuildings, otherDwelling))
 			{
+#if NKAI_TRACE_LEVEL >= 1
 				logAi->trace("cant build %d. Need other dwelling %d", toBuild.getNum(), missingBuildings.front().getNum());
+#endif
 			}
 			else if(missingBuildings[0] != toBuild)
 			{
+#if NKAI_TRACE_LEVEL >= 1
 				logAi->trace("cant build %d. Need %d", toBuild.getNum(), missingBuildings[0].num);
-
+#endif
 				BuildingInfo prerequisite = getBuildingOrPrerequisite(town, missingBuildings[0], excludeDwellingDependencies);
 
 				prerequisite.buildCostWithPrerequisites += info.buildCost;
@@ -298,19 +305,24 @@ BuildingInfo BuildAnalyzer::getBuildingOrPrerequisite(
 			}
 			else
 			{
+#if NKAI_TRACE_LEVEL >= 1
 				logAi->trace("Cant build. The building requires itself as prerequisite");
-
+#endif
 				return info;
 			}
 		}
 		else
 		{
+#if NKAI_TRACE_LEVEL >= 1
 			logAi->trace("Cant build. Reason: %d", static_cast<int>(canBuild));
+#endif
 		}
 	}
 	else
 	{
+#if NKAI_TRACE_LEVEL >= 1
 		logAi->trace("Dwelling %d exists", toBuild.getNum());
+#endif
 		info.exists = true;
 	}
 

AI/Nullkiller/Behaviors/DefenceBehavior.cpp

@@ -151,7 +151,9 @@ bool handleGarrisonHeroFromPreviousTurn(const CGTownInstance * town, Goals::TGoa
 
 void DefenceBehavior::evaluateDefence(Goals::TGoalVec & tasks, const CGTownInstance * town, const Nullkiller * ai) const
 {
+#if NKAI_TRACE_LEVEL >= 1
 	logAi->trace("Evaluating defence for %s", town->getNameTranslated());
+#endif
 
 	auto threatNode = ai->dangerHitMap->getObjectThreat(town);
 	std::vector<HitMapInfo> threats = ai->dangerHitMap->getTownThreats(town);
@@ -164,8 +166,9 @@ void DefenceBehavior::evaluateDefence(Goals::TGoalVec & tasks, const CGTownInsta
 	}
 	if(!threatNode.fastestDanger.hero)
 	{
+#if NKAI_TRACE_LEVEL >= 1
 		logAi->trace("No threat found for town %s", town->getNameTranslated());
-
+#endif
 		return;
 	}
 	
@@ -173,7 +176,9 @@ void DefenceBehavior::evaluateDefence(Goals::TGoalVec & tasks, const CGTownInsta
 
 	if(reinforcement)
 	{
+#if NKAI_TRACE_LEVEL >= 1
 		logAi->trace("Town %s can buy defence army %lld", town->getNameTranslated(), reinforcement);
+#endif
 		tasks.push_back(Goals::sptr(Goals::BuyArmy(town, reinforcement).setpriority(0.5f)));
 	}
 
@@ -181,13 +186,14 @@ void DefenceBehavior::evaluateDefence(Goals::TGoalVec & tasks, const CGTownInsta
 
 	for(auto & threat : threats)
 	{
+#if NKAI_TRACE_LEVEL >= 1
 		logAi->trace(
 			"Town %s has threat %lld in %s turns, hero: %s",
 			town->getNameTranslated(),
 			threat.danger,
 			std::to_string(threat.turn),
 			threat.hero ? threat.hero->getNameTranslated() : std::string("<no hero>"));
-
+#endif
 		handleCounterAttack(town, threat, threatNode.maximumDanger, ai, tasks);
 
 		if(isThreatUnderControl(town, threat, ai, paths))
@@ -199,7 +205,9 @@ void DefenceBehavior::evaluateDefence(Goals::TGoalVec & tasks, const CGTownInsta
 
 		if(paths.empty())
 		{
+#if NKAI_TRACE_LEVEL >= 1
 			logAi->trace("No ways to defend town %s", town->getNameTranslated());
+#endif
 
 			continue;
 		}

AI/Nullkiller/Engine/PriorityEvaluator.cpp

@@ -1201,7 +1201,9 @@ public:
 		if (bi.id == BuildingID::MARKETPLACE || bi.dailyIncome[EGameResID::WOOD] > 0)
 			evaluationContext.isTradeBuilding = true;
 
+#if NKAI_TRACE_LEVEL >= 1
 		logAi->trace("Building costs for %s : %s MarketValue: %d",bi.toString(), evaluationContext.buildingCost.toString(), evaluationContext.buildingCost.marketValue());
+#endif
 
 		if(bi.creatureID != CreatureID::NONE)
 		{

lib/CArtHandler.cpp

@@ -703,8 +703,9 @@ void CArtHandler::afterLoadFinalization()
 			assert(bonus->source == BonusSource::ARTIFACT);
 			bonus->sid = BonusSourceID(art->id);
 		}
+		art->nodeHasChanged();
 	}
-	CBonusSystemNode::treeHasChanged();
+
 }
 
 CArtifactInstance * CArtifactSet::getArt(const ArtifactPosition & pos, bool excludeLocked) const

lib/CCreatureSet.cpp

@@ -670,14 +670,18 @@ void CCreatureSet::serializeJson(JsonSerializeFormat & handler, const std::strin
 	}
 }
 
-CStackInstance::CStackInstance()
+CStackInstance::CStackInstance(bool isHypothetic)
-	: armyObj(_armyObj)
+	: CBonusSystemNode(isHypothetic),
+	armyObj(_armyObj),
+	nativeTerrain(this, Selector::type()(BonusType::TERRAIN_NATIVE)),
+	initiative(this, Selector::type()(BonusType::STACKS_SPEED))
+
 {
 	init();
 }
 
-CStackInstance::CStackInstance(const CreatureID & id, TQuantity Count, bool isHypothetic):
+CStackInstance::CStackInstance(const CreatureID & id, TQuantity Count, bool isHypothetic)
-	CBonusSystemNode(isHypothetic), armyObj(_armyObj)
+	: CStackInstance(false)
 {
 	init();
 	setType(id);
@@ -685,7 +689,7 @@ CStackInstance::CStackInstance(const CreatureID & id, TQuantity Count, bool isHy
 }
 
 CStackInstance::CStackInstance(const CCreature *cre, TQuantity Count, bool isHypothetic)
-	: CBonusSystemNode(isHypothetic), armyObj(_armyObj)
+	: CStackInstance(false)
 {
 	init();
 	setType(cre);
@@ -834,6 +838,22 @@ PlayerColor CStackInstance::getOwner() const
 	return _armyObj ? _armyObj->getOwner() : PlayerColor::NEUTRAL;
 }
 
+int32_t CStackInstance::getInitiative(int turn) const
+{
+	if (turn == 0)
+		return initiative.getValue();
+
+	return ACreature::getInitiative(turn);
+}
+
+TerrainId CStackInstance::getNativeTerrain() const
+{
+	if (nativeTerrain.hasBonus())
+		return TerrainId::ANY_TERRAIN;
+
+	return getFactionID().toEntity(VLC)->getNativeTerrain();
+}
+
 void CStackInstance::deserializationFix()
 {
 	const CArmedInstance *armyBackup = _armyObj;

lib/CCreatureSet.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include "bonuses/Bonus.h"
+#include "bonuses/BonusCache.h"
 #include "bonuses/CBonusSystemNode.h"
 #include "serializer/Serializeable.h"
 #include "GameConstants.h"
@@ -71,6 +72,9 @@ public:
 
 class DLL_LINKAGE CStackInstance : public CBonusSystemNode, public CStackBasicDescriptor, public CArtifactSet, public ACreature
 {
+	BonusValueCache nativeTerrain;
+	BonusValueCache initiative;
+
 protected:
 	const CArmedInstance *_armyObj; //stack must be part of some army, army must be part of some object
 
@@ -119,7 +123,7 @@ public:
 	CreatureID getCreatureID() const; //-1 if not available
 	std::string getName() const; //plural or singular
 	virtual void init();
-	CStackInstance();
+	CStackInstance(bool isHypothetic = false);
 	CStackInstance(const CreatureID & id, TQuantity count, bool isHypothetic = false);
 	CStackInstance(const CCreature *cre, TQuantity count, bool isHypothetic = false);
 	virtual ~CStackInstance() = default;
@@ -135,6 +139,9 @@ public:
 	std::string nodeName() const override; //from CBonusSystemnode
 	void deserializationFix();
 	PlayerColor getOwner() const override;
+
+	int32_t getInitiative(int turn = 0) const final;
+	TerrainId getNativeTerrain() const final;
 };
 
 class DLL_LINKAGE CCommanderInstance : public CStackInstance

lib/battle/BattleInfo.cpp

@@ -693,7 +693,7 @@ void BattleInfo::moveUnit(uint32_t id, BattleHex destination)
 	sta->position = destination;
 	//Bonuses can be limited by unit placement, so, change tree version 
 	//to force updating a bonus. TODO: update version only when such bonuses are present
-	CBonusSystemNode::treeHasChanged();
+	nodeHasChanged();
 }
 
 void BattleInfo::setUnitState(uint32_t id, const JsonNode & data, int64_t healthDelta)
@@ -890,7 +890,7 @@ void BattleInfo::addOrUpdateUnitBonus(CStack * sta, const Bonus & value, bool fo
 				stackBonus->turnsRemain = std::max(stackBonus->turnsRemain, value.turnsRemain);
 			}
 		}
-		CBonusSystemNode::treeHasChanged();
+		sta->nodeHasChanged();
 	}
 }
 

lib/battle/CUnitState.cpp

@@ -953,7 +953,7 @@ TConstBonusListPtr CUnitStateDetached::getAllBonuses(const CSelector & selector,
 	return bonus->getAllBonuses(selector, limit, cachingStr);
 }
 
-int64_t CUnitStateDetached::getTreeVersion() const
+int32_t CUnitStateDetached::getTreeVersion() const
 {
 	return bonus->getTreeVersion();
 }

lib/battle/CUnitState.h

@@ -279,7 +279,7 @@ public:
 
 	TConstBonusListPtr getAllBonuses(const CSelector & selector, const CSelector & limit, const std::string & cachingStr = "") const override;
 
-	int64_t getTreeVersion() const override;
+	int32_t getTreeVersion() const override;
 
 	uint32_t unitId() const override;
 	BattleSide unitSide() const override;

lib/bonuses/BonusCache.h

@@ -32,8 +32,8 @@ protected:
 
 	struct BonusCacheEntry
 	{
-		std::atomic<int64_t> version = 0;
+		std::atomic<int32_t> version = 0;
-		std::atomic<int64_t> value = 0;
+		std::atomic<int32_t> value = 0;
 
 		BonusCacheEntry() = default;
 		BonusCacheEntry(const BonusCacheEntry & other)
@@ -152,7 +152,7 @@ private:
 class PrimarySkillsCache
 {
 	const IBonusBearer * target;
-	mutable std::atomic<int64_t> version = 0;
+	mutable std::atomic<int32_t> version = 0;
 	mutable std::array<std::atomic<int32_t>, 4> skills;
 
 	void update() const;
@@ -166,7 +166,7 @@ public:
 class MagicSchoolMasteryCache
 {
 	const IBonusBearer * target;
-	mutable std::atomic<int64_t> version = 0;
+	mutable std::atomic<int32_t> version = 0;
 	mutable std::array<std::atomic<int32_t>, 4+1> schools;
 
 	void update() const;
@@ -184,7 +184,7 @@ class BonusCachePerTurn : public BonusCacheBase
 	const CSelector selector;
 	mutable TConstBonusListPtr bonusList;
 	mutable std::mutex bonusListMutex;
-	mutable std::atomic<int64_t> bonusListVersion = 0;
+	mutable std::atomic<int32_t> bonusListVersion = 0;
 	mutable std::array<BonusCacheEntry, cachedTurns> cache;
 	const BonusCacheMode mode;
 

lib/bonuses/BonusList.cpp

@@ -14,36 +14,6 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-BonusList::BonusList(bool BelongsToTree) : belongsToTree(BelongsToTree)
-{
-}
-
-BonusList::BonusList(const BonusList & bonusList): belongsToTree(false)
-{
-	bonuses.resize(bonusList.size());
-	std::copy(bonusList.begin(), bonusList.end(), bonuses.begin());
-}
-
-BonusList::BonusList(BonusList && other) noexcept: belongsToTree(false)
-{
-	std::swap(belongsToTree, other.belongsToTree);
-	std::swap(bonuses, other.bonuses);
-}
-
-BonusList& BonusList::operator=(const BonusList &bonusList)
-{
-	bonuses.resize(bonusList.size());
-	std::copy(bonusList.begin(), bonusList.end(), bonuses.begin());
-	belongsToTree = false;
-	return *this;
-}
-
-void BonusList::changed() const
-{
-    if(belongsToTree)
-		CBonusSystemNode::treeHasChanged();
-}
-
 void BonusList::stackBonuses()
 {
 	boost::sort(bonuses, [](const std::shared_ptr<Bonus> & b1, const std::shared_ptr<Bonus> & b2) -> bool
@@ -228,19 +198,16 @@ JsonNode BonusList::toJsonNode() const
 void BonusList::push_back(const std::shared_ptr<Bonus> & x)
 {
 	bonuses.push_back(x);
-	changed();
 }
 
 BonusList::TInternalContainer::iterator BonusList::erase(const int position)
 {
-	changed();
 	return bonuses.erase(bonuses.begin() + position);
 }
 
 void BonusList::clear()
 {
 	bonuses.clear();
-	changed();
 }
 
 std::vector<BonusList *>::size_type BonusList::operator-=(const std::shared_ptr<Bonus> & i)
@@ -249,20 +216,17 @@ std::vector<BonusList *>::size_type BonusList::operator-=(const std::shared_ptr<
 	if(itr == bonuses.end())
 		return false;
 	bonuses.erase(itr);
-	changed();
 	return true;
 }
 
 void BonusList::resize(BonusList::TInternalContainer::size_type sz, const std::shared_ptr<Bonus> & c)
 {
 	bonuses.resize(sz, c);
-	changed();
 }
 
 void BonusList::insert(BonusList::TInternalContainer::iterator position, BonusList::TInternalContainer::size_type n, const std::shared_ptr<Bonus> & x)
 {
 	bonuses.insert(position, n, x);
-	changed();
 }
 
 DLL_LINKAGE std::ostream & operator<<(std::ostream &out, const BonusList &bonusList)

lib/bonuses/BonusList.h

@@ -21,8 +21,6 @@ public:
 
 private:
 	TInternalContainer bonuses;
-	bool belongsToTree;
-	void changed() const;
 
 public:
 	using const_reference = TInternalContainer::const_reference;
@@ -31,10 +29,7 @@ public:
 	using const_iterator = TInternalContainer::const_iterator;
 	using iterator = TInternalContainer::iterator;
 
-	BonusList(bool BelongsToTree = false);
+	BonusList() = default;
-	BonusList(const BonusList &bonusList);
-	BonusList(BonusList && other) noexcept;
-	BonusList& operator=(const BonusList &bonusList);
 
 	// wrapper functions of the STL vector container
 	TInternalContainer::size_type size() const { return bonuses.size(); }

lib/bonuses/CBonusSystemNode.cpp

@@ -17,8 +17,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-std::atomic<int64_t> CBonusSystemNode::treeChanged(1);
+constexpr bool cachingEnabled = true;
-constexpr bool CBonusSystemNode::cachingEnabled = true;
 
 std::shared_ptr<Bonus> CBonusSystemNode::getLocalBonus(const CSelector & selector)
 {
@@ -97,11 +96,11 @@ void CBonusSystemNode::getAllBonusesRec(BonusList &out, const CSelector & select
 
 TConstBonusListPtr CBonusSystemNode::getAllBonuses(const CSelector &selector, const CSelector &limit, const std::string &cachingStr) const
 {
-	if (CBonusSystemNode::cachingEnabled)
+	if (cachingEnabled)
 	{
 		// If a bonus system request comes with a caching string then look up in the map if there are any
 		// pre-calculated bonus results. Limiters can't be cached so they have to be calculated.
-		if (cachedLast == treeChanged && !cachingStr.empty())
+		if (cachedLast == nodeChanged && !cachingStr.empty())
 		{
 			RequestsMap::const_accessor accessor;
 
@@ -114,7 +113,7 @@ TConstBonusListPtr CBonusSystemNode::getAllBonuses(const CSelector &selector, co
 		//Perform bonus selection
 		auto ret = std::make_shared<BonusList>();
 
-		if (cachedLast == treeChanged)
+		if (cachedLast == nodeChanged)
 		{
 			// Cached bonuses are up-to-date - use shared/read access and compute results
 			std::shared_lock lock(sync);
@@ -125,7 +124,7 @@ TConstBonusListPtr CBonusSystemNode::getAllBonuses(const CSelector &selector, co
 			// If the bonus system tree changes(state of a single node or the relations to each other) then
 			// cache all bonus objects. Selector objects doesn't matter.
 			std::lock_guard lock(sync);
-			if (cachedLast == treeChanged)
+			if (cachedLast == nodeChanged)
 			{
 				// While our thread was waiting, another one have updated bonus tree. Use cached bonuses.
 				cachedBonuses.getBonuses(*ret, selector, limit);
@@ -140,7 +139,7 @@ TConstBonusListPtr CBonusSystemNode::getAllBonuses(const CSelector &selector, co
 				getAllBonusesRec(allBonuses, Selector::all);
 				limitBonuses(allBonuses, cachedBonuses);
 				cachedBonuses.stackBonuses();
-				cachedLast = treeChanged;
+				cachedLast = nodeChanged;
 				cachedBonuses.getBonuses(*ret, selector, limit);
 			}
 		}
@@ -155,7 +154,7 @@ TConstBonusListPtr CBonusSystemNode::getAllBonuses(const CSelector &selector, co
 				accessor->second.first = cachedLast;
 			}
 			else
-				cachedRequests.emplace(cachingStr, std::pair<int64_t, TBonusListPtr>{ cachedLast, ret });
+				cachedRequests.emplace(cachingStr, std::pair<int32_t, TBonusListPtr>{ cachedLast, ret });
 		}
 
 		return ret;
@@ -187,8 +186,6 @@ std::shared_ptr<Bonus> CBonusSystemNode::getUpdatedBonus(const std::shared_ptr<B
 }
 
 CBonusSystemNode::CBonusSystemNode(bool isHypotetic):
-	bonuses(true),
-	exportedBonuses(true),
 	nodeType(UNKNOWN),
 	cachedLast(0),
 	isHypotheticNode(isHypotetic)
@@ -196,8 +193,6 @@ CBonusSystemNode::CBonusSystemNode(bool isHypotetic):
 }
 
 CBonusSystemNode::CBonusSystemNode(ENodeTypes NodeType):
-	bonuses(true),
-	exportedBonuses(true),
 	nodeType(NodeType),
 	cachedLast(0),
 	isHypotheticNode(false)
@@ -227,10 +222,11 @@ void CBonusSystemNode::attachTo(CBonusSystemNode & parent)
 		if(!parent.actsAsBonusSourceOnly())
 			newRedDescendant(parent);
 
-		parent.newChildAttached(*this);
+		assert(!vstd::contains(parent.children, this));
+		parent.children.push_back(this);
 	}
 
-	CBonusSystemNode::treeHasChanged();
+	nodeHasChanged();
 }
 
 void CBonusSystemNode::attachToSource(const CBonusSystemNode & parent)
@@ -244,7 +240,7 @@ void CBonusSystemNode::attachToSource(const CBonusSystemNode & parent)
 			parent.newRedDescendant(*this);
 	}
 
-	CBonusSystemNode::treeHasChanged();
+	nodeHasChanged();
 }
 
 void CBonusSystemNode::detachFrom(CBonusSystemNode & parent)
@@ -271,9 +267,15 @@ void CBonusSystemNode::detachFrom(CBonusSystemNode & parent)
 
 	if(!isHypothetic())
 	{
-		parent.childDetached(*this);
+		if(vstd::contains(parent.children, this))
+			parent.children -= this;
+		else
+		{
+			logBonus->error("Error on Detach. Node %s (nodeType=%d) is not a child of %s (nodeType=%d)"
+							, nodeShortInfo(), nodeType, parent.nodeShortInfo(), parent.nodeType);
 		}
-	CBonusSystemNode::treeHasChanged();
+	}
+	nodeHasChanged();
 }
 
 
@@ -297,7 +299,7 @@ void CBonusSystemNode::detachFromSource(const CBonusSystemNode & parent)
 			, nodeShortInfo(), nodeType, parent.nodeShortInfo(), parent.nodeType);
 	}
 
-	CBonusSystemNode::treeHasChanged();
+	nodeHasChanged();
 }
 
 void CBonusSystemNode::removeBonusesRecursive(const CSelector & s)
@@ -333,7 +335,6 @@ void CBonusSystemNode::addNewBonus(const std::shared_ptr<Bonus>& b)
 	assert(!vstd::contains(exportedBonuses, b));
 	exportedBonuses.push_back(b);
 	exportBonus(b);
-	CBonusSystemNode::treeHasChanged();
 }
 
 void CBonusSystemNode::accumulateBonus(const std::shared_ptr<Bonus>& b)
@@ -349,10 +350,14 @@ void CBonusSystemNode::removeBonus(const std::shared_ptr<Bonus>& b)
 {
 	exportedBonuses -= b;
 	if(b->propagator)
+	{
 		unpropagateBonus(b);
+	}
 	else
+	{
 		bonuses -= b;
-	CBonusSystemNode::treeHasChanged();
+		nodeHasChanged();
+	}
 }
 
 void CBonusSystemNode::removeBonuses(const CSelector & selector)
@@ -385,6 +390,7 @@ void CBonusSystemNode::propagateBonus(const std::shared_ptr<Bonus> & b, const CB
 			: b;
 		bonuses.push_back(propagated);
 		logBonus->trace("#$# %s #propagated to# %s",  propagated->Description(nullptr), nodeName());
+		nodeHasChanged();
 	}
 
 	TNodes lchildren;
@@ -402,11 +408,11 @@ void CBonusSystemNode::unpropagateBonus(const std::shared_ptr<Bonus> & b)
 		else
 			logBonus->warn("Attempt to remove #$# %s, which is not propagated to %s", b->Description(nullptr), nodeName());
 
-		bonuses.remove_if([b](const auto & bonus)
+		bonuses.remove_if([this, b](const auto & bonus)
 		{
 			if (bonus->propagationUpdater && bonus->propagationUpdater == b->propagationUpdater)
 			{
-				treeHasChanged();
+				nodeHasChanged();
 				return true;
 			}
 			return false;
@@ -419,23 +425,6 @@ void CBonusSystemNode::unpropagateBonus(const std::shared_ptr<Bonus> & b)
 		pname->unpropagateBonus(b);
 }
 
-void CBonusSystemNode::newChildAttached(CBonusSystemNode & child)
-{
-	assert(!vstd::contains(children, &child));
-	children.push_back(&child);
-}
-
-void CBonusSystemNode::childDetached(CBonusSystemNode & child)
-{
-	if(vstd::contains(children, &child))
-		children -= &child;
-	else
-	{
-		logBonus->error("Error on Detach. Node %s (nodeType=%d) is not a child of %s (nodeType=%d)"
-			, child.nodeShortInfo(), child.nodeType, nodeShortInfo(), nodeType);
-	}
-}
-
 void CBonusSystemNode::detachFromAll()
 {
 	while(!parentsToPropagate.empty())
@@ -558,11 +547,14 @@ void CBonusSystemNode::getRedAncestors(TCNodes &out) const
 void CBonusSystemNode::exportBonus(const std::shared_ptr<Bonus> & b)
 {
 	if(b->propagator)
+	{
 		propagateBonus(b, *this);
+	}
 	else
+	{
 		bonuses.push_back(b);
-
+		nodeHasChanged();
-	CBonusSystemNode::treeHasChanged();
+	}
 }
 
 void CBonusSystemNode::exportBonuses()
@@ -631,14 +623,27 @@ void CBonusSystemNode::limitBonuses(const BonusList &allBonuses, BonusList &out)
 	}
 }
 
-void CBonusSystemNode::treeHasChanged()
+void CBonusSystemNode::nodeHasChanged()
 {
-	treeChanged++;
+	static std::atomic<int32_t> globalCounter = 1;
+
+	invalidateChildrenNodes(++globalCounter);
 }
 
-int64_t CBonusSystemNode::getTreeVersion() const
+void CBonusSystemNode::invalidateChildrenNodes(int32_t changeCounter)
 {
-	return treeChanged;
+	if (nodeChanged == changeCounter)
+		return;
+
+	nodeChanged = changeCounter;
+
+	for(CBonusSystemNode * child : children)
+		child->invalidateChildrenNodes(changeCounter);
+}
+
+int32_t CBonusSystemNode::getTreeVersion() const
+{
+	return nodeChanged;
 }
 
 VCMI_LIB_NAMESPACE_END

lib/bonuses/CBonusSystemNode.h

@@ -56,15 +56,16 @@ private:
 	ENodeTypes nodeType;
 	bool isHypotheticNode;
 
-	static const bool cachingEnabled;
 	mutable BonusList cachedBonuses;
-	mutable int64_t cachedLast;
+	mutable int32_t cachedLast;
-	static std::atomic<int64_t> treeChanged;
+	std::atomic<int32_t> nodeChanged;
+
+	void invalidateChildrenNodes(int32_t changeCounter);
 
 	// Setting a value to cachingStr before getting any bonuses caches the result for later requests.
 	// This string needs to be unique, that's why it has to be set in the following manner:
 	// [property key]_[value] => only for selector
-	using RequestsMap = tbb::concurrent_hash_map<std::string, std::pair<int64_t, TBonusListPtr>, HashStringCompare>;
+	using RequestsMap = tbb::concurrent_hash_map<std::string, std::pair<int32_t, TBonusListPtr>, HashStringCompare>;
 	mutable RequestsMap cachedRequests;
 	mutable std::shared_mutex sync;
 
@@ -79,8 +80,6 @@ private:
 
 	void getAllParents(TCNodes & out) const;
 
-	void newChildAttached(CBonusSystemNode & child);
-	void childDetached(CBonusSystemNode & child);
 	void propagateBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & source);
 	void unpropagateBonus(const std::shared_ptr<Bonus> & b);
 	bool actsAsBonusSourceOnly() const;
@@ -136,9 +135,9 @@ public:
 	void setNodeType(CBonusSystemNode::ENodeTypes type);
 	const TCNodesVector & getParentNodes() const;
 
-	static void treeHasChanged();
+	void nodeHasChanged();
 
-	int64_t getTreeVersion() const override;
+	int32_t getTreeVersion() const override;
 
 	virtual PlayerColor getOwner() const
 	{

lib/bonuses/IBonusBearer.h

@@ -41,7 +41,7 @@ public:
 	TConstBonusListPtr getBonusesOfType(BonusType type) const;
 	TConstBonusListPtr getBonusesOfType(BonusType type, BonusSubtypeID subtype) const;
 
-	virtual int64_t getTreeVersion() const = 0;
+	virtual int32_t getTreeVersion() const = 0;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/CArmedInstance.cpp

@@ -115,7 +115,7 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 	
 	b->description = bonusDescription;
 
-	CBonusSystemNode::treeHasChanged();
+	nodeHasChanged();
 
 	//-1 modifier for any Undead unit in army
 	auto undeadModifier = getExportedBonusList().getFirst(Selector::source(BonusSource::ARMY, BonusCustomSource::undeadMoraleDebuff));

lib/mapObjects/CGHeroInstance.cpp

@@ -1513,7 +1513,7 @@ void CGHeroInstance::setPrimarySkill(PrimarySkill primarySkill, si64 value, ui8
 		{
 			skill->val += static_cast<si32>(value);
 		}
-		CBonusSystemNode::treeHasChanged();
+		nodeHasChanged();
 	}
 	else if(primarySkill == PrimarySkill::EXPERIENCE)
 	{
@@ -1550,7 +1550,7 @@ void CGHeroInstance::levelUp(const std::vector<SecondarySkill> & skills)
 	}
 
 	//update specialty and other bonuses that scale with level
-	treeHasChanged();
+	nodeHasChanged();
 }
 
 void CGHeroInstance::levelUpAutomatically(vstd::RNG & rand)

lib/mapObjects/CGTownInstance.cpp

@@ -730,7 +730,7 @@ void CGTownInstance::updateMoraleBonusFromArmy()
 	if (garrisonHero)
 	{
 		b->val = 0;
-		CBonusSystemNode::treeHasChanged();
+		nodeHasChanged();
 	}
 	else
 		CArmedInstance::updateMoraleBonusFromArmy();

lib/networkPacks/NetPacksLib.cpp

@@ -901,6 +901,7 @@ void SetCommanderProperty::applyGs(CGameState *gs)
 			break;
 		case EXPERIENCE:
 			commander->giveStackExp(amount); //TODO: allow setting exp for stacks via netpacks
+			commander->nodeHasChanged();
 			break;
 	}
 }
@@ -1708,7 +1709,9 @@ void RebalanceStacks::applyGs(CGameState *gs)
 		}
 	}
 
-	CBonusSystemNode::treeHasChanged();
+	srcObj->nodeHasChanged();
+	if (srcObj != dstObj)
+		dstObj->nodeHasChanged();
 }
 
 void BulkRebalanceStacks::applyGs(CGameState *gs)
@@ -2147,10 +2150,16 @@ void BattleResultAccepted::applyGs(CGameState *gs)
 	if(gs->getSettings().getBoolean(EGameSettings::MODULE_STACK_EXPERIENCE))
 	{
 		if(heroResult[BattleSide::ATTACKER].army)
+		{
 			heroResult[BattleSide::ATTACKER].army->giveStackExp(heroResult[BattleSide::ATTACKER].exp);
+			heroResult[BattleSide::ATTACKER].army->nodeHasChanged();
+		}
 		if(heroResult[BattleSide::DEFENDER].army)
+		{
 			heroResult[BattleSide::DEFENDER].army->giveStackExp(heroResult[BattleSide::DEFENDER].exp);
-		CBonusSystemNode::treeHasChanged();
+			heroResult[BattleSide::DEFENDER].army->nodeHasChanged();
+		}
+
 	}
 
 	auto currentBattle = boost::range::find_if(gs->currentBattles, [&](const auto & battle)

server/CGameHandler.cpp

@@ -378,7 +378,6 @@ void CGameHandler::giveExperience(const CGHeroInstance * hero, TExpType amountTo
 		scp.which = SetCommanderProperty::EXPERIENCE;
 		scp.amount = amountToGain;
 		sendAndApply(scp);
-		CBonusSystemNode::treeHasChanged();
 	}
 
 	expGiven(hero);

test/mock/mock_BonusBearer.cpp

@@ -38,7 +38,7 @@ TConstBonusListPtr BonusBearerMock::getAllBonuses(const CSelector & selector, co
 	return ret;
 }
 
-int64_t BonusBearerMock::getTreeVersion() const
+int32_t BonusBearerMock::getTreeVersion() const
 {
 	return treeVersion;
 }

test/mock/mock_BonusBearer.h

@@ -25,10 +25,10 @@ public:
 
 	TConstBonusListPtr getAllBonuses(const CSelector & selector, const CSelector & limit, const std::string & cachingStr = "") const override;
 
-	int64_t getTreeVersion() const override;
+	int32_t getTreeVersion() const override;
 private:
 	mutable BonusList bonuses;
 
-	mutable int64_t cachedLast;
+	mutable int32_t cachedLast;
 	int32_t treeVersion;
 };

test/mock/mock_battle_Unit.h

@@ -16,7 +16,7 @@ class UnitMock : public battle::Unit
 {
 public:
 	MOCK_CONST_METHOD3(getAllBonuses, TConstBonusListPtr(const CSelector &, const CSelector &, const std::string &));
-	MOCK_CONST_METHOD0(getTreeVersion, int64_t());
+	MOCK_CONST_METHOD0(getTreeVersion, int32_t());
 
 	MOCK_CONST_METHOD0(getCasterUnitId, int32_t());
 	MOCK_CONST_METHOD2(getSpellSchoolLevel, int32_t(const spells::Spell *, SpellSchool *));