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

update CBA_fnc_uniqueUnitItems #1157

Merged
merged 2 commits into from
Jun 23, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 58 additions & 19 deletions addons/common/fnc_uniqueUnitItems.sqf
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ Description:
Parameters:
_unit - Unit to retrieve the items from
_weaponItems - Include weapons, attachments, loaded magazines (Default: false)
_backpack - Include items in backpack (Default: true)
_vest - Include items in vest (Default: true)
_uniform - Include items in uniform (Default: true)
_assignedItems - Include assigned items (Default: true)
_backpack - Include items in backpack (Default: true)
_vest - Include items in vest (Default: true)
_uniform - Include items in uniform (Default: true)
_assignedItems - Include assigned items (Default: true)
_magazines - Include not loaded magazines (Default: false)

Example:
(begin example)
Expand All @@ -22,28 +23,66 @@ Returns:
Array of item classnames <ARRAY>

Author:
Dedmen
Dedmen, commy2
---------------------------------------------------------------------------- */
SCRIPT(uniqueUnitItems);

params [["_unit", objNull, [objNull]], ["_weaponItems", false, [true]], ["_backpack", true, [true]], ["_vest", true, [true]], ["_uniform", true, [true]], ["_assignedItems", true, [true]]];
params [
["_unit", objNull, [objNull]],
["_weaponItems", false, [false]],
["_backpack", true, [false]],
["_vest", true, [false]],
["_uniform", true, [false]],
["_assignedItems", true, [false]],
["_magazines", false, [false]]
];

private _allItems = if (_assignedItems) then {assignedItems _unit} else {[]};
if (_uniform) then {_allItems append ((getItemCargo (uniformContainer _unit)) select 0)};
if (_vest) then {_allItems append ((getItemCargo (vestContainer _unit)) select 0)};
if (_backpack) then {_allItems append ((getItemCargo (backpackContainer _unit)) select 0)};

if (_uniform) then {
_allItems append (getItemCargo uniformContainer _unit select 0);
};

if (_vest) then {
_allItems append (getItemCargo vestContainer _unit select 0);
};

if (_backpack) then {
_allItems append (getItemCargo backpackContainer _unit select 0);
};

if (_weaponItems) then {
_allItems append (primaryWeaponItems _unit);
_allItems append (secondaryWeaponItems _unit);
_allItems append (handgunItems _unit);
_allItems append (primaryWeaponMagazine _unit);
_allItems append (secondaryWeaponMagazine _unit);
_allItems append (handgunMagazine _unit);
_allItems append [
primaryWeapon _unit, secondaryWeapon _unit, handgunWeapon _unit
];
_allItems pushBack (_unit call CBA_fnc_binocularMagazine);
primaryWeapon _unit,
secondaryWeapon _unit,
handgunWeapon _unit // binocular covered by assignedItems
];

_allItems append primaryWeaponItems _unit;
_allItems append secondaryWeaponItems _unit;
_allItems append handgunItems _unit;

if (_magazines) then {
_allItems append primaryWeaponMagazine _unit;
_allItems append secondaryWeaponMagazine _unit;
_allItems append handgunMagazine _unit;
_allItems pushBack (_unit call CBA_fnc_binocularMagazine);
};

_allItems = _allItems - [""];
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be cheaper to do it at the end, after duplicate removal.
because - iterates over everything and copies everything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or if we know where it comes from (I guess primary/secondary/handgunWeapon) we can do it right there.
Would be cheaper to do in L58 for just the 3 things, than do it for everything.

Copy link
Contributor Author

@commy2 commy2 Jun 11, 2019

Choose a reason for hiding this comment

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

It comes from p/s/hWeapon as well as p/s/hWeaponItems. The magazine commands report empty arrays if no mag is loaded. I put the array subtraction below the last line that may add empty strings. CBA_fnc_binocularMagazine reports empty string too.

The placement is so that the array subtraction command is only executed when the _weaponItems flag is set, as this is the only place where the commands may add empty strings. Note that while p/s/hWeaponItems has the items in order muzzle/pointer/optics/bipod and fills the gaps with empty strings, assignedItems, while having an order, just changes in array size (and indices) if a specific slot is unoccupied.

If I were to add it to the last line after the arrayIntersect command instead, I would slow down the function for every use even when this flag is set to false. I know that a smaller array would speed up the function slightly, but this would mean adding an additional binary command (-) for all uses for a small improvement in C++ that itself wouldn't even be a problem in all other use cases. The priority is clear to me.

The only potential optimization here would be moving the weapon(items/magazines) block above the uniform/vest/backpack block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another potential optimization approach would be creating the initial array as [""] and then using pushBackUnique everywhere. And then using deleteAt 0 at the end. However, this would need appendUnique, because one source of empty strings are arrays here.

Copy link
Contributor

@dedmen dedmen Jun 11, 2019

Choose a reason for hiding this comment

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

Maybe instead store _weaponItems seperately, then remove "" from it.
And only then append it to _allItems. That way you don't have to iterate over uniform/vest/backpack items to remove ""

and then using pushBackUnique everywhere

Nah not with the number of appends we have.

Copy link
Contributor

Choose a reason for hiding this comment

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

Make a new array here
https://github.com/CBATeam/CBA_A3/pull/1157/files#diff-5281788de7c6251ec13f252782a5e72fR54
And then _allItems append _newArray after removing the ""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would mean more commands in total. I don't think there will be any case where this actually would be faster.

Copy link
Contributor

Choose a reason for hiding this comment

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

but only in the weaponsItems case
which I'd say is rare.

};

if (_magazines) then {
if (_uniform) then {
_allItems append (getMagazineCargo uniformContainer _unit select 0);
};

if (_vest) then {
_allItems append (getMagazineCargo vestContainer _unit select 0);
};

if (_backpack) then {
_allItems append (getMagazineCargo backpackContainer _unit select 0);
};
};

_allItems arrayIntersect _allItems //Remove duplicates