Core/Movement: Spline code cleanup

* Remove unneccessary reserve in MoveSplineInitArgs constructor - MoveTo always resizes it
* Remove unused function computeDuration
* Split cyclic spline reinitialization to separate function
* Don't expose TransportPathTransform outside of MoveSplineInit
* Ensure function argument names are the same in declaration and definition

8 files changed, 102 insertions, 110 deletions

diff --git a/src/server/game/Movement/MotionMaster.cpp b/src/server/game/Movement/MotionMaster.cpp
index 77491da63b7..2b43e54b4ee 100644
--- a/src/server/game/Movement/MotionMaster.cpp
+++ b/src/server/game/Movement/MotionMaster.cpp
@@ -941,6 +941,8 @@ void MotionMaster::MoveCirclePath(float x, float y, float z, float radius, bool
         Position const& pos = { x, y, z, 0.0f };
         float angle = pos.GetAbsoluteAngle(_owner->GetPositionX(), _owner->GetPositionY());
 
+        init.Path().reserve(stepCount + 1);
+
         // add the owner's current position as starting point as it gets removed after entering the cycle
         init.Path().emplace_back(_owner->GetPositionX(), _owner->GetPositionY(), _owner->GetPositionZ());
 
diff --git a/src/server/game/Movement/MovementGenerators/FlightPathMovementGenerator.cpp b/src/server/game/Movement/MovementGenerators/FlightPathMovementGenerator.cpp
index 0a95fad135e..14fa5ee3e5f 100644
--- a/src/server/game/Movement/MovementGenerators/FlightPathMovementGenerator.cpp
+++ b/src/server/game/Movement/MovementGenerators/FlightPathMovementGenerator.cpp
@@ -88,13 +88,12 @@ void FlightPathMovementGenerator::DoReset(Player* owner)
     }
 
     Movement::MoveSplineInit init(owner);
+    init.Path().reserve(end - currentNodeId + 1);
     // Providing a starting vertex since the taxi paths do not provide such
-    init.Path().push_back(G3D::Vector3(owner->GetPositionX(), owner->GetPositionY(), owner->GetPositionZ()));
+    init.Path().emplace_back(owner->GetPositionX(), owner->GetPositionY(), owner->GetPositionZ());
     for (uint32 i = currentNodeId; i != end; ++i)
-    {
-        G3D::Vector3 vertice(_path[i]->Loc.X, _path[i]->Loc.Y, _path[i]->Loc.Z);
-        init.Path().push_back(vertice);
-    }
+        init.Path().emplace_back(_path[i]->Loc.X, _path[i]->Loc.Y, _path[i]->Loc.Z);
+
     init.SetFirstPointId(GetCurrentNode());
     init.SetFly();
     init.SetSmooth();
diff --git a/src/server/game/Movement/Spline/MoveSpline.cpp b/src/server/game/Movement/Spline/MoveSpline.cpp
index 84d74b1ca2c..d71287acf28 100644
--- a/src/server/game/Movement/Spline/MoveSpline.cpp
+++ b/src/server/game/Movement/Spline/MoveSpline.cpp
@@ -114,11 +114,6 @@ void MoveSpline::computeFallElevation(int32 time_point, float& el) const
     el = std::max(z_now, final_z);
 }
 
-inline uint32 computeDuration(float length, float velocity)
-{
-    return SecToMS(length / velocity);
-}
-
 struct FallInitializer
 {
     FallInitializer(float _start_elevation) : start_elevation(_start_elevation) { }
@@ -147,16 +142,16 @@ struct CommonInitializer
 
 void MoveSpline::init_spline(MoveSplineInitArgs const& args)
 {
-    static SplineBase::EvaluationMode const modes[2] = { SplineBase::ModeLinear, SplineBase::ModeCatmullrom };
+    SplineBase::EvaluationMode mode = args.flags.isSmooth() ? SplineBase::ModeCatmullrom : SplineBase::ModeLinear;
     if (args.flags.Cyclic)
     {
         uint32 cyclic_point = 0;
         if (splineflags.Enter_Cycle)
             cyclic_point = 1;   // shouldn't be modified, came from client
-        spline.init_cyclic_spline(&args.path[0], args.path.size(), modes[args.flags.isSmooth()], cyclic_point, args.initialOrientation);
+        spline.init_cyclic_spline(args.path.data(), args.path.size(), mode, cyclic_point, args.initialOrientation);
     }
     else
-        spline.init_spline(&args.path[0], args.path.size(), modes[args.flags.isSmooth()], args.initialOrientation);
+        spline.init_spline(args.path.data(), args.path.size(), mode, args.initialOrientation);
 
     // init spline timestamps
     if (splineflags.Falling)
@@ -241,18 +236,15 @@ MoveSpline::MoveSpline() : m_Id(0), time_passed(0),
 bool MoveSplineInitArgs::Validate(Unit const* unit)
 {
 #define CHECK(exp, verbose) \
-    do if (!(exp))\
-    {\
-        if (unit)\
-            TC_LOG_ERROR("misc.movesplineinitargs", "MoveSplineInitArgs::Validate: expression '{}' failed for {}", #exp, verbose);\
-        else\
-            TC_LOG_ERROR("misc.movesplineinitargs", "MoveSplineInitArgs::Validate: expression '{}' failed for cyclic spline continuation", #exp); \
-        return false;\
-    } while (0)
+    if (!(exp)) return [&]{ \
+        TC_LOG_ERROR("misc.movesplineinitargs", "MoveSplineInitArgs::Validate: expression '{}' failed for {}", #exp, unit ? std::string_view(verbose) : "cyclic spline continuation"sv); \
+        return false; \
+    }()
+
     CHECK(path.size() > 1, unit->GetDebugInfo());
     CHECK(velocity >= 0.01f, unit->GetDebugInfo());
     CHECK(effect_start_time_percent >= 0.f && effect_start_time_percent <= 1.f, unit->GetDebugInfo());
-    CHECK(_checkPathLengths(), unit->GetGUID());
+    CHECK(_checkPathLengths(), unit->GetGUID().ToString());
     if (spellEffectExtra)
     {
         CHECK(!spellEffectExtra->ProgressCurveId || sCurveStore.LookupEntry(spellEffectExtra->ProgressCurveId), unit->GetDebugInfo());
@@ -294,12 +286,11 @@ bool MoveSplineInitArgs::_checkPathLengths()
     return true;
 }
 
-MoveSplineInitArgs::MoveSplineInitArgs(size_t path_capacity /*= 16*/) : path_Idx_offset(0), velocity(0.f),
+MoveSplineInitArgs::MoveSplineInitArgs() : path_Idx_offset(0), velocity(0.f),
 parabolic_amplitude(0.f), vertical_acceleration(0.0f), effect_start_time_percent(0.f), effect_start_time(0ms),
 splineId(0), initialOrientation(0.f),
 walk(false), HasVelocity(false), TransformForTransport(true)
 {
-    path.reserve(path_capacity);
 }
 
 MoveSplineInitArgs::MoveSplineInitArgs(MoveSplineInitArgs&& args) noexcept = default;
@@ -343,34 +334,7 @@ MoveSpline::UpdateResult MoveSpline::_updateState(int32& ms_time_diff)
                 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);
-                    }
+                    reinit_spline_for_next_cycle();
                 }
             }
             else
@@ -385,24 +349,52 @@ MoveSpline::UpdateResult MoveSpline::_updateState(int32& ms_time_diff)
     return result;
 }
 
+void MoveSpline::reinit_spline_for_next_cycle()
+{
+    MoveSplineInitArgs args;
+    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;
+    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);
+    }
+}
+
 std::string MoveSpline::ToString() const
 {
     std::stringstream str;
-    str << "MoveSpline" << std::endl;
-    str << "spline Id: " << GetId() << std::endl;
-    str << "flags: " << splineflags.ToString() << std::endl;
+    str << "MoveSpline\n";
+    str << "spline Id: " << GetId() << '\n';
+    str << "flags: " << splineflags.ToString() << '\n';
     if (facing.type == MONSTER_MOVE_FACING_ANGLE)
-        str << "facing  angle: " << facing.angle << std::endl;
+        str << "facing  angle: " << facing.angle << '\n';
     else if (facing.type == MONSTER_MOVE_FACING_TARGET)
-        str << "facing target: " << facing.target.ToString() << std::endl;
+        str << "facing target: " << facing.target.ToString() << '\n';
     else if (facing.type == MONSTER_MOVE_FACING_SPOT)
-        str << "facing  point: " << facing.f.x << " " << facing.f.y << " " << facing.f.z << std::endl;
-    str << "time passed: " << time_passed << std::endl;
-    str << "total  time: " << Duration() << std::endl;
-    str << "spline point Id: " << point_Idx << std::endl;
-    str << "path  point  Id: " << currentPathIdx() << std::endl;
+        str << "facing  point: " << facing.f.x << " " << facing.f.y << " " << facing.f.z << '\n';
+    str << "time passed: " << time_passed << '\n';
+    str << "total  time: " << Duration() << '\n';
+    str << "spline point Id: " << point_Idx << '\n';
+    str << "path  point  Id: " << currentPathIdx() << '\n';
     str << spline.ToString();
-    return str.str();
+    return std::move(str).str();
 }
 
 void MoveSpline::_Finalize()
diff --git a/src/server/game/Movement/Spline/MoveSpline.h b/src/server/game/Movement/Spline/MoveSpline.h
index df68e6c2369..45b0609378d 100644
--- a/src/server/game/Movement/Spline/MoveSpline.h
+++ b/src/server/game/Movement/Spline/MoveSpline.h
@@ -95,6 +95,7 @@ namespace Movement
         void computeFallElevation(int32 time_point, float& el) const;
 
         UpdateResult _updateState(int32& ms_time_diff);
+        void reinit_spline_for_next_cycle();
         int32 next_timestamp() const { return spline.length(point_Idx + 1); }
         int32 segment_time_elapsed() const { return next_timestamp() - time_passed; }
 
diff --git a/src/server/game/Movement/Spline/MoveSplineInit.cpp b/src/server/game/Movement/Spline/MoveSplineInit.cpp
index ffd21ad8942..0396cf4f0ef 100644
--- a/src/server/game/Movement/Spline/MoveSplineInit.cpp
+++ b/src/server/game/Movement/Spline/MoveSplineInit.cpp
@@ -17,14 +17,41 @@
 
 #include "MoveSplineInit.h"
 #include "Creature.h"
-#include "MovementPackets.h"
 #include "MoveSpline.h"
+#include "MovementPackets.h"
 #include "PathGenerator.h"
-#include "Transport.h"
 #include "Unit.h"
+#include "VehicleDefines.h"
 
 namespace Movement
 {
+    // Transforms coordinates from global to transport offsets
+    class TransportPathTransform
+    {
+    public:
+        TransportPathTransform(Unit const* owner, bool transformForTransport)
+            : _transport(transformForTransport ? owner->GetDirectTransport() : nullptr) { }
+
+        Vector3 operator()(Vector3 input) const
+        {
+            if (_transport)
+                _transport->CalculatePassengerOffset(input.x, input.y, input.z);
+
+            return input;
+        }
+
+        float operator()(float input) const
+        {
+            if (_transport)
+                input -= _transport->GetTransportOrientation();
+
+            return input;
+        }
+
+    private:
+        TransportBase* _transport;
+    };
+
     UnitMoveType SelectSpeedType(uint32 moveFlags)
     {
         if (moveFlags & MOVEMENTFLAG_FLYING)
@@ -234,23 +261,16 @@ namespace Movement
 
     void MoveSplineInit::SetFacing(float angle)
     {
-        if (args.TransformForTransport)
-        {
-            if (Unit* vehicle = unit->GetVehicleBase())
-                angle -= vehicle->GetOrientation();
-            else if (TransportBase* transport = unit->GetTransport())
-                angle -= transport->GetTransportOrientation();
-        }
-
-        args.facing.angle = G3D::wrap(angle, 0.f, (float)G3D::twoPi());
+        TransportPathTransform transform(unit, args.TransformForTransport);
+        args.facing.angle = Position::NormalizeOrientation(transform(angle));
         args.facing.type = MONSTER_MOVE_FACING_ANGLE;
     }
 
-    void MoveSplineInit::MovebyPath(PointsArray const& controls, int32 path_offset)
+    void MoveSplineInit::MovebyPath(PointsArray const& path, int32 pointId)
     {
-        args.path_Idx_offset = path_offset;
-        args.path.reserve(controls.size());
-        std::transform(controls.begin(), controls.end(), std::back_inserter(args.path), TransportPathTransform(unit, args.TransformForTransport));
+        args.path_Idx_offset = pointId;
+        args.path.resize(path.size());
+        std::ranges::transform(path, args.path.begin(), TransportPathTransform(unit, args.TransformForTransport));
     }
 
     void MoveSplineInit::MoveTo(float x, float y, float z, bool generatePath, bool forceDestination)
@@ -283,15 +303,6 @@ namespace Movement
         args.flags.FallingSlow = unit->HasUnitMovementFlag(MOVEMENTFLAG_FALLING_SLOW);
     }
 
-    Vector3 TransportPathTransform::operator()(Vector3 input)
-    {
-        if (_transformForTransport)
-            if (TransportBase* transport = _owner->GetDirectTransport())
-                transport->CalculatePassengerOffset(input.x, input.y, input.z);
-
-        return input;
-    }
-
     void MoveSplineInitFacingVisitor::operator()(Position const& point) const
     {
         _init.SetFacing(point.GetPositionX(), point.GetPositionY(), point.GetPositionZ());
diff --git a/src/server/game/Movement/Spline/MoveSplineInit.h b/src/server/game/Movement/Spline/MoveSplineInit.h
index 5b1da802a5e..cea5adc4b83 100644
--- a/src/server/game/Movement/Spline/MoveSplineInit.h
+++ b/src/server/game/Movement/Spline/MoveSplineInit.h
@@ -29,20 +29,6 @@ enum class AnimTier : uint8;
 
 namespace Movement
 {
-
-    // Transforms coordinates from global to transport offsets
-    class TC_GAME_API TransportPathTransform
-    {
-    public:
-        TransportPathTransform(Unit* owner, bool transformForTransport)
-            : _owner(owner), _transformForTransport(transformForTransport) { }
-        Vector3 operator()(Vector3 input);
-
-    private:
-        Unit* _owner;
-        bool _transformForTransport;
-    };
-
     /*  Initializes and launches spline movement
      */
     class TC_GAME_API MoveSplineInit
@@ -77,7 +63,7 @@ namespace Movement
          * @param start_time - delay between movement starting time and beginning to move by parabolic trajectory
          * can't be combined with final animation
          */
-        void SetParabolicVerticalAcceleration(float vertical_acceleration, float time_shift);
+        void SetParabolicVerticalAcceleration(float vertical_acceleration, float start_time);
         /* Plays animation after movement done
          * can't be combined with parabolic movement
          */
@@ -88,7 +74,7 @@ namespace Movement
          * you can have only one final facing: previous will be overriden
          */
         void SetFacing(float angle);
-        void SetFacing(Vector3 const& point);
+        void SetFacing(Vector3 const& spot);
         void SetFacing(float x, float y, float z);
         void SetFacing(Unit const* target);
 
@@ -196,17 +182,17 @@ namespace Movement
     inline void MoveSplineInit::SetSteering() { args.flags.Steering = true; }
     inline void MoveSplineInit::SetUnlimitedSpeed() { args.flags.UnlimitedSpeed = true; }
 
-    inline void MoveSplineInit::SetParabolic(float amplitude, float time_shift)
+    inline void MoveSplineInit::SetParabolic(float amplitude, float start_time)
     {
-        args.effect_start_time_percent = time_shift;
+        args.effect_start_time_percent = start_time;
         args.parabolic_amplitude = amplitude;
         args.vertical_acceleration = 0.0f;
         args.flags.Parabolic = true;
     }
 
-    inline void MoveSplineInit::SetParabolicVerticalAcceleration(float vertical_acceleration, float time_shift)
+    inline void MoveSplineInit::SetParabolicVerticalAcceleration(float vertical_acceleration, float start_time)
     {
-        args.effect_start_time_percent = time_shift;
+        args.effect_start_time_percent = start_time;
         args.parabolic_amplitude = 0.0f;
         args.vertical_acceleration = vertical_acceleration;
         args.flags.Parabolic = true;
@@ -230,7 +216,7 @@ namespace Movement
         args.spellEffectExtra = spellEffectExtraData;
     }
 
-    struct TC_GAME_API MoveSplineInitFacingVisitor
+    struct MoveSplineInitFacingVisitor
     {
         explicit MoveSplineInitFacingVisitor(MoveSplineInit& init) : _init(init) { }
 
@@ -242,4 +228,5 @@ namespace Movement
         MoveSplineInit& _init;
     };
 }
+
 #endif // TRINITYSERVER_MOVESPLINEINIT_H
diff --git a/src/server/game/Movement/Spline/MoveSplineInitArgs.h b/src/server/game/Movement/Spline/MoveSplineInitArgs.h
index b7716b3cd96..fe7bb527802 100644
--- a/src/server/game/Movement/Spline/MoveSplineInitArgs.h
+++ b/src/server/game/Movement/Spline/MoveSplineInitArgs.h
@@ -62,7 +62,7 @@ namespace Movement
 
     struct MoveSplineInitArgs
     {
-        explicit MoveSplineInitArgs(size_t path_capacity = 16);
+        explicit MoveSplineInitArgs();
         MoveSplineInitArgs(MoveSplineInitArgs&& args) noexcept;
         ~MoveSplineInitArgs();
 
diff --git a/src/server/game/Movement/Spline/Spline.cpp b/src/server/game/Movement/Spline/Spline.cpp
index 2fb8cca636d..1e957428b5b 100644
--- a/src/server/game/Movement/Spline/Spline.cpp
+++ b/src/server/game/Movement/Spline/Spline.cpp
@@ -306,7 +306,7 @@ std::string SplineBase::ToString() const
     for (index_type i = 0; i < count; ++i)
         str << "point " << i << " : " << points[i].toString() << std::endl;
 
-    return str.str();
+    return std::move(str).str();
 }
 
 }