[TC/AC] [Feature] [Spell] Sacred Shield - rework proposal

[Shadowcoldzx]

DO NOT REMOVE OR SKIP THE ISSUE TEMPLATE

• I understand that my issue may get closed without notice if I intentionally remove or skip any mandatory* field
• This is NOT a bug/crash report
• I've searched the bugtracker for similar questions/requests including closed ones

Description

Background

• Sacred Shield is not being utilized by NPCBots at all (checking the code, the only real occasion they would use it is in a full group/raid setting with a proper tank spec in group).
• This proposal is to make SS actually used outside of those conditions, and make it used in the world, battlegrounds, arenas, etc.
• Furthermore, the proposal is to improve the logic behind the bot's decisions as to who or when to put sacred on (as currently, they would only choose a single target for it and call it a day).

Intended result

• The bots are able to sacred shield others players/bots in group even if they are not marked by lfg as tanks and running a prot spec (e.g. rogue, or queueing into a battleground where "tank spec" is not always the right target).
• The bot is able to switch sacred shield around as needed (instead of currently letting it run out before re-applying).
• The bot can use sacred shield reactively (such as in world or a bg)

Why do I think this should be done

• I only want to duo through the content with one bot (max. 2 for 3v3 if arenas are any good when I get there)
• I cba protting, instead I'm tanking as fury with autobalance at _un_reasonably difficult scaling (0.5+ HC difficulty, 0.6+ non-HC)
• With that in mind, I get cleaved around like an undergeared dog and sometimes I get swung for over half hp meaning my bot has a lot of work to do and it gets pretty rough sometimes, but it's fun don't judge
• Adding the SS to "usable spells" makes the work for my pally a lot smoother, no longer getting cleaved like a dog
• Also, this makes it work in general, in the world, in the bgs, in duo
• The currect logic for sacred (also beacon btw) are simply too restrictive without a full group/raid, this should make paladins more useful and alive

Currently implemented changes locally - note that only this single method was changed

bot_paladin_ai.cpp

void ShieldGroup(uint32 diff)
{
    if (checkShieldTimer > diff || !IsSpellReady(SACRED_SHIELD_1, diff) ||
        me->IsMounted() || Feasting() || IsCasting() || Rand() > 50)
        return;

    checkShieldTimer = 2500; // From 1500, do not nilly willy spam ss, this should smoothen it out a little better to avoid mindless SS being thrown around for no reason in hectic situations

    if (IsTank())
    {
        if (Rand() > 15)
            return;
    }
    else if (!HasRole(BOT_ROLE_HEAL) && Rand() > 10) // not sure how I feel about these rands here, could be really annoying if the bot keeps rolling bad consistently? Especially as we increase the check timer
        return;                                      // SS should be top priority for holy, less so for prot/ret, if it's holy then SS should be one of the first spells to do in majority of cases (in full group/raid the tank still keeps getting SS so it's fine)
                                                     // prot/ret should only really be very important in arenas, a bit less in bg when peeling, which we can't really script, the pally has to know who and when to keep ss on based on gamesense

    // Get rid of this, we need to switch shields as needed! 1 minute is too long to ignore if it sits unitilized
    //if (FindAffectedTarget(GetSpell(SACRED_SHIELD_1), me->GetGUID(), 70, 3))
        //return;

    Group const* gr = !IAmFree() ? master->GetGroup() : GetGroup();
    Unit* target = nullptr;
    if (!gr)
    {
        // TODO - This should be reworked similar to "in group", but here the bot should probably just look for valid targets and not focus on master at all (wanderer bots in the world should mostly just self ss or look for _any_ valid target)
        //  a bit undecided on this one, I think a spawned/owned npcbot should just be in your group, period; so from my view this only should concern wanderers out in the open; (bg bots seem to be in group anyways so this does not apply to them)
        //  I lean towards just making this self-ss, but it may screw up their gcds on cc's or dmg rotations when they do not require to be on the defensive..
        Unit* u = master;
        if (u->IsAlive() && u->IsInCombat() && IsTank(u) && me->GetDistance(u) < 30 &&
            !u->GetAuraEffect(SPELL_AURA_DUMMY, SPELLFAMILY_PALADIN, 0x0, 0x80000, 0x0))
            target = u;

        if (!target && !IAmFree())
        {
            BotMap const* map = master->GetBotMgr()->GetBotMap();
            for (BotMap::const_iterator itr = map->begin(); itr != map->end(); ++itr)
            {
                u = itr->second;
                if (u != me && IsTank())
                    continue;
                if (!u || !u->IsInWorld() || me->GetMap() != u->FindMap() || !u->IsAlive() || !u->IsInCombat() ||
                    u->ToCreature()->IsTempBot() || !IsTank(u) || me->GetDistance(u) > 30 ||
                    u->GetAuraEffect(SPELL_AURA_DUMMY, SPELLFAMILY_PALADIN, 0x0, 0x80000, 0x0))
                    continue;

                target = u;
                break;
            }
        }
    }
    else
    {
        std::set<Unit*> targets;
        uint8 attackerCount = 0;
        float lowestHealthPCT = 100.0f;
        Unit* lowestHealthUnit = nullptr;
        std::vector<Unit*> members = BotMgr::GetAllGroupMembers(gr);
        Unit* alreadyShieldedUnit = FindAffectedTarget(GetSpell(SACRED_SHIELD_1), me->GetGUID(), 70, 3);

        for (Unit* member : members)
        {
            if ((!(member->IsPlayer() || member->IsNPCBot())) || me->GetMap() != member->FindMap() || !member->IsAlive() || me->GetDistance(member) > 39.9f || (member->IsNPCBot() && member->ToCreature()->IsTempBot()))
                continue;

            // We select the priority target here, which will always take priority over any group member in terms of pre-shielding
            // We basically never ss anyone other than the tank, which could be detrimental but there are other cooldowns to blow if a non-tank aggros
            // If there is an encounter where a non-tank requires to tank, then it should be scripted individually (such as npc or something.. can't recall any such scenario in wotlk though where ss would be needed)
            // TODO - we should load tanks into an array instead so we can SS different tanks if we have multiple paladins in group/raid..?
            if ((me->GetMap()->IsDungeon() || me->GetMap()->IsRaid()) && IsTank(member))
            {
                // we found a dedicated tank
                target = member;
            }
        }
        // Not in dungeon/raid with dedicated tank role, prioritize whomever has attackers on them and/or based on hp
        if (!target)
        {
            for (Unit* member : members)
            {
                if ((!(member->IsPlayer() || member->IsNPCBot())) || me->GetMap() != member->FindMap() || !member->IsAlive() || me->GetDistance(member) > 39.9f || (member->IsNPCBot() && member->ToCreature()->IsTempBot()))
                    continue;

                if (!member->getAttackers().empty() /* prioritize units with the most attackers on them -- this is technically not a good tracking metric, but it should be generally accurate */)
                {
                    if (member->getAttackers().size() >= attackerCount) // This will iterate over all party members in combat, prioritizing OTHERS before the paladin! Unless we evaluate that the pally should shield himself later on
                    {
                        attackerCount = member->getAttackers().size();
                        target = member;
                    }
                }
                if (member->GetHealthPct() < lowestHealthPCT)
                {
                    lowestHealthPCT = member->GetHealthPct();
                    lowestHealthUnit = member;
                }
            }
        }

        // We have now got the target with most attackers, and the lowest health unit in group
        // Let's see if we already have SS applied or not, and if so, if it's needed to switch it
        if (alreadyShieldedUnit != nullptr)
        {
            if (alreadyShieldedUnit == target || alreadyShieldedUnit == lowestHealthUnit)
            {
                return; // All good here! The shielded unit is either our attacked target, or the lowest health member, no need to do anything
            }
            else
            {
                if (target && alreadyShieldedUnit->GetHealthPct() <= target->GetHealthPct())
                {
                    return;
                    /*
                    * Also good, the shielded member is not our primary target, but it has less health than the attacked one, so we keep it as is
                    * Once the member is healed, we can switch to our desired target
                    */
                }
            }
        }
        else
        {
            // Nothing is shielded, at this point, it was either dispelled, simply ran out, or the bot just appeared in world
            // The pally now needs to decide whether to shield the "target" based on attacker count, or to prioritize the lowest health
            if (!target && lowestHealthUnit)
                target = lowestHealthUnit; // no desired target, therefore consider the least hp member
            else if (lowestHealthUnit && target->GetHealthPct() > (lowestHealthPCT + 20.0f)) // if the lowest health is substantially lower, then we will prioritize it instead, otherwise, keep shield on our target
            {
                target = lowestHealthUnit; // switch based on HP% .. possibly glitchy in some corner cases, but it should work for the most part..
            }
        }

        // Not in dungeon/raid with dedicated tank role, nobody has attackers on them, nobody is lower than 100% hp, prioritize the player because the player is GENERALLY going to be the initiator, regardless of the map
        if (!target && !IAmFree())
        {
            target = master;
        }
        else if (!target)
        {
            target = me; // Default, we found nothing to ss, so just pre-shield self..
        }
    }

    if (target && !target->GetAura(SACRED_SHIELD_1)/* do not override if the target already has ANY ss */ && doCast(target, GetSpell(SACRED_SHIELD_1)))
        return;
}

Observations

• With the changes, the bot keeps pre-shielding me as the master (therefore, the most likely initiator)
• The bot should otherwise pre-shield the tank in a pve setting (dungeon/raid) and keep that as it's sole shield target (original behaviour)
• The bot keeps me pre-shielded throughout, anywhere in the world, so I can always pull with SS up
• The bot will SS others in group (or itself) based on "threat"(attackers count) or HP, if there is no dedicated tank in dungeon/raid setting
• It does not waste SS when it does not need to, the code above is of course imperfect and it may have flaws that can be smoothened out, but it generally keeps using SS in a decent manner

Comments

• I only include the snippet to showcase what I had in mind. With these changes my pocket healer feels a whole lot smarter and the SS does help a ton.
• Tested in both dungeons and a bg (multiple paladins), other wanderer-spawned pallies are pre-shielding themselves now before the game starts, they should be able to put SS on others in the bg as well now
• Ideally, it shouldn't use a single "target", rather it should put them into a sorted array/list and then try to apply to whichever does not currently have SS on (e.g. 3 paladins in raid, I expect 3 different shields on appropriate targets from the raid, not just group), but it felt a little over-complicated really)

There is another issue with beacon not being used at all - off topic here, but I made it just maintain beacon on itself regardless of there being a tank in the group (since I intend to only duo with a single bot while I tank with sacred, it's just a local hackfix to help itself survive better - tl;dr it will otherwise never use a beacon without a full group/raid setting).
Beacon basically follows very similar logic as SS does in the vanilla code, meaning that it simply is never used outside of full dungeon parties.
So if this change is considered, beacon could use a little love as well because they both fail on the same "IsTank()" condition.

Edit: Made the bot detect if it has beacon on it and if so it will prioritize healing others in LOS. Smooooth sailing - we both keep taking aoe damage, it reliably heals us both instead of randomizing who to heal and making gaps. Again though this is off topic, but it is sort of the same pally issue. With both sacred and beacon, the bot just shines.

What is this half-baked pasta

• I won't be on for a while so figured I'd rather post this than look back at this after a month and not remember a thing
• This is just a proposal with a "sample" code that makes pallies more useful, I'm happy with it as is so not planning on changing it further for my own use case, so posting this as an "idea" because after dinging 80 my pally bot never did any SS/Beacon whatsoever so it forced me to look under the hood
• I suspect the "proper" implementation should be a little cleaner and there is probably some room for optimization, but since I don't know what I'm doing I won't dare a PR until and if I get a better hang of it in the future

TrinityCore or AzerothCore

Both

Core rev. hash/commit

AzerothCore rev. 5ec19eaddc3e+ 2024-12-04 13:07:47 +0100 (npcbots_3.3.5 branch) (Win64, RelWithDebInfo, Static)

Operating system

N/A - applies to all