petType for non controllable pets is MAX_PET_TYPE. Wtf is that?

[BarbzYHOOL]

CURRENT BEHAVIOUR:

While inspecting Pet.cpp, I tested some variables inside bool Guardian::InitStatsForLevel(uint8 petlevel) with sLog->outError("----- PetType1 %u", petType);

https://github.com/azerothcore/azerothcore-wotlk/blob/a37ea1b60ee2e42a3953687bae3237d400e6fabf/src/server/game/Entities/Pet/Pet.cpp#L727

I discovered that many pets are not even considered pets. So the function IsPet() is very misleading as it's more like "IsControllablePet()" but yes, all the names are horrible as usual.

So these class pets (which means very important pets) are considered to be petType = 4:

• Dk's risen ghoul
• Dk's ebon gargoyle
• Shaman's fire elemental
• Mage's water elemental
• Warlock's infernal
• Warlock's doom guard (not tested but probably is like that too)
• Priest's shadowfiend
• Druid's treants
• Maybe others? (like druid treant

These are the different pet types:

enum PetType
{
    SUMMON_PET              = 0,
    HUNTER_PET              = 1,
    MAX_PET_TYPE            = 4
};

Where are 2 and 3? wtf? MAX_PET_TYPE is the name of the pet type for the pets I listed above! SUPER WTF??!

Shouldn't these pets be considered "SUMMON_PET" instead?? since they are "summoned pets"... All the small guardians also belong to pet type 4 and not to pet type 0. Maybe SUMMON_PET == WARLOCK_PET exclusively but only the controllable ones. If so, it again proves that naming things correctly is of the utmost importance.

             if (petType == HUNTER_PET)
                m_unitTypeMask |= UNIT_MASK_HUNTER_PET;
            else if (petType != SUMMON_PET)
                sLog->outError("Unknown type pet %u is summoned by player class %u", GetEntry(), owner->getClass());

As you can see above, anything different from 0 and 1 is supposed to return an error. But it never returns the error because these pets are not considered pets, so that part of the code is skipped.

                // The petType was not overwritten by the hook, continue with default initialization
                if (owner->getClass() == CLASS_WARLOCK ||
                    owner->getClass() == CLASS_SHAMAN ||          // Fire Elemental
                    owner->getClass() == CLASS_DEATH_KNIGHT ||    // Risen Ghoul
                    owner->getClass() == CLASS_MAGE)              // Water Elemental with glyph
                    petType = SUMMON_PET;

that part of the code seems totally useless and the comments are misleading, there needs a refactor imo (and take into account the hook that is used in mod-npc-beastmaster)

So all in all, this looks like something is wrong here, and maybe if it were correctly named and enumerated it would not bring any confusion and feel 100% clear, but currently I'm confused. I'd like to know what you guys think of that.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/92266824-pettype-for-non-controllable-pets-is-max_pet_type-wtf-is-that?utm_campaign=plugin&utm_content=tracker%2F40032087&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F40032087&utm_medium=issues&utm_source=github).

[Goatform]

Hey, it's been a while since I responded to any bug reports for wow emulators but I'd like to help out here a little bit with what I can recall, and hopefully, it can lead to more clarity.

Now, starting with some basic information that I can recall.

SUMMON_PET - from what I remember is used to assign to pets that are not hunter's pet but are permanent summon ... the value that handles this one (at least nowadays) is MiscValue from spell data. (Which is usually 0 and 1) HUNTER_PET - Well... self-explanatory one...

Any other from what I can recall is considered a temporary summon ... something like a guardian type...ish?

Logic should be reworked a little bit within 'summon pet' parts where the summoning logic is handled with MiscValue.

Also, I remember some conversation with another developer that I had long time ago (it was somewhere around MoP) where he mentioned that those specific flags are very likely manually handled and they would separate summon types...

E.g. : regular permanent pet, hunter's pet, regular temporary pet, guardian (temporary?) pet ...now how much is that accurate ... dunno. But I thought it was worth mentioning it.

I had some work done back in days around this one, however, I no longer have access to that source code plus I mostly worked with newer expansions and I am aware that they reworked pet system around that period where they reworked resilience and things like that so that permanent pets are all benefiting in same/similar ways with some basic general stats, so maybe my knowledge could be mixed up a little bit.

Btw, your concern is the right one... there should be more information about things like this one. If I get some time I'll attempt to see if I can dig some patch or any helpful information about it.

[Goatform]

Here are some possible ways to change it a little bit but first we need to see if this is a possible and valid way to handle it with 3.3.5 data.

So, I mentioned that summon type is handled with 0 and 1 in MiscValue within a spell that does the summoning. If it's 0, it is supposed to be temporary summon and 1 is for permanent.

1. Check if this is how things work in 3.3.5 for all possible summons (and it would be nice to include some other spells from npcs etc)

There could be some exceptions maybe like that glyph for water elemental or DK's ghoul case which if a spell is not overridden by another spell (Can't recall if that was a thing in 3.3.5 or later only) then MiscValue should be also different than the original spell? If not then they probably hardcoded those few small cases.

But if this is how it works then you could pretty much have:

enum PetType
{
    TEMPORARY_SUMMON = 0,
    PERMANENT_SUMMON = 1
}

Then for example, if you want to check if summon type is hunter's pet, you check if player getClass() == CLASS_HUNTER and if PetType == PERMANENT_SUMMON .... same goes for any other class/pet combination .... warlock ... DK ... mage.

[FrancescoBorzi]

@Goatform welcome back and thanks for sharing this

[BarbzYHOOL]

Ty @Goatform and yes that would be a good way to refactor it (also need to make sure the rest of the code complies, then also test mod-npc-beastmaster coz it's using the enum values)

I change the labels

[Kitzunu]

Arent these client values?

[BarbzYHOOL]

Arent these client values?

maybe (that would explain the weird IDs), that's why i asked here