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_uniqueUnitItems #902

Merged
merged 6 commits into from
Apr 15, 2018
Merged

Conversation

dedmen
Copy link
Contributor

@dedmen dedmen commented Mar 19, 2018

Default settings return the same as items _unit but without duplicates.

iterating over items unit like in
https://github.com/acemod/ACE3/blob/8548ff7eb8fd055c5013225b6acb620c753892a4/addons/tagging/functions/fnc_addTagActions.sqf#L36

_requiredItem in ((items _unit) apply {toLower _x})

Can be very expensive if you are just searching for whether the player has one item. Because it returns all items and multiple items as multiple strings.

Was a total perf killer in TFAR when someone had a backpack with ACE medical supplies. I'm sure this can be useful for many things.

Also made it configurable as you might want to exclude backpack or include weapon items.

I don't know how I could make unit tests for this. And I also don't quite like the current Name. My creativity has it's limits.

@Dystopian
Copy link
Contributor

maybe getUniqueItems because not only players but any units?

jonpas
jonpas previously requested changes Mar 19, 2018
Copy link
Member

@jonpas jonpas left a comment

Choose a reason for hiding this comment

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

Rename to getUniqueItems or getUnitUniqueItems (former could work on boxes too if that makes sense).

_allItems append (primaryWeaponItems _unit);
_allItems append (secondaryWeaponItems _unit);
_allItems append (handgunItems _unit);
_allItems append [ primaryWeapon _unit, secondaryWeapon _unit, handgunWeapon _unit,
Copy link
Member

Choose a reason for hiding this comment

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

Array contents to new line, one indent right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

    _allItems append [
                        primaryWeapon _unit, secondaryWeapon _unit, handgunWeapon _unit,
                        primaryWeaponMagazine _unit, secondaryWeaponMagazine _unit, handgunMagazine _unit
                     ];

Or

    _allItems append [
    primaryWeapon _unit, secondaryWeapon _unit, handgunWeapon _unit,
    primaryWeaponMagazine _unit, secondaryWeaponMagazine _unit, handgunMagazine _unit
                     ];

? I'm confused now.

Copy link
Member

Choose a reason for hiding this comment

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

_allItems append [
    primaryWeapon _unit, secondaryWeapon _unit, handgunWeapon _unit,
    primaryWeaponMagazine _unit, secondaryWeaponMagazine _unit, handgunMagazine _unit
];

_allItems append (handgunItems _unit);
_allItems append [ primaryWeapon _unit, secondaryWeapon _unit, handgunWeapon _unit,
primaryWeaponMagazine _unit, secondaryWeaponMagazine _unit, handgunMagazine _unit
];
Copy link
Member

Choose a reason for hiding this comment

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

Same indent as _allItems append ....

_allItems append (secondaryWeaponItems _unit);
_allItems append (handgunItems _unit);
_allItems append [ primaryWeapon _unit, secondaryWeapon _unit, handgunWeapon _unit,
primaryWeaponMagazine _unit, secondaryWeaponMagazine _unit, handgunMagazine _unit
Copy link
Member

Choose a reason for hiding this comment

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

Only one indent right.

@jonpas jonpas added this to the 3.7 milestone Mar 19, 2018
@jonpas
Copy link
Member

jonpas commented Mar 19, 2018

Was a total perf killer in TFAR when someone had a backpack with ACE medical supplies. I'm sure this can be useful for many things.

Same thing in ACRE, will be beneficial.

@Soldia1138
Copy link

Great idea. So maybe this can be a starter to offer a unique ID framework within CBA?
There are quite a few use cases where a unique ID are beneficial.

@TheMagnetar
Copy link

TheMagnetar commented Mar 19, 2018

Same thing in ACRE, will be beneficial.

And ACE BFT as well

@commy2
Copy link
Contributor

commy2 commented Mar 19, 2018

How does this compare to:

params [["_unit", objNull, [objNull]];

private _items = items _unit;
_items arrayIntersect _items // return

?

@commy2
Copy link
Contributor

commy2 commented Mar 19, 2018

Naming sheme:

  • the "get" prefix may or may not be optional
  • "player" vs. "unit": this works on all units (persons), so "unit" instead of "player"
  • pushBack, pushBackUnique: the "unique" qualifier is used as suffix
  • I'd try to make it a 3 word compound if possible (easier to remember)

So my suggestions are:

CBA_fnc_getUnitItemsUnique
CBA_fnc_unitItemsUnique
CBA_fnc_getItemsUnique
CBA_fnc_itemsUnique

@commy2
Copy link
Contributor

commy2 commented Mar 19, 2018

This function does not report the binocularMagazine as far as I can tell.

Function: CBA_fnc_getUniquePlayerItems

Description:
A function used to retrieve a unique list of items in the units inventory
Copy link
Contributor

Choose a reason for hiding this comment

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

A function used to [R]etrieve[s] a unique list of items in the units inventory[.]

@dedmen
Copy link
Contributor Author

dedmen commented Mar 19, 2018

How does this compare to:

In default settings aka no weapon items it should be the same. I used it in TFAR as a replacement for exactly that.

@Soldia1138

So maybe this can be a starter to offer a unique ID framework within CBA?

No. Not at all.

To the names
https://github.com/dedmen/CBA_A3/blob/b2aece723d3385e7a1d106c115ff284fecb7fb0c/addons/common/CfgFunctions.hpp#L62-L74
Also doesn't have get prefix so I leave that away.
I prefer to keep the unit. Because this doesn't work for vehicles.

@Cuel
Copy link
Contributor

Cuel commented Mar 19, 2018

CBA_fnc_uniqueUnitItems in my humble opinion, more like a sentence.

@commy2
Copy link
Contributor

commy2 commented Mar 19, 2018

CBA_fnc_itemsUnique personally. Merger of items and pbUnique.

@dedmen
Copy link
Contributor Author

dedmen commented Mar 19, 2018

@commy2 In the end you decide anyway. Want me to just go for your variant or want me to keep arguing that it's so ambiguous because the "unit" is missing.

@commy2
Copy link
Contributor

commy2 commented Mar 19, 2018

It's not "arguing", but throwing out ideas. Sadly you only have one shot and are stuck with the function name later, because bwc.

@dedmen
Copy link
Contributor Author

dedmen commented Mar 19, 2018

Let's vote?
CBA_fnc_itemsUnique 😄
CBA_fnc_uniqueUnitItems 🎉
CBA_fnc_unitItemsUnique ❤️

@commy2
Copy link
Contributor

commy2 commented Mar 19, 2018

Since we are (hobby) programmers, we should at least use instant-runoff voting instead of this archaic method barely good enough to decide who runs a country.

@commy2
Copy link
Contributor

commy2 commented Apr 14, 2018

Pushed back, because #902 (comment) is still unaddressed.

@commy2 commy2 changed the title Add getUniquePlayerItems function add CBA_fnc_uniqueUnitItems Apr 14, 2018
@commy2 commy2 changed the title add CBA_fnc_uniqueUnitItems Add CBA_fnc_uniqueUnitItems Apr 14, 2018
@commy2 commy2 modified the milestones: 3.7, 3.8 Apr 14, 2018
@dedmen
Copy link
Contributor Author

dedmen commented Apr 14, 2018

@commy2 it's supposed to replace items player. Does that return binocularMagazine?
Or do you want binocMag just for the weaponItems extra?

primaryWeaponMagazine _unit, secondaryWeaponMagazine _unit, handgunMagazine _unit
];
_allItems pushBack (_unit call CBA_fnc_binocularMagazine);
};
Copy link
Contributor

@commy2 commy2 Apr 14, 2018

Choose a reason for hiding this comment

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

if (_weaponItems) then {
    _allItems append [
        primaryWeapon _unit,
        secondaryWeapon _unit,
        handgunWeapon _unit
    ];
    _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 pushBack (_unit call CBA_fnc_binocularMagazine);
};

Copy link
Contributor

@commy2 commy2 Apr 14, 2018

Choose a reason for hiding this comment

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

Remember that primaryWeaponMagazine-no S etc. report ARRAY, exactly like the XItems versions.

private _allItems = (assignedItems _unit);
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);};
Copy link
Contributor

Choose a reason for hiding this comment

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

parenthesis aaaaa

Copy link
Member

Choose a reason for hiding this comment

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

Perfectly fine here to quickly see the order they are executed in.

Copy link
Contributor

@commy2 commy2 Apr 14, 2018

Choose a reason for hiding this comment

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

aaaaa

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);
};

T_T

Copy link
Member

Choose a reason for hiding this comment

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

getItemCargo uniformContainer _unit select 0

this is just unreadable IMO.

@commy2 commy2 modified the milestones: 3.8, 3.7 Apr 15, 2018
@commy2 commy2 dismissed jonpas’s stale review April 15, 2018 06:17

requested changes were made

@Killswitch00 Killswitch00 merged commit 074c052 into CBATeam:master Apr 15, 2018
#include "script_component.hpp"
SCRIPT(uniqueUnitItems);

params [["_unit", objNull, [objNull]], ["_weaponItems", true, [true]], ["_backpack", true, [true]], ["_vest", true, [true]], ["_uniform", true, [true]]];
Copy link
Contributor Author

@dedmen dedmen May 12, 2018

Choose a reason for hiding this comment

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

Hah!
["_weaponItems", true, [true]] default false? duh..

@dedmen
Copy link
Contributor Author

dedmen commented May 12, 2018

Btw here is some perf data about the use-case I made this for: acemod/ACEX#114 (comment)
From 10ms to 0.2ms

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

8 participants