Baganator

Baganator

1M Downloads

Add border color for items used in an item set

ghyatzo opened this issue ยท 41 comments

commented

Is your feature request related to a problem? Please describe.
Not a problem

Describe the solution you'd like
All items not currently equipped but part of a saved item set have a light blue glow/border to indicate they are being used in a set.

Describe alternatives you've considered
Maybe use one of the four corner, although a change of border color seems the cleanest one, also visually.

Additional context
Something like this:
Screenshot 2023-11-17 at 01 10 28

Item rarity is still preserved in the iLvl text color, but gives a pretty clear indication of which items might be leftovers of old sets.

commented

Hi. I've got a report that the issue may be fixed now. Would you be able to check that by running the chat commands that caused it to crash before?

commented

Sorry the wait, got sidetracked. I could not crash the client anymore using /dump C_EquipmentSet.GetItemLocations(1) with a full 19 slot equipment set.

Don't know what they did but it seems fixed now!

commented

Excellent. You'll find the equipment set code now works on your Mac with the latest Baganator releases.

commented

The PR #18 is obviously incomplete, to make it proper I require some help from you.

  • where is the best location to save (and cache?) the list of items in a set?
  • where do you think should go the logic to populate such list?
  • where should we register the event to update the list upon equipment update?

I am not familiar enough with the codebase to make these decisions by myself, and I don't really have the time to properly explore it. Any insight will be appreciated.

commented

Update: Put together an preliminary version in a separate branch now. https://github.com/Auctionator/Baganator/tree/equipmentsets It does update whenever the gear changes. However I need to change the point its integrated as it generates an extra bag view refresh each time.

Baganator-0.66-1-g3ecf6c6.zip

commented

I can try and create a PR for this (i'm proficient with lua and wow addons).
It would help a lot to have a general idea of where this kind of logic is implemented in the code!

commented

I did look at it, but couldn't figure out how to make the API for checking if a bag item is in a set work

commented

That sounds good. The item button info logic is handled in https://github.com/Auctionator/Baganator/blob/main/UnifiedBags/ItemButton.lua

commented

The only way I could come up is something along the lines of:

local items_in_sets = {}
-- populate all the items that are in a set.
for i=1,GetNumEquipmentSets() do
  local set = GetEquipmentSetInfo(i);
  local itemArray = GetEquipmentSetItemIDs(set);
  for i=1, 19 do
    if itemArray[i] then
      table.insert(items_in_sets, itemArray[i])
    end
  end
end

foreach item in bag:
  if item in items_in_set => change border color

I expected there was a more direct way, since default tooltips show you if an item is in a set as I can see at the bottom
Equipment set: "set name" on items in my bags

commented
07:34 Dump: value=C_EquipmentSet.GetItemIDs(1)
07:34 [1]={
07:34   [1]=50026,
07:34   [2]=42045,
07:34   [3]=50113,
07:34   [4]=3342,
07:34   [5]=47603,
07:34   [6]=51908,
07:34   [7]=47062,
07:34   [8]=49894,
07:34   [9]=47585,
07:34   [10]=50107,
07:34   [11]=50399,
07:34   [12]=47224,
07:34   [13]=45929,
07:34   [14]=47041,
07:34   [15]=51332,
07:34   [16]=50028,
07:34   [17]=45314,
07:34   [18]=50454,
07:34   [19]=5976
07:34 }

The second one doesn't print anything

commented

Ok. Thanks for running all the commands.

It seems there isn't any way to differentiate an equipment set that causes a crash from one that doesn't.

commented

I've opened a bug report for Blizzard to review to see if the crash can be resolved: Stanzilla/WoWUIBugs#511

commented

Good call, I was actually thinking of doing it myself. I also refrained from updating the equipment sets in anyway in fear to lose the reproducibility on this absurd bug.

commented

I had the time to test it, apparently it makes my game client crash due to a stack buffer overflow.
I am on a Mac Client

commented

I have a new version that hopefully works on your system:
Baganator-0.74-2-gb86877e.zip

commented

When it crashes the client does it happen after opening the bags, on login, or something else?

(this version probably won't fix the bug, but it contains the latest fixes)
Baganator-0.75-1-g83cb00f.zip

commented

Almost complete. Got to add a visible option to disable the feature, but then its done:
Baganator-0.74-4-geccb151.zip

image (retail)
image (classic)

commented

I have an absurd bug. On one character it works fine, on another it crashes the client completely.
In the following days I will try to reset some stuff and try to figure out what it is that breaks.
Specifically to this version, the normal one works ok.

Thanks a lot for the work <3

Edit: the version that crashes was Baganator-0.74-2-gb86877e.zip

Haven't tested the new one yet.

commented

Upon logging in that character (after the loading bar completes). I only tested on two characters so far, one working the other crashing.

commented

Would you be able to run these chat commands and report if any crash the client on the character that won't load?

/dump C_EquipmentSet.GetEquipmentSetIDs()
/dump next(C_EquipmentSet.GetItemLocations(C_EquipmentSet.GetEquipmentSetIDs()[1]))
/dump EquipmentManager_UnpackLocation(next(C_EquipmentSet.GetItemLocations(C_EquipmentSet.GetEquipmentSetIDs()[1])))
commented

Ok, just got home, so:
/dump C_EquipmentSet.GetEquipmentSetIDs() returns:

Dump: value=C_EquipmentSet.GetEquipmentSetIDs()
[1]={
   [1]=1,
   [2]=0,
   [3]=2
 }

while /dump next(C_EquipmentSet.GetItemLocations(C_EquipmentSet.GetEquipmentSetIDs()[1])) hardcrashes the client

Edit: The problem seems to be with GetItemLocations and not next.

Edit 2: Not only that, it does not crash when calling the second or third item set, only with C_EquipmentSet.GetEquipmentSetIDs()[1]

commented
07:06 Dump: value=C_EquipmentSet.GetEquipmentSetInfo(C_EquipmentSet.GetEquipmentSetIDs()[1])
07:06 [1]="SpellPower",
07:06 [2]=132129,
07:06 [3]=1,
07:06 [4]=false,
07:06 [5]=19,
07:06 [6]=2,
07:06 [7]=17,
07:06 [8]=0,
07:06 [9]=0
commented

Can you try

/dump C_EquipmentSet.GetEquipmentSetInfo(C_EquipmentSet.GetEquipmentSetIDs()[1])
commented

works

commented

What's the result?

commented

(the chat output)

commented

Can you also try

/dump C_EquipmentSet.CanUseEquipmentSets()
commented
07:17 Dump: value=C_EquipmentSet.CanUseEquipmentSets()
07:17 [1]=true

I have to get out again shortly, I'll keep testing when I get back.

commented

Got some more:

/dump C_EquipmentSet.GetItemIDs(1)
/run for _, i in pairs(C_EquipmentSet.GetItemIDs(1)) do if not C_Item.DoesItemExistByID(i) then print(i) end end

Can you post the results they generate in the chat window?

commented

Be mindful of the edge case where a character has a 19 item set and a full bag, at that point the prevention may fail.
I added a comment to the blizzard bug report adding this information.

commented

I've acquired a full item set (19 items) and been unable to trigger a crash from it. Albeit I'm at level 58 with a bow instead of a relic (warrior). Also tested the PTR at level 80, and no crash either (also a warrior). Maybe it's class specific?

commented

Hi again. Can you test with just the Baganator addon enabled and report if the game still crashes? Searching Github shows a lot of addons using the equipment set item locations API so it seems weird that they don't all have crashes too.

commented

The game was crashing without even calling bagnator, but just by invoking that specific equipment set API command.
So I believe that the issue is not with bagnator itself, and probably even the API but somehow my equipset (since is that particular one) got corrupted somehow.

I guess this might one of those bugs impossible to fix, so I'll try to delete that set and make it again and see if the bug persists.

commented

I was thinking that another add-on could be doing something that breaks it. So testing with no other add-ons is to eliminate that possibility

commented

Ok tested without addons and it still crashes.
Also, and this is wierd, the crashing seems to be linket with that specific item set.
I tried deleting it and creating it again, but it still makes the game crash when called:

/dump next(C_EquipmentSet.GetItemLocations(C_EquipmentSet.GetEquipmentSetIDs()[4]))

Note that now I am calling 4, instead of 1, since the "new" item set is the last in the list. Otherwise now

/dump next(C_EquipmentSet.GetItemLocations(C_EquipmentSet.GetEquipmentSetIDs()[1]))

works properly

commented

Ok this is absurd....
Since It is clearly linked to that specific equipment set I tried playing with it, by testing various different test sets and then running: /dump C_EquipmentSet.GetItemLocations(1) (the set ID is 1) which is the call that crashes the game.

  • I removed all items, and saved the set as empty: Works
  • I added only the left half of items in the character pane (head, neck, shoulders, ..., up until wrists): Works
  • I added only the right half + weapons of items in the character pane (MH, OH, relic, hands, waits ... etc): Works
  • Full set: Crashes

EDIT: It crashes only if i save the set with All items (tabard and shirt as well), if I remove even only one of them it's ok.
I have no clue at this point. I guess I'll just save the set without the shirt and live with it ahaha

commented

That's very weird. Are your other sets full sets too?

commented

Yes, but not really. They are full item Sets, but using a 2H so only 18 items total.
Just tested, tried to replace MH+OH on my feral sets using a 2H and I get a crash.
It seems then that the main cause is the saved set being 19 items.

Ignoring Tabard and Shirt from the set, makes it work...

This really feels like an OOB error somewhere where the whole OS panics. On the crash I don't get the WOW crash window, but the Apple crash report one, so I guess it is at a low memory level, and not an uncaught exception within the client. or something like that

commented

Ok. Now that we know that's it is full sets only I can write a mitigation that removes an item from the set (probably a shirt), gets the locations of everything else and then puts the shirt back in

commented

This is also good for the blizzard bug report at this will probably replicate on other profiles

commented

As this is unfixable (due to it being a Mac version of WoW bug causing the crash) I'm going to close the issue. The item set tracking works on Windows versions of WoW, so the feature is enabled there.