-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Dev] WIP Catmullrom waypoint movement #13467
Comments
C++ changes fit more in a pull request so they can be commented and discussed. For example
is more clear, or you could even just initialize "i" to i_currentNode and avoid any other "+ i_currentNode" Storing the value of
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
--- 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().
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. |
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.
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:
|
@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:
It would be nice if @Shauren or @Subv had the time to say something about that or even fix it. |
|
Well, that is really embarrassing even for me... 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 : Cyclic pathing : (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. @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. |
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 |
…yCore#13467 (TrinityCore#18020)" (TrinityCore#18888)" This reverts commit a3c6880.
Well xD RIP my money, it was fun. :P |
fixes TrinityCore#13467 WIP? - don't know whats missing - need feedback
…nityCore#18020) Implement smooth movement for all waypoint pathing and escortai (cherry picked from commit 28050f3)
I started to play around a little with the core and made the following change to WaypointMovementGenerator.cpp
This made all the flying NPCs that have waypoint data use catmullrom movement. But there are some things that certainly need improvements which I can't take care of because I have really little knowledge when it gets to core programming:
At each WP the remaining path is rebuild which is unnecessary (memory consumption) and not blizzlike.
At the last WP there is no next WP so no path is build and point movement is used instead of catmullrom.
Flying creatures that have inhabitType!=4 (i.e. 5 or 7) don't use catmullrom movement. I guess the core doesn't handle them correctly so they don't get their movementflags.
Swimming creatures don't use catmullrom movement.
Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.
The text was updated successfully, but these errors were encountered: