Tries to summon faction specific pets as opposite faction.
gizzmo opened this issue · 14 comments
It will continue to try to summon it, resulting in a "Wrong Faction" error, and verbose spam with every attempt.
Tried several times, using the "big green button", and couldn't get it to work at all. No messages. I even enabled debug mode.
This debug output is from a horde toon, after login:
PetWalker Debug: Event: ADDON_LOADED: PetWalker
PetWalker Debug: Event: PET_JOURNAL_LIST_UPDATE --> pool_initialized = false
PetWalker Debug: Event: PLAYER_ENTERING_WORLD: Login
PetWalker Debug: Event: PET_JOURNAL_LIST_UPDATE --> pool_initialized = false
PetWalker Debug: Event: PET_JOURNAL_LIST_UPDATE --> pool_initialized = false
PetWalker Debug: Event: PET_JOURNAL_LIST_UPDATE --> pool_initialized = false
PetWalker Debug: transitioncheck() is restoring saved pet
PetWalker: Summoned your last saved pet [Finduin].
PetWalker Debug: transitioncheck() complete
PetWalker: Auto: On | Pet pool: Global favs | Timer: Off | Current Pet: [Celestial Dragon] | Saved pet: [Finduin]
PetWalker Debug: Event: PLAYER_STARTED_MOVING --> autoaction
PetWalker Debug: autoaction decided for restore_pet # Current DB (global) pet: Finduin
PetWalker Debug: restore_pet() is restoring saved pet
PetWalker: Restored your last pet [Finduin].
PetWalker Debug: Event: PLAYER_STARTED_MOVING --> autoaction
PetWalker Debug: autoaction decided for restore_pet # Current DB (global) pet: Finduin
PetWalker Debug: restore_pet() is restoring saved pet
PetWalker: Restored your last pet [Finduin].
(p.s. those spaces on the "auto action decided" are there in the output i didnt put them there)
After some more testing, as you dont have Finduin, there is some weirdness with Finduin. On an horde toon, with getting the error "Wrong Faction":
Dump: value=C_PetJournal.GetPetSummonInfo(select(2, C_PetJournal.FindPetIDByName('Finduin')))
[1]=true,
[2]=0
Tried the same command with "Argent Squire", and got a proper return. After testing other alliance pets I have, "Lunar Lantern" has the same true value as Finduin.
Went back and tested again with a pet that has a correct return, and the addon works with proper messages.
So i just noticed that in the pet journal, on a horde toon. Trying to summon Finduin is possible, and thats where the error comes from, while trying to summon Argent Squire is not possible. The pet is red, and the summon button is grayed out. So seems blizzard is also suffering from the weirdness surrounding Finduin and others.
Interesting findings!
And I was already impressed that the check with GetPetSummonInfo
works so well and without complications ;)
But OK, if they manage to mess up the pet data, or code their pet metadata inconsistently, then that sounds more like Blizzard.
They have another function, PetIsSummonable
, which is supposed to return the same info (but without the reason text and number). I didn't take it because it's 3 expansions older than the other one, but I'll try it tomorrow (with the Lunar Lantern).
(p.s. those spaces on the "auto action decided" are there in the output i didnt put them there)
Thank you. There was a stray space after the color code in one of my debugprint functions.
So I was curious and tried it now. But nah, PetIsSummonable
returns the same nonsense. (I.e. true for both factions, tested with Lunar Lantern and Festival Lantern on both sides.)
But at least I have seen the "Wrong faction" error now too ;)
OK, here a new one (not yet released):
- Hardcoded the known bugged pets (Lunar Lantern, Festival Lantern; Finduin, Gillvanas) into the summonability check
- Let me know if/when you find more of them ;)
- Summonability is now also checked when generating the random pool. (The check after login remains, as it refers to the saved pets.)
- That means for example, if your pool is set to favs only and your only fav is one that is not summonable for that toon, you get the same warning as if you had zero favorites ("No eligible pets").
Also:
- Improved function and event handling after player entering world (login, reload, instance change)
- This should ensure that no event-triggered summon function (restore, timed random summon) can run before the main checks (including the new summonability check) have finished. (With short loading screens, this could lead to redundancy, or to precipitated wrong messages with non-summonable pets.)
- Caught an Ooops! bug, that could prevent PW-summoned pets from being saved as 'current pet'.
Let me know if/when you find more of them ;)
Using this list with my collection, the pets that need to be added are the Horde Balloon and Alliance Balloon.
P.S. Fantastic quick work getting this coded in. 👍🏻
Ah, good idea with the wowhead filter. Somehow I thought we had way more than 10 faction pets…
I will add the Balloons for the next release, thank you.
Never had this, but I will try to reproduce it in the next days.
In the meantime, can you give me a bit more info? (What is your setup? Favorites only? Is the faction-specific pet the only favorite? Is it a char-specific favorite? Did you get this with a specific pet (or with various faction-specific pets)? With wich pet(s)? What exactly is the "verbose spam"? Does it also happen when you summon manually via the keybind?, etc.)
All settings are default. Global favorites, auto summon The pet it was trying to summon i believe was Finduin. The spam that was happening was the normal verbosity of it trying to summon the pet constantly and failing.
I was able to reproduce it: Login into an alliance toon; Summon an alliance only pet; Login to horde toon to get the errors.
PetWalker: Summoned your last saved pet [Finduin].
PetWalker: Auto: On | Pet pool: Global favs | Timer: 12 min | Current Pet: [Gill'dan] | Saved pet: [Finduin]
PetWalker: Restored your last pet [Finduin].
PetWalker: Restored your last pet [Finduin].
PetWalker: Restored your last pet [Finduin].
Thanks for the additional infos.
What you are describing is actually one of the cases summarized in my "Known Issues / To Do" (at the end of the ReadMe / CF Description) as: Remove erroneous “summoned” messages in a few situations where actually no pet was summoned.
Technically, this is not a bug, just the message is, shall we say, inaccurate ;) The same thing would happen when you use Blizz's /summonpet
macro command, e.g. /summonpet Finduin
on your Horde toon: No error, but nothing is summoned.
This is because the underlying function does not return success or failure, it just summons – or fails silently.
However, I have not been able to reproduce the '"Wrong Faction" error' from your OP. Where does it appear? In the error frame, or in the Lua error output / BugSack? (It is definitely not generated by PetWalker).
I could not test it with Finduin/Gillvanas, because they are missing in my collection (damn!), but I tried with other faction-specific pets, e.g. Argent Gruntling / Argent Squire. No error message for me.
Back to the "known issue":
Besides the invalid faction case, it can also happen that the pet that PW saved as the last "current" pet is actually dead (very situational, at least). And there is a third rare thing that leads to a no-summoning, and I haven't been able to figure out what it is yet, and that's actually the reason why it's still a "known issue" and not solved ;)
But I think your report was enough to nudge me into finally addressing the issue in some way, at least for the invalid faction case ;)
My quick and dirty plan is roughly:
- Testing for non-summonability of the saved current pet at least after login (this would detect the invalid faction case). Maybe also every time before restoring a pet (and maybe even before summoning a new pet from the pool), but I think this is not really worth the extra calls.
- If positive, then check summonability of the previous pet.
- If previous pet is also unsummonable, then summon from pool (favs or all, depending on user settings).
- Decorate this with accurate messages ;)
For the invalid faction case, the optimal solution would be to first check against a list of Horde/Alliance speciesID pairs (most faction-specific pets come in pairs) and then summon the appropriate one if available; otherwise continue with step 2 from above. (But obviously I need to create the list first, so maybe a thing for the next major update).
Or do you have any other ideas?
I think I can upload a test version here until weekend or earlier. Stay tuned.
Until then:
The spam that was happening was the normal verbosity
Any "Restored" messages should only be printed if you have set PW to max. verbosity (3). The default is 2, but it is possible that it was 3 somewhere in the past.
/pw s
should print your current verbosity level. You can set it to 2 with /pw vv
, or to 1 with /pw v
.
To skip the unsuccessful restore attempts, just summon a new pet, no matter how (keybind or /pw n
, or manually from Rematch/PJ).
So the error "Wrong Faction" I'm referring to does not show up in chat, but in the UIErrorsFrame
in the upper part of the UI. This error, is not an issue with the addon itself, but the fact that it constantly tries to summon a pet and fails, and tries again.
I understand I can turn down the messages from PW, I kind of like the messages, though I may turn it down.
Your "quick and dirty plan" is exactly what I was thinking. if CanSummonPet() == 'NO' then SummonAnother() end
. The function GetPetSummonInfo
looks like it will return false with "InvalidFaction". Which should save you of keeping a list of horde/alliance list. As much as summoning a pet's counterpart would be neat, I think just another one from the pool is ok too.
Ok, here is a preliminary version. I only tested it briefly, let me know if something is glitchy:
If the zip doesn't work for some reason (permissions or whatever), you can also download it directly from the repo, green button (not from the Releases).
Basically, what it does now is this:
After login (and only after login) it checks if the saved 'Current Pet' is summonable. If not, it checks all other saved pets in this order and replaces the saved 'Current Pet' GUID with the first one it finds that is summonable:
If char-specific favs are enabled:
- char previousPet,
- global currentPet,
- global previousPet
If char-specific favs are disabled:
- global previousPet,
- char currentPet,
- char previousPet
If it doesn't find any that is summonable (not very likely), it passes a flag to the main check/initialize function that triggers a random summon of a new one.
I understand I can turn down the messages from PW, I kind of like the messages
Oh, nice to hear :) I like them too, and quite some work went into them…
The function GetPetSummonInfo looks like it will return false with "InvalidFaction".
In addition, it also returns the error number and error text. Both are printed now to the chat, in case of an non summonable saved pet (all verbosity levels):
BTW: If my new system bugs out and no message is printed, but you see a thing like in your quote from your post above…
Current Pet: [Gill'dan] | Saved pet: [Finduin]
…then it always means that something is not right. If saved pet and real pet are in sync, as they should be, you will only see this (with verbosity 3):
Pet: [Gill'dan]
So the error "Wrong Faction" I'm referring to does not show up in chat, but in the UIErrorsFrame
It is in fact weird that I don't get the error. (While testing the new version, I had almost all addons unloded, so it isn't the ErrorFilter addon…)
2.1.0
2.1.1 is on CF now.
Thanks for revealing the issue, discovering the bugged pets, and for your feedback and help! Really appreciated.