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.

Sign up for GitHub

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

Jump to bottom

Synchronized building doesn't work correctly #2730

Closed
dktapps opened this issue Feb 2, 2019 · 3 comments
Closed

Synchronized building doesn't work correctly #2730

dktapps opened this issue Feb 2, 2019 · 3 comments
Labels
Category: Gameplay Related to Minecraft gameplay experience Resolution: Fixed

Comments

@dktapps
Copy link
Member

dktapps commented Feb 2, 2019

Issue description

In recent versions of Bedrock the devs added a really nice (really!) feature which I refer to as "synchronized building". This makes block placement (and breaking in creative) speed synchronized with walking speed.

Sadly, this feature has some issues in PocketMine-MP due to how the movement processing system currently works. When the player is walking backwards placing blocks, sometimes the block will not be placed because the player's current position is inside the block. On the client side, they are not inside the block, and the bug is happening because the server only processes movements once per tick, but processes block place/break immediately.

Steps to reproduce the issue

  1. Walk backwards placing blocks.
  2. Notice that if you get too close to the target block the blocks you're placing will glitch and disappear.

OS and versions

  • PocketMine-MP: .3.5.9
  • PHP: 7.2.14
  • Game version: Win10
@dktapps dktapps added Status: Reproduced Bug has been reproduced, but cause has not yet been identified Category: Gameplay Related to Minecraft gameplay experience labels Feb 2, 2019
@dktapps
Copy link
Member Author

dktapps commented Sep 18, 2019

related to #1215

@dktapps dktapps mentioned this issue Oct 27, 2019
Merged
@phoshp phoshp mentioned this issue Jan 16, 2020
Closed
dktapps added a commit that referenced this issue May 31, 2020
@dktapps
Player: remove move buffering, implement simple rate limited movement… (
485f573 
#3167)

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.
@dktapps
Copy link
Member Author

dktapps commented Jun 4, 2020

485f573 improves on this but doesn't completely fix it. It'll be necessary to handle position updates from the transaction packet itself to completely resolve this.

@dktapps dktapps mentioned this issue Sep 8, 2020
Closed
@dktapps dktapps added Resolution: Fixed and removed Status: Reproduced Bug has been reproduced, but cause has not yet been identified labels Jan 27, 2021
@dktapps
Copy link
Member Author

dktapps commented Jan 27, 2021

For posterity: This was caused by a bug in the handling for InventoryTransactionPacket, in a hack designed to reduce the overhead caused by spammy right-clicks. It did not account for player movements (or aiming at the same position on a different block). The movement changes did not have any effect on the problem (placebo).

Endermanbugzjfc pushed a commit to Endermanbugzjfc/PocketMine-MP that referenced this issue Feb 3, 2021
@dktapps @Endermanbugzjfc
Player: fix bridging, towering and various other fast building bugs
a833fa1 
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 pmmp#2730
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Category: Gameplay Related to Minecraft gameplay experience Resolution: Fixed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@dktapps