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 Engine - Remove ACE_HDBracket hitpoint #9732

Merged
merged 21 commits into from
Jun 11, 2024
Merged

Medical Engine - Remove ACE_HDBracket hitpoint #9732

merged 21 commits into from
Jun 11, 2024

Conversation

LinkIsGrim
Copy link
Contributor

@LinkIsGrim LinkIsGrim commented Jan 14, 2024 •
edited
Loading

When merged this pull request will:

  • Arma 2.16 adds "context" param to HD which lets us know if we're at the last iteration of the event: https://community.bistudio.com/wiki/Arma_3:_Event_Handlers#HandleDamage
  • Some stuff had to be moved around in the function but it'll work the same.
  • While this means that medical will still work even with broken inheritance now, arm/leg hitpoints could still be missing due to the same issue. I've added a check for presence of those hitpoints, but it's slower than just checking HDBracket by about 25% (0.05ms for 1k iterations vs. 0.04ms). This is only relevant at unit init, and time is saved during damage events (which is where performance suffers most) by reducing a hitpoint.

I'm looking into whether it's possible to simplify more of the function via other events or the directHit param.

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

@LinkIsGrim LinkIsGrim added the target/next-arma Requires something in arma dev branch label Jan 14, 2024
@LinkIsGrim LinkIsGrim marked this pull request as draft January 14, 2024 16:50
@LinkIsGrim LinkIsGrim mentioned this pull request Jan 14, 2024
Merged
@veteran29 veteran29 added the kind/enhancement Release Notes: **IMPROVED:** label Jan 14, 2024
veteran29
veteran29 previously requested changes Jan 14, 2024
View reviewed changes
addons/medical_engine/XEH_postInit.sqf Show resolved Hide resolved
@PabstMirror PabstMirror changed the title Medical Engine - Remove ACE_HDBracket hitpoint Medical Engine - Remove ACE_HDBracket hitpoint [2.16] Jan 16, 2024
@johnb432 johnb432 added the 2.16 label Feb 3, 2024
@johnb432
Copy link
Contributor

johnb432 commented Feb 8, 2024

Looks like this has been pushed to 2.18
https://community.bistudio.com/wiki?title=Arma_3:_Event_Handlers#HandleDamage

@johnb432 johnb432 added 2.18 and removed 2.16 labels Feb 10, 2024
@johnb432 johnb432 changed the title Medical Engine - Remove ACE_HDBracket hitpoint [2.16] Medical Engine - Remove ACE_HDBracket hitpoint [2.18] Feb 10, 2024
@LinkIsGrim
Copy link
Contributor Author

Check if this made it into 2.16 or not. SPOTREP says it did.

@LinkIsGrim LinkIsGrim removed target/next-arma Requires something in arma dev branch 2.18 labels Mar 26, 2024
@LinkIsGrim
Copy link
Contributor Author

Made it into 2.16. Just need to fix merge conflicts

@LinkIsGrim LinkIsGrim marked this pull request as ready for review March 26, 2024 14:15
@LinkIsGrim LinkIsGrim requested a review from veteran29 March 26, 2024 14:15
LinkIsGrim
LinkIsGrim commented Mar 26, 2024
View reviewed changes
addons/medical_engine/XEH_preInit.sqf Outdated Show resolved Hide resolved
addons/medical_engine/XEH_postInit.sqf Outdated Show resolved Hide resolved
addons/medical_engine/script_component.hpp Outdated Show resolved Hide resolved
@johnb432
Copy link
Contributor

I was not expecting this at all and thought we had more time for 2.16.

@LinkIsGrim
Copy link
Contributor Author

Eh, it'll work.

@LinkIsGrim LinkIsGrim added this to the 3.17.1 milestone Mar 26, 2024
@jonpas
Copy link
Member

jonpas commented Mar 28, 2024 •
edited
Loading

Don't we need to include this in 3.17.0 for 2.16 compatibility?

@LinkIsGrim
Copy link
Contributor Author

Nothing breaks AFAIK

@johnb432
Copy link
Contributor

johnb432 commented Apr 1, 2024

Doesn't seem like anything breaks, at least for "normal" damage (being shot, being driven over and falling). I didn't test very much, I feel more in depth testing could be better done by an RC.

@LinkIsGrim LinkIsGrim dismissed veteran29’s stale review April 4, 2024 11:08

Stale, handled

@LinkIsGrim LinkIsGrim modified the milestones: 3.17.1, 3.18.0 Apr 7, 2024
@johnb432 johnb432 self-requested a review May 26, 2024 08:36
johnb432
johnb432 requested changes Jun 2, 2024
View reviewed changes
Copy link
Contributor

@johnb432 johnb432 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested being shot, falling, being driven over, crashing and drowning - all work as expected.

addons/medical_engine/XEH_postInit.sqf Outdated Show resolved Hide resolved
@johnb432 johnb432 self-requested a review June 3, 2024 09:07
johnb432
johnb432 approved these changes Jun 6, 2024
View reviewed changes
Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

LGTM, testing suggests this is fine.

@Mike-MF
Copy link
Member

Mike-MF commented Jun 7, 2024

I played around with this for about an hour or so, tried a bunch of things to cause different types of injury, no obvious issues or errors.

Only change this PR needs to address now is this:

ACE3/addons/medical/XEH_preStart.sqf

Line 5 in 565d64d

[false] call compile preprocessFileLineNumbers QPATHTOF(dev\test_hitpointConfigs.sqf);

@LinkIsGrim
Copy link
Contributor Author

Green button go brrt (flood my inbox if something breaks)

@LinkIsGrim LinkIsGrim merged commit a85074a into master Jun 11, 2024
5 checks passed
@LinkIsGrim LinkIsGrim deleted the medical-noHDbracket branch June 11, 2024 14:11
blake8090 pushed a commit to blake8090/ACE3 that referenced this pull request Aug 18, 2024
@LinkIsGrim @rautamiekka
@johnb432 @blake8090
Medical Engine - Remove ACE_HDBracket hitpoint (acemod#9732)
f544b8d 
* Medical Engine - Remove ACE_HDBracket hitpoint

* skip context 4

* move checks to vars

* skip uav/logic entities

* add check for arm/leg hitpoints

* Update addons/medical_engine/functions/fnc_handleDamage.sqf

Co-authored-by: Jouni Järvinen <[email protected]>

* don't skip context 2

* clearer custom hitpoint array name

* reenable compile cache

* remove debug mode

* lazy eval

* whitespace

* update comment

* Update fnc_handleDamage.sqf header

* Update addons/medical_engine/functions/fnc_handleDamage.sqf

Co-authored-by: johnb432 <[email protected]>

* Update addons/medical_engine/XEH_postInit.sqf

Co-authored-by: johnb432 <[email protected]>

* update hitpoint test

* missing "

* Update addons/medical/dev/test_hitpointConfigs.sqf

Co-authored-by: johnb432 <[email protected]>

---------

Co-authored-by: Jouni Järvinen <[email protected]>
Co-authored-by: johnb432 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johnb432 johnb432 johnb432 approved these changes

@rautamiekka rautamiekka rautamiekka requested changes

@Mike-MF Mike-MF Mike-MF approved these changes

@veteran29 veteran29 Awaiting requested review from veteran29

Assignees
No one assigned
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Milestone
3.18.0
Development

Successfully merging this pull request may close these issues.
6 participants
@LinkIsGrim @johnb432 @jonpas @Mike-MF @rautamiekka @veteran29