JavaPluginLoader casts directly to SimplePluginManager implementation without checks

original-codematrix commented 2 years ago

Expected behavior

I would expect when I have my own written PluginManager that I can set it for Bukkit and I don't get any ClassCastException.

It would be nice to fix the current direct casts to the SimplePluginManager and maybe thinking about putting these method definitions also in the PluginManager-interface.

Observed/Actual behavior

At the moment in the JavaPluginLoader class, we have multiple casts from PluginManager to SimplePluginManager due to the SimplePluginManager class contains some methods that the PluginManager does not provide.

E. g. JavaPluginManager#97: final File parentFile = ((SimplePluginManager) this.server.getPluginManager()).pluginsDirectory(); // Paper There is the pluginsDirectory() method that isn't provided by the PluginManager interface.

This problem causes a problem for the test framework called MockBukkit to switch to paper due to it uses its own implementation of the PluginManager class.

Steps/models to reproduce

-

Plugin list

-

Paper version

Latest 1.17.1 (maybe some versions earlier also, I've didn't tested older versions)

Agreements

[X] I am running the latest version of Paper available from https://papermc.io/downloads.
[X] I have searched for and ensured there isn't already an open issue regarding this.
[X] My version of Minecraft is supported by Paper.

Other

The line of code I've mentioned comes from this patch: 0306-Add-command-line-option-to-load-extra-plugin-jars-no.patch

electronicboy commented 2 years ago

This assumption is made as spigot enforces this in various places elsewhere, this is defo something I'd like to see fixed, especially as I did start tryna make my own fork of MockBukkit for Paper

This is something I'd be interested in fixing but there are concerns around exposing internal methods as API here, this would probs also likely need to be addressed in spigot/CB classes too for a full proper fix here

Chew commented 2 years ago

Please provide the full /paper version output in the future :)

For reference: latest as of this issue opening: Paper 1.17.1, build 161(?)

original-codematrix commented 2 years ago

@Chew Provided the version that has been used in the issue description. Here also: I'm using the 1.17.1 paper-api dependency atm.

Maybe it helps if you check out the project here on GitHub to understand the problem here: https://github.com/mockBukkit/MockBukkit/

electronicboy commented 2 years ago

I forget if GH auto-emails for PRs linked to an issue, but: https://github.com/PaperMC/Paper/pull/6619

PaperMC / Paper