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.

Sign up for GitHub

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

Jump to bottom

Medical rewrite bug fixes #6523

Merged
merged 6 commits into from Aug 14, 2018
Merged

Medical rewrite bug fixes #6523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e51ba84
Fixes medical menu not reopening regardless of setting
redbery Aug 11, 2018
4ac3b95
Changes enteredStateDeath to ensure no heartrate or bloodpressure on …
redbery Aug 12, 2018
017b0e5
Fixes macro to point to correct variable location
redbery Aug 12, 2018
2541b65
Fixes HR being Nil due to private declaration being in the wrong scope
redbery Aug 12, 2018
fa05b02
Revert accidental formatting changes
redbery Aug 12, 2018
d8ff004
Fixes incorrect config value
redbery Aug 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion addons/medical_gui/functions/fnc_handleUI_DisplayOptions.sqf
View file
Open in desktop
Expand Up @@ -97,7 +97,7 @@ private _entries = [ACE_player, GVAR(INTERACTION_TARGET), _name] call FUNC(getTr
private _ctrl = (_display displayCtrl (START_IDC + _forEachIndex));
if (!(_forEachIndex > AMOUNT_OF_ENTRIES)) then {
_ctrl ctrlSetText (_x select 0);
private _code = format ["ace_medical_menu_pendingReopen = true; call %1;", (_x select 3)];
private _code = format ["ace_medical_gui_pendingReopen = true; call %1;", (_x select 3)];
_ctrl ctrlSetEventHandler ["ButtonClick", _code];
_ctrl ctrlSetTooltip (_x select 0); // TODO implement
_ctrl ctrlShow true;
Expand Down
2 changes: 1 addition & 1 deletion addons/medical_statemachine/functions/fnc_conditionExecutionDeath.sqf
View file
Open in desktop
Expand Up @@ -14,4 +14,4 @@

params ["_unit"];

(GVAR(fatalInjuryCondition) < 2) && {!(_unit getVariable [QGVAR(deathBlocked), false])}
(GVAR(fatalInjuryCondition) < 2) && {!(_unit getVariable [QEGVAR(medical,deathBlocked), false])}
3 changes: 3 additions & 0 deletions addons/medical_statemachine/functions/fnc_enteredStateDeath.sqf
View file
Open in desktop
Expand Up @@ -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];
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would we also want to change leftStateCardiacArrest then?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

_unit setVariable [VAR_BLOOD_PRESS, [0, 0], true];

_unit setDamage 1;
1 change: 1 addition & 0 deletions addons/medical_status/CfgEventHandlers.hpp
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@ class Extended_PostInit_EventHandlers {
class Extended_Init_EventHandlers {
class CAManBase {
class ADDON {
onRespawn = 1;
Copy link
Member

Choose a reason for hiding this comment

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

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

init = QUOTE(call FUNC(initUnit));
};
};
Expand Down
2 changes: 2 additions & 0 deletions addons/medical_status/functions/fnc_initUnit.sqf
View file
Open in desktop
Expand Up @@ -17,6 +17,8 @@

params ["_unit"];

if (!local _unit) exitWith {};

if (damage _unit > 0) then {
_unit setDamage 0;
};
Expand Down
12 changes: 8 additions & 4 deletions addons/medical_vitals/functions/fnc_updateHeartRate.sqf
View file
Open in desktop
Expand Up @@ -46,6 +46,7 @@ private _heartRate = GET_HEART_RATE(_unit);

if !IN_CRDC_ARRST(_unit) then {
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

private _hrChange = 0;
private _targetHR = 0;
private _bloodVolume = GET_BLOOD_VOLUME(_unit);
if (_bloodVolume > BLOOD_VOLUME_CLASS_4_HEMORRHAGE) then {
GET_BLOOD_PRESSURE(_unit) params ["_bloodPressureL", "_bloodPressureH"];
Expand All @@ -57,23 +58,26 @@ if !IN_CRDC_ARRST(_unit) then {
_targetBP = _targetBP * (_bloodVolume / DEFAULT_BLOOD_VOLUME);
};

private _targetHR = DEFAULT_HEART_RATE;
_targetHR = DEFAULT_HEART_RATE;
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;
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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


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

Choose a reason for hiding this comment

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

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 ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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.

_heartRate = (_heartRate + _deltaT * _hrChange) max _targetHR;
} else {
_heartRate = (_heartRate + _deltaT * _hrChange) min _targetHR;
};
};

_unit setVariable [VAR_HEART_RATE, _heartRate, _syncValue];

_heartRate