Fix mana spreader entity ID leak (#162)

src/main/java/vazkii/botania/client/core/proxy/ClientProxy.java

@@ -381,24 +381,38 @@ public class ClientProxy extends CommonProxy {
 	@Override
 	public void setExtraReach(EntityLivingBase entity, float reach) {
 		super.setExtraReach(entity, reach);
+		IExtendedPlayerController controller = getReachController(entity);
+		if(controller != null)
+			controller.setReachDistanceExtension(Math.max(0, controller.getReachDistanceExtension() + reach));
+	}
+
+	@Override
+	public void setClientExtraReach(EntityLivingBase entity, float reach) {
+		IExtendedPlayerController controller = getReachController(entity);
+		if(controller != null)
+			controller.setReachDistanceExtension(Math.max(0, reach));
+	}
+
+	private IExtendedPlayerController getReachController(EntityLivingBase entity) {
 		Minecraft mc = Minecraft.getMinecraft();
 		EntityPlayer player = mc.thePlayer;
 		// Player object might have changed, need to check the UUID.
-		if(entity.worldObj.isRemote && entity.getUniqueID().equals(player.getUniqueID())) {
-			if(!(mc.playerController instanceof IExtendedPlayerController)) {
-				GameType type = ReflectionHelper.getPrivateValue(PlayerControllerMP.class, mc.playerController, LibObfuscation.CURRENT_GAME_TYPE);
-				NetHandlerPlayClient net = ReflectionHelper.getPrivateValue(PlayerControllerMP.class, mc.playerController, LibObfuscation.NET_CLIENT_HANDLER);
-				BotaniaPlayerController controller = new BotaniaPlayerController(mc, net);
-				boolean isFlying = player.capabilities.isFlying;
-				boolean allowFlying = player.capabilities.allowFlying;
-				controller.setGameType(type);
-				player.capabilities.isFlying = isFlying;
-				player.capabilities.allowFlying = allowFlying;
-				mc.playerController = controller;
-			}
+		if(!entity.worldObj.isRemote || !entity.getUniqueID().equals(player.getUniqueID()))
+			return null;
 
-			((IExtendedPlayerController) mc.playerController).setReachDistanceExtension(Math.max(0, ((IExtendedPlayerController) mc.playerController).getReachDistanceExtension() + reach));
+		if(!(mc.playerController instanceof IExtendedPlayerController)) {
+			GameType type = ReflectionHelper.getPrivateValue(PlayerControllerMP.class, mc.playerController, LibObfuscation.CURRENT_GAME_TYPE);
+			NetHandlerPlayClient net = ReflectionHelper.getPrivateValue(PlayerControllerMP.class, mc.playerController, LibObfuscation.NET_CLIENT_HANDLER);
+			BotaniaPlayerController controller = new BotaniaPlayerController(mc, net);
+			boolean isFlying = player.capabilities.isFlying;
+			boolean allowFlying = player.capabilities.allowFlying;
+			controller.setGameType(type);
+			player.capabilities.isFlying = isFlying;
+			player.capabilities.allowFlying = allowFlying;
+			mc.playerController = controller;
 		}
+
+		return (IExtendedPlayerController) mc.playerController;
 	}
 
 	@Override

src/main/java/vazkii/botania/client/integration/nei/recipe/RecipeHandlerBrewery.java

@@ -137,7 +137,7 @@ public class RecipeHandlerBrewery extends TemplateRecipeHandler {
 
             for (ItemStack emptyContainer : BrewHelper.getBrewContainers()) {
                 final ItemStack filledContainer = recipe.getOutput(emptyContainer);
-                if (result.isItemEqual(filledContainer) && Objects.equals(result.stackTagCompound, filledContainer.stackTagCompound)) {
+                if (filledContainer != null && result.isItemEqual(filledContainer) && Objects.equals(result.stackTagCompound, filledContainer.stackTagCompound)) {
                     arecipes.add(new CachedBreweryRecipe(recipe, emptyContainer));
                 }
             }

src/main/java/vazkii/botania/client/integration/nei/recipe/RecipeHandlerPetalApothecary.java

@@ -40,6 +40,13 @@ public class RecipeHandlerPetalApothecary extends TemplateRecipeHandler {
             output = new PositionedStack(recipe.getOutput(), 111, 21);
         }
 
+        // Kept for Alfheim, which calls super(recipe, false) from its NEI handler.
+        @Deprecated
+        @SuppressWarnings("unused")
+        public CachedPetalApothecaryRecipe(RecipePetals recipe, boolean addCenterItem) {
+            this(recipe);
+        }
+
         public void setIngredients(List<Object> inputs) {
             float degreePerInput = 360F / inputs.size();
             float currentDegree = -90F;
@@ -74,10 +81,10 @@ public class RecipeHandlerPetalApothecary extends TemplateRecipeHandler {
 
     @Override
     public String getOverlayIdentifier() {
-        return "botania.petalApothecary";
+        return getRecipeID();
     }
 
-    // Used by the Alfheim addon, identical to getOverlayIdentifier()
+    // Kept for Alfheim, which overrides this instead of getOverlayIdentifier().
     @Deprecated
     public String getRecipeID() {
         return "botania.petalApothecary";

src/main/java/vazkii/botania/client/render/tile/RenderTileCocoon.java

@@ -33,7 +33,8 @@ public class RenderTileCocoon extends TileEntitySpecialRenderer {
 
 		float rot = 0F;
 		float modval = 60F - (float) cocoon.timePassed / (float) TileCocoon.TOTAL_TIME * 30F;
-		if(cocoon.timePassed % modval < 10) {
+		// At timePassed 0 with partialTicks 0 (inventory/NEI render), log(0) makes rot NaN and the model disappears.
+		if(cocoon.timePassed > 0 && cocoon.timePassed % modval < 10) {
 			float mod = (cocoon.timePassed + partialTicks) % modval;
 			float v = mod / 5 * (float) Math.PI * 2;
 			rot = (float) Math.sin(v) * (float) Math.log(cocoon.timePassed + partialTicks);

src/main/java/vazkii/botania/common/block/BlockAlfPortal.java

@@ -62,10 +62,13 @@ public class BlockAlfPortal extends BlockModContainer<TileAlfPortal> implements
 
 	@Override
 	public boolean onUsedByWand(EntityPlayer player, ItemStack stack, World world, int x, int y, int z, int side) {
-		boolean did = ((TileAlfPortal) world.getTileEntity(x, y, z)).onWanded();
-		if(did && player != null)
-			player.addStat(ModAchievements.elfPortalOpen, 1);
-		return did;
+		if(!world.isRemote) {
+			boolean did = ((TileAlfPortal) world.getTileEntity(x, y, z)).onWanded();
+			if(did && player != null)
+				player.addStat(ModAchievements.elfPortalOpen, 1);
+			return did;
+		}
+		return true;
 	}
 
 	@Override

src/main/java/vazkii/botania/common/block/tile/TileAlfPortal.java

@@ -167,7 +167,8 @@ public class TileAlfPortal extends TileMod {
 		} else closeNow = false;
 
 		if(closeNow) {
-			worldObj.setBlockMetadataWithNotify(xCoord, yCoord, zCoord, 0, 1 | 2);
+			if(!worldObj.isRemote)
+				worldObj.setBlockMetadataWithNotify(xCoord, yCoord, zCoord, 0, 1 | 2);
 			for(int i = 0; i < 36; i++)
 				blockParticle(meta);
 			closeNow = false;
@@ -175,7 +176,8 @@ public class TileAlfPortal extends TileMod {
 			if(newMeta == 0)
 				for(int i = 0; i < 36; i++)
 					blockParticle(meta);
-			worldObj.setBlockMetadataWithNotify(xCoord, yCoord, zCoord, newMeta, 1 | 2);
+			if(!worldObj.isRemote)
+				worldObj.setBlockMetadataWithNotify(xCoord, yCoord, zCoord, newMeta, 1 | 2);
 		} else if(explode) {
 			worldObj.createExplosion(null, xCoord + .5, yCoord + 2.0, zCoord + .5, 3f, true);
 			explode = false;

src/main/java/vazkii/botania/common/block/tile/mana/TileSpreader.java

@@ -302,7 +302,7 @@ public class TileSpreader extends TileSimpleInventory implements IManaCollector,
 			long least = compound.getLong(TAG_UUID_LEAST);
 			UUID identity = getIdentifierUnsafe();
 			if(identity == null || most != identity.getMostSignificantBits() || least != identity.getLeastSignificantBits())
-				identity = new UUID(most, least);
+				this.identity = new UUID(most, least);
 		} else getIdentifier();
 
 		mana = compound.getInteger(TAG_MANA);
@@ -464,8 +464,6 @@ public class TileSpreader extends TileSimpleInventory implements IManaCollector,
 	}
 
 	public EntityManaBurst getBurst(boolean fake) {
-		EntityManaBurst burst = new EntityManaBurst(this, fake);
-
 		boolean dreamwood = isDreamwood();
 		boolean ultra = isULTRA_SPREADER();
 		int maxMana = ultra ? 640 : dreamwood ? 240 : 160;
@@ -480,29 +478,31 @@ public class TileSpreader extends TileSimpleInventory implements IManaCollector,
 		if(lens != null && lens.getItem() instanceof ILensEffect)
 			((ILensEffect) lens.getItem()).apply(lens, props);
 
-		burst.setSourceLens(lens);
-		if(getCurrentMana() >= props.maxMana || fake) {
-			if(mapmakerOverride) {
-				burst.setColor(mmForcedColor);
-				burst.setMana(mmForcedManaPayload);
-				burst.setStartingMana(mmForcedManaPayload);
-				burst.setMinManaLoss(mmForcedTicksBeforeManaLoss);
-				burst.setManaLossPerTick(mmForcedManaLossPerTick);
-				burst.setGravity(mmForcedGravity);
-				burst.setMotion(burst.motionX * mmForcedVelocityMultiplier, burst.motionY * mmForcedVelocityMultiplier, burst.motionZ * mmForcedVelocityMultiplier);
-			} else {
-				burst.setColor(props.color);
-				burst.setMana(props.maxMana);
-				burst.setStartingMana(props.maxMana);
-				burst.setMinManaLoss(props.ticksBeforeManaLoss);
-				burst.setManaLossPerTick(props.manaLossPerTick);
-				burst.setGravity(props.gravity);
-				burst.setMotion(burst.motionX * props.motionModifier, burst.motionY * props.motionModifier, burst.motionZ * props.motionModifier);
-			}
+		if(getCurrentMana() < props.maxMana && !fake)
+			return null;
 
-			return burst;
+		// Construct the entity only after the mana check, every new Entity takes a global entity ID
+		EntityManaBurst burst = new EntityManaBurst(this, fake);
+		burst.setSourceLens(lens);
+		if(mapmakerOverride) {
+			burst.setColor(mmForcedColor);
+			burst.setMana(mmForcedManaPayload);
+			burst.setStartingMana(mmForcedManaPayload);
+			burst.setMinManaLoss(mmForcedTicksBeforeManaLoss);
+			burst.setManaLossPerTick(mmForcedManaLossPerTick);
+			burst.setGravity(mmForcedGravity);
+			burst.setMotion(burst.motionX * mmForcedVelocityMultiplier, burst.motionY * mmForcedVelocityMultiplier, burst.motionZ * mmForcedVelocityMultiplier);
+		} else {
+			burst.setColor(props.color);
+			burst.setMana(props.maxMana);
+			burst.setStartingMana(props.maxMana);
+			burst.setMinManaLoss(props.ticksBeforeManaLoss);
+			burst.setManaLossPerTick(props.manaLossPerTick);
+			burst.setGravity(props.gravity);
+			burst.setMotion(burst.motionX * props.motionModifier, burst.motionY * props.motionModifier, burst.motionZ * props.motionModifier);
 		}
-		return null;
+
+		return burst;
 	}
 
 	public ILensControl getLensController(ItemStack stack) {

src/main/java/vazkii/botania/common/core/proxy/CommonProxy.java

@@ -256,6 +256,13 @@ public class CommonProxy {
 			((EntityPlayerMP) entity).theItemInWorldManager.setBlockReachDistance(Math.max(5, ((EntityPlayerMP) entity).theItemInWorldManager.getBlockReachDistance() + reach));
 	}
 
+	/**
+	 * Sets (instead of adds to) the extra reach of the client player. No-op on the server.
+	 */
+	public void setClientExtraReach(EntityLivingBase entity, float reach) {
+		// NO-OP
+	}
+
 	public boolean openWikiPage(World world, Block block, MovingObjectPosition pos) {
 		return false;
 	}

src/main/java/vazkii/botania/common/item/equipment/bauble/ItemReachRing.java

@@ -11,10 +11,13 @@
 package vazkii.botania.common.item.equipment.bauble;
 
 import net.minecraft.entity.EntityLivingBase;
+import net.minecraft.entity.player.EntityPlayer;
+import net.minecraft.inventory.IInventory;
 import net.minecraft.item.ItemStack;
 import vazkii.botania.common.Botania;
 import vazkii.botania.common.lib.LibItemNames;
 import baubles.api.BaubleType;
+import baubles.api.BaublesApi;
 
 public class ItemReachRing extends ItemBauble {
 
@@ -27,6 +30,29 @@ public class ItemReachRing extends ItemBauble {
 		Botania.proxy.setExtraReach(player, 3.5F);
 	}
 
+	@Override
+	public void onPlayerLoad(ItemStack stack, EntityLivingBase player) {
+		if(player.worldObj.isRemote) {
+			// Baubles resends all baubles to the client on login and on every dimension change and calls this for each
+			// of them, but the client keeps its reach extension across dimension changes. Set it instead of adding to it.
+			Botania.proxy.setClientExtraReach(player, 3.5F * countWornRings(player));
+		} else super.onPlayerLoad(stack, player);
+	}
+
+	private static int countWornRings(EntityLivingBase player) {
+		if(!(player instanceof EntityPlayer))
+			return 0;
+
+		int count = 0;
+		IInventory baubles = BaublesApi.getBaubles((EntityPlayer) player);
+		for(int i = 0; i < baubles.getSizeInventory(); i++) {
+			ItemStack bauble = baubles.getStackInSlot(i);
+			if(bauble != null && bauble.getItem() instanceof ItemReachRing)
+				count++;
+		}
+		return count;
+	}
+
 	@Override
 	public void onUnequipped(ItemStack stack, EntityLivingBase player) {
 		Botania.proxy.setExtraReach(player, -3.5F);