Add shared platform services with Bukkit/Folia adapters - #319
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac0353315c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Objects.requireNonNull(playerId, "playerId"); | ||
| Objects.requireNonNull(task, "task"); | ||
| Player player = Bukkit.getPlayer(playerId); | ||
| if (player == null || !player.isOnline()) { |
There was a problem hiding this comment.
runPlayer is explicitly the handoff intended for asynchronous consumers, but it calls Player.isOnline() on the caller's arbitrary thread before submitting anything to the entity scheduler. On Paper/Folia, an async caller therefore accesses a Bukkit entity outside its owning thread and may trigger thread-safety failures before the callback can be handed off; use the UUID lookup only to locate the entity and perform the session/online validation inside the scheduled callback.
AGENTS.md reference: AGENTS.md:L32-L32
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 264234e.
runPlayer now uses UUID lookup only to locate the entity and rejects a null lookup without reading player state on the submitting thread. The existing captured-session online/identity validation remains inside the entity-scheduled callback, along with the plugin-enabled check. There is still no global fallback or redirection to a replacement login. Updated the scheduling contract to clarify that true means submitted, not confirmed online or completed.
Added two regression tests: a real worker-thread submission that fails if Player.isOnline() is called before the entity callback, and an already-disconnected lookup that is checked/rejected only within that callback. Existing disconnect, replacement, disable, and rejection coverage remains intact.
Verified Maven CI run 596, build job 103625631616: mvn -B -f AdvancedCore/pom.xml package on Java 21 passed with 332 tests, 0 failures, 0 errors, 0 skipped, including all 10 BukkitPlatformServicesTest cases. AdvancedCore.jar was generated and shading completed. Maven refreshed the snapshot and resolved SimpleAPI 1.0.2-20260912.212854-22; the POM remains 1.0.2-SNAPSHOT.
Locally, a guarded component probe reproduced the original off-thread read and passed all seven handoff scenarios after the fix; that probe uses test-only native boundary stubs, not a live Bukkit/Folia server. Java syntax and git diff --check passed, and all four changed file hashes match the uploaded commit. Full local Maven/downstream builds remain unavailable here (Maven absent and dependency-host DNS unavailable). GitHub Actions results are not represented as local/live-server validation, and shading warnings still require packaged-runtime verification.
Codex has automatically started a fresh code review of the new commit; no duplicate manual review request was posted. No merge or deployment performed.
1 participant
Summary
Task 2 of the Fabric/Forge/NeoForge preparation, based on merged #318.
PlatformServices,PlatformPlayer, andPlatformSchedulercontracts for UUID-based online-session lookup, literal native permissions, rendered text, console/player commands, and distinct server-wide/player-owned scheduling.BukkitPlatformServicesusing the existing plugin-owned SimpleAPI Bukkit/Folia scheduler. Construction and owner resolution remain lazy; no new executor, listener, persistent user, player cache, or global service registry is created.ConsoleCommandDispatcherand wire the existing privateMiscUtils.runConsoleCommandthrough it. The public command-list path uses the new services immediately, preserving one-leading-slash removal, parsing/logging, iteration, native dispatch, and seconds-based stagger timing.Compatibility and scope
Existing public API signatures and permission-provider helpers are unchanged. Player handles represent one online login; queued work is not redirected to a replacement login or to the global scheduler when an entity disappears. Messages and commands are already-rendered text, with no additional placeholder/script evaluation or operator privilege escalation.
Scheduling means submitted, not completed. These methods do not provide durable reward acknowledgements, persistent retries, or execution checkpoints. The platform's existing scheduler owns cancellation; this PR adds no parallel lifecycle.
One Maven project, core/Bukkit packages. No POM, workflow, dependency, storage, FLAT, user-cache, reward-queue, or loader-packaging changes. SimpleAPI remains
1.0.2-SNAPSHOTand the unwanted dependency-pinning test stays removed. #314 remains closed.Coordination with #317: that PR also touches
MiscUtils.java. This PR changes only its legacy private command-list dispatcher plus imports/field initialization. Single-command overloads and #317's async completion/checkpoint methods are not refactored here. Preserve those additions and rerun both suites when combining branches.Verified GitHub Actions build
Java CI with Maven — run 592 passed. Inspected the completed build job
103622316990and its actual Maven log:mvn -B -f AdvancedCore/pom.xml packageon Temurin Java 21.0.12.BukkitPlatformServicesTest(8),MiscUtilsPlatformCommandsTest(1), andPlatformServicesHeadlessTest(2), including isolated execution with server APIs absent.MiscUtilsTest(3) and existing reward/configuration/user/lifecycle regression tests passed.1.0.2-20260912.210028-21. The POM remains1.0.2-SNAPSHOT; this timestamp records tested inputs, not a dependency pin.AdvancedCore/target/AdvancedCore.jargenerated; shading/minimization completed; BUILD SUCCESS.b81070a0e3639a0933ae2aa61f4220ca00e112bafor candidateac0353315c587bc8b062f437fae92eb4444f1e64and base977a3f1e07c37fb47d457338461d73d0681f8e64.These full Maven results are from GitHub Actions, not a local build. Shade emitted minimization/overlap warnings; successful packaging is not packaged-runtime/linkage or live-server validation.
Validation performed locally
MiscUtils.javaagainst upstream Git blobf0a8276fa23aa21636b1c540f86322bbad9b67a7before editing.javac --release 21; ranPlatformServicesFixture: passed. This tests the actual shared dispatcher with a fake platform, not the Bukkit adapter.git diff --cached --checkpassed in an exact-baseline partial repository fixture; inspected the entire intended diff and verified all 11 uploaded file blob hashes against the local copies.Still draft / remaining validation
Full local Maven validation is blocked in this editing environment: Maven is absent and direct repository/dependency host resolution is unavailable. Local SimpleAPI -> AdvancedCore -> VotingPlugin builds against the candidate artifacts, final shaded-JAR linkage checks, live Bukkit/Paper/Folia tests, and a fresh independent review have not been performed. Mocked scheduler routing and isolated core loading do not establish live Folia or native-mod compatibility.
Base:
977a3f1e07c37fb47d457338461d73d0681f8e64.Candidate:
ac0353315c587bc8b062f437fae92eb4444f1e64.No merge, release, deployment, or manual external review request is included.
AI assistance: ChatGPT assisted with implementation, test authoring, same-context inspection, and this description.