Thaoky / DataStore_Containers

WoW DataStore Addon - Containers Module
3 stars 1 forks source link

AccountBank wiped on hearth #4

Open SpareSimian opened 3 weeks ago

SpareSimian commented 3 weeks ago

Using one's hearthstone wipes the data for the account bank. The wrong event is being used. Patch:

diff --git a/DataStore_Containers/API/WarbandBank.lua b/DataStore_Containers/API/WarbandBank.lua
index 82094672..ed0d60f8 100644
--- a/DataStore_Containers/API/WarbandBank.lua
+++ b/DataStore_Containers/API/WarbandBank.lua
@@ -81,10 +81,8 @@ end

 -- *** Event Handlers ***
-local function OnBagUpdate(event, bag)
-   if bag >= Enum.BagIndex.AccountBankTab_1 and bag <= Enum.BagIndex.AccountBankTab_5 then
-       ScanAccountBankTab(bag)
-   end
+local function OnBankFrameOpened()
+   ScanAccountBank()
 end

 local function OnAccountBankTabsChanged(event, tabID)
@@ -138,7 +136,7 @@ end)
 DataStore:OnPlayerLogin(function()
    C_Timer.After(3, function()
        -- To avoid the long list of BAG_UPDATE at startup, only register the event 3 seconds later ..
-       addon:ListenTo("BAG_UPDATE", OnBagUpdate)
+       addon:ListenTo("BANKFRAME_OPENED", OnBankFrameOpened)
    end)

    addon:ListenTo("PLAYER_INTERACTION_MANAGER_FRAME_SHOW", function(event, interactionType)
Kersplat314 commented 2 weeks ago

I've been trying to find the cause of the warband bank contents going missing for a while and gave your change a try. It now doesn't lose the warband bank contents but it also doesn't update correctly when you are crafting at a profession table etc. After crafting altoholic will be incorrect until you visit a bank which is a problem.

SpareSimian commented 2 weeks ago

I don't see how one can automatically update the warbank after remote crafting, nor will your personal reagent bank be correct if mats are pulled from it. You have to visit those banks to update their data. (With the warbank, you can use the new remote bank access spell with the 2-hour cooldown.) To compute what was taken from the bank would require a huge amount of logic currently hidden in the WoW server code.

Kersplat314 commented 2 weeks ago

Currently altoholic watches the BAG_UPDATE events and it does correctly update its data when crafting at a profession table. Items are taken from my reagent bag, then the warband bank and probably the reagent tab last but I didn't get that far. The altoholic tooltip was updated correctly on each craft. Unfortunately, on a zone change (not just hearthing) lots of BAG_UPDATE events happen and I'd guess the warband bank info is not available at that time resulting in the bank being cleared.

SpareSimian commented 2 weeks ago

That makes sense. So we need to know when the bank is accessible. Is it only when crafting at a crafting table? I don't think it's available when making things that don't require a crafting table while just standing around in some random location.

Kersplat314 commented 2 weeks ago

In general, it seems that crafting through a profession window works correctly. Quest hand ins that pull items from the warband or reagent banks do not and I'm not sure how altoholic could possibly handle that.

Kersplat314 commented 2 weeks ago

Warbandbank.zip I made a small change to warbandbank.lua which seems to fix the problem for me. There are probably better ways to fix this issue and I need to test some more but thought I'd post the change in case anyone else wants to try it.

SpareSimian commented 2 weeks ago

@Kersplat314 that's working great with this morning's update.

I threw in some prints and found that there's a huge flurry of BAG_UPDATE events when zoning, after the PLAYER_ENTERING_WORLD event. They don't hit the wipe code, so that means the bank data isn't available then. It might be useful to ignore BAG_UPDATE during the 2-3 seconds immediately following PLAYER_ENTERING_WORLD, to avoid the pointless bag scan and further slowing down the zoning process. The only time I can think of someone opening the bank immediately after zoning would be if a Vulpera had set her camp in a bank. So use the bank frame open event for that.

Gaviin1242 commented 1 week ago

Thanks for your work on this, guys! Just to be clear, should I be using the zip file Kersplat shared, or doing the code changes mentioned in the first post?

SpareSimian commented 1 week ago

I replaced my change in the first post with Kersplat314's zip file and it's working fine for me.