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

Add fnc_insert #542

Merged
merged 8 commits into from
Jan 7, 2017
Merged

Add fnc_insert #542

merged 8 commits into from
Jan 7, 2017

Conversation

654wak654
Copy link
Contributor

When merged this pull request will:

  • Title

@BaerMitUmlaut
Copy link
Contributor

You need to add it to CfgFunctions too.

@654wak654
Copy link
Contributor Author

:picard:^2

commy2
commy2 previously requested changes Nov 6, 2016
if (_index >= _size || {_index < 0}) exitWith {[]};

_right = _array select [_index, _size];
_array deleteRange [_index, _size];
Copy link
Contributor

@commy2 commy2 Nov 6, 2016

Choose a reason for hiding this comment

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

_array resize _index;

private _size = count _array;
if (_index >= _size || {_index < 0}) exitWith {[]};

_right = _array select [_index, _size];
Copy link
Contributor

Choose a reason for hiding this comment

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

missing private

params ["_array", "_index", "_element"];

private _size = count _array;
if (_index >= _size || {_index < 0}) exitWith {[]};
Copy link
Contributor

Choose a reason for hiding this comment

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

[[], 1, "test"] call CBA_fnc_insert
should report
[nil, "test"]
imo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So just exitWith {} or exitWith {nil}?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the line is needed at all and can be removed.
Maybe keep the < 0 check, but the function doesn't do input validation anyway, so you might as well cut that too.

@commy2
Copy link
Contributor

commy2 commented Nov 6, 2016

If we'd replace _element with an array to insert, all we would have to change is one pushBack to append.
If you wanted to insert a single element, instead of
[_arr, 2, "c"] call CBA_fnc_insert;
you would write:
[_arr, 2, ["c"]] call CBA_fnc_insert;
, but you would still have the option to insert multiple elements at once.

@commy2
Copy link
Contributor

commy2 commented Nov 6, 2016

There exists BIS_fnc_arrayInsert

/************************************************************
    Array Insert
    Author: Andrew Barron, optimised by Killzone_Kid

Parameters: [base array, insert array, index]
Returs: [array]

Inserts the elements of one array into another, at a specified
index.

Neither arrays are touched by reference, a new array is returned.

Example1: [[0,1,2,3,4], ["a","b","c"], 1] call BIS_fnc_arrayInsert
Returns: [0,"a","b","c",1,2,3,4]
Example2: [[1,2],[3,4],5] call BIS_fnc_arrayInsert
Returns: [1,2,<null>,<null>,<null>,3,4]
************************************************************/

/// --- validate general input
#include "..\paramsCheck.inc"
#define arr [[],[],0]
paramsCheck(_this,isEqualTypeParams,arr)

params ["_arr1", "_arr2", "_index"];

if (_index < 0) exitWith {_arr1};

private _size1 = count _arr1;
if (_index >= _size1) then {
    _arr1 = +_arr1;
    _arr1 resize _index;
};

(_arr1 select [0, _index]) + _arr2 + (_arr1 select [_index, _size1])

Differences:

  • has input validation
  • creates new array instead of modifying it
  • slightly different order of parameters

We now have to think about:

  • how does our variant perform compared to the (KK rewritten) BI function?
  • do we need this CBA function (usefull feature vs. bloat?)

@commy2 commy2 dismissed their stale review November 6, 2016 13:30

all changed, but no approve yet

@commy2 commy2 added the Feature label Nov 8, 2016
@commy2
Copy link
Contributor

commy2 commented Jan 7, 2017

@654wak654 Sorry for the delay. Can you add a unit test file for this? Would've to be added here:
https://github.com/CBATeam/CBA_A3/blob/master/addons/arrays/test.sqf

@commy2 commy2 merged commit c4272f1 into CBATeam:master Jan 7, 2017
@commy2 commy2 added this to the 3.2 milestone Jan 7, 2017
@654wak654 654wak654 deleted the fnc-insert branch January 7, 2017 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants