Add CivScheduler facade and migrate civmodcore sites

[grepsedawk]

Foundation for Folia readiness. For review — this touches civmodcore, the linchpin imported by 31 plugins.

What

civmodcore had no scheduling abstraction: 13 call sites used Bukkit.getScheduler() / BukkitRunnable directly, which throw under Folia's regionized threading.

• New vg.civcraft.mc.civmodcore.scheduling.CivScheduler — runGlobal / runRegion(Location|Block) / runEntity / runAsync, each with Later and Timer variants returning a cancellable CivTask.
• Delegates directly to the region-aware schedulers Paper exposes on every build (getGlobalRegionScheduler / getRegionScheduler / getAsyncScheduler / Entity#getScheduler). On regular Paper these run on the main thread; on Folia they regionize. No reflection, no runtime server-type detection — one shadow jar runs on both. (Verified the API is present in paper-api 1.21.8 via javap.)
• Migrated all 13 civmodcore call sites. Block/entity work routes to the region/entity schedulers so it stays correct when regionized.

Tests

CivTaskTests (cancel/isCancelled delegation). ./gradlew :plugins:civmodcore-paper:test BUILD SUCCESSFUL; existing suites still pass. No Bukkit.getScheduler() / BukkitRunnable remain in main source outside the facade.

Notes for reviewers

• Adds one public overload runAsync(Plugin, Runnable) — widens the public API on a heavily-imported module.
• aikar taskchain (used only by jukealert) is out of scope — it's not Folia-safe but the facade only replaces direct Bukkit-scheduler sites.
• Two pre-existing Folia concerns preserved as faithful mappings, flagged for follow-up:
  1. GlobalChunkMetaManager iterates all worlds/loaded chunks from the global region — cross-region iteration isn't thread-safe on Folia.
  2. DelayedItemDrop mutates the Location cumulatively (l.add(0.5,0.5,0.5) per stack inside the loop) — looks like a latent bug, left unchanged here.

This is the gate for migrating the ~300 scheduler sites across dependent plugins.

[gemini-code-assist]

Code Review

This pull request introduces a scheduling facade (CivScheduler and CivTask) over Paper's region-aware schedulers to support Folia compatibility, replacing direct usage of Bukkit.getScheduler() and BukkitRunnable across various components. While this is a solid architectural improvement, transitioning to region-aware scheduling introduces several concurrency hazards. Specifically, multiple non-thread-safe collections (such as TreeMap and HashSet in DoubleInteractFixer, BottomLine, CivScoreBoard, and PlayerNames) are now accessed concurrently across different region and global threads, which will lead to race conditions and ConcurrentModificationExceptions. Additionally, CivTask.wrap needs to safely handle null tasks to prevent NullPointerExceptions, tickCounter in TickCoolDownHandler requires proper thread visibility, and a location mutation bug in DelayedItemDrop should be resolved by cloning the location before modification.

[gemini-code-assist]

This loop mutates the Location object l cumulatively via l.add(0.5, 0.5, 0.5) on each iteration. If there are multiple item stacks, each subsequent item will be dropped with an increasing offset (e.g., first at +0.5, second at +1.0, third at +1.5, etc.).

To fix this, clone the location once outside the loop.

        CivScheduler.runRegionLater(l, () -> {
            final Location dropLoc = l.clone().add(0.5, 0.5, 0.5);
            for (ItemStack is : stacks) {
                dropLoc.getWorld().dropItem(dropLoc, is).setVelocity(new Vector(0, 0.05, 0));
            }
        }, 1L);

[grepsedawk]

Confirmed. Location.add mutates in place and returns this, so calling l.add(0.5,0.5,0.5) inside the per-stack loop accumulated the offset (+0.5, +1.0, +1.5, ...) and mutated the caller's Location. Fixed by cloning once before the loop and dropping all stacks at that centered location.

[gemini-code-assist]

Paper's region and entity schedulers can return null if a task cannot be scheduled (for example, if the target entity is invalid/unloaded, or if the plugin is disabling). Passing null to CivTask.wrap will result in a NullPointerException when cancel() or isCancelled() is subsequently called on the returned CivTask.

To prevent this, handle null by returning a safe, no-op CivTask implementation.

    static CivTask wrap(final ScheduledTask task) {
        return task == null ? NoOpCivTask.INSTANCE : new ScheduledCivTask(task);
    }

    record NoOpCivTask() implements CivTask {
        static final NoOpCivTask INSTANCE = new NoOpCivTask();
        @Override public void cancel() {}
        @Override public boolean isCancelled() { return true; }
    }

    record ScheduledCivTask(ScheduledTask task) implements CivTask {

        @Override
        public void cancel() {
            this.task.cancel();
        }

        @Override
        public boolean isCancelled() {
            return this.task.isCancelled();
        }

    }

[grepsedawk]

Confirmed for the entity scheduler paths: EntityScheduler.runDelayed/runAtFixedRate (used by runEntityLater/runEntityTimer) return null when the entity is retired before scheduling, and wrap() previously built ScheduledCivTask(null), NPEing on later cancel()/isCancelled(). Fixed by making wrap() return a NoOpCivTask singleton on null input (cancel() no-ops, isCancelled() returns true). ScheduledCivTask is unchanged for the non-null path; global/region/async callers are unaffected since they never return null.

[gemini-code-assist]

On Folia, player interactions are processed concurrently on their respective region threads, while this timer runs on the global region thread. Since locations is initialized as a non-thread-safe TreeMap, concurrent access (adding locations on region threads and clearing them here on the global thread) will lead to race conditions and ConcurrentModificationExceptions.

Please change locations to use a thread-safe map implementation, such as ConcurrentHashMap:

this.locations = new ConcurrentHashMap<>();

[grepsedawk]

Valid. checkInteracted runs from PlayerInteractEvent handlers (per-region thread on Folia) while the clear timer runs on the global region thread via getGlobalRegionScheduler, so clear() races with computeIfAbsent on the bare TreeMap. Switched the map to ConcurrentHashMap. ConcurrentHashMap (not ConcurrentSkipListMap) is correct here: keys are non-null player UUIDs and only computeIfAbsent/clear are used, so no sorted/Navigable ordering was relied on. The inner per-UUID list stays a plain LinkedList since a player's interacts always fire on that player's single region thread.

[gemini-code-assist]

On Folia, player data updates and removals occur on region-specific threads, while this periodic updater runs on the global region thread. Since texts is a non-thread-safe TreeMap, iterating over it here while other threads concurrently call updatePlayer or removePlayer will cause race conditions and ConcurrentModificationExceptions.

Please change texts to a thread-safe map, such as ConcurrentHashMap:

this.texts = new ConcurrentHashMap<>();

(Note: Since UUID keys do not require sorting, ConcurrentHashMap is a perfect high-performance replacement here.)

[grepsedawk]

Applied. texts is now a ConcurrentHashMap. The map is written by updatePlayer/removePlayer from event handlers (citadel/bastion ModeListener, finale cooldown listeners, finale AsyncPacketHandler) which run on region/entity/async threads under Folia, while updatePeriodically's task iterates and structurally modifies it on the global thread via getGlobalRegionScheduler — a real cross-thread race on a plain TreeMap. UUID keys are non-null and no Navigable/sorted-order ops are used (refreshAll copies into its own separate map), so ConcurrentHashMap suffices over ConcurrentSkipListMap.

[gemini-code-assist]

On Folia, scoreboard updates (like set, hide, or purge) are called from region-specific threads, while this periodic updater runs on the global region thread. Since currentScoreText is a non-thread-safe TreeMap, iterating over it here while other threads concurrently modify it will cause race conditions and ConcurrentModificationExceptions.

Please change currentScoreText to a thread-safe map, such as ConcurrentHashMap:

this.currentScoreText = new ConcurrentHashMap<>();

(Note: Since UUID keys do not require sorting, ConcurrentHashMap is a perfect high-performance replacement here.)

[grepsedawk]

Confirmed and applied. currentScoreText is keyed by UUID (always p.getUniqueId(), non-null) and the code uses no Navigable/sorted-order operations, so ConcurrentHashMap is the right fit. The race is real on Folia: the migrated updatePeriodically now uses CivScheduler.runGlobalTimer (global region thread) and is live in production (finale CooldownHandler/PearlCoolDownListener), while set/hide are invoked from PlayerSetting.setValue listener callbacks and event-driven HUD updates that run on player region/entity threads — concurrent mutation of a plain TreeMap during iteration. Swapped TreeMap -> ConcurrentHashMap; its entrySet iterator still supports iter.remove() and entry.setValue() used by the updater.

[gemini-code-assist]

On Folia, player login events and reads via getPlayerNames() can occur concurrently across different region threads. Since names is a non-thread-safe HashSet, modifying it here on the global region thread while other threads read or write to it will cause race conditions.

Please change names to a thread-safe set implementation, such as ConcurrentHashMap.newKeySet():

private static final Set<String> names = ConcurrentHashMap.newKeySet();

[grepsedawk]

Confirmed real under Folia: the seed task writes via runGlobal on the global region thread, onLogin (PlayerLoginEvent, MONITOR) writes from the connection/login thread, and the public static getPlayerNames() is read from arbitrary threads. Replaced the HashSet with ConcurrentHashMap.newKeySet(). Set semantics are unchanged (the original was unordered, so no Navigable/sorted ops to preserve) and the names are non-null player-name Strings, so newKeySet is the correct choice over ConcurrentSkipListMap. Collections.unmodifiableSet still gives a thread-safe live view.

[gemini-code-assist]

On Folia, cooldown checks will occur on various region threads, while tickCounter is incremented here on the global region thread. If tickCounter is a plain long or int, this concurrent access constitutes a data race, and updates may not be immediately visible to other threads.

Please ensure tickCounter is declared as an AtomicLong (using incrementAndGet()) or at least marked as volatile to guarantee cross-thread visibility.

[grepsedawk]

Confirmed and applied. tickCounter is incremented by the runGlobalTimer task (global region thread on Folia) but read by onCoolDown/putOnCoolDown/getRemainingCoolDown, which finale invokes from entity-region-thread event handlers — no cross-thread visibility guarantee for a plain long. Switched it to a final AtomicLong: incrementAndGet() in the task, .get() at the read sites. AtomicLong rather than volatile since reads do arithmetic on the value. Note: the backing cds HashMap is also touched across these same threads and is unsynchronized, but that is a separate concern from this comment.