search

PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
10.02k stars 2.32k forks source link

Paper 1.19.3 casts plugins to JavaPlugin on enable, even though other plugins should be allowed as well #8935

Open raikasdev opened 1 year ago

raikasdev commented 1 year ago

Expected behavior

Other plugins (based on PluginBase for example) would also be loaded

Observed/Actual behavior

Paper casts all plugins to JavaPlugin on the PaperPluginInstanceManager class' enablePlugin function, resulting other types of plugins implemented on PluginBase not be able to load anymore.

Steps/models to reproduce

Download latest PaperMC 1.19.3 version. Download CraftJS https://github.com/raikasdev/craftjs/releases/tag/1.19.3-broken Start server

Plugin and Datapack List

CraftJS

Paper version

This server is running Paper version git-Paper-431 (MC: 1.19.3) (Implementing API version 1.19.3-R0.1-SNAPSHOT) (Git: e574412)

Other

No response

MiniDigger commented 1 year ago

please provide a stacktrace

raikasdev commented 1 year ago 

java.lang.ClassCastException: class fi.valtakausi.craftjs.plugin.JsPlugin cannot be cast to class org.bukkit.plugin.java.JavaPlugin (fi.valtakausi.craftjs.plugin.JsPlugin is in unnamed module of loader 'craftjs.jar' @38eeaaa1; org.bukkit.plugin.java.JavaPlugin is in unnamed module of loader java.net.URLClassLoader @e2144e4)
    at io.papermc.paper.plugin.manager.PaperPluginInstanceManager.enablePlugin(PaperPluginInstanceManager.java:183) ~[paper-1.19.3.jar:git-Paper-431]
    at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.enablePlugin(PaperPluginManagerImpl.java:104) ~[paper-1.19.3.jar:git-Paper-431]
    at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:507) ~[paper-api-1.19.3-R0.1-SNAPSHOT.jar:?]
    at fi.valtakausi.craftjs.plugin.JsPluginManager.enablePlugin(JsPluginManager.java:115) ~[craftjs.jar:?]
    at fi.valtakausi.craftjs.plugin.JsPluginManager.loadInternalPlugin(JsPluginManager.java:71) ~[craftjs.jar:?]
    at fi.valtakausi.craftjs.CraftJsMain.onEnable(CraftJsMain.java:78) ~[craftjs.jar:?]
    at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:279) ~[paper-api-1.19.3-R0.1-SNAPSHOT.jar:?]
    at io.papermc.paper.plugin.manager.PaperPluginInstanceManager.enablePlugin(PaperPluginInstanceManager.java:192) ~[paper-1.19.3.jar:git-Paper-431]
    at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.enablePlugin(PaperPluginManagerImpl.java:104) ~[paper-1.19.3.jar:git-Paper-431]
    at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:507) ~[paper-api-1.19.3-R0.1-SNAPSHOT.jar:?]
    at org.bukkit.craftbukkit.v1_19_R2.CraftServer.enablePlugin(CraftServer.java:560) ~[paper-1.19.3.jar:git-Paper-431]
    at org.bukkit.craftbukkit.v1_19_R2.CraftServer.enablePlugins(CraftServer.java:471) ~[paper-1.19.3.jar:git-Paper-431]
    at net.minecraft.server.MinecraftServer.loadWorld0(MinecraftServer.java:635) ~[paper-1.19.3.jar:git-Paper-431]
    at net.minecraft.server.MinecraftServer.loadLevel(MinecraftServer.java:434) ~[paper-1.19.3.jar:git-Paper-431]
    at net.minecraft.server.dedicated.DedicatedServer.initServer(DedicatedServer.java:308) ~[paper-1.19.3.jar:git-Paper-431]
    at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1101) ~[paper-1.19.3.jar:git-Paper-431]
    at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:316) ~[paper-1.19.3.jar:git-Paper-431]
    at java.lang.Thread.run(Thread.java:833) ~[?:?]```
raikasdev commented 1 year ago

This happens because of this patch: https://github.com/PaperMC/Paper/blob/e57441254dc93fa70782c10f9a22c87dc98ca0b6/patches/server/0013-Paper-Plugins.patch#LL3619C5-L3619C6

Is it really necessary to cast the Plugin class to JavaPlugin?

raikasdev commented 1 year ago

I have converted my plugin to use it's own JavaPlugin instance to register listeners instead of creating a own plugin instance for every JS plugin (my plugin is similar to Skript).

This of course has some issues for me (for example event listeners aren't automatically unregistered), but I have somewhat fixed them.

If you guys find it not necessary to support plugins based on PluginBase class, feel free to close this issue.

Owen1212055 commented 1 year ago

It's a little tricky here, as in general most places expect plugin instances to be of JavaPlugin anyways. With the plugin loading system being discontinued, this isn't really supported anyways now, so I am not sure.

electronicboy commented 1 year ago

PluginBase is generally what we should prefer, but, that has kinda been neglected as nobody ever really used anything else much for years, and so for the most part, nobody ever really had issues with that not being the case

Phoenix616 commented 1 year ago

While I personally don't use this feature right now I feel like this shouldn't be completely removed. Being able to potentially use different languages than just Java has been a good argument for the "Bukkit" plugin system in the past and I don't see the continued support of it to be that difficult to do.

What were the reasons for explicitly requiring JavaPlugin there?

Jannyboy11 commented 1 year ago

Is there any reason to explicitly require PluginBase? Why not just allow arbitrary org.bukkit.plugin.Plugin instances?

electronicboy commented 1 year ago

allowing arbitrary Plugin instances was never a contract of the bukkit API, PluginBase was always the promise on that front

The JavaPlugin cast this issue is referencing looked like it could be removed, but, theres many weird contractual stuff which was pulled from the older system into this new one which should probably be revisited