(repair) - isRepairVehicle handling values according to wiki

[Cuel]

The wiki states that ACE_isRepairVehicle is a boolean but this function only checked for integer values, resulting in a script error for whoever used the wiki value.

When merged this pull request will:

• Make isRepairVehicle handle both types

[jonpas]

Hmh, I'd rather change wiki, that leaves open possibilities for repair levels or something.

[dedmen]

So does adding the numbering back when we want repair levels.
a isRepairVehicle being a boolean makes much more sense. If it was a repair level it should be called differently anyway

[BaerMitUmlaut]

I may be nitpicking here, but why the outer parenthesis? Why the // return comment? It's the last statement, this being the return value should be clear for somebody who knows SQF.
Additionally, right now it's a 160 character line. Please consider splitting it.

Disregarding that, I agree with dedmen. The is prefix implies that it's a boolean and it's fully backwards compatible. I don't see us moving to multiple repair levels this late into A3.

[Cuel]

In CBA //return comments have been popping up here and there (for clarification?) so I figured I'd add it. If that doesn't apply to ace I'll remove it. I just left the parenthesis since it was there earlier.

[commy2]

I do that whenever it's not clear what a return value is. It's debateable here, because on the one hand, this expression is huge, on the other, it is the last line of the script.

What bugs me more about this PR is:

[false, true] select 

[PabstMirror]

in [1, true] might be safer as it can handle any input

[Cuel]

That's what I had at first, with a small edit to cover > 1 numbers
Cuel@74f5b99

[kymckay]

[false, true] select

😱 heresy!

[Cuel]

So would you guys prefer 74f5b99 or something else or current

[kymckay]

Nah it's best as is because the getVariable can return an index. I was just being silly 😉

[Cuel]

Yeah I was talking to everyone who's commenting here 👍

[dedmen]

My preference would be a slightly modified variant of Cuel@74f5b99

private _value = _vehicle getVariable ["ACE_isRepairVehicle", -1];
if (_value == -1) exitWith { getNumber (configFile >> "CfgVehicles" >> typeOf _vehicle >> QGVAR(canRepair)) > 0 };
(_value isEqualTo true || {value > 0})  // return

Shorter and faster.
And also is better than current version. Because it only does the config lookup when necessary instead of always doing it.

[Dystopian]

@dedmen
to handle false in _value
(_value isEqualTo true || {value > 0}) // return
->
if (_value isEqualType true) then {_value} else {value > 0}

[dedmen]

^ That is slower though...
4 command calls vs 6.

[Cuel]

But you'd be checking if {false > 0}

[Cuel]

Any decision?

[kymckay]

Honestly I don't think we need to handle values greater than one for the time being. The variable name doesn't imply that anyway and I really don't foresee a need for us to add it.

If we want to add repair levels in future we can PR it separately. I say keep this code simple in [1, true]

[kymckay]

I hate the ``// return`, but will approve anyway 😄

[Cuel]

:(