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

Add unit tests for get_hit, get_dodge and related functions #39491

Merged
merged 2 commits into from
Apr 19, 2020

Conversation

wapcaplet
Copy link
Contributor

@wapcaplet wapcaplet commented Apr 12, 2020

Summary

SUMMARY: Infrastructure "Add unit tests for get_hit, get_dodge and related functions"

Purpose of change

Improved test coverage of melee aspects

Describe the solution

Adds tests/melee_dodge_hit_test.cpp with unit tests focused on these methods from

  • player/monster/Character get_hit_base
    • monster hit base is just skill level
    • player hit base is DEX / 4
  • player/monster/Character get_dodge_base
    • monster dodge base is just skill level
    • player dodge base is DEX / 2 + dodge skill
  • player get_dodge (in progress)
    • cannot dodge while asleep/narcotized or winded
    • bear trap reduces dodge to 1/2
    • bouldering reduces dodge to 1/4
    • rollerblades/skates/heelys reduce dodge to 1/5 (or 1/2 for pro skaters)
    • grabs reduce dodge (1 grab = 1/2, 2 grabs = 1/3 etc.)
    • speed reduces dodge linearly (90 speed = 90% dodge etc.)
  • monster get_dodge
    • cannot dodge while downed
    • trapped or tied: 1/2 dodge
    • unstable footing: 1/4 dodge

Describe alternatives you've considered

Unexplored mystery

Testing

tests/cata_test [melee][hit],[melee][dodge]

Additional context

This went through some rough (and I do mean rough) drafts, but I have rebased it into two clean commits now.

@wapcaplet
Copy link
Contributor Author

Several of these tests are randomly failing, even with the same execution order. Common failures are when avatar mysteriously has 1/4 the initial dodge level (causing all others in that section to fail), or when one of the four monsters in the "grab" test are checked for positioning, for example:

image

Using bash to run the same tests multiple times:

for i in `seq 10`; do ./tests/cata_test [dodge][grab] -d yes; done

The Travis CI run on this PR had a new failure I've not seen locally:

image

Note, this CHECK is occurring immediately after clear_character, and a couple of REQUIRE statements to ensure the expected DEX and dodge skill - yet it still randomly sometimes comes up 1.0 or 2.75.

@wapcaplet
Copy link
Contributor Author

Using clear_map before each test mitigates (but does not eliminate) the random failures.

@wapcaplet
Copy link
Contributor Author

I believe the random failures can be traced to a problem with the clear_character helper function; this function was erroneously setting the position of the global avatar g->u, rather than setting the position of the avatar instance passed to the function as dummy. This is now fixed, and the random failures have been eliminated.

@mlangsdorf mlangsdorf added Code: Tests Measurement, self-control, statistics, balancing. Melee Melee weapons, tactics, techniques, reach attack labels Apr 14, 2020
@mlangsdorf
Copy link
Contributor

I'm adding a melee stats counter in #39564 which may or may not simplify some of these tests.

@wapcaplet wapcaplet changed the title [WIP] Add unit tests for get_hit, get_dodge and related functions Add unit tests for get_hit, get_dodge and related functions Apr 17, 2020
@wapcaplet
Copy link
Contributor Author

wapcaplet commented Apr 18, 2020

CI test failures here in crafting_test.cpp and vehicle_interact_test.cpp appear to be legitimate for once - I believe they are a side-effect of the additional cleanup steps I have done in clear_character, and I can reproduce them locally. I can also confirm that the errors do not happen without my changes to clear_character.

Now I just need to figure out how on earth having a cleaner starting avatar is causing crafting failure...

Use dummy.setpos instead of g->u in the place_player helper function.
Global `g->u` avatar instance is implicit in `place_player`; we want to
set the position of the `dummy` instance, not necessarily `g->u`.

Also reset speed and bonuses in clear_character.
Includes new unit tests for Character, player, and monster functions
get_hit_base, get_dodge_base, and get_dodge.
@kevingranade kevingranade merged commit cba9b9f into CleverRaven:master Apr 19, 2020
@wapcaplet wapcaplet deleted the dodge-hit-tests branch April 25, 2020 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code: Tests Measurement, self-control, statistics, balancing. Melee Melee weapons, tactics, techniques, reach attack
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants