Trinitycore: Implement MoveSplineFlag::Enter_Cycle

[xvwyh]

The problem

Currently enter cycle spline flag is not being handled, or, as the comment in the code claims, "MoveSplineFlag::Enter_Cycle support dropped". But it's an important flag without which cyclic splines are broken on the server and the units end up moving in a different path than they are on the client.

What this flag does is that it specifies that the path contains an additional point at the beginning (after the fake catmull-rom point, of course) which is the unit's starting position, before it entered a looping path. It essentially informs that client that the first point in the spline is not part of the loop and must be discarded after one cycle:

Client does that, and thus the unit visually moves as intended, both on the first cycle and on every subsequent one. After the first cycle is done, it discards the first point and the spline now only contains the points of a looping path:

But the server doesn't. Server doesn't handle the flag and removes neither the flag, nor the first "entry" point from the path. This results in serverside unit's path being different from that on the client, and this also incorrectly informs all other clients, that encountered the unit on its subsequent cycles, that the path is still on its first loop. Both of these issues cause position desyncs, the severity of which are proportional to how far the unit was initially away from the first path point:

Too see the issue you would have to make a way to visualize unit's real serverside position, for example by spawning and despawning some creature every so often.

The solution

Don't blindly apply this patch, it has a few caveats which you can read below, and may require some additional work for improved experience. It's also a patch for my core, not for TC, but it should be trivial to port it.

diff -r b7421dfd4ba0 -r 7df6d3cd6702 src/server/game/Entities/Unit/Unit.cpp
--- a/src/server/game/Entities/Unit/Unit.cpp    Mon Dec 12 23:13:32 2016 +0300
+++ b/src/server/game/Entities/Unit/Unit.cpp    Fri Dec 16 00:43:26 2016 +0200
@@ -407,6 +407,20 @@
     movespline->updateState(t_diff);
     bool arrived = movespline->Finalized();

+    if (movespline->isCyclic())
+    {
+        m_splineSyncTimer.Update(t_diff);
+        if (m_splineSyncTimer.Passed())
+        {
+            m_splineSyncTimer.Reset(5000); // Retail value, do not change
+
+            WorldPacket data(SMSG_FLIGHT_SPLINE_SYNC, 4 + GetPackGUID().size());
+            Movement::PacketBuilder::WriteSplineSync(*movespline, data);
+            data.append(GetPackGUID());
+            SendMessageToSet(&data, true);
+        }
+    }
+
     if (arrived)
     {
         if (movespline->HasAnimation())
@@ -415,16 +429,11 @@
         DisableSpline();
     }

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

 void Unit::UpdateSplinePosition()
 {
-    static uint32 const positionUpdateDelay = 400;
-
-    m_movesplineTimer.Reset(positionUpdateDelay);
     Movement::Location loc = movespline->ComputePosition();

     if (movespline->onTransport)
diff -r b7421dfd4ba0 -r 7df6d3cd6702 src/server/game/Entities/Unit/Unit.h
--- a/src/server/game/Entities/Unit/Unit.h  Mon Dec 12 23:13:32 2016 +0300
+++ b/src/server/game/Entities/Unit/Unit.h  Fri Dec 16 00:43:26 2016 +0200
@@ -2567,7 +2567,7 @@
         uint32 m_state;                                     // Even derived shouldn't modify
         uint32 m_CombatTimer;
         uint32 m_lastManaUse;                               // msecs
-        TimeTrackerSmall m_movesplineTimer;
+        TimeTrackerSmall m_splineSyncTimer;

         Diminishing m_Diminishing;
         // Manage all Units threatening us
diff -r b7421dfd4ba0 -r 7df6d3cd6702 src/server/game/Movement/Spline/MoveSpline.cpp
--- a/src/server/game/Movement/Spline/MoveSpline.cpp    Mon Dec 12 23:13:32 2016 +0300
+++ b/src/server/game/Movement/Spline/MoveSpline.cpp    Fri Dec 16 00:43:26 2016 +0200
@@ -123,9 +123,8 @@
     if (args.flags.cyclic)
     {
         uint32 cyclic_point = 0;
-        // MoveSplineFlag::Enter_Cycle support dropped
-        //if (splineflags & SPLINEFLAG_ENTER_CYCLE)
-        //cyclic_point = 1;   // shouldn't be modified, came from client
+        if (splineflags.enter_cycle)
+            cyclic_point = 1;   // shouldn't be modified, came from client
         spline.init_cyclic_spline(&args.path[0], args.path.size(), args.initialOrientation, modes[args.flags.isSmooth()], cyclic_point);
     }
     else
@@ -202,7 +201,10 @@
 #define CHECK(exp) \
     if (!(exp))\
     {\
-        sLog->outError("MoveSplineInitArgs::Validate: expression '%s' failed for GUID: %u Entry: %u", #exp, unit->GetTypeId() == TYPEID_PLAYER ? unit->GetGUIDLow() : unit->ToCreature()->GetDBTableGUIDLow(), unit->GetEntry());\
+        if (unit)\
+            sLog->outError("MoveSplineInitArgs::Validate: expression '%s' failed for GUID: %u Entry: %u", #exp, unit->GetTypeId() == TYPEID_PLAYER ? unit->GetGUIDLow() : unit->ToCreature()->GetDBTableGUIDLow(), unit->GetEntry());\
+        else\
+            sLog->outError("MoveSplineInitArgs::Validate: expression '%s' failed for cyclic spline continuation", #exp);\
         return false;\
     }
     CHECK(path.size() > 1);
@@ -268,6 +270,42 @@
                 point_Idx = spline.first();
                 time_passed = time_passed % Duration();
                 result = Result_NextCycle;
+
+                // Remove first point from the path after one full cycle.
+                // That point was the position of the unit prior to entering the cycle and it shouldn't be repeated with continuous cycles.
+                if (splineflags.enter_cycle)
+                {
+                    splineflags.enter_cycle = false;
+
+                    MoveSplineInitArgs args { (size_t)spline.getPointCount() };
+                    args.path.assign(spline.getPoints().begin() + spline.first() + 1, spline.getPoints().begin() + spline.last());
+                    args.facing = facing;
+                    args.flags = splineflags;
+                    args.path_Idx_offset = point_Idx_offset;
+                    // MoveSplineFlag::Parabolic | MoveSplineFlag::Animation not supported currently
+                    //args.parabolic_amplitude = ?;
+                    //args.time_perc = ?;
+                    args.splineId = m_Id;
+                    args.initialOrientation = initialOrientation;
+                    args.velocity = 1.0f; // Calculated below
+                    args.HasVelocity = true;
+                    args.TransformForTransport = onTransport;
+                    if (args.Validate(nullptr))
+                    {
+                        // New cycle should preserve previous cycle's duration for some weird reason, even though
+                        // the path is really different now. Blizzard is weird. Or this was just a simple oversight.
+
+                        // Since our splines precalculate length with velocity in mind, if we want to find the desired
+                        // velocity, we have to make a fake spline, calculate its duration and then compare it to the
+                        // desired duration, thus finding out how much the velocity has to be increased for them to match.
+                        MoveSpline tempSpline;
+                        tempSpline.Initialize(args);
+                        args.velocity = (float)tempSpline.Duration() / Duration();
+
+                        if (args.Validate(nullptr))
+                            init_spline(args);
+                    }
+                }
             }
             else
             {
diff -r b7421dfd4ba0 -r 7df6d3cd6702 src/server/game/Movement/Spline/MoveSplineInit.cpp
--- a/src/server/game/Movement/Spline/MoveSplineInit.cpp    Mon Dec 12 23:13:32 2016 +0300
+++ b/src/server/game/Movement/Spline/MoveSplineInit.cpp    Fri Dec 16 00:43:26 2016 +0200
@@ -88,6 +88,7 @@
         // corrent first vertex
         args.path[0] = real_position;
         args.initialOrientation = real_position.orientation;
+        args.flags.enter_cycle = args.flags.cyclic;
         move_spline.onTransport = transport;

         uint32 moveFlags = unit->m_movementInfo.GetMovementFlags();
diff -r b7421dfd4ba0 -r 7df6d3cd6702 src/server/game/Movement/Spline/MovementPacketBuilder.cpp
--- a/src/server/game/Movement/Spline/MovementPacketBuilder.cpp Mon Dec 12 23:13:32 2016 +0300
+++ b/src/server/game/Movement/Spline/MovementPacketBuilder.cpp Fri Dec 16 00:43:26 2016 +0200
@@ -59,8 +59,6 @@
                 break;
         }

-        // add fake Enter_Cycle flag - needed for client-side cyclic movement (client will erase first spline vertex after first cycle done)
-        splineflags.enter_cycle = move_spline.isCyclic();
         data << uint32(splineflags & uint32(~MoveSplineFlag::Mask_No_Monster_Move));

         if (splineflags.animation)
@@ -115,10 +113,9 @@

     void WriteCatmullRomCyclicPath(const Spline<int32>& spline, ByteBuffer& data)
     {
-        uint32 count = spline.getPointCount() - 3;
-        data << uint32(count + 1);
-        data << spline.getPoint(1); // fake point, client will erase it from the spline after first cycle done
-        data.append<Vector3>(&spline.getPoint(1), count);
+        uint32 count = spline.getPointCount() - 4;
+        data << count;
+        data.append<Vector3>(&spline.getPoint(2), count);
     }

     void PacketBuilder::WriteMonsterMove(const MoveSpline& move_spline, ByteBuffer& data)
@@ -179,4 +176,9 @@
             data << (move_spline.isCyclic() ? Vector3::zero() : move_spline.FinalDestination());
         }
     }
+
+    void PacketBuilder::WriteSplineSync(MoveSpline const& move_spline, ByteBuffer& data)
+    {
+        data << (float)move_spline.timePassed() / move_spline.Duration();
+    }
 }
diff -r b7421dfd4ba0 -r 7df6d3cd6702 src/server/game/Movement/Spline/MovementPacketBuilder.h
--- a/src/server/game/Movement/Spline/MovementPacketBuilder.h   Mon Dec 12 23:13:32 2016 +0300
+++ b/src/server/game/Movement/Spline/MovementPacketBuilder.h   Fri Dec 16 00:43:26 2016 +0200
@@ -49,6 +49,7 @@
         static void WriteMonsterMove(const MoveSpline& mov, ByteBuffer& data);
         static void WriteStopMovement(Vector3 const& loc, uint32 splineId, ByteBuffer& data);
         static void WriteCreate(const MoveSpline& mov, ByteBuffer& data);
+        static void WriteSplineSync(MoveSpline const& mov, ByteBuffer& data);
     };
 }
 #endif // TRINITYSERVER_PACKET_BUILDER_H

There are a couple of points worth mentioning about this implementation, they are fairly important.

First point in the cyclic spline

With this implementation the duty lies on the programmer to call Movement::MoveSplineInit::MovebyPath with a path that contains that additional point with unit's current position in the beginning of the path. This may be inconvenient, so a better solution is advisable. The problem is that we can't add that point inside Movement::MoveSplineInit::MovebyPath automatically, because at that point (in time) it may not yet be known if the path is intended to be cyclic. The programmer may call it like this:

init.SetCyclic();
init.MovebyPath(points);
init.Launch();

or like this:

init.MovebyPath(points);
init.SetCyclic();
init.Launch();

And both versions should be equivalent in my opinion.

Duration of subsequent cycles

It may seem strange, but the client retains original spline duration (with enter_cycle point included) for every subsequent cycle. Yes, if the loop itself takes, say, 5 seconds to complete, but the unit is so far away from it that it takes him 10 seconds to even approach the loop, then each subsequent loop will be slow enough to take 15 seconds to complete, even though the unit is already in the loop and doesn't have to approach it anymore. This is blizzlike, the client handles it like this, and the patch replicates this behavior.

parabolic_amplitude and time_perc

Both are marked in my patch as not implemented, although I cannot imagine a situation when either of them would be used together with a cyclic spline.

initialOrientation

This might not be available in your spline, but if you resolve #22435, it will be.

SMSG_FLIGHT_SPLINE_SYNC

Every unit that's travelling a cyclic spline (not sure if just flying or any, didn't have a chance to examine sniffs with ground cyclic splines, if they even exist) sends SMSG_FLIGHT_SPLINE_SYNC every 5 seconds. The purpose of the packet is clear: to regularly inform the client of unit's serverside progress into the spline so they don't get out of sync, but the way client handles it might be a bit unexpected. The client doesn't snap unit's clientside position to the point provided in the packet. Nor does it ease unit's position in. What it does instead is it modifies the next cycle's duration in such a way, that by the end of it clientside unit should be in sync with serverside. So don't be surprised if during testing you don't see any immediate effect of the packet.

Also something worth noting is that its broadcast interval - 5 seconds - matches with my hypothesis about unit's heartbeat timers, which you can read about here: #22380. If such heartbeat timer is to be implemented, you could put SMSG_FLIGHT_SPLINE_SYNC broadcasting together with it.

m_movesplineTimer drop

You may also have noticed from the patch that I also completely drop m_movesplineTimer. This is intentional, although not related to cyclic splines. It completely baffles me that one would want to delay unit's position updates. I get it, it's an optimization, but it comes at a cost of position desyncs, which is not a side effect a good optimization should have. And 400ms? Are you even serious? I can see it being the cause of numerous times when a player attempted to cast a spell while being clearly in range only to be notified that the target is too far. There shouldn't be any delay, units should update their position as often as possible.

[Ovahlord]

Ah, nice to see someone cares about it. I have opened a ticket about that a while ago. This will probably help alot. What is missing here tho is the hack correction for the monster_move packet for the first vertice.

About the timer: 400ms is fine for units. Instead of dropping it, you may instead trigger an additional spline position update on casts instead.

[xvwyh]

Whoops. Sorry, forgot to check if there are any open issues about this. I suppose it's #21387, so let's link it here.

[xvwyh]

What is missing here tho is the hack correction for the monster_move packet for the first vertice.

Hmm? Could you be more specific?

About the timer: 400ms is fine for units. Instead of dropping it, you may instead trigger an additional spline position update on casts instead.

No, it's not fine. Forced position update on cast would be a dirty hack, and that wouldn't solve the issue of, for example, AoE targeting. Unless, of course, you want to force position update for every potential area/cone/chain target as well. And what about right-click interaction with NPCs? Visibility distance? You can see how quickly this would devolve into a messy pile of hacks.

How about instead making it a configurable option? Best of both worlds, let those that care about precision set the interval to 0, let those, whose toasters can't handle some basic math, keep it at 400.

[Ovahlord]

The monster_move packet for cyclic movement currently sends the first point twice to "fix" that first vertice clientside removal. That needs to be removed to properly support enter cycle now.

[xvwyh]

That is handled in the patch, take a look at WriteCatmullRomCyclicPath.

[Ovahlord]

Allright, seems to be fine from my side then. Gonna port and test the stuff soon(tm) and will give corresponding feedback

[Ovahlord]

Well, so far it seems to be working fine, I am still not a fan of that removed delay though. This will for sure cause some heavy load for transport offset calculations with every single update tick aside from vehicle passenger updates etc.

[xvwyh]

It has absolutely no effect on transports. Only the spline movement is delayed, whenever a creature is on transport it's constantly relocated to its new position.

If you mean the transport movement through a taxi spline, then transports have their own Transport::_positionChangeTimer timer, which is on 200ms delay (which I also removed in my core, because I very much like for everything to be in sync) and is not touched by this patch.

[Ovahlord]

Well then. Better later than never. The enter cycle part works so far. However there is one flaw inside that commit. It does not only erase the first but also the 2nd vertex so the circle will be missing one point so it looks a little bit off. Here an example:

https://youtu.be/gY_aUBp_1LE

My port commit: https://github.com/Ovahlord/TrinityCore/commit/3a1d24c7f5323641a7779d0e64975996ec127c4e

Used helper to trigger the cyclic movement:
summon->GetMotionMaster()->MoveCirclePath(summon->GetPositionX(), summon->GetPositionY(), summon->GetPositionZ(), 3.0f, true, 8);

[xvwyh]

You missed this part:

With this implementation the duty lies on the programmer to call Movement::MoveSplineInit::MovebyPath with a path that contains that additional point with unit's current position in the beginning of the path.

Your MotionMaster::MoveCirclePath supplies a path with JUST the cycle points: https://github.com/Ovahlord/TrinityCore/blob/3a1d24c7f5323641a7779d0e64975996ec127c4e/src/server/game/Movement/MotionMaster.cpp#L502

Either edit it to also add the unit's current position in the beginning or somehow make it that all cyclic splines have unit's current position point added automatically.

[xvwyh]

Look at this part of the code:
https://github.com/Ovahlord/TrinityCore/blob/3a1d24c7f5323641a7779d0e64975996ec127c4e/src/server/game/Movement/Spline/MoveSplineInit.cpp#L88

        // correct first vertex
        args.path[0] = real_position;

It changes the first point of the path to be unit's position. The thing is, when you're generating a path with PathGenerator, it already does that for you - makes the first point to be the initial source unit's position. But when you're building a path yourself like in MoveCirclePath, you don't do that, and thus an actual cycle path point gets overwritten with unit's original position when the spline is launched.

[Ovahlord]

Allright. So I'm gonna make an additional change and add the unit's original position as first spline point in MoveCirclePath so the vertex correction wont affect the cyclic path but the first point that gets removed later on.

Done: https://github.com/Ovahlord/TrinityCore/commit/bf03f41b15c81bab9ea3d8bbc3ff6a2b96a81dc0

[xvwyh]

Does it work correctly now for you?

[Ovahlord]

Yes. The cyclic movement seems to be fine now. No edges, no non-cyclic curves etc. I see no problems with that system anymore.

[Faq]

Can you make PR, so it can be reviewed?

[kelno]

Seems working for me as well, I'll report problems if I find any.

[Ovahlord]

Time for another odd case. It appears that the 2nd vetex which is the first cyclic point sometimes is overlapping with the last vertex so you see a 360° turnarround shortly before resuming the circle.
You can see the issue here: https://www.youtube.com/watch?v=eYeL-zs_Vto at 1:47

[xvwyh]

I haven't encountered such issues myself. I don't see Al'akir's script in your fork, so I can only offer guesses without seeing the implementation. You have the ability to debug though, so go and look at the spline points before and after completing the first cycle. IIRC they should look like this:

| Index | First cycle (w/ EnterCycle) | Index | Next cycle (w/o EnterCycle) |
|-------|-----------------------------|-------|-------------------------------|
| 0 | Start pos - 1 unit in direction of orientation | - | - |
| 1 | Start pos | 0 | Last cycle point |
| 2 | Cycle point 0 | 1 | Cycle point 0 |
| 3 | Cycle point 1 | 2 | Cycle point 1 |
| | ... | | ... |
| #-3 | Last cycle point | #-3 | Last cycle point |
| #-2 | Cycle point 0 | #-2 | Cycle point 0 |
| #-1 | Cycle point 1 | #-1 | Cycle point 1 |

On the first loop the last two points of the spline should be the duplicates of [2] and [3], on the subsequent cycles - the duplicates of [1] and [2], and the first point should be the duplicate of [count-3]. If you have MORE duplicates with the same positions, then you should debug it and figure out why it happened. The list of points that's being passed to MovebyPath shouldn't contain the first cycle point again at the end, SplineBase::InitCatmullRom takes care of that. My args.path.assign should only copy "Cycle point ..."s up to and including "Last cycle point", with the rest being taken care of by the aforementioned spline init.

If you want to know how these splines are supposed to work and why are there so many extra points:

Upon reaching [#-2] point the spline is considered completed, as with any other spline, but because this is a cyclic spline, it gets changed to "Next cycle" if it hasn't been already and the spline restarts from the beginning, which in Catmull-Rom splines means "from index [1]", as the first and last points of the splines are there only to control direction and speed at the beginning and end of the path. [#-2] happens to be the same position as [1] in both cases, hence the transitions between the loops is seamless.

...and I do hope you're not just testing how it visually looks to the client, but also checking serverside positions during the spline, moving/phasing in-and-out of visibility to check if everything behaves resyncs correctly when units reload from the server etc.

[Ovahlord]

The call for the squall line vehicle walker is plain and simple:
summon->GetMotionMaster()->MoveCirclePath(me->GetPositionX(), me->GetPositionY(), SquallLineDistanceReferencePos.GetPositionZ(), me->GetExactDist(SquallLineDistanceReferencePos), true, 11);
11 because the spline packet in the sniff is using 12 points --> start point + 11 cyclic points.
I am going to do some position debugging later this day when I finish and push Al'akir to see what's going on there.

[SilverIce]

@xvwyh maybe it's best to re-initialize the spline inside MoveSpline? It sounds like a hack (but, as you know, Blizz doing that already) - that's why I dropped that part of implementation and that's why the point that is being removed is equal to the second point - because the duration isn't being modified

[xvwyh]

This code has been running on my server for around-- actually, exactly 2 years and 1 day. I encountered no problems with spline being reinitialized that way, and I even think that's the way client does this, but I can't vouch for my memory being correct after 2 years. I can recheck if you want.

that's why I dropped that part of implementation and that's why the point that is being removed is equal to the second point - because the duration isn't being modified

Could you elaborate on what's confusing you here? Because I think I've described all this in the first post.

The point being removed isn't "equal" to the second point. The point being removed is the one from which the creature started entering the cyclic path. Of course if you summon the creature exactly on the position of the first path point, then they will be equal, but that doesn't HAVE to be that way. A creature could start from outside of the loop and run into the loop using just one cyclic spline.

The underlying reason of why the duration isn't being modified we don't really need to care about - that's the way client does things, and we're just mimicking what it does on the server so they stay in perfect sync.

---

I mean, I don't quite understand what your issue is. It IS being re-initialized inside MoveSpline, did you not notice init_spline(args);? Or did you mean initializing it outside of MoveSpline::_updateState? Replacing the entire Unit::movespline with a new one? (that's what the client does, now that I've checked)

[Ovahlord]

Necromancing this a bit. I am not sure if it's part of this PR here but I have observed that these cyclic splines are getting asynch with the real serverside position when the path is getting sent via CreateObject packet. Which means that players that have been arround when the npc started the movement are seeing things just fine while players that see the unit after the npc has started the movement are seeing asynch movement. So the question here is: does the spline still contain the first vertex or not coreside so the packet actually gets the entire path for the createObject movement data.

[xvwyh]

Before the first cycle is completed, entry point is still in the spline, and spline has "EnterCycle" flag.
Once the first cycle is completed, entry point is no longer part of the spline, it now contains only the closed loop, and "EnterCycle" flag is also removed.
CreateObject sends whatever is current spline on the unit, with all its points and flags and progress into the path. So if a player starts observing the unit after the first cycle is completed, the first point and the EnterCycle flag should no longer be there, and progress into the path should've silently overflown from first cycle into the second cycle.

At least his is how it's supposed to work, and how it's working on my server. I cannot know if something in Trinity is interfering with my proposed code, that's up to you to test and figure out.

[Ovahlord]

@xvwyh I think there's one scenario that's not handled yet and that's sending out units with ongoing cyclic splines via update_object. From what I see in sniffs, the spline path sends out the first three spline points of the cyclic path twice which probably keeps the path synchronous clientside.

At the moment, units will be sent out with the path but these three first spline points are not sent twice as it is in the sniffs so the movement gets asynchronous despite getting spline synch opcodes. Here's a sniff example:

[10] Spline flags: 6298112 (Flying, Catmullrom, Cyclic, CanSwim, UncompressedPath)
[10] RunBack Speed: 4.5
[10] SwimBack Speed: 2.5
[10] Spline Time: 28618
[10] [0] Spline Waypoint: X: 1380.3473 Y: 590.5677 Z: 132.40346
[10] [1] Spline Waypoint: X: 1322.2821 Y: 609.99603 Z: 132.40346
[10] [2] Spline Waypoint: X: 1294.9617 Y: 664.7923 Z: 132.40346
[10] [3] Spline Waypoint: X: 1314.39 Y: 722.85754 Z: 132.40346
[10] [4] Spline Waypoint: X: 1369.1863 Y: 750.178 Z: 132.40346
[10] [5] Spline Waypoint: X: 1427.2515 Y: 730.7496 Z: 132.40346
[10] [6] Spline Waypoint: X: 1454.5719 Y: 675.9533 Z: 132.40346
[10] [7] Spline Waypoint: X: 1435.1436 Y: 617.88806 Z: 132.40346
[10] [8] Spline Waypoint: X: 1380.3473 Y: 590.5677 Z: 132.40346
[10] [9] Spline Waypoint: X: 1322.2821 Y: 609.99603 Z: 132.40346
[10] [10] Spline Waypoint: X: 1294.9617 Y: 664.7923 Z: 132.40346

Any idea how to properly send that?

[xvwyh]

It has always been implemented:
https://github.com/TrinityCore/TrinityCore/blob/b304f4ad8d0683d800d9be5518911d1398609857/src/server/game/Movement/Spline/Spline.cpp#L252-L261

The actual path in your sniff example is:
[0] X: 1322.2821 Y: 609.99603 Z: 132.40346
[1] X: 1294.9617 Y: 664.7923 Z: 132.40346
[2] X: 1314.39 Y: 722.85754 Z: 132.40346
[3] X: 1369.1863 Y: 750.178 Z: 132.40346
[4] X: 1427.2515 Y: 730.7496 Z: 132.40346
[5] X: 1454.5719 Y: 675.9533 Z: 132.40346
[6] X: 1435.1436 Y: 617.88806 Z: 132.40346
[7] X: 1380.3473 Y: 590.5677 Z: 132.40346

Despite consisting of 8 positions, space is allocated for 11 points in the spline:
https://github.com/TrinityCore/TrinityCore/blob/b304f4ad8d0683d800d9be5518911d1398609857/src/server/game/Movement/Spline/Spline.cpp#L241-L243

[0] X: 0 Y: 0 Z: 0
[1] X: 0 Y: 0 Z: 0
[2] X: 0 Y: 0 Z: 0
[3] X: 0 Y: 0 Z: 0
[4] X: 0 Y: 0 Z: 0
[5] X: 0 Y: 0 Z: 0
[6] X: 0 Y: 0 Z: 0
[7] X: 0 Y: 0 Z: 0
[8] X: 0 Y: 0 Z: 0
[9] X: 0 Y: 0 Z: 0
[10] X: 0 Y: 0 Z: 0

The path gets inserted into the actual spline starting from index 1:
https://github.com/TrinityCore/TrinityCore/blob/b304f4ad8d0683d800d9be5518911d1398609857/src/server/game/Movement/Spline/Spline.cpp#L245-L248

[0] X: 0 Y: 0 Z: 0
[1] X: 1322.2821 Y: 609.99603 Z: 132.40346
[2] X: 1294.9617 Y: 664.7923 Z: 132.40346
[3] X: 1314.39 Y: 722.85754 Z: 132.40346
[4] X: 1369.1863 Y: 750.178 Z: 132.40346
[5] X: 1427.2515 Y: 730.7496 Z: 132.40346
[6] X: 1454.5719 Y: 675.9533 Z: 132.40346
[7] X: 1435.1436 Y: 617.88806 Z: 132.40346
[8] X: 1380.3473 Y: 590.5677 Z: 132.40346
[9] X: 0 Y: 0 Z: 0
[10] X: 0 Y: 0 Z: 0

And then the last point of the path gets copied into index 0 to allow it to inherit the curvature from the end of the loop:
https://github.com/TrinityCore/TrinityCore/blob/b304f4ad8d0683d800d9be5518911d1398609857/src/server/game/Movement/Spline/Spline.cpp#L255

[0] X: 1380.3473 Y: 590.5677 Z: 132.40346
[1] X: 1322.2821 Y: 609.99603 Z: 132.40346
[2] X: 1294.9617 Y: 664.7923 Z: 132.40346
[3] X: 1314.39 Y: 722.85754 Z: 132.40346
[4] X: 1369.1863 Y: 750.178 Z: 132.40346
[5] X: 1427.2515 Y: 730.7496 Z: 132.40346
[6] X: 1454.5719 Y: 675.9533 Z: 132.40346
[7] X: 1435.1436 Y: 617.88806 Z: 132.40346
[8] X: 1380.3473 Y: 590.5677 Z: 132.40346
[9] X: 0 Y: 0 Z: 0
[10] X: 0 Y: 0 Z: 0

And then the first 2 path points are copied into the end of the spline to allow it to [9] close the loop and [10] inherit the curvature from the beginning of the loop

[0] X: 1380.3473 Y: 590.5677 Z: 132.40346
[1] X: 1322.2821 Y: 609.99603 Z: 132.40346
[2] X: 1294.9617 Y: 664.7923 Z: 132.40346
[3] X: 1314.39 Y: 722.85754 Z: 132.40346
[4] X: 1369.1863 Y: 750.178 Z: 132.40346
[5] X: 1427.2515 Y: 730.7496 Z: 132.40346
[6] X: 1454.5719 Y: 675.9533 Z: 132.40346
[7] X: 1435.1436 Y: 617.88806 Z: 132.40346
[8] X: 1380.3473 Y: 590.5677 Z: 132.40346
[9] X: 1322.2821 Y: 609.99603 Z: 132.40346
[10] X: 1294.9617 Y: 664.7923 Z: 132.40346

[Ovahlord]

Yeah, I figured the issue out now. The packet building is messed up. That's the issue here:

Here's a little snipped from captured worldserver packets:

This packet here is a regular cyclic spline monster_move movement packet which works just fine.

(MovementMonsterSpline) (MovementSpline) Flags: 2112000 (Flying, Catmullrom, Cyclic, Enter_Cycle, CanSwim)
(MovementMonsterSpline) (MovementSpline) MoveTime: 44240
(MovementMonsterSpline) (MovementSpline) PointsCount: 9
(MovementMonsterSpline) (MovementSpline) Points: X: 1321.6118 Y: 594.1388 Z: 119.37121
(MovementMonsterSpline) (MovementSpline) [0] Waypoints: X: 1321.501 Y: 593.8894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [1] Waypoints: X: 1276.251 Y: 652.6394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [2] Waypoints: X: 1285.751 Y: 726.3894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [3] Waypoints: X: 1344.251 Y: 771.8894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [4] Waypoints: X: 1417.751 Y: 762.3894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [5] Waypoints: X: 1463.251 Y: 703.6394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [6] Waypoints: X: 1453.751 Y: 630.1394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [7] Waypoints: X: 1395.251 Y: 584.8894 Z: 119.37111

But now this is when the unit gets sent out via update_object:

[56] Spline flags: 2112000 (Flying, Catmullrom, Cyclic, EnterCycle, CanSwim)
[56] RunBack Speed: 4.5
[56] SwimBack Speed: 2.5
[56] Spline Time: 36399
[56] [0] Spline Waypoint: X: 1316.7345 Y: 583.6045 Z: 119.371
[56] [1] Spline Waypoint: X: 1315.89 Y: 584.14 Z: 119.371
[56] [2] Spline Waypoint: X: 1321.6118 Y: 594.1388 Z: 119.37121
[56] [3] Spline Waypoint: X: 1276.1915 Y: 652.8641 Z: 119.37121
[56] [4] Spline Waypoint: X: 1285.5996 Y: 726.50616 Z: 119.37121
[56] [5] Spline Waypoint: X: 1344.325 Y: 771.9264 Z: 119.37121
[56] [6] Spline Waypoint: X: 1417.967 Y: 762.5183 Z: 119.37121
[56] [7] Spline Waypoint: X: 1463.3873 Y: 703.79297 Z: 119.37121
[56] [8] Spline Waypoint: X: 1453.9792 Y: 630.15094 Z: 119.37121
[56] [9] Spline Waypoint: X: 1395.2539 Y: 584.73065 Z: 119.37121
[56] [10] Spline Waypoint: X: 1321.6118 Y: 594.1388 Z: 119.37121
[56] [11] Spline Waypoint: X: 1276.1915 Y: 652.8641 Z: 119.37121

So you see, the path in the create object packet is quite messed up as it shuffles in the first vertice of enter_cycle into the path but also puts in the path elements which should be added when enter_cycle is removed so you get a completely messed up path so I think that is the issue. Basically using the enter_cycle spline length and duration values while the packet already sends out the path for the post-enter_cycle spline

[xvwyh]

So you see, the path in the create object packet is quite messed up as it shuffles in the first vertice of enter_cycle into the path but also puts in the path elements which should be added when enter_cycle is removed so you get a completely messed up path so I think that is the issue.

I... don't see any problem here...

[56] [0] Spline Waypoint: X: 1316.7345 Y: 583.6045 Z: 119.371 -- "fake" (+cos,+sin,0) point for curvature (never reached)
[56] [1] Spline Waypoint: X: 1315.89 Y: 584.14 Z: 119.371 -- initial position (unit starts here)
[56] [2] Spline Waypoint: X: 1321.6118 Y: 594.1388 Z: 119.37121 -- first path point
[56] [3] Spline Waypoint: X: 1276.1915 Y: 652.8641 Z: 119.37121
[56] [4] Spline Waypoint: X: 1285.5996 Y: 726.50616 Z: 119.37121
[56] [5] Spline Waypoint: X: 1344.325 Y: 771.9264 Z: 119.37121
[56] [6] Spline Waypoint: X: 1417.967 Y: 762.5183 Z: 119.37121
[56] [7] Spline Waypoint: X: 1463.3873 Y: 703.79297 Z: 119.37121
[56] [8] Spline Waypoint: X: 1453.9792 Y: 630.15094 Z: 119.37121
[56] [9] Spline Waypoint: X: 1395.2539 Y: 584.73065 Z: 119.37121 -- last path point
[56] [10] Spline Waypoint: X: 1321.6118 Y: 594.1388 Z: 119.37121 -- first path point to close the loop (motion ends here and the spline is recreated without entercycle point)
[56] [11] Spline Waypoint: X: 1276.1915 Y: 652.8641 Z: 119.37121 -- "fake" second path point for curvature (never reached)

Basically using the enter_cycle spline length and duration values

Now this is odd. Why is the duration in SMSG_MONSTER_MOVE not equal to the one in SMSG_UPDATE_OBJECT? Is this the same unit during the same path in both examples, or are you giving me examples of 2 different paths? The "waypoints" in SMSG_MONSTER_MOVE don't quite match up with spline points in SMSG_UPDATE_OBJECT, but it very well could be due to the compression that SMSG_MONSTER_MOVE employs (offset from the center of the line encoded in 16-bit integers), I just don't remember if it was only for linear paths or was it also used for catmull-rom as well...

while the packet already sends out the path for the post-enter_cycle spline

This is the correct behavior. SMSG_MONSTER_MOVE doesn't send any "fake" points, because it doesn't need to: the client already knows where the unit is located and which direction it is facing. The first "fake" point is calculated from the unit's current clientside facing, the loop-closing point and the last "fake" point are added automatically by the client if "cyclic" flag is present. Contrast this to seeing a unit for the first time (SMSG_UPDATE_OBJECT) when it's already half-way through the motion - the client has no idea where the unit was before it started moving, hence the entire spline is needed, "fake" points and all.

I see the path in SMSG_MONSTER_MOVE correctly: final point is

(MovementMonsterSpline) (MovementSpline) Points: X: 1321.6118 Y: 594.1388 Z: 119.37121

which is the one that closes the loop, and the intermediate points are

(MovementMonsterSpline) (MovementSpline) [0] Waypoints: X: 1321.501 Y: 593.8894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [1] Waypoints: X: 1276.251 Y: 652.6394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [2] Waypoints: X: 1285.751 Y: 726.3894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [3] Waypoints: X: 1344.251 Y: 771.8894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [4] Waypoints: X: 1417.751 Y: 762.3894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [5] Waypoints: X: 1463.251 Y: 703.6394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [6] Waypoints: X: 1453.751 Y: 630.1394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [7] Waypoints: X: 1395.251 Y: 584.8894 Z: 119.37111

and you omitted the initial position.

The path the unit will take after receiving that SMSG_MONSTER_MOVE will NOT be

OMITTED INITIAL POSITION
(MovementMonsterSpline) (MovementSpline) Points: X: 1321.6118 Y: 594.1388 Z: 119.37121
(MovementMonsterSpline) (MovementSpline) [0] Waypoints: X: 1321.501 Y: 593.8894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [1] Waypoints: X: 1276.251 Y: 652.6394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [2] Waypoints: X: 1285.751 Y: 726.3894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [3] Waypoints: X: 1344.251 Y: 771.8894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [4] Waypoints: X: 1417.751 Y: 762.3894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [5] Waypoints: X: 1463.251 Y: 703.6394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [6] Waypoints: X: 1453.751 Y: 630.1394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [7] Waypoints: X: 1395.251 Y: 584.8894 Z: 119.37111

It will be

OMITTED INITIAL POSITION
(MovementMonsterSpline) (MovementSpline) [0] Waypoints: X: 1321.501 Y: 593.8894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [1] Waypoints: X: 1276.251 Y: 652.6394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [2] Waypoints: X: 1285.751 Y: 726.3894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [3] Waypoints: X: 1344.251 Y: 771.8894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [4] Waypoints: X: 1417.751 Y: 762.3894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [5] Waypoints: X: 1463.251 Y: 703.6394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [6] Waypoints: X: 1453.751 Y: 630.1394 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) [7] Waypoints: X: 1395.251 Y: 584.8894 Z: 119.37111
(MovementMonsterSpline) (MovementSpline) Points: X: 1321.6118 Y: 594.1388 Z: 119.37121

[Ovahlord]

Both packet parts come from the same unit. What you see there is the initial monster move packet and the update object packet when moving out of range and back in. If it wouldn't be the same spline, enter_cycle would have been gone by then. That field 'spline time' is btw not the duration, but the passed time. Got you a little video about the issue:

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

[killerwife]

So basically on sending SMSG_OBJECT_UPDATE create packet we need to send the spline in a different way.

[Shauren]

Problem here is that after first cycle, spline path is modified serverside and no longer holds these original points (MONSTER_MOVE is never sent again but UPDATE_OBJECT is)

[killerwife]

Here is an example of a spline I am working with:
[5] [0] Spline Waypoint: X: -1652.127 Y: 5443.76 Z: -36.10043 [5] [1] Spline Waypoint: X: -1651.403 Y: 5448.297 Z: -36.10043 [5] [2] Spline Waypoint: X: -1653.394 Y: 5452.388 Z: -36.10043 [5] [3] Spline Waypoint: X: -1658.315 Y: 5452.66 Z: -36.10043 [5] [4] Spline Waypoint: X: -1661.395 Y: 5448.715 Z: -36.10043 [5] [5] Spline Waypoint: X: -1659.505 Y: 5443.885 Z: -36.10043 [5] [6] Spline Waypoint: X: -1656.319 Y: 5442.147 Z: -36.10043 [5] [7] Spline Waypoint: X: -1652.127 Y: 5443.76 Z: -36.10043 [5] [8] Spline Waypoint: X: -1651.403 Y: 5448.297 Z: -36.10043 [5] [9] Spline Waypoint: X: -1653.394 Y: 5452.388 Z: -36.10043

This only ever appears as create object spline. The cycle in object update has those extra points at the end. So on sending object update we need to build output.

[xvwyh]

@Ovahlord, are you absolutely certain that isn't caused by grids not updating when there isn't a player around? It looks like these birds have a large AOI, which causes them to be visible from further away, but splines aren't updating on the server if the grids aren't, but on the client they are updating if the unit is visible.

@killerwife, this is fine. This is how it's supposed to be. This is how the spline is internally represented on the server and the client, and in SMSG_UPDATE_OBJECT the server sends the complete spline to the client. In SMSG_MONSTER_MOVE the server just sends enough info for the client to construct the spline on its own. Waypoint path and spline are different things.

[5] [0] Spline Waypoint: X: -1652.127 Y: 5443.76 Z: -36.10043 - this is an extra point needed for catmull-rom spline to work
[5] [1] Spline Waypoint: X: -1651.403 Y: 5448.297 Z: -36.10043 --\
[5] [2] Spline Waypoint: X: -1653.394 Y: 5452.388 Z: -36.10043   |
[5] [3] Spline Waypoint: X: -1658.315 Y: 5452.66 Z: -36.10043    |
[5] [4] Spline Waypoint: X: -1661.395 Y: 5448.715 Z: -36.10043   |-- this is the actual path as stored in the server database
[5] [5] Spline Waypoint: X: -1659.505 Y: 5443.885 Z: -36.10043   |
[5] [6] Spline Waypoint: X: -1656.319 Y: 5442.147 Z: -36.10043   |
[5] [7] Spline Waypoint: X: -1652.127 Y: 5443.76 Z: -36.10043  --/
[5] [8] Spline Waypoint: X: -1651.403 Y: 5448.297 Z: -36.10043 - this is an extra point needed for cyclic spline to work
[5] [9] Spline Waypoint: X: -1653.394 Y: 5452.388 Z: -36.10043 - this is an extra point needed for catmull-rom spline to work

[xvwyh]

Try one of the following and see if you can replicate the bug:
a) Set me->setActive(true); for that bird
b) Put another client close to the bird to keep the grids updating

Or just create some way to see where exactly the bird is without having to fly closer to it. Make it spawn a copies of itself near its position (higher or lower, so they aren't clipping into each other) continuously. And then you'll be able to see if the bird just stopped in place on the server's side, while your client continues to think it's moving.

[Ovahlord]

Yeah, looks like it's the grid unloading that causes it. Is there any smart way to mitigate this aside from hacking the units and keep them active?

[xvwyh]

I doubt it's "unloading", the grid shouldn't unload if the player is missing from it for one seconds, it's gonna degrade performance more than improve it. It's simply grids not updating. Grids are only updated around players and active objects.

The best way to handle this, I would imagine, would be to keep track of when the unit was last updated, and then, when it gets updated again, instead of passing the diff from World::Update to all the maps and units in them, have it be calculated as the difference between the current time and the time the unit was last updated.

This will mean that each unit will have its own diff in scripts that may be different from others on that map. And, of course, there might be issues in various scripts and other places that weren't designed to have diffs be unusually high, aura updates in particular. Imagine if the last player leaves the grid, and another one comes close to that grid an hour later, that would mean that the diff will be 3600000. Periodic auras will tick a shitload of times, cyclic splines will get looped a shitload of times, god knows what will happen in scripts, etc. And what if that grid wasn't visited for 2 weeks? Then the diff would be approaching the 32bit integer limit, which can cause underflows in code like timer -= diff;.

I suppose you could limit the impact by only using that "true diff" for movement code, but even then some optimizations will have to be made. In the case of a truly cyclic motion (i.e. already without enter_cycle flag) you could modulo the diff by spline duration to avoid having to repeat the loop over and over until the diff is exhaused.

But still, some major changes need to be made to how the maps are updates to fully support extended AOI. Otherwise you still run into ugly little issues like timed auras being visually stuck at 0s on the client that's observing a creature from afar (grids not updated -> auras not updated -> auras will never expire).

But slapping setActive(true) on every creature with cyclic splines and/or extended visibility is the quick and dirty solution, and the one I personally opted for (disclaimer: my setActive is a bit more complicated than a binary toggle on/off), but I wasn't stressed for server resources.

[Ovahlord]

So time for another round of questions. I'm currently working on a db side cyclic spline system and I'm pretty much done with the system itself, however, uppon packet comparison I noticed that the 1st and the last spline point in the core packets are not identical with the ones of the sniff. Lemme give you some data: https://paste2.org/NvVgeML1

What I do is building a spline with the first spline point being the mover's position (spline is normalizing that one as starting point uppon launch) and then I just feed the spline waypoints and skip the first and last two spline waypoints to reflect what you told me about the create object part. Did I misunderstand a part or how it comes that I end up with two misplaced spline points?

    bool const flying = creature->IsGravityDisabled() || creature->IsInWater(); // Swimming units also use flying and uncompressed splines
    Movement::MoveSplineInit init(creature);
    Movement::PointsArray path;

    path.reserve(_path->Waypoints.size() + 1);
    path.push_back(PositionToVector3(creature->GetPosition()));
    std::transform(_path->Waypoints.begin(), _path->Waypoints.end(), std::back_inserter(path), [](Position const& point)
    {
        return G3D::Vector3(point.GetPositionX(), point.GetPositionY(), point.GetPositionZ());
    });

    if (flying || *_enforceFlight)
    {
        init.SetFly();
        init.SetUncompressed();
    }

    if (_velocity.is_initialized())
        init.SetVelocity(*_velocity);

    if (_enforceWalk.is_initialized())
        init.SetWalk(*_enforceWalk);

    init.MovebyPath(path);
    init.SetSmooth();
    init.SetCyclic();
    init.Launch();
}

[xvwyh]

The code from your comment is correct. Put the starting point first, then all the points from the loop.

But you seem to be comparing non-enter_cycle spline from sniff with an enter_cycle spline from replica. [sniff] definitely looks like a non-enter_cycle spline, because its last points mirror first points, it doesn't have that extra one in the beginning. But in your [core replica] you do have the initial position in the spline. So figure out first if you're comparing the same splines.

Then. You missed the first point in the path. The X: 147.382 Y: 1427.51 Z: -5.42692 one. Yes, it's the initial position of your creature, but you've excluded it from the database path. Hence the creature won't be flying through it on every subsequent loop. It will START from it, on the enter_cycle spline, but once it's done - the point will be gone, and on each subsequent loop the creature will swim from
130.984, 1374.11, -4.15483
directly to
148.767, 1473.24, -4.83622
bypassing the 147.382, 1427.51, -5.42692 point entirely.

So you need to add it SOMEWHERE. Either in the beginning (will make for an awkward beginning of the first loop, if your creature is already in that position) or at the end of the path. Going by the [sniff], it must be in the beginning. And because your [sniff] spline is already a subsequent loop, you have no data about where the shark spawn point was. If you had managed to catch the shark on it's first loop right as it spawned, you would've sniffed the enter_cycle spline with the entry point intact, and that would be its spawn position.

[Ovahlord]

Allright, thanks for your input on that one. This makes all alot more sense now. Gotta watch out for a respawn packet then and check it again with a entercycle spline block

[xvwyh]

Hell, you can even extrapolate the shark's spawn facing by calculating the angle between the first fake point and the first real point, since the fake point is generated from creature's initial facing (it's generated in the backwards direction, to the distance equal to the distance between the first two real points, lerp(waypoint[0], waypoint[1], -1.0f)).

[xvwyh]

And as far as DB side is concerned, I just added a new `move_type` (`move_flag`?) to `waypoint_data` that would start this true spline movement. Several, actually, for different speeds (walk/run), and different movement types (continuous linear on the ground, smooth on the ground, swimming, flying). Took some effort to allow creatures to resume the path if it was interrupted mid-way, and also to make MovementInform work with it, since now it had to check the progress along the path to know for which points does it need to trigger inform on and in which order, and in general the code of WaypointMovementGenerator now looks like 2 giant if branches, but at least I can effortlessly assign waypoint_data paths to creatures in creature_addon. It's useful even for linear movement, because with this tech the entire path is sent to the client at once, so creatures won't stop clientside at every waypoint waiting for the next movement packet from the server.

I can share the code, but it's not exactly pretty, as I was trying to retrofit it to an existing system which really, let's be honest, should be scrapped altogether in favor of this. :D Also it doesn't support pausing via `delay` column, but it can be done.

[Ovahlord]

Well, I went for a simple solution for now. https://github.com/The-Cataclysm-Preservation-Project/TrinityCore/commit/27c1fd6aee27bc44f19f34f1af5e42a82d87234c However, it does not support specific velocity values yet since there are many cyclic splines that move with different speeds than the unit's fields indicate but I think I'll add a velocity field to waypoint_data anyways since it's useful for all movement types

[SilverIce]

@Ovahlord Nice to see that cyclic splines will be more widely used. Finally