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

Don't truncate allergy vitamins out of existence #78792

Merged

Conversation

RenechCDDA
Copy link
Member

Summary

None

Purpose of change

#77825 (comment)
image

Describe the solution

Basically camps were doing 1 * some_double_less_than_one and always receiving 0, because we truncated to integer. The truncation was intended, but this scenario was not anticipated when I wrote the operator, because we didn't have allergies as vitamins.

So now the *= operator (double overload) always keeps at least 1 of existing vitamins, instead of fully truncating. For results greater than 1 (e.g. 1.1, or 62.7, or other numbers) it still truncates normally. The only reason for this is make sure we don't truncate allergies out of existence.

This may slightly skew the resulting math, but only to a negligible degree.

This does not support being called on negative vitamin values, which are technically possible (via stored character values). The operator is only currently called for faction camps distributing food items, where I added it.

Because the math produced would be VERY, VERY wrong if negative values are used, I added an assertion that the stored value is not negative. The game will crash, loudly and pointedly, which should be sufficient to warn any C++ contributor who might use it in the future without considering this.

Describe alternatives you've considered

Require some minimum amount of allergy vitamin to avoid this pitfall, but that just papers over the problem. Besides we'll get more good PRs like #75918 (by @PatrikLundell ) who run into our bad math. Asking everyone to dodge around our bad math is not a solution.

Only keep 1 if the vitamin type is actually an allergy? We don't have such a clearly defined vit_type at the moment...

Testing

Math checks out, I've gone over it several times.
I did not compile this branch.

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Dec 27, 2024
@GuardianDll GuardianDll merged commit 35b0d6f into CleverRaven:master Jan 5, 2025
23 of 27 checks passed
@RenechCDDA RenechCDDA deleted the no_disappearing_allergy_vitamins branch January 5, 2025 12:17
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 [C++] Changes (can be) made in C++. Previously named `Code` 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