Add alert / verbose message when newly summoned pet becomes saved pet

[opussf]

There are verbose alerts for restoring a pet, and summoning the last saved pet.

Please add a verbose message that a pet is now saved, or a verbose message for why the newly summoned pet was not saved.

[tflo]

I think what you’re looking for is the debug mode. You can toggle it with /pw dm or /pw debug.

Amongst many others, you'll get also a couple of savepet-related debug messages that might be helpful:

`save_pet` debug messages [click to expand]

ns.debugprint(
	'Event: COMPANION_UPDATE (`actpet`: '
		.. ns.id_to_name(C_PetJournalGetSummonedPetGUID())
		.. ') --> `save_pet`'
)

[…]

ns.debugprint(
	'Event: COMPANION_UPDATE (`actpet`: '
		.. ns.id_to_name(C_PetJournalGetSummonedPetGUID())
		.. '): not saving bc `pet_restored`'
)

[…]

ns.debugprint '`save_pet` FAILURE, bc not `pet_verified`'

[…]

ns.debugprint '`save_pet` FAILURE: No `actpet` or `actpet` is excluded'

[…]

ns.debugprint_pet '`save_pet` completed'

---

Please note that when a pet is summoned because you put it into a team slot (for battle), it will not be saved. This is intentional, because the summoning was triggered by the Pet Journal mechanics, and not really by the user.

You should see debug messages like these in that case:

[710.067] PetWalker Debug: Hook: `SetPetLoadOutInfo` --> Setting `pet_verified` to false
[710.266] PetWalker Debug: `save_pet` FAILURE, bc not `pet_verified`

Pets will also not be saved immediately after a pet battle (15 seconds currently). You will not get any ‘save_pet’-related messages there, because PW is completely on standby for that time (events are unregistered).

The start of this after-petbattle-sleep should be marked by this debug message:

Event: PET_BATTLE_OVER --> Re-register events in %ss, unless we are in the next battle

---

Besides debug mode, you can also show a debug display, with /pw dd. This is basically an extended status display (/pw s), that also shows you the currently saved pets.

---

Hope this helps, otherwise let me know when exactly PW fails to save a newly summoned pet.

[opussf]

The Debug is nice for troubleshooting. I was thinking more something like this:

feature.patch
Based on tag 2.3.0

[tflo]

The Debug is nice for troubleshooting.

OK, so it looks like you don’t have any trouble with PW. That’s great 😌. (I automatically assumed that the reason for your request was that pets weren't being saved where they should be.)

Thanks for your diffs. When I saw your suggestion, I didn’t see any usefulness of Saved or Not-Saved messages, since normally this should just work, invisible to the user ¹, and if not, there are the debug prints.

However, the "Pet not saved because: Pet is in excluded list.” message can actually be useful, and it won't be spammy since there aren't many excluded pets. Since the Excluded list is not transparent to the user, it can help clear up situations where the Rotten Little Helper never gets restored, or where the user has marked all Winter Veil pets as favs and PW just says "Zero pets in the pet pool."

As for the other messages, I'm not sure they should be visible outside of debug mode: Not Verified will happen regularly when preparing for pet battle, and, as mentioned above, the fact that pet XY has been saved does not need to be visible, as the saved pet should be identical to the currently active pet 98% of the time.

But, I will definitely include the "Pet is in excluded list" message in the next version, as for the others, I have to try it out.

I'm a bit short on time right now, so don't expect anything before January. But since you already modded it in, there is no hurry for you, I think 😉.

Thanks for your suggestion!

Footnotes

1. The visible part is that the saved pet is summoned/restored. ↩

[opussf]

I did have problems with PW's saving of pets at one point. That seems to be fixed, Thanks.

As a wise developer once said: "The worse thing a program can do is fail quietly."

Quietly not saving a new pet falls into that "failing quietly".

For style wise, since there are alerts for things that are visible (restoring a pet), there really need to be alerts for non-visible actions, such as saving a pet, not-saving a pet (and why). If you use the favorites system, and the newly summoned pet is not a favorite, I still think that pet should be the newly saved pet. Not added to favorites, but saved, and used, or communicated why it is not being saved.

I understand the time issue.

Thanks for a useful addon.

[tflo]

If you use the favorites system, and the newly summoned pet is not a favorite, I still think that pet should be the newly saved pet. Not added to favorites, but saved, and used

If with "favorites system“ you mean 'Pet pool set to Favorites Only, Summon Timer set (or not set)’, then yes, a manually summoned pet will be saved and used (and restored when lost), no matter if it is a favorite.

It will be replaced by a favorite only when the timer is due or when you press your New Pet shortcut (or /pw n). You can unset the timer by setting it to 0 (/pw 0).

[tflo]

Since I just saw your “+1" reaction: The behavior I described is the current behavior, not a feature announcement ;)

[opussf]

I do genuinely like this addon. Thanks for putting it out there.

I think maybe one of the issues is that sometimes a pet is manually summoned during the period when PW thinks that it was an auto summon, and the pet is not saved.

Found how to reproduce this:

1. Change the pet via changing a team (drag it into the first slot, change the Rematch leveling queue sort order - Rematch changes the pet).
2. Do NOT move.
3. Manually summon a new pet (Pet Journal select and summon, or pet shortcut on the bars).
4. Pet is not saved (user is confused).
5. You can repeat 3 and 4 until you move.

If you move then the "not_verified" state ends (right?), and you can summon a new pet to be saved.

Hope that helps.

[tflo]

I can reproduce this. The behavior is the result of what I’m doing here to prevent any pet that is force-summoned by SetPetLoadOutInfo (team slotting) from being saved, which is priority #⁠1 in this situation. So, yes, this is a slight design flaw in a pretty niche case 🙄. (But still a flaw…)

But your main point is that a "your pet was not saved because..." message would be useful here, right? And I agree, this is a point, even if it is a niche case.

Of course, the message should only come up for a pet that was summoned by the user (not the ones summoned by SetPetLoadOutInfo while he's clicking through a dozen teams in Rematch). Or I find a reliable way to normalize the behavior without compromising the priority… (I have something in mind, but this needs testing.)

If you move then the "not_verified" state ends (right?)

Yes. PLAYER_STARTED_MOVING in conjunction with the not pet_verified flag triggers a function that checks the pet against the saved one and summons the correct one if necessary (and sets the flag to true again).

change the Rematch leveling queue sort order - Rematch changes the pet

Thanks for mentioning this. (I never tried this out and never will, as it would mess up my manually sorted queue of usually 200+ pets.¹) The summon happens only when active team slot #⁠1 is a leveling slot, correct? (Probably the same as when you change position 1 of the queue.)

Footnotes

1. I sell L25 pets on the AH ;) ↩

[opussf]

Yeah, I agree. It might be a flaw, one that can be minimized with a message. If the user knows that PW is not saving the pet at that time, they are less likely to be wondering why it is not working. In this case, a message can inform the user, who can then take actions to help the addon work. :D

Correct, the message about not saving is helpful. That was / is the goal of this feature request¹.

Footnotes

1. Though it has turned into a good conversation. ↩

[tflo]

I applied your patch after the last conversation and here is what I experienced/think (after 4 days of not too much play time):

• Displaying not-saved messages after login is not useful, the addon is working correctly at that point. Displaying the message just confuses the user.
• The same goes for not-saved messages for pets summoned by SetPetLoadOutInfo. As mentioned, it is intentional that these pets are not saved. Displaying the message just confuses the user.
  • I still agree with you that a msg should be displayed in the case when the user intentionally summons a pet in that phase (before moving). But I still have to find a way to isolate that w/o compromising functionality. Feel free to post a PR, or a patch file.
• On other occasions, I didn’t notice message spam with your messages.

[opussf]

The intention of the feature request is to add messages, at higher verbosity levels, that inform the user as to the actions of PetWalker. In my experience, a well worded message that describes what is going on is always better than an uninformed and confused user.

The original request is to add messages, at the higher verbosity levels, for when a pet is saved or not saved (and why).

Having a not-saved message after login, on high verbosity, IMHO is perfectly fine. The user who is choosing verbosity level 3 will understand. This should probably be reported at verbosity level 2 instead.

Yes, it is intentional for PetWalker to not save pets summoned by SetPetLoadOutInfo. At the highest verbosity level, the user might want to see those messages, and they can lower the verbosity if they do not.

Included patch file with updated, and hopefully less confusing message.

feature.patch