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 limb drying rate test #78189

Merged
merged 1 commit into from
Nov 28, 2024
Merged

Conversation

ShnitzelX2
Copy link
Contributor

Summary

Bugfixes "fix limb drying rate and related test data"

Purpose of change

A deeper dive for #78014, fixing the drying_rate test case. In retrospect, it is astounding the test even works sometimes.

  1. This test case, for drying a character off, did not take into account the current weather. Whether it's warm or freezing and foggy makes a big difference in the test's outcomes. I captured the weather in the last PR, where it was identified to indeed be the issue:
D:\a\Cataclysm-DDA\Cataclysm-DDA\tests\limb_test.cpp(187): FAILED:
  REQUIRE( base_dry == Approx( 450 ).margin( 125 ) )
with expansion:
  587 (0x24b) == Approx( 450.0 )
with messages:
  weather.weather_id.c_str() := "clear"
  units::to_fahrenheit( weather.temperature ) := 19.61243f  <--- 19 degrees F; colder = drying takes longer
  weather.weather_precise->humidity := 58.9053541728

I also isolated the same issues by running the following tests, in which drying rate nearly always failed:

acid_field_expiry_on_map
field_expiry
firebugs
fd_acid_falls_down
fire_spreading
drying_rate
  1. The approximate "correct" values for the test were mathematically wrong, period. I don't know how the given numbers were calculated, not that it mattered because the weather wasn't standardized for the test.

Describe the solution

Respectively,

  1. At beginning of test, set weather (temperature + humidity + CLEAR) to normal as defined by the drying algorithm
  2. Readjust test values to be more accurate for normal weather

Describe alternatives you've considered

Testing

Ran the test a few times after fixing and got results almost identical to the test's expected values, then ran the full suite without any errors. If you see errors again, post them here.

Additional context

weather not accounted for in test, update test values
@github-actions github-actions bot added Code: Tests Measurement, self-control, statistics, balancing. [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Nov 27, 2024
@Night-Pryanik Night-Pryanik merged commit b21c346 into CleverRaven:master Nov 28, 2024
22 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants