Spell: Fix Arcing Sear (and other TARGET_UNIT_ENEMY_NEAR_CASTER spells)

[insunaa]

🍰 Pullrequest

Spells of this type casted by NPCs don't have a UnitTarget so the function that would limit the number of chain jumps was skipped completely.

Proof

• None

Issues

• None

How2Test

• .cast 30235 into a group of enemies or
• .cast 30234 on yourself

Todo / Checklist

• None

[Velite12]

Just to add to why this is being done:

Spell 30235 (Arcing Sear) is supposed to be limited to 3 targets, this is the spell casted by the 30234 passive that Astral Flares use on The Curator fight. This is confirmed by the client dbc, the server db, guides, videos, and logs. Currently the spell cast has no target limit when used in game, so it will just hit the entire raid making the fight more difficult than necessary.

The resulting bug comes from the implicit target, since Chain Lightning shaman spells use 6 but this spell uses 2. Even though Chain Lightning uses the same scheme as Arcing Sear there is a nullptr being passed for the UnitTarget.

[killerwife]

Okay so, first of all, love spells that are one of a kind in all of tbc. Second of all,

You need for this spell to do this first:

filterUnitList.sort(TargetDistanceOrderNear(m_caster));

Because otherwise, .front() could have furthest target and possibly have no eligible jumps. (and search is from caster)

Furthermore, and this is mainly an oversight on my end:

                    SpellNotifyPushType pushType = PUSH_TARGET_CENTER;
                    if (m_spellInfo->HasAttribute(SPELL_ATTR_EX2_CHAIN_FROM_CASTER))
                        pushType = PUSH_SELF_CENTER;
                    if (m_spellInfo->HasAttribute(SPELL_ATTR_EX5_MELEE_CHAIN_TARGETING))
                        pushType = PUSH_CONE;

The chain from caster one likely needs to behave the same way. Cone should be fine already.

[insunaa]

My changes were as always minimally invasive to preserve the original intent. In your original code you break; when UnitTarget != filterUnitList.front() which to me means that this list is either already sorted or there are other reasons why front() and UnitTarget are supposed to be equivalent.

I thought about sorting first but decided against it for that reason.

For the second note I am lacking any context

[Velite12]

would it make more sense to do this then?

        case SCHEME_CLOSEST_CHAIN:
        {
            if (filterUnitList.empty())
                break;
            filterUnitList.sort(TargetDistanceOrderNear(m_caster));
            Unit* unitTarget = filterUnitList.front();
            UnitList newList;
            newList.push_back(unitTarget);
            filterUnitList.pop_front();
            Unit* prev = unitTarget;
            UnitList::iterator next = filterUnitList.begin();
            chainTargetCount -= 1; // unit target is one

Since no matter what, filterUnitList can't be empty, we sort it and then pick the closest target. This would guarantee unitTarget is the closest and also not a nullptr. So we sort it first, get it, then pop_front instead of sorting after.

The Unit* unitTarget = m_targets.getUnitTarget(); has to be the front most unit anyway, so we might as well initialize it to be that.

Just to confirm, this still solves the issue:

Raw cast of 30235 at nobody and then passive 30234 which casts automatically.

And targeting a unit manually and casting 30235 still only hit 3: