[One]Bug: hunter pets can't unlearn talents + patch

[Auntie Mangos]

Version: One, r1333 (3cf62cdc2831ef742d658dcc910cd6530ae8bda7)

Bug: when I go with a hunter to a pet trainer, I can select the "I wish to untrain my pet" option and confirm, but nothing happens.

Analysis: this seems to be caused by the [s0361] commit (b31603fc2166d5b61d6687dc9ba06b793c9c105a) that broke things for pets having spells already. The original condition for returning in WorldSession::HandlePetUnlearnOpcode, 'pet->m_spells.size() <= 1' was wrongly translated into 'pet->m_spells.size()'.

Edit: a bit late, but I did a search and found this bug report/patch: [fix][0.12]pet talent unlearn. I now also wonder why the original condition included the case where pets have exactly 1 spell?

Patch:

diff --git a/src/game/PetHandler.cpp b/src/game/PetHandler.cpp
index f895367..2d7c269 100644
--- a/src/game/PetHandler.cpp
+++ b/src/game/PetHandler.cpp
@@ -583,7 +583,7 @@ void WorldSession::HandlePetUnlearnOpcode(WorldPacket& recvPacket)
        return;
    }

-    if (pet->getPetType() != HUNTER_PET || pet->m_spells.size())
+    if (pet->getPetType() != HUNTER_PET || !pet->m_spells.size())
        return;

    CharmInfo* charmInfo = pet->GetCharmInfo();

[stfx]

Indeed, it seems to be a typo.

I also wonder why it was <= 1. Well it was changed in master in this commit which might give some insight: https://github.com/mangos/mangos/commit/d1469e366ba231d3b9d87cab53a4348f9ee8d83b#diff-3

EDIT: And before that it was added here: http://mangos.svn.sourceforge.net/viewvc/mangos?view=revision&revision=4096

[FunkyBit]

Confirmed. Is a bug in One and the linked master commit can fix the problem.

[evilatwow]

The code in master makes sense: it just avoids unlearning when the pet has no talents, basically meaning it is already/still in the unlearned state.

The code in One isn't using talents, so the question is if we can map that talent state to the (number of) spells. I tested a bit more, and even the old check (<= 1) is not working properly: I can keep unlearning although I've just unlearned. So it's not blocking properly even with that condition. Further debugging showed why: my pet, a cat, has 14 spells after unlearning! The reason is they are added after unlearning by Pet::LearnPetPassives, so spells like 17210, 19580, 19581 etc are always known. My conclusion is that even the original condition isn't properly (at least not in all cases), so whether we write <= 1 or just check for size 0 doesn't matter at all. Heck, we might just as well get rid of that condition as it is, since it's not working anyway...

Perhaps I should give merging that talent code a go instead

[stfx]

Added quick fix in [s1338]. Yeah we need to backport [7125] for it to work properly and I would be more than happy if you could do that. Thanks

[evilatwow]

On second thought, I think we don't event want to backport [7125]!

I was probably somewhat asleep when I considered giving that merge a go, but then I realized the talent trees were introduced in 3.x while TBC was using the talent point system and pet trainers! So there is no such things as pet talents in One.

Now instead of using the (number of) known spells, we could check the number of (free) talent points instead I think. I'll see if I can work out something.