Medical rewrite bug fixes #6523

redbery · 2018-08-12T03:13:47Z

When merged this pull request will:

Fix medical menu reopen setting
Fix second delay on execution condition which had a broken macro causing players to fall through cardiac arrest to death immediately
Fix people reporting 40 HR (set from leaving cardiac arrest state) when dead
Fix players having old stats on respawn (including references to arrays causing wounds to the old corpse to apply to the new player too)
Fix locality issue on unit init causing JiP to reset all variables (wounds etc) on all units
Fix updateHeartRate being allowed to go passed the target heart rate. This is an especially large issue as it made it completely impossible to perform CPR. During CPR the vitals would not be checked leading to the maximum deltaT of 10. On the first tick of handle vitals once CPR was successfully completed the function would be entered with a HR of 40 and the target HR would be 80. This would make the function
40 + ((80 - 40) / 2 )*10 = 240, which would be a HR high enough to push the unit back into cardiac arrest.

Note: I realize the respawn issue has been noted as fixed in #6518, but I can't find the respawn XEH noted in that PR. If I missed or misunderstood this I will of course happily take out my proposed fix. Also my apologies for the seemingly random mix of fixes. These were all the problems we ran into when running a test operation with the rewrite branch today.

…death Fixes medical variables not being reset on respawn Fixes JiP causing reset of all medical variables for all units Fixes updateHeartRate overshooting the target heartrate which made CPR impossible

commy2 · 2018-08-12T05:28:18Z

addons/medical_status/CfgEventHandlers.hpp

@@ -19,6 +19,7 @@ class Extended_PostInit_EventHandlers {
 class Extended_Init_EventHandlers {
    class CAManBase {
        class ADDON {
+            onRespawn = true;


This only accepts number 1, no strings. Booleans don't exist in config.
https://github.com/CBATeam/CBA_A3/blob/master/addons/xeh/fnc_compileEventHandlers.sqf#L155

Sorry about that. Took it from the XEH wiki and didn't notice the define statement 4 lines above it... Should be fixed now.

Just looked in the config viewer as I was wondering why this mistake didn't cause unwanted behavior and noticed that it already got converted from true to 1. I'll leave the change in for clarity.

Not all binarizers do this. Mikero converts it despite no define, which causes a lot of other problems.

TheMagnetar · 2018-08-12T05:56:39Z

addons/medical_vitals/functions/fnc_updateHeartRate.sqf

        if (_bloodVolume < BLOOD_VOLUME_CLASS_2_HEMORRHAGE) then {
            _targetHR = _heartRate * (_targetBP / (45 max _meanBP));
        };
        if (_painLevel > 0.2) then {
            _targetHR = _targetHR max (80 + 50 * _painLevel);
        };
-        _targetHR = _targetHR + _hrTargetAdjustment;
+        _targetHR = (_targetHR + _hrTargetAdjustment) max 0;


Isn't _targetHR always greater than 0? There is no need for max 0 in my opinion

The hrTargetAdjustment can be a rather large negative value depending on current medication I believe. I don't know if it could ever go below 0 because of that but I'd think its worth ensuring it won't. Will remove it if that's really wanted but it seems like taking a bit of a gamble just to save 6 characters.

This is good, we're just moving the max 0 up from line 73 where it was previously 👍

TheMagnetar · 2018-08-12T06:10:35Z

addons/medical_vitals/functions/fnc_updateHeartRate.sqf


        _hrChange = round(_targetHR - _heartRate) / 2;
    } else {
        _hrChange = -round(_heartRate / 10);
    };
-    _heartRate = (_heartRate + _deltaT * _hrChange) max 0;
+    if (_hrChange < 0) then {


You can put the whole new If statement directly under the _hrChange calculation and spare it since it is redundant:

_hrChange = round(_targetHR - _heartRate) / 2; _heartRate = (_heartRate + _deltaT * _hrChange) min _targetHR; } else ...

I'm sorry, I don't think I understand what you are suggesting. The heart rate is currently doing a min or max with _targetHR depending on if we are increasing or decreasing our heart rate. Wouldn't the change you are suggesting here cause any gradual change lowering the HR to happen immediately instead?

After going through the code more closely these lines look good to me 👍

This doesn't fix the issue of incorrectly large deltaT first tick after cardiac arrest, but you're right that we don't want the heart rate to pass the target anyway so it should stay because it does fix that issue (theoretically deltaT could reach 10 at any time).

For the exiting cardiac arrest deltaT issue, I think we should still be running handleUnitVitals while in cardiac arrest because it appears to handle other things not related to heart rate too, if we don't then we should refresh the stored lastTimeUpdated value on the unit when exiting cardiac arrest to avoid similar issues to the one revealed here.

TheMagnetar · 2018-08-12T06:15:42Z

addons/medical_vitals/functions/fnc_updateHeartRate.sqf

@@ -46,6 +46,7 @@ private _heartRate = GET_HEART_RATE(_unit);

 if !IN_CRDC_ARRST(_unit) then {


Isn't this preventing the unit from entering when in cardiac arrest?

On the first tick of handle vitals once CPR was successfully completed the function would be entered with a HR of 40 and the target HR would be 80. This would make the function
40 + ((80 - 40) / 2 )*10 = 240, which would be a HR high enough to push the unit back into cardiac arrest.

Regarding the XEH for the respawn issue:
https://github.com/acemod/ACE3/pull/6518/files#diff-081751a9b9de93a0af75655dce42c674R22

That is only resetting the statemachine though and not resetting the variables such as openWounds right? My worry is that if the new unit does not override the variables from the old unit on respawn it will have a reference to the actual arrays meaning they will receive any updates to the old arrays (and therefor receive wounds when shooting the old corpse).

The check indeed prevents the unit when entering when in cardiac arrest, but that doesn't matter too much as the updateUnitVitals and therefore updateHeartRate is not being called when in cardiac arrest. Problem is that the first time when the unit leaves cardiac arrest state, updateUnitVitals will run updateHeartRate with a _deltaT of 10 as updateUnitVitals hasn't been run in a while. Even if that would be fixed by running updateUnitVitals when in cardiac arrest, I don't believe that it is intentional to ever report a new HR after adjustment beyond the targetHR.

kymckay · 2018-08-12T07:45:35Z

addons/medical_status/CfgEventHandlers.hpp

@@ -19,6 +19,7 @@ class Extended_PostInit_EventHandlers {
 class Extended_Init_EventHandlers {
    class CAManBase {
        class ADDON {
+            onRespawn = 1;


👍 I overlooked that this wasn't a respawn XEH in my comment on #6518

kymckay · 2018-08-12T08:01:22Z

All looks good except I need to look more closely at the last point (there's definitely an unhandled scenario in the code there though) 👍

thojkooi

One suggestion to avoid mixing goals between components. Maybe the engine component may be a better fit, but I don't think it belongs in the statemachine logic.

thojkooi · 2018-08-12T10:27:33Z

addons/medical_statemachine/functions/fnc_enteredStateDeath.sqf

@@ -19,4 +19,7 @@ params ["_unit"];
 // Send a local event before death
 [QEGVAR(medical,death), [_unit]] call CBA_fnc_localEvent;

+_unit setVariable [VAR_HEART_RATE, 0, true];


This should be done in the medical vitals component through an eventhandler I think.

Well spotted, I would agree that an EH is better here

Would we also want to change leftStateCardiacArrest then?

We probably also want to do it for that function, yes. That may require introducing some new events though. Does not have to be done in this PR, imo. But we should create an issue for it if we do not. I will leave that to your discretion @redbery.

I don't think I'll have the time to do that today as it is somewhat involved which would mean it would need to wait until next weekend. Think it is better to create a new issue for it if you don't mind.

kymckay

Good fixes and has revealed an issue in the vitals loop when exiting cardiac arrest (time delta is incorrectly at max of 10 because loop doesn't run while in cardiac arrest). We can fix this in a different PR.

I agree with @thojkooi's requested changes, but I think we should merge this and change that in a new PR (since it's also relevant to a file not changed here).

kymckay · 2018-08-14T18:18:48Z

addons/medical_vitals/functions/fnc_updateHeartRate.sqf

        if (_bloodVolume < BLOOD_VOLUME_CLASS_2_HEMORRHAGE) then {
            _targetHR = _heartRate * (_targetBP / (45 max _meanBP));
        };
        if (_painLevel > 0.2) then {
            _targetHR = _targetHR max (80 + 50 * _painLevel);
        };
-        _targetHR = _targetHR + _hrTargetAdjustment;
+        _targetHR = (_targetHR + _hrTargetAdjustment) max 0;


This is good, we're just moving the max 0 up from line 73 where it was previously 👍

kymckay · 2018-08-14T18:26:02Z

addons/medical_vitals/functions/fnc_updateHeartRate.sqf


        _hrChange = round(_targetHR - _heartRate) / 2;
    } else {
        _hrChange = -round(_heartRate / 10);
    };
-    _heartRate = (_heartRate + _deltaT * _hrChange) max 0;
+    if (_hrChange < 0) then {


After going through the code more closely these lines look good to me 👍

This doesn't fix the issue of incorrectly large deltaT first tick after cardiac arrest, but you're right that we don't want the heart rate to pass the target anyway so it should stay because it does fix that issue (theoretically deltaT could reach 10 at any time).

For the exiting cardiac arrest deltaT issue, I think we should still be running handleUnitVitals while in cardiac arrest because it appears to handle other things not related to heart rate too, if we don't then we should refresh the stored lastTimeUpdated value on the unit when exiting cardiac arrest to avoid similar issues to the one revealed here.

thojkooi · 2018-08-14T20:16:37Z

@SilentSpike yes, sounds good.

kymckay · 2018-08-14T20:32:10Z

I've opened two issues so we don't forget the issues touched upon here:
#6530
#6531

* Fix medical menu not reopening regardless of setting * Ensure no heartrate or bloodpressure on death * Fix status variables not being reset on respawn * Fix JIP resetting status variables for all units * Fix heart rate adjustment overflow and

redbery added 5 commits August 12, 2018 01:32

Fixes medical menu not reopening regardless of setting

e51ba84

Changes enteredStateDeath to ensure no heartrate or bloodpressure on …

4ac3b95

…death Fixes medical variables not being reset on respawn Fixes JiP causing reset of all medical variables for all units Fixes updateHeartRate overshooting the target heartrate which made CPR impossible

Fixes macro to point to correct variable location

017b0e5

Fixes HR being Nil due to private declaration being in the wrong scope

2541b65

Revert accidental formatting changes

fa05b02

commy2 reviewed Aug 12, 2018

View reviewed changes

TheMagnetar requested changes Aug 12, 2018

View reviewed changes

TheMagnetar reviewed Aug 12, 2018

View reviewed changes

TheMagnetar added this to the Medical Rewrite milestone Aug 12, 2018

Fixes incorrect config value

d8ff004

kymckay reviewed Aug 12, 2018

View reviewed changes

thojkooi requested changes Aug 12, 2018

View reviewed changes

thojkooi added the kind/bug-fix Release Notes: **FIXED:** label Aug 12, 2018

thojkooi added this to Open in Medical Focus Feature via automation Aug 12, 2018

kymckay approved these changes Aug 14, 2018

View reviewed changes

thojkooi approved these changes Aug 14, 2018

View reviewed changes

kymckay mentioned this pull request Aug 14, 2018

time delta is incorrectly large when exiting cardiac arrest #6530

Closed

kymckay merged commit 5fbaf42 into acemod:medical-rewrite Aug 14, 2018

Medical Focus Feature automation moved this from Open to Completed Aug 14, 2018

redbery deleted the medical-rewrite branch August 14, 2018 21:44

PabstMirror modified the milestones: Medical Rewrite, 3.13.0, 3.13.0-temp3 Dec 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Medical rewrite bug fixes #6523

Medical rewrite bug fixes #6523

redbery commented Aug 12, 2018

commy2 Aug 12, 2018

redbery Aug 12, 2018

redbery Aug 12, 2018

commy2 Aug 12, 2018

TheMagnetar Aug 12, 2018

redbery Aug 12, 2018

kymckay Aug 14, 2018

TheMagnetar Aug 12, 2018

redbery Aug 12, 2018

kymckay Aug 14, 2018

TheMagnetar Aug 12, 2018 •

edited

redbery Aug 12, 2018

redbery Aug 12, 2018

kymckay Aug 12, 2018

kymckay commented Aug 12, 2018

thojkooi left a comment

thojkooi Aug 12, 2018

kymckay Aug 12, 2018

redbery Aug 12, 2018

thojkooi Aug 12, 2018

redbery Aug 12, 2018

kymckay left a comment

kymckay Aug 14, 2018

kymckay Aug 14, 2018

thojkooi commented Aug 14, 2018

kymckay commented Aug 14, 2018

		@@ -46,6 +46,7 @@ private _heartRate = GET_HEART_RATE(_unit);

		if !IN_CRDC_ARRST(_unit) then {

Medical rewrite bug fixes #6523

Medical rewrite bug fixes #6523

Conversation

redbery commented Aug 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheMagnetar Aug 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kymckay commented Aug 12, 2018

thojkooi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kymckay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thojkooi commented Aug 14, 2018

kymckay commented Aug 14, 2018

TheMagnetar Aug 12, 2018 •

edited