From 5ece2cc0892f835c7589366f85627cf0de762b08 Mon Sep 17 00:00:00 2001 From: Celtic Minstrel Date: Mon, 19 Feb 2018 22:00:22 -0500 Subject: [PATCH] Don't assume unit is on the map when testing ability active This refactors ability tests to use a passed-in unit instead of searching for the unit on the map based on its location. While it does fix #2247, I am not confident that this won't also have some undesirable side-effects. --- src/actions/attack.cpp | 8 +-- src/gui/dialogs/attack_predictions.cpp | 2 +- src/reports.cpp | 2 +- src/units/abilities.cpp | 75 ++++++++++++++++++++------ src/units/attack_type.hpp | 9 +++- 5 files changed, 71 insertions(+), 25 deletions(-) diff --git a/src/actions/attack.cpp b/src/actions/attack.cpp index 5deba2114eb5..1a5ae8bddd9e 100644 --- a/src/actions/attack.cpp +++ b/src/actions/attack.cpp @@ -122,10 +122,10 @@ battle_context_unit_stats::battle_context_unit_stats(const unit& u, } // Get the weapon characteristics as appropriate. - weapon->set_specials_context(u_loc, opp_loc, attacking, opp_weapon); + weapon->set_specials_context(u, opp, u_loc, opp_loc, attacking, opp_weapon); if(opp_weapon) { - opp_weapon->set_specials_context(opp_loc, u_loc, !attacking, weapon); + opp_weapon->set_specials_context(u, opp, opp_loc, u_loc, !attacking, weapon); } slows = weapon->get_special_bool("slow"); @@ -278,10 +278,10 @@ battle_context_unit_stats::battle_context_unit_stats(const unit_type* u_type, } // Get the weapon characteristics as appropriate. - weapon->set_specials_context(map_location::null_location(), attacking); + weapon->set_specials_context(u_type, map_location::null_location(), attacking); if(opp_weapon) { - opp_weapon->set_specials_context(map_location::null_location(), !attacking); + opp_weapon->set_specials_context(u_type, map_location::null_location(), !attacking); } slows = weapon->get_special_bool("slow"); diff --git a/src/gui/dialogs/attack_predictions.cpp b/src/gui/dialogs/attack_predictions.cpp index 41ae9d56e36f..66156ba6c70a 100644 --- a/src/gui/dialogs/attack_predictions.cpp +++ b/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_.get_location(), defender.unit_.get_location(), attacker.stats_.is_attacker, defender.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); // Get damage modifiers. unit_ability_list dmg_specials = weapon->get_specials("damage"); diff --git a/src/reports.cpp b/src/reports.cpp index 2c6e32020515..f793cfc144c4 100644 --- a/src/reports.cpp +++ b/src/reports.cpp @@ -658,7 +658,7 @@ static int attack_info(reports::context & rc, const attack_type &at, config &res { std::ostringstream str, tooltip; - at.set_specials_context(displayed_unit_hex, u.side() == rc.screen().playing_side()); + at.set_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; diff --git a/src/units/abilities.cpp b/src/units/abilities.cpp index 886b4881b26a..19bcbdcc87cc 100644 --- a/src/units/abilities.cpp +++ b/src/units/abilities.cpp @@ -46,19 +46,19 @@ namespace { class temporary_facing { map_location::DIRECTION save_dir_; - unit_map::const_iterator u_; + const unit* u_; public: - temporary_facing(unit_map::const_iterator u, map_location::DIRECTION new_dir) - : save_dir_(u.valid() ? u->facing() : map_location::NDIRECTIONS) + temporary_facing(const unit* u, map_location::DIRECTION new_dir) + : save_dir_(u ? u->facing() : map_location::NDIRECTIONS) , u_(u) { - if (u_.valid()) { + if (u_) { u_->set_facing(new_dir); } } ~temporary_facing() { - if (u_.valid()) { + if (u_) { u_->set_facing(save_dir_); } } @@ -696,16 +696,22 @@ std::string attack_type::weapon_specials(bool only_active, bool is_backstab) con /** * Sets the context under which specials will be checked for being active. * This version is appropriate if both units in a combat are known. + * @param[in] self A reference to the unit with this weapon. + * @param[in] other A reference to the other unit in the combat. * @param[in] unit_loc The location of the unit with this weapon. * @param[in] other_loc The location of the other unit in the combat. * @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 map_location& unit_loc, +void attack_type::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 { + self_ = &self; + other_ = &other; self_loc_ = unit_loc; other_loc_ = other_loc; is_attacker_ = attacking; @@ -716,11 +722,33 @@ void attack_type::set_specials_context(const map_location& unit_loc, /** * Sets the context under which specials will be checked for being active. * This version is appropriate if there is no specific combat being considered. + * @param[in] self A reference to the unit with this weapon. * @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 map_location& loc, bool attacking) const +void attack_type::set_specials_context(const unit& self, const map_location& loc, bool attacking) const { + self_ = &self; + other_ = nullptr; + self_loc_ = loc; + other_loc_ = map_location::null_location(); + is_attacker_ = attacking; + other_attack_ = nullptr; + is_for_listing_ = false; +} + +/** + * Sets the context under which specials will be checked for being active. + * This version is appropriate for theoretical units of a particular type. + * @param[in] self_type A reference to the type of the unit with this weapon. + * @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 +{ + UNUSED(self_type); + self_ = nullptr; + other_ = nullptr; self_loc_ = loc; other_loc_ = map_location::null_location(); is_attacker_ = attacking; @@ -830,8 +858,8 @@ namespace { // Helpers for attack_type::special_active() * @param[in] filter The filter containing the child filter to use. * @param[in] child_tag The tag of the child filter to use. */ - static bool special_unit_matches(const unit_map::const_iterator & un_it, - const unit_map::const_iterator & u2, + static bool special_unit_matches(const unit* & u, + const unit* & u2, const map_location & loc, const_attack_ptr weapon, const config & filter, @@ -852,19 +880,19 @@ namespace { // Helpers for attack_type::special_active() return true; // If the primary unit doesn't exist, there's nothing to match - if (!un_it.valid()) { + if (!u) { return false; } unit_filter ufilt{vconfig(filter_child)}; // If the other unit doesn't exist, try matching without it - if (!u2.valid()) { - return ufilt.matches(*un_it, loc); + if (!u2) { + return ufilt.matches(*u, loc); } // Check for a unit match. - if (!ufilt.matches(*un_it, loc, *u2)) { + if (!ufilt.matches(*u, loc, *u2)) { return false; } @@ -922,16 +950,29 @@ bool attack_type::special_active(const config& special, AFFECTS whom, assert(display::get_singleton()); const unit_map& units = display::get_singleton()->get_units(); - unit_map::const_iterator self = units.find(self_loc_); - unit_map::const_iterator other = units.find(other_loc_); + const unit* self = self_; + const unit* other = other_; + + if(self == nullptr) { + 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_); + 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_)); // Translate our context into terms of "attacker" and "defender". - unit_map::const_iterator & att = is_attacker_ ? self : other; - unit_map::const_iterator & def = is_attacker_ ? other : self; + const unit* & att = is_attacker_ ? self : other; + const unit* & 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_; diff --git a/src/units/attack_type.hpp b/src/units/attack_type.hpp index 619ca6a00623..fe59be9c8462 100644 --- a/src/units/attack_type.hpp +++ b/src/units/attack_type.hpp @@ -28,6 +28,7 @@ #include "units/ptr.hpp" // for attack_ptr class unit_ability_list; +class unit_type; //the 'attack type' is the type of attack, how many times it strikes, //and how much damage it does. @@ -72,9 +73,11 @@ class attack_type : public std::enable_shared_from_this unit_ability_list get_specials(const std::string& special) const; std::vector> 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 map_location& unit_loc, const map_location& other_loc, + 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 map_location& loc, bool attacking = true) 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. @@ -106,6 +109,8 @@ class attack_type : public std::enable_shared_from_this // Used via set_specials_context() to control which specials are // considered active. 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;