Dragging - Code cleanup

[johnb432]

When merged this pull request will:

• The code cleanup part of Dragging - Corpse carrying and dragging - continued #9269.

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]

This will have merge conflicts with #9225. Gonna try and finish that first.

[LinkIsGrim]

Suggested change

	if (!alive _player) exitWith {false};
	if !(alive _player) exitWith {false};

Edit: Disregard, see below.

[BrettMayson]

This is the opposite of the ACE Coding Guidelines (although I know both are used in the code) inconsistently

https://ace3.acemod.org/wiki/development/coding-guidelines.html#55-brackets-around-code

[johnb432]

My rule of thumb is: If it's one statement that is easily readable (such as the above), I add it inside the brackets.

But then we have lines such as these. I'm not sure how to interpret the ACE guidelines here - are these fine or should these types of statements be corrected?

[LinkIsGrim]

I'm not of fan of ==/!= instead of isEqualTo/isNotEqualTo in conditions and PFHs, though.

[PabstMirror]

sqf_linter will use these hints to reduce false positives
tool hasn't been updated in a long time so it's not that useful
but going forward there really isn't any reason to remove these lines

[johnb432]

Should I add them back?

[johnb432]

I'm not of fan of ==/!= instead of isEqualTo/isNotEqualTo in conditions and PFHs, though.

The reason is readability for me.
isEqualTo might be less prone to throwing errors, but it will fail just as much as == if the comparison contains a variable that is nil, which, I'd argue, is the most common type of problem.

I can change it though, it's not a problem.

[rautamiekka]

if the comparison contains a variable that is nil

That's why isNil, documentation & testing, exists. It ain't iET's fault.

[johnb432]

It ain't iET's fault.

Never claimed it to be...?
I was merely pointing out that both operators/commands were limited in the same way when it comes to variables that are nil.

[PabstMirror]

probably won't be an issue, but iet also handles nulls a little differently

_unit = objNull;
(vehicle _unit) == _unit; // false
(vehicle _unit) isEqualTo _unit; // true

[LinkIsGrim]

I'm not of fan of ==/!= instead of isEqualTo/isNotEqualTo in conditions and PFHs, though.

The reason is readability for me. isEqualTo might be less prone to throwing errors, but it will fail just as much as == if the comparison contains a variable that is nil, which, I'd argue, is the most common type of problem.

I can change it though, it's not a problem.

I'm just a fan of saving nanoseconds in things that run every frame.

[johnb432]

I'm just a fan of saving nanoseconds in things that run every frame.

Understandable.

Any further opinions on the matter? isEqualTo seems to be favored atm, will change to that in a couple of days if nobody disagrees.

[jonpas]

isEqualTo is so minisculy faster it is pointless to discuss. Just use what is more appropriate in this case. Comparing numbers / same types, == is completely fine.

[LinkIsGrim]

== it is!