1.14.0: Error interacting with bank bag slots #3 or higher

[Meorawr]

When a bag is dragged into bank bag slot 3 (or higher), a Lua error will occur when the ITEM_LOCK_CHANGED event fires for the bag slot being dragged-to. The root cause is discussed later, and is probably the same issue behind #186.

---

The test setup assumes that the player has a bag that they can drag into an empty bank bag slot, and that all bank bag slots are unlocked. A video is also provided demonstrating the issue.

1. Drag bag into bank slot 3.
2. Collect error.

2021-10-04_11-32-01.mp4

Message: Interface_Vanilla\FrameXML\BankFrame.lua:148: attempt to index local 'button' (a nil value)
Time: Mon Oct  4 11:37:32 2021
Count: 1
Stack: Interface_Vanilla\FrameXML\BankFrame.lua:148: attempt to index local 'button' (a nil value)
[string "@Interface_Vanilla\FrameXML\BankFrame.lua"]:148: in function `BankFrameItemButton_UpdateLocked'
[string "@Interface_Vanilla\FrameXML\BankFrame.lua"]:263: in function <Interface_Vanilla\FrameXML\BankFrame.lua:247>
[string "=[C]"]: in function `PickupBagFromSlot'
[string "@Interface_Vanilla\FrameXML\BankFrame.lua"]:377: in function `BankFrameItemButtonBag_Pickup'
[string "*:OnDragStart"]:1: in function <[string "*:OnDragStart"]:1>

Locals: button = nil
(*temporary) = nil
(*temporary) = nil
(*temporary) = nil
(*temporary) = nil
(*temporary) = "attempt to index local 'button' (a nil value)"

---

There's a logic issue with regards to the NUM_BANKGENERIC_SLOTS constant and the actual bank bag slot IDs.

In the Classic Era client, NUM_BANKGENERIC_SLOTS is defined as 24, whereas in In BCC and Retail, it's 28. This constant refers to the number of bank slots in the "Item Slots" section of the bank.

When it comes to the bank bag slots, those are considered part of the "Bank" container bag and take up the slots after NUM_BANKGENERIC_SLOTS - so for example, bank bag slot 1 should be NUM_BANKGENERIC_SLOTS + 1, and thus 25 onwards for Classic Era and 29 onwards for BCC/Retail.

This results in logic such as this where, if the container affected by an event is the main bank, you can determine if it was an item slot or a bag slot based upon the slot ID being <= NUM_BANKGENERIC_SLOTS.

The problem is that while NUM_BANKGENERIC_SLOTS is defined as 24 in Classic Era the bank slots don't start immediately after - instead, they start at 29 as if NUM_BANKGENERIC_SLOTS were defined as its BCC/Retail value of 28. This can be verified by opening /etrace and filtering for the ITEM_LOCK_CHANGED event, then moving a bank bag from item slot 24 to the first bank bag slot.

This unexpected gap breaks the slot - NUM_BANKGENERIC_SLOTS logic used both by the bank frame and addons since for the third bank bag slot onwards the slot will be 31 or higher, and as 31 - 24 = 7 this leads to an attempt to access BankSlotsFrame.Bag7 which doesn't exist as there's only 6 bank bag slots in Classic Era.

The ideal fix if at all possible would be to make it so that, in Classic Era, bank bag slots correctly immediately begin after NUM_BANKGENERIC_SLOTS - such that the first bank bag slot would be 25, not 29. This would both fix the bank frame UI and allows addons that are reliant upon this same bank slot logic - which has been stable and consistent across all clients for years - to work without changes.

[Meorawr]

Should be fixed in 1.14.0.40618 with the addition of the new C function GetFirstBagBankSlotIndex. At present this function is exclusive to Classic Era, will update if this changes.

[Meorawr]

The new function will be ported to clients other than Classic Era, however no ETA is given on when this will be. Consistency shall be achieved.