this can fire while the player has the inventory window open, because it also gets sent when the player swaps their held itemstack for something new.
We already had a special-case for mouseover with entity ID 0, but since
this isn't just a zero problem, a more general fix suits better
(particularly since we might need to handle the 0 case anyway).
closes#4140closes#4141
If allowFlight was toggled by the server (e.g. due to gamemode change), a race could occur due to network latency where the client could try to enable flight, and then get kicked for cheating.
Since this can happen in legitimate, non-cheating cases, we can't make any assumptions about whether a player is cheating, so instead we just revert it, like we do with every other bad input.
This reverts commit cb06be615aa3780d4c83a947520fa55c0d908618.
we can't push this to stable because it would break plugins without any
way to know (no protocol or API change).
At most, this should have been wrapped into a protocol change.
this is not a complete changeset, but it's sufficient to get servers back online.
There are additional changes to PlayerAuthInputPacket which need to be reversed.
we check if an existing player is online with a matching XUID first; if there isn't, we don't bother loading the playerdata, since that other player couldn't have joined unless they had a match or were allowed to bypass.
clickPos is relative to the base block position, so if you keep aiming at the same spot on the block and jump, it thinks you're still spamming.
closes#2730
every time a chunk passed through Level->generateChunkCallback(), it fired onChunkChanged() for chunk listeners, which in turn caused players to rerun chunk orders even though the target chunk had not been sent yet anyway.
This commit fixes the 5+ years-old bug with the movement anti-cheat that everyone has complained about: sprinting on stairs causes rubberbanding.
This commit addresses this problem at long last, along with a handful of precursor commits that were necessary to fix this problem:
- dac76f0e0f14d9beae3e55491ab9f61353db1b68
- 89fe8f7f10716fed6193646984a7e2d28f6f6ce5
- 2d77b1e3649d11a6072fbd1dbf88574b6ebf1af3
Additionally, these changes now allow the anti-cheat to be accurate to at least 0.001 of a block, perhaps even better. I didn't commit a change to the threshold here, but it was instrumental to pinning down the exact nature of these bugs.
This closes#1475, at long last.
fixes#3655, fixes#3241
instead of guessing where the end of the transaction is, we attempt validation after every piece of the transaction, with the assumption being that a crafting transaction will not validate until it's complete.
phpstan 0.12.26 starts reporting errors about the result of array_search() being given to some constructor or another because of the lack of key type specification.
while this is slightly less bandwidth efficient (1 in 92 datagrams not full vs 1 in 733), this is significantly less memory-hard.
I made this decision looking at the memory pressures that 1MB chunks exert - especially on RakNet. Client-side, these resource pack chunks all hang around in RakNet memory until the whole thing is received, and it's a lot more costly to receive 733 datagrams than it is to receive 92, especially since it's much more likely that some of the 733 will disappear along the way.
If, for example, the first couple of hundred KB split parts arrived out of 1MB, and then one of the parts got lost, all the already-received parts would hang around in memory not getting processed. With smaller chunks this is much less of a problem.
I explored taking the chunk size all the way down to 1KB to reduce the bandwidth waste caused by split packets (split headers), but this made resource pack downloading unbearably slow, so it wasn't acceptable.
Introduction
This PR is a second attempt at improving movement processing to fix#1215 , #2730 and more.
This is significantly less complex than the previous attempt #2646 -- it gets rid of the movement buffering system entirely and instead relies on a simple rate limit counter to restrict on-the-fly movement processing.
Movement is rate limited to a max average of 2 per tick. It allows up to 5 seconds' backlog to accommodate network lag. The rate limit counter is increased by 2 per tick and decreased once for every movement processed. This prevents movement processing being abused for denial of service attacks.
Changes
API changes
This PR, while obviously highly beneficial for most current users, poses some BC-breaking issues because of changes to the internal Player API.
Player->processMovement() (protected) has been removed. This is a BC concern for custom player classes which overrode it and called it as a parent. In addition, child implementations won't be called every tick any more, which could break some custom movement processing systems.
Player->newPosition (protected) has been removed. This internal field may have been accessed by custom movement implementations.
Player->isTeleporting (protected) has been removed. BC concern is same as previous point.
Player->getNextPosition() (public) has been @deprecated.
Added the following protected Player class members:
int $moveRateLimit
?Vector3 $forceMoveSync
handleMovement()
processMostRecentMovements()
revertMovement()
Behavioural changes
Player movement is now subject to less rubberbanding and has more reliable behaviour.
