Fix CME when removing ticket managers (#5861)

src/main/java/net/minecraftforge/common/FarmlandWaterManager.java

@@ -20,6 +20,7 @@
 package net.minecraftforge.common;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.MapMaker;
 import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
 import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
 import net.minecraft.util.math.AxisAlignedBB;
@@ -30,18 +31,22 @@ import net.minecraft.world.World;
 import net.minecraft.world.chunk.Chunk;
 import net.minecraftforge.common.ticket.AABBTicket;
 import net.minecraftforge.common.ticket.ChunkTicketManager;
-import net.minecraftforge.common.ticket.MultiTicketManager;
 import net.minecraftforge.common.ticket.SimpleTicket;
+import net.minecraftforge.fml.common.FMLLog;
 
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 
 public class FarmlandWaterManager
 {
+    private static boolean DEBUG = Boolean.parseBoolean(System.getProperty("forge.debugFarmlandWaterManager", "false"));
     private static final Int2ObjectMap<Map<ChunkPos, ChunkTicketManager<Vec3d>>> customWaterHandler = new Int2ObjectOpenHashMap<>();
 
+    @Deprecated
+    public static<T extends SimpleTicket<Vec3d>> T addCustomTicket(World world, T ticket, ChunkPos... chunkPoses)
+    {
+        return addCustomTicket(world, ticket, chunkPoses[0], Arrays.copyOfRange(chunkPoses, 1, chunkPoses.length));
+    }
+
     /**
      * Adds a custom ticket.
      * Use {@link #addAABBTicket(World, AxisAlignedBB)} if you just need a ticket that can water a certain area.
@@ -50,26 +55,19 @@ public class FarmlandWaterManager
      * when the region this is unloaded (e.g. your TE is unloaded or the block is removed), and validate once it is loaded
      * @param world The world where the region should be marked. Only server-side worlds are allowed
      * @param ticket Your ticket you want to have registered
-     * @param chunkPoses The chunkPoses where the ticket is located
+     * @param masterChunk The chunk pos that is controls when the ticket may be unloaded. The ticket should originate from here.
+     * @param additionalChunks The chunks in that this ticket wants to operate as well.
      * @return The ticket for your requested region.
      */
     @SuppressWarnings("unchecked")
-    public static<T extends SimpleTicket<Vec3d>> T addCustomTicket(World world, T ticket, ChunkPos... chunkPoses)
+    public static<T extends SimpleTicket<Vec3d>> T addCustomTicket(World world, T ticket, ChunkPos masterChunk, ChunkPos... additionalChunks)
     {
         Preconditions.checkArgument(!world.isRemote, "Water region is only determined server-side");
-        Preconditions.checkArgument(chunkPoses.length > 0, "Need at least one chunk pos");
-        Map<ChunkPos, ChunkTicketManager<Vec3d>> ticketMap = customWaterHandler.computeIfAbsent(world.provider.getDimension(), id -> new HashMap<>());
-        if (chunkPoses.length == 1)
-        {
-            ticket.setBackend(ticketMap.computeIfAbsent(chunkPoses[0], ChunkTicketManager::new));
-        }
-        else
-        {
-            ChunkTicketManager<Vec3d>[] tickets = new ChunkTicketManager[chunkPoses.length];
-            for (int i = 0; i < chunkPoses.length; i++)
-                tickets[i] = ticketMap.computeIfAbsent(chunkPoses[i], ChunkTicketManager::new);
-            ticket.setBackend(new MultiTicketManager<>(tickets));
-        }
+        Map<ChunkPos, ChunkTicketManager<Vec3d>> ticketMap = customWaterHandler.computeIfAbsent(world.provider.getDimension(), id -> new MapMaker().weakValues().makeMap());
+        ChunkTicketManager<Vec3d>[] additionalTickets = new ChunkTicketManager[additionalChunks.length];
+        for (int i = 0; i < additionalChunks.length; i++)
+            additionalTickets[i] = ticketMap.computeIfAbsent(additionalChunks[i], ChunkTicketManager::new);
+        ticket.setManager(ticketMap.computeIfAbsent(masterChunk, ChunkTicketManager::new), additionalTickets);
         ticket.validate();
         return ticket;
     }
@@ -87,6 +85,8 @@ public class FarmlandWaterManager
      */
     public static AABBTicket addAABBTicket(World world, AxisAlignedBB aabb)
     {
+        if (DEBUG)
+            FMLLog.log.info("FarmlandWaterManager: New AABBTicket, aabb={}", aabb);
         //First calculate all chunks the aabb is in
         ChunkPos leftUp = new ChunkPos(((int) aabb.minX) >> 4, ((int) aabb.minZ) >> 4);
         ChunkPos rightDown = new ChunkPos(((int) aabb.maxX) >> 4, ((int) aabb.maxZ) >> 4);
@@ -98,7 +98,35 @@ public class FarmlandWaterManager
                 posSet.add(new ChunkPos(x, z));
             }
         }
-        return addCustomTicket(world, new AABBTicket(aabb), posSet.toArray(new ChunkPos[0]));
+        ChunkPos masterPos = null;
+        double masterDistance = Double.MAX_VALUE;
+        for (ChunkPos pos : posSet) //Find the chunkPos with the lowest distance to the center and choose it as the master pos
+        {
+            //basically pos.getCenter but it is Client Side Only in 1.12
+            Vec3d center = new Vec3d(aabb.minX + (aabb.maxX - aabb.minX) * 0.5D, aabb.minY + (aabb.maxY - aabb.minY) * 0.5D, aabb.minZ + (aabb.maxZ - aabb.minZ) * 0.5D);
+            double distToCenter = getDistanceSq(pos, center);
+            if (distToCenter < masterDistance)
+            {
+                if (DEBUG)
+                    FMLLog.log.info("FarmlandWaterManager: New better pos then {}: {}, prev dist {}, new dist {}", masterPos, pos, masterDistance, distToCenter);
+                masterPos = pos;
+                masterDistance = distToCenter;
+            }
+        }
+        posSet.remove(masterPos);
+        if (DEBUG)
+            FMLLog.log.info("FarmlandWaterManager: {} center pos, {} dummy posses. Dist to center {}", masterPos, posSet.toArray(new ChunkPos[0]), masterDistance);
+        return addCustomTicket(world, new AABBTicket(aabb), masterPos, posSet.toArray(new ChunkPos[0]));
+    }
+
+    private static double getDistanceSq(ChunkPos pos, Vec3d vec3d)
+    {
+        //See ChunkPos#getDistanceSq
+        double d0 = (double)(pos.x * 16 + 8);
+        double d1 = (double)(pos.z * 16 + 8);
+        double d2 = d0 - vec3d.x;
+        double d3 = d1 - vec3d.z;
+        return  d2 * d2 + d3 * d3;
     }
 
     /**
@@ -110,7 +138,8 @@ public class FarmlandWaterManager
         ChunkTicketManager<Vec3d> ticketManager = getTicketManager(new ChunkPos(pos.getX() >> 4, pos.getZ() >> 4), world);
         if (ticketManager != null)
         {
-            Vec3d posAsVec3d = new Vec3d(pos);
+            //make the vec3d originate in the center of the block
+            Vec3d posAsVec3d = new Vec3d(pos.getX() + 0.5, pos.getY() + 0.5, pos.getZ() + 0.5);
             for (SimpleTicket<Vec3d> ticket : ticketManager.getTickets()) {
                 if (ticket.matches(posAsVec3d))
                     return true;
@@ -124,14 +153,16 @@ public class FarmlandWaterManager
         ChunkTicketManager<Vec3d> ticketManager = getTicketManager(chunk.getPos(), chunk.getWorld());
         if (ticketManager != null)
         {
-            for (SimpleTicket<Vec3d> ticket : ticketManager.getTickets())
-            {
-                ticket.invalidate();
-            }
+            if (DEBUG)
+                FMLLog.log.info("FarmlandWaterManager: got tickets {} at {} before", ticketManager.getTickets().size(), ticketManager.pos);
+            ticketManager.getTickets().removeIf(next -> next.unload(ticketManager)); //remove if this is the master manager of the ticket
+            if (DEBUG)
+                FMLLog.log.info("FarmlandWaterManager: got tickets {} at {} after", ticketManager.getTickets().size(), ticketManager.pos);
         }
     }
 
-    private static ChunkTicketManager<Vec3d> getTicketManager(ChunkPos pos, World world) {
+    private static ChunkTicketManager<Vec3d> getTicketManager(ChunkPos pos, World world)
+    {
         Preconditions.checkArgument(!world.isRemote, "Water region is only determined server-side");
         Map<ChunkPos, ChunkTicketManager<Vec3d>> ticketMap = customWaterHandler.get(world.provider.getDimension());
         if (ticketMap == null)

src/main/java/net/minecraftforge/common/ticket/MultiTicketManager.java

@@ -19,6 +19,10 @@
 
 package net.minecraftforge.common.ticket;
 
+/**
+ * @deprecated No longer needed, {@link SimpleTicket} now accepts dummy managers
+ */
+@Deprecated
 public class MultiTicketManager<T> implements ITicketManager<T>
 {
     private final ITicketGetter<T>[] ticketManagers;

src/main/java/net/minecraftforge/common/ticket/SimpleTicket.java

@@ -23,6 +23,7 @@ import com.google.common.base.Preconditions;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import java.util.function.Consumer;
 
 /**
  * Common class for a simple ticket based system.
@@ -31,18 +32,27 @@ import javax.annotation.Nullable;
 public abstract class SimpleTicket<T>
 {
     @Nullable
-    private ITicketManager<T> manager;
+    private ITicketManager<T> masterManager;
+    private ITicketManager<T>[] dummyManagers;
     protected boolean isValid = false;
 
+    @Deprecated
+    public final void setBackend(@Nonnull ITicketManager<T> ticketManager)
+    {
+        this.setManager(ticketManager);
+    }
+
     /**
      * Internal method that sets the collection from the managing system.
      * <br>
      * Should <b>not</b> be called if you just want to register a ticket to a system like the {@link net.minecraftforge.common.FarmlandWaterManager}
      */
-    public final void setBackend(@Nonnull ITicketManager<T> ticketManager)
+    @SafeVarargs
+    public final void setManager(@Nonnull ITicketManager<T> masterManager, @Nonnull ITicketManager<T>... dummyManagers)
     {
-        Preconditions.checkState(this.manager == null, "Ticket is already registered to a managing system");
-        this.manager = ticketManager;
+        Preconditions.checkState(this.masterManager == null, "Ticket is already registered to a managing system");
+        this.masterManager = masterManager;
+        this.dummyManagers = dummyManagers;
     }
 
     /**
@@ -59,26 +69,68 @@ public abstract class SimpleTicket<T>
      */
     public void invalidate()
     {
-        Preconditions.checkState(this.manager != null, "Ticket is not registered to a managing system");
         if (this.isValid())
         {
-            this.manager.remove(this);
+            forEachManager(ticketManager -> ticketManager.remove(this));
         }
         this.isValid = false;
     }
 
+    /**
+     * Called by the managing system when a ticket wishes to unload all of it's tickets, e.g. on chunk unload
+     * <br>The ticket must not remove itself from the manager that is calling the unload!
+     * The ticket must ensure that it removes itself from all of it's dummies when returning true
+     * @param unloadingManager The manager that is unloading this ticket
+     * @returns true if this ticket can be removed, false if not.
+     */
+    public boolean unload(ITicketManager<T> unloadingManager)
+    {
+        if (unloadingManager == masterManager)
+        {
+            this.isValid = false;
+            for (ITicketManager<T> manager : dummyManagers)
+            {
+                manager.remove(this); //remove ourself from all dummies
+            }
+            this.isValid = false; //and mark us as invalid
+            return true;
+        }
+        return false;
+    }
+
     /**
      * Re-adds your ticket to the system.
      */
     public void validate()
     {
-        Preconditions.checkState(this.manager != null, "Ticket is not registered to a managing system");
         if (!this.isValid())
         {
-            this.manager.add(this);
+            forEachManager(ticketManager -> ticketManager.add(this));
         }
         this.isValid = true;
     }
 
     public abstract boolean matches(T toMatch);
+
+    //Helper methods for custom tickets below
+
+    protected final void forEachManager(Consumer<ITicketManager<T>> consumer)
+    {
+        Preconditions.checkState(this.masterManager != null, "Ticket is not registered to a managing system");
+        consumer.accept(masterManager);
+        for (ITicketManager<T> manager : dummyManagers)
+        {
+            consumer.accept(manager);
+        }
+    }
+
+    protected final ITicketManager<T> getMasterManager()
+    {
+        return masterManager;
+    }
+
+    protected final ITicketManager<T>[] getDummyManagers()
+    {
+        return dummyManagers;
+    }
 }