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 Feedback = Adds Icon option for Low blood volume effects #7507

Merged
merged 5 commits into from
Feb 22, 2020
Merged

Medical Feedback = Adds Icon option for Low blood volume effects #7507

merged 5 commits into from
Feb 22, 2020

Conversation

10Dozen
Copy link
Contributor

@10Dozen 10Dozen commented Feb 3, 2020 •
edited
Loading

When merged this pull request will:

  • Add setting to switch Low blood volume effect with 3 options: Color fading, Icon, Color fading + Icon
  • Add 'blood drop' icon to UI Stance indicator on low blood; indicator has 6 states from almost full to empty (from class 2 to class 4+ hemorrhage) and 3 color (white, orange and red);

Notes:
stance indicator on high difficulty level become hidden when player is inactive, so for a bit milsim guys blood loss indicator shows up only when they have noticeable blood loss and are moving actively.

@10Dozen
Adds Icon option for Low blood volume at Medical Feedback
7e80026 
- Adds setting to switch Low blood volume effect with 3 options: Color fading, Icon, Color fading + Icon
- Adds icon to UI on low blood
- Adds icon PAA files
@10Dozen
Review fixes
eb9238a 
- Added ACE tags to added controls
- Added defines for GUI position/size
- Added missing newline at EOF
- Updated fnc_effectBloodVolumeIcon (removed usage of uiNamespace and minor changes)
- CBA_fnc_addSetting is now used to init settings
- Icon path macro updated to return formatted path
@10Dozen
Review fixes vol.2
baac7d3 
- Removed `disableSerialization` and added default value for indicator control and not null checks to effectBVI function
- Removed defines from RscInGameUI and used a3 defines instead
- Changed formatting of macroses
mharis001
mharis001 reviewed Feb 3, 2020
View reviewed changes
addons/medical_feedback/initSettings.sqf Outdated Show resolved Hide resolved
addons/medical_feedback/functions/fnc_effectBloodVolumeIcon.sqf Outdated Show resolved Hide resolved
addons/medical_feedback/RscInGameUI.hpp Outdated Show resolved Hide resolved
addons/medical_feedback/RscInGameUI.hpp Outdated
class RscStanceInfo {
controls[] += {QGVAR(BloodVolumeInfoIndicator)};
class GVAR(BloodVolumeInfoIndicator): RscPictureKeepAspect {
onLoad = QUOTE(uiNamespace setVariable [ARR_2(QQGVAR(BloodVolumeInfoIndicator),_this select 0)]);
Copy link
Member

Choose a reason for hiding this comment

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

Can also be written as QUOTE(with uiNamespace do {GVAR(bloodVolumeIndicator) = _this select 0});. Avoids the need for ARR_.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like it better (at least can read :D). Changed.

addons/medical_feedback/RscInGameUI.hpp Show resolved Hide resolved
addons/medical_feedback/functions/fnc_effectBloodVolumeIcon.sqf Outdated Show resolved Hide resolved
addons/medical_feedback/functions/fnc_effectBloodVolumeIcon.sqf Show resolved Hide resolved
addons/medical_feedback/initSettings.sqf Outdated
};

TRACE_1("bloodVolumeEffect setting - resetting effect",_this);
[false] call FUNC(initEffects);
Copy link
Member

Choose a reason for hiding this comment

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

This refreshes all effects. Could be improved to init only the blood ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made minor fix. Depending on param initEffects init/reset all effects or just pain & blood volume effects.

addons/medical_feedback/stringtable.xml Outdated Show resolved Hide resolved
@10Dozen
Review fixes vol.3
7a185c5 
- Fixed exec code of BV setting
- Removed ctrlCommit from eBVI function
- Changed initEffects function to be able to update pain and BV effects only
- Changed control name from `BloodVolumeInfoIndicator` to `bloodVolumeIndicator`
- Various formatting tweaks (spaces/lines/uppercase/periods)
commy2
commy2 reviewed Feb 3, 2020
View reviewed changes
addons/medical_feedback/functions/fnc_effectBloodVolumeIcon.sqf Outdated
if (!_enable || !GVAR(showBloodVolumeIcon)) exitWith {
if (ctrlText _indicatorCtrl != "") then {
_indicatorCtrl ctrlSetText "";
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you check if text != "" when you set it to "" anyway? Seems like the check is not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If != "" then indicator is displayed and i remove it, if it's empty then no extra ctrlSetText calls. Ofc it's minor thing, but why not?

Copy link
Member

Choose a reason for hiding this comment

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

It's unnecessary, think:

  • If text is empty, check is false, so it stays empty.
  • If text is NOT empty, check is true and it changes to empty.

Both result in empty text, so only setting it to empty without the check gives the same result.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there performance hit in this though?
we already know some ctrl commands are stupidly expensive for no discernable reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@10Dozen
Review fixed vol.4
0833468 
- Remove extra checks from eBVI func
- Restores contol's onLoad script using ARR_2(QQGVAR())
@commy2
Copy link
Contributor

commy2 commented Feb 5, 2020

lgtm

@PabstMirror PabstMirror added this to the 3.13.1 milestone Feb 7, 2020
@PabstMirror PabstMirror added the kind/enhancement Release Notes: **IMPROVED:** label Feb 7, 2020
@jonpas jonpas added kind/feature Release Notes: **ADDED:** and removed kind/enhancement Release Notes: **IMPROVED:** labels Feb 22, 2020
@PabstMirror PabstMirror merged commit b30f023 into acemod:master Feb 22, 2020
@PabstMirror PabstMirror changed the title Adds Icon option for Low blood volume at Medical Feedback Medical Feedback = Adds Icon option for Low blood volume effects Feb 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mharis001 mharis001 mharis001 left review comments

@jokoho48 jokoho48 jokoho48 left review comments

@commy2 commy2 commy2 left review comments

@dedmen dedmen dedmen left review comments

@PabstMirror PabstMirror PabstMirror approved these changes

@jonpas jonpas jonpas approved these changes

Assignees
No one assigned
Labels
kind/feature Release Notes: **ADDED:**
Projects
None yet
Milestone
3.13.1
Development

Successfully merging this pull request may close these issues.
7 participants
@10Dozen @commy2 @dedmen @jokoho48 @jonpas @PabstMirror @mharis001