project-topaz / topaz

Server emulator for FFXI
Other
159 stars 224 forks source link

GetBuffSpell() is missing check for active buff #1484

Open MarianArlt opened 3 years ago

MarianArlt commented 3 years ago

I have:

Mobs are currently completely unaware of whether they have a buff active or not. This leads to a situation where a mob can have very few or none spells other than buffs and will repeatedly cast a buff despite of having it already active or not.

A good example for this is Ouryu who is erroneously missing spells and currently only has two -ga's and stoneskin. Apart from the fact that he therefore will cast stoneskin way too often (first roll is 35% ga-chance [default], then 35% buff-chance [default], then it's 100% buff cause no "damage spell" available [see here how that works]) he will spam stoneskin despite of not being attacked and having the buff effect active:

https://github.com/project-topaz/topaz/blob/019d51b3dba23239a0b3ef8953e378433552ca87/src/map/mob_spell_container.cpp#L200-L205

I suggest selecting the buff returned based on a while loop. I couldn't figure out if there's a way in the core to get the spell name instead of the ID (most probably there is) and then sort of concatenate that to HasStatusEffect(). Any ideas are welcome!

std::optional<SpellID> CMobSpellContainer::GetBuffSpell()
{
    if(m_buffList.empty()) return {};

    // don't return the rolled spell directly, store it
    auto chosenBuff = m_buffList[tpzrand::GetRandomNumber(m_buffList.size())];

    // re-roll as long as the effect is already found on mob
    // I'm not fond enough of what's available here, if it's possible to get the name of the rolled spell
    // it could be tested for with HasStatusEffect maybe?
    while (m_PMob->StatusEffectContainer->HasStatusEffect(EFFECT_chosenBuffNAME))
    {
        // as long as mob has status of chosen spell roll buff spell again
        chosenBuff = m_buffList[tpzrand::GetRandomNumber(m_buffList.size())];
    }

    return chosenBuff;
}

Additional Information (Steps to reproduce/Expected behavior) :

zach2good commented 3 years ago

There isn't a need to put a while loop in here, AIContainer is "small"-ticked every 400ms, so you can either look for something randomly once per "small"-tick, or do what I do when determining valid positions for trusts and just try 3-5 times, and bail out if you don't get a good result - the next attempt is coming soon anyway.

I added tpzrand::GetRandomElement recently, which makes the picking random things out of vectors and other stl containers a bit less gruesome looking. tpzrand::GetRandomElement(m_buffList); etc.

I wouldn't recommend comparing the name of the Spell to anything at runtime, but if you need more information about the spell, you can use spell::GetSpell(spellID) to return a CSpell* which has getName() and all the other accessors a spell needs.

There is currently no way to know from a CSpell what it does. We can infer a bunch of stuff:

I'd love to figure out some information to add to spells to annotate their effects. Example: If we could mark Spell - Bio as inflicting Effect - Bio, that would let us cut out... a LOT of code. (For trusts this would make my life 200% easier)