Skip to content

Fix for "block lag" while placing blocks on higher latency clients #6803

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

Open
wants to merge 4 commits into
base: stable
Choose a base branch
from
+61 −13
Open

Fix for "block lag" while placing blocks on higher latency clients #6803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8707575
fix block stuttering while placing blocks
ethaniccc Sep 11, 2025
7f51c4c
World.php: account for multi-block placements being cancelled
ethaniccc Sep 12, 2025
83bb7bd
fix mismatching phpdoc tag
ethaniccc Sep 12, 2025
1805bcd
World.php: fix faulty logic
ethaniccc Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions src/network/mcpe/handler/InGamePacketHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,11 +498,6 @@ private function handleUseItemTransaction(UseItemTransactionData $data) : bool{
$blockPos = $data->getBlockPosition();
$vBlockPos = new Vector3($blockPos->getX(), $blockPos->getY(), $blockPos->getZ());
$this->player->interactBlock($vBlockPos, $data->getFace(), $clickPos);
//always sync this in case plugins caused a different result than the client expected
//we *could* try to enhance detection of plugin-altered behaviour, but this would require propagating
//more information up the stack. For now I think this is good enough.
//if only the client would tell us what blocks it thinks changed...
$this->syncBlocksNearby($vBlockPos, $data->getFace());
return true;
case UseItemTransactionData::ACTION_CLICK_AIR:
if($this->player->isUsingItem()){
Expand Down
13 changes: 13 additions & 0 deletions src/player/Player.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1938,12 +1938,25 @@ public function interactBlock(Vector3 $pos, int $face, Vector3 $clickOffset) : b
return true;
}
}else{
$this->syncBlocks([$pos, $pos->getSide($face)]);
$this->logger->debug("Cancelled interaction of block at $pos due to not currently being interactable");
}

return false;
}

/**
* Sync blocks sends block updates to the player at the block positions.
*
* @param Vector3[] $blocks
*/
public function syncBlocks(array $blocks) : void {
$blocks = array_filter($blocks, fn(Vector3 $block) => $block->distanceSquared($this->getLocation()) < 10000);
foreach ($this->getWorld()->createBlockUpdatePackets($blocks) as $packet) {
$this->getNetworkSession()->sendDataPacket($packet);
}
}

/**
* Attacks the given entity with the currently-held item.
* TODO: move this up the class hierarchy
Expand Down
56 changes: 48 additions & 8 deletions src/world/World.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2238,9 +2238,10 @@ private function destroyBlockInternal(Block $target, Item $item, ?Player $player
* @param bool $playSound Whether to play a block-place sound if the block was placed successfully.
* @param Item[] &$returnedItems Items to be added to the target's inventory (or dropped if the inventory is full)
*/
public function useItemOn(Vector3 $vector, Item &$item, int $face, ?Vector3 $clickVector = null, ?Player $player = null, bool $playSound = false, array &$returnedItems = []) : bool{
$blockClicked = $this->getBlock($vector);
public function useItemOn(Vector3 $clickedBlockPos, Item &$item, int $face, ?Vector3 $clickVector = null, ?Player $player = null, bool $playSound = false, array &$returnedItems = []) : bool{
Copy link
Member

@dktapps dktapps Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is BC breaking

$blockClicked = $this->getBlock($clickedBlockPos);
$blockReplace = $blockClicked->getSide($face);
$mainBlocks = [$clickedBlockPos, ($sideClickPos = $clickedBlockPos->getSide($face))];

if($clickVector === null){
$clickVector = new Vector3(0.0, 0.0, 0.0);
Expand All @@ -2254,15 +2255,18 @@ public function useItemOn(Vector3 $vector, Item &$item, int $face, ?Vector3 $cli

if(!$this->isInWorld($blockReplace->getPosition()->x, $blockReplace->getPosition()->y, $blockReplace->getPosition()->z)){
//TODO: build height limit messages for custom world heights and mcregion cap
$player?->syncBlocks($mainBlocks);
return false;
}
$chunkX = $blockReplace->getPosition()->getFloorX() >> Chunk::COORD_BIT_SIZE;
$chunkZ = $blockReplace->getPosition()->getFloorZ() >> Chunk::COORD_BIT_SIZE;
if(!$this->isChunkLoaded($chunkX, $chunkZ) || $this->isChunkLocked($chunkX, $chunkZ)){
$player?->syncBlocks($mainBlocks);
return false;
}

if($blockClicked->getTypeId() === BlockTypeIds::AIR){
$player?->syncBlocks($mainBlocks);
return false;
}

Expand All @@ -2279,6 +2283,18 @@ public function useItemOn(Vector3 $vector, Item &$item, int $face, ?Vector3 $cli
$ev->call();
if(!$ev->isCancelled()){
if($ev->useBlock() && $blockClicked->onInteract($item, $face, $clickVector, $player, $returnedItems)){
$aroundBlocks = [];
foreach ($clickedBlockPos->sidesArray() as $otherVector) {
if ($sideClickPos !== $otherVector) {
$aroundBlocks[] = $otherVector;
}
}
foreach ($sideClickPos->sidesArray() as $otherVector) {
if ($clickedBlockPos !== $otherVector) {
$aroundBlocks[] = $otherVector;
}
}
$player->syncBlocks($aroundBlocks);
Copy link
Member

@dktapps dktapps Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the block interaction was disabled, then the client would wrongly see a door being openable when it's actually cancelled on the server side.

Copy link
Author

@ethaniccc ethaniccc Sep 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should sync doors properly here, as it is also sending the blocks around the clicked block's position sides, similar to how InGamePacketHandler->syncBlocksNearby does it

Copy link
Member

@dktapps dktapps Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a plugin does $ev->setUseBlock(false), the client will predict a door being opened when it isn't. This code is gated inside $ev->useBlock() and wouldn't run in that case.

return true;
}

Expand All @@ -2289,13 +2305,15 @@ public function useItemOn(Vector3 $vector, Item &$item, int $face, ?Vector3 $cli
}
}
}else{
$player->syncBlocks($mainBlocks);
return false;
}
}elseif($blockClicked->onInteract($item, $face, $clickVector, $player, $returnedItems)){
return true;
}

if($item->isNull() || !$item->canBePlaced()){
$player?->syncBlocks($mainBlocks);
Copy link
Member

@dktapps dktapps Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember too well how this code works. But I'm pretty sure it's possible for the client & server to disagree about the currently held item. So the client might think it's able to place a door while the server doesn't in network races.

return false;
}

Expand All @@ -2307,24 +2325,43 @@ public function useItemOn(Vector3 $vector, Item &$item, int $face, ?Vector3 $cli
$item->getPlacementTransaction($blockReplace, $blockClicked, $face, $clickVector, $player);
if($tx === null){
//no placement options available
$player?->syncBlocks($mainBlocks);
Copy link
Member

@dktapps dktapps Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same again here - the client may think the placement will succeed if it doesn't have an up to date copy of the terrain

return false;
}

foreach($tx->getBlocks() as [$x, $y, $z, $block]){
/** @var Vector3[] $placementSyncBlocks */
$placementSyncBlocks = [];
$allowed = true;
$needsSync = false;
foreach($tx->getBlocks() as [$x, $y, $z, $block]) {
$blockPos = new Vector3($x, $y, $z);
$placementSyncBlocks[] = $blockPos;
$block->position($this, $x, $y, $z);
foreach($block->getCollisionBoxes() as $collisionBox){
if(count($this->getCollidingEntities($collisionBox)) > 0){
return false; //Entity in block
foreach($block->getCollisionBoxes() as $collisionBox) {
$playerCollision = false;
if ($player !== null && $player->getBoundingBox()->intersectsWith($collisionBox)) {
$allowed = false;
$playerCollision = true;
}
Copy link
Member

@dktapps dktapps Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a solution I didn't want to go for too, because it's possible the server and client may have different ideas about the block's bounding box (or the player's own). I think this is the workaround most people went for, but I'd still be happier if there was a better way to do it.

if (count($this->getCollidingEntities($collisionBox)) > 0) {
$allowed = false;
$needsSync = $needsSync || !$playerCollision;
}
}
}

if (!$allowed) {
if ($needsSync) {
$player?->syncBlocks($placementSyncBlocks);
}
return false;
}

if($player !== null){
$ev = new BlockPlaceEvent($player, $tx, $blockClicked, $item);
if($player->isSpectator()){
$ev->cancel();
}

if($player->isAdventure(true) && !$ev->isCancelled()){
$canPlace = false;
$itemParser = LegacyStringToItemParser::getInstance();
Expand All @@ -2343,11 +2380,13 @@ public function useItemOn(Vector3 $vector, Item &$item, int $face, ?Vector3 $cli

$ev->call();
if($ev->isCancelled()){
$player->syncBlocks($placementSyncBlocks);
return false;
}
}

if(!$tx->apply()){
$player?->syncBlocks($placementSyncBlocks);
return false;
}
$first = true;
Expand All @@ -2367,7 +2406,8 @@ public function useItemOn(Vector3 $vector, Item &$item, int $face, ?Vector3 $cli
}

$item->pop();

// As long as we are only syncing the relevant blocks to the actual placement, this should be fine.
$player?->syncBlocks($placementSyncBlocks);
return true;
}

Expand Down