FindFirstEmptySlotInBag not updating

Hey guys it's me again ;-)

I have some issues with the function FindFirstEmptySlotInBag(BAG_BACKPACK).

I want to move items from the bank into my inventory but it always just moves one item.
I dumped the values of the variables and found out that the Destination SlotID (slotId_Bag) always stays the same.
So I think that this is the issue - but unfortunately I cannot image why it is not updating because I call it in every loop of the for-loop so it should update correctly.

function LootManager.GetIngredientsFromBank()
    local name
    local item
    local slotId
    local count
    local bagId = BAG_BANK
    local slots = GetBagSize(bagId)
    local slotId_Bag
    local bag_Dest = BAG_BACKPACK
 
    for i=0, slots - 1 do
        slotId = i
        name = zo_strformat(SI_UNIT_NAME, GetItemName(bagId, slotId))
        item, count = GetItemInfo(bagId, slotId)
        slotId_Bag = FindFirstEmptySlotInBag(bag_Dest)
 
        if name == LootManager.savedVars.ingredient1 then
            LootManager.MoveItem(bagId, slotId, bag_Dest, slotId_Bag, count)
        end 
 
        if name == LootManager.savedVars.ingredient2 then   
            LootManager.MoveItem(bagId, slotId, bag_Dest, slotId_Bag, count)
        end 
        
        if name == LootManager.savedVars.ingredient3 then
            LootManager.MoveItem(bagId, slotId, bag_Dest, slotId_Bag, count)
        end 
 
    end
 
end
 
------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------
 
function LootManager.MoveItem(srcBag, srcSlot, destBag, destSlot, quantity)
    ClearCursor()
    zo_callLater(function() 
        if CallSecureProtected("PickupInventoryItem", srcBag, srcSlot, quantity) then
            CallSecureProtected("PlaceInInventory",destBag, destSlot)
        end
    end, 500)   
    ClearCursor()   
    d("Moved: " .. zo_strformat(SI_UNIT_NAME, GetItemName(srcBag, srcSlot)) .. " x " .. quantity .. " to inventory")
    d("SrcBag: " .. srcBag .. " SrcSlot: " .. srcSlot .. " DestBag: " .. destBag .. " DestSlot: " .. destSlot .. " Quantity: " .. quantity)
end

Do you see any error here?

Thanks in advance

[merlight]

That's probably by design. The items are not actually moved immediately, your commands are carried out after your handler exits.

Some time ago I wrote my own function for finding an empty slot, which allows me to start the search after the previous one:

local function findEmptySlotInBag(bagId, prevIndex, lastIndex)
    local slotIndex = prevIndex or -1
    while slotIndex < lastIndex do
        slotIndex = slotIndex + 1
        if GetItemType(bagId, slotIndex) == ITEMTYPE_NONE then
            return slotIndex
        end
    end
    return nil
end
 
-- usage
local numSlots = GetBagSize(BAG_BANK)
local emptyIndex = findEmptySlotInBag(BAG_BANK, nil, numSlots - 1)
while emptyIndex do
   -- we have an empty slot in bank
   ... put something in there
    emptyIndex = findEmptySlotInBag(BAG_BANK, emptyIndex, numSlots - 1)
end
-- bank full

Thanks for your reply.

I already thought about making my own Slotfinder but i hoped that there was some working built-in function ....

[circonian]

I'm pretty sure the keys in the slots table matches the slotIndices. So you could loop through the slots and check to see if each key exists in the table.

local inventory = PLAYER_INVENTORY.inventories[INVENTORY_BACKPACK].slots
local bagSize = GetBagSize(BAG_BACKPACK)
    
for i = 0, bagSize-1 do
   if not inventory[i] then
      -- do whatever inventory[i] is an empty slot
   end
end

Or you could make it into a function and each time you find an empty slot throw it into a table, then return the table & you'll have all of the empty slots to do whatever you want with them.

Although merlights answer is probably better for what your doing, just giving some options.

[Garkin]

This issue is because of 500ms delay in you function LootManager.MoveItems. All calls of the FindFirstEmptySlotInBag within this 500ms will return still the same slot. If you remove zo_callLater from the function, it should work correctly.

And I think it will be better if you use RequestMoveItem function instead of PickupInventoryItem and PlaceInInventory functions.

[merlight]

Originally Posted by Garkin This issue is because of 500ms delay in you function LootManager.MoveItems. All calls of the FindFirstEmptySlotInBag within this 500ms will return still the same slot. If you remove zo_callLater from the function, it should work correctly.

They might have changed it, but I don't think so. I did the same thing some time ago (the addon in question still has APIVersion 100008), and FindFirstEmptySlotInBag kept returning the same value over and over.

Similar thing happens if you handle EVENT_LOOT_UPDATED. Functions GetLootMoney/GetNumLootItems will keep returning the values applicable when the handler was called, even after you call LootMoney/LootItemById inside the handler. After the handler exits, items are actually moved, and a new event generated with updated state.

Originally Posted by Garkin And I think it will be better if you use RequestMoveItem function instead of PickupInventoryItem and PlaceInInventory functions.

Good point, this one was added recently. And has a better name - request indicates the action is not carried out immediately.

[Sasky]

Originally Posted by merlight They might have changed it, but I don't think so. I did the same thing some time ago (the addon in question still has APIVersion 100008), and FindFirstEmptySlotInBag kept returning the same value over and over.

Might be a base issue, but Garkin was looking at this in brekal's code:

function LootManager.MoveItem(srcBag, srcSlot, destBag, destSlot, quantity)
    --...
    zo_callLater(function()
        --Actual move in here, delayed 500ms
    end, 500)  
    --...
end

This introduces a delay for actually executing the move, and iterating through the list (hopefully) finishes in <500ms so they're all looking at the same slot.

What is the purpose of the delay? If you're trying to not flood the server (which could result in a forced logout), it doesn't work as written. zo_callLater is non-blocking, so it will exit the main function and continue in the loop without waiting at all. You'll end up with all the actual move attempts ~500ms later still close enough to flip a message rate limit.

thanks for your replies.

First I tried out circonians suggestion - but unfortunately it didn't work.

So I tried his second suggestion - to go through the inventory and save all free slots in an extra table.

When I then move the item I iterate through the table of free stlots. I had there some issues too. Because when I started at the first element sometimes it did not get the right destSlots. So I decided to start at the last element and this works fine.

I know that my solution is not the smartest but at least it is working for my purposes

Here ist the actual code:

function LootManager.GetIngredientsFromBank()
    if LootManager.savedVars.getBankIngredients ~= true then
        return
    end
    
    local name
    local item
    local slotId
    local count
    local bagId = BAG_BANK
    local slots = GetBagSize(bagId)
    local bag_Dest = BAG_BACKPACK
    
    LootManager.FindEmptySlotInBag()    
    local slotId_Bag = LootManager.emptySlotsInBag[1]
    
    for i=0, slots - 1 do
        slotId = i
        name = zo_strformat(SI_UNIT_NAME, GetItemName(bagId, slotId))
        item, count = GetItemInfo(bagId, slotId)
 
        if slotId_Bag ~= nil then
            if name == LootManager.savedVars.ingredient1 then
                LootManager.MoveItem(bagId, slotId, bag_Dest, slotId_Bag, count)
                slotId_Bag = LootManager.emptySlotsInBag[table.getn(LootManager.emptySlotsInBag)-1]
            end 
 
            if name == LootManager.savedVars.ingredient2 then   
                LootManager.MoveItem(bagId, slotId, bag_Dest, slotId_Bag, count)
                slotId_Bag = LootManager.emptySlotsInBag[table.getn(LootManager.emptySlotsInBag)-2]
            end 
        
            if name == LootManager.savedVars.ingredient3 then
                LootManager.MoveItem(bagId, slotId, bag_Dest, slotId_Bag, count)
                slotId_Bag = LootManager.emptySlotsInBag[table.getn(LootManager.emptySlotsInBag)-3]
            end 
        else
            d("Fehler: Slot-ID ist nil")
            break
        end
    end
    
    LootManager.emptySlotsInBag = {}
end
 
------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------
 
function LootManager.MoveItem(srcBag, srcSlot, destBag, destSlot, quantity)
    ClearCursor()
        if CallSecureProtected("PickupInventoryItem", srcBag, srcSlot, quantity) then
            CallSecureProtected("PlaceInInventory",destBag, destSlot)
        end
    ClearCursor()   
--  d("Moved: " .. zo_strformat(SI_UNIT_NAME, GetItemName(srcBag, srcSlot)) .. " x " .. quantity .. " to inventory")
--  d("SrcBag: " .. srcBag .. " SrcSlot: " .. srcSlot .. " DestBag: " .. destBag .. " DestSlot: " .. destSlot .. " Quantity: " .. quantity)
end
 
------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------
 
function LootManager.FindEmptySlotInBag()
    local bagSize = GetBagSize(BAG_BACKPACK)
    local name
    
    for i = 0, bagSize - 1 do
        name = zo_strformat(SI_UNIT_NAME, GetItemName(BAG_BACKPACK, i)) 
        if name == "" then
            table.insert(LootManager.emptySlotsInBag, i)    
        end
    end
 
end

I also do not understand why this delay should cause the problem because I only delay the movement of the item, not the FindFirstEmptySlotInBag-function. The interesting thing about this is, that dumping the values (as I do in line 44 and 45 of my original post) shows the correct item name and srcSlot - the only thing that stays the same is the destSlot.

I did not find any article on esoui-wiki about this "RequestMoveItem" function that Garkin suggested - do you have more infos about it?

[merlight]

Adding delay is unnecessary, the move won't happen until your handler exists anyway.

Haven't really tested it, but I'm pretty sure the client doesn't do any network communication directly from API functions. Think about it - if you called RequestMoveItem and the client would immediately send the request to the server, waited for the response and then returned the result to you, the latency would be horrid, and your add-on would completely block all other add-ons for the duration of the request (by all other add-ons I mean the whole UI, health bars and such). API functions absolutely have to put network requests on a queue.

Then I have some design notes on your free slots caching:
1) You don't know in advance how many free slots you're going to need. Thus finding all of them before you start is a bit wasteful. The function I posted earlier looks for one free slot, you call it when you need one. This is a somewhat generic principle: don't search and cache everything you might need later, search and cache things you actually need.
2) Check whether a slot is empty using something other than GetItemName (comparing strings will always be slower than comparing numbers). I used GetItemType for that.
edit: 3) you probably wanted slotId_Bag = table.remove(LootManager.emptySlotsInBag); your code doesn't really take items from the cache, it uses the same 1-4 slots over and over.

From ESOUIDocumentationP5.txt (you can find it on official forums in PTS section)

* RequestMoveItem *protected* (*integer* _sourceBag_, *integer* _sourceSlot_, *integer* _destBag_, *integer* _destSlot_, *integer* _stackCount_)

[circonian]

Originally Posted by brekal thanks for your replies. First I tried out circonians suggestion - but unfortunately it didn't work. So I tried his second suggestion - to go through the inventory and save all free slots in an extra table. When I then move the item I iterate through the table of free stlots. I had there some issues too. Because when I started at the first element sometimes it did not get the right destSlots. So I decided to start at the last element and this works fine.

You said the code did not work for you. I'm not sure if you mean there was another problem later after you used it or not, but that code does find the free slots, I tested it.

Also as merlight stated it is wasteful to loop through and find ALL of the free slots available if you do not need them all. As I stated above, merlights answer is definitely better. I was only trying to give some options on ways you can find free slots, without copying his answer because I could not think of a better one and figured who knows sometime it may be useful to find all of the free slots for you or someone else reading this later. Sorry if I did not make my reply clear enough.

[circonian]

Originally Posted by merlight 2) Check whether a slot is empty using something other than GetItemName (comparing strings will always be slower than comparing numbers). I used GetItemType for that.

merlight, this is strictly a curiosity question to see if there is something I'm missing or do not understand, it is probably a negligible difference.

But, Is there a reason you don't just check the inventory slots table instead of calling GetItemType and comparing it to see if its ITEMTYPE_NONE?
Wouldn't that be even faster?

local function findEmptySlotInBag(bagId, prevIndex, lastIndex)
   local slotIndex = prevIndex or -1
   while slotIndex < lastIndex do
      slotIndex = slotIndex + 1
      -- this
      local iInventory = PLAYER_INVENTORY.bagToInventoryType[bagId]
      if not PLAYER_INVENTORY.inventories[iInventory].slots[slotIndex] then
      -- instead of this
      --if GetItemType(bagId, slotIndex) == ITEMTYPE_NONE then
         return slotIndex
      end
   end
   return nil
end

[merlight]

Originally Posted by circonian But, Is there a reason you don't just check the inventory slots table instead of calling GetItemType and comparing it to see if its ITEMTYPE_NONE? Wouldn't that be even faster?

No particular reason. GetItemType is what I learned first, is easy to remember and also shorter, dunno about speed Although if you wanted to use pre-cached slot data, you should probably use SHARED_INVENTORY:GenerateSingleSlotData. That's what PLAYER_INVENTORY.inventories[iInventory].slots are initialized with:

-- ingame/inventory/inventory.lua
inventory.slots[slotIndex] = SHARED_INVENTORY:GenerateSingleSlotData(bagId, slotIndex)

[Ayantir]

A very old bump for 1st thank you for this feedback

I personnally wrote this :

-- Our bagcache, because game don't have it in realtime
    local tinyBagCache = {
        [BAG_BACKPACK] = {},
        [BAG_BANK] = {},
    }
    
    -- Thanks Merlight & circonian, FindFirstEmptySlotInBag don't refresh in realtime.
    local function FindEmptySlotInBag(bagId)
        for slotIndex = 0, (GetBagSize(bagId) - 1) do
            if not SHARED_INVENTORY.bagCache[bagId][slotIndex] and not tinyBagCache[bagId][slotIndex] then
                tinyBagCache[bagId][slotIndex] = true
                return slotIndex
            end
        end
        return nil
    end

But this function is not the only one to do not update in real time, a second one, more recent got the same problem : GetItemLinkStacks(itemLink)


My solution is :

-- Our bagcache for qty, because game don't have it in realtime
    local qtyBagCache = {}
 
    -- this function returns more info than desired, but it's for the exemple.
    local function StackInfoInBag(bagToCheck, slotIdFrom, bagIdFrom, itemLink)
        local stackCountBackpack, stackCountBank
        
        if not qtyBagCache[itemLink] then
            stackCountBackpack, stackCountBank = GetItemLinkStacks(itemLink) -- Not updated in realtime
            qtyBagCache[itemLink] = {}
            qtyBagCache[itemLink][BAG_BACKPACK] = stackCountBackpack
            qtyBagCache[itemLink][BAG_BANK] = stackCountBank
        else
            stackCountBackpack = qtyBagCache[itemLink][BAG_BACKPACK]
            stackCountBank = qtyBagCache[itemLink][BAG_BANK]
        end
        
        d("--------------")
        d(stackCountBackpack, stackCountBank)
        d("---")
        d(GetItemLinkStacks(itemLink))
        d("--------------")
        
        local stackSize, maxStack = GetSlotStackSize(bagIdFrom, slotIdFrom)
        if bagToCheck == BAG_BACKPACK then
            return stackCountBackpack >= maxStack, stackSize, maxStack, stackCountBackpack, stackCountBank
        elseif bagToCheck == BAG_BANK then
            return stackCountBank >= maxStack, stackSize, maxStack, stackCountBackpack, stackCountBank
        end
    end

An exemple :

[00:38:18] --------------
[00:38:18] 672 -- backpackqty with mycache func
[00:38:18] 500 -- bankqty with mycache func
[00:38:18] ---
[00:38:18] 672 -- backpackqty with ZOS func
[00:38:18] 500 -- bankqty with ZOS func
[00:38:18] --------------
[00:38:18] BMR a déplacé 72x [Toile du Vide] en banque <----- my addon moved 72x from bag to bank
[00:38:18] --------------
[00:38:18] 600 -- backpackqty with mycache func
[00:38:18] 572 -- bankqty with mycache func
[00:38:18] ---
[00:38:18] 672 -- backpackqty with ZOS func -- Wrong
[00:38:18] 500 -- bankqty with ZOS func -- Wrong
[00:38:18] --------------
[00:38:18] BMR a déplacé 28x [Toile du Vide] en banque <----- my addon moved 28x from bag to bank (from another stack, approx 1ms after 1st move
[00:38:18] --------------
[00:38:18] 572 -- backpackqty with mycache func
[00:38:18] 600 -- bankqty with mycache func
[00:38:18] ---
[00:38:18] 672 -- backpackqty with ZOS func -- Wrong
[00:38:18] 500 -- bankqty with ZOS func -- Wrong
[00:38:18] --------------