Add destroyed boat ejecting #6330

Dystopian · 2018-05-07T21:00:23Z

When merged this pull request will:

~~add destroyed boat ejecting framework;~~
make Rubber Boat, Water Scooter, RHIB eject crew if destroyed.

Some boats like rubber boat or scooter are very easy to destroy even with 1-2 bullets or if they are just collided with other object (especially by engine). It's OK except it kills its crew (Arma feature) even if it's not really possible. This PR adds HandleDamage event handler which ejects crew when boat is destroyed.

This way has at least one defect: ejected crew is not damaged with any projectile type. You can destroy boat with grenade, HE rocket or even explosive charge but crew will survive when eject. From my point of view it's not a big deal in comparison with dying from stone collision but maybe you think otherwise.

~~Config values are safe for 3rd-party mods and can be easily added to them. E.g. CUP has suitable Zodiac and RHIB boats.~~ Suitable CUP boats inherit vanilla boats and are fixed automatically.

I also tested solution with allowDamage command but it has more bugs without any advantages.

addons/vehicles/XEH_postInit.sqf

kymckay · 2018-05-07T21:34:33Z

addons/vehicles/XEH_postInit.sqf

+                    [QGVAR(ejectDestroyed), _vehicle] call CBA_fnc_globalEvent;
+                };
+            }];
+            _vehicle setVariable [QGVAR(ejectDestroyedHDEH), _eh];


You don't need to store this variable, just use _thisEventHandler in the event to remove itself

I need it because HandleDamage is local and doesn't hit on all machines, so I don't have _thisEventHandler value everywhere

🤔 Huh? Of course you have _thisEventHandler everywhere, it's a block of code used in an event handler added with addEventHandler.

Personally I would much rather have some leftover event handlers on some machines which will remove themselves if a dead vehicle ever changes locality and takes a single hit of damage again (which, let's be honest, isn't likely). It will make the code more elegant and remove unnecessary network traffic.

But why do you need it? https://github.com/acemod/ACE3/pull/6330/files#diff-71d8e1c52a77144d92094e89a77b7a55R6
That?
The eventhandler and variables are removed anyway when the vehicle is destroyed aren't they?

I have _thisEventHandler only where vehicle is local. EH is fired only on one machine so I have to clean up EHs on all other machines. I always thought cleanup is necessary. If you insist I can get rid of it, no problem.

The eventhandler and variables are removed anyway when the vehicle is destroyed aren't they?

No, they aren't removed.

They will clean themselves up in the event that a dead vehicle switches locality and receives more damage (again, not likely). It's totally harmless to leave them sitting there doing nothing.

kymckay · 2018-05-07T21:38:02Z

Regarding the defect you mention, we could surely have it check for explosives (etc.) and let the units be killed in those cases.

Also not sure whether this belong here or in cookoff (as it has damage handling for other vehicles which don't actually cook off)

Dystopian · 2018-05-07T21:52:58Z

we could surely have it check for explosives (etc.) and let the units be killed in those cases.

But we don't know how big explosion is, if it's small grenade or big satchel. Also distance is important. ATM I don't think it's worth it to check all of these parameters.

dedmen · 2018-05-08T06:58:45Z

But we don't know how big explosion is, if it's small grenade or big satchel.

HandleDamage get's the amount of damage though. Wouldn't that be enough?

Dystopian · 2018-05-08T20:09:36Z

HandleDamage get's the amount of damage though. Wouldn't that be enough?

Here is HDEH [alive _vehicle, _this] output for 30mm AP (which shouldn't kill crew) and 40mm HEDP (which should do) hits:

"[true,[s,"""",11.7853,p,""B_30mm_APFSDS_Tracer_Green"",-1,p,""""]]"
"[false,[s,""hull"",7.54619,p,""B_30mm_APFSDS_Tracer_Green"",0,p,""hitbody""]]"
"[false,[s,""engine"",0.919199,p,""B_30mm_APFSDS_Tracer_Green"",1,p,""hitengine""]]"

"[true,[s,"""",0.854012,p,""G_40mm_HEDP"",-1,p,""""]]"
"[true,[s,"""",10.1193,p,""G_40mm_HEDP"",-1,p,""""]]"
"[false,[s,""hull"",7.14085,p,""G_40mm_HEDP"",0,p,""hitbody""]]"
"[false,[s,""engine"",1.37552,p,""G_40mm_HEDP"",1,p,""hitengine""]]"

Note AP damage is more than HE damage. So looks like out of luck.

dedmen · 2018-05-09T05:52:37Z

I guessss...... One could look into the ammo config to look for explosiveness and damage

kymckay · 2018-05-09T07:43:32Z

We already do this in cook off so it's nothing new 😛

dedmen · 2018-05-09T08:00:40Z

^ https://github.com/acemod/ACE3/blob/master/addons/cookoff/script_component.hpp#L19

Dystopian · 2018-05-09T12:29:55Z

OK it works.
How do you think about component? If cookoff then config value will be ace_cookoff_ejectDestroyed. Isn't it strange?

kymckay · 2018-05-09T12:39:00Z

Good point, I'm happy to leave it in vehicles 👍

Can see what other maintainers say

dedmen · 2018-05-09T13:03:24Z

If in vehicles then maybe change https://github.com/Dystopian/ACE3/blame/master/addons/vehicles/README.md#L4
to

Various tweaks to vehicles and vehicle weapons.

Cookoff seems a little out of place. This thing is more like fixing a vanilla bug.
Also how about hard coding XEH init EH's in config. Instead of adding config entries and iterating over all vehicles?

Though mods couldn't integrate into it that well or they would need to check if ACE is present in the XEH code. So not sure if worth it. Probably not

Aand another also: https://github.com/acemod/ACE3/pull/6330/files#diff-71d8e1c52a77144d92094e89a77b7a55R15 Eventhandler recompilation. Please don't give me fps drops when getting shot ._.

dedmen · 2018-05-09T13:11:51Z

addons/vehicles/XEH_postInit.sqf

+                    if (0.5 >= getNumber (configFile >> "CfgAmmo" >> _ammo >> "explosive")) then {
+                        {
+                            if (alive _x) then {
+                                moveOut _x;


wasn't there a problem that moveOut didn't work with dead bodies? Or was that fixed recently?

but I don't move out them ^^ if (alive _x).
Or you want to move out them also? I don't think it's logical.

lol duh... Must read. Read good for thinking. 🤕
I was thinking about unconscious units but this already handles that fine. Then my brain moved from unconscious to dead.

dedmen · 2018-05-09T13:15:06Z

addons/vehicles/XEH_postInit.sqf

+            TRACE_2("init ejectDestroyed vehicle",_vehicle,typeOf _vehicle);
+            _vehicle addEventHandler ["HandleDamage", {
+                params ["_vehicle", "", "", "", "_ammo"];
+                if (!alive _vehicle) then {


if (alive _vehicle) exitWith?
get's rid of indentation (readability)
and get's rid of the ! (performance)

Isn't exitWith slower than then?

PabstMirror · 2018-05-09T13:19:28Z

Instead of the config number, scanning and adding xeh
we could just directly add a xeh for Boat_Transport_02_base_F and Rubber_duck_base_F

dedmen · 2018-05-09T13:19:41Z

addons/vehicles/XEH_postInit.sqf

+private _ejectDestroyedClasses = [];
+{
+    private _ejectDestroyedClass = configName _x;
+    if (-1 == _ejectDestroyedClasses findIf {_ejectDestroyedClass isKindOf _x}) then {


Same here. if (-1 !=... ) exitWith to have less indentation.
I'd also add a comment like //Only want to add the EH once.

no, it would exit forEach loop

Also.. doesn't his add a problem?

I don't know if the configClasses output is properly ordered.
But if you add it to the child class first. And after that get to the parent class you might add twice?
Can only happen if configClasses can return child before parent. Which it shouldn't be able to if it linearly traverses the config.

well ACE uses some of such config loops and there were no problems yet AFAIK.

dedmen · 2018-05-09T13:23:23Z

@PabstMirror #6330 (comment) :D
But what about the 3rd party mod accessibility that I mentioned there?

Dystopian · 2018-05-09T13:23:43Z

Instead of the config number, scanning and adding xeh
we could just directly add a xeh for Boat_Transport_02_base_F and Rubber_duck_base_F

But it would prevent us to use it in 3rd-party addons, no? See the 1st post. Or we'll have to add it to compats which is not so good.

PabstMirror · 2018-05-09T15:05:56Z

Current method doesn't check inheritance, e.g. it still runs on this child:

    class C_Boat_Transport_02_F: Boat_Transport_02_base_F {
        GVAR(ejectDestroyed) = 0;
    };

{} forEach ("1 == getNumber (_x >> ""ace_vehicles_ejectDestroyed"")" configClasses (configFile >> "CfgVehicles")); is about 25ms with rhs+cup

I think adding like this will be cleaner and faster.

    class Boat_Transport_02_base_F {
        class GVAR(ejectDestroyed) {
            init = QUOTE(call FUNC(ejectDestroyed));
        };
    };    
    class Rubber_duck_base_F {
        class GVAR(ejectDestroyed) {
            init = QUOTE(call FUNC(ejectDestroyed));
        };
    };

We could even add RHS vehicles directly in here as there is no load order/inheritance problems with xeh.

dedmen · 2018-05-09T19:41:21Z

Do RHS vehicles still have the same issue?

Also please stop recompiling the whole eventhandler code on every shot that hits the boat.

Dystopian · 2018-05-09T22:38:09Z

Current method doesn't check inheritance, e.g. it still runs on this child

It's intended because I didn't find suitable classes to exclude. I didn't want to add unnecessary logic.

I think adding like this will be cleaner and faster

I agree with faster. But I didn't agree with cleaner. Until I have known that CUP boats inherit from Rubber_duck_base_F 😆. So now I don't have real examples of 3rd-party addons which need this PR. If anybody know some or think this PR should rely on config value - say. Otherwise I'll rewrite PR like @PabstMirror says. Also in this case I don't object moving code to cookoff.

Do RHS vehicles still have the same issue?

RHS don't have suitable boats.

Also please stop recompiling the whole eventhandler code on every shot that hits the boat.

will do 👌

dedmen · 2018-05-16T16:50:08Z

Found a solution for the config scan problem.
Do it like ASDG_JR. Just make a global class where everyone enters
classname = 1
That way we only scan over that.

Dystopian · 2018-05-19T22:38:23Z

@dedmen really good solution. Could be used if it's needed.

Dystopian · 2018-05-21T21:47:08Z

hmm working on static weapons - they should be handled another way

Dystopian · 2018-05-24T21:08:17Z

Static weapons don't fire HDEH when become dead and can be alive if _damage == 0.950505. I don't want to mess with them. I think this PR is done.

jonpas · 2019-12-07T21:44:40Z

Needs testing if this still happens with medical, specifically on boats.

PabstMirror · 2019-12-12T23:43:12Z

This was needed on old medical, but now units won't take damage from vehicle deaths.

Pilot doing fine and staying on a sinking ship 😄

We actually need to do work on making vehicle crashes/dying more dangerous to units, keeping in mind to not just always make it lethal

Dystopian · 2019-12-13T10:31:55Z

I don't completely understand why you link unit HD and vehicle HD. This PR is about vehicle HD only. It was made with disabled ACE Medical at all. The purpose was just to eject unit from shot rubber boat.
If you think this is OK:

Pilot doing fine and staying on a sinking ship smile

then I can add somewhat like !IS_MOD_LOADED(medical).

PabstMirror · 2019-12-16T22:29:28Z

Merging now, will keep this in mind while working on #7305

Add destroyed boat ejecting

61e0f7b

kymckay reviewed May 7, 2018

View reviewed changes

addons/vehicles/XEH_postInit.sqf Outdated Show resolved Hide resolved

kymckay reviewed May 7, 2018

View reviewed changes

kymckay added the kind/feature Release Notes: **ADDED:** label May 7, 2018

kymckay added this to the 3.13.0 milestone May 7, 2018

Delete EH global cleanup

6890296

Add check explosiveness

61542ea

dedmen reviewed May 9, 2018

View reviewed changes

Optimize EH

154adb3

Dystopian added 2 commits May 19, 2018 15:10

Move EH to function, remove config values

62444f2

Move code to cookoff

a654275

Use macro for explosiveness

4c61388

Dystopian added 2 commits March 17, 2019 19:08

Merge branch 'my2' into prevent_death_in_boat

cb00702

Fix include

599e2ae

jonpas added the status/needs-testing label Dec 7, 2019

jonpas modified the milestones: 3.13.0, 3.13.1 Dec 7, 2019

Merge branch 'master' into pr/6330

7200560

PabstMirror mentioned this pull request Dec 15, 2019

Medical - Define system for Vehicle Crashes / Explosions #7305

Closed

PabstMirror removed the status/needs-testing label Dec 16, 2019

PabstMirror modified the milestones: 3.13.1, 3.13.0 Dec 16, 2019

PabstMirror approved these changes Dec 16, 2019

View reviewed changes

PabstMirror merged commit 392a077 into acemod:master Dec 16, 2019

PabstMirror modified the milestones: 3.13.0, 3.13.0-temp3 Dec 30, 2019

Dystopian deleted the prevent_death_in_boat branch March 16, 2020 11:12

Add destroyed boat ejecting #6330

Add destroyed boat ejecting #6330

Conversation

Dystopian commented May 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kymckay commented May 7, 2018

Dystopian commented May 7, 2018

dedmen commented May 8, 2018 • edited Loading

Dystopian commented May 8, 2018

dedmen commented May 9, 2018

kymckay commented May 9, 2018

dedmen commented May 9, 2018

Dystopian commented May 9, 2018

kymckay commented May 9, 2018

dedmen commented May 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dedmen May 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PabstMirror commented May 9, 2018

dedmen May 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dedmen commented May 9, 2018

Dystopian commented May 9, 2018

PabstMirror commented May 9, 2018

dedmen commented May 9, 2018

Dystopian commented May 9, 2018

dedmen commented May 16, 2018

Dystopian commented May 19, 2018

Dystopian commented May 21, 2018

Dystopian commented May 24, 2018

jonpas commented Dec 7, 2019

PabstMirror commented Dec 12, 2019

Dystopian commented Dec 13, 2019

PabstMirror commented Dec 16, 2019

Dystopian commented May 7, 2018 •

edited

Loading

dedmen commented May 8, 2018 •

edited

Loading

dedmen commented May 9, 2018 •

edited

Loading

dedmen May 9, 2018 •

edited

Loading

dedmen May 9, 2018 •

edited

Loading