PocketMine-MP
PocketMine-MP copied to clipboard
Fixed unwanted sprinting and sneaking sync.
Introduction
This issue does not appear when the client latency to the server is low, but it does occur when the player latency to the server is high. The issue is that, the server forces the client to sync the movement speed/sneaking flag relative to the server. This is unnecessary since the client has the appropriate value/flag already set before.
The problem itself causes unpleasant gameplay and unnecessary changes in momentum towards any players with higher latency. This PR aims to fix the problem.
Changes
API changes
- Added
$doSync = true(By default, it will sync movement speed/sneaking flag) inLiving::setSprinting,Living::setSneaking,Player::toggleSprint, andPlayer::toggleSneakhowever is for internal use only.
Behavioural changes
It changes how toggling sprint and sneak when the client requested to, it does not change the behavior when the server wants to change the client's sprinting and/or sneaking flag (e.g.: food exhaustion). In other words, sync when necessary.
Backwards compatibility
The changes should be backwards compatible without any significant changes to the underlying behavior of sprinting and sneaking.
Tests
You will need this software to simulate lagging behaviour locally: https://github.com/jagt/clumsy
Hint: You need to watch the player's FOV
Before the proposed PR:
After:
Dumping this in the API isn't the right way to do it. It should be masked at the network layer similar to how transaction slot predictions are handled.
I personally have to agree with you here, placing this in the API is out-of-place and needed to be done in the network layer, unfortunately the way of synchronizing the attributes are coded into the function (The networkPropertiesDirty flag) and I couldn't find the right way to do this yet. We could try to reset the network properties flag based on the action flag before and if all the actions is successful, we set it based on the previous properties.
I will write a follow-up with the changes when I do have the time.
Alright, so now instead of dumping it in the API, a syncPlayerActions function is created to toggle those actions and check if synchronization is necessary. Tried to expose the networkPropertiesDirty for the network layer to access but I think it is for internal usage only.
I believe this needs more work with, as the current implementation is a bit hacky. I'm closing this now