[Forge 1.12.2-14.23.4.2747] Bug with spawning entities that have the same class but different names & construction factories

[Cadiboo]

I have a bunch of entities that use the constructor

MyEntity(World worldIn, MyEnum typeIn) {
//...
}

I register them all in the Register EntityEntry event with

for(MyEnum enum : myEnum.values()){
	
	EntityEntryBuilder<Entity> builder = EntityEntryBuilder.create();
	//...
	builder = builder.factory(worldIn -> new MyEntity(worldIn, enum));
    //...
	event.getRegistry().register(builder.build());
	
}

When I spawn these entities with /summon they get created correctly on the server, but on the client they all get created with the same value for typeIn (the last enum in MyEnum.values()) in the constructor.

After stepping through the code with the debugger I have found that the packet they are created from (received from the server obviously) tells them (indirectly) to be created with this constructor. (It creates an entity entry from an EntityRegistration ID that corresponds to the constructor function that constructs my entity like MyEntity(worldIn, MyEnum.<last enum from MyEnum.values()>)

 

The packet is created on the server and on the server the EntityRegistration ID is gotten from an EntityRegistration that is gotten from

net.minecraftforge.fml.common.registry.EntityRegistry.lookupModSpawn(Class<? extends Entity> clazz, boolean keepLooking)

which always returns an EntityRegistration ID that corresponds to the constructor function that constructs my entity like MyEntity(worldIn, MyEnum.<last enum from MyEnum.values()) and NOT like MyEntity(worldIn, MyEnum.<Correct value>).

 

This leads to my entity having the wrong type on the client but having the correct type on the server.

I've temporarily fixed this by using the EntityDataManager in my class and putting

public EntityRailgunSlug(World worldIn, MyEnum typeIn) {
	super(worldIn);
//	this.type = typeIn;
	if (!world.isRemote)
	    this.dataManager.set(TYPE, typeIn.getId());
}

in my entity constructor. However, I don't think I should have to do this and I'm not experienced enough to make a pull request fixing this (if it is a bug)

 

Link to my GitHub where my code that shows this is

https://github.com/Cadiboo/WIPTechAlpha/blob/master/src/main/java/cadiboo/wiptech/entity/projectile/EntityRailgunSlug.java

https://github.com/Cadiboo/WIPTechAlpha/blob/master/src/main/java/cadiboo/wiptech/EventSubscriber.java (onRegisterEntitiesEvent)

https://github.com/Cadiboo/WIPTechAlpha/blob/master/src/main/java/cadiboo/wiptech/util/ModEnums.java (ModMaterials)

 

StackTrace

Spoiler

Thread [Server thread] (Suspended (entry into method lookupModSpawn in EntityRegistry))    
    owns: ArrayDeque<E>  (id=189)    
    EntityRegistry.lookupModSpawn(Class<Entity>, boolean) line: 333    
    FMLNetworkHandler.getEntitySpawningPacket(Entity) line: 124    
    EntityTrackerEntry.createSpawnPacket() line: 523    
    EntityTrackerEntry.updatePlayerEntity(EntityPlayerMP) line: 399    
    EntityTrackerEntry.updatePlayerEntities(List<EntityPlayer>) line: 509    
    EntityTracker.track(Entity, int, int, boolean) line: 235    
    EntityRegistry.tryTrackingEntity(EntityTracker, Entity) line: 369    
    EntityTracker.track(Entity) line: 82    
    ServerWorldEventHandler.onEntityAdded(Entity) line: 43    
    WorldServer(World).onEntityAdded(Entity) line: 1325    
    WorldServer.onEntityAdded(Entity) line: 1175    
    WorldServer(World).spawnEntity(Entity) line: 1316    
    WorldServer.spawnEntity(Entity) line: 1121    
    AnvilChunkLoader.readWorldEntityPos(NBTTagCompound, World, double, double, double, boolean) line: 606    
    CommandSummon.execute(MinecraftServer, ICommandSender, String[]) line: 109    
    IntegratedServerCommandManager(CommandHandler).tryExecute(ICommandSender, String[], ICommand, String) line: 126    
    IntegratedServerCommandManager(CommandHandler).executeCommand(ICommandSender, String) line: 98    
    NetworkDispatcher$1(NetHandlerPlayServer).handleSlashCommand(String) line: 1001    
    NetworkDispatcher$1(NetHandlerPlayServer).processChatMessage(CPacketChatMessage) line: 977    
    CPacketChatMessage.processPacket(INetHandlerPlayServer) line: 47    
    CPacketChatMessage.processPacket(INetHandler) line: 8    
    PacketThreadUtil$1.run() line: 21    
    Executors$RunnableAdapter<T>.call() line: 511    
    ListenableFutureTask<V>(FutureTask<V>).run() line: 266    
    Util.runTask(FutureTask<V>, Logger) line: 53    
    IntegratedServer(MinecraftServer).updateTimeLightAndEntities() line: 798    
    IntegratedServer(MinecraftServer).tick() line: 743    
    IntegratedServer.tick() line: 192    
    IntegratedServer(MinecraftServer).run() line: 592    
    Thread.run() line: 748    

 

How the EntityEntry ID is gotten - FMLNetworkHandler.getEntitySpawningPacket(Entity) line: 134    

Spoiler

net.minecraftforge.fml.common.network.internal.FMLMessage.EntitySpawnMessage.EntitySpawnMessage(EntityRegistration er, Entity entity, ModContainer modContainer)

 

Where the packet is actually created - FMLEmbeddedChannel.generatePacketFrom(Object) line: 61    

Spoiler

net.minecraftforge.fml.common.network.FMLEmbeddedChannel.generatePacketFrom(Object object)

 

Edited August 11, 2018 by Cadiboo

[Cadiboo]

10 hours ago, diesieben07 said:

Use IEntityAdditionalSpawnData instead. The constructor functions are not used in 100% of places (yet?) and this is not really their use case anyways.

Lex told me on my PR that this was intended behaviour & is by design of the MC Entity system which is specifically based on class. I’ll see if I can use IEASD, but I don’t understand how this isn’t the use case for factories. This seems to mean that you can’t use the same class for multiple similar entities (Without IEASDs or the DataManager). I’m using this method(constructor with enum variants) with Blocks and Items, am I not supposed to be doing it this way?

[LexManos]

As I said in the "issue" report. No, you can't use the same class for "similar entities" like this.

Think it through logically, how would anything from the outside know which constructor/parameters you used to create your entity?

Do you see any .getUsedConstructorFactory() on the entity? Do we force you to pass in the factory to your constructor and up the chain of Entity? No.

The Constructor Factories are just a cleaner solution to the reflection that used to be done.

This is just how the system works, there is no generic way to identify the 'type' of a entity except for the class. 

If you want sub-types then you can use SpawnData.

 

 

[Cadiboo]

10 minutes ago, LexManos said:

If you want sub-types then you can use SpawnData.

 

 

Ok thanks! I’ll use that

 

11 minutes ago, LexManos said:

Think it through logically, how would anything from the outside know which constructor/parameters you used to create your entity?

I assume because they were registered as separate entities with different factories

 

Can you explain the use of specifying a factory in the EntityEntry?

[LexManos]

As I said the 'use' of specifying a factory is to allow you to get around the hacky reflection requiring a single constructor with only a world argument.

It's better code.

And again, just because you registered the same class to multiple names, doesn't mean anyone can figure out what name you used to create the entity.

Everything is based on class.