Skip to content

Commit 22f20bb

Browse files
tastybentoclaude
andcommitted
fix: self-healing per-chunk handicap and address Copilot review
The lazy-zero handicap was a single number written once at zero scan and only updated by ChunkLoadEvent.isNewChunk=true. Chunks that materialized through any other path — Poseidon pregenerating around the island grid before the island existed, async-load misses at zero-scan time, late chunk decoration — were silently missing from the handicap and showed up as positive level on islands the player had never touched. Track handicap value per chunk on IslandLevels (keyed by worldName:chunkKey), then reconcile on every scan: chunks the live scan visits that aren't in the persisted map are folded into both the map and initialCount in one atomic write, so the next scan reads level=0 for that previously-missing terrain. Frozen-once semantics ensure player builds still grow the live total without inflating the handicap, and an empty map with a non-zero initialCount is treated as legacy migration so existing islands don't get double-credited on upgrade. Also addresses Copilot's PR #441 review: - delete unbounded queuedChunks dedup from NewChunkListener (isNewChunk() is the dedup; the defensive set grew without bound) - route per-chunk scoring through IslandLevelCalculator's real processBlock logic so custom blocks, slabs, and configured values match the regular scan - clamp completePendingZero at zero, remove the per-island map entry when it empties, and close the add/complete race via compute() Multi-dim correctness: the visited/deferred maps now key on worldName:chunkKey so the same (x,z) in different dimensions tracks separately. drainZeroScanDeferred returns the missed entries as a map which the caller reconciles into the per-chunk handicap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0548a0a commit 22f20bb

7 files changed

Lines changed: 535 additions & 183 deletions

File tree

src/main/java/world/bentobox/level/LevelsManager.java

Lines changed: 175 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,11 @@ public class LevelsManager {
6666
* scan. Populated as the scan reads each chunk's snapshot; used by the
6767
* post-scan drain to skip chunks that the scan already credited in
6868
* {@code totalPoints} (preventing double-counting when the chunk listener
69-
* also fires for the same chunk during the scan window).
69+
* also fires for the same chunk during the scan window). Keys are full
70+
* {@code worldName:chunkKey} strings so chunks at the same {@code (x,z)}
71+
* in different dimensions don't collide.
7072
*/
71-
private final Map<String, Set<Long>> zeroScanVisitedChunks = new ConcurrentHashMap<>();
73+
private final Map<String, Set<String>> zeroScanVisitedChunks = new ConcurrentHashMap<>();
7274
/**
7375
* Per-island deferred listener credits captured while a zero-island scan
7476
* is in progress. Without this, listener {@code addToInitialCount} calls
@@ -77,9 +79,11 @@ public class LevelsManager {
7779
* {@link #setInitialIslandCount setInitialIslandCount(totalPoints)}, and
7880
* those chunks' values would appear in future scan totals with no
7981
* matching handicap — producing a stable positive level on a fresh
80-
* island.
82+
* island. Keys mirror {@link #zeroScanVisitedChunks} —
83+
* {@code worldName:chunkKey} — so the same chunk position in different
84+
* dimensions is tracked separately.
8185
*/
82-
private final Map<String, Map<Long, Long>> zeroScanDeferredCredits = new ConcurrentHashMap<>();
86+
private final Map<String, Map<String, Long>> zeroScanDeferredCredits = new ConcurrentHashMap<>();
8387

8488
public LevelsManager(Level addon) {
8589
this.addon = addon;
@@ -545,21 +549,43 @@ public void addToInitialCount(@NonNull Island island, long delta) {
545549
/**
546550
* Mark that one more lazy-zero snapshot is queued for {@code island}.
547551
* Paired with {@link #completePendingZero(Island)} when the snapshot has
548-
* been processed.
552+
* been processed. The increment runs inside {@code compute} so it is
553+
* atomic with the removal done by {@code completePendingZero} — a
554+
* decrement that empties the entry cannot race a concurrent add into
555+
* leaving an orphaned counter outside the map.
549556
*/
550557
public void addPendingZero(@NonNull Island island) {
551-
pendingZeros.computeIfAbsent(island.getUniqueId(), k -> new AtomicInteger()).incrementAndGet();
558+
pendingZeros.compute(island.getUniqueId(), (k, c) -> {
559+
if (c == null) {
560+
c = new AtomicInteger();
561+
}
562+
c.incrementAndGet();
563+
return c;
564+
});
552565
}
553566

554567
/**
555568
* Mark that a previously {@link #addPendingZero queued} snapshot has
556-
* finished. Safe to call from any thread.
569+
* finished. Safe to call from any thread. Clamps at zero (an extra
570+
* complete with no matching add is a no-op rather than producing a
571+
* negative count that would let {@link #getPendingZeroCount} read 0 and
572+
* release a scan early) and drops the per-island map entry once the
573+
* counter reaches zero so the map size stays bounded by the number of
574+
* islands with in-flight work, not the total number of zero scans ever.
557575
*/
558576
public void completePendingZero(@NonNull Island island) {
559-
AtomicInteger c = pendingZeros.get(island.getUniqueId());
560-
if (c != null) {
561-
c.decrementAndGet();
562-
}
577+
pendingZeros.compute(island.getUniqueId(), (k, c) -> {
578+
if (c == null) {
579+
return null;
580+
}
581+
if (c.get() <= 0) {
582+
// No matching add — drop the orphaned entry instead of
583+
// letting it drift negative.
584+
return null;
585+
}
586+
int v = c.decrementAndGet();
587+
return v <= 0 ? null : c;
588+
});
563589
}
564590

565591
/**
@@ -625,54 +651,167 @@ public void beginZeroScan(@NonNull Island island) {
625651
}
626652

627653
/**
628-
* Record that the zero-island scan visited (counted blocks for) a chunk.
629-
* Called from the scanner on the worker thread.
654+
* Record that the zero-island scan visited (counted blocks for) a chunk
655+
* in the given world. Called from the scanner on the worker thread.
630656
*/
631-
public void recordScanVisitedChunk(@NonNull Island island, int chunkX, int chunkZ) {
632-
Set<Long> set = zeroScanVisitedChunks.get(island.getUniqueId());
657+
public void recordScanVisitedChunk(@NonNull Island island, @NonNull String worldName,
658+
int chunkX, int chunkZ) {
659+
Set<String> set = zeroScanVisitedChunks.get(island.getUniqueId());
633660
if (set != null) {
634-
set.add(chunkKey(chunkX, chunkZ));
661+
set.add(worldName + ":" + chunkKey(chunkX, chunkZ));
635662
}
636663
}
637664

638665
/**
639666
* Try to record a listener credit during an active zero scan. If no
640667
* scan is active for this island, returns false and the caller should
641-
* fall back to {@link #addToInitialCount}. If a scan is active, the
642-
* credit is stored against the chunk key for later processing by
643-
* {@link #drainZeroScanDeferred}.
668+
* fall back to {@link #addHandicapChunk}. If a scan is active, the
669+
* credit is stored against the {@code worldName:chunkKey} for later
670+
* processing by {@link #drainZeroScanDeferred}.
644671
*/
645-
public boolean tryDeferZeroScanCredit(@NonNull Island island, int chunkX, int chunkZ, long value) {
646-
Map<Long, Long> deferred = zeroScanDeferredCredits.get(island.getUniqueId());
672+
public boolean tryDeferZeroScanCredit(@NonNull Island island, @NonNull String worldName,
673+
int chunkX, int chunkZ, long value) {
674+
Map<String, Long> deferred = zeroScanDeferredCredits.get(island.getUniqueId());
647675
if (deferred == null) {
648676
return false;
649677
}
650-
deferred.put(chunkKey(chunkX, chunkZ), value);
678+
deferred.put(worldName + ":" + chunkKey(chunkX, chunkZ), value);
651679
return true;
652680
}
653681

654682
/**
655-
* End the active zero scan for {@code island} and return the sum of
656-
* deferred listener credits for chunks the scan did NOT visit. The
657-
* caller should add this sum to the initial count immediately after
658-
* {@link #setInitialIslandCount}, so chunks that the scan skipped
659-
* (ungenerated at poll time, generated mid-scan) are preserved instead
660-
* of being wiped by the baseline reset.
683+
* End the active zero scan for {@code island} and return the deferred
684+
* listener credits for chunks the scan did NOT visit, keyed by
685+
* {@code worldName:chunkKey}. The caller passes this map to {@link
686+
* #reconcileHandicapChunks} so the missed chunks become part of the
687+
* persisted per-chunk handicap and are added to the initial count in one
688+
* atomic save. Chunks the scan visited are dropped because their values
689+
* are already in {@code results.totalPoints}.
661690
*/
662-
public long drainZeroScanDeferred(@NonNull Island island) {
691+
public Map<String, Long> drainZeroScanDeferred(@NonNull Island island) {
663692
String id = island.getUniqueId();
664-
Set<Long> visited = zeroScanVisitedChunks.remove(id);
665-
Map<Long, Long> deferred = zeroScanDeferredCredits.remove(id);
693+
Set<String> visited = zeroScanVisitedChunks.remove(id);
694+
Map<String, Long> deferred = zeroScanDeferredCredits.remove(id);
666695
if (deferred == null || deferred.isEmpty()) {
667-
return 0L;
696+
return Collections.emptyMap();
668697
}
669-
long sum = 0L;
670-
for (Map.Entry<Long, Long> e : deferred.entrySet()) {
698+
Map<String, Long> missed = new HashMap<>();
699+
for (Map.Entry<String, Long> e : deferred.entrySet()) {
671700
if (visited == null || !visited.contains(e.getKey())) {
672-
sum += e.getValue();
701+
missed.put(e.getKey(), e.getValue());
702+
}
703+
}
704+
return missed;
705+
}
706+
707+
// ---- Per-chunk handicap reconciliation ----
708+
709+
/**
710+
* Fold per-chunk scan results into the island's persistent handicap-chunks
711+
* map and return the net delta to add to the initial-count handicap.
712+
* <p>
713+
* Behaviour:
714+
* <ul>
715+
* <li><b>Zero scan</b>: the map is overwritten with {@code scannedChunkValues}.
716+
* The caller is expected to {@code setInitialIslandCount(island, totalPoints)}
717+
* (a hard reset of the baseline), so the delta returned here is 0 — the new
718+
* baseline is conveyed via the totalPoints write, not added on top.</li>
719+
* <li><b>Regular scan, empty existing map, non-zero initialCount</b>: legacy
720+
* migration. Seed the map with the current scan values without touching
721+
* initialCount — the player's existing level progress is preserved, and
722+
* from this scan onward the map is the source of truth. Returns 0.</li>
723+
* <li><b>Regular scan, normal case</b>: for every chunk in
724+
* {@code scannedChunkValues} whose key is not already in the persisted map,
725+
* add the value to the map and accumulate it into the return delta. The
726+
* caller adds that delta to initialCount, which is what makes the handicap
727+
* self-healing: chunks the lazy-zero listener missed get credited the
728+
* first time a regular level scan visits them.</li>
729+
* </ul>
730+
* Once a chunk is in the map its value is frozen — subsequent growth
731+
* (player builds) increases the live scan total without changing the
732+
* handicap, which is the intended behaviour. Subsequent shrinkage (player
733+
* breaks naturally-generated blocks) likewise leaves the handicap alone;
734+
* the negative {@code totalPoints - initialCount} delta correctly drags
735+
* the level below zero.
736+
*
737+
* @param island the island being reconciled
738+
* @param scannedChunkValues per-chunk values from the calculator, keyed
739+
* by {@code worldName:chunkKey}
740+
* @param zeroScan true when this is a zero-island handicap
741+
* scan (overwrites the map)
742+
* @return the number of points to add to the island's initial count; 0
743+
* for zero scans and legacy-migration scans
744+
*/
745+
public long reconcileHandicapChunks(@NonNull Island island,
746+
@NonNull Map<String, Long> scannedChunkValues, boolean zeroScan) {
747+
IslandLevels data = getLevelsData(island);
748+
Map<String, Long> persisted = data.getHandicapChunks();
749+
if (zeroScan) {
750+
// Hard reset — the new baseline is everything the scan saw. The
751+
// initial count is rewritten separately by the caller, since a
752+
// zero scan is meant to be the canonical re-baseline.
753+
persisted.clear();
754+
persisted.putAll(scannedChunkValues);
755+
handler.saveObjectAsync(data);
756+
return 0L;
757+
}
758+
if (persisted.isEmpty()) {
759+
Long existingCount = data.getInitialCount();
760+
if (existingCount != null && existingCount > 0L) {
761+
// Legacy migration: an island that was zeroed before this
762+
// feature existed. The existing initialCount already covers
763+
// every chunk currently on disk, so seeding the map without
764+
// touching initialCount keeps the level stable while moving
765+
// future drift detection onto the per-chunk path.
766+
persisted.putAll(scannedChunkValues);
767+
handler.saveObjectAsync(data);
768+
return 0L;
673769
}
674770
}
675-
return sum;
771+
long delta = 0L;
772+
for (Map.Entry<String, Long> e : scannedChunkValues.entrySet()) {
773+
if (!persisted.containsKey(e.getKey())) {
774+
long value = e.getValue() == null ? 0L : e.getValue();
775+
persisted.put(e.getKey(), value);
776+
delta += value;
777+
}
778+
}
779+
if (delta != 0L) {
780+
// Self-heal: any chunk the scan saw but the persisted map didn't
781+
// know about gets folded into both the map and the initialCount
782+
// in the same write, so the next scan reads a level of zero for
783+
// that previously-missing terrain.
784+
long existing = data.getInitialCount() == null ? 0L : data.getInitialCount();
785+
data.setInitialCount(existing + delta);
786+
handler.saveObjectAsync(data);
787+
}
788+
return delta;
789+
}
790+
791+
/**
792+
* Record a single chunk's handicap value and add its value to {@link
793+
* IslandLevels#getInitialCount initialCount} in one atomic save. Used by
794+
* {@link world.bentobox.level.listeners.NewChunkListener} when no zero
795+
* scan is active. If the chunk is already in the map, this is a no-op
796+
* (frozen-once semantics — see {@link #reconcileHandicapChunks}). The
797+
* deferred path used during an active zero scan goes through {@link
798+
* #tryDeferZeroScanCredit} and {@link #drainZeroScanDeferred} →
799+
* {@link #reconcileHandicapChunks} instead, so the same chunk is never
800+
* credited twice.
801+
*
802+
* @return true if the chunk was newly added (and initialCount adjusted)
803+
*/
804+
public boolean addHandicapChunk(@NonNull Island island, @NonNull String key, long value) {
805+
IslandLevels data = getLevelsData(island);
806+
Map<String, Long> persisted = data.getHandicapChunks();
807+
if (persisted.containsKey(key)) {
808+
return false;
809+
}
810+
persisted.put(key, value);
811+
long existing = data.getInitialCount() == null ? 0L : data.getInitialCount();
812+
data.setInitialCount(existing + value);
813+
handler.saveObjectAsync(data);
814+
return true;
676815
}
677816

678817
/**

0 commit comments

Comments
 (0)