diff --git a/changelogs/4.19.md b/changelogs/4.19.md index b8127b335..89ef3140b 100644 --- a/changelogs/4.19.md +++ b/changelogs/4.19.md @@ -94,3 +94,18 @@ Released 14th April 2023. - Fixed player timings duplication leading to extremely large timings reports when timings runs for a long time with many players. - Packet timings are now indexed by class FQN instead of packet ID. This prevents erroneous timer reuse on packet ID reuse (e.g. multi version servers). - Fixed entity timings being shared by different classes with the same short name. This led to incorrect timings being reported for some entities when custom entities were used. + +# 4.19.3 +Released 21st April 2023. + +## General +- Error IDs for `Packet processing error` disconnects are now split into 4-character chunks to make them easier to type (since they can't be copied from the disconnection screen of a client). + +## Fixes +- Fixed entity-block intersections being checked twice per tick. Besides wasting CPU time, this may have caused unexpected behaviour during entity-block interactions with blocks like water or cacti. +- Fixed performance issue in network inventory synchronization due item NBT being prepared twice. +- Fixed `tools/simulate-chunk-selector.php` argument parsing being completely broken (weird behaviour of PHP `getopt()`). + +## Internals +- `TimingsHandler->stopTiming()` now logs an error message if a subtimer wasn't stopped, rather than throwing an exception. + - Due to interactions between `try...finally` and unexpected errors, throwing exceptions made it difficult for plugin developers to debug errors in their plugins, since it obscured the original error. diff --git a/src/crafting/CraftingManager.php b/src/crafting/CraftingManager.php index 42d11ce9f..1b1428b05 100644 --- a/src/crafting/CraftingManager.php +++ b/src/crafting/CraftingManager.php @@ -56,7 +56,7 @@ class CraftingManager{ * @var FurnaceRecipeManager[] * @phpstan-var array */ - protected array $furnaceRecipeManagers; + protected array $furnaceRecipeManagers = []; /** * @var PotionTypeRecipe[][] diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index 6b1a39d94..37811e959 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -99,7 +99,7 @@ class HandlerList{ * @return RegisteredListener[] */ public function getListenersByPriority(int $priority) : array{ - return $this->handlerSlots[$priority]; + return $this->handlerSlots[$priority] ?? []; } public function getParent() : ?HandlerList{ diff --git a/src/network/mcpe/InventoryManagerEntry.php b/src/network/mcpe/InventoryManagerEntry.php index ac2a40dfe..deb2e8e4d 100644 --- a/src/network/mcpe/InventoryManagerEntry.php +++ b/src/network/mcpe/InventoryManagerEntry.php @@ -40,7 +40,7 @@ final class InventoryManagerEntry{ public array $itemStackInfos = []; /** - * @var int[] + * @var ItemStack[] * @phpstan-var array */ public array $pendingSyncs = []; diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 95f693c9c..67c802679 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -325,6 +325,9 @@ class InGamePacketHandler extends PacketHandler{ if(count($packet->trData->getActions()) > 50){ throw new PacketHandlingException("Too many actions in inventory transaction"); } + if(count($packet->requestChangedSlots) > 10){ + throw new PacketHandlingException("Too many slot sync requests in inventory transaction"); + } $this->inventoryManager->setCurrentItemStackRequestId($packet->requestId); $this->inventoryManager->addRawPredictedSlotChanges($packet->trData->getActions()); @@ -344,6 +347,21 @@ class InGamePacketHandler extends PacketHandler{ } $this->inventoryManager->syncMismatchedPredictedSlotChanges(); + + //requestChangedSlots asks the server to always send out the contents of the specified slots, even if they + //haven't changed. Handling these is necessary to ensure the client inventory stays in sync if the server + //rejects the transaction. The most common example of this is equipping armor by right-click, which doesn't send + //a legacy prediction action for the destination armor slot. + foreach($packet->requestChangedSlots as $containerInfo){ + $windowId = ItemStackContainerIdTranslator::translate($containerInfo->getContainerId(), $this->inventoryManager->getCurrentWindowId()); + foreach($containerInfo->getChangedSlotIndexes() as $slot){ + $inventoryAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slot); + if($inventoryAndSlot !== null){ //trigger the normal slot sync logic + $this->inventoryManager->onSlotChange($inventoryAndSlot[0], $inventoryAndSlot[1]); + } + } + } + $this->inventoryManager->setCurrentItemStackRequestId(null); return $result; } diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index 124606112..b7f9b1604 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -23,7 +23,6 @@ declare(strict_types=1); namespace pocketmine\network\mcpe\handler; -use pocketmine\crafting\CraftingGrid; use pocketmine\inventory\CreativeInventory; use pocketmine\inventory\Inventory; use pocketmine\inventory\transaction\action\CreateItemAction; @@ -246,13 +245,11 @@ class ItemStackRequestExecutor{ $this->specialTransaction = new CraftingTransaction($this->player, $craftingManager, [], $recipe, $repetitions); - $currentWindow = $this->player->getCurrentWindow(); - if($currentWindow !== null && !($currentWindow instanceof CraftingGrid)){ - throw new ItemStackRequestProcessException("Player's current window is not a crafting grid"); - } - $craftingGrid = $currentWindow ?? $this->player->getCraftingGrid(); - - $craftingResults = $recipe->getResultsFor($craftingGrid); + //TODO: Since the system assumes that crafting can only be done in the crafting grid, we have to give it a + //crafting grid to make the API happy. No implementation of getResultsFor() actually uses the crafting grid + //right now, so this will work, but this will become a problem in the future for things like shulker boxes and + //custom crafting recipes. + $craftingResults = $recipe->getResultsFor($this->player->getCraftingGrid()); foreach($craftingResults as $k => $craftingResult){ $craftingResult->setCount($craftingResult->getCount() * $repetitions); $this->craftingResults[$k] = $craftingResult; diff --git a/src/timings/TimingsHandler.php b/src/timings/TimingsHandler.php index d43b432bc..d23b3a34a 100644 --- a/src/timings/TimingsHandler.php +++ b/src/timings/TimingsHandler.php @@ -174,12 +174,12 @@ class TimingsHandler{ } $record = TimingsRecord::getCurrentRecord(); - if($record !== null){ - if($record->getTimerId() !== spl_object_id($this)){ - throw new \LogicException("Timer \"" . $record->getName() . "\" should have been stopped before stopping timer \"" . $this->name . "\""); - } + $timerId = spl_object_id($this); + for(; $record !== null && $record->getTimerId() !== $timerId; $record = TimingsRecord::getCurrentRecord()){ + \GlobalLogger::get()->error("Timer \"" . $record->getName() . "\" should have been stopped before stopping timer \"" . $this->name . "\""); $record->stopTiming($now); } + $record?->stopTiming($now); if($this->parent !== null){ $this->parent->internalStopTiming($now); }