Add Structure Block and Structure Void #6045

Bqleine · 2023-09-08T19:58:03Z

Introduction

This pull request is looking to implement the structure void, and possibly the structure block as it is closely related.

I personally don't have the knowledge required to implement the full structure block, but I would like to get it working at least as an inanimate place-able block.

Relevant issues

Implemented Structure Void #5423: A previous attempt at implementing structure void
https://github.com/pmmp/PocketMine-MP/projects/14#card-49457870
https://github.com/pmmp/PocketMine-MP/projects/14#card-84464854

Changes

This is a non-disruptive addition of blocks.

Tasks

Register structure void
- Define block states (defaults as void)
- Implement serialization
Register structure block
- Implement interactions with the block
- Define block states (defaults as save mode)
- Implement serialization
- Create tile

Left for a future PR:

Add events related to structure block modification?
Create permission for using structure block?
Plugin API
saving/loading of structures to disk (probably better left for another PR for full structure API)
Link block and tile type
Unit tests

Bugs

Falling blocks landing inside a structure void should replace it instead of dropping
The air variant of structure voids gets placed as a void variant in vanilla
Sneaking and right clicking to create a structure block on another while flying will create a ghost block instead

Notes

~~I have used arbitrary values in BlockTypeIds.php, as there is a gap there I am not sure if it should have used a different number inside the gap.~~
~~I have used blast resistance values as indicated in the minecraft wiki, the block is indestructible, does it really matter ?~~
~~Right now, both blocks appear as update blocks to the client. I am not sure why, is the block state mandatory for them to appear correctly ?~~

Tests

Structure Void

/give <player> structure_void
place it
save the world

Structure Block

plugin.yml

---
name: StructureBlockTests
version: 0.0.1
main: Baleine\StructureBlockTests\Main
api: 5.5.1
src-namespace-prefix: Baleine\StructureBlockTests
commands:
  test:
    usage: "/test"
    description: "Test"
    permission: command.test
permissions:
  command.test:
    default: true
...

Main.php

<?php

declare(strict_types=1);

namespace Baleine\StructureBlockTests;

use pocketmine\block\StructureBlock;
use pocketmine\block\tile\StructureBlock as TileStructureBlock;
use pocketmine\block\utils\StructureAnimationMode;
use pocketmine\block\utils\StructureAxes;
use pocketmine\block\utils\StructureBlockType;
use pocketmine\block\utils\StructureRotation;
use pocketmine\command\Command;
use pocketmine\command\CommandSender;
use pocketmine\math\Vector3;
use pocketmine\player\Player;
use pocketmine\plugin\PluginBase;

class Main extends PluginBase{
	/**
	 * @param string[] $args
	 */
	public function onCommand(CommandSender $sender, Command $command, string $label, array $args) : bool{
		if ($sender instanceof Player){
			switch ($command->getName()){
				case "test":
					var_dump($sender->getPosition());
					$world = $sender->getPosition()->getWorld();
					$position = $sender->getPosition()->add(0, -1, 0);
					$block = $world->getBlock($position);
					$tile = $world->getTile($position);
					if (!($block instanceof StructureBlock)) return false;
					if (!($tile instanceof TileStructureBlock)) return false;
					$tile->setType(StructureBlockType::LOAD);
					$tile->setShowBoundingBox(true);
					$tile->setStructureSize(new Vector3(16, 255, 16));
					$tile->setStructureOffset(new Vector3(0, -$position->getY(), 0));
					$tile->setAnimationMode(StructureAnimationMode::BY_BLOCK);
					$tile->setAnimationSeconds(5);
					$tile->setIgnoreEntities(true);
					$tile->setIntegrity(45.8);
					$tile->setIntegritySeed(76876867);
					$tile->setMirroredAxes(new StructureAxes(false, true));
					$tile->setRemoveBlocks(false);
					$tile->setRotation(new StructureRotation(180));
					$tile->setStructureName("Test123");
					$block->setType(StructureBlockType::LOAD);
					$world->setBlock($position, $block);
					break;
			}
		}
		return true;
	}
}

Place a structure block and stand on top of it, then execute the /test command and check that the parameters have been updated.
~~Problem: if $block->setType is not called the block will not visibly update (it calls setBlock). I am not sure how to go about this.~~
Plugins should call setBlock themselves after changing block properties.

dktapps · 2023-09-09T09:17:18Z

I have used arbitrary values in BlockTypeIds.php, as there is a gap there I am not sure if it should have used a different number inside the gap.

The values can be basically anything. The usual way is to put them at the bottom and increase the FIRST_UNUSED_TYPE_ID constant (or whatever it's called, I forget) to accommodate. However, putting them in a gap will also work.

Right now, both blocks appear as update blocks to the client. I am not sure why, is the block state mandatory for them to appear correctly ?

You haven't registered them in BlockStateToObjectDeserializer and BlockObjectToStateSerializer.

Bqleine · 2023-09-09T11:44:30Z

After having implemented block states and serialization/deserialization. I'm getting this error when using the block:

[Server thread/CRITICAL]: pocketmine\utils\AssumptionFailedError: "Unmapped blockstate returned by blockstate serializer: TAG_Compound={ "name" => TAG_String="minecraft:structure_void" "version" => TAG_Int=18090528 "states" => TAG_Compound={ "structure_void_type" => TAG_Int=0 } "PMMPDataVersion" => TAG_Long=1 }" (EXCEPTION) in "pmsrc/src/network/mcpe/convert/ItemTranslator" at line 80
--- Stack trace ---
  #0 pmsrc/src/network/mcpe/convert/ItemTranslator(57): pocketmine\network\mcpe\convert\ItemTranslator->toNetworkId(object pocketmine\item\ItemBlock#148201)
  #1 pmsrc/src/network/mcpe/convert/TypeConverter(207): pocketmine\network\mcpe\convert\ItemTranslator->toNetworkIdQuiet(object pocketmine\item\ItemBlock#148201)
  #2 pmsrc/src/network/mcpe/InventoryManager(540): pocketmine\network\mcpe\convert\TypeConverter->coreItemStackToNet(object pocketmine\item\ItemBlock#148201)
  #3 pmsrc/src/network/mcpe/InventoryManager(561): pocketmine\network\mcpe\InventoryManager->syncContents(object pocketmine\inventory\PlayerInventory#160238)
  #4 pmsrc/src/network/mcpe/handler/PreSpawnPacketHandler(141): pocketmine\network\mcpe\InventoryManager->syncAll()
  #5 pmsrc/src/network/mcpe/NetworkSession(326): pocketmine\network\mcpe\handler\PreSpawnPacketHandler->setUp()
  #6 pmsrc/src/network/mcpe/NetworkSession(820): pocketmine\network\mcpe\NetworkSession->setHandler(object pocketmine\network\mcpe\handler\PreSpawnPacketHandler#160390)
  #7 pmsrc/src/network/mcpe/NetworkSession(277): pocketmine\network\mcpe\NetworkSession->beginSpawnSequence()
  #8 pmsrc/src/promise/Promise(45): pocketmine\network\mcpe\NetworkSession->onPlayerCreated(object pocketmine\player\Player#167479)
  #9 pmsrc/src/network/mcpe/NetworkSession(234): pocketmine\promise\Promise->onCompletion(object Closure#167480, object Closure#167559)
  #10 pmsrc/src/network/mcpe/NetworkSession(815): pocketmine\network\mcpe\NetworkSession->createPlayer()
  #11 pmsrc/src/network/mcpe/handler/ResourcePacksPacketHandler(142): pocketmine\network\mcpe\NetworkSession->pocketmine\network\mcpe\{closure}()
  #12 pmsrc/vendor/pocketmine/bedrock-protocol/src/ResourcePackClientResponsePacket(61): pocketmine\network\mcpe\handler\ResourcePacksPacketHandler->handleResourcePackClientResponse(object pocketmine\network\mcpe\protocol\ResourcePackClientResponsePacket#167671)
  #13 pmsrc/src/network/mcpe/NetworkSession(445): pocketmine\network\mcpe\protocol\ResourcePackClientResponsePacket->handle(object pocketmine\network\mcpe\handler\ResourcePacksPacketHandler#167540)
  #14 pmsrc/src/network/mcpe/NetworkSession(383): pocketmine\network\mcpe\NetworkSession->handleDataPacket(object pocketmine\network\mcpe\protocol\ResourcePackClientResponsePacket#167671, string[4] ....)
  #15 pmsrc/src/network/mcpe/raklib/RakLibInterface(219): pocketmine\network\mcpe\NetworkSession->handleEncoded(string[7]
c.`a`..)
  #16 pmsrc/vendor/pocketmine/raklib-ipc/src/RakLibToUserThreadMessageReceiver(40): pocketmine\network\mcpe\raklib\RakLibInterface->onPacketReceive(int 0, string[16] ....et...\^+`.[.)
  #17 pmsrc/src/network/mcpe/raklib/RakLibInterface(111): raklib\server\ipc\RakLibToUserThreadMessageReceiver->handle(object pocketmine\network\mcpe\raklib\RakLibInterface#168038)
  #18 pmsrc/vendor/pocketmine/snooze/src/SleeperHandler(120): pocketmine\network\mcpe\raklib\RakLibInterface->pocketmine\network\mcpe\raklib\{closure}()
  #19 pmsrc/src/TimeTrackingSleeperHandler(58): pocketmine\snooze\SleeperHandler->processNotifications()
  #20 pmsrc/vendor/pocketmine/snooze/src/SleeperHandler(79): pocketmine\TimeTrackingSleeperHandler->processNotifications()
  #21 pmsrc/src/Server(1681): pocketmine\snooze\SleeperHandler->sleepUntil(float 1694260926.2298)
  #22 pmsrc/src/Server(1064): pocketmine\Server->tickProcessor()
  #23 pmsrc/src/PocketMine(334): pocketmine\Server->__construct(object pocketmine\thread\ThreadSafeClassLoader#3, object pocketmine\utils\MainLogger#6, string[30] /home/bloup/Pocketmine-Source/, string[38] /home/bloup/Pocketmine-Source/plugins/)
  #24 pmsrc/src/PocketMine(357): pocketmine\server()
--- End of exception information ---

It seems these mappings are taken directly from bedrock data though. Have I used an incorrect constant ?

Bqleine · 2023-09-09T13:32:56Z

I have found the issue, I was using 0 and 1 for values instead of constant strings from BlockStateStringValues.

src/block/utils/StructureBlockType.php

Bqleine · 2023-09-15T21:53:47Z

I think this is ready for an initial review. I've added a plugin to test in the pr description :)

zSALLAZAR

These functions should not be nullable

src/data/bedrock/block/convert/BlockStateReader.php

src/data/bedrock/block/convert/BlockStateToObjectDeserializer.php

ShockedPlot7560

Some phpstan and style correction need to be adresses, but the logic looks good

src/block/tile/StructureBlock.php

src/network/mcpe/handler/InGamePacketHandler.php

Bqleine · 2023-10-13T18:12:10Z

I've done all the changes I wanted to do for this PR, the two blocks are added and the structure block can be interacted with and changes to structure settings are saved.
The rest of the implementation requires creating a plugin api, so I am leaving this work for a future PR, as written on this PR's description.
One last thing remains to be done, and that is fixing the bug shown in unit tests, but I don't understand what the issue is. Some help would be greatly appreciated.

dktapps

This PR is trying to do multiple things at once. Most of the Structure Void stuff is fine, but the majority of the PR is Structure Block stuff which I find has been implemented poorly. Please split the two blocks into separate PRs.

src/block/StructureBlock.php

dktapps · 2023-11-09T14:08:36Z

src/block/StructureBlock.php

+		if ($player instanceof Player) {
+			if (!$player->isCreative(true)) return false;
+			// the 0 is dubious but any value works
+			$pk = ContainerOpenPacket::blockInv(0, WindowTypes::STRUCTURE_EDITOR, BlockPosition::fromVector3($this->getPosition()));


I do not like this. Packets shouldn't be directly baked into the core code like this.

Should I create an Inventory extending BlockInventory and TemporaryInventory with no slots then ? Sounds odd

dktapps · 2023-11-09T14:09:27Z

src/block/utils/StructureBlockType.php

+			5 => self::EXPORT,
+			default => throw new \ValueError("Unknown structure block type " . $type),
+		};
+	}


These should not be visible in the API. The general way to handle these would be a map in pocketmine\data\bedrock.

dktapps · 2023-11-09T14:09:33Z

src/block/utils/StructureBlockType.php

+			self::INVALID => 4,
+			self::EXPORT => 5,
+		};
+	}


Same again here.

dktapps · 2023-11-09T14:11:34Z

src/block/utils/StructureVoidType.php

+
+enum StructureVoidType{
+	case VOID;
+	case AIR;


What's the difference between these two types?

From the minecraft wiki:

It exists as a data value of structure voids. It acts similarly to structure voids. When holding a structure air or structure void, it becomes visible. Structure air and structure voids can stack together, the icon of the stack is the icon of whichever block was most recently added to the stack. Transparent blocks can be placed on them just like with structure voids. In the inventory, structure air is called structure void. If you give yourself a structure air then whenever you hold a structure void it will look like a structure air in your hand in both third and first person. Structure air are copied by structure blocks. This makes structure air very useful so always use structure air for your creative builds instead of structure voids. Note: structure air becomes structure voids when placed by hand, but if placed with /setblock ~ ~ ~ structure_void ["structure_void_type":"air"] it will stay as a structure air.

I was not able to completely verify this information, the difference that matters in this pr is that vanilla minecraft will forbid you from placing structure airs, and in this pr you can place them fine.

And you can't stack two different types in the pr

The wiki entry is gibberish that conveys no actual information. That's why I asked.

The difference is that structure air will replace whatever block is at that position with an air block, while structure void will just leave it be

That's exactly what normal air does ...

Depends on the structure block settings iirc

Meaning what?

I was thinking about the Remove Blocks setting, but it doesn't help (its just another weird setting).

After testing on 1.20.41.02, I was unable to find a useful difference between structure air and structure void. I was able to reproduce normal behavior with both:

Structure voids can be placed as part of a structure and are ignored when the structure file is loaded. This results in the blocks at the location where the structure is placed being maintained and not being overridden.

This is exactly what happened :

For example, if a player saves a 2 block high structure with air at the top and a structure void at the bottom, and loads it into a 2 block high area of stone, the top block becomes air but the bottom stays stone.

What did not happen:

"Structure air are copied by structure blocks."

TL;DR I was not able to find any difference in the structure block usage. I left a comment on the wiki page

Did you try pasting structures underwater with this? I wonder if there's a difference in the water displacement behaviour.

dktapps · 2023-11-09T14:11:56Z

src/block/utils/StructureAxes.php

+	/**
+	 * @return $this
+	 */
+	public function setXMirror(bool $xMirror) : self{


Again, why is this mutable?

dktapps · 2023-11-09T14:12:02Z

src/block/utils/StructureAxes.php

+	/**
+	 * @return $this
+	 */
+	public function setYMirror(bool $yMirror) : self{


And again here

dktapps · 2023-11-09T14:12:18Z

src/block/utils/StructureAnimationMode.php

+			2 => self::BY_BLOCK,
+			default => throw new \ValueError("Unknown structure animation mode " . $mode),
+		};
+	}


These should be handled in pocketmine\data

dktapps · 2023-11-09T14:12:24Z

src/block/utils/StructureAnimationMode.php

+			self::BY_LAYER => 1,
+			self::BY_BLOCK => 2,
+		};
+	}


and again here

Bqleine · 2023-11-09T14:27:40Z

Thank you for the complete review, I will make a separate PR for the structure void.
I agree that the quality of the code is not optimal, this is my first PR and I am still learning about the inner workings of the codebase, I will take some time in the coming days to improve on the points you commented. 👍

Bqleine added 2 commits September 8, 2023 21:14

StructureBlock: register block

aec64e3

StructureVoid: register block

f7452e4

StructureVoid: add serialization and block state

0084569

Bqleine force-pushed the minor-next branch from 4dea6e6 to 0084569 Compare September 9, 2023 11:49

Bqleine added 2 commits September 9, 2023 15:47

StructureVoid: use BlockStateStringValues instead of integers

bbe05d1

StructureBlock: add serialization and block state

eb2add8

Bqleine force-pushed the minor-next branch from 2560827 to 1cf1579 Compare September 11, 2023 20:27

Bqleine added 4 commits September 11, 2023 22:30

StructureBlock: create tile to store editor settings

bb8c48b

StructureBlock: only open if the player is in creative mode

cef12c2

StructureBlock: use backed enum for StructureBlockType

93a0f91

StructureBlock: add types, getters and setters for editor settings

95d7218

Bqleine force-pushed the minor-next branch from 1cf1579 to 95d7218 Compare September 13, 2023 16:10

dktapps reviewed Sep 13, 2023

View reviewed changes

src/block/utils/StructureBlockType.php Outdated Show resolved Hide resolved

StructureBlock: stop using backed enumerations

963d14a

Bqleine marked this pull request as ready for review September 15, 2023 21:53

zSALLAZAR suggested changes Sep 16, 2023

View reviewed changes

src/data/bedrock/block/convert/BlockStateReader.php Outdated Show resolved Hide resolved

src/data/bedrock/block/convert/BlockStateReader.php Outdated Show resolved Hide resolved

StructureBlock: make function non-nullable

88e82c7

dktapps reviewed Sep 17, 2023

View reviewed changes

src/data/bedrock/block/convert/BlockStateToObjectDeserializer.php Outdated Show resolved Hide resolved

dktapps reviewed Sep 17, 2023

View reviewed changes

src/data/bedrock/block/convert/BlockStateToObjectDeserializer.php Outdated Show resolved Hide resolved

Bqleine and others added 2 commits September 20, 2023 21:14

StructureBlock: inline helper functions

cd8eae4

Merge branch 'minor-next' into minor-next

8daf0e7

ShockedPlot7560 reviewed Oct 6, 2023

View reviewed changes

src/block/tile/StructureBlock.php Outdated Show resolved Hide resolved

src/network/mcpe/handler/InGamePacketHandler.php Outdated Show resolved Hide resolved

ShockedPlot7560 added Category: Gameplay Related to Minecraft gameplay experience Status: Waiting on Author Type: Enhancement Contributes features or other improvements to PocketMine-MP labels Oct 6, 2023

Merge remote-tracking branch 'upstream/minor-next' into minor-next

554baa3

Bqleine added 4 commits October 7, 2023 12:16

Remove merge accident

2b10c63

StructureBlock: add tag type comments

7e27de1

StructureBlock: fix changed imports

e963d19

StructureBlock: last touches for pr

7c9dc8e

dktapps requested changes Nov 9, 2023

View reviewed changes

dktapps added the Multiple changes PR contains multiple changes and requires separation label Nov 9, 2023

Merge branch 'pmmp:minor-next' into minor-next

d921664

Bqleine mentioned this pull request Nov 12, 2023

Add Structure void #6156

Open

Bqleine and others added 3 commits November 13, 2023 17:34

StructureBlock: cosmetic changes

a50a348

StructureBlock: fix unit tests

a6747c1

Merge branch 'pmmp:minor-next' into minor-next

c277393

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Structure Block and Structure Void #6045

Add Structure Block and Structure Void #6045

Bqleine commented Sep 8, 2023 •

edited

dktapps commented Sep 9, 2023

Bqleine commented Sep 9, 2023 •

edited

Bqleine commented Sep 9, 2023

Bqleine commented Sep 15, 2023

zSALLAZAR left a comment

ShockedPlot7560 left a comment

Bqleine commented Oct 13, 2023

dktapps left a comment

dktapps Nov 9, 2023

Bqleine Nov 13, 2023

dktapps Nov 9, 2023

dktapps Nov 9, 2023

dktapps Nov 9, 2023

Bqleine Nov 12, 2023 •

edited

Bqleine Nov 12, 2023

dktapps Nov 13, 2023

RoyalMCPE Nov 13, 2023

dktapps Nov 13, 2023

Bqleine Nov 13, 2023

dktapps Nov 13, 2023

Bqleine Nov 13, 2023

dktapps Nov 22, 2023

dktapps Nov 9, 2023

dktapps Nov 9, 2023

dktapps Nov 9, 2023

dktapps Nov 9, 2023

Bqleine commented Nov 9, 2023

Add Structure Block and Structure Void #6045

Are you sure you want to change the base?

Add Structure Block and Structure Void #6045

Conversation

Bqleine commented Sep 8, 2023 • edited

Introduction

Relevant issues

Changes

Tasks

Bugs

Notes

Tests

Structure Void

Structure Block

dktapps commented Sep 9, 2023

Bqleine commented Sep 9, 2023 • edited

Bqleine commented Sep 9, 2023

Bqleine commented Sep 15, 2023

zSALLAZAR left a comment

Choose a reason for hiding this comment

ShockedPlot7560 left a comment

Choose a reason for hiding this comment

Bqleine commented Oct 13, 2023

dktapps left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bqleine Nov 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bqleine commented Nov 9, 2023

Bqleine commented Sep 8, 2023 •

edited

Bqleine commented Sep 9, 2023 •

edited

Bqleine Nov 12, 2023 •

edited