Skip to content
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

improve 'CBA_fnc_sortNestedArray' #380

Merged
merged 3 commits into from
Jun 16, 2016
Merged

improve 'CBA_fnc_sortNestedArray' #380

merged 3 commits into from
Jun 16, 2016

Conversation

commy2
Copy link
Contributor

@commy2 commy2 commented Jun 14, 2016

  • makes CBA_fnc_sortNestedArray use SQFs native sort command
  • adds optional parameter to reverse sorting order

BIS_fnc_sortBy is using the same method now

@commy2 commy2 added this to the 2.4.2 milestone Jun 14, 2016

{
_array set [_forEachIndex, _helperArray select _forEachIndex select 1];
} forEach _array;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do:

    {
        _array set [_forEachIndex, _x select 1];
    } forEach _helperArray;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_array = _helperArray apply {_x select 1};

Copy link
Contributor

@bux bux Jun 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apply is 1.56, so it won't work with the linux builds that are on 1.54(iirc).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would have been so beautiful.

Copy link
Contributor Author

@commy2 commy2 Jun 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CBA_fnc_sortNestedArray originally modified the array by reference, which means select and apply cannot be used. Even if you reassign the new array to the previous variable, it wouldnt behave the same.

blah = [0];
player setVariable ["blah", _blah];
blah set [0, 1];
player getVariable "blah";
-> 1

vs.

blah = [0];
player setVariable ["blah", _blah];
blah = blah apply {1};
player getVariable "blah";
-> 0

This is why select, apply and arrayIntersect are not as versatile as they could've been...

Copy link
Contributor

@bux bux Jun 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a new array is kind of industry standard though. And I'm happy BIS used that approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function defies the standard, but we should go with bwc always.

_index: integer - sub array item index to be sorted on
_array - Nested array to be sorted <ARRAY>
_index - Sub array item index to be sorted on <NUMBER>
_order - true: ascending, false: descending (optional: true) <BOOLEAN>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the "optional: true" comment mean that 1) it is optional and 2) the default is "true"? In that case,
(optional, default: true) would be easier to understand.

@Killswitch00 Killswitch00 merged commit d609d2a into master Jun 16, 2016
@thojkooi thojkooi deleted the improve_sortNested branch June 16, 2016 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants