blueboy / portal

This portal repo is for development purposes only
http://github.com/blueboy/portal
GNU General Public License v2.0
27 stars 24 forks source link

Druid, mage, warlock lack cast time for spells #8

Closed kennumen closed 12 years ago

kennumen commented 12 years ago

They all have instant casting. This can result in e.g. 10+ instant shadowbolts on a single target.

Easy test: login a mage (with both conjuring spells... is that level 4+ or 6+?). Instantly conjures both food and water.

I've checked and re-checked the PlayerbotAI::UpdateAI() function but it starts by setting a 2second 'timeout' then proceeds to call DoNextCombatManeuver and returns (at which point it should wait 2 seconds until the next spellcast).

PlayerbotAI::UpdateAI() calls PlayerbotAI::DoNextCombatManeuver calls PlayerbotClassAI::DoNextCombatManeuver calls PlayerbotClassAI::CastSpell() calls PlayerbotClassAI::CastSpellNoRanged or PlayerbotClassAICastSpellWand()

I know I broke it but I can't seem to fix this. It can't be that m_ai->CastSpell() suddenly acts differently or needs a stopcast call or some such.

blueboy commented 12 years ago

Hi,

Sorry for the delay in fixing this, I have been setting up a backup server on my network. The fault for this error is mine. About nine months ago, I fixed an issue where occasionally bots would be stuck pointing their ranged weapons at imaginary targets. The solution I found was to change the triggered spell flag with CastSpell() in PlayerbotAI.cpp from false to true. This worked well for triggered spells like AUTO_SHOT, but would cause the instant casting issue with non- triggered spells such as conjuring food and water (no time to display spell effect visual). I have now reverted this fix so the flag is again false.

I then had to rework the AUTO_SHOT code in PlayerbotHunterAI.cpp to account for the triggered effect. I also found that m_bot->FindCurrentSpellBySpellId(AUTO_SHOT) was always false, so AUTO_SHOT was repeatedly been cast without interrupt. Obviously the vaiable m_CurrentlyCastingSpellId would hold the correct value, so I created a new function GetCurrentSpellId() to retrieve this from within PlayerbotHunterAI.cpp. By using CastSpell() defined in Unit.cpp we can simplfy the code to handle AUTO_SHOT directly. The spell is correctly interrupted in UpdateAI() with the appropriate AI delay m_ai->SetIgnoreUpdateTime(2).

I have also removed some unecessary code that was causing some other problems,

o return SetIgnoreUpdateTime(0); // Was set at the start of UpdateAI, make sure we don't unnecessarily wait

was actually causing further delay.

// bot was in combat recently - loot now
if (m_botState == BOTSTATE_COMBAT)
{
    SetState(BOTSTATE_LOOTING);
    m_attackerInfo.clear();
    if (HasCollectFlag(COLLECT_FLAG_COMBAT))
        m_lootTargets.unique();
    else
        m_lootTargets.clear();

    return SetIgnoreUpdateTime(0); // Was set at the start of UpdateAI, make sure we don't unnecessarily wait
}

if (m_botState == BOTSTATE_LOOTING)
    return DoLoot();

In the program flow, Doloot() is actually executed immediately, by returning a AI delay of (0) adds a cycle to the update.

o The code below was causing the bots to be summoned as they start to loot. It is necessary to use MovementReset() so the bots return to the master after looting, but it's not necessary to teleport the bots back, so I removed it.

    if (m_bot->GetPositionZ() > (pTarget->GetPositionZ() + INTERACTION_DISTANCE) || (m_bot->GetPositionZ() + INTERACTION_DISTANCE) < pTarget->GetPositionZ())
    {
       DoTeleport(*m_followTarget);
        return;
    }

The result of these changes have speeded up the looting process and stopped that annoy slow motion effect, as the bot approach the loot.

Let me know if this resolves your issue and if it does can you close it.

Cheers

kennumen commented 12 years ago

Eager to test, unfortunately I won't have time to test tonight.

One question after a quick gander of your (great) modifications - didn't see any code for stopping the 'special' hunter autoshot. Is this code already present or... a future addition ;) Right now I can't think of any other reason to stop it other than "follow" which probably has a broad spectrum "stop casting" command, so it might not be necessary.

kennumen commented 12 years ago

Confirm fixed. Awesome.

Didn't check whether hunter autoshot stop is bugged. If someone cares enough they can open a ticket ^^

blueboy commented 12 years ago

Additional code to stop the hunter spell autoshot isn't necessary. True the original code in PlayerbotHunterAI.cpp did, but I found this to be redundant. I believe that autoshot is interrupted by the switch from ranged to melee combat. I was about to suggest that you try it for yourself, but I see you already have and closed the issue.

I am still testing the code and have already found another issue where mages (fireballs) & warlocks (shadowbolt) are left with incomplete casting, after combat (similiar to the ranged weapon issue). I have occasionally seen warlock imps with the same. The spell failure shows this to be SPELL_FAILED_UNIT_NOT_INFRONT. The fix I will push shortly will in error handling get the bots the turn to face the target if it still exists and cancel the currently running spell.

Maybe we can discuss this further on the playerbot forum, at it's new site http://playerbot.mine.nu

kennumen commented 12 years ago

I didn't mean the autoshot-melee switch (although that probably means it has no problem stopping autoshoot). I meant if the hunter is asked to do something non-combat-y (like follow), will it stop autoshot as well?

I closed the issue because that doesn't really have much to do with instant casting (although losely it does).

Do bots not check if the target is in front just before 'releasing' the spell? As a human, if a mob runs to somewhere behind you, you tend to turn around even while casting...

blueboy commented 12 years ago

I shouldn't think that a non-combat command like 'follow' alone, would be heeded by bots while in combat. Certainly, for portalR2, I found it necessary to create a more elaborate comand 'recall' to interrupt such action.

I expect that this issue is caused if the target is killed by another bot as the mage or warlock continue to cast. Similar the 'bad target'. The error is flagged and the bots are left with flaming hands after combat.