From 28a2aa4730400df52e6da451ecc912caa5ce217c Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:18:26 +0100 Subject: [PATCH 1/6] perf: cache the Hand Pick held-tool match and index sites by chunk The Hand Pick tick matched every online player's main-hand stack against the whole tool whitelist (pack plugin lookups, NBT copies) and read the mining-speed attribute every tick, and the reach check scanned every site. Main spark profiles put that at ~5.8 s per hour at 15+ players. - Match the held stack once and again only when it changes (ItemStack equality) or after 20 ticks; the legacy lock strip runs once per stack. - Skip the attribute read while the lock was confirmed absent in the last 20 ticks; adding the lock clears that, and quitting drops the state. - Index sites by world and ruin chunk (rebuilt when the set of sites changes) for findByChunk and hasEstablishedIn; status is read live. - Run the hourly estate pass a quarter hour later so it no longer shares a tick with other plugins' hourly jobs; the startup pass is unchanged. Co-Authored-By: Claude Opus 5.5 --- .../archaeo/excavation/HandPickListener.java | 1 + .../archaeo/excavation/HandPickService.java | 123 +++++++++++++++++- .../archaeo/ownership/SiteEstates.java | 16 ++- .../archaeo/site/SiteRepository.java | 103 ++++++++++++++- .../excavation/HandPickServiceTest.java | 83 +++++++++++- .../archaeo/ownership/SiteEstatesTest.java | 10 ++ .../archaeo/site/SiteRepositoryTest.java | 42 ++++++ 7 files changed, 365 insertions(+), 13 deletions(-) diff --git a/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java b/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java index 3b7c7aa..86ba5da 100644 --- a/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java +++ b/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java @@ -178,6 +178,7 @@ public void onDrop(PlayerDropItemEvent event) { public void onQuit(PlayerQuitEvent event) { pick.finish(event.getPlayer()); pick.syncHeldTool(event.getPlayer(), null); + pick.forget(event.getPlayer()); } /** diff --git a/src/main/java/net/tfminecraft/archaeo/excavation/HandPickService.java b/src/main/java/net/tfminecraft/archaeo/excavation/HandPickService.java index cc2f6ef..eb4195e 100644 --- a/src/main/java/net/tfminecraft/archaeo/excavation/HandPickService.java +++ b/src/main/java/net/tfminecraft/archaeo/excavation/HandPickService.java @@ -63,6 +63,12 @@ public class HandPickService { * the ray hits. */ private static final int REACH_CELLS = REACH + 2; + /** + * Ticks a per-tick shortcut is trusted before the server is asked again: the held tool's + * whitelist match (pack registries can reload under an unchanged stack) and a confirmed + * mining unlock (another plugin could restore the modifier). + */ + private static final int RECHECK_TICKS = 20; private final JavaPlugin plugin; private final net.tfminecraft.archaeo.ownership.CharacterWork identity; @@ -75,6 +81,7 @@ public class HandPickService { private int gameTick; private final Map cycles = new ConcurrentHashMap<>(); private final Map lastWarn = new ConcurrentHashMap<>(); + private final Map held = new ConcurrentHashMap<>(); /** Wall clock in milliseconds for the warning cooldown; tests replace it to step past the cooldown. */ LongSupplier clock = System::currentTimeMillis; @@ -108,6 +115,7 @@ public HandPickService( public void setSettings(PickSettings settings) { this.settings = settings; this.tools.setProfiles(settings.profiles()); + held.clear(); } /** @@ -131,6 +139,16 @@ public void stop() { setVanillaMineLocked(player, false); } cycles.clear(); + held.clear(); + } + + /** + * Drops the per-tick shortcuts kept for a player who is leaving. + * + * @param player quitting player + */ + public void forget(Player player) { + held.remove(player.getUniqueId()); } /** @@ -352,7 +370,7 @@ private void tick() { continue; } if (!net.tfminecraft.archaeo.ownership.CharacterActors.current(cycle.actor, player)) { finish(player); continue; } - ExcavationTool held = tools.match(player.getInventory().getItemInMainHand()).orElse(null); + ExcavationTool held = heldTool(player, player.getInventory().getItemInMainHand(), false); if (held == null || !held.id().equals(cycle.tool.id())) { finish(player); continue; @@ -403,11 +421,10 @@ private void tick() { */ private void syncAndShowHud(Player player) { ItemStack held = player.getInventory().getItemInMainHand(); - if (!tools.isAllowed(held)) { - setVanillaMineLocked(player, false); + if (heldTool(player, held, true) == null) { + keepVanillaMineUnlocked(player); return; } - tools.restoreAllowedTool(held); Cycle cycle = cycles.get(player.getUniqueId()); if (cycle != null) { setVanillaMineLocked(player, true); @@ -417,12 +434,82 @@ private void syncAndShowHud(Player player) { } Block target = aimedNearSite(player); Site site = target == null ? null : establishedAt(target); - setVanillaMineLocked(player, site != null && PrismFill.isTerrainFill(target.getType())); + if (site != null && PrismFill.isTerrainFill(target.getType())) { + setVanillaMineLocked(player, true); + } else { + keepVanillaMineUnlocked(player); + } if (site != null) { showHud(player, null, target, site); } } + /** + * The whitelist profile for the main-hand stack, matched again only when the stack differs + * from the one matched last (any change to type, amount, or data) or after + * {@link #RECHECK_TICKS}. Matching asks every whitelist entry and the pack plugins, which is + * too slow to repeat for every online player on every tick. + * + * @param player holder + * @param stack current main-hand stack + * @param restore whether to strip a legacy mine lock from a whitelisted stack, as + * {@link DigTools#restoreAllowedTool} does; skipped while the stack is the one + * already restored, since a restored stack has nothing left to strip + * @return matching profile, or {@code null} when the stack is not an excavation tool + */ + private ExcavationTool heldTool(Player player, ItemStack stack, boolean restore) { + HeldState state = state(player); + boolean same = stack != null + && state.stack != null + && gameTick - state.matchedTick < RECHECK_TICKS + && state.stack.equals(stack); + if (!same) { + state.tool = tools.match(stack).orElse(null); + state.matchedTick = gameTick; + state.restored = false; + } + if (restore && !state.restored) { + if (state.tool != null) { + tools.restoreAllowedTool(stack); + } + state.restored = true; + // Restoring may have changed the stack, so remember it as it is now. + same = false; + } + if (!same) { + state.stack = stack == null ? null : stack.clone(); + } + return state.tool; + } + + /** + * Unlocks vanilla mining, skipping the attribute read while an unlock was already confirmed + * within {@link #RECHECK_TICKS}. Only {@link #setVanillaMineLocked} adds the lock, and it + * clears that confirmation when it does. + * + * @param player holder + */ + private void keepVanillaMineUnlocked(Player player) { + HeldState state = state(player); + if (state.unlocked && gameTick - state.unlockedTick < RECHECK_TICKS) { + return; + } + setVanillaMineLocked(player, false); + } + + /** + * @param player online player + * @return shortcuts for this player object; a rejoin gets a fresh one + */ + private HeldState state(Player player) { + HeldState state = held.get(player.getUniqueId()); + if (state == null || state.player != player) { + state = new HeldState(player); + held.put(player.getUniqueId(), state); + } + return state; + } + /** * @param player viewer * @return whether the crosshair is on prism fill @@ -766,6 +853,9 @@ private void setVanillaMineLocked(Player player, boolean locked) { break; } } + HeldState state = state(player); + state.unlocked = !locked; + state.unlockedTick = gameTick; if (locked && found == null) { speed.addModifier(new AttributeModifier( noVanillaMineKey, @@ -857,6 +947,29 @@ private void warn(Player player, String message) { player.sendMessage(message); } + /** + * Per-tick shortcuts for one online player object. + */ + private static final class HeldState { + private final Player player; + /** Copy of the main-hand stack last matched, or {@code null} before the first match. */ + private ItemStack stack; + private ExcavationTool tool; + private int matchedTick; + /** Whether {@link DigTools#restoreAllowedTool} already ran on {@link #stack}. */ + private boolean restored; + /** Whether the mining lock was last seen absent, at {@link #unlockedTick}. */ + private boolean unlocked; + private int unlockedTick; + + /** + * @param player online player object + */ + private HeldState(Player player) { + this.player = player; + } + } + /** * One player's current left-click hold on a cell. */ diff --git a/src/main/java/net/tfminecraft/archaeo/ownership/SiteEstates.java b/src/main/java/net/tfminecraft/archaeo/ownership/SiteEstates.java index 5118013..2d32cf7 100644 --- a/src/main/java/net/tfminecraft/archaeo/ownership/SiteEstates.java +++ b/src/main/java/net/tfminecraft/archaeo/ownership/SiteEstates.java @@ -121,5 +121,19 @@ private static void supersede(SiteOwnership store,Path path,Map j if(!Objects.equals(SiteDocuments.read(source),journal.get("before"))||!Estates.guardHolds(current)||!Files.readString(path).equals(original))throw new SiteOwnership.Conflict("Untouched plan changed during supersession");Files.delete(path);SiteDocuments.forceDirectory(path.getParent()); } private static Outcome done(Set moved){return Outcome.done(Map.of("sites",moved.size()),List.copyOf(moved));} - public static void start(Plugin plugin,SiteRepository repository){SiteEstates participant=new SiteEstates(repository);Bukkit.getServicesManager().register(EstateParticipant.class,participant,plugin,ServicePriority.Normal);Bukkit.getScheduler().runTaskTimer(plugin,()->{for(EstateDecision decision:Estates.decided())try{Estates.report(decision.deathId(),participant.id(),decision.revision(),participant.settle(decision));}catch(Exception failure){plugin.getLogger().warning("Site estate "+decision.deathId()+" revision "+decision.revision()+": "+failure.getMessage());}},3L,20L*60*60);} + /** Ticks between estate passes. */ + static final long PASS_PERIOD_TICKS=20L*60*60; + /** Delay of the startup pass. */ + static final long STARTUP_PASS_TICKS=3L; + /** + * First hourly pass, a quarter hour later than a plain hourly timer would run it, so it does + * not land on the same tick as the hourly jobs other plugins start at enable time. + */ + static final long FIRST_HOURLY_PASS_TICKS=STARTUP_PASS_TICKS+PASS_PERIOD_TICKS+20L*60*15; + public static void start(Plugin plugin,SiteRepository repository){ + SiteEstates participant=new SiteEstates(repository);Bukkit.getServicesManager().register(EstateParticipant.class,participant,plugin,ServicePriority.Normal); + Runnable pass=()->{for(EstateDecision decision:Estates.decided())try{Estates.report(decision.deathId(),participant.id(),decision.revision(),participant.settle(decision));}catch(Exception failure){plugin.getLogger().warning("Site estate "+decision.deathId()+" revision "+decision.revision()+": "+failure.getMessage());}}; + Bukkit.getScheduler().runTaskLater(plugin,pass,STARTUP_PASS_TICKS); + Bukkit.getScheduler().runTaskTimer(plugin,pass,FIRST_HOURLY_PASS_TICKS,PASS_PERIOD_TICKS); + } } diff --git a/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java b/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java index afea3bb..336c185 100644 --- a/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java +++ b/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java @@ -34,6 +34,7 @@ import java.util.HashSet; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; /** @@ -48,6 +49,8 @@ public class SiteRepository { public static final int SCHEMA = net.tfminecraft.archaeo.ownership.SiteOwnership.SCHEMA; /** Dirty sites are flushed at least this often (~1 s). */ private static final long FLUSH_PERIOD_TICKS = 20L; + /** Larger {@link #hasEstablishedIn} ranges scan every site instead of looking up each chunk. */ + private static final long INDEXED_RANGE_CHUNKS = 64L; private final JavaPlugin plugin; private final net.tfminecraft.archaeo.ownership.SiteOwnership ownership; @@ -63,6 +66,12 @@ public void directorCommitted(UUID id, net.tfminecraft.tlibs.character.Character private final File sitesFolder; private final File indexFile; private final Map byId = new ConcurrentHashMap<>(); + /** + * Bumped after every change to which sites {@link #byId} holds, so {@link #chunkIndex()} is + * rebuilt. A site's world and ruin chunk are set before it is first stored and never change. + */ + private final AtomicLong layoutVersion = new AtomicLong(); + private volatile ChunkIndex chunkIndex = new ChunkIndex(-1L, Map.of()); private final Set dirty = ConcurrentHashMap.newKeySet(); private int nextSerial = 1; private final Set retryWrites = new HashSet<>(); @@ -131,6 +140,7 @@ public boolean loadAll() { byId.forEach((id,site)->{if(loaded.get(id)!=site)site.setOwnershipHeld(true);}); byId.clear(); byId.putAll(loaded); + layoutChanged(); dirty.clear(); nextSerial = serial; persistedSerial = serial; @@ -143,7 +153,7 @@ private void loadAvailable(String name){ if(!ownership.ready()||ownership.held(name))return; try{YamlConfiguration yaml=new YamlConfiguration();yaml.loadFromString(ownership.before(name));Site site=read(yaml); net.tfminecraft.archaeo.ownership.SiteRawOverlay.validateReadable(ownership.documents().get(name),net.tfminecraft.archaeo.ownership.SiteDocuments.parse(write(site).saveToString())); - byId.putIfAbsent(site.getId(),site);nextSerial=Math.max(nextSerial,site.getSerial()+1); + if(byId.putIfAbsent(site.getId(),site)==null)layoutChanged();nextSerial=Math.max(nextSerial,site.getSerial()+1); }catch(Exception failure){ownership.hold(name,"Unloaded dossier: "+failure.getMessage());plugin.getLogger().log(Level.WARNING,"Could not load released dossier "+name,failure);} } @@ -185,6 +195,14 @@ public int nextSerial() { * @return site occupying that chunk, if any */ public Optional findByChunk(String world, int chunkX, int chunkZ) { + List here = chunkIndex().get(new ChunkKey(world, chunkX, chunkZ)); + if (here == null) { + return Optional.empty(); + } + if (here.size() == 1) { + return Optional.of(here.getFirst()); + } + // Two ruins in one chunk: keep the map's order, which decides which one answers. return byId.values().stream() .filter(site -> site.getWorldName().equals(world) && site.getChunkX() == chunkX @@ -273,6 +291,25 @@ public Optional findEstablishedPrism(String world, int x, int y, int z) { * @return whether an established excavation's ruin chunk lies in that range */ public boolean hasEstablishedIn(String world, int minChunkX, int maxChunkX, int minChunkZ, int maxChunkZ) { + long columns = (long) maxChunkX - minChunkX + 1; + long rows = (long) maxChunkZ - minChunkZ + 1; + if (columns > 0 && rows > 0 && columns * rows <= INDEXED_RANGE_CHUNKS) { + Map> index = chunkIndex(); + for (long chunkX = minChunkX; chunkX <= maxChunkX; chunkX++) { + for (long chunkZ = minChunkZ; chunkZ <= maxChunkZ; chunkZ++) { + List here = index.get(new ChunkKey(world, (int) chunkX, (int) chunkZ)); + if (here == null) { + continue; + } + for (Site site : here) { + if (site.getStatus() == SiteStatus.ESTABLISHED) { + return true; + } + } + } + } + return false; + } for (Site site : byId.values()) { if (site.getChunkX() >= minChunkX && site.getChunkX() <= maxChunkX && site.getChunkZ() >= minChunkZ && site.getChunkZ() <= maxChunkZ @@ -284,6 +321,61 @@ public boolean hasEstablishedIn(String world, int minChunkX, int maxChunkX, int return false; } + /** + * Sites by world and ruin chunk, rebuilt from {@link #byId} after it changes. Status is not + * part of the key: it changes on the site itself, so callers read it live. + * + * @return index matching the current set of sites + */ + private Map> chunkIndex() { + ChunkIndex current = chunkIndex; + long version = layoutVersion.get(); + if (current.version() == version) { + return current.sites(); + } + Map> built = new java.util.HashMap<>(); + for (Site site : byId.values()) { + built.computeIfAbsent( + new ChunkKey(site.getWorldName(), site.getChunkX(), site.getChunkZ()), + key -> new ArrayList<>(1)).add(site); + } + chunkIndex = new ChunkIndex(version, built); + return built; + } + + /** + * Puts {@code site} in {@link #byId}, invalidating the chunk index only when the map changes. + * + * @param site dossier to hold + */ + private void store(Site site) { + if (byId.put(site.getId(), site) != site) { + layoutChanged(); + } + } + + /** + * Call after {@link #byId} gains or loses a site. + */ + private void layoutChanged() { + layoutVersion.incrementAndGet(); + } + + /** + * @param world world name + * @param x ruin chunk X + * @param z ruin chunk Z + */ + private record ChunkKey(String world, int x, int z) { + } + + /** + * @param version {@link #layoutVersion} the index was built from + * @param sites sites by ruin chunk + */ + private record ChunkIndex(long version, Map> sites) { + } + /** * Ruin or live excavation whose prism contains this block (not yet exhausted or closed). * Unclaimed hidden ruins still occupy a prism so vanilla mining can smash finds. @@ -484,7 +576,7 @@ public void delete(Site site) { Files.move(file.toPath(),trash.resolve(id+".yml"),StandardCopyOption.REPLACE_EXISTING); } }catch(IOException failure){throw new IllegalStateException("Dossier retained: trash publication failed",failure);} - byId.remove(id);dirty.remove(id);retryWrites.remove(id);ownership.removed(id+".yml"); + byId.remove(id);layoutChanged();dirty.remove(id);retryWrites.remove(id);ownership.removed(id+".yml"); } /** @@ -507,6 +599,7 @@ public void erase(Site site) { File file = new File(sitesFolder, id + ".yml"); if (file.exists() && !file.delete()) throw new IllegalStateException("Could not delete " + file.getName()); byId.remove(id); + layoutChanged(); dirty.remove(id); ownership.removed(id+".yml");retryWrites.remove(id); File trashed = new File(new File(sitesFolder, ".trash"), id + ".yml"); @@ -536,13 +629,13 @@ public void commit(Site site) { if (site.isOwnershipHeld() && !retryWrites.contains(site.getId())) throw new IllegalStateException("Held dossier " + site.getId()); String name=site.getId()+".yml";Path path=sitesFolder.toPath().resolve(name); boolean firstSave=!ownership.names().contains(name)&&unpublishedPath(path); - byId.put(site.getId(), site); + store(site); try { writeAtomic(site); dirty.remove(site.getId()); } catch (RuntimeException exception) { if(firstSave&&unpublishedPath(path)){ - byId.remove(site.getId(),site);dirty.remove(site.getId());retryWrites.remove(site.getId());ownership.removed(name); + if(byId.remove(site.getId(),site))layoutChanged();dirty.remove(site.getId());retryWrites.remove(site.getId());ownership.removed(name); }else dirty.add(site.getId()); syncOwnershipHolds(); throw exception; @@ -568,7 +661,7 @@ public void touch(Site site) { } rejectStale(site); if (site.isOwnershipHeld()) throw new IllegalStateException("Held dossier cannot be changed"); - byId.put(site.getId(), site); + store(site); dirty.add(site.getId()); } diff --git a/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java b/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java index 48ef986..198d14c 100644 --- a/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java +++ b/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java @@ -44,6 +44,8 @@ public class HandPickServiceTest { private SnapshotWorld world; private PlayerMock player; private SiteRepository sites; + private JavaPlugin plugin; + private DigTools tools; private CatalogRegistry catalogs; private HandPickService service; private Site site; @@ -60,7 +62,7 @@ public void setup() { world = new SnapshotWorld(); world.setName("pick"); server.addWorld(world); - JavaPlugin plugin = MockBukkit.createMockPlugin(); + plugin = MockBukkit.createMockPlugin(); player = spy(new PlayerMock(server, "Digger")); server.addPlayer(player); // This newer player attribute is absent from MockBukkit's default player attributes. @@ -109,7 +111,7 @@ public void setup() { catalogs = mock(CatalogRegistry.class); when(catalogs.toolWear()).thenReturn(new ToolWearSettings(0, 0, false)); profile = profile(1, 0, 20); - DigTools tools = mock(DigTools.class); + tools = mock(DigTools.class); when(tools.match(any())).thenAnswer(invocation -> Optional.ofNullable(held(invocation.getArgument(0)))); when(tools.isAllowed(any())).thenAnswer(invocation -> held(invocation.getArgument(0)) != null); service = new HandPickService(plugin, sites, catalogs, tools, settings(true, 10)); @@ -870,6 +872,83 @@ public void refusalRepeatsOnlyAfterTheWarningCooldown() { assertFalse(service.isCycling(player)); } + @Test + public void heldToolIsMatchedOncePerStackUntilTheStackChangesOrASecondPasses() { + ticks(1); + clearInvocations(tools); + ticks(19); + verify(tools, never()).match(any()); + verify(tools, never()).isAllowed(any()); + verify(tools, never()).restoreAllowedTool(any()); + // Same type, different data: matched and restored again on the next tick. + ItemStack worn = new ItemStack(Material.IRON_PICKAXE); + org.bukkit.inventory.meta.Damageable wear = (org.bukkit.inventory.meta.Damageable) worn.getItemMeta(); + wear.setDamage(3); + worn.setItemMeta(wear); + player.getInventory().setItemInMainHand(worn); + ticks(1); + verify(tools, times(1)).match(any()); + verify(tools, times(1)).restoreAllowedTool(any()); + clearInvocations(tools); + ticks(19); + verify(tools, never()).match(any()); + // A second later the unchanged stack is asked again, in case a pack plugin reloaded. + ticks(1); + verify(tools, times(1)).match(any()); + verify(tools, times(1)).restoreAllowedTool(any()); + // New settings can change the whitelist, so the next tick matches at once. + clearInvocations(tools); + service.setSettings(settings(true, 10)); + ticks(1); + verify(tools, times(1)).match(any()); + assertEquals("STRATUM I ⛏ 10", actionBars.getLast()); + } + + @Test + public void heldToolMatchIsSharedBetweenTheHoldAndTheHud() { + profile = profile(20, 0, 20); + service.setSettings(settings(true, 10)); + service.noteMining(player, target); + ticks(1); + clearInvocations(tools); + ticks(2); + assertTrue(service.isCycling(player)); + verify(tools, never()).match(any()); + player.getInventory().setItemInMainHand(new ItemStack(Material.STICK)); + ticks(1); + assertFalse(service.isCycling(player)); + verify(tools, times(1)).match(any()); + } + + @Test + public void confirmedUnlockIsReadAgainOnlyEverySecondAndAfterARejoin() { + player.getInventory().setItemInMainHand(new ItemStack(Material.STICK)); + var speed = player.getAttribute(Attribute.BLOCK_BREAK_SPEED); + ticks(1); + clearInvocations(player); + ticks(19); + verify(player, never()).getAttribute(Attribute.BLOCK_BREAK_SPEED); + // A lock put back behind the service's back comes off within a second. + speed.addModifier(new AttributeModifier(new NamespacedKey(plugin, "no_vanilla_mine"), -1.0, + AttributeModifier.Operation.MULTIPLY_SCALAR_1)); + ticks(1); + verify(player, times(1)).getAttribute(Attribute.BLOCK_BREAK_SPEED); + assertTrue(speed.getModifiers().isEmpty()); + // Locking drops the confirmation, so the next unlock is applied on the very next tick. + player.getInventory().setItemInMainHand(new ItemStack(Material.IRON_PICKAXE)); + ticks(1); + assertEquals(1, speed.getModifiers().size()); + player.getInventory().setItemInMainHand(new ItemStack(Material.STICK)); + ticks(1); + assertTrue(speed.getModifiers().isEmpty()); + clearInvocations(player); + ticks(1); + verify(player, never()).getAttribute(Attribute.BLOCK_BREAK_SPEED); + service.forget(player); + ticks(1); + verify(player, times(1)).getAttribute(Attribute.BLOCK_BREAK_SPEED); + } + private ExcavationTool held(ItemStack stack) { if (stack == null) return null; if (stack.getType() == Material.IRON_PICKAXE) return profile; diff --git a/src/test/java/net/tfminecraft/archaeo/ownership/SiteEstatesTest.java b/src/test/java/net/tfminecraft/archaeo/ownership/SiteEstatesTest.java index 51bea99..16a5b20 100644 --- a/src/test/java/net/tfminecraft/archaeo/ownership/SiteEstatesTest.java +++ b/src/test/java/net/tfminecraft/archaeo/ownership/SiteEstatesTest.java @@ -40,6 +40,16 @@ private void deletedAfterCaptureAllowsUnrelatedEstate(Recipient recipient)throws var current=decision(1,recipient);activate(current);restartRepository();var result=new SiteEstates(repository).settle(current);assertEquals(Outcome.Kind.DONE,result.kind());assertTrue(store.held(other.getId()+".yml"));assertTrue(repository.chunkOccupied("world",2,0));assertEquals(1,repository.countDirectedCamps(third));assertEquals(proof,Files.readString(snapshot));restartRepository();assertEquals(result,new SiteEstates(repository).settle(current)); var own=new EstateDecision(UUID.randomUUID(),third,Optional.of(Instant.EPOCH),Instant.EPOCH,Optional.empty(),Optional.empty(),Optional.empty(),Recipient.FROZEN,Recipient.FROZEN,Recipient.FROZEN,Recipient.SENDER,1);activate(own);assertEquals(Outcome.Kind.BLOCKED,new SiteEstates(repository).settle(own).kind());assertTrue(store.files("ownership/completions/"+own.deathId()).isEmpty());assertEquals(proof,Files.readString(snapshot)); } + @Test public void passesRunAtStartupThenHourlyFromAQuarterPastTheFirstHour(){ + SiteEstates.start(MockBukkit.createMockPlugin(),repository);clearInvocations(estates); + server.getScheduler().performTicks(SiteEstates.STARTUP_PASS_TICKS);verify(estates,times(1)).decided(); + // A plain hourly timer would have run again here, on the same tick as other plugins' hourly jobs. + server.getScheduler().performTicks(SiteEstates.PASS_PERIOD_TICKS);verify(estates,times(1)).decided(); + server.getScheduler().performTicks(SiteEstates.FIRST_HOURLY_PASS_TICKS-SiteEstates.PASS_PERIOD_TICKS-SiteEstates.STARTUP_PASS_TICKS-1);verify(estates,times(1)).decided(); + server.getScheduler().performTicks(1);verify(estates,times(2)).decided(); + server.getScheduler().performTicks(SiteEstates.PASS_PERIOD_TICKS);verify(estates,times(3)).decided(); + assertEquals(20L*60*75+3,SiteEstates.FIRST_HOURLY_PASS_TICKS); + } @Test public void unrelatedEstateSkipsAJournalOnlyWhileItsTextIsUnchanged()throws Exception{ realRepository();repository.save(runtime(id,from,4));var participant=new SiteEstates(repository);var own=decision(1,Recipient.character(to));assertEquals(Outcome.Kind.DONE,participant.settle(own).kind()); Path journal=store.files("ownership/journals").getFirst();String receipt=Files.readString(journal);assertTrue(receipt.contains(death.toString())); diff --git a/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java b/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java index 32f2a0b..52fb610 100644 --- a/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java +++ b/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java @@ -188,6 +188,48 @@ public class SiteRepositoryTest { assertThrows(UnsupportedOperationException.class, () -> repository.all().clear()); } + @Test public void chunkLookupsFollowSitesAddedRemovedReloadedAndTheirLiveStatus() { + assertTrue(repository.loadAll()); + Site hidden = site(1, "world", 0, 0, SiteStatus.HIDDEN); + net.tfminecraft.archaeo.CharacterTestSupport.touch(repository, hidden); + assertEquals(Optional.of(hidden), repository.findByChunk("world", 0, 0)); + assertFalse(repository.hasEstablishedIn("world", 0, 0, 0, 0)); + // Status lives on the site, so a change needs no repository call to show. + hidden.setStatus(SiteStatus.ESTABLISHED); + assertTrue(repository.hasEstablishedIn("world", -1, 0, 0, 1)); + assertFalse(repository.hasEstablishedIn("other", -1, 0, 0, 1)); + // Saving the same object again keeps it; a new site shows at once. + net.tfminecraft.archaeo.CharacterTestSupport.touch(repository, hidden); + Site later = site(2, "world", 5, -5, SiteStatus.ESTABLISHED); + net.tfminecraft.archaeo.CharacterTestSupport.commit(repository, later); + assertEquals(Optional.of(later), repository.findByChunk("world", 5, -5)); + assertTrue(repository.hasEstablishedIn("world", 5, 5, -5, -5)); + // Ranges too wide to look up chunk by chunk, empty ranges, and ranges at the int limits. + assertTrue(repository.hasEstablishedIn("world", -100, 100, -100, 100)); + assertFalse(repository.hasEstablishedIn("world", 1, 0, 0, 0)); + assertFalse(repository.hasEstablishedIn("world", Integer.MAX_VALUE - 1, Integer.MAX_VALUE, 0, 0)); + assertFalse(repository.hasEstablishedIn("world", 0, 0, Integer.MIN_VALUE, Integer.MIN_VALUE + 1)); + repository.delete(hidden); + assertTrue(repository.findByChunk("world", 0, 0).isEmpty()); + assertFalse(repository.hasEstablishedIn("world", 0, 0, 0, 0)); + SiteRepository restored = new SiteRepository(plugin); + assertTrue(restored.loadAll()); + assertEquals(later.getId(), restored.findByChunk("world", 5, -5).orElseThrow().getId()); + repository.erase(later); + assertTrue(repository.findByChunk("world", 5, -5).isEmpty()); + assertFalse(repository.hasEstablishedIn("world", 5, 5, -5, -5)); + } + + @Test public void twoSitesInOneChunkAnswerInTheRepositorysOwnOrder() { + Site first = site(1, "world", 3, 3, SiteStatus.HIDDEN); + Site second = site(2, "world", 3, 3, SiteStatus.ESTABLISHED); + net.tfminecraft.archaeo.CharacterTestSupport.touch(repository, first); + net.tfminecraft.archaeo.CharacterTestSupport.touch(repository, second); + assertEquals(Optional.of(repository.all().iterator().next()), repository.findByChunk("world", 3, 3)); + assertTrue(repository.hasEstablishedIn("world", 3, 3, 3, 3)); + assertTrue(repository.findByChunk("other", 3, 3).isEmpty()); + } + @Test public void flushTimerStartsOnceStopsAndWritesPendingChanges() { Server server = mock(Server.class); BukkitScheduler scheduler = mock(BukkitScheduler.class); BukkitTask task = mock(BukkitTask.class); From e26e1cd47852b471efeea49548fd38af2ebe6fd7 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:40:35 +0100 Subject: [PATCH 2/6] Rematch held tools after a pack-plugin rebind and harden the range check - Clear the Hand Pick held-tool cache when the item matcher is rebound (ItemsAdder load/reload), not only on settings reload. - hasEstablishedIn: compare the range size without a long overflow. - Tests: in-place stack change, rebind, reload of a repository whose chunk lookup is in use, failed first save, full int range. Co-Authored-By: Claude Opus 5.5 --- .../net/tfminecraft/archaeo/ArcheologyPlugin.java | 1 + .../archaeo/excavation/HandPickService.java | 8 ++++++++ .../tfminecraft/archaeo/site/SiteRepository.java | 2 +- .../archaeo/excavation/HandPickServiceTest.java | 14 ++++++++++++++ .../archaeo/site/SiteRepositoryTest.java | 11 +++++++++++ 5 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/archaeo/ArcheologyPlugin.java b/src/main/java/net/tfminecraft/archaeo/ArcheologyPlugin.java index 3f0cf3a..55b0dc9 100644 --- a/src/main/java/net/tfminecraft/archaeo/ArcheologyPlugin.java +++ b/src/main/java/net/tfminecraft/archaeo/ArcheologyPlugin.java @@ -199,6 +199,7 @@ public void bindItemMatcher(ItemMatcher matcher) { brushItem.setMatcher(bound); sketchSupplies.setMatcher(bound); digTools.setMatcher(bound); + handPick.toolsChanged(); recoveredFindItem.setMatcher(bound); sketch.setMatcher(bound); museum.setMatcher(bound); diff --git a/src/main/java/net/tfminecraft/archaeo/excavation/HandPickService.java b/src/main/java/net/tfminecraft/archaeo/excavation/HandPickService.java index eb4195e..a53c392 100644 --- a/src/main/java/net/tfminecraft/archaeo/excavation/HandPickService.java +++ b/src/main/java/net/tfminecraft/archaeo/excavation/HandPickService.java @@ -115,6 +115,14 @@ public HandPickService( public void setSettings(PickSettings settings) { this.settings = settings; this.tools.setProfiles(settings.profiles()); + toolsChanged(); + } + + /** + * Forgets every cached held-tool match, so the next tick matches again. Call after the + * whitelist or the pack-plugin lookup behind it changes. + */ + public void toolsChanged() { held.clear(); } diff --git a/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java b/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java index 336c185..a93a0d3 100644 --- a/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java +++ b/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java @@ -293,7 +293,7 @@ public Optional findEstablishedPrism(String world, int x, int y, int z) { public boolean hasEstablishedIn(String world, int minChunkX, int maxChunkX, int minChunkZ, int maxChunkZ) { long columns = (long) maxChunkX - minChunkX + 1; long rows = (long) maxChunkZ - minChunkZ + 1; - if (columns > 0 && rows > 0 && columns * rows <= INDEXED_RANGE_CHUNKS) { + if (columns > 0 && rows > 0 && columns <= INDEXED_RANGE_CHUNKS && rows <= INDEXED_RANGE_CHUNKS / columns) { Map> index = chunkIndex(); for (long chunkX = minChunkX; chunkX <= maxChunkX; chunkX++) { for (long chunkZ = minChunkZ; chunkZ <= maxChunkZ; chunkZ++) { diff --git a/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java b/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java index 198d14c..68b9ae7 100644 --- a/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java +++ b/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java @@ -896,6 +896,20 @@ public void heldToolIsMatchedOncePerStackUntilTheStackChangesOrASecondPasses() { ticks(1); verify(tools, times(1)).match(any()); verify(tools, times(1)).restoreAllowedTool(any()); + // Changing the held stack in place (wear, data written by another plugin) is seen at once. + clearInvocations(tools); + ItemStack live = player.getInventory().getItemInMainHand(); + org.bukkit.inventory.meta.ItemMeta renamed = live.getItemMeta(); + renamed.setDisplayName("Field pick"); + live.setItemMeta(renamed); + player.getInventory().setItemInMainHand(live); + ticks(1); + verify(tools, times(1)).match(any()); + // A rebound pack-plugin lookup is matched again on the next tick. + clearInvocations(tools); + service.toolsChanged(); + ticks(1); + verify(tools, times(1)).match(any()); // New settings can change the whitelist, so the next tick matches at once. clearInvocations(tools); service.setSettings(settings(true, 10)); diff --git a/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java b/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java index 52fb610..dcb0b9a 100644 --- a/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java +++ b/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java @@ -209,12 +209,20 @@ public class SiteRepositoryTest { assertFalse(repository.hasEstablishedIn("world", 1, 0, 0, 0)); assertFalse(repository.hasEstablishedIn("world", Integer.MAX_VALUE - 1, Integer.MAX_VALUE, 0, 0)); assertFalse(repository.hasEstablishedIn("world", 0, 0, Integer.MIN_VALUE, Integer.MIN_VALUE + 1)); + assertTrue(repository.hasEstablishedIn("world", Integer.MIN_VALUE, Integer.MAX_VALUE, Integer.MIN_VALUE, Integer.MAX_VALUE)); repository.delete(hidden); assertTrue(repository.findByChunk("world", 0, 0).isEmpty()); assertFalse(repository.hasEstablishedIn("world", 0, 0, 0, 0)); SiteRepository restored = new SiteRepository(plugin); assertTrue(restored.loadAll()); assertEquals(later.getId(), restored.findByChunk("world", 5, -5).orElseThrow().getId()); + // A dossier written elsewhere shows after reloading a repository whose lookup was already in use. + Site extra = site(3, "world", 7, 7, SiteStatus.ESTABLISHED); + net.tfminecraft.archaeo.CharacterTestSupport.commit(restored, extra); + assertTrue(repository.findByChunk("world", 7, 7).isEmpty()); + assertTrue(repository.loadAll()); + assertEquals(extra.getId(), repository.findByChunk("world", 7, 7).orElseThrow().getId()); + assertTrue(repository.hasEstablishedIn("world", 7, 7, 7, 7)); repository.erase(later); assertTrue(repository.findByChunk("world", 5, -5).isEmpty()); assertFalse(repository.hasEstablishedIn("world", 5, 5, -5, -5)); @@ -267,7 +275,10 @@ public class SiteRepositoryTest { @Test public void refusedUnpublishedFirstSaveDoesNotRetainPhantomWhenStorageRecovers() throws Exception { Path sites = temporary.getRoot().toPath().resolve("sites"); Files.writeString(sites, "blocked"); Site site = site(1, "world", 0, 0, SiteStatus.HIDDEN); + assertTrue(repository.findByChunk("world", 0, 0).isEmpty()); assertThrows(IllegalStateException.class, () -> net.tfminecraft.archaeo.CharacterTestSupport.commit(repository, site)); + // The rolled-back site leaves the chunk lookup too, not only the id map. + assertTrue(repository.findByChunk("world", 0, 0).isEmpty()); repository.flushDirty(); assertTrue(repository.findById(site.getId()).isEmpty()); assertFalse(repository.loadAll());assertTrue(repository.findById(site.getId()).isEmpty()); Files.delete(sites);assertTrue(repository.loadAll());Site replacement=site(1,"world",0,0,SiteStatus.HIDDEN);net.tfminecraft.archaeo.CharacterTestSupport.commit(repository,replacement);assertTrue(file(replacement).exists());assertFalse(repository.ownership().held(site.getId()+".yml")); From 982ec424a5d2f2f895c34640c61271ad3c502344 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:57:15 +0100 Subject: [PATCH 3/6] Forget Hand Pick held state on a character switch and on a failed quit - A character session end drops the cached unlock confirmation, so a switch that swaps saved attributes is read again on the next tick. - onQuit forgets the player in a finally block. - Test that the hold's first sight of a changed stack still restores it once in the same tick. Co-Authored-By: Claude Opus 5.5 --- .../archaeo/excavation/HandPickListener.java | 18 ++++++++++++++++-- .../excavation/HandPickListenerTest.java | 13 +++++++++++++ .../excavation/HandPickServiceTest.java | 13 +++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java b/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java index 86ba5da..5eabcfd 100644 --- a/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java +++ b/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java @@ -176,8 +176,22 @@ public void onDrop(PlayerDropItemEvent event) { */ @EventHandler(priority = EventPriority.MONITOR) public void onQuit(PlayerQuitEvent event) { - pick.finish(event.getPlayer()); - pick.syncHeldTool(event.getPlayer(), null); + try { + pick.finish(event.getPlayer()); + pick.syncHeldTool(event.getPlayer(), null); + } finally { + pick.forget(event.getPlayer()); + } + } + + /** + * A character switch can swap the player's saved attributes, so the next tick reads the + * mining lock again instead of trusting what it saw before. + * + * @param event character session end + */ + @EventHandler(priority = EventPriority.MONITOR) + public void onCharacterEnd(net.tfminecraft.tlibs.character.CharacterSessionEndEvent event) { pick.forget(event.getPlayer()); } diff --git a/src/test/java/net/tfminecraft/archaeo/excavation/HandPickListenerTest.java b/src/test/java/net/tfminecraft/archaeo/excavation/HandPickListenerTest.java index db23af5..014ad16 100644 --- a/src/test/java/net/tfminecraft/archaeo/excavation/HandPickListenerTest.java +++ b/src/test/java/net/tfminecraft/archaeo/excavation/HandPickListenerTest.java @@ -206,6 +206,19 @@ public void joinSyncsToolAndQuitFinishesThenUnlocksMining() { order.verify(pick).syncHeldTool(player); order.verify(pick).finish(player); order.verify(pick).syncHeldTool(player, null); + order.verify(pick).forget(player); + } + + @Test + public void quitForgetsTheHeldStateEvenWhenCleanupFailsAndACharacterSwitchForgetsIt() { + doThrow(new IllegalStateException("cleanup failed")).when(pick).finish(player); + assertThrows(IllegalStateException.class, () -> listener.onQuit(new PlayerQuitEvent(player, "left"))); + verify(pick).forget(player); + var session = new net.tfminecraft.tlibs.character.CharacterSession( + new net.tfminecraft.tlibs.character.CharacterRef(java.util.UUID.randomUUID(), java.util.UUID.randomUUID()), 1); + server.getPluginManager().callEvent(new net.tfminecraft.tlibs.character.CharacterSessionEndEvent( + player, session, net.tfminecraft.tlibs.character.CharacterSessionEndEvent.Reason.SWITCH)); + verify(pick, times(2)).forget(player); } @Test diff --git a/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java b/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java index 68b9ae7..b7383a0 100644 --- a/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java +++ b/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java @@ -928,6 +928,19 @@ public void heldToolMatchIsSharedBetweenTheHoldAndTheHud() { ticks(2); assertTrue(service.isCycling(player)); verify(tools, never()).match(any()); + // The hold sees a changed stack first; the HUD pass of the same tick still restores it once. + ItemStack renamed = new ItemStack(Material.IRON_PICKAXE); + org.bukkit.inventory.meta.ItemMeta meta = renamed.getItemMeta(); + meta.setDisplayName("Field pick"); + renamed.setItemMeta(meta); + player.getInventory().setItemInMainHand(renamed); + service.noteMining(player, target); + clearInvocations(tools); + ticks(1); + assertTrue(service.isCycling(player)); + verify(tools, times(1)).match(any()); + verify(tools, times(1)).restoreAllowedTool(any()); + clearInvocations(tools); player.getInventory().setItemInMainHand(new ItemStack(Material.STICK)); ticks(1); assertFalse(service.isCycling(player)); From 08b8ca377c2eb1f39f7ca3b93efe185b25468ecc Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 5 Nov 2026 12:09:38 +0100 Subject: [PATCH 4/6] Name the forget test for what it checks and reword the switch handler note Co-Authored-By: Claude Opus 5.5 --- .../net/tfminecraft/archaeo/excavation/HandPickListener.java | 4 ++-- .../tfminecraft/archaeo/excavation/HandPickServiceTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java b/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java index 5eabcfd..ab781e8 100644 --- a/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java +++ b/src/main/java/net/tfminecraft/archaeo/excavation/HandPickListener.java @@ -185,8 +185,8 @@ public void onQuit(PlayerQuitEvent event) { } /** - * A character switch can swap the player's saved attributes, so the next tick reads the - * mining lock again instead of trusting what it saw before. + * Defensive reset on a character switch: the next tick reads the mining lock again instead + * of trusting what it saw for the previous character. * * @param event character session end */ diff --git a/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java b/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java index b7383a0..8f82e94 100644 --- a/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java +++ b/src/test/java/net/tfminecraft/archaeo/excavation/HandPickServiceTest.java @@ -948,7 +948,7 @@ public void heldToolMatchIsSharedBetweenTheHoldAndTheHud() { } @Test - public void confirmedUnlockIsReadAgainOnlyEverySecondAndAfterARejoin() { + public void confirmedUnlockIsReadAgainOnlyEverySecondAndAfterForget() { player.getInventory().setItemInMainHand(new ItemStack(Material.STICK)); var speed = player.getAttribute(Attribute.BLOCK_BREAK_SPEED); ticks(1); From d028cd9edee15b439cb94dc156770a9219d81255 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Fri, 6 Nov 2026 16:15:27 +0100 Subject: [PATCH 5/6] fix: a stored site saved again at another chunk is looked up at its new chunk The chunk index remembers which chunk each site was filed under; saving the same site object with a changed world or chunk rebuilds it (CodeRabbit). Co-Authored-By: Claude Opus 5.5 --- .../archaeo/site/SiteRepository.java | 27 ++++++++++++++----- .../archaeo/site/SiteRepositoryTest.java | 6 +++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java b/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java index a93a0d3..df59781 100644 --- a/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java +++ b/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java @@ -71,7 +71,7 @@ public void directorCommitted(UUID id, net.tfminecraft.tlibs.character.Character * rebuilt. A site's world and ruin chunk are set before it is first stored and never change. */ private final AtomicLong layoutVersion = new AtomicLong(); - private volatile ChunkIndex chunkIndex = new ChunkIndex(-1L, Map.of()); + private volatile ChunkIndex chunkIndex = new ChunkIndex(-1L, Map.of(), Map.of()); private final Set dirty = ConcurrentHashMap.newKeySet(); private int nextSerial = 1; private final Set retryWrites = new HashSet<>(); @@ -334,12 +334,13 @@ private Map> chunkIndex() { return current.sites(); } Map> built = new java.util.HashMap<>(); + Map keys = new java.util.HashMap<>(); for (Site site : byId.values()) { - built.computeIfAbsent( - new ChunkKey(site.getWorldName(), site.getChunkX(), site.getChunkZ()), - key -> new ArrayList<>(1)).add(site); + ChunkKey key = ChunkKey.of(site); + built.computeIfAbsent(key, ignored -> new ArrayList<>(1)).add(site); + keys.put(site.getId(), key); } - chunkIndex = new ChunkIndex(version, built); + chunkIndex = new ChunkIndex(version, built, keys); return built; } @@ -349,11 +350,19 @@ private Map> chunkIndex() { * @param site dossier to hold */ private void store(Site site) { - if (byId.put(site.getId(), site) != site) { + if (byId.put(site.getId(), site) != site || moved(site)) { layoutChanged(); } } + /** + * @return whether the index holds {@code site} under another world or chunk than it has now + */ + private boolean moved(Site site) { + ChunkKey indexed = chunkIndex.keys().get(site.getId()); + return indexed != null && !indexed.equals(ChunkKey.of(site)); + } + /** * Call after {@link #byId} gains or loses a site. */ @@ -367,13 +376,17 @@ private void layoutChanged() { * @param z ruin chunk Z */ private record ChunkKey(String world, int x, int z) { + static ChunkKey of(Site site) { + return new ChunkKey(site.getWorldName(), site.getChunkX(), site.getChunkZ()); + } } /** * @param version {@link #layoutVersion} the index was built from * @param sites sites by ruin chunk + * @param keys the chunk each site was indexed under, by site id */ - private record ChunkIndex(long version, Map> sites) { + private record ChunkIndex(long version, Map> sites, Map keys) { } /** diff --git a/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java b/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java index dcb0b9a..fcb3565 100644 --- a/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java +++ b/src/test/java/net/tfminecraft/archaeo/site/SiteRepositoryTest.java @@ -223,6 +223,12 @@ public class SiteRepositoryTest { assertTrue(repository.loadAll()); assertEquals(extra.getId(), repository.findByChunk("world", 7, 7).orElseThrow().getId()); assertTrue(repository.hasEstablishedIn("world", 7, 7, 7, 7)); + // A stored site whose place changes is found at its new chunk once it is saved again. + Site moving = repository.findByChunk("world", 7, 7).orElseThrow(); + moving.setChunkX(9); + net.tfminecraft.archaeo.CharacterTestSupport.touch(repository, moving); + assertTrue(repository.findByChunk("world", 7, 7).isEmpty()); + assertEquals(moving.getId(), repository.findByChunk("world", 9, 7).orElseThrow().getId()); repository.erase(later); assertTrue(repository.findByChunk("world", 5, -5).isEmpty()); assertFalse(repository.hasEstablishedIn("world", 5, 5, -5, -5)); From c8cba8d8a382fd2a499939a7e05726f2be259757 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Fri, 6 Nov 2026 16:27:09 +0100 Subject: [PATCH 6/6] docs: the chunk index comment says when it is rebuilt Co-Authored-By: Claude Opus 5.5 --- .../java/net/tfminecraft/archaeo/site/SiteRepository.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java b/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java index df59781..917a373 100644 --- a/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java +++ b/src/main/java/net/tfminecraft/archaeo/site/SiteRepository.java @@ -67,8 +67,9 @@ public void directorCommitted(UUID id, net.tfminecraft.tlibs.character.Character private final File indexFile; private final Map byId = new ConcurrentHashMap<>(); /** - * Bumped after every change to which sites {@link #byId} holds, so {@link #chunkIndex()} is - * rebuilt. A site's world and ruin chunk are set before it is first stored and never change. + * Bumped after every change to which sites {@link #byId} holds, and when a stored site is + * saved again at another world or ruin chunk, so {@link #chunkIndex()} is rebuilt. A site's + * place is normally set before it is first stored. */ private final AtomicLong layoutVersion = new AtomicLong(); private volatile ChunkIndex chunkIndex = new ChunkIndex(-1L, Map.of(), Map.of());