PetWalker

PetWalker

4.3k Downloads

Character Specific Favorites

Syrusel opened this issue Β· 25 comments

commented

I used the command /pw c to have character favorite pets and then added a few to the list.
I used /pw 33 to summon a random pet every 33 minutes.
When I log onto a character it says i have 0 pets eligible for summon.
If I do a /pw s I get the following output:

Automatic Random-summon / Restore is enabled.
Summon Timer is 33 minutes. Next random pet in 0:00.
Pet Pool is set to Favorites Only. Eligible pets: 0
Per-character Favorites are enabled for MyName

MyName has 3 character-specific favorite pets.

I'm wondering why it says that I have 0 pets eligible when the pool is set to favorites and my character does indeed have favorite pets? This happens whenever I start walking as I've seen that that is the event that triggers it. Many thanks!

commented

I noticed now that if I do a /pw c to set it back to all of them and then another /pw c to set it back to favorites, then it works ok again.

commented

Thanks for your report.

This is weird, especially that it works after switching the favorites to global and back to char specific.

Can you reproduce the error somehow, e.g by going back and forth between global and char-specific favs again, or by disabling and re-enabling favs all together (/pw f)? If you can reproduce it, could you please check if there are any pet filters set in the Blizz Pet Journal (not in Rematch)?

In the status report, after the line "<char> has 3 character-specific favorite pets", were/are the three pets listed correctly, or was/is there an empty list, or only 1 or 2 pets listed?

commented

PS:

The reason I'm asking about pet filters is that they also affect your char or global favorites (my description may be misleading on this point, I'll fix it).

Now, in normal use, it would be against all logic to set up favorites and then exclude them via pet filters, but I've noticed that my Pet Journal sometimes has filters disabled, and I'm sure it wasn't me who disabled those filters. (I don't know if this is a Blizz bug or a glitch with some addon).

So, if, for example, your three favorite pets are Magic pets, and in your Pet Journal filters Magic pets are excluded, then you would have indeed zero eligible pets, if PW is set to summon favorite pets.

commented

Ok, tbh I don't know why it was acting wack on one of the characters. I did indeed have filters on for just collected pets. I can't reproduce it anymore but if it ever pops up again I'll try and get more data b4 opening this again. Right now it looks like it's working again.

commented

Filtering out (only) uncollected pets shouldn't harm, as they can't be in the pool anyway.

commented

Can you try this (not yet published) beta:

PetWalker-1.1.5-1-g21239a4.zip

This should at least fix the issue that after login/reload char favs are not correctly initialized, and probably also the issue with the wrong "eligble pets" message.

For the 2nd issue I say "probably" because I was unable to reliably reproduce the issue before (I could only reproduce it once, to be precise). Though, with the new version I tried a dozen times with different char favs and I didn't see it again.

Please note that the initial function after PLAYER_ENTERING_WORLD (e.g. after login/reload) runs with a delay of 10s. This means that, depending on your load time, during the first seconds after your char appears in the world, the addon is not yet fully "loaded".
This was already the case before this version, and the purpose is to avoid choking during login and to wait till the Pet Journal is fully updated.
So, if testing something, it is better to wait 5 seconds or so after login/reload/portal.


PS:

BTW, if you have s search string in the search box of the Pet Journal, this also counts as filter, since the string remains in the box even if the Pet Journal is closed.

I just searched for a char-specific favorite, then removed the pet from the favorites and was puzzled for a moment when I got the "zero eligible pets" message ;)

commented

Ok so I was afk and when I moved again it happened. Here's the message that I keep getting when I move:
image

And here's the message for /pw
image

Currently on character favorites with no filters

commented

And now after typing /pw for the above screenshot, it "fixed" it again.

commented

Kinda managed to reproduce it by doing /pw 1 to re-summon every minute but it's not consistent. My best guess right now would be that it maybe tries to re-summon the same pet that's already out?
image

So setup is following, /pw c for character favorites. Got 3 pets marked as favorite for this character and the other ones have 1 or 2 each. Did /pw 1 to test re-summon every minute. Only happens sometimes and after I'm afk a bit, not sure if correlated or just my above assumption regarding it trying to summon the same pet that's already out.

commented

PS: If I open the pet journal when this happens and then move again it fixes it.

commented

OK, I can now reproduce it too, sort of:

With exactly the same 3 char favs as you (though I don't think this matters) I got "only 1 eligible". After a reload it has set itself to global favs. Then toggling two times to char favs again "fixes" it.

So, I think there are two separate issues in play, not sure yet.

If I open the pet journal when this happens and then move again it fixes it.

Thanks for that info, can confirm this too.

I will investigate.


BTW, for testing and such, instead of /pw 1, you can also just use your hotkey to manually summon. The pool is the same.

commented

Hey, I downloaded the version you provided and also bound a key to summon a random favorite pet and so far I haven't been able to reproduce the bug with the 0 eligible pets, but I will keep trying.

To be fair, it usually happened by itself without me doing anything specific so it's hard to pinpoint exactly what triggered it. I'll just keep doing what I've been doing up until now and try to see if it pops up again and figure out how to reproduce it then.

Thank you, and I'll keep you updated!

commented

On a different character I have just 1 pet set to favorite, via the character-favorite section and it gets spammed with:
image

I'm aware this is my fault here since the pool is just 1, but I probably wouldn't send that message and just re-summon that 1 pet for the user :D

commented

Hey, yeah I can see and understand your logic and thought about it before writing it.

In my case it was intentional because I'm using the random summon on my other characters and correct me if I'm wrong but I noticed that doing /pw 10 for example will apply it to all characters and isn't character specific.

So in my case, on 2 of my characters I have a few specific pets that make sense to have out for them and for my other character I currently only have 1 pet which I like.

So I simply did a /pw 33 to have it summon from my list of favorites which for two characters is 2-4 pets and for the other it happens to be just 1.

This is just to avoid doing /pw 0 or /pw 33 whenever I switch between them, which I know isn't a big effort or whatever, it was simply more convenient. Probably an edge case, since I'll add another pet soon enough to that other character so yeah :D

commented

but I probably wouldn't send that message and just re-summon that 1 pet

Yes, I have been thinking about what to do in this case: Is having only one eligible pet a normal use case that makes sense, or rather an unintentional user error?

In the end, I decided that it was most likely a user error, and hence the message.

Why?

If you just wanted to have a specific pet out all the time, you would simply disable random summoning of new pets (/pw 0). The addon would then take care of this via the restore functionality and re-summon the pet if it is lost. On the other hand, if the user is using random summoning with only one eligible pet, then it is very likely that they have forgotten to make more pets eligible (too few favorites, pet filters unintentionally conflicting with favorites, etc.).

Well, that was my thinking when I opted for the message in this case.

So, my question: Did you do this on purpose (using random summoning with only one pet), or did you come across it by accident? If you did it on purpose, why not just turn off random summoning?

commented

I see. Loudly thinking about a few possibilities:

  • Limit the message to once per session or to once every 30 minutes or 1 h or so
    • Not sure if this is a good idea, as I still think in 90% of the cases it is a usage error, and the user would be thankful to get pointed to it instead of desperately waiting for a new pet or hammering the 'summon new pet' hotkey. (Before your post, the '90%' were rather a '98%' πŸ˜‰ )
  • Move the message from verbosity level 1 to level 2, as it is not an error in the strict sense.
    • Same considerations as above
  • To address the root of the "problem": Add a char-specific variant of the Random Summoning setting. E.g. /pw 0c or /pw 33c would only apply to the current char.
    • Personally (army-of-alts player) I have a strong allergy to any addon that makes me repeat settings on every single char. So, a design principle of this addon was to keep char-specific settings out (except where absolutely necessary, i.e. char-specific favs).
    • This would be optional, you will say. True, but it would probably also lead to confusion, as the user can no longer rely on "if I do a setting on one char it will be the global setting for all", which currently applies to everything except char favs. A break in consistency.
    • This adds complexity: New entry in the Status display; command logic for reverting the char-specific behavior; … . Not that I'm lazy (which I am πŸ˜‰), but you are saying yourself it's an edge case …

That being said, I am (slightly) considering option 3. I have a major code cleanup – and maybe refactoring – planned somewhere in the next six months anyway, maybe then...

Any other idea?

commented

I honestly think that the way you designed it is the best way i.e. have one setting apply to all characters, since that whole setting for every character thing can get out of hand real quick for both the user and you as a developer.

My only idea right now would probably be that you set a login message to the user maybe a tldr of the /pw command showing how many pets you have, what pool and timer. Maybe an extra line at the end saying having just 1 pet set to favorites acts as if /pw time was set to /pw 0 if that makes sense.

I prefer as you said just doing, in my case, a /pw 33 and having that apply to all characters, but at the same time I don't want to get spammed that I have just 1 pet since yeah, it's by choice in my case. I do however see how a user might accidentally switch between character and all favorites and end up with just 1 pet active and wondering why it won't work, but this should ideally be solved by them doing a /pw and seeing that info.

One idea maybe in regards to what I suggested above as a login message or when you type /pw to have just this part printed
or something similar so it doesn't fill up the entire chat, and maybe have a new command to display the current message (e.g. more or all of the info)

Tldr message:
image

The rest of it as you know and have it:
image

That would probably be it. In the meantime I reverted to the version present on curseforge and so far I can say that the one which you posted above solves that bug with 0 eligible pets, since I'm now getting it again :D

commented

login message to the user maybe a tldr of the /pw command showing how many pets you have

If I get you right, with "tldr of the /pw" you mean the /pw s display? The display you get with /pw is just the displays of /pw s (Status) and /pw h (Help) together in one command.

Basically the reason that I didn't add a login message is that I'm allergic to login message spam (yet another allergy πŸ˜‰), as it tends to obscure the really important login messages, like errors, or other essential info by addons or the game client.

But, ironically, not so long ago I have explicitly added a login message in my AQT fork. So I might as well add one here, but probably disabled by default.

But, despite being a good idea, for the actual problem here it wouldn't help much: The effect would be very similar to my idea to reduce the "only 1 pet" message to once per session: The user who accidentally only has one eligible pet would be very likely to miss the message, even more so than with a once-per-session message triggered by an auto-summon or hotkey summon.

get spammed that I have just 1 pet since yeah, it's by choice

Just had another idea: an option to disable this particular message. The message would than read:
"Your only eligible random pet […] is already active. Check your pet pool or disable auto-summoning or toggle this warning with '\pw onepetmsg'."

But I fear most users would then just toggle it away without really understanding it, and then dump the addon because it "doesn't work properly". So the result would probably be the same as moving it to verbosity level 2 or removing it altogether.

or when you type /pw to have just this part printed

The inclusion of the Help text in the basic /pw command is essential, IMO. From my own experience with other addons, I sometimes forget a particular command and then I enter a slash command with just the initials or the name of the addon. And if I do not get a Help text then with the list of available commands and have to open the lua files to find them, … I get angry 😀

When someone is new to the addon, it's normal to forget commands like /pw a, /pw p, /pw s. But /pw is hard to forget, and should help you with the other commands. That is, if I remove a part of the /pw display, then it would be the Status part.

In the meantime I reverted to the version present on curseforge and so far I can say that the one which you posted above solves that bug with 0 eligible pets, since I'm now getting it again :D

Thanks for that info. I'm going to release it today or tomorrow (just doing some more testing for a new adaptive length of the initial delay, depending on whether it's a login, reload or map instance transition).

The current fix for '0 eligible pets' is slightly suboptimal, but since it works for you (and for me), it will do for now until I make some major changes to that part of the code anyway.

commented

Yeah I totally get what you mean and it makes sense. What I meant by the /pw command was that it's perfectly good as is and helpful (for the reasons you outlined), what I mean by a login message was basically a very short version of the /pw s command so you see a quick status but yeah, I get that seeing that stuff all the time might be annoying.

But I fear most users would then just toggle it away without really understanding it, and then dump the addon because it "doesn't work properly". So the result would probably be the same as moving it to verbosity level 2 or removing it altogether.

For the 1 pet thing I honestly don't know at this point feels like a lose - lose. I mean in my head it makes sense why I have that 1 pet scenario and it's intended and I don't want to keep swapping the time command when switching characters, but like I said I can see people having issues with it by accident.

Maybe handle a /pw xx command only if the pool of pets is greater than 1? Otherwise just default it to a /pw 0. Perhaps this is where you could add that optional message notifying the user that they need at least 2 pets in order for it to randomly summon them at the specified interval, otherwise it will just keep current pet out or whatever your default state is :D

commented

Otherwise just default it to a /pw 0

This is actually an interesting idea.

So, in a good case scenario, it would go like this:

  1. The user gets a one-off message when the addon finds only 1 eligible pet and auto-disables random summoning.
  2. The user will probably miss that message and, after a while, either…
    • A. wonder why he always has the same pet, or…
    • B. nothing, if the "one pet" is intentional (as in your case)
    • In case B, it ends here. In case A, it continues with step 3.
  3. Maybe: He will check the status and see that the summon timer is disabled.
  4. He will re-enable the timer, thinking the addon is buggy.
  5. He will get the one-off message again and PW auto-disables the timer again. But now the user's attention level is (hopefully) elevated and he will actually see and read the message.
  6. He will fix the situation.

In a worst case scenario, steps 1 to 4 will do some more loops, unless he has uninstalled the addon in the meantime.

Sounds acceptable to me πŸ™‚

Optionally but ideally, PW would remember the previously set timer value and restore it as soon as it finds more than one eligible pets again.

commented

Optionally but ideally, PW would remember the previously set timer value and restore it as soon as it finds more than one eligible pets again.

Yeah, that's what I was thinking and would be nice to have! I would basically always save the timer set by the user and only activate it if valid pets > 1, otherwise just have it behave like /pw 0 behind the scenes. In this scenario doing a /pw s should show the timer set by you but also that extra message if your pet count is too low.

commented

The new version with the "zero pets" fix and the flexible delays in now on Curse and Wago.

The "auto-disable if only 1 pet" feature will take a bit more time, but – unless I run into serious objections – I will implement it in the near future.

In the meantime, I hope you can live with toggling the timer on your 1-pet char, or, if the char isn't too picky, try giving him/her a second pet πŸ˜‰

Thanks for your zero-pet bug report, and for your constructive and creative feedback for the 1-pet situation.

– Tom

commented

Hey Tom,

Thanks a lot for making / maintaining this addon, it's really cool and I hope more people start using it.

Thanks for all the hard work, quick responses and fixes!

  • Syrusel
commented

Having thought about it a little more, I've actually changed my mind.

I think you were right with your first proposal "[…] I probably wouldn't send that message and just re-summon that 1 pet for the user."

I also tend to think now that this is a real and "legit" use case. Aside from your case, I can also see a user who really loves a particular pet and makes that pet their "default pet". Now, with that pet as the only one in the pool, he could summon other pets from the pet journal from time to time, and the timer would automatically return him to his default pet after a while. Or he could just quickly hit the Summon hotkey to get it back.

Also, from a user perspective, I don't think there's much difference between having 1 or 2 pets in the pool, it's just one pet more or one pet less. The big difference only exists from a (technical) developer's point of view, because with one pet there is no such thing as random summoning, whereas with 2 pets in the pool there is. Wrong point of view ;)


To make it short, I made these changes to the message logic for 1 eligible pet:

Verbosity level >=2 (this is the level that displays new-random-pet messages; /pw vv):

No messages as long as the 1 eligible is out, regardless of the timer set.

If another pet was summoned (e.g. via Pet Journal) and then the 1 eligible comes back via timer or hotkey:
"Summoned your only eligible random pet [<name>]"

Verbosity level >=1 (this is the level that displays warnings only; /pw v):

If the 1 eligible pet is already summoned and the timer kicks in:
"Your only eligible random pet [<name>] is already active"

This message comes exactly once per session (as a reminder), then no more.

(If the user tries to summon via hotkey, the message will still come up every time, because then he obviously does not recognize his pet or has forgotten that he has only 1 eligible pet.)

Besides that, 1-pet pools no longer trigger the red warning message.


TL;DR, I think this should definitely remove the "message spam" for you.

If you want you can try this pre-release already:

PetWalker-1.1.6-5-g0b9b4c4.zip

commented

Having thought about it a little more, I've actually changed my mind.

Had me in the first half, ngl :D
Really nice changes, great work and thank you!