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 GUI - Show warning if tourniquet will interfere with medical menu action #9475

Merged
merged 6 commits into from
Oct 14, 2023

Conversation

amsteadrayle
Copy link
Contributor

When merged this pull request will:

  • Show a warning tooltip if a tourniquet will interfere with a medical menu action

IMPORTANT

  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Development Guidelines are read, understood and applied.
  • Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

@amsteadrayle
Copy link
Contributor Author

tourniquetwarning.mp4

To be clear, this does not prevent or obstruct medical actions. It only adds a tooltip to make the player aware of the situation.

This overlaps with #9474, and the intent is that this tooltip will override the item count tooltip.

@@ -50,6 +50,16 @@ private _shownIndex = 0;
_ctrl ctrlSetPositionY POS_H(1.1 * _shownIndex);
_ctrl ctrlCommit 0;

// Show warning if tourniquet will interfere with action
if (((_category in ["examine", "medication"]) || (_items findIf {"IV" in _x}) > -1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't there an IV category? The _items findIf {"IV" in _x}) > -1 check is too specific imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IVs are in the advanced category, shared with things like stitching and CPR. Checking the item name was the most succinct way I could figure out without having to pass extra information about the actions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Item config properties can be checked instead to determine if it's an IV, but oof...

I don't like the idea of relying on classnames.

@amsteadrayle
Copy link
Contributor Author

I've elected to not add a setting for this. To me, the warning is universal and low-impact enough to just have it always on. If you all disagree, then I'm fine with adding a setting. I just didn't want to go ahead and add yet another setting if it wasn't needed.

(By universal I mean that it's applicable regardless of what settings someone might use, as far as I'm aware.)

@LinkIsGrim
Copy link
Contributor

I'm not sure on this one. It is definitely a UX improvement for casual players, but it feels like giving up one of the finer details that makes the medical system unique (even if it's still there, some of the magic is gone with this being shown).

I think this isn't necessary with the addition of the outline, honestly.

@amsteadrayle
Copy link
Contributor Author

That does makes sense for the people who are interested in engaging with a detailed medical system - making them pay closer attention. But this warning would be a good way to teach new and casual players about one of the more subtle and otherwise invisible mistakes that can be made. Especially since that mistake can directly lead to player death and a generally negative experience.

I'd like it as a setting that's on by default and not global.

@jonpas
Copy link
Member

jonpas commented Oct 12, 2023

If you want to teach a new player something, it could be a one-time thing. But I don't think we have anything else like that in ACE3.

@amsteadrayle
Copy link
Contributor Author

I shouldn't have worded it as teaching. Thinking on it more, it's about having a user interface where, if a button is not going to do what it normally does, the user should be told that.

@Drofseh
Copy link
Contributor

Drofseh commented Oct 13, 2023

Do this and the tooltip for # of items interfere/overlap or anything?

@jonpas
Copy link
Member

jonpas commented Oct 13, 2023

I shouldn't have worded it as teaching. Thinking on it more, it's about having a user interface where, if a button is not going to do what it normally does, the user should be told that.

I understand, but in this case, the user can do it, as you can do it in real life. That button will do just fine what it says it will do. The resulting effects are a different story and have nothing to do with this UX in particular.

@amsteadrayle
Copy link
Contributor Author

Do this and the tooltip for # of items interfere/overlap or anything?

This warning will override the tooltip for # of items.

@amsteadrayle
Copy link
Contributor Author

This should be all set to merge now, whether or not you decide to add it.

addons/medical_gui/functions/fnc_updateActions.sqf Outdated Show resolved Hide resolved
[LSTRING(TourniquetWarning_DisplayName), LSTRING(TourniquetWarning_Description)],
[ELSTRING(medical,Category), LSTRING(SubCategory)],
false,
false
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
false
true

Global

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh really? It seems like something that would be left to the player unless admins specifically care. That's my assumption for UI stuff in general.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh really? It seems like something that would be left to the player unless admins specifically care. That's my assumption for UI stuff in general.

I agree with this. @LinkIsGrim Why would you want to enforce it globally?

Copy link
Contributor

Choose a reason for hiding this comment

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

Given the effect, I'd rather this be really deliberate on when it's enabled.

That being said, vox populi and all that.

Copy link
Contributor

@Drofseh Drofseh Oct 14, 2023

Choose a reason for hiding this comment

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

I don't think this needs to be global. Community admins can force if they want to.

addons/medical_gui/functions/fnc_updateActions.sqf Outdated Show resolved Hide resolved
_ctrl ctrlSetTooltip _countText;

// Show warning if tourniquet will interfere with action
if (GVAR(tourniquetWarning)
&& {(_category in ["examine", "medication"]) || (_items findIf {"IV" in _x}) > -1}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
&& {(_category in ["examine", "medication"]) || (_items findIf {"IV" in _x}) > -1}
&& {(_category in ["examine", "medication"]) || (_items findIf {"IV" in _x}) > -1}

Still don't like the IV check, but anything else requires config lookups/caching items.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really have the means to properly investigate the solution I am proposing, but maybe if

_x params ["_displayName", "_category", "_condition", "_statement", "_items"];
gives the statement defined e.g. here
callbackSuccess = QFUNC(ivBag);
we could use that instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's still specific, but the medical system wasn't designed with this in mind (and I don't see a good way to implement it). It's fine. I think most 3rd party IVs will have "IV" in their classname.

Copy link
Contributor

Choose a reason for hiding this comment

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

If anything, enforcing the " IV" (notice the leading space) name requirement would be 1 way to ensure support with ACE outside config, assuming the 3rd-party dev gives a fuck.

amsteadrayle and others added 2 commits October 14, 2023 00:13
Co-authored-by: Grim <69561145+LinkIsGrim@users.noreply.github.com>
Co-authored-by: Grim <69561145+LinkIsGrim@users.noreply.github.com>
@LinkIsGrim LinkIsGrim added this to the 3.16.0 milestone Oct 14, 2023
@LinkIsGrim LinkIsGrim added the kind/enhancement Release Notes: **IMPROVED:** label Oct 14, 2023
@LinkIsGrim
Copy link
Contributor

we ball²

@LinkIsGrim LinkIsGrim merged commit c54a26c into acemod:master Oct 14, 2023
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.

6 participants