kstange / MerchantPlus

This addon provides a modern update to merchant (vendor) interfaces in WoW
MIT License
4 stars 0 forks source link

Merchant buyback issue #10

Closed TomCats closed 1 month ago

TomCats commented 3 months ago

Hi - I've been trying out your addon - nice job! Unfortunately, it's doing something that's interfering with the buyback feature with vendors. I can click on the items but it won't but them back unless I disable the addon

kstange commented 3 months ago

Hi there.

Thanks for trying out Merchant Plus!

I tried to reproduce this but so far I haven't been able to. My guess is that there's a conflict between my addon and something else you have installed. I try to be pretty low-touch on built-in functionality so I don't run buyback actions directly from Merchant Plus and I leave the existing buyback tab intact. The only thing I do is watch for when you buy something back from that tab and sync that change with the quick buyback button if needed.

So I guess in order to figure this out:

1) Are you buying back from the quick button on the Merchant or Merchant Plus tabs, or from the Buyback tab? Or does it happen in both cases? 2) Are you getting a Lua error in the game? If you don't have errors enabled you can run /console scriptErrors 1 from the chat window, then try the action and see if you get an error. When you're done, make sure to run /console scriptErrors 0 or you will continue to get a popup on every UI error. If you get an error, please copy and paste the whole error here and I'll see what I can do that that information. 3) If you do not get an error popup, then the next step is to look at any addons and weak auras you have installed. If you have anything that specifically manages inventory in some way, that's a good place to start. You could try turning things off one at a time and seeing if the issue goes away at some point, or running Merchant Plus without other addons enabled and then enabling them one at a time to see when the problem starts happening.

If you do identify a problem addon or weak aura either through a Lua error or trial and error, let me know which one and I'll try to reproduce the issue and see if I can find a workaround or suggest a bug fix to the author.

TomCats commented 3 months ago

Lua errors are enabled for this character but none appeared. I am not able to reproduce it so the nature of the issue may be intermittent. I have minimum addons on this character since it's an underutilized alt - At the time I had HandyNotes 1.6.20.3, HandyNote: Mists of Pandaria (Treasures and Rares) v33, Merchant Plus 11.0.0.1, TomCat's Tours 2.5.40, TomTom v4.0.1-release, Trove Tally 1.0. The incident was captured on my livestream so maybe you see something that I don't, but for sure something to keep on your radar if you get other reports. Here is the VOD in case it helps later on - it should be available for 60-90 days. https://www.twitch.tv/videos/2231686002?t=01h57m01s

kstange commented 3 months ago

Interesting. I don't see anything obvious from the clip. I haven't noticed an issue before and I do use the addon every day, though I haven't played as much since 11.0.0 as I usually do, so it could be something new in this patch.

I don't know anything about TomCat's Tours or Trove Tally. But the other addons you've listed are certainly not ones I expect to cause issues. I have TomTom, HandyNotes, and I had HandyNotes: MoP installed recently. I'm not sure what's changing the format of your Addon enable/disable frame, but that's an interesting enhancement.

I didn't actually check anything about your profile when I gave you really basic troubleshooting suggestions, but since I see you're an addon author as well it makes sense you would have looked at all the stuff I suggested already. Consequently, I'm going to now speculate much more technically!

I wonder if there's a taint issue that's causing the BuybackItem() function to break. One thing that would be interesting to try is /run BuybackItem(slot) if you manage to reproduce it again and see if that runs cleanly. It's a C function which means we can't really look at how it's implemented or what it values it reads, but there are only two places where BuybackItem() is triggered and they're both from button click handlers:

The buttons in the buyback tab pass the result of self:GetID(), which is set on each button by MerchantFrame_UpdateBuybackInfo(). Calling that function from an addon could taint the buyback button. Merchant Plus doesn't call it or its antecedent function MerchantFrame_Update() because of rampant taint issues trying to do so, but if some of my code lead to that function being called somehow, it's possible. I try to be pretty careful about tainting things because all the other merchant addons I've used were riddled with those kinds of issues. It's one of the reason I made this one!

All of that said, the quick buyback button just takes the result of GetNumBuybackItems() (another C function) so I can't see how that would be broken by taint unless the OnClick action is being blocked, and you were definitely not able to use that button either.

If you find a way to reproduce this issue reliably or discover anything else interesting, let me know. I'll also keep an eye out for it. If I can narrow down the order of operations causing it I might be able to mitigate it.

kstange commented 3 months ago

Actually I re-watched the whole section from when you logged into the character which wasn't very long and I don't think you used the quick buyback button. I originally thought you clicked it, but you just moused over it, so maybe that button was still working. I still don't see a pattern here that makes sense for breaking things, but I'd be interested if the quick buyback button still works if you reproduce it as well.

kstange commented 3 months ago

Oh, one more thing to try when you reproduce the issue: /run MerchantPlus.Trace = true and see what messages are printed when you click the button. Then set it back to false to stop the messages from being printed.

kstange commented 3 months ago

An update! While I was trying to reproduce your issue, I discovered a different a bug caused by overzealous clearing of tainted values that caused some issues when using the buyback tab, though I couldn't find any case where the buyback tab itself stopped working. In any case, it's possible that fix addresses your issue as well.

Check out version 11.0.2.0 and let me know if you ever manage to reproduce this problem again!

TomCats commented 3 months ago

I'll give it a try - I was only every able to produce the issue that one time, and, well accidentally vendoring things and buying them back isn't a frequent activity, but maybe while I'm cleaning up all the alts for the new patch I'll have some opportunities here. I'll be sure to keep the addon enabled for all characters just in case I can come across it. Feel free to stop by the stream any time - I do live coding and work on addons quite often and other addon developers are definitely welcome.

kstange commented 1 month ago

Going to close this issue for now, but feel free to reopen if this issue resurfaces.