Mixin Entity Packet Loss Fixer for Server-Side Logic

src/main/java/kamkeel/npcs/network/PacketHandler.java

@@ -649,7 +649,6 @@ public class PacketHandler {
         PLAYER_PACKET.registerPacket(new SpecialKeyStatePacket());
 
         PLAYER_PACKET.registerPacket(new SyncRevisionInfoPacket());
-        PLAYER_PACKET.registerPacket(new RequestProperSpawnData());
 
         // CustomGUI Packets
         PLAYER_PACKET.registerPacket(new CustomGuiButtonPacket());

src/main/java/kamkeel/npcs/network/enums/EnumPlayerPacket.java

@@ -42,7 +42,4 @@ public enum EnumPlayerPacket {
     AbilityHotbarSave,
     AbilityHotbarSelect,
     AbilityToggle,
-
-    // NPC Spawn Data Fix
-    RequestSpawnData,
 }

src/main/java/kamkeel/npcs/network/packets/data/RequestProperSpawnData.java

@@ -1,96 +0,0 @@
-package kamkeel.npcs.network.packets.data;
-
-import cpw.mods.fml.relauncher.Side;
-import cpw.mods.fml.relauncher.SideOnly;
-import io.netty.buffer.ByteBuf;
-import kamkeel.npcs.network.AbstractPacket;
-import kamkeel.npcs.network.PacketChannel;
-import kamkeel.npcs.network.PacketClient;
-import kamkeel.npcs.network.PacketHandler;
-import kamkeel.npcs.network.enums.EnumPlayerPacket;
-import kamkeel.npcs.network.packets.data.npc.UpdateNpcPacket;
-import net.minecraft.entity.Entity;
-import net.minecraft.entity.player.EntityPlayer;
-import net.minecraft.entity.player.EntityPlayerMP;
-import net.minecraft.nbt.NBTTagCompound;
-import noppes.npcs.entity.EntityNPCInterface;
-
-import java.io.IOException;
-
-public class RequestProperSpawnData extends AbstractPacket {
-    public static final String packetName = "Player|SpawnData";
-    private static final long REQUEST_COOLDOWN_MILLIS = 1000L;
-    private int entityId;
-
-    public RequestProperSpawnData() {}
-
-    public RequestProperSpawnData(int entityId) {
-        this.entityId = entityId;
-    }
-
-    @SideOnly(Side.CLIENT)
-    public static void clear() {
-        // No backlog queue to clear.
-    }
-
-    @Override
-    public Enum getType() {
-        return EnumPlayerPacket.RequestSpawnData;
-    }
-
-    @Override
-    public PacketChannel getChannel() {
-        return PacketHandler.PLAYER_PACKET;
-    }
-
-    @Override
-    @SideOnly(Side.CLIENT)
-    public void sendData(ByteBuf out) throws IOException {
-        out.writeInt(this.entityId);
-    }
-
-    @Override
-    public void receiveData(ByteBuf in, EntityPlayer player) throws IOException {
-        if (!(player instanceof EntityPlayerMP) || player.worldObj == null || player.worldObj.isRemote)
-            return;
-
-        int entityId = in.readInt();
-        Entity entity = player.worldObj.getEntityByID(entityId);
-        if (entity instanceof EntityNPCInterface) {
-            EntityNPCInterface npc = (EntityNPCInterface) entity;
-            NBTTagCompound compound = npc.writeSpawnData();
-            compound.setInteger("EntityId", npc.getEntityId());
-            PacketHandler.Instance.sendToPlayer(new UpdateNpcPacket(compound), (EntityPlayerMP) player);
-        }
-    }
-
-    @SideOnly(Side.CLIENT)
-    public static void reportMissingData(EntityNPCInterface npc) {
-        if (npc == null || npc.worldObj == null || !npc.worldObj.isRemote)
-            return;
-
-        int entityID = npc.getEntityId();
-        if (entityID <= 0)
-            return;
-
-        long now = System.currentTimeMillis();
-        if (now - npc.clientFixLastRequestMillis < REQUEST_COOLDOWN_MILLIS)
-            return;
-        if (npc.clientFixAttempts >= 3)
-            return;
-
-        npc.clientFixLastRequestMillis = now;
-        npc.clientFixAttempts++;
-        PacketClient.sendClient(new RequestProperSpawnData(entityID));
-    }
-
-    @SideOnly(Side.CLIENT)
-    public static boolean canDoBatchUpdate() {
-        return false;
-    }
-
-    @SideOnly(Side.CLIENT)
-    public static void handleBacklog() {
-        // No batch backlog. Requests are immediate and throttled in reportMissingData().
-    }
-}

src/main/java/noppes/npcs/client/ClientTickHandler.java

@@ -9,7 +9,6 @@ import kamkeel.npcs.client.renderer.lightning.LightningBolt;
 import kamkeel.npcs.controllers.data.energycharge.EnergyChargePreviewManager;
 import kamkeel.npcs.controllers.data.telegraph.TelegraphManager;
 import kamkeel.npcs.network.PacketClient;
-import kamkeel.npcs.network.packets.data.RequestProperSpawnData;
 import kamkeel.npcs.network.packets.player.CheckPlayerValue;
 import kamkeel.npcs.network.packets.player.ScreenSizePacket;
 import net.minecraft.client.Minecraft;
@@ -50,7 +49,6 @@ public class ClientTickHandler {
             if (mc.theWorld == null) {
                 ClientCacheHandler.clearCache();
                 ClientAbilityState.reset();
-                RequestProperSpawnData.clear();
                 if (EnergyChargePreviewManager.ClientInstance != null) {
                     EnergyChargePreviewManager.ClientInstance.clear();
                 }

src/main/java/noppes/npcs/config/ConfigMixin.java

@@ -11,6 +11,7 @@ public class ConfigMixin {
     public static Configuration config;
 
     public final static String CLIENT = "CLIENT";
+    public final static String SERVER = "SERVER";
     public final static String GENERAL = "GENERAL";
 
     /**
@@ -25,6 +26,9 @@ public class ConfigMixin {
     public static Property FirstPersonAnimationMixinProperty;
     public static boolean FirstPersonAnimationMixin = true;
 
+    public static Property EntitySpawnFixMixinProperty;
+    public static boolean EntitySpawnFixMixin = true;
+
     public static void init(File configFile) {
         config = new Configuration(configFile);
 
@@ -43,6 +47,9 @@ public class ConfigMixin {
             } else {
                 FirstPersonAnimationMixin = false;
             }
+
+            EntitySpawnFixMixinProperty = config.get(SERVER, "Entity Spawn Packet Fix", true, "Fixes a Forge race condition where mod entity spawn packets can be lost or sent to the wrong player. This prevents invisible entities on servers with multiple players.");
+            EntitySpawnFixMixin = EntitySpawnFixMixinProperty.getBoolean(true);
         } catch (Exception e) {
             FMLLog.log(Level.ERROR, e, "CNPC+ has had a problem loading its mixin configuration");
         } finally {

src/main/java/noppes/npcs/entity/EntityNPCInterface.java

@@ -14,7 +14,6 @@ import kamkeel.npcs.network.PacketHandler;
 import kamkeel.npcs.network.enums.EnumSoundOperation;
 import kamkeel.npcs.network.packets.data.ChatBubblePacket;
 import kamkeel.npcs.network.packets.data.QuestCompletionPacket;
-import kamkeel.npcs.network.packets.data.RequestProperSpawnData;
 import kamkeel.npcs.network.packets.data.SoundManagementPacket;
 import kamkeel.npcs.network.packets.data.npc.UpdateNpcPacket;
 import kamkeel.npcs.network.packets.data.npc.WeaponNpcPacket;
@@ -234,10 +233,6 @@ public abstract class EntityNPCInterface extends EntityCreature implements IEnti
     public FlyingMoveHelper flyMoveHelper = new FlyingMoveHelper(this);
     public PathNavigate flyNavigator = new PathNavigateFlying(this, worldObj);
 
-    // For Client-Side Use Only
-    public int clientFixAttempts = 0;
-    public long clientFixLastRequestMillis = 0L;
-
     public EntityNPCInterface(World world) {
         super(world);
         try {
@@ -1846,7 +1841,6 @@ public abstract class EntityNPCInterface extends EntityCreature implements IEnti
         NBTTagCompound compound = new NBTTagCompound();
         display.writeToNBT(compound);
         compound.setDouble("MaxHealth", stats.maxHealth);
-        compound.setFloat("CurrentHealth", getHealth());
         compound.setTag("Armor", NBTTags.nbtItemStackList(inventory.getArmor()));
         compound.setTag("Weapons", NBTTags.nbtItemStackList(inventory.getWeapons()));
         compound.setInteger("Speed", ais.getWalkingSpeed());
@@ -1889,19 +1883,11 @@ public abstract class EntityNPCInterface extends EntityCreature implements IEnti
         try {
             readSpawnData(ByteBufUtils.readNBT(buf));
         } catch (IOException e) {
-            if (this.worldObj != null && this.worldObj.isRemote) {
-                RequestProperSpawnData.reportMissingData(this);
-            }
+            e.printStackTrace();
         }
     }
 
     public void readSpawnData(NBTTagCompound compound) {
-        if (this.worldObj != null && this.worldObj.isRemote) {
-            if(requestClientFix(compound))
-                return;
-        }
-
-        float preservedHealth = getHealth();
         stats.maxHealth = compound.getDouble("MaxHealth");
         ais.setWalkingSpeed(compound.getInteger("Speed"));
         stats.hideKilledBody = compound.getBoolean("DeadBody");
@@ -1926,12 +1912,6 @@ public abstract class EntityNPCInterface extends EntityCreature implements IEnti
         ais.bodyOffsetY = compound.getFloat("OffsetY");
 
         this.getEntityAttribute(SharedMonsterAttributes.maxHealth).setBaseValue(stats.maxHealth);
-        if (compound.hasKey("CurrentHealth", Constants.NBT.TAG_FLOAT)) {
-            setHealth(MathHelper.clamp_float(compound.getFloat("CurrentHealth"), 0.0F, getMaxHealth()));
-        } else {
-            // Keep previous client health if packet came from an older sender format.
-            setHealth(MathHelper.clamp_float(preservedHealth, 0.0F, getMaxHealth()));
-        }
         inventory.setArmor(NBTTags.getItemStackList(compound.getTagList("Armor", 10)));
         inventory.setWeapons(NBTTags.getItemStackList(compound.getTagList("Weapons", 10)));
         advanced.setRole(compound.getInteger("Role"));
@@ -2000,16 +1980,6 @@ public abstract class EntityNPCInterface extends EntityCreature implements IEnti
         }
     }
 
-    public boolean requestClientFix(NBTTagCompound compound){
-        if((compound.hasNoTags() || !compound.hasKey("MaxHealth", Constants.NBT.TAG_DOUBLE))){
-            RequestProperSpawnData.reportMissingData(this);
-            return true;
-        }
-        clientFixAttempts = 0;
-        clientFixLastRequestMillis = 0L;
-        return false;
-    }
-
     public boolean canBreatheUnderwater() {
         return stats.drowningType != 1;
     }

src/main/java/noppes/npcs/mixin/CustomNPCsMixinPlugin.java

@@ -57,6 +57,14 @@ public class CustomNPCsMixinPlugin implements IMixinConfigPlugin {
 
             mixins.add("MixinItemStack");
         }
+
+        // Server Only Mixins
+        if (!client) {
+            if (ConfigMixin.EntitySpawnFixMixin) {
+                mixins.add("MixinEntityTrackerEntry");
+            }
+        }
+
         return mixins;
     }
 

src/main/java/noppes/npcs/mixin/MixinEntityTrackerEntry.java

@@ -0,0 +1,77 @@
+package noppes.npcs.mixin;
+
+import cpw.mods.fml.common.network.internal.FMLMessage;
+import cpw.mods.fml.common.network.internal.FMLProxyPacket;
+import cpw.mods.fml.common.registry.EntityRegistry;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import net.minecraft.entity.Entity;
+import net.minecraft.entity.EntityTrackerEntry;
+import net.minecraft.network.Packet;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Redirect;
+
+import java.lang.reflect.Method;
+
+/**
+ * Fixes a race condition in Forge's FMLNetworkHandler.getEntitySpawningPacket.
+ *
+ * Forge uses a single static shared EmbeddedChannel to serialize all mod entity
+ * spawn packets. The channel's generatePacketFrom method does a non-atomic
+ * write-then-poll, so concurrent calls can swap, lose, or misdeliver packets.
+ *
+ * This mixin bypasses the shared channel by serializing directly into a fresh
+ * ByteBuf per call, eliminating the race condition entirely.
+ */
+@Mixin(EntityTrackerEntry.class)
+public abstract class MixinEntityTrackerEntry {
+    private static final Logger LOGGER = LogManager.getLogger("CustomNPCs-SpawnFix");
+    private static Method toBytesMethod;
+
+    static {
+        try {
+            toBytesMethod = FMLMessage.EntitySpawnMessage.class.getDeclaredMethod("toBytes", ByteBuf.class);
+            toBytesMethod.setAccessible(true);
+        } catch (NoSuchMethodException e) {
+            LOGGER.fatal("Failed to find FMLMessage.EntitySpawnMessage.toBytes - entity spawn fix will not work", e);
+        }
+    }
+
+    @Shadow
+    public Entity myEntity;
+
+    @Redirect(
+        method = "func_151260_c",
+        at = @At(
+            value = "INVOKE",
+            target = "Lcpw/mods/fml/common/network/internal/FMLNetworkHandler;getEntitySpawningPacket(Lnet/minecraft/entity/Entity;)Lnet/minecraft/network/Packet;"
+        ),
+        remap = false
+    )
+    private Packet cnpcplus$getEntitySpawningPacket(Entity entity) {
+        if (toBytesMethod == null) {
+            return null;
+        }
+
+        EntityRegistry.EntityRegistration er = EntityRegistry.instance().lookupModSpawn(entity.getClass(), false);
+        if (er == null || er.usesVanillaSpawning()) {
+            return null;
+        }
+
+        FMLMessage.EntitySpawnMessage message = new FMLMessage.EntitySpawnMessage(er, entity, er.getContainer());
+        ByteBuf buf = Unpooled.buffer();
+        buf.writeByte(2);
+        try {
+            toBytesMethod.invoke(message, buf);
+        } catch (Throwable e) {
+            LOGGER.warn("Failed to serialize entity spawn packet for {}", entity.getClass().getName(), e);
+            buf.release();
+            return null;
+        }
+        return new FMLProxyPacket(buf, "FML");
+    }
+}