wb: fix rare OOS caused by recall action

previously having a planned recall action could change the order of
units in the recall list, which might for example change which unit is
recalled by a [recall].

src/recall_list_manager.cpp

@@ -60,9 +60,14 @@ void recall_list_manager::erase_if_matches_id(const std::string &unit_id)
 	                       recall_list_.end());
 }
 
-void recall_list_manager::add (const unit_ptr & ptr)
+void recall_list_manager::add(const unit_ptr & ptr, int pos)
 {
-	recall_list_.push_back(ptr);
+	if (pos < 0 || pos >= static_cast<int>(recall_list_.size())) {
+		recall_list_.push_back(ptr);
+	}
+	else {
+		recall_list_.insert(recall_list_.begin() + pos, ptr);
+	}
 }
 
 size_t recall_list_manager::find_index(const std::string & unit_id) const
@@ -73,12 +78,15 @@ size_t recall_list_manager::find_index(const std::string & unit_id) const
 	return std::distance(recall_list_.begin(), it);
 }
 
-unit_ptr recall_list_manager::extract_if_matches_id(const std::string &unit_id)
+unit_ptr recall_list_manager::extract_if_matches_id(const std::string &unit_id, int * pos)
 {
 	std::vector<unit_ptr >::iterator it = std::find_if(recall_list_.begin(), recall_list_.end(),
 		[&unit_id](const unit_ptr & ptr) { return ptr->id() == unit_id; });
 	if (it != recall_list_.end()) {
 		unit_ptr ret = *it;
+		if(pos) {
+			*pos = it - recall_list_.begin();
+		}
 		recall_list_.erase(it);
 		return ret;
 	} else {

src/recall_list_manager.hpp

@@ -40,7 +40,9 @@ public:
 	unit_const_ptr operator[](size_t index) const { return recall_list_[index]; } //!< vector style dereference
 
 	unit_ptr find_if_matches_id(const std::string & unit_id); //!< Find a unit by id. Null pointer if not found.
-	unit_ptr extract_if_matches_id(const std::string & unit_id); //!< Find a unit by id, and extract from this object if found. Null if not found.
+	/// Find a unit by id, and extract from this object if found. Null if not found.
+	/// @a pos an output paramter, to know in which position the unit was.
+	unit_ptr extract_if_matches_id(const std::string & unit_id, int * pos = nullptr);
 	unit_const_ptr find_if_matches_id(const std::string & unit_id) const; //!< Const find by id.
 	void erase_if_matches_id(const std::string & unit_id); //!< Erase any unit with this id.
 
@@ -56,7 +58,9 @@ public:
 	size_t size() const { return recall_list_.size(); } //!< Get the number of units on the list.
 	bool empty() const { return recall_list_.empty(); } //!< Is it empty?
 
-	void add(const unit_ptr & ptr); //!< Add a unit to the list.
+	/// Add a unit to the list.
+	/// @a pos the location where to insert the unit, -1 for 'at end'
+	void add(const unit_ptr & ptr, int pos = -1);
 
 private:
 	std::vector<unit_ptr > recall_list_; //!< The underlying data struture. TODO: Should this be a map based on underlying id instead?

src/whiteboard/recall.cpp

@@ -64,6 +64,7 @@ recall::recall(size_t team_index, bool hidden, const unit& u, const map_location
 	, fake_unit_(u.clone())
 	, original_mp_(0)
 	, original_ap_(0)
+	, original_recall_pos_(0)
 {
 	this->init();
 }
@@ -75,6 +76,7 @@ recall::recall(const config& cfg, bool hidden)
 	, fake_unit_()
 	, original_mp_(0)
 	, original_ap_(0)
+	, original_recall_pos_(0)
 {
 	// Construct and validate temp_unit_
 	size_t underlying_id = cfg["temp_unit_"];
@@ -147,7 +149,7 @@ void recall::apply_temp_modifier(unit_map& unit_map)
 			<< "] at position " << temp_unit_->get_location() << ".\n";
 
 	//temporarily remove unit from recall list
-	unit_ptr it = resources::gameboard->teams().at(team_index()).recall_list().extract_if_matches_id(temp_unit_->id());
+	unit_ptr it = resources::gameboard->teams().at(team_index()).recall_list().extract_if_matches_id(temp_unit_->id(), &original_recall_pos_);
 	assert(it);
 
 	//Usually (temp_unit_ == it) is true here, but wml might have changed the original unit in which case not doing 'temp_unit_ = it' would result in a gamestate change.
@@ -185,8 +187,7 @@ void recall::remove_temp_modifier(unit_map& unit_map)
 	original_mp_ = 0;
 	original_ap_ = 0;
 	//Put unit back into recall list
-	//fixme: check whether this can casue OOS (it probably changes the order of unit in the recall list).
-	resources::gameboard->teams().at(team_index()).recall_list().add(temp_unit_);
+	resources::gameboard->teams().at(team_index()).recall_list().add(temp_unit_, original_recall_pos_);
 }
 
 void recall::draw_hex(const map_location& hex)

src/whiteboard/recall.hpp

@@ -89,6 +89,7 @@ private:
 	
 	int original_mp_;
 	int original_ap_;
+	int original_recall_pos_;
 };
 
 std::ostream& operator<<(std::ostream& s, recall_ptr recall);