-
Notifications
You must be signed in to change notification settings - Fork 734
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
Repair - Improve location of repair interaction points #9580
base: master
Are you sure you want to change the base?
Repair - Improve location of repair interaction points #9580
Conversation
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. |
@@ -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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this naming convention enforced by the engine or is GM gonna be special again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be enforced by the engine https://community.bistudio.com/wiki/Arma_3:_Hitpoints
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
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 |
Added Fixed an error related to full repair of vehicles that return an empty Also finally figured out what was the issue with AH-1Z - there are some hitpoints which have |
Another item to the TODO list to fix in CUP, which they must do. |
@@ -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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if RegEx is faster ? I think it's worth testing. Like this:
if ("rotor" in _hitpoint || "hull" in _hitpoint || "engine" in _hitpoint) then { | |
if (_hitpoint regexMatch ".*?(?:rotor|hull|engine).*+/o") then { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex is 2x faster here, but the performance is in one-thousands of miliseconds, so leaving it as is is probably fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO good enough.
@@ -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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_parentHitpoint = toLower _parentHitpoint; | |
_parentHitpoint = toLowerANSI _parentHitpoint; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optimal tool for the job, if there are only ANSI characters.
Co-authored-by: Jouni Järvinen <rautamiekka@users.noreply.github.com>
Co-authored-by: Jouni Järvinen <rautamiekka@users.noreply.github.com>
Switched to depends hashmap, removed single-parent depends from ignored, removed now unnecessary multi-part names
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? |
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. |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hoo boy, I imagine the performance hit is pretty high here ... What about RegEx ?
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't say anything to that, someone else will have to.
I noticed some of them had complex 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
Co-authored-by: Jouni Järvinen <rautamiekka@users.noreply.github.com>
Changes to variable names for consistency, minor formatting fixes, removal of redundand comments
Added handling of depends with complex parents (e.g. HitEngine I tested RHS, CUP and vanilla, but I don't have any of the CDLCs so those still need checking. |
8436b55
to
075be9a
Compare
|
||
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}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be much simpler to have hasItems
do count _items
instead, less remembering stuff like this needed that way.
When merged this pull request will:
depends
parameter to the name of the hitpoint they depend onIMPORTANT
Component - Add|Fix|Improve|Change|Make|Remove {changes}
.