From 863ab2ca184cf2e2dfa134d07bfc20d6a9a6a4e8 Mon Sep 17 00:00:00 2001 From: ichttt Date: Tue, 3 Sep 2019 23:22:38 +0200 Subject: [PATCH] Fix CME when removing ticket managers (#5861) --- .../common/FarmlandWaterManager.java | 85 +++++++++++++------ .../common/ticket/MultiTicketManager.java | 4 + .../common/ticket/SimpleTicket.java | 68 +++++++++++++-- 3 files changed, 122 insertions(+), 35 deletions(-) diff --git a/src/main/java/net/minecraftforge/common/FarmlandWaterManager.java b/src/main/java/net/minecraftforge/common/FarmlandWaterManager.java index 0047c31675..34e804a79b 100644 --- a/src/main/java/net/minecraftforge/common/FarmlandWaterManager.java +++ b/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>> customWaterHandler = new Int2ObjectOpenHashMap<>(); + @Deprecated + public static> 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 addCustomTicket(World world, T ticket, ChunkPos... chunkPoses) + public static> 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> ticketMap = customWaterHandler.computeIfAbsent(world.provider.getDimension(), id -> new HashMap<>()); - if (chunkPoses.length == 1) - { - ticket.setBackend(ticketMap.computeIfAbsent(chunkPoses[0], ChunkTicketManager::new)); - } - else - { - ChunkTicketManager[] 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> ticketMap = customWaterHandler.computeIfAbsent(world.provider.getDimension(), id -> new MapMaker().weakValues().makeMap()); + ChunkTicketManager[] 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 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 ticket : ticketManager.getTickets()) { if (ticket.matches(posAsVec3d)) return true; @@ -124,14 +153,16 @@ public class FarmlandWaterManager ChunkTicketManager ticketManager = getTicketManager(chunk.getPos(), chunk.getWorld()); if (ticketManager != null) { - for (SimpleTicket 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 getTicketManager(ChunkPos pos, World world) { + private static ChunkTicketManager getTicketManager(ChunkPos pos, World world) + { Preconditions.checkArgument(!world.isRemote, "Water region is only determined server-side"); Map> ticketMap = customWaterHandler.get(world.provider.getDimension()); if (ticketMap == null) diff --git a/src/main/java/net/minecraftforge/common/ticket/MultiTicketManager.java b/src/main/java/net/minecraftforge/common/ticket/MultiTicketManager.java index 9fc88e2430..af62cb02fc 100644 --- a/src/main/java/net/minecraftforge/common/ticket/MultiTicketManager.java +++ b/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 implements ITicketManager { private final ITicketGetter[] ticketManagers; diff --git a/src/main/java/net/minecraftforge/common/ticket/SimpleTicket.java b/src/main/java/net/minecraftforge/common/ticket/SimpleTicket.java index b8b85c3334..f88f10f00e 100644 --- a/src/main/java/net/minecraftforge/common/ticket/SimpleTicket.java +++ b/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 { @Nullable - private ITicketManager manager; + private ITicketManager masterManager; + private ITicketManager[] dummyManagers; protected boolean isValid = false; + @Deprecated + public final void setBackend(@Nonnull ITicketManager ticketManager) + { + this.setManager(ticketManager); + } + /** * Internal method that sets the collection from the managing system. *
* Should not 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 ticketManager) + @SafeVarargs + public final void setManager(@Nonnull ITicketManager masterManager, @Nonnull ITicketManager... 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 */ 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 + *
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 unloadingManager) + { + if (unloadingManager == masterManager) + { + this.isValid = false; + for (ITicketManager 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> consumer) + { + Preconditions.checkState(this.masterManager != null, "Ticket is not registered to a managing system"); + consumer.accept(masterManager); + for (ITicketManager manager : dummyManagers) + { + consumer.accept(manager); + } + } + + protected final ITicketManager getMasterManager() + { + return masterManager; + } + + protected final ITicketManager[] getDummyManagers() + { + return dummyManagers; + } }