Repair - Improve location of repair interaction points #9580

Ivanowicz · 2023-10-25T15:27:53Z

When merged this pull request will:

Fix Misplaced repair interaction points for helicopter rotors #9558
Add names of hitpoints with depends parameter to the name of the hitpoint they depend on
Expand ignored hipoint list to include armor parts, sideskirts and missing slats

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

Ivanowicz · 2023-10-25T15:29:02Z

The only helicopter I'm still having issues with is the RHS AH-1Z, the hull and tail hitpoints are missing. Those hitpoints have empty selection strings, but mi-8 config is also missisng them and the hitpoints do show up, so I'm currently out of ideas.

LinkIsGrim · 2023-10-25T15:40:24Z

addons/repair/functions/fnc_addRepairActions.sqf

@@ -119,6 +119,10 @@ private _turretPaths = ((fullCrew [_vehicle, "gunner", true]) + (fullCrew [_vehi

        // Find the action position
        private _position = compile format ["_target selectionPosition ['%1', 'HitPoints'];", _selection];
+        if ("rotor" in _hitpoint) then {


is this naming convention enforced by the engine or is GM gonna be special again?

oh, haven't even thought of that - sadly I have no idea and no way to check GM
all the testing I've done was on vanilla and RHS, there it seems that 100% rotors there do have rotor somewhere in the name.

~~Seems to be enforced by the engine~~ https://community.bistudio.com/wiki/Arma_3:_Hitpoints

Nevermind, I think I misread something - running the script from the link shows that just with RHS the hitpoint list grows to 600+, so this will have to be checked for GM (RHS and CUP seem ok here)

LinkIsGrim · 2023-10-25T15:41:49Z

The only helicopter I'm still having issues with is the RHS AH-1Z, the hull and tail hitpoints are missing. Those hitpoints have empty selection strings, but mi-8 config is also missisng them and the hitpoints do show up, so I'm currently out of ideas.

IIRC they use armorComponent

Ivanowicz · 2023-10-25T19:39:41Z

The only helicopter I'm still having issues with is the RHS AH-1Z, the hull and tail hitpoints are missing. Those hitpoints have empty selection strings, but mi-8 config is also missisng them and the hitpoints do show up, so I'm currently out of ideas.

IIRC they use armorComponent

Yup, exactly, that's why it's so weird that Mi-8 works. My best guess is that there's something weird in the config that causes it to skip over the check in fnc_getSelectionsToIgnore.sqf at line 100, but I just can't figure out what.

Ivanowicz · 2023-11-29T16:34:55Z

Added hull and engine to hitpoints using AveragePoint - I feel that those seem to work better in most cases. This can't really be used for all hitpoints, since it causes way more overlapping interaction points than the old method.

Fixed an error related to full repair of vehicles that return an empty getAllHitPointsDamage array - there are a few (less than 10 I think) vehicles like that in CUP. Should we handle them in some way to allow full repair?

Also finally figured out what was the issue with AH-1Z - there are some hitpoints which have depends specified, but the specified hitpoint is ignored (no selection or armorComponent), causing the hitpoint with depends to be unrepairable. In this case it's the tail rotor which can be damaged, but it depends on tail, which is ignored.

rautamiekka · 2023-11-29T17:18:39Z

empty getAllHitPointsDamage array - there are a few (less than 10 I think) vehicles like that in CUP. Should we handle them in some way to allow full repair?

Another item to the TODO list to fix in CUP, which they must do.

addons/repair/functions/fnc_addRepairActions.sqf

rautamiekka · 2023-11-29T18:06:40Z

addons/repair/functions/fnc_addRepairActions.sqf

@@ -119,6 +119,10 @@ private _turretPaths = ((fullCrew [_vehicle, "gunner", true]) + (fullCrew [_vehi

        // Find the action position
        private _position = compile format ["_target selectionPosition ['%1', 'HitPoints'];", _selection];
+        if ("rotor" in _hitpoint || "hull" in _hitpoint || "engine" in _hitpoint) then {


I wonder if RegEx is faster ? I think it's worth testing. Like this:

Suggested change

if ("rotor" in _hitpoint || "hull" in _hitpoint || "engine" in _hitpoint) then {

if (_hitpoint regexMatch ".*?(?:rotor|hull|engine).*+/o") then {

Regex is 2x faster here, but the performance is in one-thousands of miliseconds, so leaving it as is is probably fine.

IMO good enough.

addons/repair/functions/fnc_canFullRepair.sqf

rautamiekka · 2023-11-29T18:06:47Z

addons/repair/functions/fnc_getSelectionsToIgnore.sqf

@@ -128,6 +136,9 @@ private _processedSelections = [];
            ERROR_2("[%1] hitpoint [%2] is both a group-parent and a depends and will be unrepairable",_type,_hitpoint);
            ERROR_1("group: %1",_hitpointGroups # _groupIndex);
        };
+        private _parentHitpoint = getText (_vehCfg >> "HitPoints" >> _hitpoint >> "depends");
+        _parentHitpoint = toLower _parentHitpoint;


Suggested change

_parentHitpoint = toLower _parentHitpoint;

_parentHitpoint = toLowerANSI _parentHitpoint;

Optimal tool for the job, if there are only ANSI characters.

Co-authored-by: Jouni Järvinen <rautamiekka@users.noreply.github.com>

addons/repair/functions/fnc_getSelectionsToIgnore.sqf

addons/repair/functions/fnc_doRepair.sqf

Switched to depends hashmap, removed single-parent depends from ignored, removed now unnecessary multi-part names

Ivanowicz · 2023-12-03T16:19:38Z

The latest commits should solve the problems from the 2 reviews above. Since now depends are properly handled, they don't have to be always added to ignored hitpoints, which also means there's no need to make those multi-hitpoint names.

I noticed some RHS tanks have smokelauncher hitpoints - should they be left repairable?

rautamiekka · 2023-12-03T16:30:57Z

I noticed some RHS tanks have smokelauncher hitpoints - should they be left repairable?

Why not ? TBF I could see rearming doing it simultaneously and it really being useful while rearming (depending on whether rearming is only possible with a rearm vehicle), but it shouldn't hurt having the option regardless. At least it gives modders another way to expand, if anything.

rautamiekka · 2023-12-03T16:49:16Z

addons/repair/functions/fnc_getSelectionsToIgnore.sqf

+    if (_hitpoint select [0,7] isEqualTo "hitera_" || {_hitpoint select [0,4] isEqualTo "era_"}
+    || {_hitpoint select [0,8] isEqualTo "hitslat_"}  || {_hitpoint select [0,5] isEqualTo "slat_"}
+    || {_hitpoint select [0,9] isEqualTo "sideskirt"} || {_hitpoint select [0,6] isEqualTo "armor_"}
+    || {_hitpoint select [0,3] isEqualTo "mud"}) then { // skip era/slat/sideskirt/armor/mudguards


Hoo boy, I imagine the performance hit is pretty high here ... What about RegEx ?

Suggested change

if (_hitpoint select [0,7] isEqualTo "hitera_" || {_hitpoint select [0,4] isEqualTo "era_"}

|| {_hitpoint select [0,8] isEqualTo "hitslat_"} || {_hitpoint select [0,5] isEqualTo "slat_"}

|| {_hitpoint select [0,9] isEqualTo "sideskirt"} || {_hitpoint select [0,6] isEqualTo "armor_"}

|| {_hitpoint select [0,3] isEqualTo "mud"}) then { // skip era/slat/sideskirt/armor/mudguards

if (_hitpoint regexMatch "^(?:(?:(?:hit)?(?:era|slat)|armor)_|sideskirt|mud).*+/o") then { // skip era/slat/sideskirt/armor/mudguards

4x faster with RegEx (0.0004 ms)
Just to make sure - is this the correct way to check performance?
["_hitpoint regexMatch '^(?:(?:(?:hit)?(?:era|slat)|armor)_|sideskirt|mud).*+/o'",[{_hitpoint = "hitpoint_test"}],10000] call BIS_fnc_codePerformance;

I can't say anything to that, someone else will have to.

Ivanowicz · 2023-12-03T17:02:14Z

Why not ?

I noticed some of them had complex depends, so I wanted to make sure if it was worth digging into it.
And now that I look closer, not only do most of them have multiple parents but they also have depends in a chain, e.g.
smoketube_1_hitpoint depends = "era_22_hitpoint + smoketube_2_hitpoint";
smoketube_2_hitpoint depends = "era_22_hitpoint + smoketube_3_hitpoint";

Not sure how to handle that to be honest.

rautamiekka · 2023-12-03T17:48:58Z

Why not ?

I noticed some of them had complex depends, so I wanted to make sure if it was worth digging into it. And now that I look closer, not only do most of them have multiple parents but they also have depends in a chain, e.g. smoketube_1_hitpoint depends = "era_22_hitpoint + smoketube_2_hitpoint"; smoketube_2_hitpoint depends = "era_22_hitpoint + smoketube_3_hitpoint";

Not sure how to handle that to be honest.

Ahhh, fair enough, no wonder, then.

Takes cached data from getSelectionsToIgnore instead of checking for the conditions in setHitPointDamage and normalizeHitPoints

addons/repair/functions/fnc_setHitPointDamage.sqf

addons/repair/functions/fnc_normalizeHitPoints.sqf

addons/repair/functions/fnc_getSelectionsToIgnore.sqf

addons/repair/functions/fnc_doRepair.sqf

Co-authored-by: Jouni Järvinen <rautamiekka@users.noreply.github.com>

Changes to variable names for consistency, minor formatting fixes, removal of redundand comments

Ivanowicz · 2023-12-09T16:30:01Z

Added handling of depends with complex parents (e.g. HitEngine depends = 0.5 * (HitEngine1 + HitEngine2)).
Also did some cleanup (mostly changing hitpoint to hitPoint for consistency), I hope it's not too out of scope for this pull request.

I tested RHS, CUP and vanilla, but I don't have any of the CDLCs so those still need checking.
Here's a script for saving unique depends - parent pairs, so finding any weird ones should be quick.
https://pastebin.com/Q6yMz7Cv

rautamiekka · 2023-12-09T21:30:59Z

addons/repair/functions/fnc_canRepair.sqf


 private _items = _config call FUNC(getRepairItems);
-if (count _items > 0 && {!([_caller, _items] call FUNC(hasItems))}) exitWith {false};
+if (count _items > 0 && {!([_unit, _items] call FUNC(hasItems))}) exitWith {false};


I think it'd be much simpler to have hasItems do count _items instead, less remembering stuff like this needed that way.

Ivanowicz added 3 commits October 25, 2023 15:31

Fix repair point location for rotors

c446469

Add names of dependant hitpoints to parent

aab542a

Add sideskirts and armor to ignored hitpoints

e35b7dc

LinkIsGrim reviewed Oct 25, 2023

View reviewed changes

LinkIsGrim and others added 7 commits November 11, 2023 11:13

Merge remote-tracking branch 'upstream/master' into pr/Ivanowicz/9580

aec6b52

params, whitespace

c92fd14

second verse

bdc83f5

Fix for vehicles returning empty array with getAllHitPointsDamage

011cc6a

Improve repair point location for hull and engine hitpoints

f7bc73e

Fix missing return values

287a8d3

Fix for unrepairable depends hitpoints

bf5c661

rautamiekka suggested changes Nov 29, 2023

View reviewed changes

LinkIsGrim and others added 3 commits November 29, 2023 15:56

Update addons/repair/functions/fnc_addRepairActions.sqf

03dcb5e

Co-authored-by: Jouni Järvinen <rautamiekka@users.noreply.github.com>

Update addons/repair/functions/fnc_canFullRepair.sqf

42e6a9f

Co-authored-by: Jouni Järvinen <rautamiekka@users.noreply.github.com>

Update fnc_canFullRepair.sqf

3594902

LinkIsGrim reviewed Nov 29, 2023

View reviewed changes

addons/repair/functions/fnc_getSelectionsToIgnore.sqf Outdated Show resolved Hide resolved

Update addons/repair/functions/fnc_getSelectionsToIgnore.sqf

d17dbe8

LinkIsGrim reviewed Nov 29, 2023

View reviewed changes

addons/repair/functions/fnc_getSelectionsToIgnore.sqf Outdated Show resolved Hide resolved

LinkIsGrim reviewed Nov 29, 2023

View reviewed changes

addons/repair/functions/fnc_doRepair.sqf Outdated Show resolved Hide resolved

Ivanowicz added 2 commits December 3, 2023 16:19

Switch to depends hashmap, non-ignored depends

8452fe0

Switched to depends hashmap, removed single-parent depends from ignored, removed now unnecessary multi-part names

Adds RHS tank turret mudguards to ignored

99a401d

rautamiekka suggested changes Dec 3, 2023

View reviewed changes

Ivanowicz added 2 commits December 3, 2023 21:13

Small fix

2c6411f

Use cached data in setting damage and normalization

b8e93e2

Takes cached data from getSelectionsToIgnore instead of checking for the conditions in setHitPointDamage and normalizeHitPoints

rautamiekka suggested changes Dec 4, 2023

View reviewed changes

Ivanowicz and others added 3 commits December 4, 2023 19:25

Apply suggestions from code review

debeb1b

Co-authored-by: Jouni Järvinen <rautamiekka@users.noreply.github.com>

Handling of complex depends parents, some cleanup

b551fc1

Code cleanup

773dfb5

Changes to variable names for consistency, minor formatting fixes, removal of redundand comments

Small fixes

075be9a

Ivanowicz force-pushed the improve-repair-hitpoint-location branch from 8436b55 to 075be9a Compare December 9, 2023 16:53

Added handling for depends = "total"

69374de

rautamiekka reviewed Dec 9, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Repair - Improve location of repair interaction points #9580

Repair - Improve location of repair interaction points #9580

Ivanowicz commented Oct 25, 2023 •

edited

Ivanowicz commented Oct 25, 2023

LinkIsGrim Oct 25, 2023

Ivanowicz Oct 25, 2023 •

edited

Ivanowicz Oct 25, 2023 •

edited

Ivanowicz Nov 29, 2023

LinkIsGrim commented Oct 25, 2023

Ivanowicz commented Oct 25, 2023

Ivanowicz commented Nov 29, 2023

rautamiekka commented Nov 29, 2023

rautamiekka Nov 29, 2023

Ivanowicz Dec 3, 2023

rautamiekka Dec 3, 2023

rautamiekka Nov 29, 2023

LinkIsGrim Nov 29, 2023

rautamiekka Dec 1, 2023

Ivanowicz commented Dec 3, 2023

rautamiekka commented Dec 3, 2023

rautamiekka Dec 3, 2023

Ivanowicz Dec 3, 2023

rautamiekka Dec 3, 2023

Ivanowicz commented Dec 3, 2023

rautamiekka commented Dec 3, 2023

Ivanowicz commented Dec 9, 2023

rautamiekka Dec 9, 2023

	if ("rotor" in _hitpoint \|\| "hull" in _hitpoint \|\| "engine" in _hitpoint) then {
	if (_hitpoint regexMatch ".?(?:rotor\|hull\|engine).+/o") then {

	_parentHitpoint = toLower _parentHitpoint;
	_parentHitpoint = toLowerANSI _parentHitpoint;

Repair - Improve location of repair interaction points #9580

Are you sure you want to change the base?

Repair - Improve location of repair interaction points #9580

Conversation

Ivanowicz commented Oct 25, 2023 • edited

IMPORTANT

Ivanowicz commented Oct 25, 2023

Choose a reason for hiding this comment

Ivanowicz Oct 25, 2023 • edited

Choose a reason for hiding this comment

Ivanowicz Oct 25, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LinkIsGrim commented Oct 25, 2023

Ivanowicz commented Oct 25, 2023

Ivanowicz commented Nov 29, 2023

rautamiekka commented Nov 29, 2023

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

Ivanowicz commented Dec 3, 2023

rautamiekka commented Dec 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanowicz commented Dec 3, 2023

rautamiekka commented Dec 3, 2023

Ivanowicz commented Dec 9, 2023

Choose a reason for hiding this comment

Ivanowicz commented Oct 25, 2023 •

edited

Ivanowicz Oct 25, 2023 •

edited

Ivanowicz Oct 25, 2023 •

edited