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

Added basic weakpoint unittest #51375

Merged
merged 6 commits into from
Sep 11, 2021

Conversation

StStep
Copy link
Contributor

@StStep StStep commented Sep 4, 2021

Summary

None

Purpose of change

Add a unit test for helping with #51305

Describe the solution

Added a unit test that works of statistical damage rates

Describe alternatives you've considered

Splitting into projectile testing and melee testing. Including project code had it go through the crit/good-hit/glance code which made doing damage aver checks hard.

Testing

Additional context

@actual-nh actual-nh added [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. <Enhancement / Feature> New features, or enhancements on existing labels Sep 4, 2021
@StStep
Copy link
Contributor Author

StStep commented Sep 4, 2021

This is my first time making a unit test for this project, and I'm not sure how to best handle checking this. Damage average makes since to me, but it looks like it can vary outside of the current epsilon, so bad random failures. I suppose I could make the func accept percent hit of weakpoints instead of average damage and then count the number of weakpoint hits and go from there? I do want to try and check the armor changes for weakpoints are being used though.

Instead of having the programmer do the math, perhaps the test could directly parse the weakpoints json for the monster and calculate what it should be?

@Maleclypse Maleclypse requested a review from wapcaplet September 5, 2021 17:12
@wapcaplet
Copy link
Contributor

It is always nice to see more unit tests. This looks good so far. One thing I would suggest is to make your damage_monster function return the float for total damage done, then do the CHECK parts inside the TEST_CASE (instead of inside the called function), something like this:

CHECK( damage_monster( ... ) == Approx( 18.75f ).epsilon( 0.15f ) );

This is mainly a style issue, but I've found tests to be generally more readable when their expectations are structured in this way. This would also allow you a finer-grained control over the epsilon for each case.

I see you already know the importance of Approx to deal with the potential randomness of damage calculation. I am afraid I don't have specific feedback on your questions for making it more predictable, but we do have some existing tests for weapon average damage per second in tests/effective_dps_test.cpp you might draw ideas from, if you have not already.

@StStep StStep marked this pull request as ready for review September 6, 2021 23:31
@wapcaplet
Copy link
Contributor

At a glance, the latest changes look good to me. I like the idea of summarizing the results in a report structure; it helps with readability in the main test.

The continuous integration test that’s failing (basic build and test / GCC) is a field test I’ve seen false alarms from before, so you shouldn’t have to worry about that, in case you were wondering.

@kevingranade kevingranade merged commit 0144ecf into CleverRaven:master Sep 11, 2021
Venera3 pushed a commit to Venera3/Cataclysm-DDA that referenced this pull request Sep 21, 2021
* Added basic weakpoint unittest
* Using weakpoint hit access to get more specific and reliable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. <Enhancement / Feature> New features, or enhancements on existing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants