Skip to content

Fix compatibility with current Pumpkin nightlies#18

Merged
BjornTheProgrammer merged 2 commits into
Pumpkin-MC:masterfrom
Rotstein007:fix/current-pumpkin-nightly-compat
Apr 14, 2026
Merged

Fix compatibility with current Pumpkin nightlies#18
BjornTheProgrammer merged 2 commits into
Pumpkin-MC:masterfrom
Rotstein007:fix/current-pumpkin-nightly-compat

Conversation

@Rotstein007
Copy link
Copy Markdown

Note

Me (rotstein007) discovered, that PatchBukkit currently doesn't work. So I quickly made an fix with codex. I didn't read through the code, so please be careful with merging. Hopefully it's still helpful. The past section is also generated by codex:

Summary

This updates PatchBukkit so it works again with current Pumpkin nightlies.

What changed

  • synced PatchBukkit to current Pumpkin dependencies instead of building against an outdated locked commit
  • added build-time Pumpkin revision info so PatchBukkit logs which Pumpkin revision it was built against
  • replaced the fragile runtime dependency setup with a bundled fat patchbukkit.jar, so the JVM can start
    reliably without resolving Paper/Bukkit classes at runtime
  • switched protobuf generation to vendored protoc to make local builds and CI more reliable
  • fixed the current Pumpkin API break in command argument handling
  • fixed PatchBukkit directory handling so the runtime layout matches the documented server-root
    patchbukkit/ folder again
  • improved Bukkit command permission registration to use stable, plugin-specific permission nodes
  • updated CI and build scripts so nightly artifacts are rebuilt against current Pumpkin dependencies

Result

PatchBukkit starts successfully on current Pumpkin nightlies again.

I also verified this with a real Spigot plugin:

  • simple_spawn-1.3.jar loads successfully
  • the plugin is enabled
  • Bukkit commands are registered
  • shutdown works cleanly

Verification

Tested locally with:

  • current Pumpkin nightly (0.1.0-dev+26.1)
  • current PatchBukkit built from this branch
  • a real Spigot plugin (simple_spawn-1.3.jar)

Additional checks:

  • cargo +nightly-2026-03-05 test
  • cargo +nightly-2026-03-05 clippy --all-targets -- -D warnings
  • cargo +nightly-2026-03-05 build --release

Known limitation

Pumpkin still logs the native plugin as Loaded ().

PatchBukkit itself works in the tested scenario, but that remaining metadata issue appears to be on the
Pumpkin native plugin loader / ABI side rather than in PatchBukkit's startup path.

@BjornTheProgrammer
Copy link
Copy Markdown
Collaborator

This actually has a lot of changes that should be implemented. Maybe not everything should, but implementation instead of compileOnly is definitely a good idea. Also having protoc find the vendor is a good idea. And making the build script run cargo update is a smart idea. Some of the other changes do seem to be unneeded.

@Rotstein007
Copy link
Copy Markdown
Author

I gave codex the instruction to directly use and test patchbukkit with an pumpkin server, and test changes directly. Additionally I told him to downlod a test plugin from spigot to directly test changes. What would be things you don't like from the changes?

Comment thread rust/src/java/plugin/command_manager.rs Outdated
format!("{PATCHBUKKIT_PERMISSION_NAMESPACE}:command.{plugin_segment}.{command_segment}")
}

fn sanitize_permission_segment(value: &str) -> String {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we sanitizing permission segments? Is this something paper does?

Comment thread rust/src/java/plugin/command_manager.rs Outdated
"Allows running the Bukkit command `{cmd_name}` from `{}`",
plugin.name
),
PermissionDefault::Allow,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems unsafe to default to allow.

Comment thread rust/build/pumpkin.rs Outdated
})
}

pub fn setup_pumpkin_build_info(base: &Path) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this exists. And if it is for a good reason why not just use a toml parsing crate.

@Rotstein007
Copy link
Copy Markdown
Author

Any more corrections?

@BjornTheProgrammer
Copy link
Copy Markdown
Collaborator

Looks good to me. @yunuservices can you review and merge this?

@yunuservices
Copy link
Copy Markdown
Contributor

Lemme look rq

Copy link
Copy Markdown
Contributor

@yunuservices yunuservices left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. used protobuf dependencies a lot than expected. (compile-time only)
  2. Fat JAR usage. (acceptable)

Overall this PR looks good, approved.

@BjornTheProgrammer BjornTheProgrammer merged commit b285ab4 into Pumpkin-MC:master Apr 14, 2026
13 checks passed
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.

3 participants