Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix [firststrike] special with apply_to=opponent crashes Wesnoth 1.17.x #6574

Merged
114 changes: 114 additions & 0 deletions data/test/scenarios/poison_opponent.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# this unit test what 'poison' applied to opponent work perfectly after fixing.
{GENERIC_UNIT_TEST "poison_opponent" (
[event]
name=start
[modify_unit]
[filter]
[/filter]
max_hitpoints=100
hitpoints=100
attacks_left=1
[/modify_unit]
[object]
silent=yes
[effect]
apply_to=attack
[set_specials]
mode=append
[poison]
apply_to=opponent
[/poison]
[attacks]
value=1
[/attacks]
[damage]
value=1
[/damage]
[chance_to_hit]
value=100
[/chance_to_hit]
[/set_specials]
[/effect]
[filter]
id=bob
[/filter]
[/object]
[object]
silent=yes
[effect]
apply_to=attack
[set_specials]
mode=append
[poison]
apply_to=opponent
[/poison]
[attacks]
value=1
[/attacks]
[damage]
value=1
[/damage]
[chance_to_hit]
value=100
[/chance_to_hit]
[/set_specials]
[/effect]
[filter]
id=alice
[/filter]
[/object]

[store_unit]
[filter]
id=alice
[/filter]
variable=a
kill=yes
[/store_unit]
[store_unit]
[filter]
id=bob
[/filter]
variable=b
[/store_unit]
[unstore_unit]
variable=a
find_vacant=yes
x,y=$b.x,$b.y
[/unstore_unit]
[store_unit]
[filter]
id=alice
[/filter]
variable=a
[/store_unit]

[do_command]
[attack]
weapon=0
defender_weapon=0
[source]
x,y=$a.x,$a.y
[/source]
[destination]
x,y=$b.x,$b.y
[/destination]
[/attack]
[/do_command]
[store_unit]
[filter]
id=alice
[/filter]
variable=a
[/store_unit]
[store_unit]
[filter]
id=bob
[/filter]
variable=b
[/store_unit]
{ASSERT ({VARIABLE_CONDITIONAL a.status.poisoned boolean_equals yes})}
{ASSERT ({VARIABLE_CONDITIONAL b.status.poisoned boolean_equals yes})}
{SUCCEED}
[/event]
)}
39 changes: 24 additions & 15 deletions src/units/abilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1537,29 +1537,30 @@ bool attack_type::special_active_impl(const_attack_ptr self_attack, const_attack
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))) {
// Filter poison, plague, drain, slow, petrifies
// True if "whom" corresponds to "self", false if "whom" is "other"
bool whom_is_self = ((whom == AFFECT_SELF) || ((whom == AFFECT_EITHER) && special_affects_self(special, is_attacker))) ? true : false;
stevecotton marked this conversation as resolved.
Show resolved Hide resolved
unit_const_ptr whom_is_unit = whom_is_self ? other : self;
map_location whom_is_loc = whom_is_self ? other_loc : self_loc;
stevecotton marked this conversation as resolved.
Show resolved Hide resolved

if (tag_name == "drains" && whom_is_unit && whom_is_unit->get_state("undrainable")) {
return false;
}
if (tag_name == "poison" && other &&
(other->get_state("unpoisonable") || other->get_state(unit::STATE_POISONED))) {
if (tag_name == "plague" && whom_is_unit &&
(whom_is_unit->get_state("unplagueable") ||
resources::gameboard->map().is_village(whom_is_loc))) {
return false;
}
if (tag_name == "firststrike" && !is_attacker && other_attack &&
other_attack->has_special_or_ability("firststrike")) {
if (tag_name == "poison" && whom_is_unit &&
(whom_is_unit->get_state("unpoisonable") || whom_is_unit->get_state(unit::STATE_POISONED))) {
return false;
}
if (tag_name == "slow" && other &&
(other->get_state("unslowable") || other->get_state(unit::STATE_SLOWED))) {
if (tag_name == "slow" && whom_is_unit &&
(whom_is_unit->get_state("unslowable") || whom_is_unit->get_state(unit::STATE_SLOWED))) {
return false;
}
if (tag_name == "petrifies" && other &&
other->get_state("unpetrifiable")) {
if (tag_name == "petrifies" && whom_is_unit &&
whom_is_unit->get_state("unpetrifiable")) {
return false;
}

Expand All @@ -1572,6 +1573,14 @@ bool attack_type::special_active_impl(const_attack_ptr self_attack, const_attack
const_attack_ptr att_weapon = is_attacker ? self_attack : other_attack;
const_attack_ptr def_weapon = is_attacker ? other_attack : self_attack;

// Filter firststrike here, if both units have first strike then the effects cancel out. Only check
// the opponent if "whom" is the defender, otherwise this leads to infinite recursion.
if (tag_name == "firststrike") {
bool whom_is_defender = whom_is_self ? !is_attacker : is_attacker;
if (whom_is_defender && att_weapon && att_weapon->has_special_or_ability("firststrike"))
return false;
}

// Filter the units involved.
if (!special_unit_matches(self, other, self_loc, self_attack, special, is_for_listing, filter_self))
return false;
Expand Down
1 change: 1 addition & 0 deletions wml_test_schedule
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@
0 swarm_disables_upgrades_with_abilities_adjacent_fail
0 swarm_disables_upgrades_with_abilities_adjacent_leadership
0 swarm_disables_upgrades_with_abilities_adjacent_leadership_fail
0 poison_opponent
0 test_add_in_leadership_abilities
0 test_sub_in_leadership_abilities
0 unslowable_status_test
Expand Down