Merge branch 'stable' into minor-next

CONTRIBUTING.md

@@ -88,45 +88,58 @@ Depending on the changes, maintainers might ask you to make changes to the PR to
 ### Requirements
 The following are required as a minimum for pull requests. PRs that don't meet these requirements will be declined unless updated to meet them.
 
-#### Licensing
+- **All code must be licensed under the [LGPLv3 license](LICENSE)** as per PocketMine-MP's own license, or a compatible license.
-PocketMine-MP is licensed under [LGPLv3 license](LICENSE).
+  - By proposing a pull request, you agree to your code being distributed within PocketMine-MP under the same license.
-By proposing a pull request, you agree to your code being distributed within PocketMine-MP under the same license.
+  - If you take code from other projects, that code MUST be licensed under an LGPL-compatible license.
-If you take code from other projects, that code MUST be licensed under an LGPL-compatible license.
+- **PRs should be about ONE thing**
-
+  - If you want to make multiple changes, those changes should each be contributed as separate pull requests. **DO NOT** mix unrelated changes.
-#### PRs should be about exactly ONE thing
+  - **Do not include unnecessary changes.** This makes the code diff larger and more noisy, making it harder to review.
-If you want to make multiple changes, those changes should each be contributed as separate pull requests. **DO NOT** mix unrelated changes.
+    - Don't change things that aren't related to the PR's objective
-
+    - Don't reformat or rearrange existing code without a good reason related to the PR's objective
-#### PRs must not include unnecessary/unrelated changes
+    - Don't rewrite existing code just to make it "look nicer"
-Do not include changes which aren't strictly necessary. This makes it harder to review a PR, because the code diff becomes larger and harder to review.
+    - Don't change PhpDocs to native types in code you didn't write, unless that's the objective of the PR
-This means:
+- **Test code changes, and tell us what tests have been done.**
-- don't reformat or rearrange existing code
+  - Where possible, PHPUnit tests should be written for new or changed code. If that's not possible (e.g. for in-game functionality), the code must be tested manually and details of the tests done must be provided.
-- don't change things that aren't related to the PR's objective
+  - **Simply saying "Tested" is not acceptable** and could lead to your PR being declined.
-- don't rewrite existing code just to make it "look nicer"
+- **Code, comments and documentation must be written in American English.** English is the shared languages of all current maintainers.
-- don't change PhpDocs to native types in code you didn't write
+- **Code must be in the PocketMine-MP style.**
-
+  - It's your responsibility to ensure your code matches the formatting and styling of the rest of the code.
-#### Tests must be provided
+  - If you use PhpStorm, a `Project` code style is provided, which you can use to automatically format new code.
-Where possible, PHPUnit tests should be written for new or changed code.
+  - You can also use [`php-cs-fixer`](https://github.com/FriendsOfPHP/PHP-CS-Fixer) to format your code.
-If that's not possible (e.g. for in-game functionality), the code must be tested manually and details of the tests done must be provided.
+- **Use `final` and `private` wherever possible**.
-**Simply saying "Tested" is not acceptable** and will lead to your PR being declined.
+  - Changing from `private` to `protected` or `final` to non-`final` doesn't break backwards compatibility, but the opposite does.
-
+  - `private` and `final` also enable certain performance optimizations which are otherwise not possible.
-#### Comments and documentation must be written in American English
+  - `private` members can be freely changed, added and removed in the future, so it's ideal for internal functions. Abusing `protected` makes internal improvements inconvenient.
-English is the shared languages of all current maintainers.
+  - "Let's leave it protected/public in case someone needs it for ... idk what" is **not a valid reason to expose things**. If there isn't a clear reason for something to be accessible from the outside, don't expose it.
-
+  - **This is a lesson learned through years of experience.** You may not like it, but it's for the best.
-#### Code must be in the PocketMine-MP style
+- **Immutable things are almost always preferred.**
-It's your responsibility to ensure your code matches the formatting and styling of the rest of the code.
+  - Do not add unnecessary setters or public writable properties to classes. As above, "Let's leave it in case someone needs it" is **not a valid reason to expose things**.
-If you use PhpStorm, a `Project` code style is provided, which you can use to automatically format new code.
+  - Mutable classes and properties are unpredictable, since code has no way to know if the object it's working with might be randomly modified by another part of the code. This makes it harder to maintain code and debug issues.
-You can also use [`php-cs-fixer`](https://github.com/FriendsOfPHP/PHP-CS-Fixer) to format your code.
+  - Most classes exist only to hold some data. These are called "data transfer objects" (DTOs). These types of classes should pretty much always be immutable.
+  - Make use of `final`, `private` and `readonly` modifiers.
 
 ### Recommendations
-
+- **Be patient.** Reviewing pull requests takes a lot of time and energy, and maintainers are often unavailable or busy. Your PR might not receive attention for a while.
+  - Remember, PRs with small diffs are much easier to review. Small PRs are generally reviewed and merged much faster than large ones.
+- **Start small.** Try fixing minor bugs or doing something isolated (e.g. adding a new block or item) before attempting larger changes.
+  - This helps you get familiar with the codebase, the contribution process, and the expectations of maintainers.
+  - Check out the [issues page]() for something that you could tackle without too much effort.
+- **Do not copy-paste other people's code**. Many PRs involve discussion about the changes, and changes are often requested by reviewers. If you don't understand the code you're copy-pasting, your PR is likely to fail.
 - **Do not edit code directly on github.com.** We recommend learning how to use [`git`](https://git-scm.com). `git` allows you to "clone" a repository onto your computer, so that you can make changes using an IDE.
 - **Use an IDE, not a text editor.** We recommend PhpStorm or VSCode.
+- **Do not make large pull requests without an RFC.**
+  - Large changes should be discussed beforehand using the [RFC / Change Proposal](#rfcs--change-proposals) process.
+  - Large changes are much harder to review, and are more likely to be declined if maintainers don't have a good idea what you're trying to do in advance.
 - **Create a new branch on your fork for each pull request.** This allows you to use the same fork to make multiple pull requests at the same time.
+- **Make your PR diff as small as possible.** Smaller PRs are **much more likely** to be accepted, as they are easier to review.
+  - Avoid moving code around in files if possible.
+  - Don't make random CS changes. This makes the diff noisier and harder to review.
 - **Use descriptive commit titles.** You can see an example [here](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html).
-- **Do not include multiple unrelated changes in one commit.** An atomic style for commits is preferred - this means that changes included in a commit should be part of a single distinct change set. See [this link](https://www.freshconsulting.com/atomic-commits/) for more information on atomic commits. See the [documentation on `git add`](https://git-scm.com/docs/git-add) for information on how to isolate local changes for committing.
+- **Split unrelated changes into multiple commits.**
-- **Your pull request will be checked and discussed in due time.** Since the team is scattered all around the world, your PR may not receive any attention for some time.
+  - An atomic style for commits is preferred - this means that changes included in a commit should be part of a single distinct change set.
-- **Do not make large pull requests without an RFC.** Large changes should be discussed beforehand using the [RFC / Change Proposal](#rfcs--change-proposals) process. Large changes are much harder to review and are more likely to be declined if maintainers don't have a good idea what you're trying to do in advance.
+  - If you need to use "and" or "multiple changes" in your commit message, the commit probably needs to be split up. There are exceptions, but this is a good rule of thumb.
-- **Do not copy-paste code**. There are potential license issues implicit with copy-pasting, and copy-paste usually indicates a lack of understanding of the actual code. Copy-pasted code is obvious a mile off and **any PR like this is likely to be closed**. If you want to use somebody else's code from a Git repository, **use [GIT's cherry-pick feature](https://git-scm.com/docs/git-cherry-pick)** to cherry-pick the commit.
+  - See [this link](https://www.freshconsulting.com/atomic-commits/) for more information on atomic commits.
+  - See the [documentation on `git add -i` or `git add -p`](https://git-scm.com/docs/git-add) for information on how to split up local changes for committing.
 
 
 **Thanks for contributing to PocketMine-MP!**

changelogs/5.6.md

@@ -18,3 +18,17 @@ Consider using the `mcpe-protocol` directive in `plugin.yml` as a constraint if
 ## Fixes
 - Fixed support conditions for hanging roots, cave vines and dead bushes.
 - Fixed connection conditions for fences, glass panes, iron bars, and walls.
+
+# 5.6.1
+Released 20th October 2023.
+
+## Performance
+- Improved performance of cactus growth by disabling neighbour updates when only the age property was updated. While this isn't a perfect solution, it provides significant performance gains for servers with large cactus farms.
+
+## Fixes
+- Fixed `tools/generate-bedrock-data-from-packets.php` incorrectly interpreting network meta as blockstates in some cases (broken crafting recipes).
+- Fixed crafting recipes involving beds, skulls and some other items not working correctly (incorrectly interpreted data).
+- Fixed crashes when flower pot or cauldron blockentities exist in places where they shouldn't (leftovers from upgraded PM3 worlds).
+- Fixed `Entity->broadcastSound()` not firing `WorldSoundEvent` (bypassing internal sound system).
+- Fixed wooden signs, buttons and doors not being able to be used as furnace fuel.
+- Fixed bone meal and tools only working when used on the top side of dirt and grass. Bone meal now works from any side, and tools work on any side except the bottom.

composer.json

@@ -52,7 +52,7 @@
       "symfony/filesystem": "~6.3.0"
    },
    "require-dev": {
-      "phpstan/phpstan": "1.10.38",
+      "phpstan/phpstan": "1.10.39",
       "phpstan/phpstan-phpunit": "^1.1.0",
       "phpstan/phpstan-strict-rules": "^1.2.0",
       "phpunit/phpunit": "~10.3.0 || ~10.2.0 || ~10.1.0"

composer.lock

@@ -4,7 +4,7 @@
         "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
         "This file is @generated automatically"
     ],
-    "content-hash": "2282bf7835c1ced757460b083813e092",
+    "content-hash": "6c48ad06b67c3aa3890f004b197d17bc",
     "packages": [
         {
             "name": "adhocore/json-comment",
@@ -1378,16 +1378,16 @@
         },
         {
             "name": "phpstan/phpstan",
-            "version": "1.10.38",
+            "version": "1.10.39",
             "source": {
                 "type": "git",
                 "url": "https://github.com/phpstan/phpstan.git",
-                "reference": "5302bb402c57f00fb3c2c015bac86e0827e4b691"
+                "reference": "d9dedb0413f678b4d03cbc2279a48f91592c97c4"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/phpstan/phpstan/zipball/5302bb402c57f00fb3c2c015bac86e0827e4b691",
+                "url": "https://api.github.com/repos/phpstan/phpstan/zipball/d9dedb0413f678b4d03cbc2279a48f91592c97c4",
-                "reference": "5302bb402c57f00fb3c2c015bac86e0827e4b691",
+                "reference": "d9dedb0413f678b4d03cbc2279a48f91592c97c4",
                 "shasum": ""
             },
             "require": {
@@ -1436,7 +1436,7 @@
                     "type": "tidelift"
                 }
             ],
-            "time": "2023-10-06T14:19:14+00:00"
+            "time": "2023-10-17T15:46:26+00:00"
         },
         {
             "name": "phpstan/phpstan-phpunit",
@@ -2964,5 +2964,5 @@
     "platform-overrides": {
         "php": "8.1.0"
     },
-    "plugin-api-version": "2.6.0"
+    "plugin-api-version": "2.3.0"
 }

src/Server.php

@@ -570,6 +570,7 @@ class Server{
 		$playerPromiseResolver = new PromiseResolver();
 
 		$createPlayer = function(Location $location) use ($playerPromiseResolver, $class, $session, $playerInfo, $authenticated, $offlinePlayerData) : void{
+			/** @see Player::__construct() */
 			$player = new $class($this, $session, $playerInfo, $authenticated, $location, $offlinePlayerData);
 			if(!$player->hasPlayedBefore()){
 				$player->onGround = true; //TODO: this hack is needed for new players in-air ticks - they don't get detected as on-ground until they move

src/VersionInfo.php

@@ -31,7 +31,7 @@ use function str_repeat;
 
 final class VersionInfo{
 	public const NAME = "PocketMine-MP";
-	public const BASE_VERSION = "5.6.1";
+	public const BASE_VERSION = "5.6.2";
 	public const IS_DEVELOPMENT_BUILD = true;
 	public const BUILD_CHANNEL = "stable";
 

src/block/BaseSign.php

@@ -271,4 +271,8 @@ abstract class BaseSign extends Transparent{
 	public function asItem() : Item{
 		return ($this->asItemCallback)();
 	}
+
+	public function getFuelTime() : int{
+		return $this->woodType->isFlammable() ? 200 : 0;
+	}
 }

src/block/Dirt.php

@@ -54,7 +54,12 @@ class Dirt extends Opaque{
 
 	public function onInteract(Item $item, int $face, Vector3 $clickVector, ?Player $player = null, array &$returnedItems = []) : bool{
 		$world = $this->position->getWorld();
-		if($face === Facing::UP && $item instanceof Hoe){
+		if($face !== Facing::DOWN && $item instanceof Hoe){
+			$up = $this->getSide(Facing::UP);
+			if($up->getTypeId() !== BlockTypeIds::AIR){
+				return true;
+			}
+
 			$item->applyDamage(1);
 
 			$newBlock = $this->dirtType === DirtType::NORMAL ? VanillaBlocks::FARMLAND() : VanillaBlocks::DIRT();

src/block/Grass.php

@@ -82,7 +82,7 @@ class Grass extends Opaque{
 	}
 
 	public function onInteract(Item $item, int $face, Vector3 $clickVector, ?Player $player = null, array &$returnedItems = []) : bool{
-		if($face !== Facing::UP){
+		if($this->getSide(Facing::UP)->getTypeId() !== BlockTypeIds::AIR){
 			return false;
 		}
 		$world = $this->position->getWorld();
@@ -91,14 +91,16 @@ class Grass extends Opaque{
 			TallGrassObject::growGrass($world, $this->position, new Random(mt_rand()), 8, 2);
 
 			return true;
-		}elseif($item instanceof Hoe){
+		}
+		if($face !== Facing::DOWN){
+			if($item instanceof Hoe){
 				$item->applyDamage(1);
 				$newBlock = VanillaBlocks::FARMLAND();
 				$world->addSound($this->position->add(0.5, 0.5, 0.5), new ItemUseOnBlockSound($newBlock));
 				$world->setBlock($this->position, $newBlock);
 
 				return true;
-		}elseif($item instanceof Shovel && $this->getSide(Facing::UP)->getTypeId() === BlockTypeIds::AIR){
+			}elseif($item instanceof Shovel){
 				$item->applyDamage(1);
 				$newBlock = VanillaBlocks::GRASS_PATH();
 				$world->addSound($this->position->add(0.5, 0.5, 0.5), new ItemUseOnBlockSound($newBlock));
@@ -106,6 +108,7 @@ class Grass extends Opaque{
 
 				return true;
 			}
+		}
 
 		return false;
 	}

src/block/WoodenButton.php

@@ -35,4 +35,8 @@ class WoodenButton extends Button{
 	public function hasEntityCollision() : bool{
 		return false; //TODO: arrows activate wooden buttons
 	}
+
+	public function getFuelTime() : int{
+		return $this->woodType->isFlammable() ? 100 : 0;
+	}
 }

src/block/WoodenDoor.php

@@ -27,4 +27,8 @@ use pocketmine\block\utils\WoodTypeTrait;
 
 class WoodenDoor extends Door{
 	use WoodTypeTrait;
+
+	public function getFuelTime() : int{
+		return $this->woodType->isFlammable() ? 200 : 0;
+	}
 }

src/item/StringToItemParser.php

@@ -209,6 +209,7 @@ final class StringToItemParser extends StringToTParser{
 		$result->registerBlock("carpet", fn() => Blocks::CARPET());
 		$result->registerBlock("carrot_block", fn() => Blocks::CARROTS());
 		$result->registerBlock("carrots", fn() => Blocks::CARROTS());
+		$result->registerBlock("cartography_table", fn() => Blocks::CARTOGRAPHY_TABLE());
 		$result->registerBlock("carved_pumpkin", fn() => Blocks::CARVED_PUMPKIN());
 		$result->registerBlock("cauldron", fn() => Blocks::CAULDRON());
 		$result->registerBlock("cave_vines", fn() => Blocks::CAVE_VINES());
@@ -984,6 +985,7 @@ final class StringToItemParser extends StringToTParser{
 		$result->registerBlock("slime_block", fn() => Blocks::SLIME());
 		$result->registerBlock("small_amethyst_bud", fn() => Blocks::AMETHYST_CLUSTER()->setStage(AmethystCluster::STAGE_SMALL_BUD));
 		$result->registerBlock("small_dripleaf", fn() => Blocks::SMALL_DRIPLEAF());
+		$result->registerBlock("smithing_table", fn() => Blocks::SMITHING_TABLE());
 		$result->registerBlock("smoker", fn() => Blocks::SMOKER());
 		$result->registerBlock("smooth_basalt", fn() => Blocks::SMOOTH_BASALT());
 		$result->registerBlock("smooth_quartz", fn() => Blocks::SMOOTH_QUARTZ());
@@ -1056,6 +1058,8 @@ final class StringToItemParser extends StringToTParser{
 		$result->registerBlock("stripped_acacia_wood", fn() => Blocks::ACACIA_WOOD()->setStripped(true));
 		$result->registerBlock("stripped_birch_log", fn() => Blocks::BIRCH_LOG()->setStripped(true));
 		$result->registerBlock("stripped_birch_wood", fn() => Blocks::BIRCH_WOOD()->setStripped(true));
+		$result->registerBlock("stripped_cherry_log", fn() => Blocks::CHERRY_LOG()->setStripped(true));
+		$result->registerBlock("stripped_cherry_wood", fn() => Blocks::CHERRY_WOOD()->setStripped(true));
 		$result->registerBlock("stripped_crimson_hyphae", fn() => Blocks::CRIMSON_HYPHAE()->setStripped(true));
 		$result->registerBlock("stripped_crimson_stem", fn() => Blocks::CRIMSON_STEM()->setStripped(true));
 		$result->registerBlock("stripped_dark_oak_log", fn() => Blocks::DARK_OAK_LOG()->setStripped(true));

src/promise/Promise.php

@@ -41,8 +41,11 @@ final class Promise{
 	 * @phpstan-param \Closure() : void $onFailure
 	 */
 	public function onCompletion(\Closure $onSuccess, \Closure $onFailure) : void{
-		if($this->shared->resolved){
+		$state = $this->shared->state;
-			$this->shared->result === null ? $onFailure() : $onSuccess($this->shared->result);
+		if($state === true){
+			$onSuccess($this->shared->result);
+		}elseif($state === false){
+			$onFailure();
 		}else{
 			$this->shared->onSuccess[spl_object_id($onSuccess)] = $onSuccess;
 			$this->shared->onFailure[spl_object_id($onFailure)] = $onFailure;
@@ -50,6 +53,8 @@ final class Promise{
 	}
 
 	public function isResolved() : bool{
-		return $this->shared->resolved;
+		//TODO: perhaps this should return true when rejected? currently there's no way to tell if a promise was
+		//rejected or just hasn't been resolved yet
+		return $this->shared->state === true;
 	}
 }

src/promise/PromiseResolver.php

@@ -41,10 +41,10 @@ final class PromiseResolver{
 	 * @phpstan-param TValue $value
 	 */
 	public function resolve(mixed $value) : void{
-		if($this->shared->resolved){
+		if($this->shared->state !== null){
 			throw new \LogicException("Promise has already been resolved/rejected");
 		}
-		$this->shared->resolved = true;
+		$this->shared->state = true;
 		$this->shared->result = $value;
 		foreach($this->shared->onSuccess as $c){
 			$c($value);
@@ -54,10 +54,10 @@ final class PromiseResolver{
 	}
 
 	public function reject() : void{
-		if($this->shared->resolved){
+		if($this->shared->state !== null){
 			throw new \LogicException("Promise has already been resolved/rejected");
 		}
-		$this->shared->resolved = true;
+		$this->shared->state = false;
 		foreach($this->shared->onFailure as $c){
 			$c();
 		}

src/promise/PromiseSharedData.php

@@ -41,8 +41,8 @@ final class PromiseSharedData{
 	 */
 	public array $onFailure = [];
 
-	public bool $resolved = false;
+	public ?bool $state = null;
 
-	/** @phpstan-var TValue|null */
+	/** @phpstan-var TValue */
-	public mixed $result = null;
+	public mixed $result;
 }

src/world/World.php

@@ -1097,7 +1097,9 @@ class World implements ChunkManager{
 			$blockPosition = BlockPosition::fromVector3($b);
 
 			$tile = $this->getTileAt($b->x, $b->y, $b->z);
-			if($tile instanceof Spawnable && count($fakeStateProperties = $tile->getRenderUpdateBugWorkaroundStateProperties($fullBlock)) > 0){
+			if($tile instanceof Spawnable){
+				$expectedClass = $fullBlock->getIdInfo()->getTileClass();
+				if($expectedClass !== null && $tile instanceof $expectedClass && count($fakeStateProperties = $tile->getRenderUpdateBugWorkaroundStateProperties($fullBlock)) > 0){
 					$originalStateData = $blockTranslator->internalIdToNetworkStateData($fullBlock->getStateId());
 					$fakeStateData = new BlockStateData(
 						$originalStateData->getName(),
@@ -1111,6 +1113,7 @@ class World implements ChunkManager{
 						UpdateBlockPacket::DATA_LAYER_NORMAL
 					);
 				}
+			}
 			$packets[] = UpdateBlockPacket::create(
 				$blockPosition,
 				$blockTranslator->internalIdToNetworkId($fullBlock->getStateId()),
@@ -2891,18 +2894,10 @@ class World implements ChunkManager{
 					$logger->error("Cannot add tile with position outside the world bounds: x=$tilePosition->x,y=$tilePosition->y,z=$tilePosition->z");
 				}elseif($this->getTile($tilePosition) !== null){
 					$logger->error("Cannot add tile at x=$tilePosition->x,y=$tilePosition->y,z=$tilePosition->z: Another tile is already at that position");
-				}else{
-					$block = $this->getBlockAt($tilePosition->getFloorX(), $tilePosition->getFloorY(), $tilePosition->getFloorZ());
-					$expectedClass = $block->getIdInfo()->getTileClass();
-					if($expectedClass === null){
-						$logger->error("Cannot add tile at x=$tilePosition->x,y=$tilePosition->y,z=$tilePosition->z: Block at that position (" . $block->getName() . ") does not expect a tile");
-					}elseif(!($tile instanceof $expectedClass)){
-						$logger->error("Cannot add tile at x=$tilePosition->x,y=$tilePosition->y,z=$tilePosition->z: Tile is of wrong type (expected $expectedClass but have " . get_class($tile) . ")");
 				}else{
 					$this->addTile($tile);
 				}
 			}
-			}
 
 			$this->timings->syncChunkLoadTileEntities->stopTiming();
 		}

tests/phpunit/promise/PromiseTest.php

@@ -0,0 +1,42 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\promise;
+
+use PHPUnit\Framework\TestCase;
+
+final class PromiseTest extends TestCase{
+
+	public function testPromiseNull() : void{
+		$resolver = new PromiseResolver();
+		$resolver->resolve(null);
+		$resolver->getPromise()->onCompletion(
+			function(mixed $value) : void{
+				self::assertNull($value);
+			},
+			function() : void{
+				self::fail("Promise should not be rejected");
+			}
+		);
+	}
+}