replace PUSH() Macro with pushback #247

Merged
merged 2 commits into from Feb 20, 2016

Projects

None yet

5 participants

@jokoho48
Member

No description provided.

@jokoho48 jokoho48 changed the title from replace PUSH() Macro with pushback from Code to replace PUSH() Macro with pushback Jan 23, 2016
@commy2 commy2 commented on an outdated diff Jan 23, 2016
addons/common/fnc_getGroupIndex.sqf
@@ -42,7 +42,7 @@ _number = [];
// Format of player label is "<groupName>:<groupNumber> <playerName>"
for "_i" from (_groupLabelLen + 1) to ((count _labelArray) - 1) do {
if ((_labelArray select _i) == ASCII_SPACE) exitWith {};
- PUSH(_number,_labelArray select _i);
+ _number pushBack _labelArray select _i;
@commy2
commy2 Jan 23, 2016 Contributor

_number pushBack (_labelArray select _i);

@Killswitch00 Killswitch00 and 2 others commented on an outdated diff Jan 23, 2016
addons/common/XEH_postInit.sqf
@@ -96,7 +96,7 @@ if !(isDedicated) then {
[GVAR(actionList), {
TRACE_3("Inside the code for the hashPair",(vehicle player),GVAR(actionIndexes), _value);
if (!isNil "_value" && typeName(_value) == "ARRAY") then {
- PUSH(GVAR(actionIndexes), (vehicle player) addAction _value)
+ GVAR(actionIndexes) pushBack (vehicle player) addAction _value
@Killswitch00
Killswitch00 Jan 23, 2016 Contributor

Does this need safeguarding with
GVAR(actionIndexes) pushBack ((vehicle player) addAction _value) ?

@esteldunedain
esteldunedain Jan 23, 2016 Contributor

When in doubt its better to add it anyway

@commy2
commy2 Jan 23, 2016 Contributor

Yeah. This is missing the brackets too.
Evaluations of binary commands are done from left to right:
GVAR(actionIndexes) pushBack (vehicle player) addAction _value
->
<index/number> addAction _value
->
Error Number expected object

The brackets around (vehicle player) on the other hand are optional.

Correct and minimalistic:
GVAR(actionIndexes) pushBack (vehicle player addAction _value)

I personally think that it's best to use as few brackets as possbile. Only exception would be
(unaryCommand _var1) binaryCommand _var2, so in the end exactly what killswitch proposed.

@esteldunedain
esteldunedain Jan 23, 2016 Contributor

I personally think that it's best to use as few brackets as possbile

I know you do 😉.

Just to ring the opposite bell, I think it's usefull for the code to be as self-obvious as possible, so reviewers/maintainers don't have to wonder if a certain line is being interpreted correctly or not.

@commy2
commy2 Jan 23, 2016 Contributor

I think it's usefull for the code to be as self-obvious as possible

True. But everyone reviewing SQF should know the basic principles. And too many brackets hurt readability.
That discussion doesn't belong here though. The line is currently wrong and errors out.

@jokoho48
Member

ok change/fixed

@commy2
Contributor
commy2 commented Feb 18, 2016

👍
looks good

@Killswitch00 Killswitch00 merged commit a337a13 into CBATeam:master Feb 20, 2016
@Killswitch00 Killswitch00 added this to the 2.3.1 milestone Feb 20, 2016
@jokoho48 jokoho48 deleted the unknown repository branch Feb 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment