Add CivScheduler facade and migrate civmodcore sites - #30
Open
grepsedawk wants to merge 1 commit into
Open
Conversation
civmodcore had no scheduling abstraction: 13 call sites used the Bukkit scheduler directly, which throws under Folia's regionized threading. This blocks Folia readiness for every dependent plugin. Add a CivScheduler facade that delegates to the region-aware schedulers Paper exposes on every build, so a single jar runs on both Paper and Folia with no reflection or server-type detection. It offers runGlobal/runRegion/runEntity/runAsync with later and timer variants returning a cancellable CivTask. Migrate civmodcore's own sites onto it, routing block and entity work to the region and entity schedulers so it stays correct when regionized. Because a global-thread timer now touches state that region- and entity-thread handlers also write, make the affected collections concurrent: DoubleInteractFixer, BottomLine and CivScoreBoard move to ConcurrentHashMap, PlayerNames to a concurrent key set, and TickCoolDownHandler's counter to an AtomicLong. Also fix two latent bugs the migration exposed: DelayedItemDrop mutated the caller's Location cumulatively in its drop loop (clone once), and CivTask.wrap wrapped a null handle when the entity scheduler declined a retired entity (return a no-op task). Two pre-existing Folia concerns are preserved unchanged and left for follow-up: global chunk-meta iteration in GlobalChunkMetaManager and the same race class in collections adjacent to the migrated ones.
grepsedawk
force-pushed
the
civmodcore-civscheduler-facade
branch
from
June 3, 2026 06:34
4ef2f99 to
32bba2a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Foundation for Folia readiness — touches civmodcore, the linchpin imported by ~31 plugins. Already reviewed once on the fork (grepsedawk#3); review hardening is folded in.
What
civmodcore had no scheduling abstraction: 13 call sites used
Bukkit.getScheduler()/BukkitRunnabledirectly, which throw under Folia's regionized threading.vg.civcraft.mc.civmodcore.scheduling.CivScheduler—runGlobal/runRegion(Location|Block)/runEntity/runAsync, each withLaterandTimervariants returning a cancellableCivTask.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. (API presence verified against paper-api 1.21.8 viajavap.)Thread-safety hardening
Moving these sites onto region schedulers means a global-thread timer now touches state that region/entity-thread event handlers also write. Made those structures concurrent:
DoubleInteractFixer,BottomLine,CivScoreBoard:TreeMap→ConcurrentHashMap(UUID keys, no ordering relied upon).PlayerNames:HashSet→ConcurrentHashMap.newKeySet().TickCoolDownHandler:tickCounter→AtomicLong.Plus two latent bugs the migration exposed:
DelayedItemDropmutated the caller'sLocationcumulatively in its drop loop (clone once);CivTask.wrapwrapped a null handle when the entity scheduler declined a retired entity (return a no-op task).Tests
CivTaskTests(wrap/cancel/no-op)../gradlew :plugins:civmodcore-paper:testBUILD SUCCESSFUL; existing suites pass. NoBukkit.getScheduler()/BukkitRunnableremain in main source outside the facade.Notes for reviewers
taskchain(used only by jukealert) is out of scope — not Folia-safe, but the facade only replaces direct Bukkit-scheduler sites.GlobalChunkMetaManageriterates all worlds/loaded chunks from the global region (cross-region iteration isn't thread-safe on Folia); and the same race class extends to a few collections adjacent to the migrated ones (BottomLineAPI.lines,ScoreBoardAPIregistries,TickCoolDownHandler.cds, the inner list inDoubleInteractFixer) — worth a dedicated pass.This is the gate for migrating the ~300 scheduler sites across dependent plugins.