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

fixes encumbrance_covered muts being covered by integrated armor #63182

Merged
merged 3 commits into from
Jan 30, 2023

Conversation

Vaskritaya
Copy link
Contributor

Summary

Bugfixes "Integrated armors no longer cause encumbrance from some mutations"

Purpose of change

Currently, integrated armors still count as standard clothing for the purposes of mutations which become encumbering when covered tightly, such as photophore or webbed hands. Fixes #63161 . This PR is an alternative to #63171 , which solves this same problem in an alternative way. I personally think changing the check itself is a safer bet than adding a new flag to mutations, but the ultimate decision is up to the devs.

Steps to reproduce the bug are in the linked report.

Describe the solution

The check to ensure that only oversized clothing was worn on relevant bodyparts was replaced with the new "wearing_fitting_on" method from #59092 , which not only ignores oversized clothes, but also integrated armors.

Describe alternatives you've considered

I considered not doing this (and, facing compiler errors, giving up) and just letting the other PR's solution handle it.

Testing

  1. debug mutated epicuticle and reflex photophore
  2. confirmed that, when hatless, head encumbrance was 1 (from epicuticle).
  3. wore a bike helmet, gained appropriate encumbrance for both the helmet and the photophore coverage
  4. wore an oversized helmet, gained appropriate encumbrance (no +5 from photophore this time)
  5. also checked with webbed hands and sparse scales.

Additional context

I would not consider myself a programmer, certainly not for anything as annoying to work with as cpp. I'm also not a very good cdda player. I would highly recommend that this PR is tested by people very familiar with both the game and the language to ensure I didn't horribly break anything, since I only got this far by trial and error.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) labels Jan 19, 2023
@github-actions github-actions bot added 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 Jan 20, 2023
@bombasticSlacks
Copy link
Contributor

I'm making a note to review this in the coming weekend. I think unit tests will need to be added to make sure :)

@Vaskritaya
Copy link
Contributor Author

I'm making a note to review this in the coming weekend. I think unit tests will need to be added to make sure :)

Do you mean I should make a test to ensure mutations provide correct encumbrance values?

@bombasticSlacks
Copy link
Contributor

No I'll try and write some while testing and then it'll stay fixed 😃

@bombasticSlacks
Copy link
Contributor

Didn't realize how small this is, I don't think this will need a test it should just work as is 😄

@bombasticSlacks bombasticSlacks merged commit daf8acf into CleverRaven:master Jan 30, 2023
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` json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Webbed hands gives encumbrance with sleek/sparse scales
2 participants