fix segfault in abilities.cpp

when the defender doesn't counterattack the old code called
attack_type::special_active on a nullptr sometimes

src/units/abilities.cpp

@@ -1112,7 +1112,7 @@ unit_ability_list attack_type::list_ability(const std::string& ability) const
 	if(other_) {
 		abil_other_list.append((*other_).get_abilities(ability, other_loc_));
 		for(unit_ability_list::iterator i = abil_other_list.begin(); i != abil_other_list.end();) {
-			if(!other_attack_->special_active(*i->ability_cfg, AFFECT_OTHER, ability, true, "filter_student")) {
+			if(!special_active_impl(other_attack_, shared_from_this(), *i->ability_cfg, AFFECT_OTHER, ability, true, "filter_student")) {
 				i = abil_other_list.erase(i);
 			} else {
 				++i;
@@ -1141,6 +1141,12 @@ bool attack_type::bool_ability(const std::string& ability) const
 }
 //end of emulate weapon special functions.
 
+bool attack_type::special_active(const config& special, AFFECTS whom, const std::string& tag_name,
+                                 bool include_backstab, const std::string& filter_self) const
+{
+	return special_active_impl(shared_from_this(), const_attack_ptr(), special, whom, tag_name, include_backstab, filter_self);
+}
+
 /**
  * Returns whether or not the given special is active for the specified unit,
  * based on the current context (see set_specials_context).
@@ -1151,9 +1157,12 @@ bool attack_type::bool_ability(const std::string& ability) const
  *                              (usually true since backstab is usually accounted
  *                              for elsewhere)
  */
-bool attack_type::special_active(const config& special, AFFECTS whom, const std::string& tag_name,
-                                 bool include_backstab, const std::string& filter_self) const
+bool attack_type::special_active_impl(const_attack_ptr self_attack, const_attack_ptr other_attack, const config& special, AFFECTS whom, const std::string& tag_name,
+                                 bool include_backstab, const std::string& filter_self)
 {
+	assert(self_attack || other_attack);
+	bool is_attacker = self_attack ? self_attack->is_attacker_ : !other_attack->is_attacker_;
+	bool is_for_listing = self_attack ? self_attack->is_for_listing_ : other_attack->is_for_listing_;
 	//log_scope("special_active");
 
 	// Backstab check
@@ -1163,86 +1172,88 @@ bool attack_type::special_active(const config& special, AFFECTS whom, const std:
 
 	// Does this affect the specified unit?
 	if ( whom == AFFECT_SELF ) {
-		if ( !special_affects_self(special, is_attacker_) )
+		if ( !special_affects_self(special, is_attacker) )
 			return false;
 	}
 	if ( whom == AFFECT_OTHER ) {
-		if ( !special_affects_opponent(special, is_attacker_) )
+		if ( !special_affects_opponent(special, is_attacker) )
 			return false;
 	}
 
 	// Is this active on attack/defense?
 	const std::string & active_on = special["active_on"];
 	if ( !active_on.empty() ) {
-		if ( is_attacker_  &&  active_on != "offense" )
+		if ( is_attacker  &&  active_on != "offense" )
 			return false;
-		if ( !is_attacker_  &&  active_on != "defense" )
+		if ( !is_attacker  &&  active_on != "defense" )
 			return false;
 	}
 
 	// Get the units involved.
 	assert(display::get_singleton());
 	const unit_map& units = display::get_singleton()->get_units();
 
-	unit_const_ptr self = self_;
-	unit_const_ptr other = other_;
-
+	unit_const_ptr self = self_attack ? self_attack->self_ : other_attack->other_;
+	unit_const_ptr other = self_attack ? self_attack->other_ : other_attack->self_;
+	map_location self_loc = self_attack ? self_attack->self_loc_ : other_attack->other_loc_;
+	map_location other_loc = self_attack ? self_attack->other_loc_ : other_attack->self_loc_;
+	//TODO: why is this needed?
 	if(self == nullptr) {
-		unit_map::const_iterator it = units.find(self_loc_);
+		unit_map::const_iterator it = units.find(self_loc);
 		if(it.valid()) {
 			self = it.get_shared_ptr().get();
 		}
 	}
 	if(other == nullptr) {
-		unit_map::const_iterator it = units.find(other_loc_);
+		unit_map::const_iterator it = units.find(other_loc);
 		if(it.valid()) {
 			other = it.get_shared_ptr().get();
 		}
 	}
 
 	// Make sure they're facing each other.
-	temporary_facing self_facing(self, self_loc_.get_relative_dir(other_loc_));
-	temporary_facing other_facing(other, other_loc_.get_relative_dir(self_loc_));
+	temporary_facing self_facing(self, self_loc.get_relative_dir(other_loc));
+	temporary_facing other_facing(other, other_loc.get_relative_dir(self_loc));
 
 	// Filter poison, plague, drain, first strike
 	if (tag_name == "drains" && other && other->get_state("undrainable")) {
 		return false;
 	}
 	if (tag_name == "plague" && other &&
 		(other->get_state("unplagueable") ||
-		 resources::gameboard->map().is_village(other_loc_))) {
+		 resources::gameboard->map().is_village(other_loc))) {
 		return false;
 	}
 	if (tag_name == "poison" && other &&
 		(other->get_state("unpoisonable") || other->get_state(unit::STATE_POISONED))) {
 		return false;
 	}
-	if (tag_name == "firststrike" && !is_attacker_ && other_attack_ &&
-		other_attack_->get_special_bool("firststrike", false)) {
+	if (tag_name == "firststrike" && !is_attacker && other_attack &&
+		other_attack->get_special_bool("firststrike", false)) {
 		return false;
 	}
 
 
 	// Translate our context into terms of "attacker" and "defender".
-	unit_const_ptr & att = is_attacker_ ? self : other;
-	unit_const_ptr & def = is_attacker_ ? other : self;
-	const map_location & att_loc   = is_attacker_ ? self_loc_ : other_loc_;
-	const map_location & def_loc   = is_attacker_ ? other_loc_ : self_loc_;
-	const_attack_ptr att_weapon = is_attacker_ ? shared_from_this() : other_attack_;
-	const_attack_ptr def_weapon = is_attacker_ ? other_attack_ : shared_from_this();
+	unit_const_ptr & att = is_attacker ? self : other;
+	unit_const_ptr & def = is_attacker ? other : self;
+	const map_location & att_loc   = is_attacker ? self_loc : other_loc;
+	const map_location & def_loc   = is_attacker ? other_loc : self_loc;
+	const_attack_ptr att_weapon = is_attacker ? self_attack : other_attack;
+	const_attack_ptr def_weapon = is_attacker ? other_attack : self_attack;
 
 	// Filter the units involved.
-	if (!special_unit_matches(self, other, self_loc_, shared_from_this(), special, is_for_listing_, filter_self))
+	if (!special_unit_matches(self, other, self_loc, self_attack, special, is_for_listing, filter_self))
 		return false;
-	if (!special_unit_matches(other, self, other_loc_, other_attack_, special, is_for_listing_, "filter_opponent"))
+	if (!special_unit_matches(other, self, other_loc, other_attack, special, is_for_listing, "filter_opponent"))
 		return false;
-	if (!special_unit_matches(att, def, att_loc, att_weapon, special, is_for_listing_, "filter_attacker"))
+	if (!special_unit_matches(att, def, att_loc, att_weapon, special, is_for_listing, "filter_attacker"))
 		return false;
-	if (!special_unit_matches(def, att, def_loc, def_weapon, special, is_for_listing_, "filter_defender"))
+	if (!special_unit_matches(def, att, def_loc, def_weapon, special, is_for_listing, "filter_defender"))
 		return false;
 
 	adjacent_loc_array_t adjacent;
-	get_adjacent_tiles(self_loc_, adjacent.data());
+	get_adjacent_tiles(self_loc, adjacent.data());
 
 	// Filter the adjacent units.
 	for (const config &i : special.child_range("filter_adjacent"))

src/units/attack_type.hpp

@@ -110,6 +110,16 @@ class attack_type : public std::enable_shared_from_this<attack_type>
 	bool special_active(const config& special, AFFECTS whom, const std::string& tag_name,
 	                    bool include_backstab=true, const std::string& filter_self ="filter_self") const;
 
+	static bool special_active_impl(
+		const_attack_ptr self_attack,
+		const_attack_ptr other_attack,
+		const config& special,
+		AFFECTS whom,
+		const std::string& tag_name,
+		bool include_backstab=true,
+		const std::string& filter_self ="filter_self"
+	);
+
 	// Used via specials_context() to control which specials are
 	// considered active.
 	friend class specials_context_t;