Xruptor / BagSync

BagSync tracks your characters items and displays it within tooltips.
http://www.wowinterface.com/downloads/info15351-BagSync.html
Other
32 stars 21 forks source link

[Bug?] Currency tab tooltips #325

Closed zeenk closed 6 months ago

zeenk commented 6 months ago

I have this setting enabled to only show the tooltips in search but I still see the tooltip info in default currency tab 2024-02-19_04-29-53 2024-02-19_04-30-42

Xruptor commented 6 months ago

Hello there! Terribly sorry for the delay, I've been out of town for the last few days. That's actually not a bug. A long time ago there were were a couple of people that didn't want the item totals to show up in any of their bags except in the BagSync Search window. The exception to this was the currency counts which is calculated separately than item counts. Instead, they still wanted currency counts to show up as long as you were hovering over the currency in the tab which is out of the way, unlike bags which are used often. Perhaps the description of that feature should be updated to mention item counts specifically. In addition I suppose I can add an option to hide currency counts in the currency window as well. I'll have to think on it and see if it's a worthwhile addition.

zeenk commented 6 months ago

Hello there! Terribly sorry for the delay, I've been out of town for the last few days. That's actually not a bug. A long time ago there were were a couple of people that didn't want the item totals to show up in any of their bags except in the BagSync Search window. The exception to this was the currency counts which is calculated separately than item counts. Instead, they still wanted currency counts to show up as long as you were hovering over the currency in the tab which is out of the way, unlike bags which are used often. Perhaps the description of that feature should be updated to mention item counts specifically. In addition I suppose I can add an option to hide currency counts in the currency window as well. I'll have to think on it and see if it's a worthwhile addition.

No worries :) Yeah I was already using character info display from Baganator for the tooltips so was a bit annoying to have double displays for that so had to disable the BagSync currency module entirely, an option to disable BagSync info in default currency tab would be nice to have though so I could still get to use the currency module in BagSync in case I'd want to check a currency that my current character does not have 😄

2024-02-20_23-57-26

zeenk commented 6 months ago

Thanks for adding the option 😄 however, I found a bug(?) If you disable the "Display BagSync tooltip data in the Blizzard Currency window." it also disabled the tooltip data in BagSync currency module, which I assume was not intended?

https://github.com/Xruptor/BagSync/assets/68170439/cc97aa6b-f268-47d3-802f-350b58ac0b2f

zeenk commented 6 months ago

Also noticed that Trader's Tenders currency does not work in BagSync, probably because it's an account-wide currency

https://github.com/Xruptor/BagSync/assets/68170439/b24d4c2b-c6c4-4169-a88a-5151120880db

Xruptor commented 6 months ago

Yep that is definitely a bug. Odd, it shouldn't be doing that at all. I'll have to see what's going on with the tooltip capture. Thanks for reporting it. I'll look into it.

Also yes Tenders where blacklisted as they are a account-wide currency. It was summing up the same amount for multiple characters. 30+30+30+30 etc... instead of just 30.

zeenk commented 6 months ago

Yep that is definitely a bug. Odd, it shouldn't be doing that at all. I'll have to see what's going on with the tooltip capture. Thanks for reporting it. I'll look into it.

Also yes Tenders where blacklisted as they are a account-wide currency. It was summing up the same amount for multiple characters. 30+30+30+30 etc... instead of just 30.

For the tenders issue, you could maybe add a special check to ONLY displaying the current character and only grab the tenders amount from 1 character so it atleast still works in the currency module? 😛

Xruptor commented 6 months ago

Yep that is definitely a bug. Odd, it shouldn't be doing that at all. I'll have to see what's going on with the tooltip capture. Thanks for reporting it. I'll look into it. Also yes Tenders where blacklisted as they are a account-wide currency. It was summing up the same amount for multiple characters. 30+30+30+30 etc... instead of just 30.

For the tenders issue, you could maybe add a special check by ONLY displaying the current character and the tenders amount for that one so it atleast still works? 😛

Believe it or not, I had it that way before. People still complained. They said they wanted to see the counts for the other characters, not knowing it's account wide. Then they would file a ticket saying it's "broken" lol. Then there was a number of people that just wanted it blacklisted since it was only really necessary at the trader itself. XD I can never satisfy everyone.

zeenk commented 6 months ago

Oh lol, I see. You could add a text at the bottom in the tenders tooltip under the character that says (This currency is account-wide) to avoid issues with people thinking it's broken so other people still can use it to see their current amount while browsing the currency module in BagSync, but yeah do whatever you think is best there for the addon. (if you still want to blacklist it or not) 😛

Xruptor commented 6 months ago

@zeenk Please test out this experimental version. Let me know if the currency window functions properly now.

In addition can you check to see that ONLY your character is shown for the tenders with a note for it being account-wide?

BagSync.zip

Let me know if it works please. Otherwise I'm just going to blacklist it again.

zeenk commented 6 months ago

Perfect, everything works now 👍

2024-03-04_22-37-40 2024-03-04_22-39-15 2024-03-04_22-40-51

zeenk commented 6 months ago

Might have found another bug though with the modifier key setting for people that use that.

If you set it to ALT for example it doesn't work in BagSync search window when you hold down ALT to toggle the data, it only seem to work in bags, didn't test bank and in both blizzard and the currency module it's always showing the data even if you have it set to only show when holding ALT

2024-03-04_22-50-30

Xruptor commented 6 months ago

Might have found another bug though with the modifier key setting for people that use that.

If you set it to ALT for example it doesn't work in BagSync search window when you hold down ALT to toggle the data, it only seem to work in bags, didn't test bank and in both blizzard and the currency module it's always showing the data even if you have it set to only show when holding ALT

2024-03-04_22-50-30

That's by design. It's supposed to work that way. The modifier is only for stuff outside of the BagSync windows.

Xruptor commented 6 months ago

@zeenk okay thanks for testing, I'll go ahead and push the updated fixes then.

zeenk commented 6 months ago

That's by design. It's supposed to work that way. The modifier is only for stuff outside of the BagSync windows.

So you would have to click the key icon to see tooltip data inside BagSync search everytime if you use tooltip modifier? nothing shows on hover and nothing happens when holding ALT 2024-03-04_23-02-19

Xruptor commented 6 months ago

Let me double check that, but the modifier should only affect stuff outside of the BagSync windows.

zeenk commented 6 months ago

For me it breaks the tooltip data displays in BagSync search window

zeenk commented 6 months ago

My settings if you want to try to replicate the issue:

2024-03-04_23-09-13 2024-03-04_23-08-50 2024-03-04_23-09-04

Xruptor commented 6 months ago

It may be a bug that was introduced with some of the changes I did. The tooltip should work on the BagSync search window without the modifier. Once the PTR server goes back up I'll check. I don't have an active subscription, so all my debugging is done on the PTR servers.

zeenk commented 6 months ago

That's by design. It's supposed to work that way. The modifier is only for stuff outside of the BagSync windows.

It works as intended on bag items, but it's always active on Blizzard currency tooltips

https://github.com/Xruptor/BagSync/assets/68170439/fa11f442-c26c-4e89-a365-550993920d27

zeenk commented 6 months ago

It may be a bug that was introduced with some of the changes I did. The tooltip should work on the BagSync search window without the modifier. Once the PTR server goes back up I'll check. I don't have an active subscription, so all my debugging is done on the PTR servers.

I see, I can revert to an old version and see if the issue is the same there

zeenk commented 6 months ago

Yeah you don't see any tooltip data in search window on old versions as well if you have mod key enabled :P

zeenk commented 6 months ago

So summary for what you should investigate when you have access to PTR again:

If mod key enabled:

Xruptor commented 6 months ago

It may be a bug that was introduced with some of the changes I did. The tooltip should work on the BagSync search window without the modifier. Once the PTR server goes back up I'll check. I don't have an active subscription, so all my debugging is done on the PTR servers.

I see, I can revert to an old version and see if the issue is the same there

That's so crazy. I remember it working on an older version I had when I implemented it. >.> Maybe I forgot to save the changes when I pushed it.

zeenk commented 6 months ago

Oh nvm it does work in bagsync search as well, I thought it worked the same as the bags that you can hold shift while hovering over the item to trigger the display but you have to hold the mod key BEFORE you hover for it to show in BagSync

https://github.com/Xruptor/BagSync/assets/68170439/a32b5e64-ebcd-41bb-9453-f005ab587133

zeenk commented 6 months ago

The blizzard currency tab is still an issue though, doesn't respect the mod key so it's always showing the characters

Xruptor commented 6 months ago

I found the issue, it looks like somehow the modifier check for the BagSync window was never pushed. I fixed that and also added the code for the Currency Window.

zeenk commented 6 months ago

Great, I can test and confirm for ya if you need any help with that :)

Xruptor commented 6 months ago

Here is the latest changes BagSync.zip

zeenk commented 6 months ago

For the blizzard currency tab tooltips you have to hold the mod key before hovering on the currency for it to show is that intended? on items in bags it can show the character when already having the tooltip open and then press the mod key

zeenk commented 6 months ago

The bagsync search tooltip info works atleast 👍

zeenk commented 6 months ago

For the blizzard currency tab tooltips you have to hold the mod key before hovering on the currency for it to show is that intended? on items in bags it can show the character when already having the tooltip open and then press the mod key

Like this, see how it updates as I press my mod key, but it doesn't update if I do it on the currency items

https://github.com/Xruptor/BagSync/assets/68170439/339653f7-9126-4eb5-94f2-75a1fca985ac

zeenk commented 6 months ago

Currency module in BagSync also broke with this latest test build, you have to hold the mod key for it to show now

https://github.com/Xruptor/BagSync/assets/68170439/c2447fc8-46af-4aab-b8f4-a30801548694

Xruptor commented 6 months ago

Yeah the alt key is required for the currency window because it's for "BagSync Tooltips" outside of the BagSync windows. That includes currency window. The reason it doesn't show up right away is because blizzard doesn't push a tooltip update on the Currency window like they do in the bags. So you have to hold alt BEFORE you hover over something.

There was a slight bug though with the BagSync Currency that I just fixed as well.

BagSync.zip

zeenk commented 6 months ago

The reason it doesn't show up right away is because blizzard doesn't push a tooltip update on the Currency window like they do in the bags. So you have to hold alt BEFORE you hover over something.

Oh okay, I see... yeah everything seems to be good in this build. Ready for pushing. Thanks for the quick fixes 👍

Xruptor commented 6 months ago

Alrighty, I'll go ahead and close this one with a push. If you catch anything else, file a new ticket. It's easier to track that way.

zeenk commented 6 months ago

Sure :)