diff --git a/README.md b/README.md index f91fdd31d..fb3a09459 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ * [Building and running from source](BUILDING.md) * [Developer documentation](https://devdoc.pmmp.io) - General documentation for PocketMine-MP plugin developers * [Latest release API documentation](https://apidoc.pmmp.io) - Doxygen API documentation generated for each release - * [Latest bleeding-edge API documentation](https://apidoc-dev.pmmp.io) - Doxygen API documentation generated weekly from `next-major` branch + * [Latest bleeding-edge API documentation](https://apidoc-dev.pmmp.io) - Doxygen API documentation generated weekly from `major-next` branch * [DevTools](https://github.com/pmmp/DevTools/) - Development tools plugin for creating plugins * [ExamplePlugin](https://github.com/pmmp/ExamplePlugin/) - Example plugin demonstrating some basic API features * [Contributing Guidelines](CONTRIBUTING.md) diff --git a/changelogs/4.20.md b/changelogs/4.20.md index 357cdaced..bc8c931b2 100644 --- a/changelogs/4.20.md +++ b/changelogs/4.20.md @@ -34,3 +34,33 @@ Released 26th April 2023. ### `pocketmine\player` - The following API methods have been added: - `public Player->openSignEditor(Vector3 $position) : void` - opens the client-side sign editor GUI for the given position + +# 4.20.1 +Released 27th April 2023. + +## Fixes +- Fixed server crash when firing a bow while holding arrows in the offhand slot. + +## Internals +- `ItemStackContainerIdTranslator::translate()` now requires an additional `int $slotId` parameter and returns `array{int, int}` (translated window ID, translated slot ID) to be used with `InventoryManager->locateWindowAndSlot()`. +- `InventoryManager->locateWindowAndSlot()` now checks if the translated slot actually exists in the requested inventory, and returns `null` if not. Previously, it would return potentially invalid slot IDs without checking them, potentially leading to crashes. + +# 4.20.2 +Released 4th May 2023. + +## Fixes +- Fixed all types of wooden logs appearing as oak in the inventory. +- Fixed a performance issue in `BaseInventory->canAddItem()` (missing `continue` causing useless logic to run). + +# 4.20.3 +Released 6th May 2023. + +## Improvements +- Reduced memory usage of `RuntimeBlockMapping` from 25 MB to 9 MB. Since every thread has its own copy of the block map, this saves a substantial amount of memory. + +## Fixes +- Fixed players falling through blocks in spectator mode. +- Fixed timings reports getting bloated by prolific usage of `PluginManager->registerEvent()`. + - This was caused by creating a new timings handler for each call, regardless of whether a timer already existed for the given event and callback. +- Fixed `Full Server Tick` and other records being missing from timings reports. + - This was caused by timings handler depth not getting reset when timings was disabled and later re-enabled. diff --git a/composer.lock b/composer.lock index 79805d7ee..bb9936eb7 100644 --- a/composer.lock +++ b/composer.lock @@ -1085,16 +1085,16 @@ }, { "name": "symfony/filesystem", - "version": "v5.4.21", + "version": "v5.4.23", "source": { "type": "git", "url": "https://github.com/symfony/filesystem.git", - "reference": "e75960b1bbfd2b8c9e483e0d74811d555ca3de9f" + "reference": "b2f79d86cd9e7de0fff6d03baa80eaed7a5f38b5" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/filesystem/zipball/e75960b1bbfd2b8c9e483e0d74811d555ca3de9f", - "reference": "e75960b1bbfd2b8c9e483e0d74811d555ca3de9f", + "url": "https://api.github.com/repos/symfony/filesystem/zipball/b2f79d86cd9e7de0fff6d03baa80eaed7a5f38b5", + "reference": "b2f79d86cd9e7de0fff6d03baa80eaed7a5f38b5", "shasum": "" }, "require": { @@ -1129,7 +1129,7 @@ "description": "Provides basic utilities for the filesystem", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/filesystem/tree/v5.4.21" + "source": "https://github.com/symfony/filesystem/tree/v5.4.23" }, "funding": [ { @@ -1145,7 +1145,7 @@ "type": "tidelift" } ], - "time": "2023-02-14T08:03:56+00:00" + "time": "2023-03-02T11:38:35+00:00" }, { "name": "symfony/polyfill-ctype", diff --git a/src/block/inventory/DoubleChestInventory.php b/src/block/inventory/DoubleChestInventory.php index 4cfe516b0..a7eb4a439 100644 --- a/src/block/inventory/DoubleChestInventory.php +++ b/src/block/inventory/DoubleChestInventory.php @@ -85,6 +85,20 @@ class DoubleChestInventory extends BaseInventory implements BlockInventory, Inve $this->right->setContents($rightContents); } + protected function getMatchingItemCount(int $slot, Item $test, bool $checkTags) : int{ + $leftSize = $this->left->getSize(); + return $slot < $leftSize ? + $this->left->getMatchingItemCount($slot, $test, $checkTags) : + $this->right->getMatchingItemCount($slot - $leftSize, $test, $checkTags); + } + + public function isSlotEmpty(int $index) : bool{ + $leftSize = $this->left->getSize(); + return $index < $leftSize ? + $this->left->isSlotEmpty($index) : + $this->right->isSlotEmpty($index - $leftSize); + } + protected function getOpenSound() : Sound{ return new ChestOpenSound(); } protected function getCloseSound() : Sound{ return new ChestCloseSound(); } diff --git a/src/inventory/BaseInventory.php b/src/inventory/BaseInventory.php index 764659353..254e44b1e 100644 --- a/src/inventory/BaseInventory.php +++ b/src/inventory/BaseInventory.php @@ -107,12 +107,22 @@ abstract class BaseInventory implements Inventory{ $this->onContentChange($oldContents); } + /** + * Helper for utility functions which search the inventory. + * TODO: make this abstract instead of providing a slow default implementation (BC break) + */ + protected function getMatchingItemCount(int $slot, Item $test, bool $checkTags) : int{ + $item = $this->getItem($slot); + return $item->equals($test, true, $checkTags) ? $item->getCount() : 0; + } + public function contains(Item $item) : bool{ $count = max(1, $item->getCount()); $checkTags = $item->hasNamedTag(); - foreach($this->getContents() as $i){ - if($item->equals($i, true, $checkTags)){ - $count -= $i->getCount(); + for($i = 0, $size = $this->getSize(); $i < $size; $i++){ + $slotCount = $this->getMatchingItemCount($i, $item, $checkTags); + if($slotCount > 0){ + $count -= $slotCount; if($count <= 0){ return true; } @@ -125,9 +135,9 @@ abstract class BaseInventory implements Inventory{ public function all(Item $item) : array{ $slots = []; $checkTags = $item->hasNamedTag(); - foreach($this->getContents() as $index => $i){ - if($item->equals($i, true, $checkTags)){ - $slots[$index] = $i; + for($i = 0, $size = $this->getSize(); $i < $size; $i++){ + if($this->getMatchingItemCount($i, $item, $checkTags) > 0){ + $slots[$i] = $this->getItem($i); } } @@ -138,18 +148,9 @@ abstract class BaseInventory implements Inventory{ $count = $exact ? $item->getCount() : max(1, $item->getCount()); $checkTags = $exact || $item->hasNamedTag(); - foreach($this->getContents() as $index => $i){ - if($item->equals($i, true, $checkTags) && ($i->getCount() === $count || (!$exact && $i->getCount() > $count))){ - return $index; - } - } - - return -1; - } - - public function firstEmpty() : int{ - foreach($this->getContents(true) as $i => $slot){ - if($slot->isNull()){ + for($i = 0, $size = $this->getSize(); $i < $size; $i++){ + $slotCount = $this->getMatchingItemCount($i, $item, $checkTags); + if($slotCount > 0 && ($slotCount === $count || (!$exact && $slotCount > $count))){ return $i; } } @@ -157,6 +158,20 @@ abstract class BaseInventory implements Inventory{ return -1; } + public function firstEmpty() : int{ + for($i = 0, $size = $this->getSize(); $i < $size; $i++){ + if($this->isSlotEmpty($i)){ + return $i; + } + } + + return -1; + } + + /** + * TODO: make this abstract and force implementations to implement it properly (BC break) + * This default implementation works, but is slow. + */ public function isSlotEmpty(int $index) : bool{ return $this->getItem($index)->isNull(); } @@ -167,14 +182,16 @@ abstract class BaseInventory implements Inventory{ public function getAddableItemQuantity(Item $item) : int{ $count = $item->getCount(); + $maxStackSize = min($this->getMaxStackSize(), $item->getMaxStackSize()); + for($i = 0, $size = $this->getSize(); $i < $size; ++$i){ - $slot = $this->getItem($i); - if($item->canStackWith($slot)){ - if(($diff = min($slot->getMaxStackSize(), $item->getMaxStackSize()) - $slot->getCount()) > 0){ + if($this->isSlotEmpty($i)){ + $count -= $maxStackSize; + }else{ + $slotCount = $this->getMatchingItemCount($i, $item, true); + if($slotCount > 0 && ($diff = $maxStackSize - $slotCount) > 0){ $count -= $diff; } - }elseif($slot->isNull()){ - $count -= min($this->getMaxStackSize(), $item->getMaxStackSize()); } if($count <= 0){ @@ -208,22 +225,29 @@ abstract class BaseInventory implements Inventory{ return $returnSlots; } - private function internalAddItem(Item $slot) : Item{ + private function internalAddItem(Item $newItem) : Item{ $emptySlots = []; + $maxStackSize = min($this->getMaxStackSize(), $newItem->getMaxStackSize()); + for($i = 0, $size = $this->getSize(); $i < $size; ++$i){ - $item = $this->getItem($i); - if($item->isNull()){ + if($this->isSlotEmpty($i)){ $emptySlots[] = $i; + continue; + } + $slotCount = $this->getMatchingItemCount($i, $newItem, true); + if($slotCount === 0){ + continue; } - if($slot->canStackWith($item) && $item->getCount() < $item->getMaxStackSize()){ - $amount = min($item->getMaxStackSize() - $item->getCount(), $slot->getCount(), $this->getMaxStackSize()); + if($slotCount < $maxStackSize){ + $amount = min($maxStackSize - $slotCount, $newItem->getCount()); if($amount > 0){ - $slot->setCount($slot->getCount() - $amount); - $item->setCount($item->getCount() + $amount); - $this->setItem($i, $item); - if($slot->getCount() <= 0){ + $newItem->setCount($newItem->getCount() - $amount); + $slotItem = $this->getItem($i); + $slotItem->setCount($slotItem->getCount() + $amount); + $this->setItem($i, $slotItem); + if($newItem->getCount() <= 0){ break; } } @@ -232,64 +256,66 @@ abstract class BaseInventory implements Inventory{ if(count($emptySlots) > 0){ foreach($emptySlots as $slotIndex){ - $amount = min($slot->getMaxStackSize(), $slot->getCount(), $this->getMaxStackSize()); - $slot->setCount($slot->getCount() - $amount); - $item = clone $slot; - $item->setCount($amount); - $this->setItem($slotIndex, $item); - if($slot->getCount() <= 0){ + $amount = min($maxStackSize, $newItem->getCount()); + $newItem->setCount($newItem->getCount() - $amount); + $slotItem = clone $newItem; + $slotItem->setCount($amount); + $this->setItem($slotIndex, $slotItem); + if($newItem->getCount() <= 0){ break; } } } - return $slot; + return $newItem; } public function remove(Item $item) : void{ $checkTags = $item->hasNamedTag(); - foreach($this->getContents() as $index => $i){ - if($item->equals($i, true, $checkTags)){ - $this->clear($index); + for($i = 0, $size = $this->getSize(); $i < $size; $i++){ + if($this->getMatchingItemCount($i, $item, $checkTags) > 0){ + $this->clear($i); } } } public function removeItem(Item ...$slots) : array{ - /** @var Item[] $itemSlots */ + /** @var Item[] $searchItems */ /** @var Item[] $slots */ - $itemSlots = []; + $searchItems = []; foreach($slots as $slot){ if(!$slot->isNull()){ - $itemSlots[] = clone $slot; + $searchItems[] = clone $slot; } } for($i = 0, $size = $this->getSize(); $i < $size; ++$i){ - $item = $this->getItem($i); - if($item->isNull()){ + if($this->isSlotEmpty($i)){ continue; } - foreach($itemSlots as $index => $slot){ - if($slot->equals($item, true, $slot->hasNamedTag())){ - $amount = min($item->getCount(), $slot->getCount()); - $slot->setCount($slot->getCount() - $amount); - $item->setCount($item->getCount() - $amount); - $this->setItem($i, $item); - if($slot->getCount() <= 0){ - unset($itemSlots[$index]); + foreach($searchItems as $index => $search){ + $slotCount = $this->getMatchingItemCount($i, $search, $search->hasNamedTag()); + if($slotCount > 0){ + $amount = min($slotCount, $search->getCount()); + $search->setCount($search->getCount() - $amount); + + $slotItem = $this->getItem($i); + $slotItem->setCount($slotItem->getCount() - $amount); + $this->setItem($i, $slotItem); + if($search->getCount() <= 0){ + unset($searchItems[$index]); } } } - if(count($itemSlots) === 0){ + if(count($searchItems) === 0){ break; } } - return $itemSlots; + return $searchItems; } public function clear(int $index) : void{ diff --git a/src/inventory/DelegateInventory.php b/src/inventory/DelegateInventory.php index ba9e5a983..a211732cf 100644 --- a/src/inventory/DelegateInventory.php +++ b/src/inventory/DelegateInventory.php @@ -85,6 +85,10 @@ class DelegateInventory extends BaseInventory{ $this->backingInventory->setContents($items); } + public function isSlotEmpty(int $index) : bool{ + return $this->backingInventory->isSlotEmpty($index); + } + protected function onSlotChange(int $index, Item $before) : void{ if($this->backingInventoryChanging){ parent::onSlotChange($index, $before); diff --git a/src/inventory/SimpleInventory.php b/src/inventory/SimpleInventory.php index aae11c84c..19fd6da34 100644 --- a/src/inventory/SimpleInventory.php +++ b/src/inventory/SimpleInventory.php @@ -83,4 +83,13 @@ class SimpleInventory extends BaseInventory{ } } } + + protected function getMatchingItemCount(int $slot, Item $test, bool $checkTags) : int{ + $slotItem = $this->slots[$slot]; + return $slotItem !== null && $slotItem->equals($test, $checkDamage, $checkTags) ? $slotItem->getCount() : 0; + } + + public function isSlotEmpty(int $index) : bool{ + return $this->slots[$index] === null || $this->slots[$index]->isNull(); + } } diff --git a/src/network/mcpe/InventoryManager.php b/src/network/mcpe/InventoryManager.php index c8434f753..fc7c6641f 100644 --- a/src/network/mcpe/InventoryManager.php +++ b/src/network/mcpe/InventoryManager.php @@ -207,11 +207,13 @@ class InventoryManager{ if($entry === null){ return null; } + $inventory = $entry->getInventory(); $coreSlotId = $entry->mapNetToCore($netSlotId); - return $coreSlotId !== null ? [$entry->getInventory(), $coreSlotId] : null; + return $coreSlotId !== null && $inventory->slotExists($coreSlotId) ? [$inventory, $coreSlotId] : null; } - if(isset($this->networkIdToInventoryMap[$windowId])){ - return [$this->networkIdToInventoryMap[$windowId], $netSlotId]; + $inventory = $this->networkIdToInventoryMap[$windowId] ?? null; + if($inventory !== null && $inventory->slotExists($netSlotId)){ + return [$inventory, $netSlotId]; } return null; } diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 0f5da336a..abab59825 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -928,6 +928,12 @@ class NetworkSession{ [ //TODO: dynamic flying speed! FINALLY!!!!!!!!!!!!!!!!! new AbilitiesLayer(AbilitiesLayer::LAYER_BASE, $boolAbilities, 0.05, 0.1), + + //TODO: HACK! In 1.19.80, the client starts falling in our faux spectator mode when it clips into a + //block. I have no idea why this works, since we don't actually use the real spectator mode. + new AbilitiesLayer(AbilitiesLayer::LAYER_SPECTATOR, [ + AbilitiesLayer::ABILITY_FLYING => true, + ], null, null) ] ))); } diff --git a/src/network/mcpe/handler/InGamePacketHandler.php b/src/network/mcpe/handler/InGamePacketHandler.php index 807b9239f..d8b6caf1f 100644 --- a/src/network/mcpe/handler/InGamePacketHandler.php +++ b/src/network/mcpe/handler/InGamePacketHandler.php @@ -353,8 +353,8 @@ class InGamePacketHandler extends PacketHandler{ //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){ + foreach($containerInfo->getChangedSlotIndexes() as $netSlot){ + [$windowId, $slot] = ItemStackContainerIdTranslator::translate($containerInfo->getContainerId(), $this->inventoryManager->getCurrentWindowId(), $netSlot); $inventoryAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slot); if($inventoryAndSlot !== null){ //trigger the normal slot sync logic $this->inventoryManager->onSlotChange($inventoryAndSlot[0], $inventoryAndSlot[1]); diff --git a/src/network/mcpe/handler/ItemStackContainerIdTranslator.php b/src/network/mcpe/handler/ItemStackContainerIdTranslator.php index a0ad3b26c..f9ea9ef5f 100644 --- a/src/network/mcpe/handler/ItemStackContainerIdTranslator.php +++ b/src/network/mcpe/handler/ItemStackContainerIdTranslator.php @@ -33,15 +33,21 @@ final class ItemStackContainerIdTranslator{ //NOOP } - public static function translate(int $containerInterfaceId, int $currentWindowId) : int{ + /** + * @return int[] + * @phpstan-return array{int, int} + * @throws PacketHandlingException + */ + public static function translate(int $containerInterfaceId, int $currentWindowId, int $slotId) : array{ return match($containerInterfaceId){ - ContainerUIIds::ARMOR => ContainerIds::ARMOR, + ContainerUIIds::ARMOR => [ContainerIds::ARMOR, $slotId], ContainerUIIds::HOTBAR, ContainerUIIds::INVENTORY, - ContainerUIIds::COMBINED_HOTBAR_AND_INVENTORY => ContainerIds::INVENTORY, + ContainerUIIds::COMBINED_HOTBAR_AND_INVENTORY => [ContainerIds::INVENTORY, $slotId], - ContainerUIIds::OFFHAND => ContainerIds::OFFHAND, + //TODO: HACK! The client sends an incorrect slot ID for the offhand as of 1.19.70 (though this doesn't really matter since the offhand has only 1 slot anyway) + ContainerUIIds::OFFHAND => [ContainerIds::OFFHAND, 0], ContainerUIIds::ANVIL_INPUT, ContainerUIIds::ANVIL_MATERIAL, @@ -68,7 +74,7 @@ final class ItemStackContainerIdTranslator{ ContainerUIIds::TRADE2_INGREDIENT1, ContainerUIIds::TRADE2_INGREDIENT2, ContainerUIIds::TRADE_INGREDIENT1, - ContainerUIIds::TRADE_INGREDIENT2 => ContainerIds::UI, + ContainerUIIds::TRADE_INGREDIENT2 => [ContainerIds::UI, $slotId], ContainerUIIds::BARREL, ContainerUIIds::BLAST_FURNACE_INGREDIENT, @@ -80,7 +86,7 @@ final class ItemStackContainerIdTranslator{ ContainerUIIds::FURNACE_RESULT, ContainerUIIds::LEVEL_ENTITY, //chest ContainerUIIds::SHULKER_BOX, - ContainerUIIds::SMOKER_INGREDIENT => $currentWindowId, + ContainerUIIds::SMOKER_INGREDIENT => [$currentWindowId, $slotId], //all preview slots are ignored, since the client shouldn't be modifying those directly diff --git a/src/network/mcpe/handler/ItemStackRequestExecutor.php b/src/network/mcpe/handler/ItemStackRequestExecutor.php index b7f9b1604..f9532291c 100644 --- a/src/network/mcpe/handler/ItemStackRequestExecutor.php +++ b/src/network/mcpe/handler/ItemStackRequestExecutor.php @@ -111,12 +111,7 @@ class ItemStackRequestExecutor{ * @throws ItemStackRequestProcessException */ protected function getBuilderInventoryAndSlot(ItemStackRequestSlotInfo $info) : array{ - $windowId = ItemStackContainerIdTranslator::translate($info->getContainerId(), $this->inventoryManager->getCurrentWindowId()); - $slotId = $info->getSlotId(); - if($info->getContainerId() === ContainerUIIds::OFFHAND && $slotId === 1){ - //TODO: HACK! The client sends an incorrect slot ID for the offhand as of 1.19.70 - $slotId = 0; - } + [$windowId, $slotId] = ItemStackContainerIdTranslator::translate($info->getContainerId(), $this->inventoryManager->getCurrentWindowId(), $info->getSlotId()); $windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slotId); if($windowAndSlot === null){ throw new ItemStackRequestProcessException("No open inventory matches container UI ID: " . $info->getContainerId() . ", slot ID: " . $info->getSlotId()); diff --git a/src/network/mcpe/handler/ItemStackResponseBuilder.php b/src/network/mcpe/handler/ItemStackResponseBuilder.php index 2a55c2d95..09af69f2a 100644 --- a/src/network/mcpe/handler/ItemStackResponseBuilder.php +++ b/src/network/mcpe/handler/ItemStackResponseBuilder.php @@ -53,11 +53,7 @@ final class ItemStackResponseBuilder{ * @phpstan-return array{Inventory, int} */ private function getInventoryAndSlot(int $containerInterfaceId, int $slotId) : ?array{ - if($containerInterfaceId === ContainerUIIds::OFFHAND && $slotId === 1){ - //TODO: HACK! The client sends an incorrect slot ID for the offhand as of 1.19.70 - $slotId = 0; - } - $windowId = ItemStackContainerIdTranslator::translate($containerInterfaceId, $this->inventoryManager->getCurrentWindowId()); + [$windowId, $slotId] = ItemStackContainerIdTranslator::translate($containerInterfaceId, $this->inventoryManager->getCurrentWindowId(), $slotId); $windowAndSlot = $this->inventoryManager->locateWindowAndSlot($windowId, $slotId); if($windowAndSlot === null){ return null; diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index bb27e6d2a..67ca8cc37 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -37,7 +37,7 @@ use pocketmine\permission\DefaultPermissions; use pocketmine\permission\PermissionManager; use pocketmine\permission\PermissionParser; use pocketmine\Server; -use pocketmine\timings\TimingsHandler; +use pocketmine\timings\Timings; use pocketmine\utils\AssumptionFailedError; use pocketmine\utils\Utils; use Symfony\Component\Filesystem\Path; @@ -651,7 +651,7 @@ class PluginManager{ throw new PluginException("Plugin attempted to register event handler " . $handlerName . "() to event " . $event . " while not enabled"); } - $timings = new TimingsHandler($handlerName . "(" . (new \ReflectionClass($event))->getShortName() . ")", group: $plugin->getDescription()->getFullName()); + $timings = Timings::getEventHandlerTimings($event, $handlerName, $plugin->getDescription()->getFullName()); $registeredListener = new RegisteredListener($handler, $priority, $plugin, $handleCancelled, $timings); HandlerListManager::global()->getListFor($event)->register($registeredListener); diff --git a/src/timings/Timings.php b/src/timings/Timings.php index 583624046..5cc06fdb7 100644 --- a/src/timings/Timings.php +++ b/src/timings/Timings.php @@ -118,6 +118,8 @@ abstract class Timings{ /** @var TimingsHandler[] */ private static array $events = []; + /** @var TimingsHandler[][] */ + private static array $eventHandlers = []; public static function init() : void{ if(self::$initialized){ @@ -299,4 +301,16 @@ abstract class Timings{ return self::$events[$eventClass]; } + + /** + * @phpstan-template TEvent of Event + * @phpstan-param class-string $event + */ + public static function getEventHandlerTimings(string $event, string $handlerName, string $group) : TimingsHandler{ + if(!isset(self::$eventHandlers[$event][$handlerName])){ + self::$eventHandlers[$event][$handlerName] = new TimingsHandler($handlerName . "(" . self::shortenCoreClassName($event, "pocketmine\\event\\") . ")", group: $group); + } + + return self::$eventHandlers[$event][$handlerName]; + } } diff --git a/src/timings/TimingsHandler.php b/src/timings/TimingsHandler.php index d23b3a34a..73e963875 100644 --- a/src/timings/TimingsHandler.php +++ b/src/timings/TimingsHandler.php @@ -96,7 +96,7 @@ class TimingsHandler{ } public static function reload() : void{ - TimingsRecord::clearRecords(); + TimingsRecord::reset(); if(self::$enabled){ self::$timingStart = hrtime(true); } @@ -204,8 +204,9 @@ class TimingsHandler{ /** * @internal */ - public function destroyCycles() : void{ + public function reset() : void{ $this->rootRecord = null; $this->recordsByParent = []; + $this->timingDepth = 0; } } diff --git a/src/timings/TimingsRecord.php b/src/timings/TimingsRecord.php index 5254d2e7d..f09984b5b 100644 --- a/src/timings/TimingsRecord.php +++ b/src/timings/TimingsRecord.php @@ -42,9 +42,12 @@ final class TimingsRecord{ private static ?self $currentRecord = null; - public static function clearRecords() : void{ + /** + * @internal + */ + public static function reset() : void{ foreach(self::$records as $record){ - $record->handler->destroyCycles(); + $record->handler->reset(); } self::$records = []; self::$currentRecord = null; diff --git a/src/world/TickingChunkEntry.php b/src/world/TickingChunkEntry.php deleted file mode 100644 index ca965463d..000000000 --- a/src/world/TickingChunkEntry.php +++ /dev/null @@ -1,37 +0,0 @@ - ChunkTicker - * @phpstan-var array - */ - public array $tickers = []; - - public bool $ready = false; -} diff --git a/src/world/World.php b/src/world/World.php index 8707fc47a..433d03f66 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -212,10 +212,25 @@ class World implements ChunkManager{ private int $maxY; /** - * @var TickingChunkEntry[] chunkHash => TickingChunkEntry - * @phpstan-var array + * @var ChunkTicker[][] chunkHash => [spl_object_id => ChunkTicker] + * @phpstan-var array> */ - private array $tickingChunks = []; + private array $registeredTickingChunks = []; + + /** + * Set of chunks which are definitely ready for ticking. + * + * @var int[] + * @phpstan-var array + */ + private array $validTickingChunks = []; + + /** + * Set of chunks which might be ready for ticking. These will be checked at the next tick. + * @var int[] + * @phpstan-var array + */ + private array $recheckTickingChunks = []; /** * @var ChunkLoader[][] chunkHash => [spl_object_id => ChunkLoader] @@ -1153,14 +1168,14 @@ class World implements ChunkManager{ } /** - * Returns a list of chunk position hashes (as returned by World::chunkHash()) which are currently registered for + * Returns a list of chunk position hashes (as returned by World::chunkHash()) which are currently valid for * ticking. * * @return int[] * @phpstan-return list */ public function getTickingChunks() : array{ - return array_keys($this->tickingChunks); + return array_keys($this->validTickingChunks); } /** @@ -1169,11 +1184,8 @@ class World implements ChunkManager{ */ public function registerTickingChunk(ChunkTicker $ticker, int $chunkX, int $chunkZ) : void{ $chunkPosHash = World::chunkHash($chunkX, $chunkZ); - $entry = $this->tickingChunks[$chunkPosHash] ?? null; - if($entry === null){ - $entry = $this->tickingChunks[$chunkPosHash] = new TickingChunkEntry(); - } - $entry->tickers[spl_object_id($ticker)] = $ticker; + $this->registeredTickingChunks[$chunkPosHash][spl_object_id($ticker)] = $ticker; + $this->recheckTickingChunks[$chunkPosHash] = $chunkPosHash; } /** @@ -1183,42 +1195,40 @@ class World implements ChunkManager{ public function unregisterTickingChunk(ChunkTicker $ticker, int $chunkX, int $chunkZ) : void{ $chunkHash = World::chunkHash($chunkX, $chunkZ); $tickerId = spl_object_id($ticker); - if(isset($this->tickingChunks[$chunkHash]->tickers[$tickerId])){ - unset($this->tickingChunks[$chunkHash]->tickers[$tickerId]); - if(count($this->tickingChunks[$chunkHash]->tickers) === 0){ - unset($this->tickingChunks[$chunkHash]); + if(isset($this->registeredTickingChunks[$chunkHash][$tickerId])){ + unset($this->registeredTickingChunks[$chunkHash][$tickerId]); + if(count($this->registeredTickingChunks[$chunkHash]) === 0){ + unset( + $this->registeredTickingChunks[$chunkHash], + $this->recheckTickingChunks[$chunkHash], + $this->validTickingChunks[$chunkHash] + ); } } } private function tickChunks() : void{ - if($this->chunkTickRadius <= 0 || count($this->tickingChunks) === 0){ + if($this->chunkTickRadius <= 0 || count($this->registeredTickingChunks) === 0){ return; } - $this->timings->randomChunkUpdatesChunkSelection->startTiming(); + if(count($this->recheckTickingChunks) > 0){ + $this->timings->randomChunkUpdatesChunkSelection->startTiming(); - /** @var bool[] $chunkTickList chunkhash => dummy */ - $chunkTickList = []; + $chunkTickableCache = []; - $chunkTickableCache = []; - - foreach($this->tickingChunks as $hash => $entry){ - if(!$entry->ready){ + foreach($this->recheckTickingChunks as $hash => $_){ World::getXZ($hash, $chunkX, $chunkZ); if($this->isChunkTickable($chunkX, $chunkZ, $chunkTickableCache)){ - $entry->ready = true; - }else{ - //the chunk has been flagged as temporarily not tickable, so we don't want to tick it this time - continue; + $this->validTickingChunks[$hash] = $hash; } } - $chunkTickList[$hash] = true; + $this->recheckTickingChunks = []; + + $this->timings->randomChunkUpdatesChunkSelection->stopTiming(); } - $this->timings->randomChunkUpdatesChunkSelection->stopTiming(); - - foreach($chunkTickList as $index => $_){ + foreach($this->validTickingChunks as $index => $_){ World::getXZ($index, $chunkX, $chunkZ); $this->tickChunk($chunkX, $chunkZ); @@ -1267,16 +1277,23 @@ class World implements ChunkManager{ } /** - * Marks the 3x3 chunks around the specified chunk as not ready to be ticked. This is used to prevent chunk ticking - * while a chunk is being populated, light-populated, or unloaded. - * Each chunk will be rechecked every tick until it is ready to be ticked again. + * Marks the 3x3 square of chunks centered on the specified chunk for chunk ticking eligibility recheck. + * + * This should be used whenever the chunk's eligibility to be ticked is changed. This includes: + * - Loading/unloading the chunk (the chunk may be registered for ticking before it is loaded) + * - Locking/unlocking the chunk (e.g. world population) + * - Light populated state change (i.e. scheduled for light population, or light population completed) + * - Arbitrary chunk replacement (i.e. setChunk() or similar) */ - private function markTickingChunkUnavailable(int $chunkX, int $chunkZ) : void{ + private function markTickingChunkForRecheck(int $chunkX, int $chunkZ) : void{ for($cx = -1; $cx <= 1; ++$cx){ for($cz = -1; $cz <= 1; ++$cz){ $chunkHash = World::chunkHash($chunkX + $cx, $chunkZ + $cz); - if(isset($this->tickingChunks[$chunkHash])){ - $this->tickingChunks[$chunkHash]->ready = false; + unset($this->validTickingChunks[$chunkHash]); + if(isset($this->registeredTickingChunks[$chunkHash])){ + $this->recheckTickingChunks[$chunkHash] = $chunkHash; + }else{ + unset($this->recheckTickingChunks[$chunkHash]); } } } @@ -1287,7 +1304,7 @@ class World implements ChunkManager{ $lightPopulatedState = $this->chunks[$chunkHash]->isLightPopulated(); if($lightPopulatedState === false){ $this->chunks[$chunkHash]->setLightPopulated(null); - $this->markTickingChunkUnavailable($chunkX, $chunkZ); + $this->markTickingChunkForRecheck($chunkX, $chunkZ); $this->workerPool->submitTask(new LightPopulationTask( $this->chunks[$chunkHash], @@ -1311,6 +1328,7 @@ class World implements ChunkManager{ $chunk->getSubChunk($y)->setBlockSkyLightArray($lightArray); } $chunk->setLightPopulated(true); + $this->markTickingChunkForRecheck($chunkX, $chunkZ); } )); } @@ -2362,7 +2380,7 @@ class World implements ChunkManager{ throw new \InvalidArgumentException("Chunk $chunkX $chunkZ is already locked"); } $this->chunkLock[$chunkHash] = $lockId; - $this->markTickingChunkUnavailable($chunkX, $chunkZ); + $this->markTickingChunkForRecheck($chunkX, $chunkZ); } /** @@ -2377,6 +2395,7 @@ class World implements ChunkManager{ $chunkHash = World::chunkHash($chunkX, $chunkZ); if(isset($this->chunkLock[$chunkHash]) && ($lockId === null || $this->chunkLock[$chunkHash] === $lockId)){ unset($this->chunkLock[$chunkHash]); + $this->markTickingChunkForRecheck($chunkX, $chunkZ); return true; } return false; @@ -2428,7 +2447,7 @@ class World implements ChunkManager{ unset($this->blockCache[$chunkHash]); unset($this->changedBlocks[$chunkHash]); $chunk->setTerrainDirty(); - $this->markTickingChunkUnavailable($chunkX, $chunkZ); //this replacement chunk may not meet the conditions for ticking + $this->markTickingChunkForRecheck($chunkX, $chunkZ); //this replacement chunk may not meet the conditions for ticking if(!$this->isChunkInUse($chunkX, $chunkZ)){ $this->unloadChunkRequest($chunkX, $chunkZ); @@ -2710,6 +2729,7 @@ class World implements ChunkManager{ foreach($this->getChunkListeners($x, $z) as $listener){ $listener->onChunkLoaded($x, $z, $this->chunks[$chunkHash]); } + $this->markTickingChunkForRecheck($x, $z); //tickers may have been registered before the chunk was loaded $this->timings->syncChunkLoad->stopTiming(); @@ -2851,8 +2871,8 @@ class World implements ChunkManager{ unset($this->chunks[$chunkHash]); unset($this->blockCache[$chunkHash]); unset($this->changedBlocks[$chunkHash]); - unset($this->tickingChunks[$chunkHash]); - $this->markTickingChunkUnavailable($x, $z); + unset($this->registeredTickingChunks[$chunkHash]); + $this->markTickingChunkForRecheck($x, $z); if(array_key_exists($chunkHash, $this->chunkPopulationRequestMap)){ $this->logger->debug("Rejecting population promise for chunk $x $z");