Skip to content

Support Minecraft 26.1 through 26.2, and fix the Base64Coder crash they introduce - #178

Open
Aelshi-nui wants to merge 4 commits into
wargamer:masterfrom
Aelshi-nui:master
Open

Support Minecraft 26.1 through 26.2, and fix the Base64Coder crash they introduce#178
Aelshi-nui wants to merge 4 commits into
wargamer:masterfrom
Aelshi-nui:master

Conversation

@Aelshi-nui

Copy link
Copy Markdown
Contributor

Minecraft changed how it numbers releases in 2026. The drops after 1.21.11 are 26.1, 26.1.1, 26.1.2 and 26.2, with the leading "1." gone for good, so these are newer than 1.21.11 even though the numbers look smaller. This branch gets SignShop running on all four of them while still loading on 1.21 servers, and bumps the version to 5.2.3.

Two things genuinely broke on the new drops, and neither was visible at build time.

The first one crashes shops outright. Every 26.x Spigot ships SnakeYAML 2.6, and 2.6 deleted the Base64Coder class that 2.2 and earlier provided. BukkitSerialization imported that class straight from the server's copy of SnakeYAML, so the moment anything touched the legacy item paths, it died with ClassNotFoundException: Base64Coder. Since it only happens at runtime, and only when reading or converting older shop data, it looks like a data problem rather than a library one. It now encodes with java.util.Base64 instead. The output is deliberately identical to what SnakeYAML produced, right down to the 76 character lines and the trailing line break, so re-saving a shop leaves sellers.yml byte for byte the same as before, and the decoder accepts LF, CRLF and completely unwrapped input so nothing written by an older build is rejected.

The second one is quieter and worse for it. PlayerHeadIncompatibilityDetector decided whether it applied by matching on the version string, with checks like version.contains("1.21.1") and a couple of guesses at future versions. On a 26.x server none of those match, so the detector concluded it was irrelevant and switched itself off, which let the empty player head NPE back in on exactly the versions that still have the bug. Version handling now lives in versionUtil, which parses versions into numbers and exposes isAtLeast. Because 26 is greater than 1, both numbering schemes order correctly against each other without any special cases, and the detector simply asks whether the server is at least 1.21.10.

The build now targets spigot-api 26.2, and the source is verified to compile against 1.21, 1.21.11, 26.1, 26.1.1, 26.1.2 and 26.2, so nothing here depends on API that only exists in the newest drop. api-version in plugin.yml deliberately stays at 1.21, because that field is a minimum rather than a target: a server refuses a plugin that asks for something newer than itself, so leaving it alone is what lets one jar cover 1.21 through 26.2. There is a spigot.version property if you want to build against an older drop.

Dependencies moved up to releases that work on the new drops: WorldGuard 7.0.18, which itself declares api-version 26.1, WorldEdit 7.4.5, Towny 0.103.2.0, BentoBox 2.7.0, GriefPrevention 18.0.0, Lands 7.25.4, BlockLocker 1.13, Dynmap 3.8, Vault 1.7.1, bStats 3.2.1 and sqlite-jdbc 3.53.2.1. gson 2.14.0 and annotations 24.1.0 were picked to match exactly what spigot-api 26.2 hands the plugin at runtime. Towny is built against paper-api these days and its Resident and Town classes now extend Adventure audiences, so javac needs those interfaces to resolve the types; adventure-api is on the compile classpath for that reason alone and nothing calls into it, which keeps the plugin working on plain Spigot.

While updating dependencies it turned out the CMI, Essentials, Residence and BlockLocker integrations had been excluded from compilation because their jars were unavailable, which left the worth handlers and the Essentials sign conflict check disabled. They now point at real artifacts, CMI-API 9.8.6.4, EssentialsX 2.21.2, Residence 6.0.2.3 and BlockLocker 1.13, and the wiring is enabled again behind the usual plugin presence guards. LWCX is the one exception, since nothing publishes an artifact for it that Maven can resolve, so its hook stays out of the build with a comment explaining why.

The last two commits stop tracking target/ and dependency-reduced-pom.xml, which were committed build output. They do not affect behaviour, they just stop every local build from showing up as a diff.

On verification: mvn clean package succeeds, the compile matrix across the six versions above passes, the replacement Base64 encoder was checked against SnakeYAML 2.2 and produces identical output for eleven payload sizes from empty up to 64 KB, and the version comparison has twenty assertions covering the ordering across the scheme change. What has not happened yet is a run on a live 26.x server, so the runtime side rests on the API and library evidence rather than on play testing.

One thing worth knowing before reading the file list: most of the changed file count is file permission differences the fork carries, not content. The files with actual changes are the pom, .gitignore, README, versionUtil, BukkitSerialization, PlayerHeadIncompatibilityDetector, IncompatibilityDetector, SignShop, SignShopServerListener and the Folia scheduling classes.

- add FoliaDetector and SchedulerAdapter
- switch schedulers to Folia-safe Consumer<ScheduledTask> API
- fix region-based thread-safety issues
- exclude optional integrations via stub JARs
- tested on Folia 1.21.11 and Paper 1.21
Minecraft switched to year based version numbers after 1.21.11, so the
drops are 26.1 and up rather than 1.26.x. Build against spigot-api 26.2;
the source is verified to compile against 1.21, 1.21.11, 26.1, 26.1.1,
26.1.2 and 26.2. api-version stays at 1.21 because it is a minimum, so
one jar still loads on 1.21 servers.

Two things actually broke on 26.x:

- Every 26.x Spigot ships snakeyaml 2.6, which removed
  Base64Coder, so the legacy item paths died with
  ClassNotFoundException: Base64Coder. BukkitSerialization now uses
  java.util.Base64 with the same 76 character lines and trailing break,
  so saved shops stay byte for byte identical, and the decoder accepts
  LF, CRLF and unwrapped data.

- PlayerHeadIncompatibilityDetector decided whether it applied with
  version.contains("1.21.1") and friends, which match nothing on 26.x,
  so it quietly disabled itself and let the empty player head NPE back
  in. versionUtil now parses versions into numbers and offers
  isAtLeast(), which orders both schemes correctly since 26 > 1.

Dependency updates: worldguard 7.0.18, worldedit 7.4.5, towny 0.103.2.0,
bentobox 2.7.0, GriefPrevention 18.0.0, LandsAPI 7.25.4, blocklocker
1.13, DynmapCoreAPI 3.8, VaultAPI 1.7.1, bstats 3.2.1, sqlite-jdbc
3.53.2.1, gson 2.14.0 and annotations 24.1.0 (the last two match what
spigot-api 26.2 provides). Towny leaks Adventure types through Resident
and Town now, so adventure-api is on the compile classpath only.

The CMI, Essentials, Residence and BlockLocker integrations were
excluded from the build because their jars were unavailable. They now
point at real artifacts (CMI-API 9.8.6.4, EssentialsX 2.21.2, Residence
6.0.2.3, blocklocker 1.13) and the worth handler and Essentials sign
conflict wiring is enabled again. Only LWCX stays out, since nothing
publishes a resolvable artifact for it.

Version bumped to 5.2.3.
target/ was committed, so every build showed up as a diff and the
repository carried ~389k lines of generated files. It is ignored now,
along with FOLIA_COMPATIBILITY.md, which stays a local note. The README
covered the same Folia information, so the link to it is gone and the
short version lives in the README instead.
The shade plugin rewrites it on every package, so it only ever showed up
as noise in diffs. Same reasoning as target/.
@weaves7

weaves7 commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this — the version-detection rework (versionUtil.isAtLeast()) and the Base64Coder replacement are solid, both root-caused and correctly handled for backward compat. I checked all the bumped dependencies (Towny,
BentoBox, GriefPrevention, LandsAPI, Residence, EssentialsX, CMI-API, Dynmap, BlockLocker, adventure-api) and they all resolve and compile cleanly.

One thing I need clarified before anything else:

  1. Did you actually verify a clean build across the full version spread you're claiming? The README says this builds against spigot-api 26.2 and Java 21, and compiles for everything from 1.13.2 up through 26.2. But
    worldguard-bukkit:7.0.18 and worldedit-bukkit:7.4.5 don't compile for me against maven.compiler.source/target=21 as currently set — something in that dependency's own toolchain requirements is out ahead of the Java version
    this project targets. Did you build and test this end-to-end on a clean environment across that whole matrix, or mainly on whatever JDK/setup you had locally? If it's the latter, I'd like to see it actually verified
    across the stated range before merging — happy to help narrow down which combination breaks if useful.

Other things I'd like addressed before merging:

  1. File mode churn: ~326 unrelated files flipped 100644→100755 with zero content change (looks like a WSL/Docker-on-Windows mount artifact). Run git config core.fileMode false, normalize back to 644, force-push.
  2. SchedulerAdapter.initializeFoliaReflection(): unconditional logger.info() spam with emoji, fires once at startup on Folia servers specifically (properly gated behind FoliaDetector.isFolia(), non-Folia users unaffected).
    Still worth gating behind Debugging:/debugMessage() for consistency.
  3. true on CMI/EssentialsX/Residence/Dynmap/LWCX/BlockLocker in pom.xml: no-op for a shaded plugin jar (already provided scope), inconsistently applied. Drop it.
  4. Storage.java/itemUtil.clickedSign(): swallow validation failures under a guessed "might be Folia" comment instead of a confirmed cause. If you hit a real stack trace, note it; otherwise this risks masking a genuinely
    broken shop.
  5. This silently drops LWC support. LWCHook.java is excluded from compilation, but plugin.yml still lists LWC in softdepend and HookManager swallows the resulting ClassNotFoundException at runtime — no error, the protection
    check just stops working. LWCX 2.2.5 resolves fine here already; if you can't resolve it locally, install it into your local repo manually (mvn install:install-file -Dfile=LWCX-2.2.5.jar -DgroupId=com.griefcraft.lwc
    -DartifactId=LWCX -Dversion=2.2.5 -Dpackaging=jar) rather than excluding the hook.

Heads up — I've already got my own overlapping fixes queued (Base64Coder in particular, and a few of the others touch the same files), so I'll be PRing separately and merging whichever lands cleaner/first rather than blocking
on full overlap resolution here.

@Aelshi-nui

Copy link
Copy Markdown
Contributor Author

I went through everything and checked each one, and you're right on all of them.

The main thing I missed was the build. My matrix runs were on JDK 25, so I didn't catch that WorldGuard 7.0.18 and WorldEdit 7.4.3+ require Java 25. Pinning WorldGuard 7.0.17 and WorldEdit 7.4.2 works cleanly on both JDK 21 and 25.

The README's 1.13.2 support was also wrong since the plugin uses "api-version: 1.21".

Everything else checks out too, including the file modes, Folia logging, optional flags, and sign validation.

One note: LWCX 2.2.5 doesn't seem to exist in any public repo I checked, so it may only be resolving from your local ".m2". That should either use a real repo or fail loudly instead of silently throwing "ClassNotFoundException".

I won't have time to work through these this week, so go ahead and merge yours. It'll be cleaner. I'm happy to close this one or rebase anything left afterward.

@weaves7

weaves7 commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

I went through the delta between what's in master (7c00717) and this branch, and it turned up something that changes my thinking on the Folia side.

The January version couldn't have loaded on Folia. The reflection looked up:

regionSchedulerClass.getMethod("run", Plugin.class, Location.class, Runnable.class)

but Folia's RegionScheduler.run() takes Consumer<ScheduledTask>. So that's a NoSuchMethodException, caught by the outer block and rethrown as RuntimeException("Failed to initialize Folia reflection") — every scheduler lookup in that version had the same wrong signature. On a real Folia server the plugin would have failed to enable outright. Which also means the seven months it's been in master tell us nothing about Folia; isFolia is false on Paper and Spigot, so none of that code has ever run.

Your Consumer fix here is correct and would be the first version that actually functions. But two changes in it aren't Folia-gated and regress Paper and Spigot:

itemUtil.clickedSign() catches NPE and returns false with no isFolia check, and Storage now comments out the SIGN_LOCATION_NOT_ACTUALLY_SIGN throw. Together that disables shop sign validation on every platform — a shop whose sign was destroyed or replaced now loads with a warning instead of being rejected. The comment says it'll be validated on first use. There's no such mechanism on master — getSeller() is a bare map lookup with no re-check, and nothing re-validates a loaded shop anywhere. A shop that gets past addSeller() is trusted permanently, so an invalid sign that slips through at startup stays trusted until something downstream tries to treat that block as a Sign and throws.

I also think the diagnosis behind it is off, though this is the part where I'm on shakier ground — I've been leaning on Claude for the Folia specifics since I don't work with it directly. My understanding is that Folia's thread ownership checks throw IllegalStateException rather than NPE, so an NPE at bBlock.getType() more likely means bBlock itself is null — the world not being loaded yet when Storage runs at startup. That would be an ordering problem rather than a threading one, and it'd reproduce on Paper under the same conditions. Correct me if that's wrong. Either way the catch is load-bearing in a way it probably wasn't meant to be — it's the only thing standing between a bad sign and the seller map. If you have the original stack trace I'd genuinely like to see it, because if that reading holds it's a real startup bug worth fixing properly on all platforms.

Where I've landed: I'm going to pull the Folia support back out rather than take this forward. It's never worked, I have no way to test it, and I'm not in a position to audit the plugin's shared state — Storage's seller map, the static priceCache, the misc-items cache — for region threading on a platform I can't run and don't have working knowledge of. For a shop plugin the failure mode there is dupes, and I'd rather ship no Folia support than unverifiable Folia support. That's about my capacity, not the quality of the work here.

What I do want is the 26.x fix. versionUtil and PlayerHeadIncompatibilityDetector are untouched in the Folia commit, so that part is cleanly separable, and the head NPE is live right now on servers that exist. If you're up for a narrow PR off current master with just those two — no dependency bumps, no Folia — I'll review and merge it. No rush on it though; I don't have much time this week either, so whenever it suits you. If you'd rather not, tell me and I'll cherry-pick them with attribution to you.

Either way, thanks for chasing the 26.x versioning down. The isAtLeast approach is the right call and I'd have written something worse.

@Aelshi-nui

Copy link
Copy Markdown
Contributor Author

That one's on me and I'm sorry. I never pushed the fix. The commit that corrects the scheduler signatures sat on my local branch from 27 January until last week, so master has had the broken version this whole time. You read the code that was actually there, and you read it correctly. When I said Folia was working I was talking about a jar I'd built locally the day after, and I hadn't clocked that it wasn't the same thing as what I'd published. Sorry for the runaround.

For the record, here's what the difference actually is. I dug out that old build and it's from the fixed commit, Consumer in all four lookups, and I checked both versions against Folia's real API jar (dev.folia:folia-api:26.2.build.6-beta). Every Runnable lookup from the version in master throws NoSuchMethodException; every Consumer lookup from my local build resolves and returns ScheduledTask. Runnable only exists on execute, which is what I got wrong in the first place. So the fix is real, it just never reached you.

None of that excuses the other thing you found. clickedSign swallowing the NPE and Storage dropping the SIGN_LOCATION_NOT_ACTUALLY_SIGN throw aren't Folia-gated. They do change Paper and Spigot behaviour, and there's no re-validation anywhere to back up what that comment promises. You were right, and that should come out regardless of what happens with Folia.

So it's your call. If you want Folia support, I'm happy to do it properly. Gate those two changes behind isFolia or drop them outright, and go through the shared state, the seller map and the static price cache and the misc-items cache, for region threading. I'll be straight that what I've verified is the reflection resolving, not that shop operations are safe across regions, so that audit is the actual work and I'd be doing it on a platform you can't review. If you'd rather not carry that, say the word and I'll just do the narrow one you asked for: versionUtil and the one method in the detector, off current master, nothing else. Worth doing either way, since isRelevantForCurrentVersion() still gates on contains("1.21.1") and IncompatibilityChecker skips detect() when that's false. So on 26.x servers your own head-detector fix from #179 isn't running yet. Either way I'll close #178.

@weaves7

weaves7 commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

No apology needed — that explains it, and the local-build thing is an easy one to miss. Thanks for digging out the old jar and checking both versions against the real API; that settles it.

Taking the narrow one. Folia stays out for the reasons I gave, and your own framing is why — the audit is the actual work, and it's work I can't review. That's not a judgement on whether you'd do it well.

The isRelevantForCurrentVersion() catch is a good one and I'd missed it. If the #179 fix isn't running on 26.x then that's live in 5.3.0 right now, which makes this more urgent than I'd been treating it. A fresh PR off current master with just versionUtil and the detector method would be ideal — new branch rather than your fork's master, so we skip the file mode churn and the dependency changes. Whenever you get to it, and thanks for spotting that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants