Joint Rails - Optimize CBA_fnc_compatibleItems

[johnb432]

For updated information, see #1558 (comment).

When merged this pull request will:
- Uses 2.10 command (compatibleItems)

• Caches result for filters, as it's expensive to filter the items every time.

When 2.12 is released, the following (or similar) can probably replace everything after and including line 44 (if it's same or faster):

GVAR(namespace) getOrDefaultCall [format ["%1#%2", _weapon, _typeFilter], {
    private _cfgWeapons = configFile >> "CfgWeapons";
    (compatibleItems _weapon) select {_typefilter == getNumber (_cfgWeapons >> _x >> "itemInfo" >> "type")}
}, true];

[commy2]

Current implementation reports config case of the items. What case does this return? All lower? Config? Whatever is listed inside compatibleItems, regardless of if that is the capitalization of the item?

This must not change for sake of bwc.

[johnb432]

Current implementation reports config case of the items. What case does this return? All lower? Config? Whatever is listed inside compatibleItems, regardless of if that is the capitalization of the item?

This must not change for sake of bwc.

This still returns config case (as far as my testing has shown). It's much the same case as for compatibleMagazines - I can't guarantee that it returns config case, but my testing of the entire ace arsenal suggests it does.

[johnb432]

Dedmen was able to confirm that both compatibleMagazines and compatibleItems return config sensitive strings.

[commy2]

I can't guarantee that it returns config case

Someone who knows how to set up a config with broken case should test that. Otherwise, this PR may silently break a ton of stuff in ACE and other places.

[johnb432]

Someone who knows how to set up a config with broken case should test that. Otherwise, this PR may silently break a ton of stuff in ACE and other places.

I don't quite know what you mean by broken case, but I have tried the following:

• Making an item with different non-standard (sorry, I don't know my different standards well enough, but I believe it's ANSI?) characters in config names, such 인, р and ý. I couldn't get HEMTT to build the addon, but the vanilla addon builder allowed me to pack it. However, when I started the game, it complained about the character and closed.
• Editing an item's config name before its config should be loaded. I made an addon with requiredAddons[] = {}; that changed ACE_30Rnd_556x45_Stanag_M995_AP_mag to ace_30Rnd_556x45_Stanag_M995_AP_mag.
  Both CBA_fnc_compatibleMagazines and compatibleMagazines returned the exact same results, both containing ace_30Rnd_556x45_Stanag_M995_AP_mag. To me, that's expected.

I will gladly test broken case configs, but I need to know how to create one.

[PabstMirror]

seems like we could use the alt syntax to do the filtering as well?

params [["_weapon", "", [""]], ["_typefilter", nil, ["", 0]]];

if (_weapon == "") exitWith {[]};

if (isNil "_typefilter") exitWith {
    compatibleItems _weapon
};

_typefilter = ["", "MuzzleSlot", "CowsSlot", "PointerSlot", "UnderBarrelSlot"] param [["", "muzzle", "optic", "pointer", "bipod"] find _typefilter, -1];

if (_typefilter == "") exitWith {[]};

compatibleItems [_weapon, _typefilter]

[PabstMirror]

On my big modset

count compatibleItems "CUP_arifle_MG36" = 756
count (["CUP_arifle_MG36"] call CBA_fnc_compatibleItems) = 622

but

x1 = compatibleItems "CUP_arifle_MG36";
x2 = ["CUP_arifle_MG36"] call CBA_fnc_compatibleItems;
x1 = (x1 arrayIntersect x1);
x1 isEqualTo x2  = true // lucky that the order is the same so this is easy

compatibleItems has several duplicates (e.g CUP_acc_ANPEQ_2_Flashlight_Black_L is listed twice)

[commy2]

class CfgMagazines {
    class RPG32_F;
    class CBA_TestMagazine: RPG32_F {};
};

class CfgWeapons {
    class CBA_MiscItem;
    class CBA_MiscItem_ItemInfo;

    class CBA_TestOptic: CBA_MiscItem {
        class ItemInfo: CBA_MiscItem_ItemInfo {};
    };

    class Launcher;
    class Launcher_Base_F: Launcher {
        class WeaponSlotsInfo;
    };
    class CBA_TestLauncher: Launcher_Base_F {
        magazines[] = {"CBA_TESTmagazine"};

        class WeaponSlotsInfo: WeaponSlotsInfo {
            allowedSlots[] = {};

            class CowsSlot {
                compatibleItems[] = {"CBA_TESToptic"};
            };

            class MuzzleSlot {};
            class PointerSlot {};
        };
    };
};

From memory:

CBA_fnc_compatibleMagazines must report CBA_TestMagazine. CBA_fnc_compatibleItems must report CBA_TestOptic.

Please double check that case does not change before and after the PR.

Duplicates should be fine I guess.

To complete this, would also need time comparisions between both versions.

[johnb432]

Well.. I managed to break CBA_fnc_compatibleItems with a broken config (well, it depends on what you consider breaking).

I used the following config and packed it into a PBO:

class CfgWeapons {
    class rhs_weap_ak74m_Base_F;
    class rhs_weap_ak74m: rhs_weap_ak74m_Base_F {
        class WeaponSlotsInfo;
    };
    class rhs_weap_ak74m_zenitco01: rhs_weap_ak74m {
        class WeaponSlotsInfo: WeaponSlotsInfo {};
    };
    class rhs_weap_ak74m_zenitco01_b33: rhs_weap_ak74m_zenitco01 {
        class WeaponSlotsInfo: WeaponSlotsInfo {};
    };
};

This gave rhs_weap_ak74m_zenitco01_b33 the subconfig WeaponSlotsInfo the class rhs_npz_slot, which has the only attribute

compatibleItems[] = {"rhs_acc_npz"};

The game does not allow you to equip rhs_acc_npz on rhs_weap_ak74m_zenitco01_b33 and both canAdd and compatibleItems show that too. But CBA_fnc_compatibleItems returns that it's compatible.

---

In some cases, CBA_fnc_compatibleItems returns duplicates.

[johnb432]

seems like we could use the alt syntax to do the filtering as well?

params [["_weapon", "", [""]], ["_typefilter", nil, ["", 0]]];

if (_weapon == "") exitWith {[]};

if (isNil "_typefilter") exitWith {
    compatibleItems _weapon
};

_typefilter = ["", "MuzzleSlot", "CowsSlot", "PointerSlot", "UnderBarrelSlot"] param [["", "muzzle", "optic", "pointer", "bipod"] find _typefilter, -1];

if (_typefilter == "") exitWith {[]};

compatibleItems [_weapon, _typefilter]

Unfortunately not, because of broken configs. This can be tested by anyone fairly easily:
3CB Faction's UK3CB_G3SG1_RIS has a broken config for scopes. It has both CowsSlot and CowSlot. With compatibleItems, you can't get items from CowSlot using the filter CowsSlot and vice versa. However, items from both categories can be added to the weapon (compatibleItems without filter and canAdd show that).
So, unfortunately, I don't think we can use the filters for compatibleItems.

[johnb432]

class CfgMagazines {
    class RPG32_F;
    class CBA_TestMagazine: RPG32_F {};
};

class CfgWeapons {
    class CBA_MiscItem;
    class CBA_MiscItem_ItemInfo;

    class CBA_TestOptic: CBA_MiscItem {
        class ItemInfo: CBA_MiscItem_ItemInfo {};
    };

    class Launcher;
    class Launcher_Base_F: Launcher {
        class WeaponSlotsInfo;
    };
    class CBA_TestLauncher: Launcher_Base_F {
        magazines[] = {"CBA_TESTmagazine"};

        class WeaponSlotsInfo: WeaponSlotsInfo {
            allowedSlots[] = {};

            class CowsSlot {
                compatibleItems[] = {"CBA_TESToptic"};
            };

            class MuzzleSlot {};
            class PointerSlot {};
        };
    };
};

From memory:

CBA_fnc_compatibleMagazines must report CBA_TestMagazine. CBA_fnc_compatibleItems must report CBA_TestOptic.

Please double check that case does not change before and after the PR.

Duplicates should be fine I guess.

To complete this, would also need time comparisions between both versions.

compatibleItems "CBA_TestLauncher" // returns ["CBA_TestOptic"], 0.0009 ms
"CBA_TestLauncher" call CBA_fnc_compatibleItems // returns ["CBA_TestOptic"], 0.0046 ms (old)

compatibleMagazines "CBA_TestLauncher" // returns ["CBA_TestMagazine"], 0.0009 ms
"CBA_TestLauncher" call CBA_fnc_compatibleMagazines  // returns ["CBA_TestMagazine"], 0.0054 ms (old)

I'll add the new compile versions shortly.

[johnb432]

When running (the proposed changed one)

"CBA_TestLauncher" call CBA_fnc_compatibleItems

it throws errors:

• a popup one, if you haven't had any previous popups
• in the RPT:

10:53:24 Warning Message: Error: creating weapon CBA_TestLauncher with scope=private
10:53:24 Warning Message: Error: creating weapon CBA_TESToptic with scope=private

It returns the correct answer, but given the errors - I doubt this will be accepted as it is at the moment.
I will make a bug report with the short comings of both compatibleItems and compatibleMagazines.

---

However, if wanted, I still suggest:

• Switching to a hashmap.
• Adding caching to filtered results, as it's expensive to check every time.
• Removing duplicates.

So, if you guys give me the green light, I will apply the above mentioned suggestions to the original function in this PR.

[dedmen]

Please send that bug report to me, at best via Discord there I won't miss it.
Usually you don't check compatible items on a private class that noone can spawn/create. So I don't think these warnings are relevant.

Add scope=1 or scope=2 to the config (like every usable weapon has) and the warnings go away

[neilzar]

The more I try to figure out a more robust way to select which slot to pick, the more I realise the current CBA_fnc_compatibleItems filter is also not reliable.
There is no connection between "itemInfo" >> "type" and the compatibleItems array as stored in the engine class of a weapon's slot. Unless I missed some "type" check somewhere, the compatibleItems in engine are never checked against the type; so an item with type 101 (MuzzleSlot) could be put in the compatibleItems for the PointerSlot and then filtering for the muzzleSlot would return that item, but it would not be compatible.
I'll keep digging to figure out a solution that is more reliable than the slotNames the compatibileItems command currently takes as second right hand argument. But right now, apart from someone making a typo like CowSlot, the command is as reliable as it can get.

[neilzar]

So the duplicate items issue is related to wrong configs, where a weapon has multiple slots defined that have similar compatible items. So the CUP_arifle_MG36 example has duplicates because it defines both a PointerSlot and a CUP_PicatinnySideMountG36, both with similar, if not the same, items defined. It never gets picked up by the current implementation of CBA_fnc_compatibleItems because it uses pushBackUnique to add new items.
I should have a solution that filters out duplicates, although it is quite a bit slower than just the raw return but still faster than using arrayIntersect afterwards.

[neilzar]

As for the type filter, the most reliable method I can think of so far (although it is still a config thing that could be changed by someone) is checking which linkProxy value is used for a WeaponSlotsInfo child. I don't want to use that in engine, as it would be using hard-coded PBO file paths. I can't find a method that could be used in engine that doesn't require a complete rewrite of the system or data changes (which ends up breaking compatibility).

These are the linkProxy values: (These are not always capitalised the same, so would need a toLower when checking if you don't just check for the value after the last \)
Optics Slot = "\a3\data_f\proxies\weapon_slots\TOP"
Pointer Slot = "\a3\data_f\proxies\weapon_slots\SIDE"
Muzzle Slot = "\A3\data_f\proxies\weapon_slots\MUZZLE"
Underbarrel Slot = "\a3\data_f_mark\Proxies\Weapon_Slots\UNDERBARREL"

There could be multiple slots defined for each slot type, as there is a limit of 5 slots in the engine. (Not sure what the fifth is supposed to be used for normally)

[commy2]

That's not how any of this works lol ^

[neilzar]

That's not how any of this works lol ^

Enlighten me how it does work then? Because I spent quite a bit of time looking at the engine code and at several mods to see how it works and how they are configured...

[johnb432]

My project was to improve CBA_fnc_compatibleItems' performance by using compatibleItems. However, it doesn't seem to be the right way forward.
For me, there are other ways that CBA_fnc_compatibleItems can be improved in:

• Switching to a hashmap.
• Adding caching to filtered results, as it's expensive to check every time.
• Removing duplicates (for rare cases).
• Removing "" from the result, if an item doesn't exist (for rare cases).

This PR with commit e703926 addresses or incorporates all of the points above. However, there is a small cost to pay:

As it is now in CBA:

["arifle_Mk20_plain_F"] call CBA_fnc_compatibleItems // about 0.006 ms (6 ns)
["arifle_Mk20_plain_F", 201] call CBA_fnc_compatibleItems // about 0.115 ms (115 ns)
["arifle_Mk20_plain_F", "optic"] call CBA_fnc_compatibleItems // about 0.115 ms (115 ns)

// Non-existent weapon
["test"] call CBA_fnc_compatibleItems // about 0.0185 ms (18.5 ns)
["test", "optic"] call CBA_fnc_compatibleItems // about 0.0205 ms (20.5 ns)

// Bad filtering type
["arifle_Mk20_plain_F", 0] call CBA_fnc_compatibleItems // about 0.115 ms (115 ns)

As suggested with e703926:

["arifle_Mk20_plain_F"] call CBA_fnc_compatibleItems // about 0.009 ms (9 ns)
["arifle_Mk20_plain_F", 201] call CBA_fnc_compatibleItems // about 0.010 ms (10 ns)
["arifle_Mk20_plain_F", "optic"] call CBA_fnc_compatibleItems // about 0.011 ms (11 ns)

// Non-existent weapon
["test"] call CBA_fnc_compatibleItems // about 0.0165 ms (16.5 ns)
["test", "optic"] call CBA_fnc_compatibleItems // about 0.0165 ms (16.5 ns)

// Bad filtering type
["arifle_Mk20_plain_F", 0] call CBA_fnc_compatibleItems // about 0.006 ms (6 ns)

The proposed option is faster (sometimes much faster) in nearly every case, except the first one - which is probably the most common use case. Slower by 3 ns, due to more checks for the type filter.

My question is: Is this acceptable?

[johnb432]

e35831c:

["arifle_Mk20_plain_F"] call CBA_fnc_compatibleItems // about 0.008 ms (8 ns)
["arifle_Mk20_plain_F", 201] call CBA_fnc_compatibleItems // about 0.009 ms (9 ns)
["arifle_Mk20_plain_F", "optic"] call CBA_fnc_compatibleItems // about 0.010 ms (10 ns)

// Non-existent weapon
["test"] call CBA_fnc_compatibleItems // about 0.0165 ms (16.5 ns)
["test", "optic"] call CBA_fnc_compatibleItems // about 0.0165 ms (16.5 ns)

// Bad filtering type
["arifle_Mk20_plain_F", 0] call CBA_fnc_compatibleItems // about 0.006 ms (6 ns)

[johnb432]

Reverted changes from e35831c, because of the same reasons given in #1585.