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

Unit tests for [drains], [poison] and [slow], with apply_to=opponent #6625

Merged

Conversation

stevecotton
Copy link
Contributor

@stevecotton stevecotton commented Apr 14, 2022

Slightly different to PR #6582, which was the 1.16 version of this. The five
lines that were labelled preserving known bug are changed to test that it's
been fixed. Once #6582 is approved, I'm intending to merge this too.

Here apply_to=opponent means that the weapon special gives the opponent the
ability, the unit that should get poisoned or slowed is the unit that has the
weapon special.

There's a known bug in 1.16, that apply_to=opponent check the wrong unit to
see it it's unpoisonable, undrainable etc. It also checks the wrong unit to
see if it's already poisoned or slowed, so a battle between two units that both
have reverse-poison results in at most one being poisoned.

Most of the credit for this is Newfrenchy's, as he's already written a fix
and a WML based test. This commit uses a Lua test instead to test more
combinations of statuses.

This adds a COMMON_KEEP_A_B_UNIT_TEST macro, which is a counterpart to the
GENERIC_UNIT_TEST macro that starts the leaders next to each other, ready
to attack. The A_B is because I'm planning a multiple-side variant too.

There's no test for [petrify], as simulate_combat doesn't provide a stat for it.

This tests only 3 of the 6 abilities whose behavior changed in 650f704.
My thoughts on testing the others are:

  • [firststrike]'s test is in 650f704.
  • [drains], [poison] and [slow] are tested here.
  • [petrify] ends combat, it's also not exposed in simulate_combat's stats.
  • [plague] triggers after combat ends.

@stevecotton stevecotton added Bug Issues involving unexpected behavior. Unit Tests Issues involving Wesnoth's unit test suite. Units Issues that involve unit definitions or their implementation in the engine. labels Apr 14, 2022
@stevecotton stevecotton self-assigned this Apr 14, 2022
@stevecotton stevecotton force-pushed the test_reflexive_specials_master branch from c95216f to 566618c Compare April 14, 2022 17:53
Slightly different to PR wesnoth#6582, which was the 1.16 version of this. The five
lines that were labelled `preserving known bug` are changed to test that it's
been fixed.

Here `apply_to=opponent` means that the weapon special gives the opponent the
ability, the unit that should get poisoned or slowed is the unit that has the
weapon special.

There's a known bug in 1.16, that `apply_to=opponent` check the wrong unit to
see it it's `unpoisonable`, `undrainable` etc. It also checks the wrong unit to
see if it's already poisoned or slowed, so a battle between two units that both
have reverse-poison results in at most one being poisoned.

Most of the credit for this is Newfrenchy's, as he's already written a fix
and a WML based test. This commit uses a Lua test instead to test more
combinations of statuses.

This adds a `COMMON_KEEP_A_B_UNIT_TEST` macro, which is a counterpart to the
`GENERIC_UNIT_TEST` macro that starts the leaders next to each other, ready
to attack. The `A_B` is because I'm planning a multiple-side variant too.

There's no test for [petrify], as simulate_combat doesn't provide a stat for it.

This tests only 3 of the 6 abilities whose behavior changed in 650f704.
My thoughts on testing the others are:
* [firststrike]'s test is in 650f704.
* [drains], [poison] and [slow] are tested here.
* [petrify] ends combat, it's also not exposed in simulate_combat's stats.
* [plague] triggers after combat ends.
@stevecotton stevecotton force-pushed the test_reflexive_specials_master branch from 566618c to a2968dc Compare April 17, 2022 19:14
@stevecotton stevecotton merged commit 0f6a94f into wesnoth:master Apr 19, 2022
@stevecotton stevecotton deleted the test_reflexive_specials_master branch April 19, 2022 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues involving unexpected behavior. Unit Tests Issues involving Wesnoth's unit test suite. Units Issues that involve unit definitions or their implementation in the engine.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant