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 - Add treatment status events #8385

Merged
merged 2 commits into from Sep 3, 2021
Merged

Medical - Add treatment status events #8385

merged 2 commits into from Sep 3, 2021

Conversation

ghost
Copy link

@ghost ghost commented Aug 16, 2021

Adds

  • ace_medical_treatment_treatmentStartedLocal
  • ace_medical_treatment_treatmentSucceededLocal, and
  • ace_medical_treatment_treatmentFailedLocal
    CBA events. All of them have the same six parameters passed unchanged from the treatment action:
    _medic, _patient, _bodyPart, _classname, _itemUser, _usedItem.

Rationale:
I've been trying to write a ShackTac-style medical extension without forking ACE. When trying to add a "PlayerName is bandaging you"/"You are being assisted" hint, I've been stopped by the following drawbacks in existing ace_medical_treatment_{TREATMENT}Local (e.g. ace_medical_treatment_bandageLocal) events:

  1. They are not a single convenient entry point;
  2. They do not carry a _medic parameter.

Here's an example of someone else trying to do this kind of hint with the existing API. I think this PR would improve the situation.

Existing "ace_treatmentSucceded" event seems grandfathered in from before the medical rewrite, so I'm leaving it alone.

@@ -147,6 +147,8 @@ if (_callbackProgress isEqualTo {}) then {

[_medic, _patient, _bodyPart, _classname, _itemUser, _usedItem] call _callbackStart;

[QGVAR(treatmentStartedLocal), [_medic, _patient, _bodyPart, _classname, _itemUser, _usedItem]] call CBA_fnc_localEvent;
Copy link
Member

Choose a reason for hiding this comment

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

These don't need to have Local at the end of them, that is implied by localEvent and is documented.

It looks like these should also be simplified to ace_treatmentStarted rather than ace_medical_treatment_treatmentStartedLocal, but I'd let someone else confirm that

Copy link
Author

@ghost ghost Aug 17, 2021

Choose a reason for hiding this comment

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

Removed the Local tags.

Quite a lot of events in ACE are QGVAR. Is this convention supposed to be internal then? If yes, I could append the arguments to ace_treatmentSucceded instead of making a new event.

Copy link
Author

Choose a reason for hiding this comment

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

  • Updated ace_treatmentSucceded with _itemUser and _usedItem parameters;
  • Renamed other two events to ace_treatmentStarted and ace_treatmentFailed

Copy link
Member

Choose a reason for hiding this comment

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

Yes I believe we decided at some point that QGVAR events for internal only and public events can just use ace_, probably not documented anywhere though

@jonpas jonpas added this to the 3.14.0 milestone Aug 16, 2021
Copy link
Contributor

@PabstMirror PabstMirror left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Release Notes: **ADDED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants