tflo / PetWalker

WoW Retail addon. Automatically restores your lost companion pet or/and periodically auto-summons a new one from a configurable pool. Can be set to work globally (same pet(s) for all alts), or per char.
GNU General Public License v3.0
3 stars 0 forks source link

Tries to summon faction specific pets as opposite faction. #7

Closed gizzmo closed 1 year ago

gizzmo commented 1 year ago

It will continue to try to summon it, resulting in a "Wrong Faction" error, and verbose spam with every attempt.

tflo commented 1 year ago

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.)

gizzmo commented 1 year ago

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].

tflo commented 1 year ago

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:

  1. 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.
  2. If positive, then check summonability of the previous pet.
  3. If previous pet is also unsummonable, then summon from pool (favs or all, depending on user settings).
  4. 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).

gizzmo commented 1 year ago

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.

tflo commented 1 year ago

Ok, here is a preliminary version. I only tested it briefly, let me know if something is glitchy:

PetWalker 2.0.9-dev1.zip

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:

  1. char previousPet,
  2. global currentPet,
  3. global previousPet

If char-specific favs are disabled:

  1. global previousPet,
  2. char currentPet,
  3. 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):

WoWScrnShot_101123_064538

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…)

gizzmo commented 1 year ago

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.

gizzmo commented 1 year ago

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.

tflo commented 1 year ago

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.

tflo commented 1 year ago

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 ;)

tflo commented 1 year ago

OK, here a new one (not yet released):

PetWalker-master.zip


Also:

gizzmo commented 1 year ago

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. 👍🏻

tflo commented 1 year ago

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.

tflo commented 1 year ago

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.

gizzmo commented 1 year ago

You're very welcome. No one likes it when addons don't work properly.