From 4c0a67041bddac6b28039bba428a6f37844df394 Mon Sep 17 00:00:00 2001
From: Henning Koehler <henning.koehler.nz@gmail.com>
Date: Thu, 1 Mar 2018 08:44:05 +1300
Subject: [PATCH] secondary skill bonuses get updated via replacement (issue
 2796)

---
 lib/HeroBonus.cpp                 |  8 ++++++++
 lib/HeroBonus.h                   |  1 +
 lib/mapObjects/CGHeroInstance.cpp | 19 ++++---------------
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/lib/HeroBonus.cpp b/lib/HeroBonus.cpp
index eaaeb66d2..f682533a0 100644
--- a/lib/HeroBonus.cpp
+++ b/lib/HeroBonus.cpp
@@ -850,6 +850,14 @@ void CBonusSystemNode::removeBonus(const std::shared_ptr<Bonus>& b)
 	CBonusSystemNode::treeHasChanged();
 }
 
+void CBonusSystemNode::removeBonuses(const CSelector &selector)
+{
+	BonusList toRemove;
+	exportedBonuses.getBonuses(toRemove, selector);
+	for(auto bonus : toRemove)
+		removeBonus(bonus);
+}
+
 bool CBonusSystemNode::actsAsBonusSourceOnly() const
 {
 	switch(nodeType)
diff --git a/lib/HeroBonus.h b/lib/HeroBonus.h
index d71d96f77..f1bf8fae8 100644
--- a/lib/HeroBonus.h
+++ b/lib/HeroBonus.h
@@ -709,6 +709,7 @@ public:
 	void propagateBonus(std::shared_ptr<Bonus> b);
 	void unpropagateBonus(std::shared_ptr<Bonus> b);
 	void removeBonus(const std::shared_ptr<Bonus>& b);
+	void removeBonuses(const CSelector &selector);
 	void newRedDescendant(CBonusSystemNode *descendant); //propagation needed
 	void removedRedDescendant(CBonusSystemNode *descendant); //de-propagation needed
 
diff --git a/lib/mapObjects/CGHeroInstance.cpp b/lib/mapObjects/CGHeroInstance.cpp
index 4aa877720..9bd6bea8b 100644
--- a/lib/mapObjects/CGHeroInstance.cpp
+++ b/lib/mapObjects/CGHeroInstance.cpp
@@ -536,8 +536,8 @@ void CGHeroInstance::recreateSecondarySkillsBonuses()
 		removeBonus(bonus);
 
 	for(auto skill_info : secSkills)
-		for(int level = 1; level <= skill_info.second; level++)
-			updateSkill(SecondarySkill(skill_info.first), level);
+		if(skill_info.second > 0)
+			updateSkill(SecondarySkill(skill_info.first), skill_info.second);
 }
 
 void CGHeroInstance::recreateSpecialtyBonuses(std::vector<HeroSpecial *> & specialtyDeprecated)
@@ -559,21 +559,10 @@ void CGHeroInstance::recreateSpecialtyBonuses(std::vector<HeroSpecial *> & speci
 
 void CGHeroInstance::updateSkill(SecondarySkill which, int val)
 {
+	removeBonuses(Selector::source(Bonus::SECONDARY_SKILL, which));
 	auto skillBonus = (*VLC->skillh)[which]->getBonus(val);
 	for (auto b : skillBonus)
-	{
-		// bonuses provided by different levels of a secondary skill are aggregated via max (not + as usual)
-		// different secondary skills providing the same bonus (e.g. ballistics might improve archery as well) are kept separate
-		std::shared_ptr<Bonus> existing = getBonusLocalFirst(
-			Selector::typeSubtype(b->type, b->subtype).And(
-			Selector::source(Bonus::SECONDARY_SKILL, b->sid).And(
-			Selector::valueType(b->valType))));
-		if(existing)
-			vstd::amax(existing->val, b->val);
-		else
-			addNewBonus(std::make_shared<Bonus>(*b));
-	}
-	CBonusSystemNode::treeHasChanged();
+		addNewBonus(std::make_shared<Bonus>(*b));
 }
 void CGHeroInstance::setPropertyDer( ui8 what, ui32 val )
 {