Use RAII for weapon specials context to ensure no dangling pointers

src/actions/attack.cpp

@@ -122,10 +122,11 @@ battle_context_unit_stats::battle_context_unit_stats(const unit& u,
 	}
 
 	// Get the weapon characteristics as appropriate.
-	weapon->set_specials_context(u, opp, u_loc, opp_loc, attacking, opp_weapon);
+	auto ctx = weapon->specials_context(u, opp, u_loc, opp_loc, attacking, opp_weapon);
+	boost::optional<decltype(ctx)> opp_ctx;
 
 	if(opp_weapon) {
-		opp_weapon->set_specials_context(u, opp, opp_loc, u_loc, !attacking, weapon);
+		opp_ctx.emplace(opp_weapon->specials_context(u, opp, opp_loc, u_loc, !attacking, weapon));
 	}
 
 	slows = weapon->get_special_bool("slow");
@@ -278,10 +279,11 @@ battle_context_unit_stats::battle_context_unit_stats(const unit_type* u_type,
 	}
 
 	// Get the weapon characteristics as appropriate.
-	weapon->set_specials_context(u_type, map_location::null_location(), attacking);
+	auto ctx = weapon->specials_context(*u_type, map_location::null_location(), attacking);
+	boost::optional<decltype(ctx)> opp_ctx;
 
 	if(opp_weapon) {
-		opp_weapon->set_specials_context(u_type, map_location::null_location(), !attacking);
+		opp_ctx.emplace(opp_weapon->specials_context(*u_type, map_location::null_location(), !attacking));
 	}
 
 	slows = weapon->get_special_bool("slow");

src/gui/dialogs/attack_predictions.cpp

@@ -127,7 +127,7 @@ void attack_predictions::set_data(window& window, const combatant_data& attacker
 
 	// Set specials context (for safety, it should not have changed normally).
 	const_attack_ptr weapon = attacker.stats_.weapon;
-	weapon->set_specials_context(attacker.unit_, defender.unit_, attacker.unit_.get_location(), defender.unit_.get_location(), attacker.stats_.is_attacker, defender.stats_.weapon);
+	auto ctx = weapon->specials_context(attacker.unit_, defender.unit_, attacker.unit_.get_location(), defender.unit_.get_location(), attacker.stats_.is_attacker, defender.stats_.weapon);
 
 	// Get damage modifiers.
 	unit_ability_list dmg_specials = weapon->get_specials("damage");

src/reports.cpp

@@ -657,8 +657,10 @@ static inline const color_t attack_info_percent_color(int resistance)
 static int attack_info(reports::context & rc, const attack_type &at, config &res, const unit &u, const map_location &displayed_unit_hex)
 {
 	std::ostringstream str, tooltip;
+	int damage = 0;
 
-	at.set_specials_context(u, displayed_unit_hex, u.side() == rc.screen().playing_side());
+	{
+	auto ctx = at.specials_context(u, displayed_unit_hex, u.side() == rc.screen().playing_side());
 	int base_damage = at.damage();
 	int specials_damage = at.modified_damage(false);
 	int damage_multiplier = 100;
@@ -671,7 +673,7 @@ static int attack_info(reports::context & rc, const attack_type &at, config &res
 	bool slowed = u.get_state(unit::STATE_SLOWED);
 	int damage_divisor = slowed ? 20000 : 10000;
 	// Assume no specific resistance (i.e. multiply by 100).
-	int damage = round_damage(specials_damage, damage_multiplier * 100, damage_divisor);
+	damage = round_damage(specials_damage, damage_multiplier * 100, damage_divisor);
 
 	// Hit points are used to calculate swarm, so they need to be bounded.
 	unsigned max_hp = u.max_hitpoints();
@@ -810,8 +812,10 @@ static int attack_info(reports::context & rc, const attack_type &at, config &res
 			}
 		add_text(res, flush(str), flush(tooltip));
 	}
+	}
 
-	at.set_specials_context_for_listing();
+	{
+	auto ctx = at.specials_context_for_listing();
 	boost::dynamic_bitset<> active;
 	const std::vector<std::pair<t_string, t_string>> &specials = at.special_tooltips(&active);
 	const size_t specials_size = specials.size();
@@ -832,6 +836,7 @@ static int attack_info(reports::context & rc, const attack_type &at, config &res
 
 		add_text(res, flush(str), flush(tooltip), help_page);
 	}
+	}
 	return damage;
 }
 

src/units/abilities.cpp

@@ -702,20 +702,22 @@ std::string attack_type::weapon_specials(bool only_active, bool is_backstab) con
  * @param[in]  attacking     Whether or not the unit with this weapon is the attacker.
  * @param[in]  other_attack  The attack used by the other unit.
  */
-void attack_type::set_specials_context(const unit& self,
+attack_type::specials_context_t::specials_context_t(const attack_type& weapon,
+                                       const_attack_ptr other_attack,
+									   const unit& self,
                                        const unit& other,
                                        const map_location& unit_loc,
                                        const map_location& other_loc,
-                                       bool attacking,
-                                       const_attack_ptr other_attack) const
+                                       bool attacking)
+	: parent(weapon)
 {
-	self_ = &self;
-	other_ = &other;
-	self_loc_ = unit_loc;
-	other_loc_ = other_loc;
-	is_attacker_ = attacking;
-	other_attack_ = other_attack;
-	is_for_listing_ = false;
+	weapon.self_ = &self;
+	weapon.other_ = &other;
+	weapon.self_loc_ = unit_loc;
+	weapon.other_loc_ = other_loc;
+	weapon.is_attacker_ = attacking;
+	weapon.other_attack_ = other_attack;
+	weapon.is_for_listing_ = false;
 }
 
 /**
@@ -725,15 +727,16 @@ void attack_type::set_specials_context(const unit& self,
  * @param[in]  loc           The location of the unit with this weapon.
  * @param[in]  attacking     Whether or not the unit with this weapon is the attacker.
  */
-void attack_type::set_specials_context(const unit& self, const map_location& loc, bool attacking) const
+attack_type::specials_context_t::specials_context_t(const attack_type& weapon, const unit& self, const map_location& loc, bool attacking)
+	: parent(weapon)
 {
-	self_ = &self;
-	other_ = nullptr;
-	self_loc_ = loc;
-	other_loc_ = map_location::null_location();
-	is_attacker_ = attacking;
-	other_attack_ = nullptr;
-	is_for_listing_ = false;
+	weapon.self_ = &self;
+	weapon.other_ = nullptr;
+	weapon.self_loc_ = loc;
+	weapon.other_loc_ = map_location::null_location();
+	weapon.is_attacker_ = attacking;
+	weapon.other_attack_ = nullptr;
+	weapon.is_for_listing_ = false;
 }
 
 /**
@@ -743,23 +746,35 @@ void attack_type::set_specials_context(const unit& self, const map_location& loc
  * @param[in]  loc           The location of the unit with this weapon.
  * @param[in]  attacking     Whether or not the unit with this weapon is the attacker.
  */
-void attack_type::set_specials_context(const unit_type* self_type, const map_location& loc, bool attacking) const
+attack_type::specials_context_t::specials_context_t(const attack_type& weapon, const unit_type& self_type, const map_location& loc, bool attacking)
+	: parent(weapon)
 {
 	UNUSED(self_type);
-	self_ = nullptr;
-	other_ = nullptr;
-	self_loc_ = loc;
-	other_loc_ = map_location::null_location();
-	is_attacker_ = attacking;
-	other_attack_ = nullptr;
-	is_for_listing_ = false;
+	weapon.self_ = nullptr;
+	weapon.other_ = nullptr;
+	weapon.self_loc_ = loc;
+	weapon.other_loc_ = map_location::null_location();
+	weapon.is_attacker_ = attacking;
+	weapon.other_attack_ = nullptr;
+	weapon.is_for_listing_ = false;
 }
 
-void attack_type::set_specials_context_for_listing() const
+attack_type::specials_context_t::specials_context_t(const attack_type& weapon)
+	: parent(weapon)
 {
-	is_for_listing_ = true;
+	weapon.is_for_listing_ = true;
 }
 
+attack_type::specials_context_t::~specials_context_t()
+{
+	parent.self_ = nullptr;
+	parent.other_ = nullptr;
+	parent.self_loc_ = map_location::null_location();
+	parent.other_loc_ = map_location::null_location();
+	parent.is_attacker_ = false;
+	parent.other_attack_ = nullptr;
+	parent.is_for_listing_ = false;
+}
 
 /**
  * Calculates the number of attacks this weapon has, considering specials.

src/units/attack_type.hpp

@@ -73,12 +73,6 @@ class attack_type : public std::enable_shared_from_this<attack_type>
 	unit_ability_list get_specials(const std::string& special) const;
 	std::vector<std::pair<t_string, t_string>> special_tooltips(boost::dynamic_bitset<>* active_list = nullptr) const;
 	std::string weapon_specials(bool only_active=false, bool is_backstab=false) const;
-	void set_specials_context(const unit& self, const unit& other,
-	                          const map_location& unit_loc, const map_location& other_loc,
-	                          bool attacking, const_attack_ptr other_attack) const;
-	void set_specials_context(const unit& self, const map_location& loc, bool attacking = true) const;
-	void set_specials_context(const unit_type* self_type, const map_location& loc, bool attacking = true) const;
-	void set_specials_context_for_listing() const;
 
 	/// Calculates the number of attacks this weapon has, considering specials.
 	void modified_attacks(bool is_backstab, unsigned & min_attacks,
@@ -106,14 +100,48 @@ class attack_type : public std::enable_shared_from_this<attack_type>
 	bool special_active(const config& special, AFFECTS whom,
 	                    bool include_backstab=true) const;
 
-	// Used via set_specials_context() to control which specials are
+	// Used via specials_context() to control which specials are
 	// considered active.
+	struct specials_context_t {
+		const attack_type& parent;
+		/// Initialize weapon specials context for listing
+		specials_context_t(const attack_type& weapon);
+		/// Initialize weapon specials context for a unit type
+		specials_context_t(const attack_type& weapon, const unit_type& self_type, const map_location& loc, bool attacking = true);
+		/// Initialize weapon specials context for a single unit
+		specials_context_t(const attack_type& weapon, const_attack_ptr other_weapon,
+			const unit& self, const unit& other,
+			const map_location& self_loc, const map_location& other_loc,
+			bool attacking);
+		/// Initialize weapon specials context for a pair of units
+		specials_context_t(const attack_type& weapon, const unit& self, const map_location& loc, bool attacking);
+		~specials_context_t();
+	};
+	friend struct specials_context_t;
 	mutable map_location self_loc_, other_loc_;
 	mutable const unit* self_;
 	mutable const unit* other_;
 	mutable bool is_attacker_;
 	mutable const_attack_ptr other_attack_;
 	mutable bool is_for_listing_ = false;
+public:
+	// Set up a specials context.
+	// Usage: auto ctx = weapon.specials_context(...);
+	specials_context_t specials_context(const unit& self, const unit& other,
+		const map_location& unit_loc, const map_location& other_loc,
+		bool attacking, const_attack_ptr other_attack) const {
+		return specials_context_t(*this, other_attack, self, other, unit_loc, other_loc, attacking);
+	}
+	specials_context_t specials_context(const unit& self, const map_location& loc, bool attacking = true) const {
+		return specials_context_t(*this, self, loc, attacking);
+	}
+	specials_context_t specials_context(const unit_type& self_type, const map_location& loc, bool attacking = true) const {
+		return specials_context_t(*this, self_type, loc, attacking);
+	}
+	specials_context_t specials_context_for_listing() const {
+		return specials_context_t(*this);
+	}
+private:
 
 	t_string description_;
 	std::string id_;