Classic Castbars

Classic Castbars

18M Downloads

Bad detection of NPC's Pets as Player's Pets, and wrong pushbacks applied to a Pets

millanzarreta opened this issue ยท 4 comments

commented

What is the problem? Is there any Lua errors?
The code uses this to know if a player is NPC or Player:

local bit_band = _G.bit.band
...
local COMBATLOG_OBJECT_TYPE_PLAYER_OR_PET = _G.COMBATLOG_OBJECT_TYPE_PLAYER + _G.COMBATLOG_OBJECT_TYPE_PET
...
local isPlayer = bit_band(srcFlags, COMBATLOG_OBJECT_TYPE_PLAYER_OR_PET) > 0
...

But this is wrong. This does not mean: A player unit and player's pets units, this means: A player unit, player's pets units and NPC's pets units

This is because COMBATLOG_OBJECT_TYPE_PET is also present for NPC's pets. For COMBATLOG_OBJECT_TYPE_PET, is also needed to test if the COMBATLOG_OBJECT_CONTROL_PLAYER flag is present.

This can be fixed with something like this:

local bit_band = _G.bit.band
...
local isPlayer =(bit_band(srcFlags, COMBATLOG_OBJECT_TYPE_PLAYER) > 0) or ((bit_band(srcFlags, COMBATLOG_OBJECT_TYPE_PET) > 0) and (bit_band(srcFlags, COMBATLOG_OBJECT_CONTROL_PLAYER) > 0))
...

It seems more messy but it's a right way to check this.

BUT one more thing!! in this section of code:

...
function addon:COMBAT_LOG_EVENT_UNFILTERED()
    ...
    elseif eventType == "SWING_DAMAGE" or eventType == "ENVIRONMENTAL_DAMAGE" or eventType == "RANGE_DAMAGE" or eventType == "SPELL_DAMAGE" then
        if bit_band(dstFlags, COMBATLOG_OBJECT_TYPE_PLAYER_OR_PET) > 0 then -- is player
            return self:CastPushback(dstGUID)
        end
    end
end
...

In this section of code a pushback is applied to a Players and Pets. Pets are never affected by pushbacks, even player's pets (I personally test it in my imp). So the correct code here is:

...
function addon:COMBAT_LOG_EVENT_UNFILTERED()
    ...
    elseif eventType == "SWING_DAMAGE" or eventType == "ENVIRONMENTAL_DAMAGE" or eventType == "RANGE_DAMAGE" or eventType == "SPELL_DAMAGE" then
        if bit_band(dstFlags, COMBATLOG_OBJECT_TYPE_PLAYER) > 0 then -- is player
            return self:CastPushback(dstGUID)
        end
    end
end
...

What steps will reproduce the problem?
Hit pet of NPC and see how wrong pushback is applied.

Does the issue still occur when all addons except ClassicCastbars are disabled?
Yes

Was it working in a previous version? If yes, which was the last good one?
I don't know.

Any additional info? If you play on a non english client, please include your locale:
Spanish locale.

commented

Already aware of this, just haven't gotten the time to fix it yet. (Didn't know player pets don't have pushback though, thats interesting.)

Will be fixed in next release.

commented

This can be fixed with something like this:

local isPlayer =(bit_band(srcFlags, COMBATLOG_OBJECT_TYPE_PLAYER) > 0) or ((bit_band(srcFlags, COMBATLOG_OBJECT_TYPE_PET) > 0) and (bit_band(srcFlags, COMBATLOG_OBJECT_CONTROL_PLAYER) > 0))

Wouldn't just
local isPlayer = bit_band(srcFlags, COMBATLOG_OBJECT_CONTROL_PLAYER) > 0
suffice here or is there something im forgetting?

commented

Yes, should be the same, good point. But testing stuff I have discovered some smalls differences in these flags when Charm effect are present that, maybe, must be taken into account.

If a unit is a physical human player, and is charmed by NPC, its flags will be (I have tested these cases to be sure):

COMBATLOG_OBJECT_TYPE_PET and COMBATLOG_OBJECT_CONTROL_NPC

If a unit is a NPC, and is charmed by a physical human, its flags will be:

COMBATLOG_OBJECT_TYPE_PET and COMBATLOG_OBJECT_CONTROL_PLAYER

This happens with spells like Mind Control, Gnomish Mind Control Cap, Enslave Demon and similars.

You have to first think about how you want to consider the units in your addon when they are affected by a charm effect.

When Player charms a NPC, this charmed NPC is a Player's pet or should it still be considered an NPC?

When NPC charms a Player, this charmed Player is a NPC's pet or should it still be considered a Player?


As summary, if you only uses:

local isPlayer = bit_band(srcFlags, COMBATLOG_OBJECT_CONTROL_PLAYER) > 0

When a player is charmed by an NPC, the isPlayer returns false for this player unit. When a NPC is charmed by a player, the isPlayer returns true for this NPC unit. If you are ok with this behaviour all will be fine! If not, you should look for another more complicated solution (maybe check the UnitGUID string or something similar). However, the solution I proposed in the previous post seems to have this same "problem" (we will still need to check the UnitGUID string), in case this is a "problem", of course.

commented

Ah I see. I think its fine just using COMBATLOG_OBJECT_CONTROL_PLAYER for now then. The side effects aren't really an issue and will happen very rarely. (I just have to make sure it doesn't try to cache cast times for players.)