diff --git a/src/main/java/kamkeel/npcs/network/PacketHandler.java b/src/main/java/kamkeel/npcs/network/PacketHandler.java index 71b8a0092..8466f55fe 100644 --- a/src/main/java/kamkeel/npcs/network/PacketHandler.java +++ b/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()); diff --git a/src/main/java/kamkeel/npcs/network/enums/EnumPlayerPacket.java b/src/main/java/kamkeel/npcs/network/enums/EnumPlayerPacket.java index 3453b6eb4..e1587467b 100644 --- a/src/main/java/kamkeel/npcs/network/enums/EnumPlayerPacket.java +++ b/src/main/java/kamkeel/npcs/network/enums/EnumPlayerPacket.java @@ -42,7 +42,4 @@ public enum EnumPlayerPacket { AbilityHotbarSave, AbilityHotbarSelect, AbilityToggle, - - // NPC Spawn Data Fix - RequestSpawnData, } diff --git a/src/main/java/kamkeel/npcs/network/packets/data/RequestProperSpawnData.java b/src/main/java/kamkeel/npcs/network/packets/data/RequestProperSpawnData.java deleted file mode 100644 index fce6c13fa..000000000 --- a/src/main/java/kamkeel/npcs/network/packets/data/RequestProperSpawnData.java +++ /dev/null @@ -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(). - } -} diff --git a/src/main/java/noppes/npcs/client/ClientTickHandler.java b/src/main/java/noppes/npcs/client/ClientTickHandler.java index 43d619462..1fa03a34a 100644 --- a/src/main/java/noppes/npcs/client/ClientTickHandler.java +++ b/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(); } diff --git a/src/main/java/noppes/npcs/config/ConfigMixin.java b/src/main/java/noppes/npcs/config/ConfigMixin.java index c9491fe0e..5ee2029fb 100644 --- a/src/main/java/noppes/npcs/config/ConfigMixin.java +++ b/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 { diff --git a/src/main/java/noppes/npcs/entity/EntityNPCInterface.java b/src/main/java/noppes/npcs/entity/EntityNPCInterface.java index 3eed36105..c21e13c8b 100644 --- a/src/main/java/noppes/npcs/entity/EntityNPCInterface.java +++ b/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; } diff --git a/src/main/java/noppes/npcs/mixin/CustomNPCsMixinPlugin.java b/src/main/java/noppes/npcs/mixin/CustomNPCsMixinPlugin.java index 9b09759c0..3848edcd4 100644 --- a/src/main/java/noppes/npcs/mixin/CustomNPCsMixinPlugin.java +++ b/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; } diff --git a/src/main/java/noppes/npcs/mixin/MixinEntityTrackerEntry.java b/src/main/java/noppes/npcs/mixin/MixinEntityTrackerEntry.java new file mode 100644 index 000000000..a66297040 --- /dev/null +++ b/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"); + } +}