Merge pull request #5110 from IvanSavenko/ai_optimize

[1.6.1] AI optimization

AI/Nullkiller/AIGateway.cpp

@@ -97,6 +97,8 @@ void AIGateway::heroMoved(const TryMoveHero & details, bool verbose)
 	if(!hero)
 		validateObject(details.id); //enemy hero may have left visible area
 
+	nullkiller->invalidatePathfinderData();
+
 	const int3 from = hero ? hero->convertToVisitablePos(details.start) : (details.start - int3(0,1,0));
 	const int3 to   = hero ? hero->convertToVisitablePos(details.end)   : (details.end   - int3(0,1,0));
 
@@ -358,6 +360,7 @@ void AIGateway::newObject(const CGObjectInstance * obj)
 {
 	LOG_TRACE(logAi);
 	NET_EVENT_HANDLER;
+	nullkiller->invalidatePathfinderData();
 	if(obj->isVisitable())
 		addVisitableObj(obj);
 }
@@ -582,6 +585,7 @@ void AIGateway::yourTurn(QueryID queryID)
 {
 	LOG_TRACE_PARAMS(logAi, "queryID '%i'", queryID);
 	NET_EVENT_HANDLER;
+	nullkiller->invalidatePathfinderData();
 	status.addQuery(queryID, "YourTurn");
 	requestActionASAP([=](){ answerQuery(queryID, 0); });
 	status.startedTurn();

AI/Nullkiller/Analyzers/BuildAnalyzer.cpp

@@ -239,8 +239,7 @@ BuildingInfo BuildAnalyzer::getBuildingOrPrerequisite(
 
 	auto info = BuildingInfo(buildPtr, creature, baseCreatureID, town, ai);
 
-	logAi->trace("checking %s", info.name);
+	//logAi->trace("checking %s buildInfo %s", info.name, info.toString());
-	logAi->trace("buildInfo %s", info.toString());
 
 	int highestFort = 0;
 	for (auto twn : ai->cb->getTownsInfo())
@@ -258,7 +257,7 @@ BuildingInfo BuildAnalyzer::getBuildingOrPrerequisite(
 		}
 		else if(canBuild == EBuildingState::NO_RESOURCES)
 		{
-			logAi->trace("cant build. Not enough resources. Need %s", info.buildCost.toString());
+			//logAi->trace("cant build. Not enough resources. Need %s", info.buildCost.toString());
 			info.notEnoughRes = true;
 		}
 		else if(canBuild == EBuildingState::PREREQUIRES)

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 specialSecondarySkillBonuses = hero->getBonuses(heroSpecial.And(secondarySkillBonus), "HeroManager::evaluateSpeciality");
-	auto secondarySkillBonuses = hero->getBonuses(Selector::sourceTypeSel(BonusSource::SECONDARY_SKILL));
+	auto secondarySkillBonuses = hero->getBonusesFrom(BonusSource::SECONDARY_SKILL);
 	float specialityScore = 0.0f;
 
 	for(auto bonus : *secondarySkillBonuses)

AI/Nullkiller/Engine/Nullkiller.cpp

@@ -37,6 +37,7 @@ Nullkiller::Nullkiller()
 	: activeHero(nullptr)
 	, scanDepth(ScanDepth::MAIN_FULL)
 	, useHeroChain(true)
+	, pathfinderInvalidated(false)
 	, memory(std::make_unique<AIMemory>())
 {
 
@@ -239,6 +240,11 @@ void Nullkiller::resetAiState()
 	}
 }
 
+void Nullkiller::invalidatePathfinderData()
+{
+	pathfinderInvalidated = true;
+}
+
 void Nullkiller::updateAiState(int pass, bool fast)
 {
 	boost::this_thread::interruption_point();
@@ -253,7 +259,10 @@ void Nullkiller::updateAiState(int pass, bool fast)
 	decomposer->reset();
 	buildAnalyzer->update();
 
-	if(!fast)
+	if (!pathfinderInvalidated)
+		logAi->trace("Skipping paths regeneration - up to date");
+
+	if(!fast && pathfinderInvalidated)
 	{
 		memory->removeInvisibleObjects(cb.get());
 
@@ -304,11 +313,13 @@ void Nullkiller::updateAiState(int pass, bool fast)
 		boost::this_thread::interruption_point();
 
 		objectClusterizer->clusterize();
+
+		pathfinderInvalidated = false;
 	}
 
 	armyManager->update();
 
-	logAi->debug("AI state updated in %ld", timeElapsed(start));
+	logAi->debug("AI state updated in %ld ms", timeElapsed(start));
 }
 
 bool Nullkiller::isHeroLocked(const CGHeroInstance * hero) const
@@ -379,7 +390,7 @@ void Nullkiller::makeTurn()
 
 		Goals::TTask bestTask = taskptr(Goals::Invalid());
 
-		while(true)
+		for(int j = 1; j <= settings->getMaxPriorityPass() && cb->getPlayerStatus(playerID) == EPlayerStatus::INGAME; j++)
 		{
 			bestTasks.clear();
 

AI/Nullkiller/Engine/Nullkiller.h

@@ -78,6 +78,7 @@ private:
 	AIGateway * gateway;
 	bool openMap;
 	bool useObjectGraph;
+	bool pathfinderInvalidated;
 
 public:
 	static std::unique_ptr<ObjectGraph> baseGraph;
@@ -121,6 +122,7 @@ public:
 	bool isOpenMap() const { return openMap; }
 	bool isObjectGraphAllowed() const { return useObjectGraph; }
 	bool handleTrading();
+	void invalidatePathfinderData();
 
 private:
 	void resetAiState();

AI/Nullkiller/Engine/Settings.cpp

@@ -32,7 +32,8 @@ namespace NKAI
 		retreatThresholdRelative(0.3),
 		retreatThresholdAbsolute(10000),
 		safeAttackRatio(1.1),
-		maxpass(10),
+		maxPass(10),
+		maxPriorityPass(10),
 		pathfinderBucketsCount(1),
 		pathfinderBucketSize(32),
 		allowObjectGraph(true),
@@ -48,7 +49,8 @@ namespace NKAI
 		maxRoamingHeroes = node["maxRoamingHeroes"].Integer();
 		mainHeroTurnDistanceLimit = node["mainHeroTurnDistanceLimit"].Integer();
 		scoutHeroTurnDistanceLimit = node["scoutHeroTurnDistanceLimit"].Integer();
-		maxpass = node["maxpass"].Integer();
+		maxPass = node["maxPass"].Integer();
+		maxPriorityPass = node["maxPriorityPass"].Integer();
 		pathfinderBucketsCount = node["pathfinderBucketsCount"].Integer();
 		pathfinderBucketSize = node["pathfinderBucketSize"].Integer();
 		maxGoldPressure = node["maxGoldPressure"].Float();

AI/Nullkiller/Engine/Settings.h

@@ -24,7 +24,8 @@ namespace NKAI
 		int maxRoamingHeroes;
 		int mainHeroTurnDistanceLimit;
 		int scoutHeroTurnDistanceLimit;
-		int maxpass;
+		int maxPass;
+		int maxPriorityPass;
 		int pathfinderBucketsCount;
 		int pathfinderBucketSize;
 		float maxGoldPressure;
@@ -41,7 +42,8 @@ namespace NKAI
 	public:
 		explicit Settings(int difficultyLevel);
 
-		int getMaxPass() const { return maxpass; }
+		int getMaxPass() const { return maxPass; }
+		int getMaxPriorityPass() const { return maxPriorityPass; }
 		float getMaxGoldPressure() const { return maxGoldPressure; }
 		float getRetreatThresholdRelative() const { return retreatThresholdRelative; }
 		float getRetreatThresholdAbsolute() const { return retreatThresholdAbsolute; }

AI/Nullkiller/Pathfinding/AINodeStorage.h

@@ -23,8 +23,6 @@ constexpr int NKAI_GRAPH_TRACE_LEVEL = 0;
 #include "Actions/SpecialAction.h"
 #include "Actors.h"
 
-#include <boost/container/small_vector.hpp>
-
 namespace NKAI
 {
 namespace AIPathfinding

AI/Nullkiller/Pathfinding/AIPathfinder.cpp

@@ -106,7 +106,7 @@ void AIPathfinder::updatePaths(const std::map<const CGHeroInstance *, HeroRole>
 
 	if(!pathfinderSettings.useHeroChain)
 	{
-		logAi->trace("Recalculated paths in %ld", timeElapsed(start));
+		logAi->trace("Recalculated paths in %ld ms", timeElapsed(start));
 
 		return;
 	}
@@ -141,7 +141,7 @@ void AIPathfinder::updatePaths(const std::map<const CGHeroInstance *, HeroRole>
 		}
 	} while(storage->increaseHeroChainTurnLimit());
 
-	logAi->trace("Recalculated paths in %ld", timeElapsed(start));
+	logAi->trace("Recalculated paths in %ld ms", timeElapsed(start));
 }
 
 void AIPathfinder::updateGraphs(

Global.h

@@ -134,6 +134,7 @@ static_assert(sizeof(bool) == 1, "Bool needs to be 1 byte in size.");
 #include <random>
 #include <regex>
 #include <set>
+#include <shared_mutex>
 #include <sstream>
 #include <string>
 #include <unordered_map>
@@ -168,6 +169,8 @@ static_assert(sizeof(bool) == 1, "Bool needs to be 1 byte in size.");
 #include <boost/algorithm/string.hpp>
 #include <boost/crc.hpp>
 #include <boost/current_function.hpp>
+#include <boost/container/small_vector.hpp>
+#include <boost/container/static_vector.hpp>
 #include <boost/date_time/posix_time/posix_time_types.hpp>
 #include <boost/date_time/posix_time/time_formatters.hpp>
 #include <boost/filesystem.hpp>

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/mapView/MapRenderer.cpp

@@ -135,7 +135,7 @@ int MapTileStorage::groupCount(size_t fileIndex, size_t rotationIndex, size_t im
 	const auto & animation = animations[fileIndex][rotationIndex];
 	if (animation)
 		for(int i = 0;; i++)
-			if(!animation->getImage(imageIndex, i, false))
+			if(animation->size(i) <= imageIndex)
 				return i;
 	return 1;
 }

client/renderSDL/SDL_Extensions.cpp

@@ -671,10 +671,6 @@ SDL_Surface * CSDL_Ext::scaleSurfaceIntegerFactor(SDL_Surface * surf, int factor
 	const uint32_t * srcPixels = static_cast<const uint32_t*>(intermediate->pixels);
 	uint32_t * dstPixels = static_cast<uint32_t*>(ret->pixels);
 
-	// avoid excessive granulation - xBRZ prefers at least 8-16 lines per task
-	// TODO: compare performance and size of images, recheck values for potentially better parameters
-	const int granulation = std::clamp(surf->h / 64 * 8, 8, 64);
-
 	switch (algorithm)
 	{
 		case EScalingAlgorithm::NEAREST:
@@ -687,11 +683,22 @@ SDL_Surface * CSDL_Ext::scaleSurfaceIntegerFactor(SDL_Surface * surf, int factor
 		case EScalingAlgorithm::XBRZ_OPAQUE:
 		{
 			auto format = algorithm == EScalingAlgorithm::XBRZ_OPAQUE ? xbrz::ColorFormat::ARGB_CLAMPED : xbrz::ColorFormat::ARGB;
+
+			if(intermediate->h < 32)
+			{
+				// for tiny images tbb incurs too high overhead
+				xbrz::scale(factor, srcPixels, dstPixels, intermediate->w, intermediate->h, format, {});
+			}
+			else
+			{
+				// xbrz recommends granulation of 16, but according to tests, for smaller images granulation of 4 is actually the best option
+				const int granulation = intermediate->h > 400 ? 16 : 4;
 				tbb::parallel_for(tbb::blocked_range<size_t>(0, intermediate->h, granulation), [factor, srcPixels, dstPixels, intermediate, format](const tbb::blocked_range<size_t> & r)
 				{
-
 					xbrz::scale(factor, srcPixels, dstPixels, intermediate->w, intermediate->h, format, {}, r.begin(), r.end());
 				});
+			}
+
 			break;
 		}
 		default:

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);
 	}

config/ai/nkai/nkai-settings.json

@@ -33,17 +33,18 @@
 	
 	
 	"pawn" : {
-		"maxRoamingHeroes" : 4, //H3 value: 3,
+		"maxRoamingHeroes" : 3, //H3 value: 3,
-		"maxpass" : 30,
+		"maxPass" : 30,
+		"maxPriorityPass" : 10,
 		"mainHeroTurnDistanceLimit" : 10,
 		"scoutHeroTurnDistanceLimit" : 5,
 		"maxGoldPressure" : 0.3,
 		"updateHitmapOnTileReveal" : false,
 		"useTroopsFromGarrisons" : true,
 		"openMap": true,
-		"allowObjectGraph": false,
+		"allowObjectGraph": true,
-		"pathfinderBucketsCount" : 1, // old value: 3,
+		"pathfinderBucketsCount" : 4, // old value: 3,
-		"pathfinderBucketSize" : 32, // old value: 7,
+		"pathfinderBucketSize" : 8, // old value: 7,
 		"retreatThresholdRelative" : 0,
 		"retreatThresholdAbsolute" : 0,
 		"safeAttackRatio" : 1.1,
@@ -52,17 +53,18 @@
 	},
 	
 	"knight" : {
-		"maxRoamingHeroes" : 6, //H3 value: 3,
+		"maxRoamingHeroes" : 3, //H3 value: 3,
-		"maxpass" : 30,
+		"maxPass" : 30,
+		"maxPriorityPass" : 10,
 		"mainHeroTurnDistanceLimit" : 10,
 		"scoutHeroTurnDistanceLimit" : 5,
 		"maxGoldPressure" : 0.3,
 		"updateHitmapOnTileReveal" : false,
 		"useTroopsFromGarrisons" : true,
 		"openMap": true,
-		"allowObjectGraph": false,
+		"allowObjectGraph": true,
-		"pathfinderBucketsCount" : 1, // old value: 3,
+		"pathfinderBucketsCount" : 4, // old value: 3,
-		"pathfinderBucketSize" : 32, // old value: 7,
+		"pathfinderBucketSize" : 8, // old value: 7,
 		"retreatThresholdRelative" : 0.1,
 		"retreatThresholdAbsolute" : 5000,
 		"safeAttackRatio" : 1.1,
@@ -71,17 +73,18 @@
 	},
 	
 	"rook" : {
-		"maxRoamingHeroes" : 8, //H3 value: 4
+		"maxRoamingHeroes" : 4, //H3 value: 4
-		"maxpass" : 30,
+		"maxPass" : 30,
+		"maxPriorityPass" : 10,
 		"mainHeroTurnDistanceLimit" : 10,
 		"scoutHeroTurnDistanceLimit" : 5,
 		"maxGoldPressure" : 0.3,
 		"updateHitmapOnTileReveal" : false,
 		"useTroopsFromGarrisons" : true,
 		"openMap": true,
-		"allowObjectGraph": false,
+		"allowObjectGraph": true,
-		"pathfinderBucketsCount" : 1, // old value: 3,
+		"pathfinderBucketsCount" : 4, // old value: 3,
-		"pathfinderBucketSize" : 32, // old value: 7,
+		"pathfinderBucketSize" : 8, // old value: 7,
 		"retreatThresholdRelative" : 0.3,
 		"retreatThresholdAbsolute" : 10000,
 		"safeAttackRatio" : 1.1,
@@ -90,17 +93,18 @@
 	},
 	
 	"queen" : {
-		"maxRoamingHeroes" : 8, //H3 value: 5
+		"maxRoamingHeroes" : 6, //H3 value: 5
-		"maxpass" : 30,
+		"maxPass" : 30,
+		"maxPriorityPass" : 10,
 		"mainHeroTurnDistanceLimit" : 10,
 		"scoutHeroTurnDistanceLimit" : 5,
 		"maxGoldPressure" : 0.3,
 		"updateHitmapOnTileReveal" : false,
 		"useTroopsFromGarrisons" : true,
 		"openMap": true,
-		"allowObjectGraph": false,
+		"allowObjectGraph": true,
-		"pathfinderBucketsCount" : 1, // old value: 3,
+		"pathfinderBucketsCount" : 4, // old value: 3,
-		"pathfinderBucketSize" : 32, // old value: 7,
+		"pathfinderBucketSize" : 8, // old value: 7,
 		"retreatThresholdRelative" : 0.3,
 		"retreatThresholdAbsolute" : 10000,
 		"safeAttackRatio" : 1.1,
@@ -110,16 +114,17 @@
 	
 	"king" : {
 		"maxRoamingHeroes" : 8, //H3 value: 6
-		"maxpass" : 30,
+		"maxPass" : 30,
+		"maxPriorityPass" : 10,
 		"mainHeroTurnDistanceLimit" : 10,
 		"scoutHeroTurnDistanceLimit" : 5,
 		"maxGoldPressure" : 0.3,
 		"updateHitmapOnTileReveal" : false,
 		"useTroopsFromGarrisons" : true,
 		"openMap": true,
-		"allowObjectGraph": false,
+		"allowObjectGraph": true,
-		"pathfinderBucketsCount" : 1, // old value: 3,
+		"pathfinderBucketsCount" : 4, // old value: 3,
-		"pathfinderBucketSize" : 32, // old value: 7,
+		"pathfinderBucketSize" : 8, // old value: 7,
 		"retreatThresholdRelative" : 0.3,
 		"retreatThresholdAbsolute" : 10000,
 		"safeAttackRatio" : 1.1,

include/vcmi/Creature.h

@@ -23,7 +23,8 @@ class DLL_LINKAGE ACreature: public AFactionMember
 {
 public:
 	bool isLiving() const; //non-undead, non-non living or alive
-	ui32 getMovementRange(int turn = 0) const; //get speed (in moving tiles) of creature with all modificators
+	ui32 getMovementRange(int turn) const; //get speed (in moving tiles) of creature with all modificators
+	ui32 getMovementRange() const; //get speed (in moving tiles) of creature with all modificators
 	virtual ui32 getMaxHealth() const; //get max HP of stack with all modifiers
 };
 

launcher/modManager/cdownloadmanager_moc.cpp

@@ -122,7 +122,7 @@ void CDownloadManager::downloadFinished(QNetworkReply * reply)
 	}
 
 	if(downloadComplete)
-		emit finished(successful, failed, encounteredErrors);
+		finished(successful, failed, encounteredErrors);
 
 	file.reply->deleteLater();
 	file.reply = nullptr;
@@ -149,7 +149,7 @@ void CDownloadManager::downloadProgressChanged(qint64 bytesReceived, qint64 byte
 	if(received > total)
 		total = received;
 
-	emit downloadProgress(received, total);
+	downloadProgress(received, total);
 }
 
 bool CDownloadManager::downloadInProgress(const QUrl & url) const

launcher/modManager/cmodlistview_moc.cpp

@@ -825,7 +825,7 @@ void CModListView::installFiles(QStringList files)
 		
 		while(futureExtract.wait_for(std::chrono::milliseconds(10)) != std::future_status::ready)
 		{
-			emit extractionProgress(static_cast<int>(prog * 1000.f), 1000);
+			extractionProgress(static_cast<int>(prog * 1000.f), 1000);
 			qApp->processEvents();
 		}
 		

launcher/modManager/modstatecontroller.cpp

@@ -214,7 +214,7 @@ bool ModStateController::doInstallMod(QString modname, QString archivePath)
 	
 	while(futureExtract.wait_for(std::chrono::milliseconds(10)) != std::future_status::ready)
 	{
-		emit extractionProgress(filesCounter, filesToExtract.size());
+		extractionProgress(filesCounter, filesToExtract.size());
 		qApp->processEvents();
 	}
 	

launcher/modManager/modstateitemmodel_moc.cpp

@@ -206,7 +206,7 @@ void ModStateItemModel::modChanged(QString modID)
 	int index = modNameToID.indexOf(modID);
 	QModelIndex parent = this->parent(createIndex(0, 0, index));
 	int row = modIndex[modIndexToName(parent)].indexOf(modID);
-	emit dataChanged(createIndex(row, 0, index), createIndex(row, 4, index));
+	dataChanged(createIndex(row, 0, index), createIndex(row, 4, index));
 }
 
 void ModStateItemModel::endResetModel()

lib/BasicTypes.cpp

@@ -32,38 +32,27 @@ bool INativeTerrainProvider::isNativeTerrain(TerrainId terrain) const
 
 TerrainId AFactionMember::getNativeTerrain() const
 {
-	const std::string cachingStringNoTerrainPenalty = "type_TERRAIN_NATIVE_NONE";
-	static const auto selectorNoTerrainPenalty = Selector::typeSubtype(BonusType::TERRAIN_NATIVE, BonusSubtypeID());
-
 	//this code is used in the CreatureTerrainLimiter::limit to setup battle bonuses
 	//and in the CGHeroInstance::getNativeTerrain() to setup movement bonuses or/and penalties.
-	return getBonusBearer()->hasBonus(selectorNoTerrainPenalty, cachingStringNoTerrainPenalty)
+	return getBonusBearer()->hasBonusOfType(BonusType::TERRAIN_NATIVE)
 			 ? TerrainId::ANY_TERRAIN : getFactionID().toEntity(VLC)->getNativeTerrain();
 }
 
 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;
 }
 
 int AFactionMember::getAttack(bool ranged) const
 {
-	const std::string cachingStr = "type_PRIMARY_SKILLs_ATTACK";
+	return getBonusBearer()->valOfBonuses(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::ATTACK));
-
-	static const auto selector = Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::ATTACK));
-
-	return getBonusBearer()->valOfBonuses(selector, cachingStr);
 }
 
 int AFactionMember::getDefense(bool ranged) const
 {
-	const std::string cachingStr = "type_PRIMARY_SKILLs_DEFENSE";
+	return getBonusBearer()->valOfBonuses(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::DEFENSE));
-
-	static const auto selector = Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::DEFENSE));
-
-	return getBonusBearer()->valOfBonuses(selector, cachingStr);
 }
 
 int AFactionMember::getMinDamage(bool ranged) const
@@ -82,11 +71,9 @@ int AFactionMember::getMaxDamage(bool ranged) const
 
 int AFactionMember::getPrimSkillLevel(PrimarySkill id) const
 {
-	static const CSelector selectorAllSkills = Selector::type()(BonusType::PRIMARY_SKILL);
+	auto allSkills = getBonusBearer()->getBonusesOfType(BonusType::PRIMARY_SKILL);
-	static const std::string keyAllSkills = "type_PRIMARY_SKILL";
+	int ret = allSkills->valOfBonuses(Selector::subtype()(BonusSubtypeID(id)));
-	auto allSkills = getBonusBearer()->getBonuses(selectorAllSkills, keyAllSkills);
+	int minSkillValue = VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, id.getNum());
-	auto ret = allSkills->valOfBonuses(Selector::subtype()(BonusSubtypeID(id)));
-	auto minSkillValue = VLC->engineSettings()->getVector(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS)[id.getNum()];
 	return std::max(ret, minSkillValue); //otherwise, some artifacts may cause negative skill value effect, sp=0 works in old saves
 }
 
@@ -114,9 +101,7 @@ int AFactionMember::moraleValAndBonusList(TConstBonusListPtr & bonusList) const
 		return 0;
 	}
 
-	static const auto moraleSelector = Selector::type()(BonusType::MORALE);
+	bonusList = getBonusBearer()->getBonusesOfType(BonusType::MORALE);
-	static const std::string cachingStrMor = "type_MORALE";
-	bonusList = getBonusBearer()->getBonuses(moraleSelector, cachingStrMor);
 
 	return std::clamp(bonusList->totalValue(), maxBadMorale, maxGoodMorale);
 }
@@ -140,9 +125,7 @@ int AFactionMember::luckValAndBonusList(TConstBonusListPtr & bonusList) const
 		return 0;
 	}
 
-	static const auto luckSelector = Selector::type()(BonusType::LUCK);
+	bonusList = getBonusBearer()->getBonusesOfType(BonusType::LUCK);
-	static const std::string cachingStrLuck = "type_LUCK";
-	bonusList = getBonusBearer()->getBonuses(luckSelector, cachingStrLuck);
 
 	return std::clamp(bonusList->totalValue(), maxBadLuck, maxGoodLuck);
 }
@@ -161,25 +144,39 @@ int AFactionMember::luckVal() const
 
 ui32 ACreature::getMaxHealth() const
 {
-	const std::string cachingStr = "type_STACK_HEALTH";
+	auto value = getBonusBearer()->valOfBonuses(BonusType::STACK_HEALTH);
-	static const auto selector = Selector::type()(BonusType::STACK_HEALTH);
-	auto value = getBonusBearer()->valOfBonuses(selector, cachingStr);
 	return std::max(1, value); //never 0
 }
 
+ui32 ACreature::getMovementRange() const
+{
+	//war machines cannot move
+	if (getBonusBearer()->hasBonusOfType(BonusType::SIEGE_WEAPON))
+		return 0;
+
+	if (getBonusBearer()->hasBonusOfType(BonusType::BIND_EFFECT))
+		return 0;
+
+	return getBonusBearer()->valOfBonuses(BonusType::STACKS_SPEED);
+}
+
 ui32 ACreature::getMovementRange(int turn) const
 {
-	//war machines cannot move
+	if (turn == 0)
-	if(getBonusBearer()->hasBonus(Selector::type()(BonusType::SIEGE_WEAPON).And(Selector::turns(turn))))
+		return getMovementRange();
-	{
-		return 0;
-	}
-	if(getBonusBearer()->hasBonus(Selector::type()(BonusType::BIND_EFFECT).And(Selector::turns(turn))))
-	{
-		return 0;
-	}
 
-	return getBonusBearer()->valOfBonuses(Selector::type()(BonusType::STACKS_SPEED).And(Selector::turns(turn)));
+	const std::string cachingStrSW = "type_SIEGE_WEAPON_turns_" + std::to_string(turn);
+	const std::string cachingStrBE = "type_BIND_EFFECT_turns_" + std::to_string(turn);
+	const std::string cachingStrSS = "type_STACKS_SPEED_turns_" + std::to_string(turn);
+
+	//war machines cannot move
+	if(getBonusBearer()->hasBonus(Selector::type()(BonusType::SIEGE_WEAPON).And(Selector::turns(turn)), cachingStrSW))
+		return 0;
+
+	if(getBonusBearer()->hasBonus(Selector::type()(BonusType::BIND_EFFECT).And(Selector::turns(turn)), cachingStrBE))
+		return 0;
+
+	return getBonusBearer()->valOfBonuses(Selector::type()(BonusType::STACKS_SPEED).And(Selector::turns(turn)), cachingStrSS);
 }
 
 bool ACreature::isLiving() const //TODO: theoreticaly there exists "LIVING" bonus in stack experience documentation

lib/CBonusTypeHandler.cpp

@@ -93,7 +93,7 @@ std::string CBonusTypeHandler::bonusToString(const std::shared_ptr<Bonus> & bonu
 	}
 
 	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/CPlayerState.cpp

@@ -112,24 +112,24 @@ std::vector<T> PlayerState::getObjectsOfType() const
 	return result;
 }
 
-std::vector<const CGHeroInstance *> PlayerState::getHeroes() const
+const std::vector<const CGHeroInstance *> & PlayerState::getHeroes() const
 {
-	return getObjectsOfType<const CGHeroInstance *>();
+	return constOwnedHeroes;
 }
 
-std::vector<const CGTownInstance *> PlayerState::getTowns() const
+const std::vector<const CGTownInstance *> & PlayerState::getTowns() const
 {
-	return getObjectsOfType<const CGTownInstance *>();
+	return constOwnedTowns;
 }
 
-std::vector<CGHeroInstance *> PlayerState::getHeroes()
+const std::vector<CGHeroInstance *> & PlayerState::getHeroes()
 {
-	return getObjectsOfType<CGHeroInstance *>();
+	return ownedHeroes;
 }
 
-std::vector<CGTownInstance *> PlayerState::getTowns()
+const std::vector<CGTownInstance *> & PlayerState::getTowns()
 {
-	return getObjectsOfType<CGTownInstance *>();
+	return ownedTowns;
 }
 
 std::vector<const CGObjectInstance *> PlayerState::getOwnedObjects() const
@@ -141,11 +141,51 @@ void PlayerState::addOwnedObject(CGObjectInstance * object)
 {
 	assert(object->asOwnable() != nullptr);
 	ownedObjects.push_back(object);
+
+	auto * town = dynamic_cast<CGTownInstance*>(object);
+	auto * hero = dynamic_cast<CGHeroInstance*>(object);
+
+	if (town)
+	{
+		ownedTowns.push_back(town);
+		constOwnedTowns.push_back(town);
+	}
+
+	if (hero)
+	{
+		ownedHeroes.push_back(hero);
+		constOwnedHeroes.push_back(hero);
+	}
+}
+
+void PlayerState::postDeserialize()
+{
+	for (const auto& object : ownedObjects)
+	{
+		auto* town = dynamic_cast<CGTownInstance*>(object);
+		auto* hero = dynamic_cast<CGHeroInstance*>(object);
+
+		if (town)
+		{
+			ownedTowns.push_back(town);
+			constOwnedTowns.push_back(town);
+		}
+
+		if (hero)
+		{
+			ownedHeroes.push_back(hero);
+			constOwnedHeroes.push_back(hero);
+		}
+	}
 }
 
 void PlayerState::removeOwnedObject(CGObjectInstance * object)
 {
 	vstd::erase(ownedObjects, object);
+	vstd::erase(ownedTowns, object);
+	vstd::erase(constOwnedTowns, object);
+	vstd::erase(ownedHeroes, object);
+	vstd::erase(constOwnedHeroes, object);
 }
 
 

lib/CPlayerState.h

@@ -49,6 +49,11 @@ class DLL_LINKAGE PlayerState : public CBonusSystemNode, public Player
 
 	std::vector<CGObjectInstance*> ownedObjects;
 
+	std::vector<const CGTownInstance*> constOwnedTowns; //not serialized
+	std::vector<const CGHeroInstance*> constOwnedHeroes; //not serialized
+	std::vector<CGTownInstance*> ownedTowns; //not serialized
+	std::vector<CGHeroInstance*> ownedHeroes; //not serialized
+
 	template<typename T>
 	std::vector<T> getObjectsOfType() const;
 
@@ -92,15 +97,16 @@ public:
 	std::string getNameTextID() const override;
 	void registerIcons(const IconRegistar & cb) const override;
 
-	std::vector<const CGHeroInstance* > getHeroes() const;
+	const std::vector<const CGHeroInstance* > & getHeroes() const;
-	std::vector<const CGTownInstance* > getTowns() const;
+	const std::vector<const CGTownInstance* > & getTowns() const;
-	std::vector<CGHeroInstance* > getHeroes();
+	const std::vector<CGHeroInstance* > & getHeroes();
-	std::vector<CGTownInstance* > getTowns();
+	const std::vector<CGTownInstance* > & getTowns();
 
 	std::vector<const CGObjectInstance* > getOwnedObjects() const;
 
 	void addOwnedObject(CGObjectInstance * object);
 	void removeOwnedObject(CGObjectInstance * object);
+	void postDeserialize();
 
 	bool checkVanquished() const
 	{
@@ -145,6 +151,9 @@ public:
 		h & enteredWinningCheatCode;
 		h & static_cast<CBonusSystemNode&>(*this);
 		h & destroyedObjects;
+
+		if (!h.saving)
+			postDeserialize();
 	}
 };
 

lib/GameSettings.cpp

@@ -33,6 +33,11 @@ std::vector<int> IGameSettings::getVector(EGameSettings option) const
 	return getValue(option).convertTo<std::vector<int>>();
 }
 
+int IGameSettings::getVectorValue(EGameSettings option, size_t index) const
+{
+	return getValue(option)[index].Integer();
+}
+
 GameSettings::GameSettings() = default;
 GameSettings::~GameSettings() = default;
 

lib/IGameSettings.h

@@ -101,6 +101,7 @@ public:
 	int64_t getInteger(EGameSettings option) const;
 	double getDouble(EGameSettings option) const;
 	std::vector<int> getVector(EGameSettings option) const;
+	int getVectorValue(EGameSettings option, size_t index) const;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/IHandlerBase.h

@@ -48,12 +48,15 @@ template <class _ObjectID, class _ObjectBase, class _Object, class _ServiceBase>
 {
 	const _Object * getObjectImpl(const int32_t index) const
 	{
-		if(index < 0 || index >= objects.size())
+		try
+		{
+			return objects.at(index).get();
+		}
+		catch (const std::out_of_range&)
 		{
 			logMod->error("%s id %d is invalid", getTypeNames()[0], index);
 			throw std::runtime_error("Attempt to access invalid index " + std::to_string(index) + " of type " + getTypeNames().front());
 		}
-		return objects[index].get();
 	}
 
 public:

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));
+			battleRepositionHex[i] += heroes[i]->valOfBonuses(BonusType::BEFORE_BATTLE_REPOSITION);
-			battleRepositionHexBlock[i] += heroes[i]->valOfBonuses(Selector::type()(BonusType::BEFORE_BATTLE_REPOSITION_BLOCK));
+			battleRepositionHexBlock[i] += heroes[i]->valOfBonuses(BonusType::BEFORE_BATTLE_REPOSITION_BLOCK);
 		}
 	}
 	int tacticsSkillDiffAttacker = battleRepositionHex[BattleSide::ATTACKER] - battleRepositionHexBlock[BattleSide::DEFENDER];

lib/battle/CBattleInfoCallback.cpp

@@ -711,19 +711,21 @@ bool CBattleInfoCallback::battleCanShoot(const battle::Unit * attacker) const
 	if (attacker->creatureIndex() == CreatureID::CATAPULT) //catapult cannot attack creatures
 		return false;
 
+	if (!attacker->canShoot())
+		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)
 			return false;
 	}
 
-	return attacker->canShoot()	&& (!battleIsUnitBlocked(attacker)
+	return !battleIsUnitBlocked(attacker) || attacker->hasBonusOfType(BonusType::FREE_SHOOTING);
-			|| attacker->hasBonusOfType(BonusType::FREE_SHOOTING));
 }
 
 bool CBattleInfoCallback::battleCanTargetEmptyHex(const battle::Unit * attacker) const
@@ -1878,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->hasBonusOfType(BonusType::KING);
-
-				return stack->hasBonus(isKing);
 			});
 
 			if (!kingMonster)
@@ -1905,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;
 
@@ -1969,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();
 
@@ -1988,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->hasBonusOfType(BonusType::HYPNOTIZED))
-
-	if(unit->hasBonus(selector))
 		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))),
+	noRetaliation(Owner, Selector::type()(BonusType::SIEGE_WEAPON).Or(Selector::type()(BonusType::HYPNOTIZED)).Or(Selector::type()(BonusType::NO_RETALIATION)), "CRetaliations::noRetaliation"),
-	unlimited(Owner, Selector::type()(BonusType::UNLIMITED_RETALIATIONS))
+	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 !hasBonusOfType(BonusType::NOT_ACTIVE);
+
+	std::string cachingStr = "type_NOT_ACTIVE_turns_" + std::to_string(turn);
+	return !hasBonus(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))
+	if(info.chargeDistance > 0 && info.attacker->hasBonusOfType(BonusType::JOUSTING) && !info.defender->hasBonusOfType(BonusType::CHARGE_IMMUNITY))
-		return info.chargeDistance * (info.attacker->valOfBonuses(selectorJousting))/100.0;
+		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";
+	auto allHateEffects = info.attacker->getBonusesOfType(BonusType::HATE);
-	static const auto selectorHate = Selector::type()(BonusType::HATE);
-
-	auto allHateEffects = info.attacker->getBonuses(selectorHate, cachingStrHate);
-
 	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/BonusList.cpp

@@ -195,7 +195,6 @@ std::shared_ptr<const Bonus> BonusList::getFirst(const CSelector &selector) cons
 
 void BonusList::getBonuses(BonusList & out, const CSelector &selector, const CSelector &limit) const
 {
-	out.reserve(bonuses.size());
 	for(const auto & b : bonuses)
 	{
 		if(selector(b.get()) && (!limit || ((bool)limit && limit(b.get()))))
@@ -259,11 +258,6 @@ void BonusList::resize(BonusList::TInternalContainer::size_type sz, const std::s
 	changed();
 }
 
-void BonusList::reserve(TInternalContainer::size_type sz)
-{
-	bonuses.reserve(sz);
-}
-
 void BonusList::insert(BonusList::TInternalContainer::iterator position, BonusList::TInternalContainer::size_type n, const std::shared_ptr<Bonus> & x)
 {
 	bonuses.insert(position, n, x);

lib/bonuses/BonusList.h

@@ -17,7 +17,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 class DLL_LINKAGE BonusList
 {
 public:
-	using TInternalContainer = std::vector<std::shared_ptr<Bonus>>;
+	using TInternalContainer = boost::container::small_vector<std::shared_ptr<Bonus>, 16>;
 
 private:
 	TInternalContainer bonuses;
@@ -43,7 +43,6 @@ public:
 	void clear();
 	bool empty() const { return bonuses.empty(); }
 	void resize(TInternalContainer::size_type sz, const std::shared_ptr<Bonus> & c = nullptr);
-	void reserve(TInternalContainer::size_type sz);
 	TInternalContainer::size_type capacity() const { return bonuses.capacity(); }
 	STRONG_INLINE std::shared_ptr<Bonus> &operator[] (TInternalContainer::size_type n) { return bonuses[n]; }
 	STRONG_INLINE const std::shared_ptr<Bonus> &operator[] (TInternalContainer::size_type n) const { return bonuses[n]; }

lib/bonuses/CBonusProxy.cpp

@@ -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,7 +211,7 @@ bool CCheckProxy::getHasBonus() const
 
 	if(treeVersion != cachedLast)
 	{
-		hasBonus = target->hasBonus(selector);
+		hasBonus = target->hasBonus(selector, cachingStr);
 		cachedLast = treeVersion;
 	}
 

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,6 +82,7 @@ public:
 
 private:
 	const IBonusBearer * target;
+	std::string cachingStr;
 	CSelector selector;
 
 	mutable int64_t cachedLast;

lib/bonuses/CBonusSystemNode.cpp

@@ -63,22 +63,10 @@ void CBonusSystemNode::getAllParents(TCNodes & out) const //retrieves list of pa
 
 void CBonusSystemNode::getAllBonusesRec(BonusList &out, const CSelector & selector) const
 {
-	//out has been reserved sufficient capacity at getAllBonuses() call
-
 	BonusList beforeUpdate;
 	TCNodes lparents;
 	getAllParents(lparents);
 
-	if(!lparents.empty())
-	{
-		//estimate on how many bonuses are missing yet - must be positive
-		beforeUpdate.reserve(std::max(out.capacity() - out.size(), bonuses.size()));
-	}
-	else
-	{
-		beforeUpdate.reserve(bonuses.size()); //at most all local bonuses
-	}
-
 	for(const auto * parent : lparents)
 	{
 		parent->getAllBonusesRec(beforeUpdate, selector);
@@ -111,46 +99,64 @@ TConstBonusListPtr CBonusSystemNode::getAllBonuses(const CSelector &selector, co
 {
 	if (CBonusSystemNode::cachingEnabled)
 	{
-		// Exclusive access for one thread
-		boost::lock_guard<boost::mutex> lock(sync);
-
-		// 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.
-		if (cachedLast != treeChanged)
-		{
-			BonusList allBonuses;
-			allBonuses.reserve(cachedBonuses.capacity()); //we assume we'll get about the same number of bonuses
-
-			cachedBonuses.clear();
-			cachedRequests.clear();
-
-			getAllBonusesRec(allBonuses, Selector::all);
-			limitBonuses(allBonuses, cachedBonuses);
-			cachedBonuses.stackBonuses();
-
-			cachedLast = treeChanged;
-		}
-
 		// 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(!cachingStr.empty())
+		if (cachedLast == treeChanged && !cachingStr.empty())
-		{
-			auto it = cachedRequests.find(cachingStr);
-			if(it != cachedRequests.end())
 		{
+			RequestsMap::const_accessor accessor;
+
 			//Cached list contains bonuses for our query with applied limiters
-				return it->second;
+			if (cachedRequests.find(accessor, cachingStr) && accessor->second.first == cachedLast)
-			}
+				return accessor->second.second;
 		}
 
 		//We still don't have the bonuses (didn't returned them from cache)
 		//Perform bonus selection
 		auto ret = std::make_shared<BonusList>();
+
+		if (cachedLast == treeChanged)
+		{
+			// Cached bonuses are up-to-date - use shared/read access and compute results
+			std::shared_lock lock(sync);
 			cachedBonuses.getBonuses(*ret, selector, limit);
+		}
+		else
+		{
+			// 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)
+			{
+				// While our thread was waiting, another one have updated bonus tree. Use cached bonuses.
+				cachedBonuses.getBonuses(*ret, selector, limit);
+			}
+			else
+			{
+				// Cached bonuses may be outdated - regenerate them
+				BonusList allBonuses;
+
+				cachedBonuses.clear();
+
+				getAllBonusesRec(allBonuses, Selector::all);
+				limitBonuses(allBonuses, cachedBonuses);
+				cachedBonuses.stackBonuses();
+				cachedLast = treeChanged;
+				cachedBonuses.getBonuses(*ret, selector, limit);
+			}
+		}
 
 		// Save the results in the cache
 		if (!cachingStr.empty())
-			cachedRequests[cachingStr] = ret;
+		{
+			RequestsMap::accessor accessor;
+			if (cachedRequests.find(accessor, cachingStr))
+			{
+				accessor->second.second = ret;
+				accessor->second.first = cachedLast;
+			}
+			else
+				cachedRequests.emplace(cachingStr, std::pair<int64_t, TBonusListPtr>{ cachedLast, ret });
+		}
 
 		return ret;
 	}

lib/bonuses/CBonusSystemNode.h

@@ -14,6 +14,8 @@
 
 #include "../serializer/Serializeable.h"
 
+#include <tbb/concurrent_hash_map.h>
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 using TNodes = std::set<CBonusSystemNode *>;
@@ -30,6 +32,19 @@ public:
 		UNKNOWN, STACK_INSTANCE, STACK_BATTLE, SPECIALTY, ARTIFACT, CREATURE, ARTIFACT_INSTANCE, HERO, PLAYER, TEAM,
 		TOWN_AND_VISITOR, BATTLE, COMMANDER, GLOBAL_EFFECTS, ALL_CREATURES, TOWN
 	};
+
+	struct HashStringCompare {
+		static size_t hash(const std::string& data)
+		{
+			std::hash<std::string> hasher;
+			return hasher(data);
+		}
+		static bool equal(const std::string& x, const std::string& y)
+		{
+			return x == y;
+		}
+	};
+
 private:
 	BonusList bonuses; //wielded bonuses (local or up-propagated here)
 	BonusList exportedBonuses; //bonuses coming from this node (wielded or propagated away)
@@ -49,8 +64,9 @@ private:
 	// 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
-	mutable std::map<std::string, TBonusListPtr > cachedRequests;
+	using RequestsMap = tbb::concurrent_hash_map<std::string, std::pair<int64_t, TBonusListPtr>, HashStringCompare>;
-	mutable boost::mutex sync;
+	mutable RequestsMap cachedRequests;
+	mutable std::shared_mutex sync;
 
 	void getAllBonusesRec(BonusList &out, const CSelector & selector) const;
 	TConstBonusListPtr getAllBonusesWithoutCaching(const CSelector &selector, const CSelector &limit) const;

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;
+	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;
+	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 std::string &cachingStr = {}) const;
-	bool hasBonus(const CSelector &selector, const CSelector &limit, 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 CSelector &limit, const std::string &cachingStr = {}) const;
-	TConstBonusListPtr getBonuses(const CSelector &selector, 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/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);
@@ -325,7 +323,7 @@ const Skill * SecondarySkill::toEntity(const Services * services) const
 
 const CCreature * CreatureIDBase::toCreature() const
 {
-	return dynamic_cast<const CCreature *>(toEntity(VLC));
+	return (*VLC->creh)[num];
 }
 
 const Creature * CreatureIDBase::toEntity(const Services * services) const
@@ -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 std::array<PrimarySkill, 4> & ALL_SKILLS();
-	static const PrimarySkill END;
 
 	static const PrimarySkill EXPERIENCE;
 

lib/entities/hero/CHeroClassHandler.cpp

@@ -32,7 +32,7 @@ void CHeroClassHandler::fillPrimarySkillData(const JsonNode & node, CHeroClass *
 {
 	const auto & skillName = NPrimarySkill::names[pSkill.getNum()];
 	auto currentPrimarySkillValue = static_cast<int>(node["primarySkills"][skillName].Integer());
-	int primarySkillLegalMinimum = VLC->engineSettings()->getVector(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS)[pSkill.getNum()];
+	int primarySkillLegalMinimum = VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, pSkill.getNum());
 
 	if(currentPrimarySkillValue < primarySkillLegalMinimum)
 	{

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/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/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

@@ -257,8 +257,7 @@ void CGHeroInstance::setMovementPoints(int points)
 
 int CGHeroInstance::movementPointsLimit(bool onLand) const
 {
-	TurnInfo ti(this);
+	return valOfBonuses(BonusType::MOVEMENT, onLand ? BonusCustomSubtype::heroMovementLand : BonusCustomSubtype::heroMovementSea);
-	return movementPointsLimitCached(onLand, &ti);
 }
 
 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);
 
@@ -705,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;
@@ -725,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
-{
-	return sqrt((1.0 + 0.05 * getPrimSkillLevel(PrimarySkill::KNOWLEDGE)) * (1.0 + 0.05 * getPrimSkillLevel(PrimarySkill::SPELL_POWER)));
 }
 
 double CGHeroInstance::getHeroStrength() const
 {
-	return sqrt(pow(getFightingStrength(), 2.0) * pow(getMagicStrength(), 2.0));
+	const auto & primarySkills = getPrimarySkills();
+	return getFightingStrengthImpl(primarySkills) * getMagicStrengthImpl(primarySkills);
 }
 
-double CGHeroInstance::getHeroStrengthForCampaign() const
+uint64_t CGHeroInstance::getValueForDiplomacy() const
 {
-	return sqrt(pow(getFightingStrength(), 2.0) * pow(getMagicStrengthForCampaign(), 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;
+}
+
+uint32_t CGHeroInstance::getValueForCampaign() const
+{
+	/// 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
@@ -973,7 +1017,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 +1185,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 +1324,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)
@@ -1655,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);
 			}
 		}
 	}
@@ -1850,7 +1893,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,9 +1964,9 @@ 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()->getVector(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS)[which.getNum()];
+	auto minSkillValue = VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, which.getNum());
 	return std::max(valOfBonuses(selector, cachingStr), minSkillValue);
 }
 

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

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/CPathfinder.cpp

@@ -596,11 +596,12 @@ void CPathfinderHelper::getNeighbours(
 			continue;
 
 		const TerrainTile & destTile = map->getTile(destCoord);
-		if(!destTile.getTerrain()->isPassable())
+		const TerrainType* terrain = destTile.getTerrain();
+		if(!terrain->isPassable())
 			continue;
 
 		/// Following condition let us avoid diagonal movement over coast when sailing
-		if(srcTile.isWater() && limitCoastSailing && destTile.isWater() && dir.x && dir.y) //diagonal move through water
+		if(srcTile.isWater() && limitCoastSailing && terrain->isWater() && dir.x && dir.y) //diagonal move through water
 		{
 			const int3 horizontalNeighbour = srcCoord + int3{dir.x, 0, 0};
 			const int3 verticalNeighbour = srcCoord + int3{0, dir.y, 0};
@@ -608,7 +609,7 @@ void CPathfinderHelper::getNeighbours(
 				continue;
 		}
 
-		if(indeterminate(onLand) || onLand == destTile.isLand())
+		if(indeterminate(onLand) || onLand == terrain->isLand())
 		{
 			vec.push_back(destCoord);
 		}

lib/pathfinder/CPathfinder.h

@@ -13,9 +13,6 @@
 #include "../IGameCallback.h"
 #include "../bonuses/BonusEnum.h"
 
-#include <boost/container/static_vector.hpp>
-#include <boost/container/small_vector.hpp>
-
 VCMI_LIB_NAMESPACE_BEGIN
 
 class CGWhirlpool;

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/rewardable/Interface.cpp

@@ -38,11 +38,8 @@ std::vector<ui32> Rewardable::Interface::getAvailableRewards(const CGHeroInstanc
 		const Rewardable::VisitInfo & visit = configuration.info[i];
 
 		if(event == visit.visitType && (!hero || visit.limiter.heroAllowed(hero)))
-		{
-			logGlobal->trace("Reward %d is allowed", i);
 			ret.push_back(static_cast<ui32>(i));
 	}
-	}
 	return ret;
 }
 
@@ -119,6 +116,7 @@ void Rewardable::Interface::grantRewardBeforeLevelup(const Rewardable::VisitInfo
 	}
 
 	for(int i=0; i< info.reward.primary.size(); i++)
+		if (info.reward.primary[i] != 0)
 			cb->changePrimSkill(hero, static_cast<PrimarySkill>(i), info.reward.primary[i], false);
 
 	TExpType expToGive = 0;

lib/serializer/BinaryDeserializer.h

@@ -215,6 +215,15 @@ public:
 			load( data[i]);
 	}
 
+	template <typename T, size_t N>
+	void load(boost::container::small_vector<T, N>& data)
+	{
+		uint32_t length = readAndCheckLength();
+		data.resize(length);
+		for (uint32_t i = 0; i < length; i++)
+			load(data[i]);
+	}
+
 	template <typename T, typename std::enable_if_t < !std::is_same_v<T, bool >, int  > = 0>
 	void load(std::deque<T> & data)
 	{

lib/serializer/BinarySerializer.h

@@ -275,6 +275,15 @@ public:
 		for(uint32_t i=0;i<length;i++)
 			save(data[i]);
 	}
+	template <typename T, size_t N>
+	void save(const boost::container::small_vector<T, N>& data)
+	{
+		uint32_t length = data.size();
+		*this& length;
+		for (uint32_t i = 0; i < length; i++)
+			save(data[i]);
+	}
+
 	template <typename T, typename std::enable_if_t < !std::is_same_v<T, bool >, int  > = 0>
 	void save(const std::deque<T> & data)
 	{

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;

mapeditor/CMakeLists.txt

@@ -223,6 +223,9 @@ if(APPLE)
 	set_property(GLOBAL PROPERTY AUTOGEN_TARGETS_FOLDER vcmieditor)
 endif()
 
+# Qt defines 'emit' as macros, which conflicts with TBB definition of method with same name
+target_compile_definitions(vcmieditor PRIVATE QT_NO_EMIT)
+
 if(ENABLE_STATIC_LIBS OR NOT (ENABLE_EDITOR AND ENABLE_LAUNCHER))
 	target_compile_definitions(vcmieditor PRIVATE VCMIQT_STATIC)
 endif()

mapeditor/inspector/artifactwidget.cpp

@@ -23,7 +23,7 @@ ArtifactWidget::ArtifactWidget(CArtifactFittingSet & fittingSet, QWidget * paren
 
 	connect(ui->saveButton, &QPushButton::clicked, this, [this]() 
 	{
-		emit saveArtifact(ui->artifact->currentData().toInt(), ArtifactPosition(ui->possiblePositions->currentData().toInt()));
+		saveArtifact(ui->artifact->currentData().toInt(), ArtifactPosition(ui->possiblePositions->currentData().toInt()));
 		close();
 	});
 	connect(ui->cancelButton, &QPushButton::clicked, this, &ArtifactWidget::close);

mapeditor/inspector/questwidget.cpp

@@ -428,8 +428,8 @@ bool QuestDelegate::eventFilter(QObject * object, QEvent * event)
 			return false;
 		if(event->type() == QEvent::Close)
 		{
-			emit commitData(ed);
+			commitData(ed);
-			emit closeEditor(ed);
+			closeEditor(ed);
 			return true;
 		}
 	}

mapeditor/mapsettings/rumorsettings.cpp

@@ -60,7 +60,7 @@ void RumorSettings::on_add_clicked()
 	item->setData(Qt::UserRole, QVariant(""));
 	item->setFlags(item->flags() | Qt::ItemIsEditable);
 	ui->rumors->addItem(item);
-	emit ui->rumors->itemActivated(item);
+	ui->rumors->itemActivated(item);
 }
 
 void RumorSettings::on_remove_clicked()

mapeditor/mapview.cpp

@@ -47,7 +47,7 @@ void MinimapView::mouseMoveEvent(QMouseEvent *mouseEvent)
 	
 	auto pos = mapToScene(mouseEvent->pos());
 	pos *= 32;
-	emit cameraPositionChanged(pos);
+	cameraPositionChanged(pos);
 }
 
 void MinimapView::mousePressEvent(QMouseEvent* event)
@@ -90,7 +90,7 @@ void MapView::mouseMoveEvent(QMouseEvent *mouseEvent)
 	if(tile == tilePrev) //do not redraw
 		return;
 
-	emit currentCoordinates(tile.x, tile.y);
+	currentCoordinates(tile.x, tile.y);
 
 	switch(selectionTool)
 	{
@@ -563,7 +563,7 @@ void MapView::mouseReleaseEvent(QMouseEvent *event)
 		auto selection = sc->selectionObjectsView.getSelection();
 		if(selection.size() == 1)
 		{
-			emit openObjectProperties(*selection.begin(), tab);
+			openObjectProperties(*selection.begin(), tab);
 		}
 		break;
 	}
@@ -618,7 +618,7 @@ void MapView::dropEvent(QDropEvent * event)
 		{
 			auto * obj = sc->selectionObjectsView.newObject;
 			controller->commitObjectCreate(sc->level);
-			emit openObjectProperties(obj, false);
+			openObjectProperties(obj, false);
 		}
 		else
 		{
@@ -736,13 +736,13 @@ void MapScene::updateViews()
 void MapScene::terrainSelected(bool anythingSelected)
 {
 	isTerrainSelected = anythingSelected;
-	emit selected(isTerrainSelected || isObjectSelected);
+	selected(isTerrainSelected || isObjectSelected);
 }
 
 void MapScene::objectSelected(bool anythingSelected)
 {
 	isObjectSelected = anythingSelected;
-	emit selected(isTerrainSelected || isObjectSelected);
+	selected(isTerrainSelected || isObjectSelected);
 }
 
 MinimapScene::MinimapScene(int lvl):

mapeditor/scenelayer.cpp

@@ -182,14 +182,14 @@ void ObjectPickerLayer::select(const CGObjectInstance * obj)
 	if(obj && possibleObjects.count(obj))
 	{
 		clear();
-		emit selectionMade(obj);
+		selectionMade(obj);
 	}
 }
 
 void ObjectPickerLayer::discard()
 {
 	clear();
-	emit selectionMade(nullptr);
+	selectionMade(nullptr);
 }
 
 SelectionTerrainLayer::SelectionTerrainLayer(MapSceneBase * s): AbstractLayer(s)
@@ -277,7 +277,7 @@ const std::set<int3> & SelectionTerrainLayer::selection() const
 
 void SelectionTerrainLayer::onSelection()
 {
-	emit selectionMade(!area.empty());
+	selectionMade(!area.empty());
 }
 
 
@@ -610,7 +610,7 @@ void SelectionObjectsLayer::clear()
 
 void SelectionObjectsLayer::onSelection()
 {
-	emit selectionMade(!selectedObjects.empty());
+	selectionMade(!selectedObjects.empty());
 }
 
 void SelectionObjectsLayer::setLockObject(const CGObjectInstance * object, bool lock)

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)