Conversation
There was a problem hiding this comment.
Pull request overview
This PR bumps CyberLevels to 1.3.0 and rewrites /clv about to surface runtime state (server version, authors, database connection/enabled/type, auto-save, leaderboard, and the EXP engine). It also moves the CyberCore dependency from a bundled jar to a composite build resolved from source via settings.gradle.kts, exposes library() as TextLibrary so the lang prefix API is reachable, and updates the Gradle wrapper to 9.6.1. CI workflows now check out CyberCore alongside the plugin so the composite build resolves in CI as well as locally.
Changes:
- build.gradle.kts: version bumped to 1.3.0; CyberCore dependency switched from
files("libraries/CyberCore-2.0.0.jar")tocom.bitaspire:CyberCore:2.1.0(resolved via composite build). - settings.gradle.kts: new
includeBuildblock that locates a siblingCyberCorecheckout (or aCyberCore/subfolder in CI) and substitutes it for the published coordinate. - .github/workflows/build.yml and release.yml: added a
Checkout CyberCore dependencystep that clonesBitAspire/CyberCoreintoCyberCore/so the composite build resolves. - gradle/wrapper/gradle-wrapper.properties: bumped to Gradle 9.6.1.
- CyberLevels.java:
library()return type changed fromTakionLibtoTextLibrary(import added). - CLVCommand.java:
/clv aboutnow delegates to a newsendAbouthelper that reports server, authors, database state, config flags, and engine; console path usesmain.logger(lines). - CLVCommand.java:
ChatColor.translateAlternateColorCodesreplaced withPrismaticAPI.colorizeinsendLangMessage(two sites). - CLVCommand.java:
resolveUserByNamenow usesmain.getServer()instead of the staticBukkitaccessor. - libraries/CyberCore-2.0.0.jar: deleted (no longer bundled).
Reviewed changes
- High
settings.gradle.kts- Composite build silently falls back to the published coordinate when no checkout is present
firstOrNull { it.resolve("settings.gradle.kts").isFile }means that if neither../CyberCorenor./CyberCoreexists,includeBuildis never called and Gradle resolvescom.bitaspire:CyberCore:2.1.0frommavenLocal()/Maven Central. CyberCore 2.1.0 is not published there, so a fresh clone without the sibling checkout will fail with an obscure dependency-resolution error instead of a clear message. Either fail fast when no candidate is found, or document that a sibling checkout is mandatory. - High
.github/workflows/release.yml- Release workflow checks out CyberCore from the default branch, not the version being released
The new step usesactions/checkout@v4with noref, so it pullsBitAspire/CyberCore@main. If a release is cut from a CyberLevels commit that targets a CyberCore tag/branch other thanmain, the released artifact will be built against an unrelated CyberCore HEAD. Pin the checkout to the CyberCore ref that matches the2.1.0coordinate (e.g. a tag or aref:input). - Medium
src/main/java/com/bitaspire/cyberlevels/command/CLVCommand.java-sendAboutreadsuserManager()without a null guard on the manager itself
main.userManager() == null ? null : main.userManager().getDatabase()is fine, but the surrounding code callsmain.cache().config().database().isEnabled()unconditionally. Ifcache()orconfig()can be null before the plugin is fully enabled (e.g./clv aboutduring reload), this NPEs. Consider guarding or asserting the plugin is enabled before reporting config-derived fields. - Medium
src/main/java/com/bitaspire/cyberlevels/command/CLVCommand.java-databaseConnectedis computed but never used to gate the message
The boolean is only printed as part of theDatabase:line. When the database is configured but disconnected, the message still claims the plugin is healthy. Consider surfacing a warning prefix (e.g.&c) or a separate&cDisconnectedline so operators can spot the issue at a glance. - Medium
src/main/java/com/bitaspire/cyberlevels/command/CLVCommand.java-serverInfo()swallows the real cause whenVNC.SERVERis null
Returning the literal string"Unknown"hides why the version is unknown (VNC not initialized, shaded away, etc.). At minimum log a warning the first time it happens, or include the exception cause if one is available. - Medium
src/main/java/com/bitaspire/cyberlevels/CyberLevels.java-library()return type change is a breaking API change for downstream consumers
Any addon or integration that calledCyberLevels#getLibrary()and assigned it toTakionLibwill fail to compile against 1.3.0. IfTakionLibis still part of the public API, consider keeping the method signature and adding a new accessor (e.g.textLibrary()), or document the breaking change in the changelog. - Low
src/main/java/com/bitaspire/cyberlevels/command/CLVCommand.java- Console path drops color codes silently
main.logger(lines)is called with raw&-prefixed strings. If the logger does not colorize, the console will show literal&d&lCyber&f&lLevels. Verify thatmain.logger(String[])runs the same colorization pipeline ascreateSender(...).send(...), or strip/translate the codes before logging. - Low
.github/workflows/build.yml- Workflow comment duplicates the same instruction in two files
The "Keep the path in sync with the candidates listed there" comment is repeated verbatim inbuild.ymlandrelease.yml. If the path ever changes, both comments must be updated. Consider referencing a single source of truth (e.g. aREADMEsection) or extracting the checkout into a composite action / reusable workflow. - Low
gradle/wrapper/gradle-wrapper.properties- Gradle 9.6.1 wrapper bump is not mentioned in the PR description
The summary lists the wrapper bump, but reviewers should confirm 9.6.1 is intentional (vs. 9.6.0) and that no plugin in the build (Spigot, Shadow, Lombok) has known incompatibilities with 9.6.x.
Reviewed 8 changed files in this pull request and generated 3 inline comments.
Generated by pull_request.opened.
| // plugin can be developed against unpublished changes. Locally the checkouts are sibling folders; | ||
| // CI checks them out inside this repository instead. They are not on any maven repository, so the | ||
| // checkout is required rather than a fallback. | ||
| listOf("CyberCore").forEach { dependency -> |
There was a problem hiding this comment.
Fail fast when no CyberCore checkout is found so a fresh clone doesn't fall through to Maven Central and produce a confusing resolution error.
| listOf("CyberCore").forEach { dependency -> | |
| listOf("CyberCore").forEach { dependency -> | |
| val included = listOf("../$dependency", dependency) | |
| .map(::file) | |
| .firstOrNull { it.resolve("settings.gradle.kts").isFile } | |
| checkNotNull(included) { | |
| "Cannot locate a local checkout of '$dependency'. Expected a sibling folder or './$dependency' containing settings.gradle.kts." | |
| } | |
| includeBuild(included) | |
| } |
|
|
||
| # settings.gradle.kts picks this checkout up as an included build. Keep the path | ||
| # in sync with the candidates listed there, otherwise the build cannot resolve it. | ||
| - name: Checkout CyberCore dependency |
There was a problem hiding this comment.
Pin the CyberCore checkout to the ref that matches the 2.1.0 coordinate so releases aren't built against an unrelated HEAD.
| - name: Checkout CyberCore dependency | |
| - name: Checkout CyberCore dependency | |
| uses: actions/checkout@v4 | |
| with: | |
| repository: BitAspire/CyberCore | |
| ref: 2.1.0 | |
| path: CyberCore |
| " &d&lCyber&f&lLevels &fv" + main.getDescription().getVersion() + "&7.", | ||
| " &7Server: &f" + serverInfo(), | ||
| " &7Authors: &f" + main.getAuthors(), | ||
| " &7Database: &f" + databaseConnected + " &7(enabled: &f" + databaseEnabled + |
There was a problem hiding this comment.
Make the disconnected state visually obvious so operators can spot a broken DB at a glance.
| " &7Database: &f" + databaseConnected + " &7(enabled: &f" + databaseEnabled + | |
| " &7Database: &f" + (databaseConnected ? "&aconnected" : "&cdisconnected") + | |
| " &7(enabled: &f" + databaseEnabled + | |
| "&7, type: &f" + main.cache().config().database().getType() + "&7)", |
Summary
/clv aboutto report server, authors, database state and configlibrary()asTextLibraryso the lang prefix API is reachableContributors
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.