Add Folia support - #330
Conversation
- plugin.yml: declare folia-supported: true (hard requirement to load on Folia). - CrateManager.playCrateEffects(): moved off the async task pool and now dispatches the actual particle spawn per crate-block location via plugin.runTask(Location, Runnable), instead of calling world/player particle APIs from a non-region thread. - HologramManager: same pattern for hologram creation/rendering (createIfAbsent/render), split render() into a per-group renderGroup() dispatched per block location. displayMap and FakeDisplay's internal group map are now ConcurrentHashMap since group creation can now be dispatched concurrently across regions for the same crate. - OpeningManager.tickOpenings(): dispatches each opening's tick onto its own player's thread instead of the global region thread, since ticking mutates that player's inventory/menu state. Not yet done: ProtocolLib/packetevents Folia-safety audit, and a real build/test pass (no Maven available in this environment to compile against the private nightexpress/spigot-snapshot repos).
nightcore 2.10.0 (the version this project was pinned to) has no Folia scheduler bridge at all -- NightPlugin.runTask() there just delegates to the plain BukkitScheduler. The runTask(Location, Runnable) and runTask(Entity, Runnable) overloads used by the ticker fixes only exist from a later nightcore version onward, so Folia support requires this bump regardless of the ticker changes. Also add the missing java.util.concurrent.ConcurrentHashMap import in HologramManager (java.util.* doesn't cover java.util.concurrent). Verified: set up Maven 3.9.16 locally and ran a full `mvn clean package` against the real, published nightcore 2.16.3 artifact -- builds clean, only pre-existing deprecation warnings unrelated to this change.
Set up a local Folia 1.21.8 test server (build 6), built nightcore 2.16.3 and packetevents from their real shaded/release artifacts, and ran ExcellentCrates against it. Two genuine bugs surfaced that a compile-only check couldn't catch: 1. DataManager/CrateManager/KeyManager used nightcore's legacy runTask(Consumer<BukkitTask>) / runTaskAsync(Consumer<BukkitTask>) overloads via lambdas like `task -> ...`. That overload bypasses nightcore's Folia-safe AdaptedScheduler entirely and calls the raw BukkitScheduler directly, which throws UnsupportedOperationException on Folia -- this crashed ExcellentCrates on every enable. Switched all 10 call sites to the zero-arg `() -> ...` form, which resolves to the Runnable overload and correctly routes through the scheduler bridge. 2. Crate.load() read block state (block.isEmpty()) synchronously while filtering out air-block positions, straight off whatever thread loaded/reloaded the crate config. Reproduced live: `/crates reload` threw "Cannot read world asynchronously" because the command thread wasn't the block's owning region thread. Fixed by dispatching the block read through plugin.runTask(location, ...) per position, and made Crate.blockPositions a concurrent set since removal can now happen from multiple region threads. Verified: full mvn build succeeds, plugin loads/enables/reloads cleanly on a real Folia server with an actual configured crate (block position, hologram, and particle effect all enabled), and idles for 90s with zero exceptions from the periodic tickers.
|
What's the difference between this and #324 beyond the seemingly AI-generated PR message? |
|
Fair question, and happy to be concrete about it rather than just asserting it's different. Functionally, #324 and this PR both fix the same
On the PR description: yes, I used AI assistance to help draft the summary/test-plan text, but the changes themselves were driven by reproducing actual crashes on a live Folia server, not generated blind. Happy to expand on any part of the diff if useful. |
|
The violations you're describing derive from Canvas (not Folia) explicitly. You're right that these should be considered; however, it's not worth saying much more, given that your response (on top of the PR) is fully AI-generated. It'd be better to PR changes against my existing PR (since it's fully up-to-date), rather than to reinvent the wheel |
|
Sorry, I should have been clearer from the start. English is not my first language, so I use AI to help me write the PR description and the comments, and I use it while coding too. I am not going to pretend otherwise. But I have a degree in software development, so I am not leaning on it 100%. I read what goes in and I understand the scheduler changes I made here. The two bugs in this PR did not come from a model guessing at what might be wrong. I hit them by running the plugin on a real server and reading the stack traces. About Canvas, I never installed it. My testing was on a plain Folia 1.21.8 server. The On reinventing the wheel, that is fair and I do not mind. If you and nulli0n prefer one PR, I can open a PR against your branch with the |
3 participants
Summary
folia-supported: trueinplugin.ymland bumps thenightcoredependency to 2.16.3 (2.10.0, the version currently pinned, has no Folia scheduler bridge at all —NightPlugin.runTask()there just calls the plainBukkitSchedulerdirectly).CrateManager.playCrateEffects,HologramManagercreation/rendering,OpeningManager.tickOpenings) to dispatch their actual world/entity-touching work throughplugin.runTask(Location, …)/runTask(Entity, …)instead of running it on the global-region or async thread pool.DataManager/KeyManager/CrateManagerused nightcore'srunTask(task -> …)pattern. That single-arg lambda resolves to a legacyConsumer<BukkitTask>overload that bypasses nightcore's Folia-safe scheduler bridge and calls the rawBukkitSchedulerdirectly — which throwsUnsupportedOperationExceptionon Folia, crashing the plugin on every enable. Switched all 10 call sites to zero-arg() -> …lambdas, which resolve to the Folia-safeRunnableoverload.Crate.load()read block state (block.isEmpty()) synchronously while filtering out air-block positions, off whatever thread loaded/reloaded the crate config. Reproduced live:/crates reloadthrewIllegalStateException: Cannot read world asynchronously. Fixed by dispatching that read throughplugin.runTask(location, …)per position;Crate.blockPositionsandHologramManager's internal maps are now concurrent-safe since this can now be touched from multiple region threads.Test plan
mvn clean packagesucceeds against the real published nightcore 2.16.3 artifact./crates reload(the exact path that previously crashed) completes cleanly with a real crate configured (block position + hologram + particle effect enabled).