Trinitycore: Core/Movement: movement delay is too high

[Seyden]

Description:

Movement delay is too high.

Current behaviour:

Actually if you open 2 wow clients at the same time and you move with one of them.. you will see the huge delay.

As example.. if you would use a spell that gives you an animation.. the animation would be instant visible for the other client.. but the movement that should be handled with the animation is not sync

Steps to reproduce the problem:

1. Open 2 WoW clients
2. Move with one of the characters
3. Look at the delay when the character moves in the other wow process

Branch(es): master

TC rev. hash/commit: 6067d19dfc43f40590a2052d3ce9b75a14920aa3

Operating system: Windows Server 2012 R2

[Eredar]

Tested it and it's not too high. For some reason it says 9-12 ms ping on localhost and actually it looks like that. For example other Blizzard game Overwatch has way more bad sync with 50 ms ping.

[Seyden]

i don't talk about the ping.. try knockback or just move forward and backward.. i will make a video later

[Eredar]

Reported knockback in other issue. It has delay only for caster. But not sure about movement, for me it seems fine.

[Seyden]

Knockback doesn't have delay for caster.. the caster sees the delay correct.. all other players not.. but its not only knockback

[Eredar]

Looks like we have different cores XD Because typhoon target was knocked back instantly for me and seen only half way in the air for caster. Use it on other player, not npc.

[Seyden]

Wait wait.. you have to knock yourself back while looking on the other client.

[streetrat]

in WorldSession::HandleMovementOpcode
try to comment out this line
https://github.com/TrinityCore/TrinityCore/blob/master/src/server/game/Handlers/MovementHandler.cpp#L386
movementInfo.time = movementInfo.time + m_clientTimeDelay + MOVEMENT_PACKET_TIME_DELAY;
and see if anything changed

[Takenbacon]

When you first login movement is usually a bit delayed (assuming it has something to do with client interpolation calculation), wait about a minute before checking it.

[Palabola]

Delay exists, and it has some connection with Speed update, if a player got new speed any speed or effect like knockback it can be desynced for other clients.
It can be reproduced with .mod speed command aswell.

[Seyden]

@Takenbacon thats what i saw to.. but if you do something with a special movement, the movement gets a delay again xD after sometime, the delay is gone and your movement is instant again..

@streetrat i tried outcomment this part, yes, it helped a bit.. i just uploaded a video with this outcommented..
movementInfo.time = movementInfo.time + m_clientTimeDelay + MOVEMENT_PACKET_TIME_DELAY;

@Palabola oh ok,, didn't checked this

[Seyden]

https://www.youtube.com/watch?v=VxJkjRP_xUs

That video is with this line being commented out

movementInfo.time = movementInfo.time + m_clientTimeDelay + MOVEMENT_PACKET_TIME_DELAY;

Look specially at the first 15-20 seconds and after that at 3:50

Without this line commented out, the delay is a lot bigger

[P-Kito]

Wtf 2 seconds delay Oô

[Seyden]

Summons @Shauren

[chaodhib]

The 3.3.5 branch has the same issue. The issue is indeed related to the timestamp field inside the movement packets. This field and the way it must be handled is currently not well understood. This PR is also linked to the issue. Progress is slow though.

WoW uses a _jitter buffer_ which allows the client to receive a movement packet at a certain time but "play the movement" later (see this interesting read), on the time specified by the timestamp. In other games, this buffer is used to avoid jitters when the player's latency is fluctuating. The problem in TC is that this timestamp is not well computed.

[Seyden]

push.. the delay is way bigger than imagined..

[Langerz82]

Not sure if this will help, but it takes an average of defined samples and adds it onto the timer, rather than individual values which may have lag spikes.

See:
https://gist.github.com/Langerz82/264ecae42b007f22c468cf4316b7b77e

edit: give me time I'm investigating.
edit: It happens for the first 10 movement packets so I'm trialing send several move packets at the start and whether it has any affect.

[Langerz82]

I made it load the movement packets super fast right away, but it's clipping, see if i can get it stable.

edit:
Changes I made looks promising.

[Langerz82]

Fuck so excited think I got it working. I'll send the patch once I am done. How do I do a second PR without affecting one already waiting?

[tje3d]

@Langerz82 You need new branch that track's TrinityCore/TrinityCore -> 3.3.5

[chaodhib]

Just submit what you got for review. The other PR (if you are talking a bout mine) is nowhere near ready.

Ps: sorry about close/reopen issue. It was a fat finger missclick on the phone.

[Langerz82]

Here is the gist:
https://gist.github.com/Langerz82/77770f68f55a9d0f6ed6b97d6e539817

Note - object.h Contains is not needed so I removed it from this patch.

[Langerz82]

It was shown only 1 extra packet is needed to sync the client so I'll make a patch later today that condenses the code.

[Seyden]

Why I didn't see your comments :o and btw... are you sure that this spamMovement is correct ?

[Langerz82]

It's code is over the top for what it needs to do. I will be making a much shorter patch and PR it for it to be discussed and tested properly. My advice best wait till my new patch comes out and do not use it on a production server until it's committed.

[Langerz82]

Here is the revised patch I will be submitting for PR (in-case you want to test early).

https://gist.github.com/Langerz82/9a894c226710da08af1ccfa22c0490b0

[Langerz82]

The following code only sends a heartbeat to other players in the map when the player has changed to one. This will hopefully sync the clients without messing with packet flooding etc.
Needs testing, I couldnt de-sync my clients on the latest 335 version even with 3 clients (my computer is beast).

diff --git a/src/server/game/Handlers/MovementHandler.cpp b/src/server/game/Handlers/MovementHandler.cpp
index dd8f9bd94c..a47fc0e541 100644
--- a/src/server/game/Handlers/MovementHandler.cpp
+++ b/src/server/game/Handlers/MovementHandler.cpp
@@ -104,6 +104,22 @@ void WorldSession::HandleMoveWorldportAck()
         return;
     }

+    // Send a heartbeat to all players in the map to sync clients.
+    Map::PlayerList const& players = GetPlayer()->GetMap()->GetPlayers();
+    if (!players.isEmpty())
+    {
+        for (Map::PlayerList::const_iterator it = players.begin(); it != players.end(); ++it)
+        {
+            Player* target = it->GetSource();
+            if (target && target->IsInWorld())
+            {
+                WorldPacket* data;
+                GetPlayer()->BuildHeartBeatMsg(data);
+                target->GetSession()->SendPacket(data);
+            }
+        }
+    }
+
     // battleground state prepare (in case join to BG), at relogin/tele player not invited
     // only add to bg group and object, if the player was invited (else he entered through command)
     if (_player->InBattleground())

ref:
https://gist.github.com/Langerz82/9a894c226710da08af1ccfa22c0490b0

edit: Might of help uncommenting the difference :D I fixed in post.

[chaodhib]

Please test again once https://github.com/TrinityCore/TrinityCore/pull/18189 is merged and give me your feedback.

PS: the animation should not appear instantly on the second client. In WoW clients, there is a movement buffer: movement packets coming from other players are not shown as soon as the client receives them. Instead they are queued and played around 500ms later (the exact value is still to be proved). This is done to eliminate the impact of dropped packet and fluctuating latency. This technique is often used in video games and this kind buffer is often referred to as “de-jitter buffer”.

resources for the de-jitter buffer:

• https://gafferongames.com/post/state_synchronization/ at "Jitter Buffer" section
• The book "Development & Deployment of Multiplayer Online Games Vol. I" by "No Bugs Hare" has a nice section about this

[chaodhib]

The PR has been merged now. Please test again.

[Killyana]

The delay is approximately 500ms, so works as intended.

[chaodhib]

Good news! I'll wait for more feedback before closing this issue.

[Sorikoff]

I can confirm movement is way more smooth than before.

[chaodhib]

Nice! Thank you for the feedback. Closing the issue since it seems solved now. Will re-open if needed.