fix [firststrike] special with apply_to=opponent crashes Wesnoth 1.17.x #6574
Conversation
github-actions
bot
added
the
Units
newfrenchy83
force-pushed
the
fix_special_with_apply_to_opponent
branch
from
77222e6
to
93ddc19
Compare
|
@Pentarctagon why the checking don't work? |
github-actions
bot
added
the
Unit Tests
|
@Pentarctagon test added and validated here too. |
newfrenchy83
force-pushed
the
fix_special_with_apply_to_opponent
branch
from
7306a2f
to
29cfe1b
Compare
like reported in forum in https://forums.wesnoth.org/viewtopic.php?p=672374#p672374 if unit with "last strike" ([firststrike] special with apply_to=opponent, on any of its attacks) is targeted by other unit for attack, wesnoth crashes, and other problems exist also for other weapon specials like poisons, when apply_to=opponent used(id two user of this specials fight, one fighter only could be poisoned)
newfrenchy83
force-pushed
the
fix_special_with_apply_to_opponent
branch
2 times, most recently
from
f4f8d84
to
abc4096
Compare
newfrenchy83
force-pushed
the
fix_special_with_apply_to_opponent
branch
from
abc4096
to
c879331
Compare
|
@Pentarctagon don't forget this PRfor master branch |
|
@stevecotton should I look at this one as well, or did you only mean the 1.16 version of the PR? |
Unit test for issue wesnoth#6575, and confirming that the fix in wesnoth#6573 works. Will rebase on top of the fix once the fix is merged. by https://github.com/wesnoth/wesnoth/commits?author=stevecotton
Like i added @stevecotton test today, i could said yes, but i is my own opinion. |
src/units/abilities.cpp
Outdated
| // 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))); | ||
| unit_const_ptr whom_unit = whom_is_self ? other : self; | ||
| map_location whom_loc = whom_is_self ? other_loc : self_loc; |
There was a problem hiding this comment.
Sorry, I was confused when suggesting names. whom_unit would make sense for the unit that is whom, but the unit here is the opponent of whom. How about whom_is_self, them_unit and them_loc; alternatively whom_is_self, them, their_loc?
So the comment could be
// There are only two units involved in this logic, but three pairs of names for them: {self, other},
// {attacker, defender} and {whom, them}.
There was a problem hiding this comment.
@stevecotton names changed, but i not added comment.
I think it's good, except for the mixed-up names (sorry newfrenchy83, those were my fault). Comments would be welcome, but I think it'll be ready to merge once those names change. It behaves as expected with the tests in PR #6582 - the ones that say "known bug in 1.16" fail and all the other pass. That's good, I'll open a PR with the 1.17 versions of those tests once this merges. |
like reported in forum in https://forums.wesnoth.org/viewtopic.php?p=672374#p672374
if unit with "last strike" ([firststrike] special with apply_to=opponent, on any of its attacks) is targeted by other unit for attack, wesnoth crashes,
and other problems exist also for other weapon specials like poisons, when apply_to=opponent used(id two user of this specials fight, one fighter only could be poisoned)