Skip to content
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

"Speed hack" potential in MoveManager #368

Open
Valcle opened this issue Oct 22, 2020 · 2 comments
Open

"Speed hack" potential in MoveManager #368

Valcle opened this issue Oct 22, 2020 · 2 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@Valcle
Copy link

Valcle commented Oct 22, 2020

First, notice this line:

stream->writeInt(py, 6);

It writes 6 bits for the "py" field, so the value ends up being anywhere [0 to 64).

Now look at this line:

y = (py - 16) / F32(16);

So, if the value is 63 here, the y field will be much higher than 1, and it will end up being passed to the respective ::processTick(...) eventually and cause 1. players to move much faster and 2. vehicles to accelerate much faster, maybe other things. This was the case in Blockland.

This doesn't happen i.e. when you do $mvForwardAction = 3 or something, because the range for these members on the move struct are clamped on the client side!

py = clampRangeClamp(y);

and

For a fix, changing the bits read to 5 probably isn't the right solution, since ideally the range should be [0 to 32], not [0 to 32). So, throw in a call to ::clamp(...) right after the move is unclamped on the server end, or simply write in some if-thens to limit the range of x/y/z after unclamping (or px/py/pz before).

(note I haven't tested if this bug applies to this version of the engine, I'm simply reporting from my experience with Blockland which is a TGE 1.5.1 game)

@lukaspj lukaspj added bug Something isn't working good first issue Good for newcomers labels Dec 12, 2020
@chaigler
Copy link
Contributor

chaigler commented Dec 14, 2020

Interesting!

Clamping the values on the server seems like an easy solution but I'm wondering if this entire bit of MoveManger code is still sensible. It's jumping through hoops (and introducing slight inaccuracies to the move state) just to shave a few bits off each packet. That definitely made sense in 2001 but how about now?

On the other end, this old post makes it sound like we could go even further with the optimization and cut another 3 bits/packet with some more clever float packing.

@WageCrusader
Copy link

This does not seem to affect Torque3D MIT, please list some steps that could be taken to reproduce this behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants