search

doadin / Baggins

zlib License
6 stars 7 forks source link

Currently equipped weapon/armor are showing in bags #112

Closed Anemun closed 2 months ago

Anemun commented 2 months ago

Good day to you! I'm playing Cataclysm Classic, russian language. After updating to the new version of Baggins 5.2.0-cata, the inventory now displays items that are currently equipped on the character: weapons, armor, and some (but not all) bags. baggins-issue

Elberet commented 2 months ago

I have the same issue as OP, also on Cataclysm Classic with the english language client.

I've also tried downgrading to v5.1.9 and v5.1.8 without success, so presumably the Small Indie Company silently changed some APIs?

doadin commented 2 months ago

Apparently there was api for keyring in cata ,maybe left over from wrath? idk but apparently keyring is not a thing in cata? Thats part of the issue second part is for some reason Baggins thinks items equiped is in the keyring, im trying to look into it. Sorry for this.

Anemun commented 2 months ago

empty keyring slots are gone, thank you! equipped items are still showing in bags, looking forward for the fix

Elberet commented 2 months ago

Commenting out lines 446 and 447 in ForceFullUpdate() removes equipment slots from Baggins:

https://github.com/doadin/Baggins/blob/eb29b2d89f729c7caedc10f04479f0d6f5d69e5d/Baggins-Filtering.lua#L441-L449

Seems related to the changes to how the bank is handled, since bank slots are no longer retrieved as slots in bag -1, but through the character inventory API?

Anemun commented 2 months ago

Commenting out lines 446 and 447 in ForceFullUpdate() removes equipment slots from Baggins:

https://github.com/doadin/Baggins/blob/eb29b2d89f729c7caedc10f04479f0d6f5d69e5d/Baggins-Filtering.lua#L441-L449

Seems related to the changes to how the bank is handled, since bank slots are no longer retrieved as slots in bag -1, but through the character inventory API?

worked for me, thanks! upd: not really, after any change in bags (new items or even item moved) equipped items shows up again

Elberet commented 2 months ago

Yeah, you also need to change the comparison in Baggins.lua:1155 from <= 4 to < 0.

Anemun commented 2 months ago

Yeah, you also need to change the comparison in Baggins.lua:1155 from <= 4 to < 0.

that worked, thank you

doadin commented 2 months ago

@Elberet @Anemun maybe you guys can confirm, but making those changes breaks the bank updating, puting items in or removing and bank bags never update, even if I close bank and go back.

Elberet commented 2 months ago

Yeah, it's most likely the change in Baggins:OnBagUpdate that the bank. I believe the BAG_UPDATE event still sends -1 when the bank contents were updated, even tho items in that bank should really be queried through the inventory API instead of the container API. I really just hacked the thing so I could play the game. 😉

That said, I've changed the conditional to if bagid < -1 then return end and that seem to allow bank updates as normal.

At least in Cataclysm right now. I haven't the foggiest how any of these changes would affect classic or retail.

doadin commented 2 months ago

@Elberet @Anemun I have commited this change and pushed out a release with it, https://github.com/doadin/Baggins/commit/a68c391a53473a59da08d821b90759ed39f5a122 . This seemed to work including bank updating and changes only apply to cata, Other wow versions don't appear to have this issue, either they are not using new api or there is a bug in the cata api, either way for now this change will only apply to cata. Let me know if you guys still have issues.