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 CBA_fnc_selectRandomArray #1049

Merged
merged 10 commits into from
Feb 9, 2019
Merged

Conversation

neilzar
Copy link
Contributor

@neilzar neilzar commented Jan 26, 2019

When merged this pull request will:

  • Adds a function that will select a specified amount of elements from an array at random without picking the same element multiple times.

Unsure if the function name is the best option, but I think it might become too long otherwise.

@commy2
Copy link
Contributor

commy2 commented Jan 26, 2019

Atm. this function has inconsistent behaviour about modifying the input array by reference.

private _original = [1,2,3];
private _selected = [_original, 2] call CBA_fnc_selectRandom;

hint str _original; // []

vs.

private _original = [1,2,3];
private _selected = [_original, 10] call CBA_fnc_selectRandom;

hint str _original; // [1,2,3]

I think a function like this should not modify its arguments on the principle of avoiding side effects.

The solution would be to deep copy after the params line:

_array = + _array;

But if the array is itself made up of arrays, then it may be interesting to not copy these sub arrays as well, but to keep their references. This is an edge case, but I personally would expect that behaviour. This could be implemented easily by doing a shallow copy of the input array instead:

_array = [] + _array;

@kymckay
Copy link
Contributor

kymckay commented Jan 26, 2019

@commy2 I think deep vs shallow copy behaviour should be consistent across array functions (i.e. shuffle should also do shallow if that's done here - currently it does deep).

Co-Authored-By: neilzar <neil.evers.1995@gmail.com>
@commy2
Copy link
Contributor

commy2 commented Jan 26, 2019

OK, now all we need is a better name. selectRandom is a command that picks one element at random, so what about CBA_fnc_selectSomeRandom or CBA_fnc_selectRandomArray?

@neilzar
Copy link
Contributor Author

neilzar commented Jan 26, 2019

I think selectSomeRandom is more descriptive. selectRandomArray can still be confused as to what the function does, it could mean that the function selects a random array from an array of arrays, for example.

@neilzar neilzar changed the title Array - selectRandom function Array - selectSome'Random function Jan 30, 2019
@neilzar neilzar changed the title Array - selectSome'Random function Array - selectSomeRandom function Jan 30, 2019
@commy2
Copy link
Contributor

commy2 commented Feb 8, 2019

Anyone else prefers selectRandomArray? Makes me think of inArea vs. inAreaArray, isEqualType vs. isEqualTypeArray, actionKeysNames vs. actionKeysNamesArray etc.
The only command with "Some" is someAmmo.

@commy2 commy2 added the Feature label Feb 8, 2019
@commy2 commy2 added this to the 3.10 milestone Feb 8, 2019
@dedmen
Copy link
Contributor

dedmen commented Feb 8, 2019

selectRandomArray sounds like it would select a random array out of an array of arrays

@commy2
Copy link
Contributor

commy2 commented Feb 8, 2019

It selects an array of elements out of an array as opposed to a single element out of an array. The name fits really well.

@commy2 commy2 changed the title Array - selectSomeRandom function add CBA_fnc_selectRandomArray Feb 8, 2019
@commy2 commy2 merged commit d38acc9 into CBATeam:master Feb 9, 2019
@neilzar neilzar deleted the array-selectrandom branch October 2, 2019 14:59
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

6 participants