shirsig / aux-addon

Auction House addOn for Classic (1.13) IMPORTANT: The folder name must be "aux-addon" IMPORTANT: The Vanilla (1.12) version moved here https://github.com/shirsig/aux-addon-vanilla
https://www.curseforge.com/wow/addons/aux
196 stars 42 forks source link

wow classic lua errors on mailbox and bags and AH #197

Closed mooreatv closed 5 years ago

mooreatv commented 5 years ago

Hi there, first I am using the "retail" branch, is that the right one ? (I noticed there is also a classic branch but less updated... which one is the right one?)

the error when opening mail:

74x aux-addon\core\tooltip.lua:148: Usage: GetInboxItem(messageIndex, attachIndex)
[C]: in function `GetInboxItem'
aux-addon\core\tooltip.lua:148: in function <aux-addon\core\tooltip.lua:147>
aux-addon\core\tooltip.lua:26: in function `SetInboxItem'
FrameXML\MailFrame.lua:313: in function `InboxFrameItem_OnEnter'
[string "*:OnEnter"]:1: in function <[string "*:OnEnter"]:1>

Locals:
(*temporary) = 2
mooreatv commented 5 years ago

when I have both aux-addon and my "better vendor price" I get

3x aux-addon\util\info-Info.lua:282: Usage: AuxTooltip:SetHyperlink(link)
[C]: in function `SetHyperlink'
aux-addon\util\info-Info.lua:282: in function `tooltip'
aux-addon\core\tooltip.lua:84: in function `extend_tooltip'
aux-addon\core\tooltip.lua:143: in function <aux-addon\core\tooltip.lua:140>
aux-addon\core\tooltip.lua:26: in function `SetBagItem'
FrameXML\ContainerFrame.lua:1318: in function `ContainerFrameItemButton_OnEnter'
[string "*:OnEnter"]:1: in function <[string "*:OnEnter"]:1>

Locals:
(*temporary) = AuxTooltip {
 0 = <userdata>
 updateTooltip = 0.200000
 needsReset = true
 BottomOverlay = <unnamed> {
 }
 TopOverlay = <unnamed> {
 }
 money = 0
}
(*temporary) = nil

this could be my addon's fault though not sure why they would interact that way

the relevant code in my addon is https://github.com/mooreatv/BetterVendorPrice/blob/master/BetterVendorPrice/BetterVendorPrice.lua#L213-L254

mooreatv commented 5 years ago

at the AH, with just your addon (and bugsack):

1x aux-addon\core\scan.lua:186: attempt to index global 'p' (a nil value)
aux-addon\core\scan.lua:186: in function <aux-addon\core\scan.lua:184>
(tail call): ?
(tail call): ?
(tail call): ?
aux-addon\control.lua:46: in function <aux-addon\control.lua:28>

Locals:
(*temporary) = nil
(*temporary) = 0
(*temporary) = 0
(*temporary) = 0
(*temporary) = 0
(*temporary) = "attempt to index global 'p' (a nil value)"

when doing a search for [Linen Bag]/exact (aux did auto complete when I typed linen, to the above)

any search really gets the above

maybe I am using the wrong branch ?

shirsig commented 5 years ago

It's the retail branch. Thanks for the bug reports. I hope I can get it mostly fixed till the release.

On Sun, Aug 11, 2019, 9:48 PM mooreatv notifications@github.com wrote:

Hi there, first I am using the "retail" branch, is that the right one ? (I noticed there is also a classic branch but less updated... which one is the right one?)

the error when opening mail:

74x aux-addon\core\tooltip.lua:148: Usage: GetInboxItem(messageIndex, attachIndex) [C]: in function GetInboxItem' aux-addon\core\tooltip.lua:148: in function <aux-addon\core\tooltip.lua:147> aux-addon\core\tooltip.lua:26: in functionSetInboxItem' FrameXML\MailFrame.lua:313: in function `InboxFrameItem_OnEnter'

Locals: (*temporary) = 2

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/shirsig/aux-addon/issues/197?email_source=notifications&email_token=AAKSKI2TKQB5HRQ22JXMSTTQEBUKJA5CNFSM4IK4SFCKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HETECIQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKSKI2FRRCB6NKRYYEGUNLQEBUKJANCNFSM4IK4SFCA .

mooreatv commented 5 years ago

the stress test servers seem to be still up so let me know (quickly) if there is some fix or something you'd like me to try (I am afraid our next chance after that is release day)

mooreatv commented 5 years ago

I made a full dump of the AH with GetAuctionItemInfo("list", i) for each result (and the item link too)

looks like this on classic and happy to share it

                    ["info"] = {
                        "Short Bastard Sword of Agility", -- [1]
                        135350, -- [2]
                        1, -- [3]
                        2, -- [4]
                        false, -- [5]
                        7, -- [6]
                        "REQ_LEVEL_ABBR", -- [7]
                        2000, -- [8]
                        0, -- [9]
                        2000, -- [10]
                        0, -- [11]
                        false, -- [12]
                        nil, -- [13]
                        "Hordemagic", -- [14]
                        nil, -- [15]
                        0, -- [16]
                        3192, -- [17]
                        true, -- [18]
                    },
                    ["link"] = "|cff1eff00|Hitem:3192::::::18:1775396224:10:::::::|h[Short Bastard Sword of Agility]|h|r",

btw if I may ask for a "trade secret" what do you wait on to know a) that GetNumAuctionItems("list") isn't 0 anymore / is correctly the full result count? b) that GetAuctionItemInfo() / GetAuctionItemLink() isn't going to be partially/completely nil ?

shirsig commented 5 years ago

Nothing comes to mind right now, thanks. I already identified some issues yesterday that will keep me busy.

The dump would be very useful.

After making a query there is an event to listen to for when the results come in, something like AUCTIIN_ITEM_LIST_UPDATE. After that GetNumAuctionItems should always be accurate for the last query. GetAuctionItemInfo should also return everything at this point except possibly the owner name which may come later with additional events. At least that's how it worked in real vanilla.

mooreatv commented 5 years ago

here is the dump: https://gist.github.com/mooreatv/1614c2393617bbcec10dfa4c7ed43b24 and the code that generated it is https://github.com/mooreatv/MoLib/blob/master/MoLibAH.lua#L38

thanks for the tip; added as TODO. About the nil item link and item text, I tested my code also on bfa and got 65k or so items, some of which had missing text and link somehow

shirsig commented 5 years ago

It seems in retail there is now an explicit hasAllInfo boolean from the GetAuctionItemInfo call, so I guess one would have to keep listening to update events until that is true (the reason for this whole mess is that the default AH shows partial data for performance, but actually not sure if this is even still the case). Seeing how fast the queries were during the stress test I think I'll just remove this optimization altogether and make it always wait for all info.

mooreatv commented 5 years ago

so the error on comment 1 is gone but... it also removed my tooltip/hook somehow ? the way I add my stuff is

GameTooltip:HookScript("OnTooltipSetItem", BVP.ToolTipHook)
ItemRefTooltip:HookScript("OnTooltipSetItem", BVP.ToolTipHook)

I guess your addon overwrites it ?

shirsig commented 5 years ago

I don't think so. I'm not hooking OnTooltipSetItem at all.

mooreatv commented 5 years ago

how do you add the Value: as that seems to be what replaces my 3 lines

btw I just re-tested the head of retail branch (on bfa as there is no classic atm) and a search for linen yields

1x aux-addon\core\scan.lua:231: Usage: GetAuctionItemInfo("type", index)
[C]: in function `GetAuctionItemInfo'
aux-addon\core\scan.lua:231: in function `data_complete'
aux-addon\core\scan.lua:214: in function `c'
aux-addon\control.lua:110: in function <aux-addon\control.lua:109>
(tail call): ?
aux-addon\control.lua:46: in function <aux-addon\control.lua:28>

Locals:
(*temporary) = 1
(*temporary) = "list"
mooreatv commented 5 years ago

mousing over mailbox item on retail is still:

63x aux-addon\core\tooltip.lua:148: Usage: GetInboxItemLink(messageIndex, attachIndex)
[C]: in function `GetInboxItemLink'
aux-addon\core\tooltip.lua:148: in function <aux-addon\core\tooltip.lua:147>
aux-addon\core\tooltip.lua:26: in function `SetInboxItem'
FrameXML\MailFrame.lua:313: in function `InboxFrameItem_OnEnter'
[string "*:OnEnter"]:1: in function <[string "*:OnEnter"]:1>

Locals:
(*temporary) = 1
(*temporary) = nil
shirsig commented 5 years ago

Should both be fixed.

mooreatv commented 5 years ago

cool ! indeed, new one now (mousing over my bags:)

17x aux-addon\core\history-History.lua:63: attempt to compare nil with number
aux-addon\core\history-History.lua:63: in function `value'
aux-addon\core\tooltip.lua:86: in function `extend_tooltip'
aux-addon\core\tooltip.lua:143: in function <aux-addon\core\tooltip.lua:140>
aux-addon\core\tooltip.lua:26: in function `SetBagItem'
FrameXML\ContainerFrame.lua:1403: in function `UpdateTooltip'
FrameXML\GameTooltip.lua:504: in function <FrameXML\GameTooltip.lua:483>

Locals:
item_key = "30810:0"
(*temporary) = nil
(*temporary) = 1566110065
(*temporary) = 1566110065
(*temporary) = nil
(*temporary) = nil
....

edit: this stopped happening after reload - bad history/savedvar from previous errors ? or first time seeing an item error? I can delete my saved var to see

mooreatv commented 5 years ago

repro steps: delete saved vars for aux-addon.lua search for "linen", click "linen cloth"

1x aux-addon\core\scan.lua:144: attempt to compare nil with number
aux-addon\core\scan.lua:144: in function <aux-addon\core\scan.lua:140>
(tail call): ?
(tail call): ?
aux-addon\control.lua:46: in function <aux-addon\control.lua:28>

Locals:
i = 1
page_size = nil
(*temporary) = <function> defined =[C]:-1
(*temporary) = <table> {
 list = <table> {
 }
}
(*temporary) = "list"
(*temporary) = "list"
(*temporary) = <table> {
 id = 18
 query_index = 1
 params = <table> {
 }
}
(*temporary) = 18
(*temporary) = 18
(*temporary) = 18
(*temporary) = "attempt to compare nil with number"
PAGE_SIZE = 50
info = <table> {
}
aux = <table> {
}
history = <table> {
}
shirsig commented 5 years ago

fixed the scan bug. not sure about the history bug. Noticed it myself before but haven't been able to figure out how it could happen without a corrupted history.

mooreatv commented 5 years ago

first click on post tab

1x aux-addon\tabs\post\core.lua:437: attempt to index field 'availability' (a nil value)
aux-addon\tabs\post\core.lua:437: in function `update_inventory_records'
aux-addon\tabs\post\core.lua:70: in function `pass'
aux-addon\aux-addon.lua:117: in function <aux-addon\aux-addon.lua:113>
aux-addon\gui\core.lua:259: in function `select'
aux-addon\gui\core.lua:234: in function <aux-addon\gui\core.lua:231>

Locals:
auctionable_map = <table> {
mooreatv commented 5 years ago

also on first login (checking starter edition so you can test without even a sub)

2x aux-addon\aux-addon.lua:184: attempt to index global 'account_data' (a nil value)
aux-addon\aux-addon.lua:184: in function `f'
aux-addon\aux-addon.lua:35: in function <aux-addon\aux-addon.lua:26>

Locals:
(*temporary) = <function> defined =[C]:-1
(*temporary) = aux_frame {
 0 = <userdata>
 content = <unnamed> {
 }
}
(*temporary) = nil
(*temporary) = "attempt to index global 'account_data' (a nil value)"
mooreatv commented 5 years ago

you can repro the above with a starter account, no sub needed, just try to list the Neophyte's Boots image

shirsig commented 5 years ago

I can't reproduce the account_data bug with arctium. Deleted both savedvariables and caches.

shirsig commented 5 years ago

By the way, are you testing this on current retail or is the classic beta still up? If it is and you have time there are two things I need to know about the API:

1) Is automated buying of auctions possible? You can test it by searching for some cheap item on the default AH and running this /run local f = CreateFrame'Frame' f:SetScript('OnUpdate', function() PlaceAuctionBid('list', 1, 1000) f:SetScript('OnUpdate', nil) end)

2) Is automated posting of multipe stacks possible? In vanilla StartAuction was used to post auctions. The function exists but the blizzard interface code seems to use only PostAuction. PostAuction seems to also support posting multiple stacks in a single call but perhaps they disabled this for classic. You can test it by putting an auctionable item in the auctioning slot and trying these: /run local f = CreateFrame'Frame' f:SetScript('OnUpdate', function() StartAuction(1, 1, 1) f:SetScript('OnUpdate', nil) end) Should post a single stack of 1. /run local f = CreateFrame'Frame' f:SetScript('OnUpdate', function() PostAuction(1, 1, 1, 1) f:SetScript('OnUpdate', nil) end) Should post a single stack of 1. /run local f = CreateFrame'Frame' f:SetScript('OnUpdate', function() PostAuction(1, 1, 1, 1, 2) f:SetScript('OnUpdate', nil) end) Should post 2 stacks of 1.

mooreatv commented 5 years ago

the beta is over (and I never had beta access anyway, I was using the stress test), so I am testing in retail, which you should do too :) (happy to test corner cases but the super basics would be nice you go over?)

https://us.battle.net/account/creation/en/tos.html?theme=wow&style=wow-trial

shirsig commented 5 years ago

Oh, well, I guess I'll have to wait for the release to test those two things as they are specifically about whether things are different in classic. I resubbed already to reserve some classic names but I'll have to download the client. I didn't know the API was similar enough to classic that it would make sense to test there. I'll give it a try.

mooreatv commented 5 years ago

I think the AH is pretty much unchanged compared to retail (except retail has the non silly sort by default) and different categories https://wow.gamepedia.com/Patch_1.13.2/API_changes

shirsig commented 5 years ago

Well, there are some subtle differences which might require very different solutions.

For example in retail there's PostAuction which can post multiple stacks per call. This function itself requires a hardware event though.

In vanilla there is StartAuction which can only post one stack but doesn't require a hardware event.

If they go with vanilla-like then I don't need to change anything (a lot of code for automating stacking and posting of items). If they go with retail-like I can replace all that code with one call to PostAuction.

If they use the vanilla function but restrict it to require hardware events I have to keep the automated posting but extend it to prompt the user for hardware events like TSM does.

mooreatv commented 5 years ago

the link above has all the difference between retail and classic (which are none for AH as far as I can see) https://wow.gamepedia.com/API_StartAuction says " StartAuction() requires a hardware event." so probably same but there PostAuction indeed

from a lua perspective most api work and are exactly the same vs retail (though maybe there are happy surprises but I doubt it)

shirsig commented 5 years ago

I wonder if postauction actually works though. The blizzard ah doesnt use it and on arctium it gave me some strange error.

mooreatv commented 5 years ago

how do you get arctium to have an AH ? (is there something else than the sandbox)

if you get the code to work on retail/bfa I'm pretty sure it'll work with classic as is (modulo the AH categories etc)

mooreatv commented 5 years ago

side notes

shirsig commented 5 years ago

It doesn't have a working AH api but you can still load the AH interface.

I've been testing on retail and got scanning to work fully I think. It seems hasAllInfo being true doesn't mean that the owner is there. I guess this must mean that even the other data might be missing, unlike in vanilla. Anyway, there shouldn't be timeouts/retries needed unless you're waiting for owner names (no way to tell whether it's still loading or one of the rare cases where it's unavailable afaik).

What do you mean by prefetching exactly?

By the way, do you know if getAll scans have been disabled in classic? That would of course be the best way to make snapshots if it's not disabled.

I don't use discord, sorry, and for now I don't think I'll have time for more addon development beyond porting from 1.12.

mooreatv commented 5 years ago

yeah the seller info is internally now guid based and the mapping guid->name comes in async it seems

getall worked in the stress test (that's how I produced the gist above) so I expect it'll work in classic at release (and I hope it stays working)

the algorithm I use for now is: try to get N items, remember the first one that was nil/incomplete; retry at that one for another N (skipping over the non nil ones) and so on

the question being what value of N is good or is there a better way (though it's pretty good as I can get upward of 10k auctions/sec that way)

shirsig commented 5 years ago

The seller was always coming in asynchronously, but I'm wondering if anything else is coming in asynchronously or else I don't know what the purpose of the hasallinfo value is (because it does not mean that the seller is available)

I'm not sure I understand. Is there a parameter for the page size that I'm not aware of? I thought there was only one state for the active page which is wiped when a new page is loaded. And since there is no unique id for an auction there is no way to remember an auction across pages, and pages may also change with auctions expiring/being bought out and the rest being moved to fill the gap, so position doesn't work either.

mooreatv commented 5 years ago

I'll get back to my addon scan later (maybe a separate thread) [the good news is my scan still happens when aux is the UI]

I updated the latest from github and aux-addon eliminates my tooltip still somehow how can I make it not replace my data by "Value ?" ?

without aux: image

with aux: image

code I use for the tool tip: https://github.com/mooreatv/BetterVendorPrice/blob/master/BetterVendorPrice/BetterVendorPrice.lua#L180

I really want to make sure our addons are compatible

ps: side note your 20c value is incorrect unless it's know by users that that's full stack and not their current stack?

shirsig commented 5 years ago

That 20c is not from me. Mine looks like the value but brown and you enable it with /aux tooltip merchant sell As for the conflict, no idea yet, I'll check.

mooreatv commented 5 years ago

so what seems to happen is your value and my last SetTooltipMoney value (the full stack one) collide... I wonder if I am doing something wrong and how to avoid/fix it

shirsig commented 5 years ago

I was overriding SetTooltipMoney because I wanted the sell price at vendors to be below my tooltip additions. I removed it for now and also removed the quantity feature from the tooltip and simplified it a lot.

mooreatv commented 5 years ago

oh I am so happy, it works beautifully now 👍 my info and yours don't collide anymore: image

random question: where do you get the DE into data from?

I think we can close this now and use new issues for new stuff

The biggest one for me would be how to have your search and UI use my last instant scan data ?

mooreatv commented 5 years ago

I found a bug with both our addons; when mousing over a recipe, it shows the info for the recipe twice, instead of once the recipe, once the created item, eg

image

shirsig commented 5 years ago

random question: where do you get the DE into data from?

From item class and item level. The only limitation is that it can't detect when certain special items cannot be disenchanted.

mooreatv commented 5 years ago

Thanks ! I noticed it was telling me about what I could get from my Greater Magic Wand I had just created :)

Is there an API / Which function should I search for?

any idea for double tooltip?

shirsig commented 5 years ago

You can see everything in my disenchant.lua. I'm using slot, quality, and level from I think GetItemInfo and then the table of which combination disenchants to what is just hardcoded also in that file.

Haven't had time to look into the tooltip issue yet.

mooreatv commented 5 years ago

I noticed you did fix the tooltip - yours now only show once for the recipe (though having the crafted results data would be even better but at least it's good now, for yours)

mind sharing what you did or pointing me in the right direction (as mine is still duplicate)?

TIA!

shirsig commented 5 years ago

I'm just hooking all the tooltip setter functions instead of that onitemset or whatever its called. (Didn't do it specifically to fix the recipe issue but for getting the stack sizes) you can see it all I think in the tooltip.lua file. (I'm abroad until friday and only have my phone)

I'll close this issue, there's too much in here.