Chain Spells Range (Thorim lightning et al) with Suggested Fix.

[HelloKitty]

Issue:

Chain spells are chaining at a range of n + 1.5y for most players where n is the chain distance they are suppose to have.

Affects:

• Resto shaman chain heal
• Cthun Eye Chains
• Thorim Chain Lightning
• Anything that chains basically

Resto shamans, your reign will be no longer =).

So, this is something that affects most servers. I believe what I'm about to describe it NOT expected behavior but I have no proof off-hand. Most servers just hack fix it at best.

Chain targets are currently using the API designed for object searching. They use the predicate WorldObjectSpellAreaTargetCheck. This is all well and good BUT chain targets should NOT be using this predicate imo. I think it was designed with the concept of AOE spells. This can be seen by analyzing WorldObjectSpellAreaTargetCheck::operator()

bool WorldObjectSpellAreaTargetCheck::operator()(WorldObject* target)
{
    if (!target->IsWithinDist3d(_position, _range) && !(target->ToGameObject() && target->ToGameObject()->IsInRange(_position->GetPositionX(), _position->GetPositionY(), _position->GetPositionZ(), _range)))
        return false;
    return WorldObjectSpellTargetCheck::operator ()(target);
}

It looks fine and reasonable until you actually dig into the implementation of the distance calculations.

bool WorldObject::IsWithinDist3d(const Position* pos, float dist) const
{
    return IsInDist(pos, dist + GetObjectSize());
}

This is how they compute whether an object is within distance. They use the dist additively with GetObjectSize(). What is GetObjectSize()? It's defined by a Units UNIT_COMBAT_REACH field which defaults to 1.5y and can be modified with things like Noggen Fogger. UNIT_COMBAT_REACH is currently defined by unit object scale * default reach. So, that's another issue. Only hack fixing it will still make noggenfogger or baby spice abusable.

Solution:

Create a separate predicate to be used in target check for the world object searcher that either uses a different calculation method for distance/range. Maybe more flags like the horrific LoS fix I did for TC? That was unfun to deal with though. So I'd recommend just creating another method like IsWithinCenterDist3d or something. Would be pretty easy to fix if C++ wasn't a 1970s language that supposed predicates easier. But I digress.

Yea, soooo that should work? Who knows.

[HelloKitty]

I'm bored so I may try to fix it and open a PR on TC for it.

[HelloKitty]

Well, this fix seems to work https://github.com/TrinityCore/TrinityCore/pull/19412 however it has yet to be scrutinized by TC and depending on how old Feenix' fork is you'll meet some changes that differ from the commits here. They may not be immediately mergeable. However, the concept is the same regardless of when you forked.

[Tapswhenoom]

TC was kinda different from our core but the pull request is in ^^

[HelloKitty]

@Fapswhenoom Interesting, let me know how it goes! There may be some side effects and I've asked for some guidance on that PR but haven't received much of a reply. All I can say is it will chain exactly the distance it is suppose to from the center but I'm unsure if this is a grounded center (2D) or a middle of the object. This may cause problems with large bosses who are being chain lighting'd, as noted in a WoW forums post I linked. I'm unsure how the expected behavior and tbh unsure of how it behaves against large bosses since the original code and my code both rely on a mixture of both 2D and 3D distance calculations.

[Tapswhenoom]

Yeah, I tried to apply some common sense when it came to mergin your PR into our code but it compiles and doesn't immediately causes a crash. My PSU died so I haven't been able to test it properly but that's what PTR is for ^^