moody / Dejunk

Simple junk selling and destroying for WoW.
https://www.curseforge.com/wow/addons/dejunk
MIT License
10 stars 3 forks source link

Newest Dejunk version appears to cause a crash when interacting with a vendor on Mac #190

Closed LordOfOtakus closed 8 months ago

LordOfOtakus commented 10 months ago

Title says it all, but I will note it only happens on my main, none of my other toons seems to experience an issue, so it may be related to some character specific settings.

Edit: Attempting to open the Junk List also crashes the game. Turning off "Exclude Equipment Set" fixed all of it, though.

moody commented 10 months ago

I see the problem. Will fix asap.

moody commented 10 months ago

Hopefully this is fixed in version 1.7.1.

LordOfOtakus commented 10 months ago

The game now crashes if I even attempt to log into my main with the addon loaded, or attempting to load the addon after logging in. Alts are still fine.

Edit: I should maybe mention, the crash log specifically mentions that WoW is returning a "stack buffer overflow" as the reason for the crash.

Edit 2: The crash doesn't occur on my alts no matter what I do. Deleting all my sets on my main fixed it, but it just started again when I re-made them. One last thing I can think of, way early on in Wrath, the Equipment Manager was super buggy, and at one point I know my main had corrupted set data. I think Blizzard had resolved it all, but thats the only thing I got.

moody commented 10 months ago

I can't seem to reproduce crashes in either Wrath or Retail, but it seems likely that the issue stems from calls made to C_EquipmentSet functions. I may need to try refactoring the code to perform tooltip scans for equipment sets instead (which is expensive and has the potential to dip frame rates, but it's better than crashing).

LordOfOtakus commented 10 months ago

I have another update, I poked around in what Dejunk is doing, I narrowed the crash down to being caused by or related to Lines 38-40 in equipment-sets-cache.lua

if bag ~= nil and slot ~= nil then cache[getCacheKey(bag, slot)] = true end

Edit 2: Nevermind, it only stops crashing when that block is commented out with a / /, not individually with --. In fact, just putting /**/ at the very end of the file makes it stop crashing and instead lets me log in and see UI errors.

Lots of spam about items.lua attempting to call Refresh, a nil value

Edit 3: I am dumb and didn't know Lua had different multi-line comments. Checking again now.

Edit 4: The actual issue seems to perhaps step from the stuff around line 27 of equipment-sets-cache.lua

for _, equipmentSetId in pairs(CEquipmentSet.GetEquipmentSetIDs()) do for , itemLocation in pairs(CEquipmentSet.GetItemLocations(equipmentSetId)) do if itemLocation and itemLocation ~= 1 then local , , , voidStorage, slot, bag = EquipmentManager_UnpackLocation(itemLocation)

LordOfOtakus commented 10 months ago

Alright, maybe last post, I ran pieces of the Refresh function as individual bits using Macros and printed the output, when I ran /run print(C_EquipmentSet.GetItemLocations(0)), the game crashed. the SetIds from the previous step were 0-5, since i have 6 sets. the GetItemLocations function causes the buffer overflow i mentioned before.

LordOfOtakus commented 10 months ago

HEY! I found the cause. Running C_EquipmentSet.GetItemLocations() when your set has 19 items saved (aka, all slots in Classic) it crashes.

This is likely a Blizzard issue, so maybe report it to them if you can. It may also be Mac-only, so I made a post on the Mac Technical Support forums.