Trinitycore: [3.3.5] Spells movement problem - Scatter Shot visual bug

[Re3os]

Rev https://github.com/TrinityCore/TrinityCore/commit/bffaa92633937d220ab19f565faa5906995530a2
Branch 3.3.5
Patch: Clean TC.

Example:
If you use Scatter Shot in moviment targets, he "teleport" in desorientation moviment.

Old Commit has fixed that, dont work anymore (https://github.com/TrinityCore/TrinityCore/pull/7975)

Videos of servers based at Trinity

https://www.youtube.com/watch?v=28aRmHO-WpQ
https://www.youtube.com/watch?v=pSDQ5yZGmx4

How should work : https://www.youtube.com/watch?time_continue=119&v=PVhwiHmV1BI
(Even as a private server based on TC, is there working properly, i dont have time for search vid on retail)

Tc Rev 8a0bbc386982612077e6f83f1816b02be757e371

Old Issues: #2689 #2013 #466 #1011

old fix for this was https://github.com/TrinityCore/TrinityCore/commit/3d644d8d119b9009f10ea637c52a01dad32708c9

but this no longer works

Close issue still problem
https://github.com/TrinityCore/TrinityCore/issues/9475
https://github.com/TrinityCore/TrinityCore/issues/11530
https://github.com/TrinityCore/TrinityCore/issues/11530
https://github.com/TrinityCore/TrinityCore/issues/11597

[Rushor]

the must be some option to stop the client-side movement. there is a similar problem if you cast scatter on someone. https://github.com/TrinityCore/TrinityCore/issues/466

example:
hunter A und mage B start duel
mage B start running in any direction
hunter A casts scatter on mage B
if the disorient effect hits mage B, he directly stops movement (from the point of viwe of the mage)
if the disorient effect hits the mage, he runs 1-2 seconds in the direction he originally wants to reach and then he teleports back to the position where the disorient effect was applied (from the point of viwe of the hunter A)

[Rushor]

the old issue with a fix, but this fix no longer works https://github.com/TrinityCore/TrinityCore/pull/7975/commits

[Rushor]

similar to the scatter issue https://www.youtube.com/watch?v=VDUshI-Ul9A (only found the video in this random-github-issue https://github.com/dalaranwow/dalaran-wow/issues/3030)

[Rushor]

@Re3os: you deleted your video.
@DevRival: you deleted your gist/fixtry

Pls update your links and post it again.

[Natureknight]

bet should be something like this? (i have modified alot regarding movement in my core, but the problem with 'teleporting' described in this issue is gone)

original: https://www.youtube.com/watch?v=OuEmsLhc2IA
modified: https://www.youtube.com/watch?v=UKIjYihhgzY

Hash: 923a368

[Rushor]

cuz noone actually described the problem exactly and the people removed their videos. open a seperated issue for this with a CLEAR description and without removing the videos after 1 month.

and share your pserver fixes!

[Keader]

Share share share :+1:

[NobruGit]

@Natureknight Share please

[Laintime]

@Natureknight why this post?bla bla bla?

[Goatform]

I know that @Natureknight has freedom to chose if he's going to share his fix(es) or not but isn't that dumb what he did. He acts like: "Ohh I fixed this on my private server..." ... I mean... so what? If you're not going to share it with TC then what's the purpose of your comment at all. It's just very rude. By the way, I'll try to fix this issue somehow and share fix with TC community, instead of begging some people. Also, I know that @Keader and @Rushor are talented developers willing to help TC's community, so I would like to step in contact with you guys if you're willing to help me fixing this issue.

[Aokromes]

Irc is the best way to contact other members.
irc.rizon.net #trinity

[Natureknight]

@Goatform just asked if it looks alright, cuz i did NOT fixed the issue, just made a workaround/hackfix which im using for a temporary solution till i find the source of the problem (which i thinks is in the movespline system (specially in MoveSplineInit::Launch()) where player's dest are not calculated right if the previous movement is not yet finished.

I can ofc share it, but as far as TC won't accept hackfixes it wont be approved you know

[Keader]

Can you share your hackfix ?!

[Goatform]

@Natureknight, no disrespect man. Also, from what I know, every hackfix is welcome here as well as blizzlike fix because it will help us more to find better solution to fix this issue.

[Natureknight]

well im pretty sure that problem is in MoveSplineInit::Launch()

As I said, I did a lot of changes regarding movement, but i think this which affects this problem is located in void FleeingMovementGenerator::DoInitialize(T* owner)

Try this:

void FleeingMovementGenerator<T>::DoInitialize(T* owner)
{
    if (!owner)
        return;

    owner->SetFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_FLEEING);
    owner->AddUnitState(UNIT_STATE_FLEEING | UNIT_STATE_FLEEING_MOVE);

+    if (owner->HasUnitMovementFlag(MOVEMENTFLAG_MASK_MOVING))
+    {
+        owner->StopMoving();
+        i_nextCheckTime.Reset(200);
+        return;
+    }
+
    _setTargetLocation(owner);
}

you can test and provide feedback, but as I said, i think the problem is in MoveSplineInit::Launch() where dest can't be got correctly if the unit's movespline is not yet finalized

[Keader]

@Natureknight Thanks for share
use ``` diff
And give me error

Error 4 error C2065: 'MOVEMENTFLAG_MOVING' : undeclared identifier D:\tc\src\server\game\Movement\MovementGenerators\FleeingMovementGenerator.cpp 138 1 game

MOVEMENTFLAG_MOVING = ?

[Goatform]

@Keader, here we go:

diff --git a/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp b/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp
index dd33ef2..65ef0ec 100644
--- a/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp
+++ b/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp
@@ -111,6 +111,14 @@ void FleeingMovementGenerator<T>::DoInitialize(T* owner)

     owner->SetFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_FLEEING);
     owner->AddUnitState(UNIT_STATE_FLEEING);
+
+    if (owner->HasUnitMovementFlag(MOVEMENTFLAG_MASK_MOVING))
+    {
+        owner->StopMoving();
+        i_nextCheckTime.Reset(200);
+        return;
+    }
+
     _setTargetLocation(owner);
 }

@Natureknight, I also think that you could implement this to ConfusedMovementGenerator:

diff --git a/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp b/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
index b04f860..3317ed9 100755
--- a/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
+++ b/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
@@ -39,8 +39,15 @@ void ConfusedMovementGenerator<T>::DoInitialize(T* unit)
     if (!unit->IsAlive() || unit->IsStopped())
         return;

-    unit->StopMoving();
     unit->AddUnitState(UNIT_STATE_CONFUSED_MOVE);
+
+    if (unit->HasUnitMovementFlag(MOVEMENTFLAG_MASK_MOVING))
+    {
+        unit->StopMoving();
+        i_nextMoveTime.Reset(200);
+        return;
+    }
+
 }

 template<class T>

[Keader]

@Goatform missing some ()

diff --git a/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp b/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp
index 6cdd299..e02db90 100644
--- a/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp
+++ b/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp
@@ -121,6 +121,14 @@ void FleeingMovementGenerator<T>::DoInitialize(T* owner)

     owner->SetFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_FLEEING);
     owner->AddUnitState(UNIT_STATE_FLEEING | UNIT_STATE_FLEEING_MOVE);
+
+    if (owner->HasUnitMovementFlag(MOVEMENTFLAG_MASK_MOVING))
+    {
+        owner->StopMoving();
+        i_nextCheckTime.Reset(200);
+        return;
+    }
+
     _setTargetLocation(owner);
 }

For Fear it works. But Confusing Spells (Scatter Shot) dont

[Goatform]

@Keader, did you tried ConfusedMovementGenerator part from my diff ?

[Keader]

Yeap, but dont work for this case

[Goatform]

Aham, okay. I'll do some research about this.

[Laintime]

@Goatform

 Movement/MovementGenerators/ConfusedMovementGenerator.cpp:44:9: error: ‘i_nextCheckTime’ was not declared in this scope
         i_nextCheckTime.Reset(200);

[Keader]

@Laintime change for i_nextMoveTime.Reset(200);
But dont work anyway xD

[Laintime]

@Keader thx,i change.
Confirm dont work(

[NobruGit]

any solution to the problem of fear?
https://www.youtube.com/watch?v=WnstKQFITV8

[Laintime]

@Natureknight meybe more change?

[Natureknight]

I think Scatter shot visual issues are caused cuz of update in movespline system

[Rushor]

the old update from healthstone/RE

diff --git a/src/server/game/Entities/Unit/Unit.h b/src/server/game/Entities/Unit/Unit.h
index 30d9698..bd7f0b2 100644
--- a/src/server/game/Entities/Unit/Unit.h
+++ b/src/server/game/Entities/Unit/Unit.h
@@ -2213,6 +2213,7 @@ class Unit : public WorldObject
         bool IsAlwaysVisibleFor(WorldObject const* seer) const override;
         bool IsAlwaysDetectableFor(WorldObject const* seer) const override;

+    public:
         void DisableSpline();
     private:
         bool IsTriggeredAtSpellProcEvent(Unit* victim, Aura* aura, SpellInfo const* procSpell, uint32 procFlag, uint32 procExtra, WeaponAttackType attType, bool isVictim, bool active, SpellProcEventEntry const* & spellProcEvent);
diff --git a/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp b/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
index 108276c..4bcf84f 100644
--- a/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
+++ b/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
@@ -74,6 +74,28 @@ bool ConfusedMovementGenerator<T>::DoUpdate(T* unit, uint32 diff)
         i_nextMoveTime.Update(diff);
         if (i_nextMoveTime.Passed())
         {
+            if (owner->HasUnitMovementFlag(MOVEMENTFLAG_MASK_MOVING))
+            {
+                owner->StopMoving();
+                owner->DisableSpline();
+
+                Movement::Location loc = owner->movespline->ComputePosition();
+
+                if (owner->movespline->onTransport)
+                {
+                    Position& pos = owner->m_movementInfo.transport.pos;
+                    pos.m_positionX = loc.x;
+                    pos.m_positionY = loc.y;
+                    pos.m_positionZ = loc.z;
+                    pos.SetOrientation(loc.orientation);
+
+                    if (TransportBase* transport = owner->GetDirectTransport())
+                        transport->CalculatePassengerPosition(loc.x, loc.y, loc.z, &loc.orientation);
+                }
+
+                owner->UpdatePosition(loc.x, loc.y, loc.z, loc.orientation);
+            }
+
             // start moving
             unit->AddUnitState(UNIT_STATE_CONFUSED_MOVE);

@@ -83,19 +105,26 @@ bool ConfusedMovementGenerator<T>::DoUpdate(T* unit, uint32 diff)
             pos.Relocate(i_x, i_y, i_z);
             unit->MovePositionToFirstCollision(pos, dest, 0.0f);

-            PathGenerator path(unit);
-            path.SetPathLengthLimit(30.0f);
-            bool result = path.CalculatePath(pos.m_positionX, pos.m_positionY, pos.m_positionZ);
-            if (!result || (path.GetPathType() & PATHFIND_NOPATH))
-            {
-                i_nextMoveTime.Reset(100);
-                return true;
-            }
-
             Movement::MoveSplineInit init(unit);
-            init.MovebyPath(path.GetPath());
+            init.MoveTo(pos.m_positionX, pos.m_positionY, pos.m_positionZ, true, false);
             init.SetWalk(true);
             init.Launch();
+
+            Movement::Location loc = unit->movespline->ComputePosition();
+
+            if (unit->movespline->onTransport)
+            {
+                Position& pos = unit->m_movementInfo.transport.pos;
+                pos.m_positionX = loc.x;
+                pos.m_positionY = loc.y;
+                pos.m_positionZ = loc.z;
+                pos.SetOrientation(loc.orientation);
+
+                if (TransportBase* transport = unit->GetDirectTransport())
+                    transport->CalculatePassengerPosition(loc.x, loc.y, loc.z, &loc.orientation);
+            }
+
+            unit->UpdatePosition(loc.x, loc.y, loc.z, loc.orientation);
         }
     }

diff --git a/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp b/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp
index 6cdd299..0ef32ab 100644
--- a/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp
+++ b/src/server/game/Movement/MovementGenerators/FleeingMovementGenerator.cpp
@@ -38,6 +38,29 @@ void FleeingMovementGenerator<T>::_setTargetLocation(T* owner)
     if (owner->HasUnitState(UNIT_STATE_ROOT | UNIT_STATE_STUNNED))
         return;

+
+    if (owner->HasUnitMovementFlag(MOVEMENTFLAG_MASK_MOVING))
+    {
+        owner->StopMoving();
+        owner->DisableSpline();
+
+        Movement::Location loc = owner->movespline->ComputePosition();
+
+        if (owner->movespline->onTransport)
+        {
+            Position& pos = owner->m_movementInfo.transport.pos;
+            pos.m_positionX = loc.x;
+            pos.m_positionY = loc.y;
+            pos.m_positionZ = loc.z;
+            pos.SetOrientation(loc.orientation);
+
+            if (TransportBase* transport = owner->GetDirectTransport())
+                transport->CalculatePassengerPosition(loc.x, loc.y, loc.z, &loc.orientation);
+        }
+
+        owner->UpdatePosition(loc.x, loc.y, loc.z, loc.orientation);
+    }
+
     owner->AddUnitState(UNIT_STATE_FLEEING_MOVE);

     float x, y, z;
@@ -52,24 +75,31 @@ void FleeingMovementGenerator<T>::_setTargetLocation(T* owner)
                                                                                 x, y, z + 2.0f);
     if (!isInLOS)
     {
-        i_nextCheckTime.Reset(200);
-        return;
-    }
-
-    PathGenerator path(owner);
-    path.SetPathLengthLimit(30.0f);
-    bool result = path.CalculatePath(x, y, z);
-    if (!result || (path.GetPathType() & PATHFIND_NOPATH))
-    {
         i_nextCheckTime.Reset(100);
         return;
     }

     Movement::MoveSplineInit init(owner);
-    init.MovebyPath(path.GetPath());
+    init.MoveTo(x, y, z, true, false);
     init.SetWalk(false);
     int32 traveltime = init.Launch();
     i_nextCheckTime.Reset(traveltime + urand(800, 1500));
+
+    Movement::Location loc = owner->movespline->ComputePosition();
+
+    if (owner->movespline->onTransport)
+    {
+        Position& pos = owner->m_movementInfo.transport.pos;
+        pos.m_positionX = loc.x;
+        pos.m_positionY = loc.y;
+        pos.m_positionZ = loc.z;
+        pos.SetOrientation(loc.orientation);
+
+        if (TransportBase* transport = owner->GetDirectTransport())
+            transport->CalculatePassengerPosition(loc.x, loc.y, loc.z, &loc.orientation);
+    }
+
+    owner->UpdatePosition(loc.x, loc.y, loc.z, loc.orientation);
 }

 template<class T>
@@ -111,6 +141,8 @@ void FleeingMovementGenerator<T>::_getPoint(T* owner, float &x, float &y, float
     x = pos.m_positionX;
     y = pos.m_positionY;
     z = pos.m_positionZ;
+
+    owner->UpdateOrientation(angle);
 }

 template<class T>
@@ -121,6 +153,29 @@ void FleeingMovementGenerator<T>::DoInitialize(T* owner)

     owner->SetFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_FLEEING);
     owner->AddUnitState(UNIT_STATE_FLEEING | UNIT_STATE_FLEEING_MOVE);
+
+    if (owner->HasUnitMovementFlag(MOVEMENTFLAG_MASK_MOVING))
+    {
+        owner->StopMoving();
+        owner->DisableSpline();        
+
+        Movement::Location loc = owner->movespline->ComputePosition();
+
+        if (owner->movespline->onTransport)
+        {
+            Position& pos = owner->m_movementInfo.transport.pos;
+            pos.m_positionX = loc.x;
+            pos.m_positionY = loc.y;
+            pos.m_positionZ = loc.z;
+            pos.SetOrientation(loc.orientation);
+
+            if (TransportBase* transport = owner->GetDirectTransport())
+                transport->CalculatePassengerPosition(loc.x, loc.y, loc.z, &loc.orientation);
+        }
+
+        owner->UpdatePosition(loc.x, loc.y, loc.z, loc.orientation);
+    }
+
     _setTargetLocation(owner);
 }

maybe someone can try this, do you think there is any smarter solution to stop this visual movementissue?

[robinsch]

IIRC: Some spells should wait for the target to reach the selected destination before sending them to a new one. For example Fear, some should not.

// Spells with SPELL_AURA_MOD_CONFUSE and BasePoints == 1 (or higher) should skip finalized check in movement function
// This will effect Scatter Shot and Blind movementgen

For the visual issue you need to call StopMoving and remove movementflags before adding new one.

[Rushor]

diff --git a/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp b/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
index 2ab4089..f7cf863 100755
--- a/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
+++ b/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
@@ -76,6 +76,8 @@ bool ConfusedMovementGenerator<T>::DoUpdate(T* unit, uint32 diff)
         i_nextMoveTime.Update(diff);
         if (i_nextMoveTime.Passed())
         {
+            unit->StopMoving();
+            unit->RemoveUnitMovementFlag(MOVEMENTFLAG_MASK_MOVING);
             // start moving
             unit->AddUnitState(UNIT_STATE_CONFUSED_MOVE);

hm no that doesn't solve the visual issue :>

[Keader]

@Rushor about: https://github.com/TrinityCore/TrinityCore/issues/15952#issuecomment-189687013 on ConfusedMovementGenerator<T>::DoUpdate(T* unit, uint32 diff) owner should be unit i think
And this fix works for Fear but dont work for Scatter shot

[Rushor]

well yeah, that's why i asked about:

maybe someone can try this, do you think there is any smarter solution to stop this visual movementissue?

[robinsch]

I'm 100% sure that the issue is caused by not removing movementflags / not stopping movement. This fixed it for me.

[ghost]

@Re3os : why did you close this issue? Unless this issue has been fixed in TrinityCore, don't close.

[Greymane]

This has to be the gayest bug i've ever seen, you can basically force the player to stop moving and update its position to everyone around, and he will still keep running even after getting the scatter shot, lol

[Rushor]

so adding:

void Unit::UpdateSplineMovement(uint32 t_diff)
{
+    // prevent spline to be updated if motionmaster is not
+    if (HasUnitState(UNIT_STATE_ROOT | UNIT_STATE_STUNNED | UNIT_STATE_CONFUSED))
+       return;

    if (movespline->Finalized())
        return;

    movespline->updateState(t_diff);
    bool arrived = movespline->Finalized();

    if (arrived)
        DisableSpline();

    m_movesplineTimer.Update(t_diff);
    if (m_movesplineTimer.Passed() || arrived)
        UpdateSplinePosition();
}

like in https://github.com/TrinityCore/TrinityCore/issues/16681 could fix this?

[L30m4nc3r]

@Rushor I tested, it did not help.

[Rushor]

k thanks for the test :(

[sucklifex]

Stunning flying units when they are in moving state, client show them in wrong position ( most of times they blink forward )

how to reproduce:
player A get distance from ground ( could be easily done by .gm fly on )
player B use stun on player A when player A is moving and have distance from ground ( like flying or swimming )

( The lichking's Valkyrie has same problem )

any of these patch from this issue works ?

[Keader]

@chaodhib maybe you can see it (when you have free time)?

[chaodhib]

@Keader i'll have a look when i can

[Aokromes]

This bug/fix(es) is(are) still valid after fe68b2e99851a888c5182d7bebddfea67459afd3 ?

[Aknunx]

@Chaodhib I reported https://github.com/TrinityCore/TrinityCore/issues/18306 which could be related to this, could you take a look at it please?

[robinsch]

This should work.

diff --git a/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp b/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
index 108276c..a8857b8 100755
--- a/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
+++ b/src/server/game/Movement/MovementGenerators/ConfusedMovementGenerator.cpp
@@ -31,15 +31,13 @@
 template<class T>
 void ConfusedMovementGenerator<T>::DoInitialize(T* unit)
 {
-    if (!unit->IsAlive() || unit->IsStopped())
+    if (!unit->IsAlive())
         return;

     unit->StopMoving();

[chaodhib]

So I took a look at the issue today. The teleport thing is due to the way player movements are handled by TC. The server does not have the exact player positions at all times. What I mean by that is that when player->GetPosition() is called somewhere in the code, it actually WONT return the current position of the player. What it WILL return is the last received position of the player.

This PR is linked to the same issue but here is a more complete explanation:
To transmit player movements, the client sends to the server movement packets (1). These packets contains the current position of the player + the player inputs (2). And in order to reduce the amount of packets exchanged by the client and the server, the client only send a movement packet when the player inputs change (3) or if the last time a movement packet was sent was over 500ms ago (in that case, a "heartbeat packet" is sent).

And now, you may wonder, if each client receive the other players position in discrete steps, how come their movement look fluid on our screens? What we should see is them moving from point to point, right? The answer is that WoW clients use a strategy called dead reckoning, aka extrapolation. It uses the current position and the current direction of each player and make them move in a straight line until a new movement packet is received (4). And so on and so forth, thus creating the illusion of a continuous movement using only discrete positions. Tadaaaaa.

Now back to the issue:
A hunter casts Scatter shot on another player. When the player gets affected by the spell, he will be at a certain position. That position is what the players will see on their screens when the spell land. However because the server does not use extrapolation, it thinks that the player is still at the latest received position. Therefore, when the Confused MoveGen is gonna kick in, it's gonna start the spline at the wrong place.

In conclusion: I need more time to test and verify everything but I believe a clean fix to this issue involve (but may not be limited to) adding extrapolation support on the server, in order to have the actual position of the player, at all times. And this is the goal of the (work in progress) PR mentioned above.

(1) : Movement packets are named MSG_MOVE_* in TC and in sniffs produced by WPP.

(2) : player inputs are which movement keys are pressed by the player. Example: forward arrow + left arrow. Here is an example in a parsed sniff (using WPP): Movement Flags: Forward, TurnRight

(3) : this is a simplification. What actually triggers the sending of a new movement packet is the change of direction. direction and player inputs differ only in 3D, because of gravity. If a player runs toward a cliff and then starts to fall, the client will send a new movement packet at the beginning of the fall even though player inputs haven't changed.

(4) the client is actually smarter than that because it also takes into account any collision with the map, the landscape slope and gravity.

@Aknunx Yes, I believe these 2 issues are linked. They have the same root cause which is what I described above.

@robinsch I haven't tested but don't think this will work.

PS: OP linked a video from AT. Here is also a video from retail: https://youtu.be/jo-gUPRnFlc?t=5m32s showing the combo.

[chaodhib]

*clear throat hum, I was wrong. I looked more closely at the code today. While everything I explained above is true, it's not the main root cause of the issue at hand.

First, I noticed that the target was not stopped right there, as soon as the spell lands, if the target is a player. By sniffing the communication between the client and TC while testing (something that more developers should do, it really helps debugging), i realized that the server wasn't sending the packet to make the player stop. By logging some key methods, I managed the trace the problem. First, @robinsch's change is needed because unit->IsStopped() returns true on moving players for some reason. Second, the call to unit->StopMoving() does nothing if the condition movespline->Finalized() is verified, which is usually the case for players.

Here is a quick fix: https://github.com/TrinityCore/TrinityCore/pull/18336

I will look for a cleaner solution when I find time.

[Killyana]

I cannot reproduce it in rev. ffede34ba9a3