C++ changes fit more in a pull request so they can be commented and discussed.

For example

i_currentNode + i < i_path->size()

is more clear, or you could even just initialize "i" to i_currentNode and avoid any other "+ i_currentNode"

Storing the value of

i_path->at(i_currentNode + i)

is better than writing the same code 3 times (otherwise if you need to change 1 place, you have to remember to do it 3 times)

About the issue with recalculating the path every time, you can just store the previously calculated path in the WayPointMovementGenerator class, you'll have to modify the code that calls MovementInform though, because as far as I remember it works by using _spline->Finished() which won't ever be true in this case except when the creature reaches the final waypoint.

@joschiwald thanks for the tips. The reason why I don't create a PR is simple. I don't know how to and haven't got the time to learn it at the moment. I just want to share what I found out so far because someone else with more experience might bring this to an end meanwhile.

@Subv I investigated a little more and am now convinced that the path isn't recalculated at every WP. It is only calculated when the creature spawns and at end of path which is fine imo.

I modified my piece of code from above and got the following improvements

• For smooth movement at the last WP I added the first WP after the loop. This might need a check for repeating flag though...
• Flying creatures with inhabitType=5 or 7 now also use catmullrom movement.
• Swimming creatures now use catmullrom movement, too.

--- a/src/server/game/Movement/MovementGenerators/WaypointMovementGenerator.cpp
+++ b/src/server/game/Movement/MovementGenerators/WaypointMovementGenerator.cpp
@@ -153,6 +153,17 @@ bool WaypointMovementGenerator<Creature>::StartMove(Creature* creature)
             trans->CalculatePassengerPosition(formationDest.x, formationDest.y, formationDest.z, &formationDest.orientation);
     }

+   if (creature->IsFlying() || creature->HasUnitMovementFlag(MOVEMENTFLAG_SPLINE_ENABLED & MOVEMENTFLAG_CAN_FLY) || creature->HasUnitMovementFlag(MOVEMENTFLAG_SWIMMING))
+   {
+       for (uint32 i = i_currentNode; i < i_path->size(); i++)
+       {
+           init.Path().push_back(G3D::Vector3(i_path->at(i)->x, i_path->at(i)->y, i_path->at(i)->z));
+       }
+       init.Path().push_back(G3D::Vector3(i_path->at(0)->x, i_path->at(0)->y, i_path->at(0)->z));
+        init.SetSmooth();
+       init.Launch();
+       return true;
+   }
     //! Do not use formationDest here, MoveTo requires transport offsets due to DisableTransportPathTransformations() call
     //! but formationDest contains global coordinates
     init.MoveTo(node->x, node->y, node->z);

I got one new issue with flying creatures with inhabitType=5 or 7. They sort of fall and jump through the air. I think this is a core issue because they don't have movementflag MOVEMENTFLAG_FLYING | MOVEMENTFLAG_DISABLE_GRAVITY but MOVEMENTFLAG_SPLINE_ENABLED & MOVEMENTFLAG_CAN_FLY when they actually moving in the air. This needs further research.

Use init.SetSmooth only for non-flying creatures - for flying you've got init.EnableFlying().

init.Path().push_back(G3D::Vector3(i_path->at(0)->x, i_path->at(0)->y, i_path->at(0)->z));
Use current creature position instead and add this point at the begin of the path.

To call Movement Inform, look at FlightPathGenerator::update and use this code in waypointgenerator code.

Nice work.

Would this type of path calculation be able to fix the taxi paths that fly inside Ironforge when they're supposed to go around it (eg: Stormwind to Menethil should not fly inside Ironforge).

I know taxipaths are loaded from the DBC but we should be able to recalculate them. I've tried in the past and only succeeded in either crashing the server or flying through the ground.

That's a grid problem @MrSmite if i recall correctly.
Ontopic: I guess this can be used for normal walking creatures too? Ill test tonight!

That's a grid problem @MrSmite if i recall correctly.

Nope, it's a waypoint problem. The taxipath that is loaded includes points that go inside IF to the flightmaster where you temporarily land and then take off again for the second half of the flight.

No, it would not be able to fix taxi paths, there is more going on there - i once wrote a hack for that (it sort of worked but was also crashing, never looked more at that)

Yeah i remember that XD

@akrom23 thanks for the suggestions. But I have some questions about them.

• I think you are speaking about init.SetFly() because there is no init.EnableFlying() at least on my core.
• I don't see the point in adding the current position at the beginning of the spline instead of adding the first WP of the path in the end to get a smooth movement at the end of the spline. Maybe you can explain that?
• Why would I want to call movementInform?

By using SetFly I fixed the strange flying behavior of creatures with inhabitType=5 or 7. Btw creature's movementflags are blizzlike after that change. They were wrong before compared to sniff from 3.3.5.

Unfortunately I found some flying creatures that are not respecting smooth movement at the end of the path when they start to repeat it but others do it like a charm. I don't have any idea about that right now.

@Pitcrawler:
I think you are speaking about init.SetFly() because there is no init.EnableFlying() at least on my core.
Yes, i meant SetFly

I don't see the point in adding the current position at the beginning of the spline instead of adding the first WP of the path in the end to get a smooth movement at the end of the spline. Maybe you can explain that?
Because in case of only 2 waypoint points creature's spline will have only 1 arg in path and you will have errors in console. Besides, without this, you will have some visual problems - spline always begins with current position. See, how init.Moveto is designed - first point is the current unit position -> the start from where spline is launched.

Why would I want to call movementInform?
See OnArrived code. In some nodes creature stops or do sth.

@Shauren Too bad. Maybe it will correct itself in 6.x :)

I have checked a number of sniffs and haven't found any spline that contains current position of the creature. So I will leave this as it is until somebody can give a proof of this. I think WP scripts are handled via OnArrived() which is called when a WP is reached. So I left this as it is as well.

But I changed the build process of the spline. Now the spline is always build for the whole path and the first WP is then taken from the current WP+1.

I also added the cyclic movement flag when the spline is meant to be repeated which is correct for all the waypoint NPCs. Maybe this check is not necessary.

I had to set speed because when such an NPC is aggroed and it returns back to its homeposition and continues the spline it will use walking speed otherwise.

--- a/src/server/game/Movement/MovementGenerators/WaypointMovementGenerator.cpp
+++ b/src/server/game/Movement/MovementGenerators/WaypointMovementGenerator.cpp
@@ -153,6 +153,21 @@ bool WaypointMovementGenerator<Creature>::StartMove(Creature* creature)
             trans->CalculatePassengerPosition(formationDest.x, formationDest.y, formationDest.z, &formationDest.orientation);
     }

+    if (creature->IsFlying() || creature->HasUnitMovementFlag(MOVEMENTFLAG_SPLINE_ENABLED | MOVEMENTFLAG_CAN_FLY) || creature->HasUnitMovementFlag(MOVEMENTFLAG_SWIMMING))
+    {
+        for (uint32 i = 0; i < i_path->size(); i++)
+        {
+            init.Path().push_back(G3D::Vector3(i_path->at(i)->x, i_path->at(i)->y, i_path->at(i)->z));
+        }
+        init.SetFirstPointId(i_currentNode + 1);
+        if (repeating)
+            init.SetCyclic();
+        init.SetFly();
+        init.SetVelocity(creature->GetSpeed(MOVE_RUN));
+        init.Launch();
+        return true;
+    }
+
     //! Do not use formationDest here, MoveTo requires transport offsets due to DisableTransportPathTransformations() call
     //! but formationDest contains global coordinates
     init.MoveTo(node->x, node->y, node->z);

I couldn't get rid of two minor things:

• When the creature reaches the last WP of the spline it sort of rotates about half a second or so and then starts a new round of the spline as suppossed. I can only describe the behavior in a very poor way. I'd be best to see it for yourself.
  I tracked this down to the function MoveSpline::UpdateResult MoveSpline::_updateState(int32& ms_time_diff). I think the handling of cyclic splines needs to be improved there. As far as I understand it the first WP of the spline is loaded when the last one is reached and at the next update the new cycle is started. Possibly this takes too long so the creature doesn't know where to go between two updates. But this can be completly wrong as I already said that I have only very poor core knowledge.
• After returning to its homeposition the creature also rotates about half a second before continuing the spline.

It would be nice if @Shauren or @Subv had the time to say something about that or even fix it.

creature->HasUnitMovementFlag(MOVEMENTFLAG_SPLINE_ENABLED & MOVEMENTFLAG_CAN_FLY) this is always false. (MOVEMENTFLAG_SPLINE_ENABLED & MOVEMENTFLAG_CAN_FLY = 0, HasUnitMovementFlag(0)... go figure)

Well, that is really embarrassing even for me...
But I added this check, which should have been with bitwise OR, because of those creatures with inhabitType=5 or 7. I changed it above.

The best solution to this would be when they get MOVEMENTFLAG_DISABLE_GRAVITY as soon as they are in the air. Currently they just get MOVEMENTFLAG_CAN_FLY. I think this not correct compared to sniffs.

MOVEMENTFLAG_DISABLE_GRAVITY and MOVEMENTFLAG_CAN_FLY are exclusive with each other, disable is set for inhabit = 4, fly for 4 | 1

Is anyone still into this?

any update on this ?

No one is working on it, hope someone picks it up.

Ok, let's continue on this one ...

Single pathing :
Example with 20 "real" waypoints:
1-2-3...18-19-20-[21]
[21] = 20 - describes that we're on the end of a single-run path.

Cyclic pathing :
Example with 20 "real" waypoints:
[1-2-3]-4-5-6-7....18-19-20-[21-22-23]
[21-22-23] = [1-2-3] - describes that we are on a cyclic path.

(Courtesy of Malcroms research on the issue).

Question: After a path is generated would it make sense to save it to the DB (since the path should technically never change)?

Isn't the client doing the smoothing? we have many catmullrom paths in waypoint_data the same as offi sends to client. We are just sending them a point at a time.

@MrSmite : no, the path is actually picked up FROM the database, core just need to know the criterias above.
Final point twice = Full endpoint
First three repeated as last three = cyclic
The client most probably also knows about this criteria, so it can interpolate correctly clientside.

@malcrom: The client does the smoothing based on standard Catmullrom definitions and us shipping the right info to it. Which we obviously don't per today.

@click Ah, ok. I thought the goal was to calculate a path for use with mmaps to replace the waypoints we already had.

https://www.bountysource.com/issues/5546590-dev-wip-catmullrom-waypoint-movement-15-bounty

Bountysource do something xD

i mailed them, still waiting for answer.

Fix reverted in a3c6880 so... reopen this

lul the bounty was awarded

ez money

Well xD RIP my money, it was fun. :P