GetInventoryItemOfType overload engine

[ToldYouSo]

Hello, I have that function

function GetInventoryIndexOfItemTypeJ takes unit whichUnit, integer itemId returns integer
    local integer index
    local item    indexItem

    set index = 0
    loop
        set indexItem = UnitItemInSlot(whichUnit, index)
        if (indexItem != null) and (GetItemTypeId(indexItem) == itemId) then
            return index + 1
        endif

        set index = index + 1
        exitwhen index >= bj_MAX_INVENTORY
    endloop
    return 0
endfunction

F..e if I check for units to have a specific item by this function EVERY time units get damaged, would it cause many massive weight on engine?
Should I change my method to store specific boolean value on units upon PICK/DROP event, and use load boolean instead?

// DAMAGE EVENT
//MY CURRENT METHOD
if GetInventoryIndexOfItemTypeJ(target,'I00C') > 0 then
//actions
endif


//THE METHOD I BELIEVE WOULD CAUSE LESS WEIGHT 
if LoadBoolean(Hash,GetHandleId(target),StringHash("Item56")) == true then
//actions
endif

 

[Uncle]

It'll probably be fine but to answer your question, yes, a Hashtable lookup would make a massive difference and perform way better. It's not too difficult to create a system for tracking Items as well since the Acquiring/Losing item Events are simple and easily managed.

A StringHash seems unnecessary to me, why not use the Item-Type id? Keep it nice and efficient with Integers, Strings are bleh.

 

[ToldYouSo]

It'll probably be fine but to answer your question, yes, a Hashtable lookup would make a massive difference and perform way better. It's not too difficult to create a system for tracking Items as well since the Acquiring/Losing item Events are simple and easily managed.

A StringHash seems unnecessary to me, why not use the Item-Type id? Keep it nice and efficient with Integers, Strings are bleh.

You mean like that?

call SaveBoolean(Hash,GetHandleId(unitthatpicked),GetItemTypeId(pickeditem),true)

 

[Halithor]

Both methods of checking for the item, either going through the slots or using a hashtable, are constant time algorithms (both O(1) big-o). There may be some minor performance improvements to using one method over the other, but it probably won't matter.

As a rule of thumb in software, don't optimize until you hit performance problems. And when you hit performance problems, benchmark to figure out what you need to optimize.

 

[Uncle]

Both methods of checking for the item, either going through the slots or using a hashtable, are constant time algorithms (both O(1) big-o). There may be some minor performance improvements to using one method over the other, but it probably won't matter.

As a rule of thumb in software, don't optimize until you hit performance problems. And when you hit performance problems, benchmark to figure out what you need to optimize.

Isn't he using two for loops in the first function?

UnitItemInSlot() probably loops from 1 to InventorySize which he's calling inside of the original functions For Loop. I feel like that's got to be much worse.

Surely providing the exact keys in a Hashtable is faster? -> return Array[GetUnitId()][GetItemTypeId()]

Anyway, I agree on what you said about optimization. Using the Damage Engine alone is really the performance problem here, you'll see an actual FPS drop in large fights because of it, and I doubt the Item checks really add much to that.

 

[Halithor]

As far as I know, the UnitItemInSlot is just a lookup into the array that is the 6 inventory slots? It may be a loop, but it's a native so it's running in the game engine, not the interpreted code, so it'll be really fast either way. Otherwise I only see one loop and it's bounded by bj_MAX_INVENTORY which is just 6. Even if the native loops through inventory max, that's still just 6, leaving the algorithm O(1) (36 is a constant time loop count).

Basically you'd need to benchmark the speed of the hashing algorithm, because it's speed probably determines which one is faster practically.