Optimized getHeroStrength method

- replaced 4x access to bonus system with single access
- fixed formula for Diplomacy
- fxied formula for hero transfer in campaigns
- removed pointless sqrt(pow()) construct

lib/campaign/CampaignState.cpp

@@ -339,7 +339,7 @@ void CampaignState::setCurrentMapAsConquered(std::vector<CGHeroInstance *> heroe
 {
 	range::sort(heroes, [](const CGHeroInstance * a, const CGHeroInstance * b)
 	{
-		return a->getHeroStrengthForCampaign() > b->getHeroStrengthForCampaign();
+		return a->getValueForCampaign() > b->getValueForCampaign();
 	});
 
 	logGlobal->info("Scenario %d of campaign %s (%s) has been completed", currentMap->getNum(), getFilename(), getNameTranslated());

lib/constants/EntityIdentifiers.cpp

@@ -97,8 +97,6 @@ const PrimarySkill PrimarySkill::ATTACK(0);
 const PrimarySkill PrimarySkill::DEFENSE(1);
 const PrimarySkill PrimarySkill::SPELL_POWER(2);
 const PrimarySkill PrimarySkill::KNOWLEDGE(3);
-const PrimarySkill PrimarySkill::BEGIN(0);
-const PrimarySkill PrimarySkill::END(4);
 const PrimarySkill PrimarySkill::EXPERIENCE(4);
 
 const BoatId BoatId::NONE(-1);
@@ -630,6 +628,18 @@ std::string GameResID::entityType()
 	return "resource";
 }
 
+const std::array<PrimarySkill, 4> & PrimarySkill::ALL_SKILLS()
+{
+	static const std::array allSkills = {
+		PrimarySkill(ATTACK),
+		PrimarySkill(DEFENSE),
+		PrimarySkill(SPELL_POWER),
+		PrimarySkill(KNOWLEDGE)
+	};
+
+	return allSkills;
+}
+
 const std::array<GameResID, 7> & GameResID::ALL_RESOURCES()
 {
 	static const std::array allResources = {

lib/constants/EntityIdentifiers.h

@@ -230,8 +230,7 @@ public:
 	static const PrimarySkill SPELL_POWER;
 	static const PrimarySkill KNOWLEDGE;
 
-	static const PrimarySkill BEGIN;
-	static const PrimarySkill END;
+	static const std::array<PrimarySkill, 4> & ALL_SKILLS();
 
 	static const PrimarySkill EXPERIENCE;
 

lib/gameState/CGameStateCampaign.cpp

@@ -82,13 +82,13 @@ void CGameStateCampaign::trimCrossoverHeroesParameters(const CampaignTravel & tr
 		//trimming prim skills
 		for(auto & hero : campaignHeroReplacements)
 		{
-			for(auto g = PrimarySkill::BEGIN; g < PrimarySkill::END; ++g)
+			for(auto skill : PrimarySkill::ALL_SKILLS())
 			{
 				auto sel = Selector::type()(BonusType::PRIMARY_SKILL)
-					.And(Selector::subtype()(BonusSubtypeID(g)))
+					.And(Selector::subtype()(BonusSubtypeID(skill)))
 					.And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL));
 
-				hero.hero->getLocalBonus(sel)->val = hero.hero->getHeroClass()->primarySkillInitial[g.getNum()];
+				hero.hero->getLocalBonus(sel)->val = hero.hero->getHeroClass()->primarySkillInitial[skill.getNum()];
 			}
 		}
 	}
@@ -337,14 +337,14 @@ void CGameStateCampaign::giveCampaignBonusToHero(CGHeroInstance * hero)
 		case CampaignBonusType::PRIMARY_SKILL:
 		{
 			const ui8 * ptr = reinterpret_cast<const ui8 *>(&curBonus->info2);
-			for(auto g = PrimarySkill::BEGIN; g < PrimarySkill::END; ++g)
+			for(auto skill : PrimarySkill::ALL_SKILLS())
 			{
-				int val = ptr[g.getNum()];
+				int val = ptr[skill.getNum()];
 				if(val == 0)
 					continue;
 
 				auto currentScenario = *gameState->scenarioOps->campState->currentScenario();
-				auto bb = std::make_shared<Bonus>( BonusDuration::PERMANENT, BonusType::PRIMARY_SKILL, BonusSource::CAMPAIGN_BONUS, val, BonusSourceID(currentScenario), BonusSubtypeID(g) );
+				auto bb = std::make_shared<Bonus>( BonusDuration::PERMANENT, BonusType::PRIMARY_SKILL, BonusSource::CAMPAIGN_BONUS, val, BonusSourceID(currentScenario), BonusSubtypeID(skill) );
 				hero->addNewBonus(bb);
 			}
 			break;
@@ -551,7 +551,7 @@ void CGameStateCampaign::initHeroes()
 			int maxB = -1;
 			for (int b=0; b<heroes.size(); ++b)
 			{
-				if (maxB == -1 || heroes[b]->getTotalStrength() > heroes[maxB]->getTotalStrength())
+				if (maxB == -1 || heroes[b]->getValueForCampaign() > heroes[maxB]->getValueForCampaign())
 				{
 					maxB = b;
 				}

lib/mapObjects/CGCreature.cpp

@@ -336,7 +336,7 @@ void CGCreature::setPropertyDer(ObjProperty what, ObjPropertyID identifier)
 int CGCreature::takenAction(const CGHeroInstance *h, bool allowJoin) const
 {
 	//calculate relative strength of hero and creatures armies
-	double relStrength = static_cast<double>(h->getTotalStrength()) / getArmyStrength();
+	double relStrength = static_cast<double>(h->getValueForDiplomacy()) / getArmyStrength();
 
 	int powerFactor;
 	if(relStrength >= 7)

lib/mapObjects/CGHeroInstance.cpp

@@ -704,19 +704,46 @@ void CGHeroInstance::setPropertyDer(ObjProperty what, ObjPropertyID identifier)
 		setStackCount(SlotID(0), identifier.getNum());
 }
 
+std::array<int, 4> CGHeroInstance::getPrimarySkills() const
+{
+	std::array<int, 4> result;
+
+	auto allSkills = getBonusBearer()->getBonusesOfType(BonusType::PRIMARY_SKILL);
+	for (auto skill : PrimarySkill::ALL_SKILLS())
+	{
+		int ret = allSkills->valOfBonuses(Selector::subtype()(BonusSubtypeID(skill)));
+		int minSkillValue = VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, skill.getNum());
+		result[skill] = std::max(ret, minSkillValue); //otherwise, some artifacts may cause negative skill value effect
+	}
+
+	return result;
+}
+
 double CGHeroInstance::getFightingStrength() const
 {
-	return sqrt((1.0 + 0.05*getPrimSkillLevel(PrimarySkill::ATTACK)) * (1.0 + 0.05*getPrimSkillLevel(PrimarySkill::DEFENSE)));
+	const auto & primarySkills = getPrimarySkills();
+	return getFightingStrengthImpl(primarySkills);
+}
+
+double CGHeroInstance::getFightingStrengthImpl(const std::array<int, 4> & primarySkills) const
+{
+	return sqrt((1.0 + 0.05*primarySkills[PrimarySkill::ATTACK]) * (1.0 + 0.05*primarySkills[PrimarySkill::DEFENSE]));
 }
 
 double CGHeroInstance::getMagicStrength() const
+{
+	const auto & primarySkills = getPrimarySkills();
+	return getMagicStrengthImpl(primarySkills);
+}
+
+double CGHeroInstance::getMagicStrengthImpl(const std::array<int, 4> & primarySkills) const
 {
 	if (!hasSpellbook())
 		return 1;
 	bool atLeastOneCombatSpell = false;
 	for (auto spell : spells)
 	{
-		if (spellbookContainsSpell(spell) && spell.toSpell()->isCombat())
+		if (spell.toSpell()->isCombat())
 		{
 			atLeastOneCombatSpell = true;
 			break;
@@ -724,22 +751,40 @@ double CGHeroInstance::getMagicStrength() const
 	}
 	if (!atLeastOneCombatSpell)
 		return 1;
-	return sqrt((1.0 + 0.05*getPrimSkillLevel(PrimarySkill::KNOWLEDGE) * mana / manaLimit()) * (1.0 + 0.05*getPrimSkillLevel(PrimarySkill::SPELL_POWER) * mana / manaLimit()));
+	return sqrt((1.0 + 0.05*primarySkills[PrimarySkill::KNOWLEDGE] * mana / manaLimit()) * (1.0 + 0.05*primarySkills[PrimarySkill::SPELL_POWER] * mana / manaLimit()));
 }
 
-double CGHeroInstance::getMagicStrengthForCampaign() const
+double CGHeroInstance::getHeroStrength() const
 {
-	return sqrt((1.0 + 0.05 * getPrimSkillLevel(PrimarySkill::KNOWLEDGE)) * (1.0 + 0.05 * getPrimSkillLevel(PrimarySkill::SPELL_POWER)));
+	const auto & primarySkills = getPrimarySkills();
+	return getFightingStrengthImpl(primarySkills) * getMagicStrengthImpl(primarySkills);
 }
 
-double CGHeroInstance::getHeroStrength() const
+uint64_t CGHeroInstance::getValueForDiplomacy() const
 {
-	return sqrt(pow(getFightingStrength(), 2.0) * pow(getMagicStrength(), 2.0));
+	// H3 formula for hero strength when considering diplomacy skill
+	uint64_t armyStrength = getArmyStrength();
+	double heroStrength = sqrt(
+		(1.0 + 0.05 * getPrimSkillLevel(PrimarySkill::ATTACK)) *
+		(1.0 + 0.05 * getPrimSkillLevel(PrimarySkill::DEFENSE))
+		);
+
+	return heroStrength * armyStrength;
 }
 
-double CGHeroInstance::getHeroStrengthForCampaign() const
+uint32_t CGHeroInstance::getValueForCampaign() const
 {
-	return sqrt(pow(getFightingStrength(), 2.0) * pow(getMagicStrengthForCampaign(), 2.0));
+	/// Determined by testing H3: hero is preferred for transfer in campaigns if total sum of his primary skills and his secondary skill levels is greatest
+	uint32_t score = 0;
+	score += getPrimSkillLevel(PrimarySkill::ATTACK);
+	score += getPrimSkillLevel(PrimarySkill::DEFENSE);
+	score += getPrimSkillLevel(PrimarySkill::SPELL_POWER);
+	score += getPrimSkillLevel(PrimarySkill::DEFENSE);
+
+	for (const auto& secondary : secSkills)
+		score += secondary.second;
+
+	return score;
 }
 
 ui64 CGHeroInstance::getTotalStrength() const
@@ -1653,11 +1698,11 @@ void CGHeroInstance::serializeCommonOptions(JsonSerializeFormat & handler)
 		{
 			auto primarySkills = handler.enterStruct("primarySkills");
 
-			for(auto i = PrimarySkill::BEGIN; i < PrimarySkill::END; ++i)
+			for(auto skill : PrimarySkill::ALL_SKILLS())
 			{
-				int value = valOfBonuses(Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(i)).And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL)));
+				int value = valOfBonuses(Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(skill)).And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL)));
 
-				handler.serializeInt(NPrimarySkill::names[i.getNum()], value, 0);
+				handler.serializeInt(NPrimarySkill::names[skill.getNum()], value, 0);
 			}
 		}
 	}

lib/mapObjects/CGHeroInstance.h

@@ -62,6 +62,9 @@ private:
 	mutable int lowestCreatureSpeed;
 	ui32 movement; //remaining movement points
 
+	double getFightingStrengthImpl(const std::array<int, 4> & primarySkills) const;
+	double getMagicStrengthImpl(const std::array<int, 4> & primarySkills) const;
+
 public:
 
 	//////////////////////////////////////////////////////////////////////////
@@ -201,6 +204,7 @@ public:
 	std::vector<SecondarySkill> getLevelUpProposedSecondarySkills(vstd::RNG & rand) const;
 
 	ui8 getSecSkillLevel(const SecondarySkill & skill) const; //0 - no skill
+	std::array<int, 4> getPrimarySkills() const;
 
 	/// Returns true if hero has free secondary skill slot.
 	bool canLearnSkill() const;
@@ -225,9 +229,11 @@ public:
 
 	double getFightingStrength() const; // takes attack / defense skill into account
 	double getMagicStrength() const; // takes knowledge / spell power skill but also current mana, whether the hero owns a spell-book and whether that books contains anything into account
-	double getMagicStrengthForCampaign() const; // takes knowledge / spell power skill into account
 	double getHeroStrength() const; // includes fighting and magic strength
-	double getHeroStrengthForCampaign() const; // includes fighting and the for-campaign-version of magic strength
+
+	uint32_t getValueForCampaign() const;
+	uint64_t getValueForDiplomacy() const;
+	
 	ui64 getTotalStrength() const; // includes fighting strength and army strength
 	TExpType calculateXp(TExpType exp) const; //apply learning skill
 	int getBasePrimarySkillValue(PrimarySkill which) const; //the value of a base-skill without items or temporary bonuses