EntityFactory now exclusively handles loading data from disk

this commit removes the ability to replace centrally registered entity classes in favour of using constructors directly.
In future commits I may introduce a dedicated factory interface which allows an _actual_ factory pattern (e.g. factory->createArrow(world, pos, shooter, isCritical) with proper static analysability) but for now it's peripheral to my intended objective.
The purpose of this change is to facilitate untangling of NBT from entity constructors so that they can be properly created without using NBT at all, and instead use nice APIs.

Spawn eggs now support arbitrary entity creation functions like EntityFactory does, allowing much more flexibility in what can be passed to an entity's constructor (e.g. a Plugin reference can be injected by use()ing it in a closure or via traditional DI.

src/block/TNT.php

@@ -96,8 +96,7 @@ class TNT extends Opaque{
 		$nbt = EntityFactory::createBaseNBT($this->pos->add(0.5, 0, 0.5), new Vector3(-sin($mot) * 0.02, 0.2, -cos($mot) * 0.02));
 		$nbt->setShort("Fuse", $fuse);
 
-		/** @var PrimedTNT $tnt */
-		$tnt = EntityFactory::getInstance()->create(PrimedTNT::class, $this->pos->getWorldNonNull(), $nbt);
+		$tnt = new PrimedTNT($this->pos->getWorldNonNull(), $nbt);
 		$tnt->spawnToAll();
 	}
 

src/block/utils/FallableTrait.php

@@ -55,8 +55,7 @@ trait FallableTrait{
 			$nbt->setInt("TileID", $this->getId());
 			$nbt->setByte("Data", $this->getMeta());
 
-			/** @var FallingBlock $fall */
-			$fall = EntityFactory::getInstance()->create(FallingBlock::class, $pos->getWorldNonNull(), $nbt);
+			$fall = new FallingBlock($pos->getWorldNonNull(), $nbt);
 			$fall->spawnToAll();
 		}
 	}

src/entity/EntityFactory.php

@@ -54,12 +54,8 @@ use function in_array;
 use function reset;
 
 /**
- * This class manages the creation of entities loaded from disk (and optionally entities created at runtime).
- *
- * You need to register your entity class into this factory if:
- * a) you want to load/save your entity on disk (saving with chunks)
- * b) you want to allow custom things to provide a custom class for your entity. Note that you must use
- *    create(MyEntity::class) instead of `new MyEntity()` if you want to allow this.
+ * This class manages the creation of entities loaded from disk.
+ * You need to register your entity into this factory if you want to load/save your entity on disk (saving with chunks).
  */
 final class EntityFactory{
 	use SingletonTrait;
@@ -69,7 +65,7 @@ final class EntityFactory{
 
 	/**
 	 * @var \Closure[] base class => creator function
-	 * @phpstan-var array<class-string<Entity>, \Closure(World, CompoundTag, mixed...) : Entity>
+	 * @phpstan-var array<class-string<Entity>, \Closure(World, CompoundTag) : Entity>
 	 */
 	private $creationFuncs = [];
 	/**
@@ -87,20 +83,20 @@ final class EntityFactory{
 		//define legacy save IDs first - use them for saving for maximum compatibility with Minecraft PC
 		//TODO: index them by version to allow proper multi-save compatibility
 
-		$this->register(Arrow::class, function(World $world, CompoundTag $nbt, ...$extraArgs) : Arrow{
-			return new Arrow($world, $nbt, ...$extraArgs);
+		$this->register(Arrow::class, function(World $world, CompoundTag $nbt) : Arrow{
+			return new Arrow($world, $nbt);
 		}, ['Arrow', 'minecraft:arrow'], EntityLegacyIds::ARROW);
 
-		$this->register(Egg::class, function(World $world, CompoundTag $nbt, ...$extraArgs) : Egg{
-			return new Egg($world, $nbt, ...$extraArgs);
+		$this->register(Egg::class, function(World $world, CompoundTag $nbt) : Egg{
+			return new Egg($world, $nbt);
 		}, ['Egg', 'minecraft:egg'], EntityLegacyIds::EGG);
 
-		$this->register(EnderPearl::class, function(World $world, CompoundTag $nbt, ...$extraArgs) : EnderPearl{
-			return new EnderPearl($world, $nbt, ...$extraArgs);
+		$this->register(EnderPearl::class, function(World $world, CompoundTag $nbt) : EnderPearl{
+			return new EnderPearl($world, $nbt);
 		}, ['ThrownEnderpearl', 'minecraft:ender_pearl'], EntityLegacyIds::ENDER_PEARL);
 
-		$this->register(ExperienceBottle::class, function(World $world, CompoundTag $nbt, ...$extraArgs) : ExperienceBottle{
-			return new ExperienceBottle($world, $nbt, ...$extraArgs);
+		$this->register(ExperienceBottle::class, function(World $world, CompoundTag $nbt) : ExperienceBottle{
+			return new ExperienceBottle($world, $nbt);
 		}, ['ThrownExpBottle', 'minecraft:xp_bottle'], EntityLegacyIds::XP_BOTTLE);
 
 		$this->register(ExperienceOrb::class, function(World $world, CompoundTag $nbt) : ExperienceOrb{
@@ -123,12 +119,12 @@ final class EntityFactory{
 			return new PrimedTNT($world, $nbt);
 		}, ['PrimedTnt', 'PrimedTNT', 'minecraft:tnt'], EntityLegacyIds::TNT);
 
-		$this->register(Snowball::class, function(World $world, CompoundTag $nbt, ...$extraArgs) : Snowball{
-			return new Snowball($world, $nbt, ...$extraArgs);
+		$this->register(Snowball::class, function(World $world, CompoundTag $nbt) : Snowball{
+			return new Snowball($world, $nbt);
 		}, ['Snowball', 'minecraft:snowball'], EntityLegacyIds::SNOWBALL);
 
-		$this->register(SplashPotion::class, function(World $world, CompoundTag $nbt, ...$extraArgs) : SplashPotion{
-			return new SplashPotion($world, $nbt, ...$extraArgs);
+		$this->register(SplashPotion::class, function(World $world, CompoundTag $nbt) : SplashPotion{
+			return new SplashPotion($world, $nbt);
 		}, ['ThrownPotion', 'minecraft:potion', 'thrownpotion'], EntityLegacyIds::SPLASH_POTION);
 
 		$this->register(Squid::class, function(World $world, CompoundTag $nbt) : Squid{
@@ -152,14 +148,13 @@ final class EntityFactory{
 
 	/**
 	 * @phpstan-param class-string<Entity> $baseClass
-	 * @phpstan-param \Closure(World, CompoundTag, mixed...) : Entity $creationFunc
+	 * @phpstan-param \Closure(World, CompoundTag) : Entity $creationFunc
 	 */
 	private static function validateCreationFunc(string $baseClass, \Closure $creationFunc) : void{
 		$sig = new CallbackType(
 			new ReturnType($baseClass),
 			new ParameterType("world", World::class),
-			new ParameterType("nbt", CompoundTag::class),
-			new ParameterType("extraArgs", null, ParameterType::VARIADIC | ParameterType::CONTRAVARIANT | ParameterType::OPTIONAL)
+			new ParameterType("nbt", CompoundTag::class)
 		);
 		if(!$sig->isSatisfiedBy($creationFunc)){
 			throw new \TypeError("Declaration of callable `" . CallbackType::createFromCallable($creationFunc) . "` must be compatible with `" . $sig . "`");
@@ -173,7 +168,7 @@ final class EntityFactory{
 	 * @param \Closure $creationFunc
 	 * @param string[] $saveNames An array of save names which this entity might be saved under. Defaults to the short name of the class itself if empty.
 	 * @phpstan-param class-string<Entity> $className
-	 * @phpstan-param \Closure(World $world, CompoundTag $nbt, mixed ...$args) : Entity $creationFunc
+	 * @phpstan-param \Closure(World $world, CompoundTag $nbt) : Entity $creationFunc
 	 *
 	 * NOTE: The first save name in the $saveNames array will be used when saving the entity to disk. The reflection
 	 * name of the class will be appended to the end and only used if no other save names are specified.
@@ -201,27 +196,6 @@ final class EntityFactory{
 		$this->saveNames[$className] = $saveNames;
 	}
 
-	/**
-	 * Registers a class override for the given class. When a new entity is constructed using the factory, the new class
-	 * will be used instead of the base class.
-	 *
-	 * @param string   $baseClass Already-registered entity class to override
-	 * @param \Closure $newCreationFunc
-	 *
-	 * @phpstan-param class-string<Entity> $baseClass
-	 * @phpstan-param \Closure(World, CompoundTag, mixed...) : Entity $newCreationFunc
-	 *
-	 * @throws \InvalidArgumentException
-	 */
-	public function override(string $baseClass, \Closure $newCreationFunc) : void{
-		if(!isset($this->creationFuncs[$baseClass])){
-			throw new \InvalidArgumentException("Class $baseClass is not a registered entity");
-		}
-
-		self::validateCreationFunc($baseClass, $newCreationFunc);
-		$this->creationFuncs[$baseClass] = $newCreationFunc;
-	}
-
 	/**
 	 * Returns an array of all registered entity classpaths.
 	 *
@@ -239,36 +213,6 @@ final class EntityFactory{
 		return self::$entityCount++;
 	}
 
-	/**
-	 * Creates an entity with the specified type, world and NBT, with optional additional arguments to pass to the
-	 * entity's constructor.
-	 *
-	 * TODO: make this NBT-independent
-	 *
-	 * @phpstan-template TEntity of Entity
-	 *
-	 * @param mixed       ...$args
-	 * @phpstan-param class-string<TEntity> $baseClass
-	 *
-	 * @return Entity instanceof $baseClass
-	 * @phpstan-return TEntity
-	 *
-	 * @throws \InvalidArgumentException if the class doesn't exist or is not registered
-	 */
-	public function create(string $baseClass, World $world, CompoundTag $nbt, ...$args) : Entity{
-		if(isset($this->creationFuncs[$baseClass])){
-			$func = $this->creationFuncs[$baseClass];
-			/**
-			 * @var Entity $entity
-			 * @phpstan-var TEntity $entity
-			 */
-			$entity = $func($world, $nbt, ...$args);
-			return $entity;
-		}
-
-		throw new \InvalidArgumentException("Class $baseClass is not a registered entity");
-	}
-
 	/**
 	 * Creates an entity from data stored on a chunk.
 	 *

src/item/Bow.php

@@ -62,8 +62,7 @@ class Bow extends Tool{
 		$p = $diff / 20;
 		$baseForce = min((($p ** 2) + $p * 2) / 3, 1);
 
-		/** @var ArrowEntity $entity */
-		$entity = EntityFactory::getInstance()->create(ArrowEntity::class, $location->getWorldNonNull(), $nbt, $player, $baseForce >= 1);
+		$entity = new ArrowEntity($location->getWorldNonNull(), $nbt, $player, $baseForce >= 1);
 
 		$infinity = $this->hasEnchantment(Enchantment::INFINITY());
 		if($infinity){

src/item/Egg.php

@@ -36,15 +36,12 @@ class Egg extends ProjectileItem{
 		return 16;
 	}
 
-	protected function createEntity(EntityFactory $factory, Location $location, Vector3 $velocity, Player $thrower) : Throwable{
-		/** @var EggEntity $projectile */
-		$projectile = $factory->create(
-			EggEntity::class,
+	protected function createEntity(Location $location, Vector3 $velocity, Player $thrower) : Throwable{
+		return new EggEntity(
 			$location->getWorldNonNull(),
 			EntityFactory::createBaseNBT($location, $velocity, $location->yaw, $location->pitch),
 			$thrower
 		);
-		return $projectile;
 	}
 
 	public function getThrowForce() : float{

src/item/EnderPearl.php

@@ -36,15 +36,12 @@ class EnderPearl extends ProjectileItem{
 		return 16;
 	}
 
-	protected function createEntity(EntityFactory $factory, Location $location, Vector3 $velocity, Player $thrower) : Throwable{
-		/** @var EnderPearlEntity $projectile */
-		$projectile = $factory->create(
-			EnderPearlEntity::class,
+	protected function createEntity(Location $location, Vector3 $velocity, Player $thrower) : Throwable{
+		return new EnderPearlEntity(
 			$location->getWorldNonNull(),
 			EntityFactory::createBaseNBT($location, $velocity, $location->yaw, $location->pitch),
 			$thrower
 		);
-		return $projectile;
 	}
 
 	public function getThrowForce() : float{

src/item/ExperienceBottle.php

@@ -32,15 +32,12 @@ use pocketmine\player\Player;
 
 class ExperienceBottle extends ProjectileItem{
 
-	protected function createEntity(EntityFactory $factory, Location $location, Vector3 $velocity, Player $thrower) : Throwable{
-		/** @var ExperienceBottleEntity $projectile */
-		$projectile = $factory->create(
-			ExperienceBottleEntity::class,
+	protected function createEntity(Location $location, Vector3 $velocity, Player $thrower) : Throwable{
+		return new ExperienceBottleEntity(
 			$location->getWorldNonNull(),
 			EntityFactory::createBaseNBT($location, $velocity, $location->yaw, $location->pitch),
 			$thrower
 		);
-		return $projectile;
 	}
 
 	public function getThrowForce() : float{

src/item/ItemFactory.php

@@ -29,12 +29,17 @@ use pocketmine\block\utils\DyeColor;
 use pocketmine\block\utils\SkullType;
 use pocketmine\block\utils\TreeType;
 use pocketmine\block\VanillaBlocks;
+use pocketmine\entity\Entity;
 use pocketmine\entity\EntityFactory;
-use pocketmine\entity\Living;
+use pocketmine\entity\Squid;
+use pocketmine\entity\Villager;
+use pocketmine\entity\Zombie;
 use pocketmine\inventory\ArmorInventory;
+use pocketmine\math\Vector3;
 use pocketmine\nbt\tag\CompoundTag;
+use pocketmine\network\mcpe\protocol\types\entity\EntityLegacyIds;
 use pocketmine\utils\SingletonTrait;
-use function is_a;
+use pocketmine\world\World;
 
 /**
  * Manages Item instance creation and registration
@@ -50,6 +55,7 @@ class ItemFactory{
 
 	public function __construct(){
 		$this->registerArmorItems();
+		$this->registerSpawnEggs();
 		$this->registerTierToolItems();
 
 		$this->register(new Apple(ItemIds::APPLE, 0, "Apple"));
@@ -250,13 +256,6 @@ class ItemFactory{
 			$this->register(new SplashPotion(ItemIds::SPLASH_POTION, $type, "Splash Potion"));
 		}
 
-		foreach(EntityFactory::getInstance()->getKnownTypes() as $className){
-			/** @var Living|string $className */
-			if(is_a($className, Living::class, true) and $className::getNetworkTypeId() !== -1){
-				$this->register(new SpawnEgg(ItemIds::SPAWN_EGG, $className::getNetworkTypeId(), "Spawn Egg", $className));
-			}
-		}
-
 		foreach(TreeType::getAll() as $type){
 			$this->register(new Boat(ItemIds::BOAT, $type->getMagicNumber(), $type->getDisplayName() . " Boat", $type));
 		}
@@ -316,6 +315,25 @@ class ItemFactory{
 		//endregion
 	}
 
+	private function registerSpawnEggs() : void{
+		//TODO: the meta values should probably be hardcoded; they won't change, but the EntityLegacyIds might
+		$this->register(new class(ItemIds::SPAWN_EGG, EntityLegacyIds::ZOMBIE, "Zombie Spawn Egg") extends SpawnEgg{
+			protected function createEntity(World $world, Vector3 $pos, float $yaw, float $pitch) : Entity{
+				return new Zombie($world, EntityFactory::createBaseNBT($pos, null, $yaw, $pitch));
+			}
+		});
+		$this->register(new class(ItemIds::SPAWN_EGG, EntityLegacyIds::SQUID, "Squid Spawn Egg") extends SpawnEgg{
+			public function createEntity(World $world, Vector3 $pos, float $yaw, float $pitch) : Entity{
+				return new Squid($world, EntityFactory::createBaseNBT($pos, null, $yaw, $pitch));
+			}
+		});
+		$this->register(new class(ItemIds::SPAWN_EGG, EntityLegacyIds::VILLAGER, "Villager Spawn Egg") extends SpawnEgg{
+			public function createEntity(World $world, Vector3 $pos, float $yaw, float $pitch) : Entity{
+				return new Villager($world, EntityFactory::createBaseNBT($pos, null, $yaw, $pitch));
+			}
+		});
+	}
+
 	private function registerTierToolItems() : void{
 		$this->register(new Axe(ItemIds::DIAMOND_AXE, "Diamond Axe", ToolTier::DIAMOND()));
 		$this->register(new Axe(ItemIds::GOLDEN_AXE, "Golden Axe", ToolTier::GOLD()));

src/item/PaintingItem.php

@@ -93,8 +93,7 @@ class PaintingItem extends Item{
 		$nbt->setInt("TileY", $clickedPos->getFloorY());
 		$nbt->setInt("TileZ", $clickedPos->getFloorZ());
 
-		/** @var Painting $entity */
-		$entity = EntityFactory::getInstance()->create(Painting::class, $replacePos->getWorldNonNull(), $nbt);
+		$entity = new Painting($replacePos->getWorldNonNull(), $nbt);
 		$this->pop();
 		$entity->spawnToAll();
 

src/item/ProjectileItem.php

@@ -23,7 +23,6 @@ declare(strict_types=1);
 
 namespace pocketmine\item;
 
-use pocketmine\entity\EntityFactory;
 use pocketmine\entity\Location;
 use pocketmine\entity\projectile\Throwable;
 use pocketmine\event\entity\ProjectileLaunchEvent;
@@ -35,12 +34,12 @@ abstract class ProjectileItem extends Item{
 
 	abstract public function getThrowForce() : float;
 
-	abstract protected function createEntity(EntityFactory $factory, Location $location, Vector3 $velocity, Player $thrower) : Throwable;
+	abstract protected function createEntity(Location $location, Vector3 $velocity, Player $thrower) : Throwable;
 
 	public function onClickAir(Player $player, Vector3 $directionVector) : ItemUseResult{
 		$location = $player->getLocation();
 
-		$projectile = $this->createEntity(EntityFactory::getInstance(), Location::fromObject($player->getEyePos(), $player->getWorld(), $location->yaw, $location->pitch), $directionVector, $player);
+		$projectile = $this->createEntity(Location::fromObject($player->getEyePos(), $player->getWorld(), $location->yaw, $location->pitch), $directionVector, $player);
 		$projectile->setMotion($projectile->getMotion()->multiply($this->getThrowForce()));
 
 		$projectileEv = new ProjectileLaunchEvent($projectile);

src/item/Snowball.php

@@ -36,15 +36,12 @@ class Snowball extends ProjectileItem{
 		return 16;
 	}
 
-	protected function createEntity(EntityFactory $factory, Location $location, Vector3 $velocity, Player $thrower) : Throwable{
-		/** @var SnowballEntity $projectile */
-		$projectile = $factory->create(
-			SnowballEntity::class,
+	protected function createEntity(Location $location, Vector3 $velocity, Player $thrower) : Throwable{
+		return new SnowballEntity(
 			$location->getWorldNonNull(),
 			EntityFactory::createBaseNBT($location, $velocity, $location->yaw, $location->pitch),
 			$thrower
 		);
-		return $projectile;
 	}
 
 	public function getThrowForce() : float{

src/item/SpawnEgg.php

@@ -29,36 +29,19 @@ use pocketmine\entity\EntityFactory;
 use pocketmine\math\Vector3;
 use pocketmine\player\Player;
 use pocketmine\utils\Utils;
+use pocketmine\world\World;
 use function lcg_value;
 
-class SpawnEgg extends Item{
+abstract class SpawnEgg extends Item{
 
-	/**
-	 * @var string
-	 * @phpstan-var class-string<Entity>
-	 */
-	private $entityClass;
-
-	/**
-	 * @param string $entityClass instanceof Entity
-	 * @phpstan-param class-string<Entity> $entityClass
-	 *
-	 * @throws \InvalidArgumentException
-	 */
-	public function __construct(int $id, int $variant, string $name, string $entityClass){
-		parent::__construct($id, $variant, $name);
-		Utils::testValidInstance($entityClass, Entity::class);
-		$this->entityClass = $entityClass;
-	}
+	abstract protected function createEntity(World $world, Vector3 $pos, float $yaw, float $pitch) : Entity;
 
 	public function onActivate(Player $player, Block $blockReplace, Block $blockClicked, int $face, Vector3 $clickVector) : ItemUseResult{
-		$nbt = EntityFactory::createBaseNBT($blockReplace->getPos()->add(0.5, 0, 0.5), null, lcg_value() * 360, 0);
+		$entity = $this->createEntity($player->getWorld(), $blockReplace->getPos()->add(0.5, 0, 0.5), lcg_value() * 360, 0);
 
 		if($this->hasCustomName()){
-			$nbt->setString("CustomName", $this->getCustomName());
+			$entity->setNameTag($this->getCustomName());
 		}
-
-		$entity = EntityFactory::getInstance()->create($this->entityClass, $player->getWorld(), $nbt);
 		$this->pop();
 		$entity->spawnToAll();
 		//TODO: what if the entity was marked for deletion?

src/item/SplashPotion.php

@@ -36,10 +36,8 @@ class SplashPotion extends ProjectileItem{
 		return 1;
 	}
 
-	protected function createEntity(EntityFactory $factory, Location $location, Vector3 $velocity, Player $thrower) : Throwable{
-		/** @var SplashPotionEntity $projectile */
-		$projectile = $factory->create(
-			SplashPotionEntity::class,
+	protected function createEntity(Location $location, Vector3 $velocity, Player $thrower) : Throwable{
+		$projectile = new SplashPotionEntity(
 			$location->getWorldNonNull(),
 			EntityFactory::createBaseNBT($location, $velocity, $location->yaw, $location->pitch),
 			$thrower

src/world/World.php

@@ -1391,8 +1391,7 @@ class World implements ChunkManager{
 		$nbt->setShort("PickupDelay", $delay);
 		$nbt->setTag("Item", $item->nbtSerialize());
 
-		/** @var ItemEntity $itemEntity */
-		$itemEntity = EntityFactory::getInstance()->create(ItemEntity::class, $this, $nbt);
+		$itemEntity = new ItemEntity($this, $nbt);
 		$itemEntity->spawnToAll();
 		return $itemEntity;
 
@@ -1416,8 +1415,7 @@ class World implements ChunkManager{
 			);
 			$nbt->setShort(ExperienceOrb::TAG_VALUE_PC, $split);
 
-			/** @var ExperienceOrb $orb */
-			$orb = EntityFactory::getInstance()->create(ExperienceOrb::class, $this, $nbt);
+			$orb = new ExperienceOrb($this, $nbt);
 			$orb->spawnToAll();
 			$orbs[] = $orb;
 		}

tests/phpstan/configs/phpstan-bugs.neon

@@ -20,16 +20,6 @@ parameters:
 			count: 3
 			path: ../../../src/command/CommandReader.php
 
-		-
-			message: "#^Closure invoked with 2 parameters, at least 3 required\\.$#"
-			count: 1
-			path: ../../../src/entity/EntityFactory.php
-
-		-
-			message: "#^Only iterables can be unpacked, mixed given in argument \\#3\\.$#"
-			count: 6
-			path: ../../../src/entity/EntityFactory.php
-
 		-
 			message: "#^Call to function assert\\(\\) with false and 'unknown hit type' will always evaluate to false\\.$#"
 			count: 1