doadin / Baggins

zlib License
6 stars 7 forks source link

Item quality improvements #88

Closed WanderingFox closed 1 year ago

WanderingFox commented 1 year ago

EDIT: See comment below, there's a helper method that I missed and crafting quality can be done with a 1-liner!

First thank you so much for implementing this! It works pretty good!

Having played around with it for a bit I've noticed a couple things:

  1. When an item with quality appears for the first time and it reuses a frame that didn't have quality the frame does not update until the button states update (something gets added or removed) image (The Hochenblume in that screen shot is quality 2 but it isn't displayed)

  2. It looks like the overlay frame is generated net new even if one already exists

I was able to fix the second one pretty easily with adding a few conditional checks:

    if Baggins:IsRetailWow() and p.EnableItemReagentQuality then
        local itemQuality = link and (C_TradeSkillUI.GetItemReagentQualityByItemInfo(link) or C_TradeSkillUI.GetItemCraftedQualityByItemInfo(link))

        if not button.ProfessionQualityOverlay then
              button.ProfessionQualityOverlay = button:CreateTexture(nil, "OVERLAY")
              button.ProfessionQualityOverlay:SetPoint("TOPLEFT", -3, 2)
              button.ProfessionQualityOverlay:SetDrawLayer("OVERLAY", 7)
        end

        if itemQuality then
            local atlas = ("Professions-Icon-Quality-Tier%d-Inv"):format(itemQuality)
            button.ProfessionQualityOverlay:SetAtlas(atlas, TextureKitConstants.UseAtlasSize)  
        end

        button.ProfessionQualityOverlay:SetShown(true)
    end

I can't figure out what keeps resetting the visibility state of the buttons though. If I toggle reagent quality off and on they all show up and work until the button visibility state changes

WanderingFox commented 1 year ago

Did some more digging and there's a helper for this that I missed! DOH

The whole thing can be reduced to

    if Baggins:IsRetailWow() and link and p.EnableItemReagentQuality then
        SetItemCraftingQualityOverlay(button, link)
    end

Works perfectly so far in my testing!

edit: One issue is toggling the option needs a reload currently with using the helper since there isn't a call to ClearItemCraftingQualityOverlay(button) anywhere

doadin commented 1 year ago

Thank you for this! Updated: 1e423505b8f845ad8d3a855c0c5c3ac39c6e8155

Let me know if you have any other issues/updates.

doadin commented 1 year ago

I don't know if its just the new one liner you posted but I am not seeing the quality for items like gear.

WanderingFox commented 1 year ago

Oh interesting... It works for profession gear (which is what I had used to test), but not actual crafted armor... let me poke at that.

edit: stranger still it works once an item moves... image

WanderingFox commented 1 year ago

Even weirder it works with potions and everything else... just not gear.

WanderingFox commented 1 year ago

Figured it out.

Apparently item quality only shows in your bags (even the blizzard ones) if a profession window is open!

image image

First image is the default bag frame with a profession open, second is the same default bag frame just after i closed the profession window.

Personally I think that's not terribly useful. Thankfully, there's an override!

This makes them show up all the time:

    if Baggins:IsRetailWow() and link and p.EnableItemReagentQuality then
        button.alwaysShowProfessionsQuality = true
        SetItemCraftingQualityOverlay(button, link)
    end

edit: Which is why it was working for me, I was opening alchemy to grab an item with quality... which was triggering all the frames to update their quality overlays...

doadin commented 1 year ago

ah, nice! I have added it in as an option so you can have it either way. Always or when profession is open.

doadin commented 1 year ago

I think this is working now so I am going to go ahead and close this.

doadin commented 1 year ago

Thank you for all your help!