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

Fix environmental damage sources #6515

Merged
merged 5 commits into from
Aug 15, 2018
Merged

Fix environmental damage sources #6515

merged 5 commits into from
Aug 15, 2018

Conversation

kymckay
Copy link
Member

@kymckay kymckay commented Aug 11, 2018

When merged this pull request will:

  • Tidy, optimise and comment handle damage code.
  • Fix some environmental damage conditions:
    • There was no sanity check on fall damage to see if the unit was in a vehicle to avoid weird edge cases. Outdated, we now first check for a _shooter to handle this.
    • Vehicle crashes would never trigger because they don't fire the EH for all hitpoints like most damage sources do.
    • Being hit by a vehicle would work (unintentionally), but was not explicitly checked for - which is now done by checking if a shooter exists. Outdated, we now only check if a shooter exists to know collision damage has occurred, it's irrelevant if a vehicle caused it.

I tested all of this in the editor and can confirm the added traces for each damage source now fire when expected.

Point of interest: as you can see in the comments, vehicle collisions do fire the EH multiple times and that seems to scale to crash intensity so I think we can just let it happen and avoid complicated code to work around that.

- Vehicle damage would never occur because it doesn't fire all the
hitpoints
- Missing sanity check on fall damage to check that unit isn't in a
vehicle
- Getting hit by a vehicle while on foot can be detected by checking for
a shooter
@kymckay kymckay added kind/bug-fix Release Notes: **FIXED:** kind/optimization Release Notes: **IMPROVED:** labels Aug 11, 2018
@kymckay kymckay added this to the Medical Rewrite milestone Aug 11, 2018
@kymckay kymckay requested a review from thojkooi August 11, 2018 12:27
@kymckay kymckay changed the title Rw handle damage Fix environmental damage sources Aug 11, 2018
@kymckay kymckay requested a review from TheMagnetar August 12, 2018 09:15
if (_ammo isEqualTo "") then {
if (velocity _unit select 2 < -2) then {
// Downward velocity indicates falling damage
if ((vehicle _unit == _unit) && {velocity _unit select 2 < -2}) then {
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 I actually need to swap the fall damage check with the hit by vehicle check (to handle the extreme edge case of being hit by a plane/helicopter while skydiving).

Copy link
Member

Choose a reason for hiding this comment

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

This is more a suggestion for code legibility. I like putting brackets to separate commands, etc...: {((velocity _unit) # 2) < -2 }

We can use also vehicle _unit isEqualTo _unit

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 quickest way to check if a unit is on foot is actually _unit in _unit, we should probably have added a macro for that to CBA ages ago

Copy link
Contributor

Choose a reason for hiding this comment

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

OBJECT in OBJECT is pretty slow.

Copy link
Member Author

@kymckay kymckay Aug 14, 2018

Choose a reason for hiding this comment

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

Oh really? I figured because it was one less command it'd be faster 🤷‍♂️

Still, an IN_VEHICLE(unit) (alternatively ON_FOOT) macro would improve code readability imo (it's more clear what's being checked).

private _vehicle = vehicle _unit;
if (
_hitPoint isEqualTo "#structural" &&
_ammo isEqualTo "" &&
Copy link
Member

@TheMagnetar TheMagnetar Aug 12, 2018

Choose a reason for hiding this comment

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

lazy evaluation also on the second condition {_ammo isEqualTo ""} &&

if (_ammo isEqualTo "") then {
if (velocity _unit select 2 < -2) then {
// Downward velocity indicates falling damage
if ((vehicle _unit == _unit) && {velocity _unit select 2 < -2}) then {
Copy link
Member

Choose a reason for hiding this comment

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

This is more a suggestion for code legibility. I like putting brackets to separate commands, etc...: {((velocity _unit) # 2) < -2 }

We can use also vehicle _unit isEqualTo _unit

- Correctly identify a piloted vehicle hitting a falling unit
- Correctly identify burning damage while falling
@kymckay
Copy link
Member Author

kymckay commented Aug 13, 2018

Did some more testing and debugging and have updated the code to better handle situations where damage could be classified incorrectly (burning while falling and hit by vehicle while falling)

// Significant damage suggests high relative velocity
// Momentum transfers to body/head for worse wounding
if (_receivedDamage > 0.35) then {
_woundedHitPoint = selectRandom ["Body", "Head"];
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 feel like we should actually raise an additional wounding event in these cases (if you hit the ground at high speed the injury wouldn't fully transfer to your body - you'd still break your legs).

@kymckay
Copy link
Member Author

kymckay commented Aug 14, 2018

So we can probably merge this now, but it definitely needs testing in multiplayer environment (in particular vehicles crashing with multiple players inside - I'm confident everything other than this will behave the same as in singleplayer).

@thojkooi
Copy link
Contributor

thojkooi commented Aug 14, 2018

We need to do various test sessions in MP anyway, better we merge this in now, so we can combine that effort.

Anything in particular that makes you say we need to pay attention to those scenario's specifically in MP, @SilentSpike ?

@kymckay
Copy link
Member Author

kymckay commented Aug 14, 2018

Yeah just because a vehicle crashing is the only kind of damage where I could see potential for locality to introduce a change in behaviour. If you're not the driver then the vehicle isn't local to you (I believe) so it's possible that the handle damage EH could fire differently (e.g. maybe it does fire for all hitpoints and not just structural) for such passengers.

ShackTac have also reported that they've observed issues with passengers not taking damage when vehicles crash (example being a helicopter crash) in the dedicated environment - admittedly that's on the old code which in my testing never correctly identified vehicle crashes at all.

@kymckay kymckay merged commit 11547d7 into medical-rewrite Aug 15, 2018
@kymckay kymckay deleted the rw-handle-damage branch August 15, 2018 06:57
@dedmen
Copy link
Contributor

dedmen commented Aug 15, 2018

If you're not the driver then the vehicle isn't local to you (I believe)

correct. Except the driver is a local AI.

BaerMitUmlaut pushed a commit that referenced this pull request Aug 5, 2019
* Tidy handle damage code
* Fix environmental damage conditions

- Vehicle damage would never occur because it doesn't fire all the
hitpoints
- Missing sanity check on fall damage to check that unit isn't in a
vehicle
- Getting hit by a vehicle while on foot can be detected by checking for
a shooter

* Improve collision handling

- All collisions have a `_shooter` so we simply first check for that because burning doesn't and it could happen at any velocity
- We don't actually care what caused the collision because they should all cause the same type of wounding
- The exception is fall damage because we want to prioritise wounding the legs, we assume if the `_unit` had downward velocity and `_shooter == _unit` that it was fall damage (not necessarily always true, but almost always is and there doesn't look to be a more specific way to check).
@PabstMirror PabstMirror modified the milestones: Medical Rewrite, 3.13.0 Dec 30, 2019
@PabstMirror PabstMirror modified the milestones: 3.13.0, 3.13.0-temp3 Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug-fix Release Notes: **FIXED:** kind/optimization Release Notes: **IMPROVED:**
Projects
Development

Successfully merging this pull request may close these issues.

6 participants