Trinitycore: Clientside spline speed limit

[xvwyh]

Client forcibly limits spline movement speed to 50 yards/sec for flying and catullrom-spline-using units and to run_speed*4 - but not lower than 28 - for everybody else. This is important, because charge-like spells usually have higher speed than this, and since this restriction is missing on the server, position of the unit on server and client gets desynced: server moves the unit with full intended speed, but the client displays it moving slower.
Proof for this from IDA-decompiled CGUnit_C::OnMonsterMovePacket:
http://i.imgur.com/4BZloyu.png
(may be outdated on post-3.3.5 clients, I don't know)

Possible implementation:
MoveSplineInit::Launch

             args.velocity = unit->GetSpeed(SelectSpeedType(moveFlagsForSpeed));
         }

+        args.velocity = std::min(args.velocity, args.flags.catmullrom || args.flags.flying ? 50.0f : std::max(28.0f, unit->GetSpeed(MOVE_RUN) * 4.0f));

         if (!args.Validate(unit))
             return 0;

And just to please @TrinityCoreBot: 0f3156d324ff8134171bada01b40f5c23f5c43cc

[illfated]

Maybe you know this already, but in case you don't:
GitHub Flavored MarkDown can be used to highlight code syntax or diff changes.

Diff: (```diff on top of the block):

             args.velocity = unit->GetSpeed(SelectSpeedType(moveFlagsForSpeed));
         }

+        args.velocity = std::min(args.velocity, args.flags.catmullrom || args.flags.flying ? 50.0f : std::max(28.0f, unit->GetSpeed(MOVE_RUN) * 4.0f));

         if (!args.Validate(unit))
             return 0;

C++ (```cpp on top of the block):

             args.velocity = unit->GetSpeed(SelectSpeedType(moveFlagsForSpeed));
         }

+        args.velocity = std::min(args.velocity, args.flags.catmullrom || args.flags.flying ? 50.0f : std::max(28.0f, unit->GetSpeed(MOVE_RUN) * 4.0f));

         if (!args.Validate(unit))
             return 0;

[xvwyh]

I didn't, thanks.

[Kittnz]

In nosts core they fixed it like this: https://github.com/vmangos/core/blob/development/src/game/Movement/spline/MoveSplineInit.cpp#L159

[kelno]

Nost solution does make the movement actually match the velocity of the spell instead of reducing it, so that seems a better solution, unless Blizzard also did limit the velocity as proposed here.

[xvwyh]

I highly doubt blizzard would change unit's speed back and forth just so a charge can execute at its full velocity. If they wanted to do so, why would they even add the limit at all? Why complicate things?

[kelno]

I just find strange they set a speed that'll never get reached on a spell, but that wouldn't be the first non consistent thing they did, you're probably right.

[xvwyh]

Well for starters - it's not known, who's gonna be using the spell. Any creature in the game could potentially cast it, so blizzard's spell editor can't show a validation error to the designer that the speed they entered is too high - maybe the creature that's intended to use the spell does have a high base speed and so their speed times 4 will be higher than the spell's. And making designers keep such technical details in mind is probably outside of the realm of possibility :D.

[jackpoz]

https://github.com/TrinityCore/TrinityCore/commit/0dd3f0497dcb8c8dd8bed15916a8ca036b67673b

[jidevlpr]

@xvwyh indeed outdated post-3.3.5

Here are the limits for Cataclysm 4.3.4 Client, Parabolic and Falling are also limited to 50:
https://i.gyazo.com/ed1e92b52bb20147da389fa50cf829b5.png

[xvwyh]

Thank you! This actually answers one of the unanswered questions I had regarding another feature that I was going to post about next.

Actually, scratch that. This answers all the questions.