Jaliborc / Bagnon

Single window displays for you items
196 stars 111 forks source link

7.2.2 - Pawn upgrade icon remains after consuming/moving #547

Open voidzone opened 7 years ago

voidzone commented 7 years ago

image

silox2 commented 7 years ago

Having this very issue . For me the "ghost" pawn upgrade arrows occur when I have (separate reagent bank) setting checked and then proceed to sort. Seems like an issue with the separate reagent bag option, I liked how it would only show the reagent bag and with a click it goes back to all the other bags. screenofbankwindows

christinaa commented 7 years ago

Can replicate, seems like a bug in ItemSlot:UpdateUpgradeIcon where the IsUpgrade may return nil for an empty slot, causing this function to repeatedly get called until the slot is no longer empty. I don't understand the purpose of the delay/retry thing there (seems to create infinite cycles for newly empty slots) so I won't touch it.

Easy fix would be to change external/Wildpants/components/item.lua, find ItemSlot:UpdateUpgradeIcon and add self.UpgradeIcon:SetShown(false) just above the self:After call, which will clear the upgrade icon if the slot has been updated and is now empty or any other reason for why this would return nil.

Not opening a PR since I'm not quite sure why the timer is there in the first place so I'll wait for the Bagnon developer to reply.

Jaliborc commented 7 years ago

I'm not the developer of Pawn, so theres nothing much I can do here. Has Pawn supported Bagnon before?

The "After" avoids the UI system from locking down Bagnon for computing too much into one frame. It divides the item layout phases into two render calls. The high priority things are done in the first. Everything related with searching and categorization is left to be called afterwords.

voidzone commented 7 years ago

There were no issues with it prior to the 7.2.2 release.

voidzone commented 7 years ago

I can open the same ticket with them if it'll help you guys collaborate?

voidzone commented 7 years ago

Pawn devs have apparently already commented about the issue:

VgerAN Posted 4 days ago Hi! The green arrows in Bagnon are actually put there by Bagnon, not Pawn. So unfortunately there isn't anything that I can do to improve that. (Pawn changes the way that WoW decides whether an item is worthy of having a green arrow, but the bag itself is what puts it there.) The Bagnon developers will want to know about this bug.

Sorry that I wasn't able to help! Good luck.

https://mods.curse.com/addons/wow/pawn?comment=4132

2ndHouse commented 7 years ago

Can confirm this problem only came about with the 7.2.2 and 7.2.3 versions

SirThrocken commented 7 years ago

Ditto here. Can remove the icon by dragging another item to the slot, just tedious. Persists through bag opening/closing as well.

Jaliborc commented 7 years ago

@voidzone Ok, thanks for contacting the Pawn developer. I'll check whats going on.

voidzone commented 7 years ago

Pawn Version 2.2.5 Made a change to try to improve integration with Bagnon.

It's not perfect but it's better. There's a delay but the icon now clears.

[added]

VgerAN - Posted 2 days ago I don't have an account on GitHub, but it might help them to know that the only thing Pawn does to affect the green arrows in bags feature is to replace "IsContainerItemAnUpgrade". They'll know what that means.

https://mods.curse.com/addons/wow/pawn?comment=4136

christinaa commented 7 years ago

Well, this is probably a dirty fix but it works. I explained it above, could merge it if there's no regressions.

function ItemSlot:UpdateUpgradeIcon()
    local isUpgrade = self:IsUpgrade()
    if isUpgrade == nil then
        self.UpgradeIcon:SetShown(false)
        self:After(0.5, 'UpdateUpgradeIcon')
    else
        self.UpgradeIcon:SetShown(isUpgrade)
    end
end

And yes this is not related to Pawn but rather to Bagnon not resetting UpdgradeIcon if IsUpgrade returns nil, which it does on empty bag slots.