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

Medical Treatment - Allow variable CPR chance based on blood volume (invalidates old setting) #7983

Merged
merged 5 commits into from
Jul 23, 2021

Conversation

pterolatypus
Copy link
Contributor

@pterolatypus pterolatypus commented Nov 4, 2020

When merged this pull request will:

  • CPR success chance is interpolated between two settings, based on current blood volume
  • Default values are the same to maintain current behaviour

I've left the old stringtable entries in for reference - is this a bad idea?

Interpolates based on blood volume between 60% (below this value you would immediately arrest again) and 85% (minimum threshold for waking up)

@jonpas jonpas added the kind/enhancement Release Notes: **IMPROVED:** label Nov 7, 2020
@jonpas jonpas added this to the 3.14.0 milestone Nov 7, 2020
@pterolatypus pterolatypus force-pushed the feature/cprinterp branch 2 times, most recently from fbcbb32 to 41adb13 Compare December 24, 2020 13:39
@kymckay
Copy link
Member

kymckay commented Dec 24, 2020

I like what you're doing here functionally, but I think the settings aren't very transparent. It's fine for anyone like me who can read the code and see that "minimum chance" really means "chance at hemorrhage level 4 and "maximum chance" means at level 1.

I think they need to be presented in user terms "CPR success chance with high blood loss" and "with low blood loss" (or whatever in-game corresponding terminology we use).

@pterolatypus
Copy link
Contributor Author

pterolatypus commented Dec 24, 2020

I think the settings aren't very transparent

Agreed. I'll try to think of a better tooltip, but it's hard to explain interpolation in a concise way.

@jonpas
Copy link
Member

jonpas commented Apr 20, 2021

@mharis001 settings guru, please check.

Copy link
Member

@mharis001 mharis001 left a comment

Choose a reason for hiding this comment

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

Should remove strings for old settings.

Although including the interpolation details may be too technical for setting descriptions, I think it is better to provide as much detail as possible so that the user can completely understand the behaviour from the description instead of leaving them to guess at exactly when the minimum and maximum values are used.

addons/medical_treatment/stringtable.xml Outdated Show resolved Hide resolved
addons/medical_treatment/stringtable.xml Outdated Show resolved Hide resolved
@jonpas jonpas modified the milestones: 3.14.1, 3.14.0 Apr 20, 2021
@pterolatypus pterolatypus requested a review from mharis001 May 26, 2021 22:01
@jonpas jonpas changed the title Medical Treatment - Allow variable CPR chance based on blood volume Medical Treatment - Allow variable CPR chance based on blood volume (invalidates old setting) Jul 23, 2021
@jonpas jonpas merged commit c81f371 into acemod:master Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants