Mirroar / TopFit

A World of Warcraft Addon that helps you with Equipment choices.
http://www.wowinterface.com/downloads/info16177-TopFit.html
8 stars 6 forks source link

Update code to account for Unique Equipped #44

Closed ghost closed 9 years ago

ghost commented 9 years ago

Once you complete the quest to turn [Solium Band of Endurance] into [Timeless Solium Band of the Bulwark], TopFit doesn't know how to handle both rings. Both are better than anything else, so it just shows a long chain of, "You can only have one..." errors.

That's fine. But the issue then is that it's really random which one gets equipped. I hit the "Play" button on the character menu... sometimes it's my 640 ring, sometimes it's the 680 ring.

Ideally it should check the "Unique Equipped" set type and only use one from each.

! This is a nice to have, clearly the easiest thing is for me to put my old ring in the bank.

Mirroar commented 9 years ago

The code is actually supposed to check for the "Unique" and "Unique Equipped" properties of items, so that seems to be buggy. I don't have the 680 ring yet so I can't directly test it yet.

ghost commented 9 years ago

You can get the same error by buying an alternative spec ring from the vendor right outside the house... so like for my Pally I have Tank and Ret rings... and it wants to equip them both but gets hung up.

Buy the ring here:

11-28-2014-5-49-17-pm-02a0

ghost commented 9 years ago

I have

http://www.wowhead.com/item=118290

and

http://www.wowhead.com/item=118293

Mirroar commented 9 years ago

Blizzard has API functions to detect uniqueness of items. It tells you either "You can only have x of this item" or "You can only have x of this unique category of items".

In the case of "Warlords-Crafted"-Items, like http://www.wowhead.com/item=109173&bonus=527, we get an ID telling us what items belong into a unique category. You can see how it says "Unique-Equipped: Warlords Crafted (3)" in the tooltip.

In the case of Solium bands, the tooltip only states "Unique-Equipped", and so does the API function. The only way we can interpret this is by checking for matching item-IDs and names of items, but these rings differ in both name and ID.

We're looking for a solution, but if all else fails we'll have to hardcode the item-IDs of these rings...

ckaotik commented 9 years ago

Basically this is one of the many Blizzard API issues we've run into:

For now since the last hotfix, lower level rings should be properly removed by the game. Therefore the issue of TopFit trying to equip two different tiers of rings would be ... avoided. However, there is still the component when additional rings have been purchased.

ghost commented 9 years ago

I'll forward these on to a friend at Blizzard.

Mirroar commented 9 years ago

Added some workaround code to fix this problem for now, so I'm closing this ticket.

ghost commented 9 years ago

This was marked as closed, but I can't get the latest code to work. What's live on Curse now is still broken, any ETA on this fix for the various Legendary Quest rings? What file do I have to download to get this to work?

ghost commented 9 years ago

Nevermind, I'm dumb. I was uploading the file to the wrong location. Replaced the two files I have with the updated versions, works like a charm! Thanks!

Mirroar commented 9 years ago

Have you tried the current git version? I'm doing some testing to see if there's any lua errors popping up, but the uniqueness bug is definitively resolved. Once the new version is uploaded to curse, it should work fine for everybody.

Mirroar commented 9 years ago

Okay, or that :)

ghost commented 9 years ago

Ok, so small feedback:

When no rings are equipped. It works. Equips the correct rings, only tries to equip one ring.

When the correct ring is equipped. It works. Doesn't try and equip the incorrect spec ring.

When the incorrect ring is equipped... like when I change from Ret to Prot... it still gives the "only 1 ring..." error.

Mirroar commented 9 years ago

While that might be a bit annoying, it's totally normal. The error message pops up when TopFit tries to put the Solium Band in the ring slot that doesn't have the other Solium Band, so the UI correctly throws an error. At the same time, the old Solium Band gets swapped for a new ring, and TopFit tries equipping the new Solium Band again a few moments later - this time, successfully and without error.

This could already happen before with unique items, or if TopFit tried to equip an off-hand item when the character was wearing a two-hander. While it might be possible to fix, the error message is only a minor inconvenience and in my opinion not worth the considerable effort of changing the code that equips the new items.

ghost commented 9 years ago

I am not seeing that the correct ring ever gets equipped. The error message comes up, I left it alone for a whole minute, and I still had the incorrect ring on after changing specs. I think most people who have multiple rings will have multiple specs, so this error will be common. It's the number one thing the raid group I have using this addon complains about right now.

Mirroar commented 9 years ago

I really can't reproduce this on my end and have no idea what would be causing it. It works fine for me, both when calculating the different sets one after another, as well as when changing specs, i.e. when auto-equipping happens. In my case it's between Restoration and Enhancement on a Shaman.

I'm going to release the new version for now, since it's an improvement if it fixes the problem for some people even if not for everyone.