From 9bbfb57b930de394d9d5a2580dc2f6d4446e222f Mon Sep 17 00:00:00 2001
From: Henning Koehler <henning.koehler.nz@gmail.com>
Date: Sun, 27 Aug 2017 15:35:04 +1200
Subject: [PATCH] cleaned up secondary skill bonus merging

---
 lib/CSkillHandler.cpp             |  2 +-
 lib/HeroBonus.cpp                 |  5 +++++
 lib/HeroBonus.h                   |  1 +
 lib/mapObjects/CGHeroInstance.cpp | 15 +++++++--------
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/lib/CSkillHandler.cpp b/lib/CSkillHandler.cpp
index 8d5504b61..a1f288cce 100644
--- a/lib/CSkillHandler.cpp
+++ b/lib/CSkillHandler.cpp
@@ -257,7 +257,7 @@ const std::shared_ptr<Bonus> CSkillHandler::defaultBonus(SecondarySkill skill, i
 	case SecondarySkill::FIRST_AID:
 		bonusVal = 25 + 25 * level; break;
 	default:
-		valueType = Bonus::INDEPENDENT_MIN; break;
+		break;
 	}
 
 	return std::make_shared<Bonus>(Bonus::PERMANENT, bonusType, Bonus::SECONDARY_SKILL, bonusVal, skill, skill, valueType);
diff --git a/lib/HeroBonus.cpp b/lib/HeroBonus.cpp
index 12af178eb..25d246669 100644
--- a/lib/HeroBonus.cpp
+++ b/lib/HeroBonus.cpp
@@ -1166,6 +1166,11 @@ namespace Selector
 		return CSelectFieldEqual<Bonus::BonusSource>(&Bonus::source)(source);
 	}
 
+	CSelector DLL_LINKAGE valueType(Bonus::ValueType valType)
+	{
+		return CSelectFieldEqual<Bonus::ValueType>(&Bonus::valType)(valType);
+	}
+
 	DLL_LINKAGE CSelector all([](const Bonus * b){return true;});
 	DLL_LINKAGE CSelector none([](const Bonus * b){return false;});
 
diff --git a/lib/HeroBonus.h b/lib/HeroBonus.h
index 7ba710a42..a3160af69 100644
--- a/lib/HeroBonus.h
+++ b/lib/HeroBonus.h
@@ -950,6 +950,7 @@ namespace Selector
 	CSelector DLL_LINKAGE typeSubtypeInfo(Bonus::BonusType type, TBonusSubtype subtype, si32 info);
 	CSelector DLL_LINKAGE source(Bonus::BonusSource source, ui32 sourceID);
 	CSelector DLL_LINKAGE sourceTypeSel(Bonus::BonusSource source);
+	CSelector DLL_LINKAGE valueType(Bonus::ValueType valType);
 
 	/**
 	 * Selects all bonuses
diff --git a/lib/mapObjects/CGHeroInstance.cpp b/lib/mapObjects/CGHeroInstance.cpp
index 6840e2fc2..a49dead50 100644
--- a/lib/mapObjects/CGHeroInstance.cpp
+++ b/lib/mapObjects/CGHeroInstance.cpp
@@ -769,15 +769,14 @@ void CGHeroInstance::updateSkill(SecondarySkill which, int val)
 	auto skillBonus = (*VLC->skillh)[which]->getBonus(val);
 	for (auto b : skillBonus)
 	{
-		// TODO: add standard method for joining bonuses, should match on valType as well
-		std::shared_ptr<Bonus> existing = getBonusLocalFirst(Selector::typeSubtype(b->type,b->subtype).And(Selector::source(Bonus::SECONDARY_SKILL, b->sid)));
+		// 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)
-		{
-			if(b->valType == Bonus::INDEPENDENT_MIN || b->valType == Bonus::BASE_NUMBER)
-				existing->val = b->val;
-			else
-				existing->val += b->val;
-		}
+			vstd::amax(existing->val, b->val);
 		else
 			addNewBonus(std::make_shared<Bonus>(*b));
 	}