PocketMine-MP icon indicating copy to clipboard operation
PocketMine-MP copied to clipboard

Fixed unwanted sprinting and sneaking sync.

Open larryTheCoder opened this issue 3 years ago • 3 comments

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) in Living::setSprinting, Living::setSneaking, Player::toggleSprint, and Player::toggleSneak however 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:

larryTheCoder avatar Jul 08 '22 18:07 larryTheCoder

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.

dktapps avatar Jul 09 '22 15:07 dktapps

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.

larryTheCoder avatar Jul 10 '22 16:07 larryTheCoder

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.

larryTheCoder avatar Jul 10 '22 18:07 larryTheCoder

I believe this needs more work with, as the current implementation is a bit hacky. I'm closing this now

larryTheCoder avatar Aug 15 '22 10:08 larryTheCoder