wb: fix recall actions changing gamestate

The firstproblem was that:
apply_temp_modifier adds the temp_unit_ to the map, then
remove_temp_modifier adds the temp_unit_ from the map to the recall
list, which resulted in the original recall unit beeing replaced by the
temp_unit_ of the recall_action We fix that by making sure that
temp_unit_ is always the same as athe recall unti not just a copy.

The second problem was that remove_temp_modifier reset the unit mp/ap to
a value differnt form the original mp/ap which could casue OOS later
since the ap/mp might be changed form that unsynced context, we fix that
by resetting the mp/ap in remove_temp_modifier

src/units/unit.hpp

@@ -868,6 +868,18 @@ public:
 		return (attacks_left_ == 0 || incapacitated()) ? 0 : attacks_left_;
 	}
 
+	/**
+	 * Gets the remaining number of attacks this unit can perform this turn.
+	 *
+	 * @param base_value          If false, consider the `incapacitated` flag.
+	 *
+	 * @returns                   If @a base_value is true, the raw value is returned.
+	 */
+	int attacks_left(bool base_value) const
+	{
+		return base_value ? attacks_left_ : attacks_left();
+	}
+
 	/**
 	 * Sets the number of attacks this unit has left this turn.
 	 * @param left The number of attacks left

src/whiteboard/recall.cpp

@@ -57,11 +57,13 @@ std::ostream& recall::print(std::ostream &s) const
 	return s;
 }
 
-recall::recall(size_t team_index, bool hidden, const unit& u, const map_location& recall_hex):
-		action(team_index,hidden),
-		temp_unit_(u.clone()),
-		recall_hex_(recall_hex),
-		fake_unit_(u.clone())
+recall::recall(size_t team_index, bool hidden, const unit& u, const map_location& recall_hex)
+	: action(team_index,hidden)
+	, temp_unit_(u.clone())
+	, recall_hex_(recall_hex)
+	, fake_unit_(u.clone())
+	, original_mp_(0)
+	, original_ap_(0)
 {
 	this->init();
 }
@@ -71,14 +73,16 @@ recall::recall(const config& cfg, bool hidden)
 	, temp_unit_()
 	, recall_hex_(cfg.child("recall_hex_")["x"],cfg.child("recall_hex_")["y"], wml_loc())
 	, fake_unit_()
+	, original_mp_(0)
+	, original_ap_(0)
 {
 	// Construct and validate temp_unit_
 	size_t underlying_id = cfg["temp_unit_"];
-	for(const unit_const_ptr & recall_unit : resources::gameboard->teams().at(team_index()).recall_list())
+	for(const unit_ptr & recall_unit : resources::gameboard->teams().at(team_index()).recall_list())
 	{
 		if(recall_unit->underlying_id()==underlying_id)
 		{
-			temp_unit_ = recall_unit->clone(); //TODO: is it necessary to make a copy?
+			temp_unit_ = recall_unit;
 			break;
 		}
 	}
@@ -93,9 +97,6 @@ recall::recall(const config& cfg, bool hidden)
 
 void recall::init()
 {
-	temp_unit_->set_movement(0, true);
-	temp_unit_->set_attacks(0);
-
 	fake_unit_->set_location(recall_hex_);
 	fake_unit_->set_movement(0, true);
 	fake_unit_->set_attacks(0);
@@ -140,7 +141,7 @@ void recall::execute(bool& success, bool& complete)
 void recall::apply_temp_modifier(unit_map& unit_map)
 {
 	assert(valid());
-	temp_unit_->set_location(recall_hex_);
+
 
 	DBG_WB << "Inserting future recall " << temp_unit_->name() << " [" << temp_unit_->id()
 			<< "] at position " << temp_unit_->get_location() << ".\n";
@@ -149,6 +150,15 @@ void recall::apply_temp_modifier(unit_map& unit_map)
 	unit_ptr it = resources::gameboard->teams().at(team_index()).recall_list().extract_if_matches_id(temp_unit_->id());
 	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.
+	temp_unit_ = it;
+	original_mp_ = temp_unit_->movement_left(true);
+	original_ap_ = temp_unit_->attacks_left(true);
+
+	temp_unit_->set_movement(0, true);
+	temp_unit_->set_attacks(0);
+	temp_unit_->set_location(recall_hex_);
+
 	//Add cost to money spent on recruits.
 	int cost = resources::gameboard->teams().at(team_index()).recall_cost();
 	if (it->recall_cost() > -1) {
@@ -169,10 +179,13 @@ void recall::remove_temp_modifier(unit_map& unit_map)
 	temp_unit_ = unit_map.extract(recall_hex_);
 	assert(temp_unit_.get());
 
-	temp_unit_->set_movement(0, true);
-	temp_unit_->set_attacks(0);
+	temp_unit_->set_movement(original_mp_, true);
+	temp_unit_->set_attacks(original_ap_);
 
+	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_);
 }
 

src/whiteboard/recall.hpp

@@ -82,9 +82,13 @@ private:
 	virtual void do_hide();
 	virtual void do_show();
 
+	// This is the pointer to the real recall unit.
 	unit_ptr temp_unit_;
 	map_location recall_hex_;
 	fake_unit_ptr fake_unit_;
+	
+	int original_mp_;
+	int original_ap_;
 };
 
 std::ostream& operator<<(std::ostream& s, recall_ptr recall);