From 3e1b623fb60b684217cd5fd4ea9e4574c9ed9abb Mon Sep 17 00:00:00 2001
From: Dydzio <blood990@gmail.com>
Date: Tue, 1 Jan 2019 15:41:52 +0100
Subject: [PATCH] Refactor goal removal from resource manager queue

---
 AI/VCAI/AIhelper.cpp        |  5 +++++
 AI/VCAI/AIhelper.h          |  1 +
 AI/VCAI/ResourceManager.cpp | 43 +++++++++++++++++++++++--------------
 AI/VCAI/ResourceManager.h   |  3 ++-
 AI/VCAI/VCAI.cpp            | 17 ++-------------
 5 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/AI/VCAI/AIhelper.cpp b/AI/VCAI/AIhelper.cpp
index bf9149bf1..87e5ae5f1 100644
--- a/AI/VCAI/AIhelper.cpp
+++ b/AI/VCAI/AIhelper.cpp
@@ -88,6 +88,11 @@ bool AIhelper::hasTasksLeft() const
 	return resourceManager->hasTasksLeft();
 }
 
+bool AIhelper::removeOutdatedObjectives(std::function<bool(const Goals::TSubgoal&)> predicate)
+{
+	return resourceManager->removeOutdatedObjectives(predicate);
+}
+
 bool AIhelper::canAfford(const TResources & cost) const
 {
 	return resourceManager->canAfford(cost);
diff --git a/AI/VCAI/AIhelper.h b/AI/VCAI/AIhelper.h
index 28bf82dad..688081055 100644
--- a/AI/VCAI/AIhelper.h
+++ b/AI/VCAI/AIhelper.h
@@ -47,6 +47,7 @@ public:
 	Goals::TSubgoal whatToDo() const override;
 	bool containsObjective(Goals::TSubgoal goal) const;
 	bool hasTasksLeft() const override;
+	bool removeOutdatedObjectives(std::function<bool(const Goals::TSubgoal &)> predicate) override;
 
 	bool getBuildingOptions(const CGTownInstance * t) override;
 	BuildingID getMaxPossibleGoldBuilding(const CGTownInstance * t);
diff --git a/AI/VCAI/ResourceManager.cpp b/AI/VCAI/ResourceManager.cpp
index 918956ef3..e466805da 100644
--- a/AI/VCAI/ResourceManager.cpp
+++ b/AI/VCAI/ResourceManager.cpp
@@ -232,22 +232,12 @@ bool ResourceManager::notifyGoalCompleted(Goals::TSubgoal goal)
 	if (goal->invalid())
 		logAi->warn("Attempt to complete Invalid goal");
 
-	bool removedGoal = false;
-	while (true)
-	{ //unfortunatelly we can't use remove_if on heap
-		auto it = boost::find_if(queue, [goal](const ResourceObjective & ro) -> bool
-		{
-			return ro.goal == goal || ro.goal->fulfillsMe (goal);
-		});
-		if(it != queue.end()) //removed at least one
-		{
-			logAi->debug("Removing goal %s from ResourceManager.", it->goal->name());
-			queue.erase(queue.s_handle_from_iterator(it));
-			removedGoal = true;
-		}
-		else //found nothing more to remove
-			break;
-	}
+	std::function<bool(const Goals::TSubgoal &)> equivalentGoalsCheck = [goal](const Goals::TSubgoal & x) -> bool
+	{
+		return x == goal || x->fulfillsMe(goal);
+	};
+
+	bool removedGoal = removeOutdatedObjectives(equivalentGoalsCheck);
 
 	dumpToLog();
 
@@ -315,6 +305,27 @@ bool ResourceManager::hasTasksLeft() const
 	return !queue.empty();
 }
 
+bool ResourceManager::removeOutdatedObjectives(std::function<bool(const Goals::TSubgoal &)> predicate)
+{
+	bool removedAnything = false;
+	while(true)
+	{ //unfortunately we can't use remove_if on heap
+		auto it = boost::find_if(queue, [&](const ResourceObjective & ro) -> bool
+		{
+			predicate(ro.goal);
+		});
+		if(it != queue.end()) //removed at least one
+		{
+			logAi->debug("Removing goal %s from ResourceManager.", it->goal->name());
+			queue.erase(queue.s_handle_from_iterator(it));
+			removedAnything = true;
+		}
+		else //found nothing more to remove
+			break;
+	}
+	return removedAnything;
+}
+
 TResources ResourceManager::reservedResources() const
 {
 	TResources res;
diff --git a/AI/VCAI/ResourceManager.h b/AI/VCAI/ResourceManager.h
index 5ec08433c..61f1c98e5 100644
--- a/AI/VCAI/ResourceManager.h
+++ b/AI/VCAI/ResourceManager.h
@@ -52,7 +52,7 @@ public:
 	virtual Goals::TSubgoal whatToDo(TResources &res, Goals::TSubgoal goal) = 0;
 	virtual bool containsObjective(Goals::TSubgoal goal) const = 0;
 	virtual bool hasTasksLeft() const = 0;
-private:
+	virtual bool removeOutdatedObjectives(std::function<bool(const Goals::TSubgoal &)> predicate) = 0; //remove ResourceObjectives from queue if ResourceObjective->goal meets specific criteria
 	virtual bool notifyGoalCompleted(Goals::TSubgoal goal) = 0;
 };
 
@@ -82,6 +82,7 @@ public:
 	Goals::TSubgoal whatToDo(TResources & res, Goals::TSubgoal goal); //can we afford this goal or need to CollectRes?
 	bool containsObjective(Goals::TSubgoal goal) const;
 	bool hasTasksLeft() const override;
+	bool removeOutdatedObjectives(std::function<bool(const Goals::TSubgoal &)> predicate) override;
 
 protected: //not-const actions only for AI
 	virtual void reserveResoures(const TResources & res, Goals::TSubgoal goal = Goals::TSubgoal());
diff --git a/AI/VCAI/VCAI.cpp b/AI/VCAI/VCAI.cpp
index 4ab21e5e1..29e0fada0 100644
--- a/AI/VCAI/VCAI.cpp
+++ b/AI/VCAI/VCAI.cpp
@@ -446,21 +446,8 @@ void VCAI::objectRemoved(const CGObjectInstance * obj)
 	for(auto goal : ultimateGoalsFromBasic)
 		vstd::erase_if(goal.second, checkRemovalValidity);
 
-	//clear resource manager goal cache - logic copied from ResourceManager::notifyGoalCompleted
-	while(true)
-	{ //unfortunately we can't use remove_if on heap
-		auto & queue = ah->resourceManager->queue;
-		auto iteratorToRemove = boost::find_if(queue, [&](const ResourceObjective & x) -> bool
-		{
-			return checkRemovalValidity(x.goal);
-		});
-		if(iteratorToRemove != queue.end()) //removed at least one
-		{
-			queue.erase(queue.s_handle_from_iterator(iteratorToRemove));
-		}
-		else //found nothing more to remove
-			break;
-	}
+	//clear resource manager goal cache
+	ah->removeOutdatedObjectives(checkRemovalValidity);
 
 	//TODO: Find better way to handle hero boat removal
 	if(auto hero = dynamic_cast<const CGHeroInstance *>(obj))