Trinitycore: Range attacking mobs not attacking from distance

[ZenoX92]

Description:

(This started to happen at the same time that all those issues with the waypoint system.)
When you aggro some range attacking mobs (casters, archers, etc), they should remain still and start attacking from their position, but instead they move toward you.

This happens for all creatures using SMART_EVENT_IN_COMBAT (0) with event_param1=0 and event_param2=0 and SMART_ACTION_CAST_SPELL (11) and action_param2=64 (Cast Flag: Disable moving while casting); which means they should start casting their spell immediately after entering in combat and not move, but it's as if they skip the first spell cast.

For instance:
('15213','0','0','0','0','0','100','0','0','0','3400','4800','0','11','9672','64','0','0','0','0','2','0','0','0','0','0','0','0','Twilight Overlord - In Combat - Cast Frostbolt');

(15213,0,0,0,0,0,100,0,0,0,3400,4800,0,11,9672,64,0,0,0,0,2,0,0,0,0,0,0,0,'Twilight Overlord - In Combat - Cast Frostbolt');

Twilight Overlords shoud start casting Frostbolt immediately when aggroed and remain still, but they start moving toward the attacker without casting the spell. After 3400-4800 ms, they stop moving and start casting Frostbolt.

Steps to reproduce the problem:

Make sure you're rolling a range attacking hero.

1. .tele staghelmpoint
2. Engage some Twilight Overlord or Twilight Masters

Or find any other range attacking mob. There's plenty of them all around the world.

Branch(es):

3.3.5

TC rev. hash/commit:

TrinityCore rev. 1cc3d33cfe7e 2018-02-19 20:21:57 +0100 (3.3.5 branch) (Win64, Release, Static) (worldserver-daemon)

TDB version: 335.64

Operating system: Windows 7

[Killyana]

This Twilight Overlord and Twilight Masters are using an other spell cast with a castflag=1 so the first bolt is interrupted and the nova is cast instead.
To reproduce the issue better to test npc=430 it casts only the spell 20802 with cast_flag=64, the initial min and max=0 this means the npc must start casting on aggro without moving at all, but actually it's not the case, even if we set event_param1=100, event_param2=100 (initial min and max=100 ms) it changes nothing, the npc will always chase the player for a while before start casting.

Forget about the initial min max, it's not related at all

``` sql
SELECT * FROM smart_scripts WHERE entryorguid IN (430 );
-- UPDATE smart_scripts SET event_param1=100, event_param2=100 WHERE entryorguid IN (430) AND source_type AND id=1;

[TerrorBringer]

I tried with commit 425b181 and this bug is already present in it.

Edit: I tested a bunch of old commits and that's the oldest I tested.

[TerrorBringer]

Tested with commit e2a97ba7e789748aaa46ace153382573fd7d7df5 and this bug is already present.

[TerrorBringer]

Hello, I did some experimenting with the script of the Defias Smuggler and Defias Pillager and if you change the event_type from 0( SMART_EVENT_UPDATE_IC) to 9 (SMART_EVENT_RANGE) and change the event_param2 to 40 (which is max distance) the mob will behave correctly, meaning the mob will stop combat movement and cast the spell without moving.

here is the sql for the 2 mobs for anyone who wants to try it out.

DELETE FROM `smart_scripts` WHERE `entryorguid`=589 AND `source_type`=0;
INSERT INTO `smart_scripts` (`entryorguid`, `source_type`, `id`, `link`, `event_type`, `event_phase_mask`, `event_chance`, `event_flags`, `event_param1`, `event_param2`, `event_param3`, `event_param4`, `action_type`, `action_param1`, `action_param2`, `action_param3`, `action_param4`, `action_param5`, `action_param6`, `target_type`, `target_param1`, `target_param2`, `target_param3`, `target_x`, `target_y`, `target_z`, `target_o`, `comment`) VALUES 
(589, 0, 0, 0, 1, 0, 100, 0, 1000, 1000, 1800000, 1800000, 11, 12544, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, "Defias Pillager - Out of Combat - Cast 'Frost Armor'"),
(589, 0, 1, 0, 4, 0, 15, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, "Defias Pillager - On Aggro - Say Line 0"),
(589, 0, 2, 0, 9, 0, 100, 0, 0, 40, 3400, 5400, 11, 20793, 64, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, "Defias Pillager - In Combat CMC - Cast 'Fireball'"),
(589, 0, 3, 0, 2, 0, 100, 1, 0, 15, 0, 0, 25, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, "Defias Pillager - Between 0-15% Health - Flee For Assist (No Repeat)");

DELETE FROM `smart_scripts` WHERE `entryorguid`=95 AND `source_type`=0;
INSERT INTO `smart_scripts` (`entryorguid`, `source_type`, `id`, `link`, `event_type`, `event_phase_mask`, `event_chance`, `event_flags`, `event_param1`, `event_param2`, `event_param3`, `event_param4`, `action_type`, `action_param1`, `action_param2`, `action_param3`, `action_param4`, `action_param5`, `action_param6`, `target_type`, `target_param1`, `target_param2`, `target_param3`, `target_x`, `target_y`, `target_z`, `target_o`, `comment`) VALUES 
(95, 0, 0, 0, 4, 0, 15, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, "Defias Smuggler - On Aggro - Say Line 0"),
(95, 0, 1, 0, 9, 0, 100, 0, 0, 40, 3500, 4100, 11, 10277, 64, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, "Defias Smuggler - In Combat CMC - Cast 'Throw'"),
(95, 0, 2, 0, 67, 0, 100, 0, 1300, 7300, 4800, 4900, 11, 53, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, "Defias Smuggler - Behind Target - Cast 'Backstab'"),
(95, 0, 3, 0, 2, 0, 100, 1, 0, 15, 0, 0, 25, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, "Defias Smuggler - Between 0-15% Health - Flee For Assist (No Repeat)");

I hope this helps to identify the error, this annoys me very much.

Best regards and have a nice weekend.

[ghost]

I'm not saying that this is either wrong or 100% correct.
I mean to say that it needs testing & verification.

[TerrorBringer]

The mobs still behave incorrectly from time to time, sometimes mob x runs away to mob y, and mob y (which can attack from range) doesn't attack from range distance, instead they still run toward the player.

Another problem is when you taunt a mob (that can attack from range) (ex: Hand of Reckoning from Paladin), they still run toward the player even if at a 40 yards range (happens sometimes, not always).

So no, this is not an ideal fix, the problem must be related to some AI C++ modification.

[ghost]

Interesting idea. Would like to see if anyone has got more clues to what could cause this issue and possibly how to approach a working solution here.

[Killyana]

The behavior of this issue changed:
Now the npc follow the player for some seconds before start casting (in case it's the first aggro after a respawn)
And after an evade it stays without moving for some seconds before casting.

[Langerz82]

Looking into it.

[Langerz82]

Couple things...
Here is a patch you need to apply to fix the timings:
https://gist.github.com/Langerz82/14e10a876866c3daf26b3bbf98aba83d

Expect a delay of up to 500ms and they will be moving towards the target before the ranged attacker casts. This can be shortened further but I think that value in between checks is reasonable.

For Twilight Overlord and other Mobs:
You need to set the min and max ranges with event_param1 and event_param2 respectively. Also Frost Nova for the Twilight overlord needs event_type 0 (SMART_EVENT_UPDATE_IC) not 9 (SMART_EVENT_RANGE)

Example:

UPDATE smart_scripts SET event_param2 = 40 WHERE entryorguid=15213 AND id=0;
UPDATE smart_scripts SET event_type = 0 WHERE entryorguid=15213 AND id=1;

Edit:
I also think the conditional check interval should be configurable and be able to be set in the DB, I see event_param5 is 0 so maybe we can use this?
@Shauren to confirm.

[Killyana]

This is the old scipt from master https://github.com/TrinityCore/TrinityCore/blob/master/src/server/game/AI/SmartScripts/SmartScript.cpp#L2612
I don't understand why it was changed.

[illfated]

in case you want to dig into the reasons behind that change, it was changed in PR https://github.com/TrinityCore/TrinityCore/pull/18673 / commit https://github.com/TrinityCore/TrinityCore/commit/b0ae5fadd19fd172ec5154cde4f4fd14aa20ff88 .
For a more detailed breakdown, you want to check the blame view:
https://github.com/TrinityCore/TrinityCore/blame/3.3.5/src/server/game/AI/SmartScripts/SmartScript.cpp#L2278-L2287

[Langerz82]

Using min/max in the delay between conditions should be configurable or else it will take that amount of time in-between checks, not just the delay between use.
Shauren needs to confirm the go ahead to use a new param to set the interval check. Min/Max is still called on success of the cast.
Note - the min (min,5000) will also get taken out in the change.

edit: my patch is bugged still working on it. I'm going to use param5 as the interval check;

note - also dont blame others code, as the bug has always existed as far as I can tell.

Patch:
https://gist.github.com/Langerz82/14e10a876866c3daf26b3bbf98aba83d

Edit: Max range doesnt need to be set I just checked then. So Twilight Overlords should work as intended without any changes to SQL.

[Langerz82]

Updated

[Killyana]

@Langerz82 can you explain when this 500ms is used and the purpose of param5.
This is the old behavior event_param1=initial min, event_param2=initial max, event_param1=repeat min, event_param2=repeat max what changed exactly, an when this line is executed:
``` cpp
else
RecalcTimer(e, std::min(min, 5000), std::min(min, 5000));

[Langerz82]

the 500ms is the interval that the Smart Event will check against the conditions if the cast cannot be made or if the condition resulted in false. 500ms is the default value but it can be overridden with the event_param5.
The min values where removed with my patch, as they cause a huge delay when checking against the condition.
This is only for SMART_EVENT_UPDATE_IC that the interval is 500ms default and SMART_ACTION_CAST when there is not target.
This can be done for ranged attacks too but atm we just testing Magic and making sure it's working fine.

[Killyana]

I tested the patch, and the result is the same:
If the npc never evaded before it will chase the player for some seconds.
If the player go out of range or not in los the npc will start chasing and cast at the same time.

I tested and old build https://github.com/TrinityCore/TrinityCore/commit/052fc24315ace866ea1cf610e85df119b68100c9
And the issue doesn't happen, the SmartScript.cpp is exactly the same as the current revision, so probably, it's related to something else

[Langerz82]

Can you post the Smart scripts for the twilight overlord?

SELECT * FROM smart_scripts WHERE entryorguid=15213;

[Killyana]

INSERT INTO smart scripts (entryorguid, source_type, id, link, event_type, event_phase_mask, event_chance, event_flags, event_param1, event_param2, event_param3, event_param4, event_param5, action_type, action_param1, action_param2, action_param3, action_param4, action_param5, action_param6, target_type, target_param1, target_param2, target_param3, target_x, target_y, target_z, target_o, COMMENT) VALUES
(15213, 0, 0, 0, 0, 0, 100, 0, 0, 0, 3400, 4800, 0, 11, 9672, 64, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, "Twilight Overlord - In Combat - Cast Frostbolt"),
(15213, 0, 1, 0, 9, 0, 100, 0, 0, 8, 13000, 15000, 0, 11, 11831, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, "Twilight Overlord - Within 0-8 Range - Cast Frost Nova"),
(15213, 0, 2, 0, 0, 0, 100, 0, 8000, 9000, 24000, 25000, 0, 11, 12058, 1, 0, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, "Twilight Overlord - In Combat - Cast Chain Lightning"),
(15213, 0, 3, 0, 0, 0, 100, 0, 5000, 6000, 15000, 21000, 0, 11, 13339, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, "Twilight Overlord - In Combat - Cast Fire Blast");

But it's better to test with this npc, there's only one action during the combat:
``` sql
INSERT INTO smart scripts (entryorguid, source_type, id, link, event_type, event_phase_mask, event_chance, event_flags, event_param1, event_param2, event_param3, event_param4, event_param5, action_type, action_param1, action_param2, action_param3, action_param4, action_param5, action_param6, target_type, target_param1, target_param2, target_param3, target_x, target_y, target_z, target_o, COMMENT) VALUES
(430, 0, 0, 0, 4, 0, 15, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, "Redridge Mystic - On Aggro - Say Line 0"),
(430, 0, 1, 0, 0, 0, 100, 0, 0, 0, 3000, 5000, 0, 11, 20802, 64, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, "Redridge Mystic - In Combat CMC - Cast 'Lightning Bolt'"),
(430, 0, 2, 0, 2, 0, 100, 1, 0, 15, 0, 0, 0, 25, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, "Redridge Mystic - Between 0-15% Health - Flee For Assist (No Repeat)");

[Langerz82]

Made a fix, turns out MotionMaster remove is not forcing the Creatures to stop, so I called a force clear instead.

See:
https://gist.github.com/Langerz82/14e10a876866c3daf26b3bbf98aba83d

[Killyana]

It fixes the issue, but:

• The recheckDelay is used as initial min/max if the creature aggro for the first time
• After checking, it looks the 500ms for recheckDelay is too low.

It's better to test on retail and get the exact value for recheckDelay.
EDIT: I made some tests on retail an the recheckDelay is 5 secs

[Langerz82]

Try the patch again, I edited it directly so it may error when applying. The 500ms gets added onto the min/max values.

[Langerz82]

I added 1second to the interval check and put it as an addition to the min/max values.

[Langerz82]

now the mob charges because the interval check is too slow. Let me tinker with the values a bit.

[Langerz82]

We need to set in SmartAI mCanCombatMove = false for Casters and Ranged Creature Types. @Shauren is there a variable we can use for detecting if there Melee, Casters or Ranged? I've been looking but have not found anything yet.

[Killyana]

Maybe @ccrs ca help with this.

[TerrorBringer]

Any progress on this?

[pmcloudwow]

Is there already a solution to the problem?

[Bluechen80]

Just tested this with the Defias Pillager in Moonbrook. The bug is still present. They move toward you hit u with the staff and then cast a fireball instead of casting the fireball instantly.

[joshwhedon]

So, quick question : any specific reason in the UpdateAI of SmartAI that we call UpdateVictim() after all the rest ?
Because the root issue seems to be that we execute the first event (at 0), at this point there is no victim, so when we process the action, nothing really happens, movement starts, the creature chases.
Now I tried moving UpdateVictim() before the event processing, and ... it seems to work well ? Victim is selected, spell is casted, movement is stopped.
So yeah, is it as simple as moving the UpdateVictim() earlier ? Or was it put there for a reason and doing so will make things worse.

[Bluechen80]

For me that's one of the most serious bugs ever. I am surprised that no one has ever responded.

[ZenoX92]

@joshwhedon, why don't you open a pull request for this?

[joshwhedon]

@ZenoX92 a few reasons. The first one, I am lazy. The second one, I have no idea if it is the right thing to do, I am just providing the result of some investigation of my part and turning toward more experienced people to see if the assumption is correct. The third one, I really don't want to take the responsibility if whatever I propose make things worse than they are :D .

[Bluechen80]

Well it looks like nobody cares... thats so sad.

[Aokromes]

you are welcomed to create a fix for it!

[joshwhedon]

I made the change on my side, we'll deploy it to production probably in a few weeks, if it is not fixed I will confirm if moving the UpdateVictim() has no negative impact or not.

[Bluechen80]

any news on this ?

[illfated]

Any results yet from your testing, @joshwhedon ?

[joshwhedon]

Still didn't go live with it, all I have is my personal testing which seems fine. Will prepare a patch this week end and provide it and we'll see.

[ccrs]

https://github.com/TrinityCore/TrinityCore/issues/21562#issuecomment-504722402

nice catch, It really feels like its the root problem

[ccrs]

nvm that, retest again, that commit most likely closes this

[Killyana]

The issue is the same after https://github.com/TrinityCore/TrinityCore/commit/0e3e4353a1824dd6e40ca10a01c4465aa1b1fbad

[joshwhedon]

https://gist.github.com/joshwhedon/4db267583efa36f7118b5fc27765dbeb

Prepared the following patch. Worked for me. Limited the changes to the maximum (it might be possible to move the UpdateVictim() around if some issues arise).

[Bluechen80]

any news on this ? is it fixed ?

[TerrorBringer]

The fix by joshwhedon is working!

[Bluechen80]

The fix by joshwhedon is working!

Wow this are fantastic news :)

[TerrorBringer]

Completed Nagrand and currently in Blade's Edge, it is still working fine.

[illfated]

Looks a bit surprising to me that the patch is effective, seeing how simple it looks.
Anyway, updated for current commit, b98f858cd2976804ef75e2fd90b4bb914eb7eb71 :

https://github.com/TrinityCore/TrinityCore/blob/b98f858cd2976804ef75e2fd90b4bb914eb7eb71/src/server/game/AI/SmartScripts/SmartAI.cpp#L275-L300

diff --git a/src/server/game/AI/SmartScripts/SmartAI.cpp b/src/server/game/AI/SmartScripts/SmartAI.cpp
index 9dd7f4dbb15..89b918478cf 100644
--- a/src/server/game/AI/SmartScripts/SmartAI.cpp
+++ b/src/server/game/AI/SmartScripts/SmartAI.cpp
@@ -276,30 +276,32 @@ void SmartAI::UpdateAI(uint32 diff)
 {
     if (!me->IsAlive())
     {
         if (IsEngaged())
             EngagementOver();
         return;
     }

     CheckConditions(diff);

+    bool hasVictim = UpdateVictim();
+
     GetScript()->OnUpdate(diff);

     UpdatePath(diff);
     UpdateFollow(diff);
     UpdateDespawn(diff);

     if (!IsAIControlled())
         return;

-    if (!UpdateVictim())
+    if (!hasVictim)
         return;

     if (_canAutoAttack)
         DoMeleeAttackIfReady();
 }

 bool SmartAI::IsEscortInvokerInRange()
 {
     if (ObjectVector const* targets = GetScript()->GetStoredTargetVector(SMART_ESCORT_TARGETS, *me))
     {

[TerrorBringer]

Update: Completed Blade's Egde and nearly finished Netherstrom, cleared some dungeons, such as Shadow Labyrinth and Botanica, everything is working .

[Aokromes]

@joshwhedon can you open one PR for this?

[Bluechen80]

Is this fixed now ?

[Killyana]

No, even with the fix above the npc will cast while chasing its target.
This happen only with npcs with MovementType=0, if the npc use random movements or waypoints everything works fine.

[Killyana]

@ccrs can I push https://github.com/TrinityCore/TrinityCore/issues/21562#issuecomment-530450684 as it fixes this issue, the remaining one is not related to this issue.