From ad6f7dfedb31a299d6d9c30961a76acf1f888f65 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Sun, 20 Apr 2025 19:48:28 +0100
Subject: [PATCH] World: verify saveability of blocks, entities and tiles at
 entry points I want to do the same for items, but items are going to be a
 pain in the ass. For items there are multiple possible entry points and all
 of them will need to be checked: - dropped items - inventory contents -
 lecterns - item frames

I don't see a good way to deal with all these. We can't check for registration in the constructor
because we need to fully construct the item in order to register it.

Blocks are also a potential issue in other areas, but setBlock() is definitely the biggest offender.
---
 src/block/tile/TileFactory.php                  |  7 +++++++
 .../convert/BlockObjectToStateSerializer.php    |  4 ++++
 src/entity/EntityFactory.php                    |  7 +++++++
 src/world/World.php                             | 17 +++++++++++++++++
 4 files changed, 35 insertions(+)

diff --git a/src/block/tile/TileFactory.php b/src/block/tile/TileFactory.php
index 515dd8c63..26e0af6a5 100644
--- a/src/block/tile/TileFactory.php
+++ b/src/block/tile/TileFactory.php
@@ -114,6 +114,13 @@ final class TileFactory{
 		$this->saveNames[$className] = reset($saveNames);
 	}
 
+	/**
+	 * @phpstan-param class-string<Tile> $class
+	 */
+	public function isRegistered(string $class) : bool{
+		return isset($this->saveNames[$class]);
+	}
+
 	/**
 	 * @internal
 	 * @throws SavedDataLoadingException
diff --git a/src/data/bedrock/block/convert/BlockObjectToStateSerializer.php b/src/data/bedrock/block/convert/BlockObjectToStateSerializer.php
index 367d38449..45784d409 100644
--- a/src/data/bedrock/block/convert/BlockObjectToStateSerializer.php
+++ b/src/data/bedrock/block/convert/BlockObjectToStateSerializer.php
@@ -225,6 +225,10 @@ final class BlockObjectToStateSerializer implements BlockStateSerializer{
 		return $this->cache[$stateId] ??= $this->serializeBlock(RuntimeBlockStateRegistry::getInstance()->fromStateId($stateId));
 	}
 
+	public function isRegistered(Block $block) : bool{
+		return isset($this->serializers[$block->getTypeId()]);
+	}
+
 	/**
 	 * @phpstan-template TBlockType of Block
 	 * @phpstan-param TBlockType $block
diff --git a/src/entity/EntityFactory.php b/src/entity/EntityFactory.php
index 03d9c03e6..970fd986f 100644
--- a/src/entity/EntityFactory.php
+++ b/src/entity/EntityFactory.php
@@ -219,6 +219,13 @@ final class EntityFactory{
 		$this->saveNames[$className] = reset($saveNames);
 	}
 
+	/**
+	 * @phpstan-param class-string<Entity> $class
+	 */
+	public function isRegistered(string $class) : bool{
+		return isset($this->saveNames[$class]);
+	}
+
 	/**
 	 * Creates an entity from data stored on a chunk.
 	 *
diff --git a/src/world/World.php b/src/world/World.php
index 3a7d0c538..afd01c628 100644
--- a/src/world/World.php
+++ b/src/world/World.php
@@ -2047,6 +2047,15 @@ class World implements ChunkManager{
 			throw new WorldException("Cannot set a block in un-generated terrain");
 		}
 
+		//TODO: this computes state ID twice (we do it again in writeStateToWorld()). Not great for performance :(
+		$stateId = $block->getStateId();
+		if(!$this->blockStateRegistry->hasStateId($stateId)){
+			throw new \LogicException("Block state ID not known to RuntimeBlockStateRegistry (probably not registered)");
+		}
+		if(!GlobalBlockStateHandlers::getSerializer()->isRegistered($block)){
+			throw new \LogicException("Block not registered with GlobalBlockStateHandlers serializer");
+		}
+
 		$this->timings->setBlock->startTiming();
 
 		$this->unlockChunk($chunkX, $chunkZ, null);
@@ -2769,6 +2778,11 @@ class World implements ChunkManager{
 				throw new AssumptionFailedError("Found two different entities sharing entity ID " . $entity->getId());
 			}
 		}
+		if(!EntityFactory::getInstance()->isRegistered($entity::class)){
+			//canSaveWithChunk is mutable, so that means it could be toggled after adding the entity and cause a crash
+			//later on. Better we just force all entities to have a save ID, even if it might not be needed.
+			throw new \LogicException("Entity " . $entity::class . " is not registered for a save ID in EntityFactory");
+		}
 		$pos = $entity->getPosition()->asVector3();
 		$this->entitiesByChunk[World::chunkHash($pos->getFloorX() >> Chunk::COORD_BIT_SIZE, $pos->getFloorZ() >> Chunk::COORD_BIT_SIZE)][$entity->getId()] = $entity;
 		$this->entityLastKnownPositions[$entity->getId()] = $pos;
@@ -2870,6 +2884,9 @@ class World implements ChunkManager{
 		if(!$this->isInWorld($pos->getFloorX(), $pos->getFloorY(), $pos->getFloorZ())){
 			throw new \InvalidArgumentException("Tile position is outside the world bounds");
 		}
+		if(!TileFactory::getInstance()->isRegistered($tile::class)){
+			throw new \LogicException("Tile " . $tile::class . " is not registered for a save ID in TileFactory");
+		}
 
 		$chunkX = $pos->getFloorX() >> Chunk::COORD_BIT_SIZE;
 		$chunkZ = $pos->getFloorZ() >> Chunk::COORD_BIT_SIZE;