`PET_JOURNAL_LIST_UPDATE` event spam when hovering over pets (e.g. in bag)
tflo opened this issue · 5 comments
BPBID is forcing the PET_JOURNAL_LIST_UPDATE event to fire by calling the C_PetJournal.ClearSearchFilter() function. It does this every time the user hovers over a pet in the bags.
IDK if the event by itself causes any non-negligible performance impact, but there may be other addons that work with or use this event.
It happens that one of my addons, PetWalker, is relying on this event ;)
PetWalker is relying on the PET_JOURNAL_LIST_UPDATE event in order to know whether it has to do certain things or not. But it doesnt expect an event spam, like caused by BPBID. Since v2.4.2 I have mitigated this problem by hooking one of your functions, and setting a ignoreevent_listupdate flag. This is literal from my current version:
if C_AddOnsIsAddOnLoaded('BattlePetBreedID') then
-- Hovering over any pet in the bags or the AH triggers the PET_JOURNAL_LIST_UPDATE event.
-- This is caused by the BattlePetBreedID addon: if the 'collected pets' option is active,
-- it calls `C_PetJournal.ClearSearchFilter()`, in order to make the collected pets info fetchable.
-- BreedTooltips.lua, line 116
-- We set the `ignoreevent_listupdate` flag via a hook to BPBID's `BPBID_SetBreedTooltip` at login/reload.
-- Works fine w/o timer (and with timer it gets out of sync if the spam rate is high).
hooksecurefunc('BPBID_SetBreedTooltip', function(parent)
-- Same conditions as used by the func to trigger `ClearSearchFilter`
if BPBID_Options.Breedtip.Collected and (not PetJournalPetCardPetInfo or not PetJournalPetCardPetInfo:IsVisible() or parent == FloatingBattlePetTooltip) then
ignoreevent_listupdate = true
ns.debugprint 'Hook: BPBID_SetBreedTooltip --> ignoreevent_listupdate'
end
end)
endThis works, of course. But, somehow, I find it would be “cleaner” if BPBID wouldn’t cause the event spam in first place. And, as said, it might also affect other addons that work with this event.
What do you think?
Yeah, I should find a better way to work with this.
Maybe I can only have that function called if the search filter is known to be set.
That seems to work to bypass the issue.
Yes indeed. Thank you. I removed the BPBID hook from my code.
As a side note:
I think your code possibly doesn’t quite do what you aim for (already before your last change): If the user has entered some search filter text, then everything OK, it clears that, so that the collected breeds can be displayed. However, when the user sets any of the other (checkbox) filters, like for example (worst case)…
…then you won’t get any collected breeds either (the image example has a similar effect like a text filter set to e.g. “hjhsf7hjksh”). So, in order to get rid of both filters (text and checkboxes), you would have to do something like this:
-- Set line for "Collected"
if (BPBID_Options.Breedtip.Collected) then
-- [comments]
if ((not PetJournalPetCardPetInfo) or (not PetJournalPetCardPetInfo:IsVisible()) or (parent == FloatingBattlePetTooltip)) then
local filterText = C_PetJournal.GetSearchFilter()
if filterText and not (filterText == "") then
C_PetJournal.ClearSearchFilter()
end
if not C_PetJournal.IsUsingDefaultFilters() then C_PetJournal.SetDefaultFilters() end
end
numPets, numOwned = C_PetJournal.GetNumPets()
-- [etc., etc.](BTW, the numPets seem to be an accidental global, and the numOwned isn’t used anywhere)
Side note on the side note:
Actually I’m rather happy that you don’t do it like that, because the users of my addon (PetWalker) can actively utilize the Pet Journal filters to establish a specific pool where a random pet is summoned from (for example, select only Mechanical pets).
BPBID would then mercilessly destroy that custom pool each time it gathers the “collected breeds” info 😧 (and de facto annihilate this feature of my addon for anyone who has BPBID's “show collected breeds” option enabled).1
But of course I’m aware that you have to do that if you want to display correct “collected breeds” info.
I would have proposed that you (or me via another hook) save the user’s filters and restore them after the info has been displayed, but unfortunately the API doesn’t have a function like GetFilters and SetFilters (like it has for the text search filter).
I yet have to figure out how this could be achieved… (or do you have an idea?)
Footnotes
-
It seems Blizz’s
\randompetmacro ignores the filters, so the good thing is that it only affects me and (some of) my users 😉 ↩
Yeah, I think I'm okay with it remaining like this. The existing code is already a compromise for people who don't use Rematch, which replaces the (imho) not very good Blizzard Pet Journal with a superior one, which does not need to use events to run filters.
If compromising even further for that relatively small subset of people would hurt your addon, then that's even more reason to not do it.
Yes, of course, anybody who does more than a dozen pet battles per year will probably use Rematch. Blizz’s Pet Journal is a sad joke. But this doesn’t mean that Rematch replaces the Pet Journal
Well, actually Rematch never replaces the PetJournal. Even if you tick the “Rematch” checkbox in the Pet Journal (or untick “Use Default Journal” in the Rematch Options tab), then all that happens is that a call to the Pet Journal opens the Rematch UI instead. But this doesn’t replace or overwrite any Pet Journal functionality, like for example… the filters. All this is still there and working, even if the frame is hidden.
So, when the user sets filters in the Pet Journal and then ticks the “Rematch” checkbox in the Pet Journal, the filters will remain active (and persistent across logins/reloads). Rematch’s filters do not touch the Pet Journal’s filters in any way, they are a completely separate system.
The thing with the Pet Journal filters is that they actually remove pets from the “pets list” in a way that affects even the most fundamental C_PetJournal functions, as you can test with something like this:
local idx = 1
while true do
local id = C_PetJournal.GetPetInfoByIndex(idx)
if not id then break end
idx = idx + 1
end
print(idx) -- total number of pets visible to the APIFiltered pets are basically no longer existent, hence they fire the PET_JOURNAL_LIST_UPDATE event. IMHO, this is a design flaw in the Blizz API1, and is what messes up your “collected breeds” data – but also what I’m using to my advantage in my addon 🙄
In case you’ve changed your opinion now: what I said in the last post (“the API doesn’t have a function like GetFilters and SetFilters”) isn’t true. I’ve found these function in the meantime, they are just missing on townlong-yak, and are not categorized under C_PetJournal in the WoW Wiki. But they are all there:
Type filters (“Families”):
C_PetJournal.SetAllPetTypesChecked
C_PetJournal.SetPetTypeFilter
C_PetJournal.IsPetTypeChecked
C_PetJournal.GetNumPetTypes
Source filters:
C_PetJournal.SetAllPetSourcesChecked
C_PetJournal.SetPetSourceChecked
C_PetJournal.IsPetSourceChecked
C_PetJournal.GetNumPetSources
Collected/Uncollected filters:
-- for example:
C_PetJournal.SetFilterChecked(LE_PET_JOURNAL_FILTER_COLLECTED, true)
C_PetJournal.SetFilterChecked(LE_PET_JOURNAL_FILTER_NOT_COLLECTED, true)C_PetJournal.IsFilterChecked(LE_PET_JOURNAL_FILTER_COLLECTED)
C_PetJournal.IsFilterChecked(LE_PET_JOURNAL_FILTER_NOT_COLLECTED)Footnotes
-
Rematch does it better by limiting the reach of its own filters mainly to the GUI. ↩