fix tag_name check deficiencies
The tag_name check is deficient for two reasons:
1.when apply_to=both is used, in the case of [damage_type] [filter_opponent][filter_weapon]type= will check the modified type for application to the owner and the original type for application to the opponent, hence a risk of infinite recursion, especially if [filter_self][filter_weapon]type= is used in the same weapon.
2. in the case of apply_to=attacker/defender, the check for [filter_attacker/defender] must take into account both the value of the variable is_attacker but also the application AFFECT_SELF/OTHER, otherwise in the case apply_to=defender with [filter_defender][filter_weapon]type=, and[filter_attacker][filter_weapon]type= otherwise we end up in the same configuration as for apply_to=both
Finally tag_name is renamed check_if_recursion to mean that this variable is used to ensure that a recursion cannot be set up even if 743b146efc prevents it from causing a crash but with an error log
This commit is contained in:
newfrenchy83
Pentarctagon
parent
d6f922f990
commit
979289b579
6 changed files with 27 additions and 23 deletions
|
|
@ -10,7 +10,6 @@
|
|||
# Have Alice attack with his weapon.
|
||||
##
|
||||
# Expected end state:
|
||||
# BROKE_STRICT - error logged due to recursive ability.
|
||||
# Damage_type is active
|
||||
# Bob takes 15 damage.
|
||||
#####
|
||||
|
|
|
|||
|
|
@ -10,7 +10,6 @@
|
|||
# Have Alice attack with his weapon.
|
||||
##
|
||||
# Expected end state:
|
||||
# BROKE_STRICT - error logged due to recursive ability.
|
||||
# Damage_type is active
|
||||
# Bob takes 15 damage.
|
||||
#####
|
||||
|
|
|
|||
|
|
@ -1346,7 +1346,7 @@ namespace { // Helpers for attack_type::special_active()
|
|||
* @param[in] filter The filter containing the child filter to use.
|
||||
* @param[in] for_listing
|
||||
* @param[in] child_tag The tag of the child filter to use.
|
||||
* @param[in] tag_name Parameter used for don't have infinite recusion for some filter attribute.
|
||||
* @param[in] check_if_recursion Parameter used for don't have infinite recusion for some filter attribute.
|
||||
*/
|
||||
static bool special_unit_matches(unit_const_ptr & u,
|
||||
unit_const_ptr & u2,
|
||||
|
|
@ -1354,7 +1354,7 @@ namespace { // Helpers for attack_type::special_active()
|
|||
const_attack_ptr weapon,
|
||||
const config & filter,
|
||||
const bool for_listing,
|
||||
const std::string & child_tag, const std::string& tag_name)
|
||||
const std::string & child_tag, const std::string& check_if_recursion)
|
||||
{
|
||||
if (for_listing && !loc.valid())
|
||||
// The special's context was set to ignore this unit, so assume we pass.
|
||||
|
|
@ -1381,7 +1381,7 @@ namespace { // Helpers for attack_type::special_active()
|
|||
|
||||
// Check for a weapon match.
|
||||
if (auto filter_weapon = filter_child->optional_child("filter_weapon") ) {
|
||||
if ( !weapon || !weapon->matches_filter(*filter_weapon, tag_name) )
|
||||
if ( !weapon || !weapon->matches_filter(*filter_weapon, check_if_recursion) )
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
@ -2020,17 +2020,23 @@ bool attack_type::special_active_impl(
|
|||
//If filter concerns the unit on which special is applied,
|
||||
//then the type of special must be entered to avoid calling
|
||||
//the function of this special in matches_filter()
|
||||
std::string self_tag_name = whom_is_self ? tag_name : "";
|
||||
if (!special_unit_matches(self, other, self_loc, self_attack, special, is_for_listing, filter_self, self_tag_name))
|
||||
//In apply_to=both case, tag_name must be checked in all filter because special applied to both self and opponent.
|
||||
bool applied_both = special["apply_to"] == "both";
|
||||
std::string self_check_if_recursion = (applied_both || whom_is_self) ? tag_name : "";
|
||||
if (!special_unit_matches(self, other, self_loc, self_attack, special, is_for_listing, filter_self, self_check_if_recursion))
|
||||
return false;
|
||||
std::string opp_tag_name = !whom_is_self ? tag_name : "";
|
||||
if (!special_unit_matches(other, self, other_loc, other_attack, special_backstab, is_for_listing, "filter_opponent", opp_tag_name))
|
||||
std::string opp_check_if_recursion = (applied_both || !whom_is_self) ? tag_name : "";
|
||||
if (!special_unit_matches(other, self, other_loc, other_attack, special_backstab, is_for_listing, "filter_opponent", opp_check_if_recursion))
|
||||
return false;
|
||||
std::string att_tag_name = is_attacker ? tag_name : "";
|
||||
if (!special_unit_matches(att, def, att_loc, att_weapon, special, is_for_listing, "filter_attacker", att_tag_name))
|
||||
//in case of apply_to=attacker|defender, if both [filter_attacker] and [filter_defender] are used,
|
||||
//check what is_attacker is true(or false for (filter_defender]) in affect self case only is necessary for what unit affected by special has a tag_name check.
|
||||
bool applied_to_attacker = applied_both || (whom_is_self && is_attacker) || (!whom_is_self && !is_attacker);
|
||||
std::string att_check_if_recursion = applied_to_attacker ? tag_name : "";
|
||||
if (!special_unit_matches(att, def, att_loc, att_weapon, special, is_for_listing, "filter_attacker", att_check_if_recursion))
|
||||
return false;
|
||||
std::string def_tag_name = !is_attacker ? tag_name : "";
|
||||
if (!special_unit_matches(def, att, def_loc, def_weapon, special, is_for_listing, "filter_defender", def_tag_name))
|
||||
bool applied_to_defender = applied_both || (whom_is_self && !is_attacker) || (!whom_is_self && is_attacker);
|
||||
std::string def_check_if_recursion= applied_to_defender ? tag_name : "";
|
||||
if (!special_unit_matches(def, att, def_loc, def_weapon, special, is_for_listing, "filter_defender", def_check_if_recursion))
|
||||
return false;
|
||||
|
||||
const auto adjacent = get_adjacent_tiles(self_loc);
|
||||
|
|
|
|||
|
|
@ -152,7 +152,7 @@ void show_recursion_warning(const attack_type& attack, const config& filter) {
|
|||
* Returns whether or not *this matches the given @a filter, ignoring the
|
||||
* complexities introduced by [and], [or], and [not].
|
||||
*/
|
||||
bool matches_simple_filter(const attack_type& attack, const config& filter, const std::string& tag_name)
|
||||
bool matches_simple_filter(const attack_type& attack, const config& filter, const std::string& check_if_recursion)
|
||||
{
|
||||
//update and check variable_recursion for prevent check special_id/type_active in case of infinite recursion.
|
||||
attack_type::recursion_guard filter_lock= attack.update_variables_recursion();
|
||||
|
|
@ -215,7 +215,7 @@ bool matches_simple_filter(const attack_type& attack, const config& filter, cons
|
|||
// should always use the base type of the weapon. Otherwise it will flip-flop between the
|
||||
// special being active or inactive based on whether ATTACK_RECURSION_LIMIT is even or odd;
|
||||
// without this it will also behave differently when calculating resistance_against.
|
||||
if(tag_name == "damage_type"){
|
||||
if(check_if_recursion == "damage_type"){
|
||||
if (filter_type.count(attack.type()) == 0){
|
||||
return false;
|
||||
}
|
||||
|
|
@ -327,25 +327,25 @@ bool matches_simple_filter(const attack_type& attack, const config& filter, cons
|
|||
/**
|
||||
* Returns whether or not *this matches the given @a filter.
|
||||
*/
|
||||
bool attack_type::matches_filter(const config& filter, const std::string& tag_name) const
|
||||
bool attack_type::matches_filter(const config& filter, const std::string& check_if_recursion) const
|
||||
{
|
||||
// Handle the basic filter.
|
||||
bool matches = matches_simple_filter(*this, filter, tag_name);
|
||||
bool matches = matches_simple_filter(*this, filter, check_if_recursion);
|
||||
|
||||
// Handle [and], [or], and [not] with in-order precedence
|
||||
for (const config::any_child condition : filter.all_children_range() )
|
||||
{
|
||||
// Handle [and]
|
||||
if ( condition.key == "and" )
|
||||
matches = matches && matches_filter(condition.cfg, tag_name);
|
||||
matches = matches && matches_filter(condition.cfg, check_if_recursion);
|
||||
|
||||
// Handle [or]
|
||||
else if ( condition.key == "or" )
|
||||
matches = matches || matches_filter(condition.cfg, tag_name);
|
||||
matches = matches || matches_filter(condition.cfg, check_if_recursion);
|
||||
|
||||
// Handle [not]
|
||||
else if ( condition.key == "not" )
|
||||
matches = matches && !matches_filter(condition.cfg, tag_name);
|
||||
matches = matches && !matches_filter(condition.cfg, check_if_recursion);
|
||||
}
|
||||
|
||||
return matches;
|
||||
|
|
|
|||
|
|
@ -139,7 +139,7 @@ public:
|
|||
|
||||
// In unit_types.cpp:
|
||||
|
||||
bool matches_filter(const config& filter, const std::string& tag_name = "") const;
|
||||
bool matches_filter(const config& filter, const std::string& check_if_recursion = "") const;
|
||||
bool apply_modification(const config& cfg);
|
||||
bool describe_modification(const config& cfg,std::string* description);
|
||||
|
||||
|
|
|
|||
|
|
@ -377,8 +377,8 @@
|
|||
0 damage_type_test
|
||||
0 damage_type_with_filter_test
|
||||
0 damage_secondary_type_test
|
||||
9 damage_type_apply_to_both_filter_self_opponent
|
||||
9 damage_type_apply_to_attacker_filter_attacker_defender
|
||||
0 damage_type_apply_to_both_filter_self_opponent
|
||||
0 damage_type_apply_to_attacker_filter_attacker_defender
|
||||
9 event_test_filter_damage_type_recursion
|
||||
9 four_cycle_recursion_branching
|
||||
9 four_cycle_recursion_by_id
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue