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

Treat bleeding from pets #75274

Merged
merged 4 commits into from
Jul 28, 2024

Conversation

Roguecop
Copy link
Contributor

@Roguecop Roguecop commented Jul 27, 2024

Summary

Features "Add option to make the bleeding of a pet stop"

Purpose of change

In the game until now, when a pet starts bleeding, we have no option to stop it using bandages or other meds that stop the bleeding effect.

This was tried to be solved by @Maleclypse in this PR #62216, but was unfinished and moved to Abandoned but Desired PRs

Describe the solution

I wanted something easy to solve the problem.

  1. When a pet is bleeding, a new option is added in the menu opened when we examine (key "e") our companion
  2. We need to have any "heal" item with "bleed" value in hand
  3. We use the items wielded, and the bleeding of the pet disappears.

In case we don't have an item in hand or the item cannot be used to stop the bleeding, a message pops up in the log

Describe alternatives you've considered

There were other options I considered:

  • When clicking to stop bleeding, open a menu to select a healing item
  • Using the wielded item but only healing proportionately to the effect of the healing item to stop bleeding

I chose the option implemented because it was the easiest one and could make the work without complications or make the pet die while the player is making the healing.

Testing

I did manual testing, summoning a pet and making him bleed. After that, I tried:

  • Heal without wielded items
  • Heal with items without "heal" value
  • Heal with items with "heal" value, but without "bleed" value
  • Heal with items with "heal" and "bleed" values

I wanted to create some tests to automate this, but I was a bit confused when I found there's no tests in the test folder related to examine a pet (Swap, saddle, rename, etc.).

Because I don't know if this is a decision or something we lack, I just wanted to wait and maybe create the tests in a different PR

Additional context

stop_bleeding

-Easy way to treat the bleeding of a pet

-Add message when bleeding stops
@github-actions github-actions bot added the [C++] Changes (can be) made in C++. Previously named `Code` label Jul 27, 2024
Better style

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions <Enhancement / Feature> New features, or enhancements on existing labels Jul 27, 2024
@Termineitor244
Copy link
Contributor

Finally!! I have been hoping for someone to take on this since the PR from Maleclypse was closed! Long live our pet overlords!

@github-actions github-actions bot added the astyled astyled PR, label is assigned by github actions label Jul 27, 2024
Roguecop and others added 2 commits July 27, 2024 18:36
Solve error with message:

Error: /home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/monexamine.cpp:58:1: error: string_id declarations should be sorted. [cata-static-string_id-constants,-warnings-as-errors]

/home/runner/work/Cataclysm-DDA/Cataclysm-DDA/src/monexamine.cpp:58:1: note: 'effect_bleed' should be before 'effect_tied'.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions and removed astyled astyled PR, label is assigned by github actions labels Jul 27, 2024
@Roguecop
Copy link
Contributor Author

I'm a bit confused with the check failing. Anyone know what could be wrong?

error

@Maleclypse
Copy link
Member

That failure isn't yours. Clang and all the required tests passed so you should be good on this.

@Maleclypse Maleclypse merged commit 4752f59 into CleverRaven:master Jul 28, 2024
20 of 26 checks passed
@Roguecop Roguecop deleted the bandage_pets_old_pr_62216 branch July 28, 2024 08:38
@DSeyka
Copy link
Contributor

DSeyka commented Jul 30, 2024

Why have this been merged without even basic proofreading?

@zachary-kaelan
Copy link
Contributor

I don't know if this is a decision or something we lack

By default assume it's something we lack. The tests in general seem to be outdated in many places.

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` <Enhancement / Feature> New features, or enhancements on existing json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants