From 97ef209c5fafd7553a249484c50d42e6d8c08c2e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 19 Dec 2022 16:26:36 +0000 Subject: [PATCH 01/16] HandlerList: added missing class-string type for constructor --- src/event/HandlerList.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index 7d93c2ebe..0e16555b3 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -31,6 +31,10 @@ class HandlerList{ /** @var RegisteredListener[][] */ private array $handlerSlots = []; + /** + * @phpstan-template TEvent of Event + * @phpstan-param class-string $class + */ public function __construct( private string $class, private ?HandlerList $parentList From aebcfc516ff97492c58ee478a3944915a777a75e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 19 Dec 2022 20:17:29 +0000 Subject: [PATCH 02/16] World: do not refresh ticked chunks list every tick this is just wasting CPU time, since the effects aren't noticeable on such a small timescale anyway. This reduces the CPU impact of chunk selection by 95%. However, this is the lesser part of chunk ticking, and the lion's share of the performance impact still comes from actually ticking the chunks. --- src/world/World.php | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/world/World.php b/src/world/World.php index b20960755..a448a9b62 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -160,6 +160,12 @@ class World implements ChunkManager{ public const DEFAULT_TICKED_BLOCKS_PER_SUBCHUNK_PER_TICK = 3; + /** + * Ticking chunk cache is refreshed after this many ticks. Increasing this value increases the interval between + * cache refreshes, which can cause longer delays before newly loaded chunks start being ticked. + */ + private const TICKING_CHUNK_CACHE_REFRESH_INTERVAL = 20; + /** * @var Player[] entity runtime ID => Player * @phpstan-var array @@ -321,6 +327,12 @@ class World implements ChunkManager{ private int $chunkTickRadius; private int $tickedBlocksPerSubchunkPerTick = self::DEFAULT_TICKED_BLOCKS_PER_SUBCHUNK_PER_TICK; + private int $tickingChunkCacheRefreshTicker = 0; + /** + * @var true[] + * @phpstan-var array + */ + private array $tickingChunkCache = []; /** * @var true[] * @phpstan-var array @@ -1141,15 +1153,8 @@ class World implements ChunkManager{ $this->chunkTickRadius = $radius; } - private function tickChunks() : void{ - if($this->chunkTickRadius <= 0 || count($this->tickingLoaders) === 0){ - return; - } - - $this->timings->randomChunkUpdatesChunkSelection->startTiming(); - - /** @var bool[] $chunkTickList chunkhash => dummy */ - $chunkTickList = []; + private function selectTickedChunks() : void{ + $this->tickingChunkCache = []; $centerChunks = []; @@ -1170,15 +1175,27 @@ class World implements ChunkManager{ $centerChunkZ ) as $hash){ World::getXZ($hash, $chunkX, $chunkZ); - if(!isset($chunkTickList[$hash]) && isset($this->chunks[$hash]) && $this->isChunkTickable($chunkX, $chunkZ)){ - $chunkTickList[$hash] = true; + if(!isset($this->tickingChunkCache[$hash]) && isset($this->chunks[$hash]) && $this->isChunkTickable($chunkX, $chunkZ)){ + $this->tickingChunkCache[$hash] = true; } } } + } - $this->timings->randomChunkUpdatesChunkSelection->stopTiming(); + private function tickChunks() : void{ + if($this->chunkTickRadius <= 0 || count($this->tickingLoaders) === 0){ + $this->tickingChunkCache = []; + return; + } - foreach($chunkTickList as $index => $_){ + if(++$this->tickingChunkCacheRefreshTicker >= self::TICKING_CHUNK_CACHE_REFRESH_INTERVAL){ + $this->tickingChunkCacheRefreshTicker = 0; + $this->timings->randomChunkUpdatesChunkSelection->startTiming(); + $this->selectTickedChunks(); + $this->timings->randomChunkUpdatesChunkSelection->stopTiming(); + } + + foreach($this->tickingChunkCache as $index => $_){ World::getXZ($index, $chunkX, $chunkZ); $this->tickChunk($chunkX, $chunkZ); @@ -2783,6 +2800,7 @@ class World implements ChunkManager{ unset($this->chunks[$chunkHash]); unset($this->blockCache[$chunkHash]); unset($this->changedBlocks[$chunkHash]); + unset($this->tickingChunkCache[$chunkHash]); if(array_key_exists($chunkHash, $this->chunkPopulationRequestMap)){ $this->logger->debug("Rejecting population promise for chunk $x $z"); From 1e5597f0d5d0b9f1aaa1c9db1acbfc9e498c4349 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 19 Dec 2022 20:20:52 +0000 Subject: [PATCH 03/16] World: account for null chunk edge case in tickChunk() the target chunk may no longer be loaded if it was unloaded during a previous chunk's tick (e.g. during BlockGrowEvent). Since the parent function iterates over a pre-selected array of chunks, the chunk will still be present in the list even if it's no longer loaded by the time it's reached. --- src/world/World.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/world/World.php b/src/world/World.php index 4c8d6a44c..54fd7863b 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -1222,7 +1222,8 @@ class World implements ChunkManager{ private function tickChunk(int $chunkX, int $chunkZ) : void{ $chunk = $this->getChunk($chunkX, $chunkZ); if($chunk === null){ - throw new \InvalidArgumentException("Chunk is not loaded"); + //the chunk may have been unloaded during a previous chunk's update (e.g. during BlockGrowEvent) + return; } foreach($this->getChunkEntities($chunkX, $chunkZ) as $entity){ $entity->onRandomUpdate(); From 923dcec4e766314e8393c8d0926854505296e027 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 19 Dec 2022 20:57:51 +0000 Subject: [PATCH 04/16] Revert "World: do not refresh ticked chunks list every tick" This reverts commit aebcfc516ff97492c58ee478a3944915a777a75e. this has edge cases in the handling of adjacent chunk locks which I didn't consider at the time. Once accounting for those edge cases, it became significantly more complex to the point that I realized this needed more planning. --- src/world/World.php | 44 +++++++++++++------------------------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/src/world/World.php b/src/world/World.php index a448a9b62..b20960755 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -160,12 +160,6 @@ class World implements ChunkManager{ public const DEFAULT_TICKED_BLOCKS_PER_SUBCHUNK_PER_TICK = 3; - /** - * Ticking chunk cache is refreshed after this many ticks. Increasing this value increases the interval between - * cache refreshes, which can cause longer delays before newly loaded chunks start being ticked. - */ - private const TICKING_CHUNK_CACHE_REFRESH_INTERVAL = 20; - /** * @var Player[] entity runtime ID => Player * @phpstan-var array @@ -327,12 +321,6 @@ class World implements ChunkManager{ private int $chunkTickRadius; private int $tickedBlocksPerSubchunkPerTick = self::DEFAULT_TICKED_BLOCKS_PER_SUBCHUNK_PER_TICK; - private int $tickingChunkCacheRefreshTicker = 0; - /** - * @var true[] - * @phpstan-var array - */ - private array $tickingChunkCache = []; /** * @var true[] * @phpstan-var array @@ -1153,8 +1141,15 @@ class World implements ChunkManager{ $this->chunkTickRadius = $radius; } - private function selectTickedChunks() : void{ - $this->tickingChunkCache = []; + private function tickChunks() : void{ + if($this->chunkTickRadius <= 0 || count($this->tickingLoaders) === 0){ + return; + } + + $this->timings->randomChunkUpdatesChunkSelection->startTiming(); + + /** @var bool[] $chunkTickList chunkhash => dummy */ + $chunkTickList = []; $centerChunks = []; @@ -1175,27 +1170,15 @@ class World implements ChunkManager{ $centerChunkZ ) as $hash){ World::getXZ($hash, $chunkX, $chunkZ); - if(!isset($this->tickingChunkCache[$hash]) && isset($this->chunks[$hash]) && $this->isChunkTickable($chunkX, $chunkZ)){ - $this->tickingChunkCache[$hash] = true; + if(!isset($chunkTickList[$hash]) && isset($this->chunks[$hash]) && $this->isChunkTickable($chunkX, $chunkZ)){ + $chunkTickList[$hash] = true; } } } - } - private function tickChunks() : void{ - if($this->chunkTickRadius <= 0 || count($this->tickingLoaders) === 0){ - $this->tickingChunkCache = []; - return; - } + $this->timings->randomChunkUpdatesChunkSelection->stopTiming(); - if(++$this->tickingChunkCacheRefreshTicker >= self::TICKING_CHUNK_CACHE_REFRESH_INTERVAL){ - $this->tickingChunkCacheRefreshTicker = 0; - $this->timings->randomChunkUpdatesChunkSelection->startTiming(); - $this->selectTickedChunks(); - $this->timings->randomChunkUpdatesChunkSelection->stopTiming(); - } - - foreach($this->tickingChunkCache as $index => $_){ + foreach($chunkTickList as $index => $_){ World::getXZ($index, $chunkX, $chunkZ); $this->tickChunk($chunkX, $chunkZ); @@ -2800,7 +2783,6 @@ class World implements ChunkManager{ unset($this->chunks[$chunkHash]); unset($this->blockCache[$chunkHash]); unset($this->changedBlocks[$chunkHash]); - unset($this->tickingChunkCache[$chunkHash]); if(array_key_exists($chunkHash, $this->chunkPopulationRequestMap)){ $this->logger->debug("Rejecting population promise for chunk $x $z"); From ee7d4728d8c813af39cab5b03736cc6df76f2a2b Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 19 Dec 2022 21:20:21 +0000 Subject: [PATCH 05/16] World: added cache for isChunkTickable() this considerably reduces the amount of work done by the function, since it's usually checking the same chunks over and over again. --- src/world/World.php | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/world/World.php b/src/world/World.php index b20960755..8c14c7fc1 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -1151,6 +1151,8 @@ class World implements ChunkManager{ /** @var bool[] $chunkTickList chunkhash => dummy */ $chunkTickList = []; + $chunkTickableCache = []; + $centerChunks = []; $selector = new ChunkSelector(); @@ -1170,7 +1172,7 @@ class World implements ChunkManager{ $centerChunkZ ) as $hash){ World::getXZ($hash, $chunkX, $chunkZ); - if(!isset($chunkTickList[$hash]) && isset($this->chunks[$hash]) && $this->isChunkTickable($chunkX, $chunkZ)){ + if(!isset($chunkTickList[$hash]) && isset($this->chunks[$hash]) && $this->isChunkTickable($chunkX, $chunkZ, $chunkTickableCache)){ $chunkTickList[$hash] = true; } } @@ -1185,14 +1187,29 @@ class World implements ChunkManager{ } } - private function isChunkTickable(int $chunkX, int $chunkZ) : bool{ + /** + * @param bool[] &$cache + * + * @phpstan-param array $cache + * @phpstan-param-out array $cache + */ + private function isChunkTickable(int $chunkX, int $chunkZ, array &$cache) : bool{ for($cx = -1; $cx <= 1; ++$cx){ for($cz = -1; $cz <= 1; ++$cz){ + $chunkHash = World::chunkHash($chunkX + $cx, $chunkZ + $cz); + if(isset($cache[$chunkHash])){ + if(!$cache[$chunkHash]){ + return false; + } + continue; + } if($this->isChunkLocked($chunkX + $cx, $chunkZ + $cz)){ + $cache[$chunkHash] = false; return false; } $adjacentChunk = $this->getChunk($chunkX + $cx, $chunkZ + $cz); if($adjacentChunk === null || !$adjacentChunk->isPopulated()){ + $cache[$chunkHash] = false; return false; } $lightPopulatedState = $adjacentChunk->isLightPopulated(); @@ -1200,8 +1217,11 @@ class World implements ChunkManager{ if($lightPopulatedState === false){ $this->orderLightPopulation($chunkX + $cx, $chunkZ + $cz); } + $cache[$chunkHash] = false; return false; } + + $cache[$chunkHash] = true; } } From 17dde140a5f63593dbadff11c2037bc575477f8e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 22 Dec 2022 15:23:10 +0000 Subject: [PATCH 06/16] Bump phpstan/phpstan-phpunit from 1.3.2 to 1.3.3 (#5465) Bumps [phpstan/phpstan-phpunit](https://github.com/phpstan/phpstan-phpunit) from 1.3.2 to 1.3.3. - [Release notes](https://github.com/phpstan/phpstan-phpunit/releases) - [Commits](https://github.com/phpstan/phpstan-phpunit/compare/1.3.2...1.3.3) --- updated-dependencies: - dependency-name: phpstan/phpstan-phpunit dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- composer.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/composer.lock b/composer.lock index b9626d576..fd1325531 100644 --- a/composer.lock +++ b/composer.lock @@ -1880,16 +1880,16 @@ }, { "name": "phpstan/phpstan-phpunit", - "version": "1.3.2", + "version": "1.3.3", "source": { "type": "git", "url": "https://github.com/phpstan/phpstan-phpunit.git", - "reference": "cd9c6938f8bbfcb6da3ed5a3c7ea60873825d088" + "reference": "54a24bd23e9e80ee918cdc24f909d376c2e273f7" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpstan-phpunit/zipball/cd9c6938f8bbfcb6da3ed5a3c7ea60873825d088", - "reference": "cd9c6938f8bbfcb6da3ed5a3c7ea60873825d088", + "url": "https://api.github.com/repos/phpstan/phpstan-phpunit/zipball/54a24bd23e9e80ee918cdc24f909d376c2e273f7", + "reference": "54a24bd23e9e80ee918cdc24f909d376c2e273f7", "shasum": "" }, "require": { @@ -1926,9 +1926,9 @@ "description": "PHPUnit extensions and rules for PHPStan", "support": { "issues": "https://github.com/phpstan/phpstan-phpunit/issues", - "source": "https://github.com/phpstan/phpstan-phpunit/tree/1.3.2" + "source": "https://github.com/phpstan/phpstan-phpunit/tree/1.3.3" }, - "time": "2022-12-13T15:08:22+00:00" + "time": "2022-12-21T15:25:00+00:00" }, { "name": "phpstan/phpstan-strict-rules", From a9e5f929585a99bc4c8f010751083355d41bf30d Mon Sep 17 00:00:00 2001 From: IvanCraft623 <57236932+IvanCraft623@users.noreply.github.com> Date: Thu, 22 Dec 2022 10:36:31 -0500 Subject: [PATCH 07/16] Show death message on death screen (#5386) --- src/network/mcpe/NetworkSession.php | 4 +-- .../mcpe/handler/DeathPacketHandler.php | 26 ++++++++++++++++++- src/player/Player.php | 2 +- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index 1cd31e180..fcae7e8d8 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -743,9 +743,9 @@ class NetworkSession{ $this->setHandler(new InGamePacketHandler($this->player, $this, $this->invManager)); } - public function onServerDeath() : void{ + public function onServerDeath(Translatable|string $deathMessage) : void{ if($this->handler instanceof InGamePacketHandler){ //TODO: this is a bad fix for pre-spawn death, this shouldn't be reachable at all at this stage :( - $this->setHandler(new DeathPacketHandler($this->player, $this, $this->invManager ?? throw new AssumptionFailedError())); + $this->setHandler(new DeathPacketHandler($this->player, $this, $this->invManager ?? throw new AssumptionFailedError(), $deathMessage)); } } diff --git a/src/network/mcpe/handler/DeathPacketHandler.php b/src/network/mcpe/handler/DeathPacketHandler.php index 0ae5f444f..43deec463 100644 --- a/src/network/mcpe/handler/DeathPacketHandler.php +++ b/src/network/mcpe/handler/DeathPacketHandler.php @@ -23,19 +23,23 @@ declare(strict_types=1); namespace pocketmine\network\mcpe\handler; +use pocketmine\lang\Translatable; use pocketmine\network\mcpe\InventoryManager; use pocketmine\network\mcpe\NetworkSession; use pocketmine\network\mcpe\protocol\ContainerClosePacket; +use pocketmine\network\mcpe\protocol\DeathInfoPacket; use pocketmine\network\mcpe\protocol\PlayerActionPacket; use pocketmine\network\mcpe\protocol\RespawnPacket; use pocketmine\network\mcpe\protocol\types\PlayerAction; use pocketmine\player\Player; +use function array_map; class DeathPacketHandler extends PacketHandler{ public function __construct( private Player $player, private NetworkSession $session, - private InventoryManager $inventoryManager + private InventoryManager $inventoryManager, + private Translatable|string $deathMessage ){} public function setUp() : void{ @@ -44,6 +48,26 @@ class DeathPacketHandler extends PacketHandler{ RespawnPacket::SEARCHING_FOR_SPAWN, $this->player->getId() )); + + /** @var string[] $parameters */ + $parameters = []; + if($this->deathMessage instanceof Translatable){ + //we can't send nested translations to the client, so make sure they are always pre-translated by the server + $language = $this->player->getLanguage(); + $parameters = array_map(fn(string|Translatable $p) => $p instanceof Translatable ? $language->translate($p) : $p, $this->deathMessage->getParameters()); + if(!$this->player->getServer()->isLanguageForced()){ + foreach($parameters as $i => $p){ + $parameters[$i] = $language->translateString($p, [], "pocketmine."); + } + $message = $language->translateString($this->deathMessage->getText(), $parameters, "pocketmine."); + }else{ + $message = $language->translateString($this->deathMessage->getText(), $parameters); + $parameters = []; + } + }else{ + $message = $this->deathMessage; + } + $this->session->sendDataPacket(DeathInfoPacket::create($message, $parameters)); } public function handlePlayerAction(PlayerActionPacket $packet) : bool{ diff --git a/src/player/Player.php b/src/player/Player.php index 8ec47b335..ba40e9f84 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -2300,7 +2300,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ $this->startDeathAnimation(); - $this->getNetworkSession()->onServerDeath(); + $this->getNetworkSession()->onServerDeath($ev->getDeathMessage()); } protected function onDeathUpdate(int $tickDiff) : bool{ From 566a8a261f1ceaa15cc81b5c766fc623b365e62c Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 22 Dec 2022 16:32:54 +0000 Subject: [PATCH 08/16] Player: deprecate sendTranslation() in favour of sendMessage() --- src/player/Player.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/player/Player.php b/src/player/Player.php index ba40e9f84..e24a282c5 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -2003,6 +2003,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ } /** + * @deprecated Use {@link Player::sendMessage()} with a Translatable instead. * @param string[]|Translatable[] $parameters */ public function sendTranslation(string $message, array $parameters = []) : void{ From 9c9929ff39233da1f32ba574568c50f13efd916a Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 22 Dec 2022 16:37:36 +0000 Subject: [PATCH 09/16] Player: break cycle between sendMessage() and sendTranslation() --- src/player/Player.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/player/Player.php b/src/player/Player.php index e24a282c5..a4a516213 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -1995,7 +1995,16 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ */ public function sendMessage(Translatable|string $message) : void{ if($message instanceof Translatable){ - $this->sendTranslation($message->getText(), $message->getParameters()); + //we can't send nested translations to the client, so make sure they are always pre-translated by the server + $parameters = array_map(fn(string|Translatable $p) => $p instanceof Translatable ? $this->getLanguage()->translate($p) : $p, $message->getParameters()); + if(!$this->server->isLanguageForced()){ + foreach($parameters as $i => $p){ + $parameters[$i] = $this->getLanguage()->translateString($p, [], "pocketmine."); + } + $this->getNetworkSession()->onTranslatedChatMessage($this->getLanguage()->translateString($message->getText(), $parameters, "pocketmine."), $parameters); + }else{ + $this->getNetworkSession()->onRawChatMessage($this->getLanguage()->translateString($message->getText(), $parameters)); + } return; } @@ -2007,16 +2016,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ * @param string[]|Translatable[] $parameters */ public function sendTranslation(string $message, array $parameters = []) : void{ - //we can't send nested translations to the client, so make sure they are always pre-translated by the server - $parameters = array_map(fn(string|Translatable $p) => $p instanceof Translatable ? $this->getLanguage()->translate($p) : $p, $parameters); - if(!$this->server->isLanguageForced()){ - foreach($parameters as $i => $p){ - $parameters[$i] = $this->getLanguage()->translateString($p, [], "pocketmine."); - } - $this->getNetworkSession()->onTranslatedChatMessage($this->getLanguage()->translateString($message, $parameters, "pocketmine."), $parameters); - }else{ - $this->sendMessage($this->getLanguage()->translateString($message, $parameters)); - } + $this->sendMessage(new Translatable($message, $parameters)); } /** From b03733442b335a6c235f5776ac135d1edc3d225b Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 22 Dec 2022 16:51:09 +0000 Subject: [PATCH 10/16] Move translation flattening logic from Player to NetworkSession this is network-specific stuff, so it doesn't belong in Player. --- src/network/mcpe/NetworkSession.php | 25 ++++++++++++++++--------- src/player/Player.php | 17 +---------------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index fcae7e8d8..b9c6219fd 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -930,15 +930,22 @@ class NetworkSession{ $this->sendDataPacket(AvailableCommandsPacket::create($commandData, [], [], [])); } - public function onRawChatMessage(string $message) : void{ - $this->sendDataPacket(TextPacket::raw($message)); - } - - /** - * @param string[] $parameters - */ - public function onTranslatedChatMessage(string $key, array $parameters) : void{ - $this->sendDataPacket(TextPacket::translation($key, $parameters)); + public function onChatMessage(Translatable|string $message) : void{ + if($message instanceof Translatable){ + //we can't send nested translations to the client, so make sure they are always pre-translated by the server + $language = $this->player->getLanguage(); + $parameters = array_map(fn(string|Translatable $p) => $p instanceof Translatable ? $language->translate($p) : $p, $message->getParameters()); + if(!$this->server->isLanguageForced()){ + foreach($parameters as $i => $p){ + $parameters[$i] = $language->translateString($p, [], "pocketmine."); + } + $this->sendDataPacket(TextPacket::translation($language->translateString($message->getText(), $parameters, "pocketmine."), $parameters)); + }else{ + $this->sendDataPacket(TextPacket::raw($language->translateString($message->getText(), $parameters))); + } + }else{ + $this->sendDataPacket(TextPacket::raw($message)); + } } /** diff --git a/src/player/Player.php b/src/player/Player.php index a4a516213..1d80d9db6 100644 --- a/src/player/Player.php +++ b/src/player/Player.php @@ -132,7 +132,6 @@ use pocketmine\world\World; use Ramsey\Uuid\UuidInterface; use function abs; use function array_filter; -use function array_map; use function assert; use function count; use function explode; @@ -1994,21 +1993,7 @@ class Player extends Human implements CommandSender, ChunkListener, IPlayer{ * Sends a direct chat message to a player */ public function sendMessage(Translatable|string $message) : void{ - if($message instanceof Translatable){ - //we can't send nested translations to the client, so make sure they are always pre-translated by the server - $parameters = array_map(fn(string|Translatable $p) => $p instanceof Translatable ? $this->getLanguage()->translate($p) : $p, $message->getParameters()); - if(!$this->server->isLanguageForced()){ - foreach($parameters as $i => $p){ - $parameters[$i] = $this->getLanguage()->translateString($p, [], "pocketmine."); - } - $this->getNetworkSession()->onTranslatedChatMessage($this->getLanguage()->translateString($message->getText(), $parameters, "pocketmine."), $parameters); - }else{ - $this->getNetworkSession()->onRawChatMessage($this->getLanguage()->translateString($message->getText(), $parameters)); - } - return; - } - - $this->getNetworkSession()->onRawChatMessage($message); + $this->getNetworkSession()->onChatMessage($message); } /** From 2a33c9ed3baf83a77e3cd293771ad8e1d018e9bb Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 22 Dec 2022 16:53:14 +0000 Subject: [PATCH 11/16] Fix PHPStan --- tests/phpstan/configs/actual-problems.neon | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index 0afae17b6..1d5e3dfb3 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -627,7 +627,7 @@ parameters: - message: "#^Cannot call method getLanguage\\(\\) on pocketmine\\\\player\\\\Player\\|null\\.$#" - count: 1 + count: 2 path: ../../../src/network/mcpe/NetworkSession.php - From 7d1d62042c60a6fb76f8571edb2dbce3444ce4de Mon Sep 17 00:00:00 2001 From: Dylan T Date: Thu, 22 Dec 2022 18:13:03 +0000 Subject: [PATCH 12/16] attempted fix for GitHub rate limiting php-cs-fixer installation from https://github.com/shivammathur/setup-php/issues/678#issuecomment-1363128626 --- .github/workflows/main.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 300f57eb3..653a56496 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -199,6 +199,8 @@ jobs: with: php-version: 8.0 tools: php-cs-fixer:3.11 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Run PHP-CS-Fixer run: php-cs-fixer fix --dry-run --diff --ansi From 5d2b0acfc862f44f4cde96d1d705a1ede4c8e4fd Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 22 Dec 2022 18:38:10 +0000 Subject: [PATCH 13/16] GamemodeCommand: report failure if the target's game mode is already the desired game mode this has irritated me for years. --- src/command/defaults/GamemodeCommand.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/command/defaults/GamemodeCommand.php b/src/command/defaults/GamemodeCommand.php index 1363d34dc..81d4fd3f8 100644 --- a/src/command/defaults/GamemodeCommand.php +++ b/src/command/defaults/GamemodeCommand.php @@ -72,6 +72,11 @@ class GamemodeCommand extends VanillaCommand{ throw new InvalidCommandSyntaxException(); } + if($target->getGamemode()->equals($gameMode)){ + $sender->sendMessage(KnownTranslationFactory::pocketmine_command_gamemode_failure($target->getName())); + return true; + } + $target->setGamemode($gameMode); if(!$gameMode->equals($target->getGamemode())){ $sender->sendMessage(KnownTranslationFactory::pocketmine_command_gamemode_failure($target->getName())); From 43e69041fc148d325e38e736754d22baa0c4f50a Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Fri, 23 Dec 2022 16:00:38 +0000 Subject: [PATCH 14/16] PermissionParser: use constants for keys --- src/permission/PermissionParser.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/permission/PermissionParser.php b/src/permission/PermissionParser.php index d498b9315..cc54f270e 100644 --- a/src/permission/PermissionParser.php +++ b/src/permission/PermissionParser.php @@ -53,6 +53,10 @@ class PermissionParser{ "false" => self::DEFAULT_FALSE, ]; + private const KEY_DEFAULT = "default"; + private const KEY_CHILDREN = "children"; + private const KEY_DESCRIPTION = "description"; + /** * @param bool|string $value * @@ -86,16 +90,16 @@ class PermissionParser{ $result = []; foreach(Utils::stringifyKeys($data) as $name => $entry){ $desc = null; - if(isset($entry["default"])){ - $default = PermissionParser::defaultFromString($entry["default"]); + if(isset($entry[self::KEY_DEFAULT])){ + $default = PermissionParser::defaultFromString($entry[self::KEY_DEFAULT]); } - if(isset($entry["children"])){ + if(isset($entry[self::KEY_CHILDREN])){ throw new PermissionParserException("Nested permission declarations are no longer supported. Declare each permission separately."); } - if(isset($entry["description"])){ - $desc = $entry["description"]; + if(isset($entry[self::KEY_DESCRIPTION])){ + $desc = $entry[self::KEY_DESCRIPTION]; } $result[$default][] = new Permission($name, $desc); From 51a684c0ea9b7a41f9c7c646a113cd9be981a6e9 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Fri, 23 Dec 2022 16:03:01 +0000 Subject: [PATCH 15/16] PermissionParser: remove hardcoded default strings in defaultFromString() --- src/permission/PermissionParser.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/permission/PermissionParser.php b/src/permission/PermissionParser.php index cc54f270e..d1a901e67 100644 --- a/src/permission/PermissionParser.php +++ b/src/permission/PermissionParser.php @@ -64,11 +64,7 @@ class PermissionParser{ */ public static function defaultFromString($value) : string{ if(is_bool($value)){ - if($value){ - return "true"; - }else{ - return "false"; - } + return $value ? self::DEFAULT_TRUE : self::DEFAULT_FALSE; } $lower = strtolower($value); if(isset(self::DEFAULT_STRING_MAP[$lower])){ From 3987ee6cb242a28800881dccc84c29e9bdc3a562 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Fri, 23 Dec 2022 16:11:03 +0000 Subject: [PATCH 16/16] PluginDescription: const-ify plugin.yml parsing keys --- src/plugin/PluginDescription.php | 98 ++++++++++++++++++++------------ 1 file changed, 62 insertions(+), 36 deletions(-) diff --git a/src/plugin/PluginDescription.php b/src/plugin/PluginDescription.php index 7574d7297..614fb4d08 100644 --- a/src/plugin/PluginDescription.php +++ b/src/plugin/PluginDescription.php @@ -37,6 +37,32 @@ use function stripos; use function yaml_parse; class PluginDescription{ + private const KEY_NAME = "name"; + private const KEY_VERSION = "version"; + private const KEY_MAIN = "main"; + private const KEY_SRC_NAMESPACE_PREFIX = "src-namespace-prefix"; + private const KEY_API = "api"; + private const KEY_MCPE_PROTOCOL = "mcpe-protocol"; + private const KEY_OS = "os"; + private const KEY_DEPEND = "depend"; + private const KEY_SOFTDEPEND = "softdepend"; + private const KEY_LOADBEFORE = "loadbefore"; + private const KEY_EXTENSIONS = "extensions"; + private const KEY_WEBSITE = "website"; + private const KEY_DESCRIPTION = "description"; + private const KEY_LOGGER_PREFIX = "prefix"; + private const KEY_LOAD = "load"; + private const KEY_AUTHOR = "author"; + private const KEY_AUTHORS = "authors"; + private const KEY_PERMISSIONS = "permissions"; + + private const KEY_COMMANDS = "commands"; + private const KEY_COMMAND_PERMISSION = "permission"; + private const KEY_COMMAND_DESCRIPTION = self::KEY_DESCRIPTION; + private const KEY_COMMAND_USAGE = "usage"; + private const KEY_COMMAND_ALIASES = "aliases"; + private const KEY_COMMAND_PERMISSION_MESSAGE = "permission-message"; + /** * @var mixed[] * @phpstan-var array @@ -107,49 +133,49 @@ class PluginDescription{ private function loadMap(array $plugin) : void{ $this->map = $plugin; - $this->name = $plugin["name"]; + $this->name = $plugin[self::KEY_NAME]; if(preg_match('/^[A-Za-z0-9 _.-]+$/', $this->name) === 0){ throw new PluginDescriptionParseException("Invalid Plugin name"); } $this->name = str_replace(" ", "_", $this->name); - $this->version = (string) $plugin["version"]; - $this->main = $plugin["main"]; + $this->version = (string) $plugin[self::KEY_VERSION]; + $this->main = $plugin[self::KEY_MAIN]; if(stripos($this->main, "pocketmine\\") === 0){ throw new PluginDescriptionParseException("Invalid Plugin main, cannot start within the PocketMine namespace"); } - $this->srcNamespacePrefix = $plugin["src-namespace-prefix"] ?? ""; + $this->srcNamespacePrefix = $plugin[self::KEY_SRC_NAMESPACE_PREFIX] ?? ""; - $this->api = array_map("\strval", (array) ($plugin["api"] ?? [])); - $this->compatibleMcpeProtocols = array_map("\intval", (array) ($plugin["mcpe-protocol"] ?? [])); - $this->compatibleOperatingSystems = array_map("\strval", (array) ($plugin["os"] ?? [])); + $this->api = array_map("\strval", (array) ($plugin[self::KEY_API] ?? [])); + $this->compatibleMcpeProtocols = array_map("\intval", (array) ($plugin[self::KEY_MCPE_PROTOCOL] ?? [])); + $this->compatibleOperatingSystems = array_map("\strval", (array) ($plugin[self::KEY_OS] ?? [])); - if(isset($plugin["commands"]) && is_array($plugin["commands"])){ - foreach($plugin["commands"] as $commandName => $commandData){ + if(isset($plugin[self::KEY_COMMANDS]) && is_array($plugin[self::KEY_COMMANDS])){ + foreach($plugin[self::KEY_COMMANDS] as $commandName => $commandData){ if(!is_string($commandName)){ throw new PluginDescriptionParseException("Invalid Plugin commands, key must be the name of the command"); } if(!is_array($commandData)){ throw new PluginDescriptionParseException("Command $commandName has invalid properties"); } - if(!isset($commandData["permission"]) || !is_string($commandData["permission"])){ + if(!isset($commandData[self::KEY_COMMAND_PERMISSION]) || !is_string($commandData[self::KEY_COMMAND_PERMISSION])){ throw new PluginDescriptionParseException("Command $commandName does not have a valid permission set"); } $this->commands[$commandName] = new PluginDescriptionCommandEntry( - $commandData["description"] ?? null, - $commandData["usage"] ?? null, - $commandData["aliases"] ?? [], - $commandData["permission"], - $commandData["permission-message"] ?? null + $commandData[self::KEY_COMMAND_DESCRIPTION] ?? null, + $commandData[self::KEY_COMMAND_USAGE] ?? null, + $commandData[self::KEY_COMMAND_ALIASES] ?? [], + $commandData[self::KEY_COMMAND_PERMISSION], + $commandData[self::KEY_COMMAND_PERMISSION_MESSAGE] ?? null ); } } - if(isset($plugin["depend"])){ - $this->depend = (array) $plugin["depend"]; + if(isset($plugin[self::KEY_DEPEND])){ + $this->depend = (array) $plugin[self::KEY_DEPEND]; } - if(isset($plugin["extensions"])){ - $extensions = (array) $plugin["extensions"]; + if(isset($plugin[self::KEY_EXTENSIONS])){ + $extensions = (array) $plugin[self::KEY_EXTENSIONS]; $isLinear = $extensions === array_values($extensions); foreach($extensions as $k => $v){ if($isLinear){ @@ -160,20 +186,20 @@ class PluginDescription{ } } - $this->softDepend = (array) ($plugin["softdepend"] ?? $this->softDepend); + $this->softDepend = (array) ($plugin[self::KEY_SOFTDEPEND] ?? $this->softDepend); - $this->loadBefore = (array) ($plugin["loadbefore"] ?? $this->loadBefore); + $this->loadBefore = (array) ($plugin[self::KEY_LOADBEFORE] ?? $this->loadBefore); - $this->website = (string) ($plugin["website"] ?? $this->website); + $this->website = (string) ($plugin[self::KEY_WEBSITE] ?? $this->website); - $this->description = (string) ($plugin["description"] ?? $this->description); + $this->description = (string) ($plugin[self::KEY_DESCRIPTION] ?? $this->description); - $this->prefix = (string) ($plugin["prefix"] ?? $this->prefix); + $this->prefix = (string) ($plugin[self::KEY_LOGGER_PREFIX] ?? $this->prefix); - if(isset($plugin["load"])){ - $order = PluginEnableOrder::fromString($plugin["load"]); + if(isset($plugin[self::KEY_LOAD])){ + $order = PluginEnableOrder::fromString($plugin[self::KEY_LOAD]); if($order === null){ - throw new PluginDescriptionParseException("Invalid Plugin \"load\""); + throw new PluginDescriptionParseException("Invalid Plugin \"" . self::KEY_LOAD . "\""); } $this->order = $order; }else{ @@ -181,24 +207,24 @@ class PluginDescription{ } $this->authors = []; - if(isset($plugin["author"])){ - if(is_array($plugin["author"])){ - $this->authors = $plugin["author"]; + if(isset($plugin[self::KEY_AUTHOR])){ + if(is_array($plugin[self::KEY_AUTHOR])){ + $this->authors = $plugin[self::KEY_AUTHOR]; }else{ - $this->authors[] = $plugin["author"]; + $this->authors[] = $plugin[self::KEY_AUTHOR]; } } - if(isset($plugin["authors"])){ - foreach($plugin["authors"] as $author){ + if(isset($plugin[self::KEY_AUTHORS])){ + foreach($plugin[self::KEY_AUTHORS] as $author){ $this->authors[] = $author; } } - if(isset($plugin["permissions"])){ + if(isset($plugin[self::KEY_PERMISSIONS])){ try{ - $this->permissions = PermissionParser::loadPermissions($plugin["permissions"]); + $this->permissions = PermissionParser::loadPermissions($plugin[self::KEY_PERMISSIONS]); }catch(PermissionParserException $e){ - throw new PluginDescriptionParseException("Invalid Plugin \"permissions\": " . $e->getMessage(), 0, $e); + throw new PluginDescriptionParseException("Invalid Plugin \"" . self::KEY_PERMISSIONS . "\": " . $e->getMessage(), 0, $e); } } }