Fixed glowing item frames

due to technical limitations, this requires separating them back into two different block types. However, this isn't too egregious since it's just one flag, and actually simplifies some code.

closes #5478

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

@@ -990,6 +990,7 @@ final class BlockObjectToStateSerializer implements BlockStateSerializer{
 			})
 				->writeHorizontalFacing($block->getFacing());
 		});
+		$this->map(Blocks::GLOWING_ITEM_FRAME(), fn(ItemFrame $block) => Helper::encodeItemFrame($block, Ids::GLOW_FRAME));
 		$this->map(Blocks::GRANITE(), fn() => Helper::encodeStone(StringValues::STONE_TYPE_GRANITE));
 		$this->map(Blocks::GRANITE_SLAB(), fn(Slab $block) => Helper::encodeStoneSlab3($block, StringValues::STONE_SLAB_TYPE_3_GRANITE));
 		$this->mapStairs(Blocks::GRANITE_STAIRS(), Ids::GRANITE_STAIRS);
@@ -1019,12 +1020,7 @@ final class BlockObjectToStateSerializer implements BlockStateSerializer{
 				->writeString(StateNames::MONSTER_EGG_STONE_TYPE, StringValues::MONSTER_EGG_STONE_TYPE_STONE_BRICK));
 		$this->map(Blocks::IRON_DOOR(), fn(Door $block) => Helper::encodeDoor($block, new Writer(Ids::IRON_DOOR)));
 		$this->map(Blocks::IRON_TRAPDOOR(), fn(Trapdoor $block) => Helper::encodeTrapdoor($block, new Writer(Ids::IRON_TRAPDOOR)));
-		$this->map(Blocks::ITEM_FRAME(), function(ItemFrame $block) : Writer{
-			return Writer::create($block->isGlowing() ? Ids::GLOW_FRAME : Ids::FRAME)
-				->writeBool(StateNames::ITEM_FRAME_MAP_BIT, $block->hasMap())
-				->writeBool(StateNames::ITEM_FRAME_PHOTO_BIT, false)
-				->writeFacingDirection($block->getFacing());
-		});
+		$this->map(Blocks::ITEM_FRAME(), fn(ItemFrame $block) => Helper::encodeItemFrame($block, Ids::FRAME));
 		$this->map(Blocks::JUNGLE_BUTTON(), fn(WoodenButton $block) => Helper::encodeButton($block, new Writer(Ids::JUNGLE_BUTTON)));
 		$this->map(Blocks::JUNGLE_DOOR(), fn(WoodenDoor $block) => Helper::encodeDoor($block, new Writer(Ids::JUNGLE_DOOR)));
 		$this->map(Blocks::JUNGLE_FENCE(), fn() => Writer::create(Ids::FENCE)

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

@@ -49,7 +49,6 @@ use pocketmine\block\Trapdoor;
 use pocketmine\block\utils\CopperOxidation;
 use pocketmine\block\utils\DyeColor;
 use pocketmine\block\VanillaBlocks;
-use pocketmine\block\VanillaBlocks as Blocks;
 use pocketmine\block\Wall;
 use pocketmine\block\WallCoralFan;
 use pocketmine\block\WallSign;
@@ -172,12 +171,11 @@ final class BlockStateDeserializerHelper{
 			->setFacing($in->readHorizontalFacing());
 	}
 
-	public static function decodeItemFrame(BlockStateReader $in, bool $glowing) : ItemFrame{
+	public static function decodeItemFrame(ItemFrame $block, BlockStateReader $in) : ItemFrame{
 		$in->todo(StateNames::ITEM_FRAME_PHOTO_BIT); //TODO: not sure what the point of this is
-		return Blocks::ITEM_FRAME()
+		return $block
 			->setFacing($in->readFacingDirection())
-			->setHasMap($in->readBool(StateNames::ITEM_FRAME_MAP_BIT))
-			->setGlowing($glowing);
+			->setHasMap($in->readBool(StateNames::ITEM_FRAME_MAP_BIT));
 	}
 
 	/** @throws BlockStateDeserializeException */

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

@@ -32,6 +32,7 @@ use pocketmine\block\DoublePlant;
 use pocketmine\block\FenceGate;
 use pocketmine\block\FloorSign;
 use pocketmine\block\Furnace;
+use pocketmine\block\ItemFrame;
 use pocketmine\block\Leaves;
 use pocketmine\block\Liquid;
 use pocketmine\block\RedMushroomBlock;
@@ -50,6 +51,7 @@ use pocketmine\block\Wood;
 use pocketmine\data\bedrock\block\BlockStateNames;
 use pocketmine\data\bedrock\block\BlockStateNames as StateNames;
 use pocketmine\data\bedrock\block\BlockTypeNames as Ids;
+use pocketmine\data\bedrock\block\convert\BlockStateWriter as Writer;
 use pocketmine\data\bedrock\MushroomBlockTypeIdMap;
 use pocketmine\math\Facing;
 use pocketmine\utils\AssumptionFailedError;
@@ -138,6 +140,13 @@ final class BlockStateSerializerHelper{
 			->writeHorizontalFacing($block->getFacing());
 	}
 
+	public static function encodeItemFrame(ItemFrame $block, string $id) : BlockStateWriter{
+		return Writer::create($id)
+			->writeBool(StateNames::ITEM_FRAME_MAP_BIT, $block->hasMap())
+			->writeBool(StateNames::ITEM_FRAME_PHOTO_BIT, false)
+			->writeFacingDirection($block->getFacing());
+	}
+
 	private static function encodeLeaves(Leaves $block, BlockStateWriter $out) : BlockStateWriter{
 		return $out
 			->writeBool(BlockStateNames::PERSISTENT_BIT, $block->isNoDecay())

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

@@ -731,7 +731,7 @@ final class BlockStateToObjectDeserializer implements BlockStateDeserializer{
 		});
 		$this->map(Ids::FLOWING_LAVA, fn(Reader $in) => Helper::decodeFlowingLiquid(Blocks::LAVA(), $in));
 		$this->map(Ids::FLOWING_WATER, fn(Reader $in) => Helper::decodeFlowingLiquid(Blocks::WATER(), $in));
-		$this->map(Ids::FRAME, fn(Reader $in) => Helper::decodeItemFrame($in, false));
+		$this->map(Ids::FRAME, fn(Reader $in) => Helper::decodeItemFrame(Blocks::ITEM_FRAME(), $in));
 		$this->map(Ids::FROSTED_ICE, function(Reader $in) : Block{
 			return Blocks::FROSTED_ICE()
 				->setAge($in->readBoundedInt(StateNames::AGE, 0, 3));
@@ -741,7 +741,7 @@ final class BlockStateToObjectDeserializer implements BlockStateDeserializer{
 				->setFacing($in->readHorizontalFacing())
 				->setLit(false);
 		});
-		$this->map(Ids::GLOW_FRAME, fn(Reader $in) => Helper::decodeItemFrame($in, true));
+		$this->map(Ids::GLOW_FRAME, fn(Reader $in) => Helper::decodeItemFrame(Blocks::GLOWING_ITEM_FRAME(), $in));
 		$this->map(Ids::GOLDEN_RAIL, function(Reader $in) : Block{
 			return Blocks::POWERED_RAIL()
 				->setPowered($in->readBool(StateNames::RAIL_DATA_BIT))

src/data/bedrock/item/ItemSerializerDeserializerRegistrar.php

@@ -25,7 +25,6 @@ namespace pocketmine\data\bedrock\item;
 
 use pocketmine\block\Bed;
 use pocketmine\block\Block;
-use pocketmine\block\ItemFrame;
 use pocketmine\block\Skull;
 use pocketmine\block\utils\DyeColor;
 use pocketmine\block\utils\SkullType;
@@ -57,7 +56,6 @@ final class ItemSerializerDeserializerRegistrar{
 		$this->register1to1BlockWithMetaMappings();
 		$this->register1to1ItemWithMetaMappings();
 		$this->register1ToNItemMappings();
-		$this->registerMiscBlockMappings();
 		$this->registerMiscItemMappings();
 	}
 
@@ -141,6 +139,8 @@ final class ItemSerializerDeserializerRegistrar{
 		$this->map1to1Block(Ids::CRIMSON_DOOR, Blocks::CRIMSON_DOOR());
 		$this->map1to1Block(Ids::DARK_OAK_DOOR, Blocks::DARK_OAK_DOOR());
 		$this->map1to1Block(Ids::FLOWER_POT, Blocks::FLOWER_POT());
+		$this->map1to1Block(Ids::FRAME, Blocks::ITEM_FRAME());
+		$this->map1to1Block(Ids::GLOW_FRAME, Blocks::GLOWING_ITEM_FRAME());
 		$this->map1to1Block(Ids::HOPPER, Blocks::HOPPER());
 		$this->map1to1Block(Ids::IRON_DOOR, Blocks::IRON_DOOR());
 		$this->map1to1Block(Ids::JUNGLE_DOOR, Blocks::JUNGLE_DOOR());
@@ -502,19 +502,6 @@ final class ItemSerializerDeserializerRegistrar{
 		);
 	}
 
-	/**
-	 * Registers serializers and deserializers for blocks that don't fit any other pattern.
-	 * Ideally we want to get rid of this completely, if possible.
-	 *
-	 * Most of these are single PocketMine-MP items which map to multiple IDs depending on their properties, which is
-	 * complex to implement in a generic way.
-	 */
-	private function registerMiscBlockMappings() : void{
-		$this->deserializer?->mapBlock(Ids::FRAME, fn() => Blocks::ITEM_FRAME()->setGlowing(false));
-		$this->deserializer?->mapBlock(Ids::GLOW_FRAME, fn() => Blocks::ITEM_FRAME()->setGlowing(true));
-		$this->serializer?->mapBlock(Blocks::ITEM_FRAME(), fn(ItemFrame $block) => new Data($block->isGlowing() ? Ids::GLOW_FRAME : Ids::FRAME));
-	}
-
 	/**
 	 * Registers serializers and deserializers for items that don't fit any other pattern.
 	 * Ideally we want to get rid of this completely, if possible.