PTS: Stack All Items Bug

[circonian]

The new "Stack All Items" is calling

PLAYER_INVENTORY:UpdateList(...)

way to often. It needs to loop through the inventory, stack all of the items, & only call UpdateList(...) once. At the very least StackBag(...) looks like it needs a check to see if a slot is already at max stack size before attempting to stack it with another stack of the same type of item. As it is now its attempting to stack slots, even if they are at max stack size, with every other stack of the same type of item and calling UpdateList(...) each time.

If you have, for example (it could be any stackable item in any slot, this is just an example)

slotIndex, item, stackSize
1, Sanded Nightwood, 200
2, Sanded Nightwood, 200
3, Sanded Nightwood, 200
4, Sanded Nightwood, 200
5, Sanded Nightwood, 45

and you hit the "Stack All Item" keybind PLAYER_INVENTORY:UpdateList(...) will get called 20 times. Even if that's all you have in that inventory.

Each slot/stack attempts to stack with every other slot/stack (of the same type of item), even if they are at max stack size, causing

#NumOfStacks * (#NumOfStacks-1)

calls to:

PLAYER_INVENTORY:UpdateList(...)

Reproduction Steps:
1) Empty the inventory to be tested on (this is just so we can see the exact update count caused by the stacks we add and don't have to worry about other stacks throwing off the count).
2) Place 2 stacks of the same type of item into the inventory. Must have over 200 (max stack size) items of the same type so you still have 2 slot/stacks after it stacks the items. Both stacks can be max stack size it does not matter.
3) Use the following code and watch how many times the UpdateList gets called. It will get called 2 times.

local iCount = 0
local function OnUpdateList(self, iInventoryType)
    iCount = iCount + 1
    d("OnUpdateList Count: "..iCount..", "..iInventoryType)end
ZO_PreHook(PLAYER_INVENTORY, "UpdateList", OnUpdateList)

4) Now add a 3rd stack (must have a total of over 400 of the same type of item, 2 slots at max stack size) and run the "Stack All Items" again. This time it will call UpdateList(...) 6 times.
5) With 4 stacks (over 600 of the same type of item) it will call UpdateList(...) 12 times
6) With 5 stacks (over 800 of the same type of item) it will call UpdateList(...) 20 times
Exc...exc....


A user was having a problem with my addon crashing & after great help from the users I managed to figure this out. He had a LOT of stacked items & lots of them were full stacks. UpdateList(...) was getting called nearly 200 times in a single use of "Stack All Items" which in turn was causing way to much of my code to run and it was crashing the game. I think I need to optimize something, but StackBag(...) needs some work too.

[Ayantir]

That's how I do in Roomba to stack in stackable bags, it does exactly the same feature as the function :
Little bit modified for the exemple!

-- Called when user click on the keybind / press the key.
    
local bagToScan = SHARED_INVENTORY:GenerateFullSlotData(nil, bagId)
Roomba.scanInStackableBag(bagToScan)
if NonContiguousCount(Roomba.duplicates) > 0 then Roomba.restackStackableBag(bagId, duplicateList) end
 
-- Scan in a stackable bag
function Roomba.scanInStackableBag(bagToScan)
 
    Roomba.lookUp = {}
    Roomba.duplicates = {}
    local duplitemp = {}
    
    -- We only need to store slots with items
    for index, slot in pairs(bagToScan) do
        
        -- Stack at max?
        local stack, maxStack = GetSlotStackSize(slot.bagId, slot.slotIndex)
        
        -- Stack is not at max
        if stack ~= maxStack then
            
            -- itemId
            local itemInstanceId = slot.itemInstanceId
            
            -- We already find this item before
            if Roomba.lookUp[itemInstanceId] then
                -- Already marked as duplicate?
                if not duplitemp[itemInstanceId] then
                    -- Duplicate
                    duplitemp[itemInstanceId] = Roomba.lookUp[itemInstanceId]
                end
            else
                -- New item found
                Roomba.lookUp[itemInstanceId] = {}
            end
            
            -- Now group all items by id
            table.insert(Roomba.lookUp[itemInstanceId], {slot = slot.slotIndex, stack = stack, texture = slot.iconFile, name = slot.name, itemInstanceId = slot.itemInstanceId})
            
        end
        
    end
    
    local i = 1
    for _, data in pairs(duplitemp) do
        Roomba.duplicates[i] = data
        i = i + 1
    end
 
end
 
-- duplicateList is Roomba.duplicates
function Roomba.restackStackableBag(bagId, duplicateList)
 
    local restartAfter = false
    local itemIndex = 0
    local indexItemsDuplicated, dataItems = next(duplicateList)
    local result = {}
    
    -- Loop for item X/Y/Z
    while indexItemsDuplicated do
        
        local index = 1
        local itemInfo = dataItems[index]
        local baseSlot = nil
        local lastMoveWasSingleStack = false
        local lastMoveWasMultiStack = false
        result[indexItemsDuplicated] = {}
        
        -- Loop for item X and slots 1/2/3/y
        while itemInfo do
            
            -- 1st loop : we go in
            if baseSlot == nil then
                -- Our actual stack / Our max size
                baseSlot = itemInfo
                baseSlot.actualStack, baseSlot.maxStack = GetSlotStackSize(bagId, itemInfo.slot)
            else
                
                local qty
                -- If stacking, will we get 1 or 2 stacks ?
                itemInfo.maxStack = baseSlot.maxStack
                
                if (baseSlot.actualStack + itemInfo.stack) <= baseSlot.maxStack then
                    
                    -- Only 1 stack, we can merge stacks
                    qty = itemInfo.stack
                    
                    -- Merging
                    if IsProtectedFunction("RequestMoveItem") then
                        CallSecureProtected("RequestMoveItem", bagId, itemInfo.slot, bagId, baseSlot.slot, qty)
                    else
                        RequestMoveItem(bagId, itemInfo.slot, bagId, baseSlot.slot, qty)
                    end
                    
                    -- Update values baseSlot can still be used in next loop
                    baseSlot.actualStack = baseSlot.actualStack + itemInfo.stack
                    baseSlot.stack = baseSlot.actualStack
                    
                    if lastMoveWasMultiStack == true then
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]] = baseSlot
                        lastMoveWasSingleStack = true
                        lastMoveWasMultiStack = false
                    elseif lastMoveWasSingleStack == true then
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]] = baseSlot
                        lastMoveWasMultiStack = false
                    else
                        table.insert(result[indexItemsDuplicated], baseSlot)
                        lastMoveWasSingleStack = true
                        lastMoveWasMultiStack = false
                    end
                    
                else
                    
                    -- It won't fit, just move the qty to match maxStack, no need to rescan because slots don't move, only stacks.
                    qty = baseSlot.maxStack - baseSlot.actualStack
                    
                    -- Merging
                    if IsProtectedFunction("RequestMoveItem") then
                        CallSecureProtected("RequestMoveItem", bagId, itemInfo.slot, bagId, baseSlot.slot, qty)
                    else
                        RequestMoveItem(bagId, itemInfo.slot, bagId, baseSlot.slot, qty)
                    end
                    
                    if lastMoveWasMultiStack == true then
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].stack = baseSlot.maxStack
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].actualStack = baseSlot.maxStack
                        table.insert(result[indexItemsDuplicated], itemInfo)
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].stack = itemInfo.stack - qty
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].actualStack = itemInfo.stack - qty
                        lastMoveWasSingleStack = false
                    elseif lastMoveWasSingleStack == true then
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].stack = baseSlot.maxStack
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].actualStack = baseSlot.maxStack
                        table.insert(result[indexItemsDuplicated], itemInfo)
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].stack = itemInfo.stack - qty
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].actualStack = itemInfo.stack - qty
                        lastMoveWasSingleStack = false
                        lastMoveWasMultiStack = true
                    else
                        table.insert(result[indexItemsDuplicated], baseSlot)
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].stack = baseSlot.maxStack
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].actualStack = baseSlot.maxStack
                        table.insert(result[indexItemsDuplicated], itemInfo)
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].stack = itemInfo.stack - qty
                        result[indexItemsDuplicated][#result[indexItemsDuplicated]].actualStack = itemInfo.stack - qty
                        lastMoveWasSingleStack = false
                        lastMoveWasMultiStack = true
                    end
                    
                    -- Baseslot is now at max, we cannot use it anymore
                    baseSlot = itemInfo
                    baseSlot.actualStack = itemInfo.stack - qty
                    
                end
                
            end
            
            index = index + 1
            itemInfo = dataItems[index]
        
        end
        
        itemIndex = itemIndex + 1
        indexItemsDuplicated, dataItems = next(duplicateList, itemIndex)
        
    end
    
    -- Rescan to permit the user press the keybind again. Because Bank got a lag, we need to call Roomba.onBankReady() and its zo_callLater to avoid having our restacking job still unknown
    if bagId == BAG_BANK then
        Roomba.onBankReady()
    -- Only on restacking bag
    elseif bagId == BAG_BACKPACK then
        Roomba.onSceneStateChange(SCENE_HIDDEN, SCENE_SHOWING)
    end
    
end

[QuadroTony]

we tested it all night, ye

we spent like 1.5m gold on different upgdares and 10k crowns
really annoying thing

and looks like it need very specific conditions to reproduce
i will be fine if some1 just write a code to hide this new Stack All Items feature from my eyes

[ZOS_ChipHilseberg]

This is now fixed internally.

[Baertram]

Just to be informed: There will be a "Stack all items" at the banks implemented by ZOs with the next update?
Hope Roomba will get it's credit then

[QuadroTony]

roomba still nedeed for a guild bank tho

[Baertram]

Great news!
Alright, thought they would implement it for bank & guild banks

[circonian]

Originally Posted by ZOS_ChipHilseberg This is now fixed internally.

Thanks Chip !!

[Ayantir]

Test in live :

stack of 199 items
another of 1

stack all should :

set a stack at 200

now it's :

a stack of 200
a false stack of 1

I did a /bug
thanks

[DerBombo]

I only just now noticed this new feature.
Well.. no need to update Restacker anymore, I guess..