better resources and mines gathering; include TODOs for all the things that need to be investigated

AI/Nullkiller2/Analyzers/DangerHitMapAnalyzer.cpp

@@ -140,7 +140,7 @@ void DangerHitMapAnalyzer::updateHitMap()
 				newThreat.hero = path.targetHero;
 				newThreat.turn = path.turn();
 				newThreat.threat = path.getHeroStrength() * (1 - path.movementCost() / 2.0);
-				// Why is this danger calculated so differently than FuzzyHelper::evaluateDanger?
+				// TODO: Mircea: Why is this danger calculated so differently than FuzzyHelper::evaluateDanger?
 				// shouldn't it use the path danger instead of hero strength?
 				newThreat.danger = path.getHeroStrength();
 				auto danger2 = aiNk->dangerEvaluator->evaluateDanger(pos, path.targetHero);

AI/Nullkiller2/Analyzers/ObjectClusterizer.cpp

@@ -425,12 +425,15 @@ void ObjectClusterizer::clusterizeObject(
 	for(auto & path : pathCache)
 	{
 #if NKAI_TRACE_LEVEL >= 2
-		logAi->trace("Checking path %s", path.toString());
+		logAi->trace("ObjectClusterizer Checking path %s", path.toString());
 #endif
 
 		if(aiNk->heroManager->getHeroRole(path.targetHero) == HeroRole::SCOUT)
 		{
-			if(path.movementCost() > 2.0f)
+			// TODO: Mircea: Shouldn't this be linked with scoutHeroTurnDistanceLimit?
+			// TODO: Mircea: Move to constant
+			// if(path.movementCost() > 2.0f)
+			if(path.movementCost() > aiNk->settings->getScoutHeroTurnDistanceLimit())
 			{
 #if NKAI_TRACE_LEVEL >= 2
 				logAi->trace("Path is too far %f", path.movementCost());
@@ -438,11 +441,12 @@ void ObjectClusterizer::clusterizeObject(
 				continue;
 			}
 		}
+		// TODO: Mircea: Move to constant
 		else if(path.movementCost() > 4.0f && obj->ID != Obj::TOWN)
 		{
 			auto strategicalValue = valueEvaluator.getStrategicalValue(obj);
 
-			if(strategicalValue < 0.3f)
+			if(strategicalValue < MINIMUM_STRATEGICAL_VALUE_NON_TOWN)
 			{
 #if NKAI_TRACE_LEVEL >= 2
 				logAi->trace("Object value is too low %f", strategicalValue);
@@ -454,7 +458,7 @@ void ObjectClusterizer::clusterizeObject(
 		if(!shouldVisit(aiNk, path.targetHero, obj))
 		{
 #if NKAI_TRACE_LEVEL >= 2
-			logAi->trace("Hero %s does not need to visit %s", path.targetHero->getObjectName(), obj->getObjectName());
+			logAi->trace("Hero %s shouldn't visit %s", path.targetHero->getObjectName(), obj->getObjectName());
 #endif
 			continue;
 		}
@@ -484,7 +488,7 @@ void ObjectClusterizer::clusterizeObject(
 
 				if(aiNk->settings->isUseFuzzy() && priority < MIN_PRIORITY)
 					continue;
-				else if (priority <= 0)
+				if (priority <= 0)
 					continue;
 
 				ClusterMap::accessor cluster;
@@ -510,9 +514,10 @@ void ObjectClusterizer::clusterizeObject(
 
 		if (aiNk->settings->isUseFuzzy() && priority < MIN_PRIORITY)
 			continue;
-		else if (priority <= 0)
+		if (priority <= 0)
 			continue;
 
+		// TODO: Mircea: Move to constant
 		bool interestingObject = path.turn() <= 2 || priority > (aiNk->settings->isUseFuzzy() ? 0.5f : 0);
 
 		if(interestingObject)

AI/Nullkiller2/Analyzers/ObjectClusterizer.h

@@ -15,6 +15,8 @@
 namespace NK2AI
 {
 
+static constexpr float MINIMUM_STRATEGICAL_VALUE_NON_TOWN = 0.3f;
+
 struct ClusterObjectInfo
 {
 	float priority = 0.f;

AI/Nullkiller2/Behaviors/ClusterBehavior.cpp

@@ -62,7 +62,7 @@ Goals::TGoalVec ClusterBehavior::decomposeCluster(const Nullkiller * aiNk, std::
 	for(auto path = paths.begin(); path != paths.end();)
 	{
 #if NKAI_TRACE_LEVEL >= 2
-		logAi->trace("Checking path %s", path->toString());
+		logAi->trace("ClusterBehavior Checking path %s", path->toString());
 #endif
 
 		auto blocker = aiNk->objectClusterizer->getBlocker(*path);

AI/Nullkiller2/Behaviors/DefenceBehavior.cpp

@@ -425,6 +425,7 @@ void DefenceBehavior::evaluateDefence(Goals::TGoalVec & tasks, const CGTownInsta
 
 void DefenceBehavior::evaluateRecruitingHero(Goals::TGoalVec & tasks, const HitMapInfo & threat, const CGTownInstance * town, const Nullkiller * aiNk) const
 {
+	// TODO: Mircea: Shouldn't it be threat.turn < 1? How does the current one make sense?
 	if (threat.turn > 0 || town->getGarrisonHero() || town->getVisitingHero())
 		return;
 	
@@ -435,6 +436,8 @@ void DefenceBehavior::evaluateRecruitingHero(Goals::TGoalVec & tasks, const HitM
 
 		for(auto hero : heroesInTavern)
 		{
+			// TODO: Mircea: Investigate if this logic might be off, as the attacker will most probably be more powerful than a tavern hero
+			// A new hero improves the defence strength of town's army if it has defence > 0 in primary skills
 			if(hero->getTotalStrength() < threat.danger)
 				continue;
 
@@ -484,9 +487,11 @@ void DefenceBehavior::evaluateRecruitingHero(Goals::TGoalVec & tasks, const HitM
 				}
 
 				// avoid dismissing one weak hero in order to recruit another.
+				// TODO: Mircea: Move to constant
 				if(heroToDismiss && heroToDismiss->getArmyStrength() + 500 > hero->getArmyStrength())
 					continue;
 			}
+			// TODO: Mircea: Check if it immediately dismisses after losing a castle, though that implies losing a hero too if present in the castle
 			else if(aiNk->heroManager->heroCapReached())
 			{
 				heroToDismiss = aiNk->heroManager->findWeakHeroToDismiss(hero->getArmyStrength(), town);

AI/Nullkiller2/Behaviors/GatherArmyBehavior.cpp

@@ -290,9 +290,7 @@ Goals::TGoalVec GatherArmyBehavior::upgradeArmy(const Nullkiller * aiNk, const C
 		auto upgrade = aiNk->armyManager->calculateCreaturesUpgrade(path.heroArmy, upgrader, availableResources);
 
 		if(!upgrader->getGarrisonHero()
-			&& (
-				hasMainAround
-				|| aiNk->heroManager->getHeroRole(path.targetHero) == HeroRole::MAIN))
+		   && (hasMainAround || aiNk->heroManager->getHeroRole(path.targetHero) == HeroRole::MAIN))
 		{
 			ArmyUpgradeInfo armyToGetOrBuy;
 
@@ -310,20 +308,19 @@ Goals::TGoalVec GatherArmyBehavior::upgradeArmy(const Nullkiller * aiNk, const C
 			vstd::concatenate(upgrade.resultingArmy, armyToGetOrBuy.resultingArmy);
 
 			if(!upgrade.upgradeValue
-				&& armyToGetOrBuy.upgradeValue > 20000
+				&& armyToGetOrBuy.upgradeValue > 20000 // TODO: Mircea: Move to constant
 				&& aiNk->heroManager->canRecruitHero(upgrader)
-				&& path.turn() < aiNk->settings->getScoutHeroTurnDistanceLimit())
+				&& path.turn() < aiNk->settings->getScoutHeroTurnDistanceLimit()) // TODO: Mircea: Inspect what this does
 			{
 				for(auto hero : ccTl->getAvailableHeroes(upgrader))
 				{
 					auto scoutReinforcement = aiNk->armyManager->howManyReinforcementsCanGet(hero, upgrader);
 
 					if(scoutReinforcement >= armyToGetOrBuy.upgradeValue
-						&& aiNk->getFreeGold() >20000
+						&& aiNk->getFreeGold() > 20000 // TODO: Mircea: Move to constant
 						&& !aiNk->buildAnalyzer->isGoldPressureOverMax())
 					{
 						Composition recruitHero;
-
 						recruitHero.addNext(ArmyUpgrade(path.targetHero, town, armyToGetOrBuy)).addNext(RecruitHero(upgrader, hero));
 					}
 				}
@@ -332,6 +329,7 @@ Goals::TGoalVec GatherArmyBehavior::upgradeArmy(const Nullkiller * aiNk, const C
 
 		auto armyValue = (float)upgrade.upgradeValue / path.getHeroStrength();
 
+		// TODO: Mircea: Move to constant
 		if((armyValue < 0.25f && upgrade.upgradeValue < 40000) || upgrade.upgradeValue < 2000) // avoid small upgrades
 		{
 #if NKAI_TRACE_LEVEL >= 2
@@ -341,7 +339,6 @@ Goals::TGoalVec GatherArmyBehavior::upgradeArmy(const Nullkiller * aiNk, const C
 		}
 
 		auto danger = path.getTotalDanger();
-
 		auto isSafe = isSafeToVisit(path.targetHero, path.heroArmy, danger, aiNk->settings->getSafeAttackRatio());
 
 #if NKAI_TRACE_LEVEL >= 2

AI/Nullkiller2/Engine/DeepDecomposer.cpp

@@ -68,10 +68,12 @@ void DeepDecomposer::decompose(TGoalVec & results, TSubgoal behavior, int depthL
 
 			if(subgoal->isElementar())
 			{
-				// need to get rid of priority control in behaviors like Startup to avoid this check.
+				// TODO: need to get rid of priority control in behaviors like Startup to avoid this check.
 				// 0 - goals directly from behavior
 				Goals::TSubgoal task = depth >= 1 ? aggregateGoals(0, subgoal) : subgoal;
 
+				// TODO: Mircea: Issue with CGameHandler::spawnWanderingMonsters, see getFreeTiles(tiles, true);
+				// danger not linked GraphPaths::addChainInfo, so spawning only with nearby unblocked
 #if NKAI_TRACE_LEVEL >= 1
 				logAi->trace("Found task %s", task->toString());
 #endif

AI/Nullkiller2/Engine/Nullkiller.cpp

@@ -295,7 +295,7 @@ void Nullkiller::updateState(bool partialUpdate)
 
 		if(scanDepth != ScanDepth::ALL_FULL || isObjectGraphAllowed())
 		{
-			cfg.scoutTurnDistanceLimit =settings->getScoutHeroTurnDistanceLimit();
+			cfg.scoutTurnDistanceLimit = settings->getScoutHeroTurnDistanceLimit();
 		}
 
 		makingTurnInterrupption.interruptionPoint();

AI/Nullkiller2/Engine/PriorityEvaluator.cpp

@@ -377,12 +377,8 @@ float RewardEvaluator::getEnemyHeroStrategicalValue(const CGHeroInstance * enemy
 	return std::min(1.5f, objectValue * 0.9f + (1.5f - (1.5f / (1 + enemy->level))));
 }
 
-/**
- * getNowResourceRequirementStrength
- * @param resType
- * @return between 0-1.0f
- */
-float RewardEvaluator::getResourceRequirementStrength(GameResID resType) const
+/// @return between 0-1.0f
+float RewardEvaluator::getNowResourceRequirementStrength(GameResID resType) const
 {
 	TResources requiredResources = aiNk->buildAnalyzer->getResourcesRequiredNow();
 	TResources dailyIncome = aiNk->buildAnalyzer->getDailyIncome();
@@ -393,14 +389,10 @@ float RewardEvaluator::getResourceRequirementStrength(GameResID resType) const
 	if(dailyIncome[resType] == 0)
 		return 1.0f;
 
-	return 0.95f;
+	return 0.8f;
 }
 
-/**
- *
- * @param resType
- * @return between 0-1.0f
- */
+/// @return between 0-1.0f
 float RewardEvaluator::getTotalResourceRequirementStrength(GameResID resType) const
 {
 	TResources requiredResources = aiNk->buildAnalyzer->getTotalResourcesRequired();
@@ -412,7 +404,7 @@ float RewardEvaluator::getTotalResourceRequirementStrength(GameResID resType) co
 	if(dailyIncome[resType] == 0)
 		return 1.0f;
 
-	return 0.95f;
+	return 0.8f;
 }
 
 uint64_t RewardEvaluator::townArmyGrowth(const CGTownInstance * town) const
@@ -436,22 +428,18 @@ float RewardEvaluator::getManaRecoveryArmyReward(const CGHeroInstance * hero) co
 	return aiNk->heroManager->getMagicStrength(hero) * 10000 * (1.0f - std::sqrt(static_cast<float>(hero->mana) / hero->manaLimit()));
 }
 
-/**
- * getCombinedResourceRequirementStrength
- * @param res
- * @return between 0-1.0f
- */
-float RewardEvaluator::getResourceRequirementStrength(const TResources & res) const
+/// @return between 0-1.0f
+float RewardEvaluator::getCombinedResourceRequirementStrength(const TResources & res) const
 {
 	float sum = 0.0f;
 
 	for(TResources::nziterator it(res); it.valid(); it++)
 	{
-		auto calculation = 0.6f * getResourceRequirementStrength(it->resType)
-			+ 0.4f * getTotalResourceRequirementStrength(it->resType);
+		auto calculation = 0.5f * getNowResourceRequirementStrength(it->resType)
+			+ 0.5f * getTotalResourceRequirementStrength(it->resType);
 
 		// Even not required resources should be valuable because they shouldn't be left for the enemies to collect
-		sum += std::min(0.5f, calculation);
+		sum += std::min(MINIMUM_STRATEGICAL_VALUE_NON_TOWN, calculation);
 	}
 
 	return sum;
@@ -471,7 +459,7 @@ float RewardEvaluator::getStrategicalValue(const CGObjectInstance * target, cons
 		res[mine->producedResource] = mine->producedQuantity;
 
 		// Mines should have higher priority than resources
-		return 1.0f + getResourceRequirementStrength(res);
+		return 1.0f + getCombinedResourceRequirementStrength(res);
 	}
 
 	case Obj::RESOURCE:
@@ -480,7 +468,7 @@ float RewardEvaluator::getStrategicalValue(const CGObjectInstance * target, cons
 		TResources res;
 		res[resource->resourceID()] = resource->getAmount();
 
-		return getResourceRequirementStrength(res);
+		return getCombinedResourceRequirementStrength(res);
 	}
 
 	case Obj::TOWN:
@@ -506,8 +494,8 @@ float RewardEvaluator::getStrategicalValue(const CGObjectInstance * target, cons
 
 		if(fortLevel < CGTownInstance::CITADEL)
 			return booster * (town->hasFort() ? 1.0 : 0.8);
-		else
-			return booster * (fortLevel == CGTownInstance::CASTLE ? 1.4 : 1.2);
+
+		return booster * (fortLevel == CGTownInstance::CASTLE ? 1.4 : 1.2);
 	}
 
 	case Obj::HERO:
@@ -530,7 +518,7 @@ float RewardEvaluator::getStrategicalValue(const CGObjectInstance * target, cons
 
 		for(int index : rewardable->getAvailableRewards(hero, Rewardable::EEventType::EVENT_FIRST_VISIT))
 		{
-			resourceReward += getResourceRequirementStrength(rewardable->configuration.info[index].reward.resources);
+			resourceReward += getCombinedResourceRequirementStrength(rewardable->configuration.info[index].reward.resources);
 		}
 
 		return resourceReward;
@@ -1383,7 +1371,7 @@ float PriorityEvaluator::evaluate(Goals::TSubgoal task, int priorityTier)
 			return 0;
 		}
 		const bool amIInDanger = aiNk->cc->getTownsInfo().empty();
-		// Shouldn't it default to 0 instead of 1.0 in the end?
+		// TODO: Mircea: Shouldn't it default to 0 instead of 1.0 in the end?
 		const float maxWillingToLose = amIInDanger ? 1 : aiNk->settings->getMaxArmyLossTarget() * evaluationContext.powerRatio > 0 ? aiNk->settings->getMaxArmyLossTarget() * evaluationContext.powerRatio : 1.0;
 		float dangerThreshold = 1;
 		dangerThreshold *= evaluationContext.powerRatio > 0 ? evaluationContext.powerRatio : 1.0;
@@ -1512,47 +1500,72 @@ float PriorityEvaluator::evaluate(Goals::TSubgoal task, int priorityTier)
 			case PriorityTier::HUNTER_GATHER: //Collect guarded stuff
 				//FALL_THROUGH
 			case PriorityTier::FAR_HUNTER_GATHER:
-			// FIXME: Should not go to something that gives army if no slots available in the hero, but probably not in the evaluator, but in the finder
+			// TODO: Mircea: Should not go to something that gives army if no slots available in the hero, but probably not in the evaluator, but in the finder
 			// task.get()->hero->getSlotFor(creature, 7) == false (not sure I get to know which creature is there in Orc Tower building)
 			// /// so I can't know for sure if it fits my stacks or not, but at least we can avoid going there with all 7 stacks occupied by other units
 			// task.get()->hero->getFreeSlots(7) == 7
 			// getDuplicatingSlots(task.get()->hero) == false
 			{
 				if (evaluationContext.enemyHeroDangerRatio > dangerThreshold && !evaluationContext.isDefend && priorityTier != PriorityTier::FAR_HUNTER_GATHER)
+				{
+					logAi->trace("case PriorityTier::FAR_HUNTER_GATHER if 1");
 					return 0;
+				}
 				if (evaluationContext.buildingCost.marketValue() > 0)
+				{
+					logAi->trace("case PriorityTier::FAR_HUNTER_GATHER if 2");
 					return 0;
+				}
 				if (priorityTier != PriorityTier::FAR_HUNTER_GATHER && evaluationContext.isDefend && (evaluationContext.enemyHeroDangerRatio > dangerThreshold || evaluationContext.threatTurns > 0 || evaluationContext.turn > 0))
+				{
+					logAi->trace("case PriorityTier::FAR_HUNTER_GATHER if 3");
 					return 0;
+				}
 				if (evaluationContext.explorePriority == 3)
+				{
+					logAi->trace("case PriorityTier::FAR_HUNTER_GATHER if 4");
 					return 0;
+				}
 				if (priorityTier != PriorityTier::FAR_HUNTER_GATHER && ((evaluationContext.enemyHeroDangerRatio > 0 && arriveNextWeek) || evaluationContext.enemyHeroDangerRatio > dangerThreshold))
+				{
+					logAi->trace("case PriorityTier::FAR_HUNTER_GATHER if 5");
 					return 0;
+				}
 				if (maxWillingToLose - evaluationContext.armyLossRatio < 0)
+				{
+					logAi->trace("case PriorityTier::FAR_HUNTER_GATHER if 6");
 					return 0;
+				}
 				if (vstd::isAlmostZero(evaluationContext.armyLossRatio) && evaluationContext.closestWayRatio < 1.0)
+				{
+					logAi->trace("case PriorityTier::FAR_HUNTER_GATHER if 7");
 					return 0;
+				}
+
 				score += evaluationContext.strategicalValue * 1000;
 				score += evaluationContext.goldReward;
 				score += evaluationContext.skillReward * evaluationContext.armyInvolvement * (1 - evaluationContext.armyLossRatio) * 0.05;
 				score += evaluationContext.armyReward;
 				score += evaluationContext.armyGrowth;
-				// score -= evaluationContext.goldCost; // don't include School of Magic cost or others because those locations are beneficial
+				score -= evaluationContext.goldCost / 2; // don't include the full cost of School of Magic or others because those locations are beneficial
 				score -= evaluationContext.armyInvolvement * evaluationContext.armyLossRatio * 0.1;
+
 				logAi->trace("case PriorityTier::FAR_HUNTER_GATHER score %f, strategicalValue %f, goldReward %f, skillRewardMultiplied %f, armyReward %f, armyGrowth %f, goldCost -%f, armyInvolvementMultiplied -%f, "
 				 "armyLossPersentage %f, movementCost %f, enemyHeroDangerRatio %f",
 					score, evaluationContext.strategicalValue, evaluationContext.goldReward, evaluationContext.skillReward * evaluationContext.armyInvolvement * (1 - evaluationContext.armyLossRatio) * 0.05,
 					evaluationContext.armyReward, evaluationContext.armyGrowth, evaluationContext.goldCost, evaluationContext.armyInvolvement * evaluationContext.armyLossRatio,
 					evaluationContext.armyLossRatio, evaluationContext.movementCost, evaluationContext.enemyHeroDangerRatio);
+
 				if (score > 0)
 				{
-					score = 1000;
+					// score = 1000;
 					if (evaluationContext.movementCost > 0)
 					{
 						logAi->trace("case PriorityTier::FAR_HUNTER_GATHER if 8");
 						score -= evaluationContext.movementCost / 20 * score; // we expect movement won't be over 20 turns
 					}
-					if(evaluationContext.enemyHeroDangerRatio > 0) // This doesn't make sense at it always seems to be 0
+					// TODO: Mircea: This doesn't make sense at it always seems to be 0. To test
+					if(evaluationContext.enemyHeroDangerRatio > 0)
 					{
 						logAi->trace("case PriorityTier::FAR_HUNTER_GATHER if 9");
 						score *= 1 - evaluationContext.enemyHeroDangerRatio;
@@ -1563,6 +1576,8 @@ float PriorityEvaluator::evaluate(Goals::TSubgoal task, int priorityTier)
 						score *= 1 - evaluationContext.armyLossRatio;
 					}
 				}
+
+				logAi->trace("case PriorityTier::FAR_HUNTER_GATHER score final %f", score);
 				break;
 			}
 			case PriorityTier::LOW_PRIO_EXPLORE:
@@ -1650,7 +1665,7 @@ float PriorityEvaluator::evaluate(Goals::TSubgoal task, int priorityTier)
 	}
 
 #if NKAI_TRACE_LEVEL >= 2
-	logAi->trace("priorityTier %d, Evaluated %s, loss: %f, turn: %d, turns main: %f, scout: %f, army-involvement: %f, gold: %f, cost: %d, army gain: %f, army growth: %f skill: %f danger: %d, threatTurns: %d, threat: %d, role: %s, strategical value: %f, conquest value: %f cwr: %f, fear: %f, result %f",
+	logAi->trace("priorityTier %d, Evaluated %s, loss: %f, turn: %d, turns main: %f, scout: %f, armyInvolvement: %f, goldRewardVsMovement: %f, cost: %d, armyReward: %f, armyGrowth: %f skillReward: %f danger: %d, threatTurns: %d, threat: %d, heroRole: %s, strategicalValue: %f, conquestValue: %f closestWayRatio: %f, enemyHeroDangerRatio: %f, result %f",
 		priorityTier,
 		task->toString(),
 		evaluationContext.armyLossRatio,

AI/Nullkiller2/Engine/PriorityEvaluator.h

@@ -38,8 +38,8 @@ public:
 	uint64_t getArmyGrowth(const CGObjectInstance * target, const CGHeroInstance * hero, const CCreatureSet * army) const;
 	int getGoldCost(const CGObjectInstance * target, const CGHeroInstance * hero, const CCreatureSet * army) const;
 	float getEnemyHeroStrategicalValue(const CGHeroInstance * enemy) const;
-	float getResourceRequirementStrength(GameResID resType) const;
-	float getResourceRequirementStrength(const TResources & res) const;
+	float getNowResourceRequirementStrength(GameResID resType) const;
+	float getCombinedResourceRequirementStrength(const TResources & res) const;
 	float getStrategicalValue(const CGObjectInstance * target, const CGHeroInstance * hero = nullptr) const;
 	float getConquestValue(const CGObjectInstance* target) const;
 	float getTotalResourceRequirementStrength(GameResID resType) const;

AI/Nullkiller2/Pathfinding/GraphPaths.cpp

@@ -292,6 +292,7 @@ void GraphPaths::addChainInfo(std::vector<AIPath> & paths, int3 tile, const CGHe
 
 			path.armyLoss += loss;
 			path.targetObjectDanger = aiNk->dangerEvaluator->evaluateDanger(tile, path.targetHero, !allowBattle);
+			// TODO: Mircea: This is similar same as 263, so what's happening here? Why strength is passed differently?
 			path.targetObjectArmyLoss = aiNk->pathfinder->getStorage()->evaluateArmyLoss(path.targetHero, path.heroArmy->getArmyStrength(), path.targetObjectDanger);
 
 			paths.push_back(path);