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 - Combine advancedBandages and woundReopening settings #7392

Merged
merged 5 commits into from
Feb 22, 2020

Conversation

mharis001
Copy link
Member

When merged this pull request will:

  • Combine advancedBandages and woundReopening settings into a single 3 option setting
    • woundReopening was only used when advancedBandages was enabled - since BasicBandage can't reopen
  • Remove "grey bandaged wounds" from displaying in injury lists
    • Caused confusion as these weren't actual bandaged wounds instead, open wounds with amount of zero
  • Improve injury lists wound entry adding code

@mharis001 mharis001 added the kind/enhancement Release Notes: **IMPROVED:** label Jan 4, 2020
@mharis001 mharis001 added this to the 3.13.1 milestone Jan 4, 2020
};
} forEach GET_STITCHED_WOUNDS(_target);
[GET_OPEN_WOUNDS(_target), "%1", [1, 1, 1, 1]] call _fnc_processWounds;
[GET_BANDAGED_WOUNDS(_target), "[B] %1", [0.88, 0.7, 0.65, 1]] call _fnc_processWounds;
Copy link
Contributor

@PabstMirror PabstMirror Jan 9, 2020

Choose a reason for hiding this comment

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

`if ((missionNamespace getVariable [QEGVAR(medical_treatment,advancedBandages), 1]) == 2) then {`
    [GET_BANDAGED_WOUNDS(_target), "[B] %1", [0.88, 0.7, 0.65, 1]] call _fnc_processWounds;

might save a little time as you can only get bandaged/stitched in mode2

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we'd still want to show any bandaged wounds in case the setting was changed mid-mission because those would still be able to be stitched.

Copy link
Member

Choose a reason for hiding this comment

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

I'd be for only listing them when reopening is active. Wouldn't they then appear when the UI is recreated if the setting is changed mid-mission?

Copy link
Member Author

Choose a reason for hiding this comment

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

The injuries list is constantly refreshed. The concern is that if a unit originally has a bandaged wound that could reopen or be stitched (advancedBandaged == 2), changing the setting would cause the originally bandaged wound to not be shown but it can still reopen and be stitched.

Bandaging open wounds after changing the setting would not produce any new bandaged wounds - not able to reopen or be stitched.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see the distinction here now between "bandaged wounds" the array and "bandaged wounds" when re-opening isn't enabled. Missed that before.

Wonder if we could handle that setting changing better 🤔 Probably beyond the scope of this PR though.

@PabstMirror
Copy link
Contributor

Might just be me, but I will miss seeing all the old "bandaged" wounds

Copy link
Member

@kymckay kymckay left a comment

Choose a reason for hiding this comment

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

I think this can be merged as is and perhaps a further PR could be made to better handle bandaged wounds with the setting being changed mid-mission.

@jonpas jonpas merged commit f6dda74 into master Feb 22, 2020
@jonpas jonpas deleted the combine-bandage-settings branch February 22, 2020 21:09
Vdauphin added a commit to Vdauphin/HeartsAndMinds that referenced this pull request Mar 9, 2020
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
Development

Successfully merging this pull request may close these issues.

6 participants