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

refactor: Remove most of body_part enum usage #5721

Merged
merged 4 commits into from
Dec 27, 2024

Conversation

Coolthulhu
Copy link
Member

@Coolthulhu Coolthulhu commented Nov 15, 2024

Checklist

Required

Optional

Purpose of change

Working towards removable limbs

Describe the solution

Grunt work consisting of:

  • Replacing iteration over the enum to iteration over player's body parts
  • Replacing num_bp comparisons with null id checks

Describe alternatives you've considered

Testing

Things that need to be specifically tested:

  • Wearing power armor accessories respecting limits on 2 accessory per bp
  • Switching sides on sided equipment respecting usual limits
  • Body part hp display
  • Body part encumbrance description in @ menu
  • Layering penalty
  • NPCs being given splints
  • Monster basic melee attack printing correct messages regarding player's body parts
  • Shotgun trap picking different body parts
  • Triffid queen attack damaging different body parts

Additional context

@github-actions github-actions bot added src changes related to source code. tests changes related to tests labels Nov 15, 2024
@Coolthulhu Coolthulhu marked this pull request as ready for review November 25, 2024 08:31
@scarf005 scarf005 requested a review from chaosvolt November 26, 2024 00:31
Copy link
Member

@chaosvolt chaosvolt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Compiled and load-tested, no oddities seen with an old save.
  2. Spawned some lava nearby, checked that heat gain/loss and blistering seemed to work right.
  3. Put on the white glove, confirmed right hand specifically was warmer than left hand.
  4. Put on breeches, a tunic, a duster, chaps, and knee-high boots. Compact flag still worked.
  5. Put on two pairs of bone greaves, got the encumbrance penalty as expected.
  6. Confirmed I can't put on three pairs of bone greaves.
  7. 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.

image

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.

@chaosvolt
Copy link
Member

Oh wait, I tested in my old desktop build CBN 2024-11-26 and it's doing this too, dafuq.
image

I thought power armor accessories were supposed to be limited to one per limb, hence the earlier discovery that it was bugged? And now that bug has gone and vanished since last time I encountered it... XD

@Coolthulhu
Copy link
Member Author

"PA accessories are limited to one per body part"

The code suggests it was supposed to be 2 accessories per bp.

@chaosvolt
Copy link
Member

Then guess it's working as intended yeah, will resume testing in a bit when I get home.

Copy link
Member

@chaosvolt chaosvolt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continued testing:

  1. 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.
  2. Spawned a zombie predator, it flailed at me uselessly (heavy survivor gear woot) until biting the leg leg which correctly showed as injured.
  3. Waited until it impaled me, it said it hit the torso and that's what's showing as injured and bleeding.
  4. Bandaged my torso, bleeding was stopped and the bandaged effect is on the correct bodypart.
  5. Inflicted myself with eepy and took a nap, torso healed up due to being bandaged as expected.
  6. Recruited NPC friend, broke one of their legs and made them put on a splint, they wore it on the correct leg as expected.
  7. Waited several days, examined the NPC to find the limb was making progress on recovering.
  8. Blindfolded myself, spawned a line of shotgun traps and walked through them, different parts get hit and message matches the part taking damage.
  9. 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.
  10. 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.

@scarf005
Copy link
Member

scarf005 commented Dec 26, 2024

is it ready for merging?

@chaosvolt
Copy link
Member

Unless anyone can think of anything else to test, it seems good to go.

@scarf005
Copy link
Member

lets goooooo

@scarf005 scarf005 merged commit 87737db into cataclysmbnteam:main Dec 27, 2024
16 checks passed
@@ -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" );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src changes related to source code. tests changes related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants