-
Notifications
You must be signed in to change notification settings - Fork 283
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
refactor: Remove most of body_part
enum usage
#5721
Conversation
6da5ef4
to
a2b8acf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Compiled and load-tested, no oddities seen with an old save.
- Spawned some lava nearby, checked that heat gain/loss and blistering seemed to work right.
- Put on the white glove, confirmed right hand specifically was warmer than left hand.
- Put on breeches, a tunic, a duster, chaps, and knee-high boots. Compact flag still worked.
- Put on two pairs of bone greaves, got the encumbrance penalty as expected.
- Confirmed I can't put on three pairs of bone greaves.
- Confirmed I can put on power armor and wear a helmet over it.
One regression: power armor accessories no longer limit what you can put on each bodypart.
I'm mixed on this however. Prior behavior was kinda poorly-done, it treated "either leg/arm" as a single bodypart and thus forbid you from wearing a power armor leg sheath on one leg and a leg mount on the other, when sane behavior would be for the "PA accessories are limited to one per body part" to correctly interpret "either side" as only one part, not both at once even after it's been assigned a side.
The code suggests it was supposed to be 2 accessories per bp. |
Then guess it's working as intended yeah, will resume testing in a bit when I get home. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continued testing:
- Encumberance display on the
@
screen looks right, also slapped on heavy survivor set to see what that does and seems to do the right stuff. - Spawned a zombie predator, it flailed at me uselessly (heavy survivor gear woot) until biting the leg leg which correctly showed as injured.
- Waited until it impaled me, it said it hit the torso and that's what's showing as injured and bleeding.
- Bandaged my torso, bleeding was stopped and the bandaged effect is on the correct bodypart.
- Inflicted myself with eepy and took a nap, torso healed up due to being bandaged as expected.
- Recruited NPC friend, broke one of their legs and made them put on a splint, they wore it on the correct leg as expected.
- Waited several days, examined the NPC to find the limb was making progress on recovering.
- Blindfolded myself, spawned a line of shotgun traps and walked through them, different parts get hit and message matches the part taking damage.
- Stripped naked and spawned in a triffid queen, it would impale different parts when I get dinged by a tree spawning and the expected part takes damage.
- Returned to my heavy survivor set, put it on, and spawned a second pair of heavy survivor gloves to wear. My hands correctly take a bonus encumbrance penalty.
is it ready for merging? |
Unless anyone can think of anything else to test, it seems good to go. |
lets goooooo |
@@ -542,7 +542,6 @@ void show_armor_layers_ui( Character &who ) | |||
ctxt.register_action( "NEXT_TAB" ); | |||
ctxt.register_action( "MOVE_ARMOR" ); | |||
ctxt.register_action( "CHANGE_SIDE" ); | |||
ctxt.register_action( "TOGGLE_CLOTH" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops
Checklist
Required
main
so it won't cause conflict when updatingmain
branch later.Optional
Purpose of change
Working towards removable limbs
Describe the solution
Grunt work consisting of:
num_bp
comparisons with null id checksDescribe alternatives you've considered
Testing
Things that need to be specifically tested:
@
menuAdditional context
hp_part
enum #5685 is merged