GurliGebis / WoWAddon-BattlePetCompletionist

GNU General Public License v3.0
7 stars 2 forks source link

AH Tooltips Still Have Issues #60

Open GnuclearGnome opened 1 week ago

GnuclearGnome commented 1 week ago

The original issue that resulted the creation of this addon doesn't appear to have been completely resolved. The tooltips in the AH are much, MUCH better, but there still appears to an issue with the tooltip sizing at the bottom. Auction House Tooltip - Annotated 2

GurliGebis commented 1 week ago

Hey,

I have been looking into it - I can add extra padding to the bottom, but that would make some empty lines in the bottom of other pet battle item tooltips.

I don't think there is a good way to make it perfect for all tooltips. @davidmc24 sorry for disturbing, but do you happen to have any ideas? 😊

GnuclearGnome commented 1 week ago

Gotcha. It's certainly not critical. You can still read the bottom line fairly well (unless there's more below it hidden from view), it just makes your addon look a bit "unfinished." The important parts, the parts you set out to fix, are totally great. I appreciate your work here and have deleted that other Battle Pet addon and switched to yours instead. Keep up the good work! :D

davidmc24 commented 1 week ago

I'd noticed this before in my normal set of addons, but hadn't taken the time to report or look into it yet, with all the new expansion stuff going on. Will do so now.

So... I don't think this is a BattlePetCompletionist problem.

Step by step, I disabled all my likely related addons and added them back one by one.

No addons:

image

Just BattlePetCompletionist:

image

It even mostly works with Battle Pet UI Tweaks, Battle Pet BreedID, BattlePetCompletionist, and TradeSkillMaster (stats all showing as zero appears to be a TSM bug).

image

What doesn't work is when BattlePetCountNG is loaded, even if it's the only relevant addon.

image

Screenshots above were from the AH search listing, but the problem appears to be consistent across the AH screens.

So, this issue probably should be moved to https://github.com/GurliGebis/WoWAddon-BattlePetCountNG at some point, but I'll continue to investigate in the next comment...

davidmc24 commented 1 week ago

With BattlePetCountNG loaded (and everything else likely related disabled just to rule out interactions), next up is to narrow down the behavior. Going through the configuration options, the tooltip issue only appears to present when the "Items > Alter Caged Pet Tooltip" setting is checked.

Under other, it doesn't appear to be an issue if you check "Use extra attached tooltip" (though personally I hate that style). It only appears to impact "Unowned" pets, as far as I can tell.

So, now, to find the code responsible for that combination...

davidmc24 commented 1 week ago

Potential fix PR: https://github.com/GurliGebis/WoWAddon-BattlePetCountNG/pull/11

It looks to me like it fixes the behavior, but I don't understand why the offset is needed in the first place, or why it appears to vary.

GurliGebis commented 1 week ago

I have merged your PR (haven't tested it - I trust you 🙂) - 11.0.5-20241022-1 has now been released for CountNG.

tflo commented 1 week ago

Potential fix PR: GurliGebis/WoWAddon-BattlePetCountNG#11

It looks to me like it fixes the behavior, but I don't understand why the offset is needed in the first place, or why it appears to vary.

I was wondering the same when I changed the height calculation some months ago.

When I saw the new PR today, out of curiosity I tried the following minimal snippet and it produces no height problems at all, not in combination with other tooltip addons (tested with BattlePetBreedID, Auctionator, Syndicator and TSM), and not with AH tooltips:

(This is a mini standalone addon that adds randomly 0 to 3 lines to item tooltips and pet tooltips.)

local function create_lines()
    local texts = { [0] = nil, [1] = 'One line added', [2] = 'Two lines added\nTwo lines added', [3] = 'Three lines added\nThree lines added\nThree lines added' }
    return texts[random(0, 3)]
end

local function add_lines_item(tooltip)
    local lines = create_lines()
    if lines then tooltip:AddLine(lines) end
end

local function add_lines_pet()
    local lines = create_lines()
    if lines then BattlePetTooltipTemplate_AddTextLine(BattlePetTooltip, lines) end
end

TooltipDataProcessor.AddTooltipPostCall(Enum.TooltipDataType.Item, add_lines_item)
hooksecurefunc('BattlePetToolTip_Show', add_lines_pet)

The tooltip height adapts to every number of lines — without any additional tweaks.

Example screenshot from AH pet image

So I tend to think it is a question how the tooltip lines are added. Maybe there is still old tooltip code (from before the 10.0.2 changes) in BattlePetCount?


Note: This would normally belong to the BattlePetCountNG Issues, but I posted it here because of the given context.