Jannyboy11 / ScalaPluginLoader

PluginLoader for Bukkit that provides Scala runtime classes and enhanced APIs.
https://www.spigotmc.org/resources/scalaloader.59568/
GNU Lesser General Public License v3.0
26 stars 10 forks source link

ScalaLoader stops to load plugin with Paper 405+ #20

Closed EpiCanard closed 1 year ago

EpiCanard commented 1 year ago

Hello,

It seems that Paper 405 including this pull request https://github.com/PaperMC/Paper/pull/8108 totally break the loading process of ScalaLoader.

They added some sort of PaperPluginManager to load plugin that support exclusively paper first and then spigot plugin. But with this system you can't have dynamic description like ScalaPluginDescription (if they are not in plugin.yml you have another error) and worst they cast the plugin into JavaPlugin. Obviously it doesn't work.

Logs errors on plugin loading :

[18:06:56 ERROR]: [io.papermc.paper.plugin.entrypoint.strategy.ModernPluginLoadingStrategy] Could not load plugin 'MapSaver-1.0.2+1-07081234+20230305-1729.jar' in folder 'plugins/ScalaLoader/scalaplugins'
org.bukkit.plugin.InvalidPluginException: main class `fr.epicanard.mapsaver.MapSaverPlugin' does not extend JavaPlugin
        at org.bukkit.plugin.java.PluginClassLoader.<init>(PluginClassLoader.java:86) ~[paper-api-1.19.3-R0.1-SNAPSHOT.jar:?]
        at io.papermc.paper.plugin.provider.type.spigot.SpigotPluginProvider.createInstance(SpigotPluginProvider.java:116) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.provider.type.spigot.SpigotPluginProvider.createInstance(SpigotPluginProvider.java:30) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.entrypoint.strategy.ModernPluginLoadingStrategy.loadProviders(ModernPluginLoadingStrategy.java:122) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.storage.SimpleProviderStorage.enter(SimpleProviderStorage.java:30) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.manager.SingularRuntimePluginProviderStorage.enter(SingularRuntimePluginProviderStorage.java:63) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.manager.RuntimePluginEntrypointHandler.enter(RuntimePluginEntrypointHandler.java:40) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.manager.PaperPluginInstanceManager.loadPlugin(PaperPluginInstanceManager.java:121) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.loadPlugin(PaperPluginManagerImpl.java:82) ~[paper-1.19.3.jar:git-Paper-405]
        at org.bukkit.plugin.SimplePluginManager.loadPlugin(SimplePluginManager.java:406) ~[paper-api-1.19.3-R0.1-SNAPSHOT.jar:?]
        at xyz.janboerman.scalaloader.ScalaLoader.onLoad(ScalaLoader.java:130) ~[ScalaLoader-0.17.17-SNAPSHOT.jar:?]
        at io.papermc.paper.plugin.storage.ServerPluginProviderStorage$1.load(ServerPluginProviderStorage.java:41) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.storage.ServerPluginProviderStorage$1.load(ServerPluginProviderStorage.java:23) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.entrypoint.strategy.ModernPluginLoadingStrategy.loadProviders(ModernPluginLoadingStrategy.java:123) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.storage.SimpleProviderStorage.enter(SimpleProviderStorage.java:30) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.entrypoint.LaunchEntryPointHandler.enter(LaunchEntryPointHandler.java:37) ~[paper-1.19.3.jar:git-Paper-405]
        at org.bukkit.craftbukkit.v1_19_R2.CraftServer.loadPlugins(CraftServer.java:429) ~[paper-1.19.3.jar:git-Paper-405]
        at net.minecraft.server.dedicated.DedicatedServer.initServer(DedicatedServer.java:273) ~[paper-1.19.3.jar:git-Paper-405]
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1101) ~[paper-1.19.3.jar:git-Paper-405]
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:316) ~[paper-1.19.3.jar:git-Paper-405]
        at java.lang.Thread.run(Thread.java:833) ~[?:?]
Caused by: java.lang.ClassCastException: class fr.epicanard.mapsaver.MapSaverPlugin
        at java.lang.Class.asSubclass(Class.java:3924) ~[?:?]
        at org.bukkit.plugin.java.PluginClassLoader.<init>(PluginClassLoader.java:84) ~[paper-api-1.19.3-R0.1-SNAPSHOT.jar:?]
        ... 20 more
[18:06:56 ERROR]: [ScalaLoader] Could not load plugin from file: /home/nicolas/Data/Projects/Servers/paper-1.19/plugins/ScalaLoader/scalaplugins/MapSaver-1.0.2+1-07081234+20230305-1729.jar
org.bukkit.plugin.InvalidPluginException: Plugin didn't load any plugin providers?
        at io.papermc.paper.plugin.manager.PaperPluginInstanceManager.lambda$loadPlugin$1(PaperPluginInstanceManager.java:127) ~[paper-1.19.3.jar:git-Paper-405]
        at java.util.Optional.orElseThrow(Optional.java:403) ~[?:?]
        at io.papermc.paper.plugin.manager.PaperPluginInstanceManager.loadPlugin(PaperPluginInstanceManager.java:127) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.loadPlugin(PaperPluginManagerImpl.java:82) ~[paper-1.19.3.jar:git-Paper-405]
        at org.bukkit.plugin.SimplePluginManager.loadPlugin(SimplePluginManager.java:406) ~[paper-api-1.19.3-R0.1-SNAPSHOT.jar:?]
        at xyz.janboerman.scalaloader.ScalaLoader.onLoad(ScalaLoader.java:130) ~[ScalaLoader-0.17.17-SNAPSHOT.jar:?]
        at io.papermc.paper.plugin.storage.ServerPluginProviderStorage$1.load(ServerPluginProviderStorage.java:41) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.storage.ServerPluginProviderStorage$1.load(ServerPluginProviderStorage.java:23) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.entrypoint.strategy.ModernPluginLoadingStrategy.loadProviders(ModernPluginLoadingStrategy.java:123) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.storage.SimpleProviderStorage.enter(SimpleProviderStorage.java:30) ~[paper-1.19.3.jar:git-Paper-405]
        at io.papermc.paper.plugin.entrypoint.LaunchEntryPointHandler.enter(LaunchEntryPointHandler.java:37) ~[paper-1.19.3.jar:git-Paper-405]
        at org.bukkit.craftbukkit.v1_19_R2.CraftServer.loadPlugins(CraftServer.java:429) ~[paper-1.19.3.jar:git-Paper-405]
        at net.minecraft.server.dedicated.DedicatedServer.initServer(DedicatedServer.java:273) ~[paper-1.19.3.jar:git-Paper-405]
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1101) ~[paper-1.19.3.jar:git-Paper-405]
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:316) ~[paper-1.19.3.jar:git-Paper-405]
        at java.lang.Thread.run(Thread.java:833) ~[?:?]

Minecraft : 1.19.3 Paper : 405 (or later here) ScalaLoader : 0.17.17

I didn't take the time to analyze what they did with this PR but I think it is a good idea to still report this issue to you.

Best Regards

Jannyboy11 commented 1 year ago

Hello, thanks for the report. I saw Paper's update regarding Paper plugins recently, but I didn't think it would cause trouble for ScalaLoader. It kind of looks like a bug on Paper's end to be honest, because it expects that all plugins are either a JavaPlugin or they have a registered PluginProvider. ScalaLoader uses bukkit's own (now legacy, I suppose) PluginLoader api, so ScalaPlugin's don't have a PluginProvider. I guess ScalaLoader will need to load as a PaperPlugin so it can provide a PluginProvider to ScalaPlugins. Anyway, I will have a look what I can do about this this afternoon. Worst case scenario I have to implement my own paper plugin loader and/or bootstrapper. That shouldn't be too difficult, but will take a bit of time. Perhaps the latter approach will even provide some benefits.

Jannyboy11 commented 1 year ago

Browsing through the Paper sources, the paper team have decided to deprecate Bukkit's PluginLoader api for removal. I will have to adapt to Paper's new system. I will keep you updated.

Jannyboy11 commented 1 year ago

Current plan is to implement a PluginLoader and Bootstrapper as described here: https://docs.papermc.io/paper/dev/getting-started/paper-plugin. When ScalaLoader is running on Paper, ScalaPlugins will be loaded as Paper Plugins. No changes should be needed for ScalaPlugin implementors.

EpiCanard commented 1 year ago

So you will have two loading system ? One for Paper 405+ et one for spigot and Paper 404- ?

You say "ScalaPlugins will be loaded as Paper Plugins", will ScalaPlugins still be able to use plugins.yml as config file or should they add also a paper-plugins.yml ?

Jannyboy11 commented 1 year ago

So you will have two loading system ? One for Paper 405+ et one for spigot and Paper 404- ?

Indeed.

You say "ScalaPlugins will be loaded as Paper Plugins", will ScalaPlugins still be able to use plugins.yml as config file or should they add also a paper-plugins.yml ?

What I meant to say was, that ScalaPlugins will make use of Paper's new infrastructure to load themselves, as well as dependencies and such to load the scala standard library and libraries from the libraries section. Because I value backwards compatibility all three plugin description types will be supported: plugin.yml, paper-plugin.yml as well as no plugin.yml.

Jannyboy11 commented 1 year ago

Update: In order to continue with this approach I will have to depend on Paper server internals. There is no easy maven dependency for this, and the only way that Paper supports this is through the paperweight-userdev gradle plugin. So I will have to move the build over to Gradle. While I have used gradle in the past, I'm definitely less comfortable with it then using Maven. I created a separate branch to track progress: https://github.com/Jannyboy11/ScalaPluginLoader/tree/gradle. I very much dislike this move by the PaperMC team, it feels as if they are strongarming me and other pluginloader developers into using a build tool of their liking.

EpiCanard commented 1 year ago

What I meant to say was, that ScalaPlugins will make use of Paper's new infrastructure to load themselves, as well as dependencies and such to load the scala standard library and libraries from the libraries section. Because I value backwards compatibility all three plugin description types will be supported: plugin.yml, paper-plugin.yml as well as no plugin.yml.

Good to know, thank you for this information.

Update: In order to continue with this approach I will have to depend on Paper server internals. There is no easy maven dependency for this, and the only way that Paper supports this is through the paperweight-userdev gradle plugin. So I will have to move the build over to Gradle. While I have used gradle in the past, I'm definitely less comfortable with it then using Maven. I created a separate branch to track progress: https://github.com/Jannyboy11/ScalaPluginLoader/tree/gradle. I very much dislike this move by the PaperMC team, it feels as if they are strongarming me and other pluginloader developers into using a build tool of their liking.

It seems very painful to migrate to this system. It would have been easier if they had left the old api in place. It's clearly a breaking change for pluginloader developpers

Jannyboy11 commented 1 year ago

Alright I found a way to do it with Maven: https://github.com/Jannyboy11/ScalaPluginLoader/commit/7282e3dff193d6c616e172769b75439eed27691f I did have to run gradlew applyPatches, gradlew build and gradlew publishToMavenLocal in a local clone of the paper server repository in order for this to work.

EpiCanard commented 1 year ago

Nice ! It will prevent you some useless refactor

Jannyboy11 commented 1 year ago

I plan to finish this next week - I have a lot of free time since I've graduated now.

Jannyboy11 commented 1 year ago

The implementation of ScalaLoader on Paper is now more or less complete (as of https://github.com/Jannyboy11/ScalaPluginLoader/commit/d9bacb6b9922165f3e1110b94533ada362551e1b). There are still a few small features remaining and I have some testing to do, but I'm hopeful that I can draft a new release next saturday.

EpiCanard commented 1 year ago

It's a good news ! Thank you for your work !

Jannyboy11 commented 1 year ago

Success: latest.log!

I still have to test whether permissions and event listeners work as expected. I also have to test what happens when ScalaPlugins depend on JavaPlugins or the other way around. Saturday I will do some clean up, fix the dependency to Paper 1.19.4 and create a new release.

In the mean time you can use this jar for testing: ScalaLoader-0.18.0-SNAPSHOT.zip (need to rename the file back from .zip to .jar because GitHub won't let me upload java archives..)

EpiCanard commented 1 year ago

Thanks !

I will test this evening or tomorrow evening depending on my free time.

(Sad that GitHub doesn't let you upload jar file. Maybe for security reason ?)

Jannyboy11 commented 1 year ago

I've identified two more issues so far: the ScalaPlugins' load status (STARTUP, POSTWORLD) is still ignored at this point, and the data directory is not in the location where it should be.

Here is a new jar, where the data directory bug is fixed: ScalaLoader-0.18.0-SNAPSHOT.zip

Jannyboy11 commented 1 year ago

How did your test go?

EpiCanard commented 1 year ago

I tried and it doesn't seem to work. It search for NoArgsConstructor but it doesn't find one.

[23:15:40 ERROR]: [ScalaLoader] Could not find main class in: MapSaver-1.0.2.jar
xyz.janboerman.scalaloader.plugin.ScalaPluginLoaderException: Could not find NoArgsConstructor in class fr.epicanard.mapsaver.MapSaverPlugin
    at xyz.janboerman.scalaloader.util.ScalaLoaderUtils.createScalaPluginInstance(ScalaLoaderUtils.java:141) ~[ScalaLoader-0.18.0-SNAPSHOT.jar:?]
    at xyz.janboerman.scalaloader.plugin.paper.ScalaLoader.loadScalaPlugins(ScalaLoader.java:173) ~[ScalaLoader-0.18.0-SNAPSHOT.jar:?]
    at xyz.janboerman.scalaloader.plugin.paper.ScalaLoader.loadScalaPlugins(ScalaLoader.java:139) ~[ScalaLoader-0.18.0-SNAPSHOT.jar:?]
    at xyz.janboerman.scalaloader.plugin.paper.ScalaLoader.onLoad(ScalaLoader.java:116) ~[ScalaLoader-0.18.0-SNAPSHOT.jar:?]
    at io.papermc.paper.plugin.storage.ServerPluginProviderStorage.processProvided(ServerPluginProviderStorage.java:59) ~[paper-1.19.4.jar:git-Paper-470]
    at io.papermc.paper.plugin.storage.ServerPluginProviderStorage.processProvided(ServerPluginProviderStorage.java:18) ~[paper-1.19.4.jar:git-Paper-470]
    at io.papermc.paper.plugin.storage.SimpleProviderStorage.enter(SimpleProviderStorage.java:36) ~[paper-1.19.4.jar:git-Paper-470]
    at io.papermc.paper.plugin.entrypoint.LaunchEntryPointHandler.enter(LaunchEntryPointHandler.java:36) ~[paper-1.19.4.jar:git-Paper-470]
    at org.bukkit.craftbukkit.v1_19_R3.CraftServer.loadPlugins(CraftServer.java:423) ~[paper-1.19.4.jar:git-Paper-470]
    at net.minecraft.server.dedicated.DedicatedServer.initServer(DedicatedServer.java:273) ~[paper-1.19.4.jar:git-Paper-470]
    at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1103) ~[paper-1.19.4.jar:git-Paper-470]
    at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:319) ~[paper-1.19.4.jar:git-Paper-470]
    at java.lang.Thread.run(Thread.java:833) ~[?:?]
Caused by: java.lang.NoSuchMethodException: fr.epicanard.mapsaver.MapSaverPlugin.<init>()
    at java.lang.Class.getConstructor0(Class.java:3585) ~[?:?]
    at java.lang.Class.getConstructor(Class.java:2271) ~[?:?]
    at xyz.janboerman.scalaloader.util.ScalaLoaderUtils.createScalaPluginInstance(ScalaLoaderUtils.java:132) ~[ScalaLoader-0.18.0-SNAPSHOT.jar:?]
    ... 12 more

You can find the ScalaPlugin here https://github.com/EpiCanard/MapSaver/blob/master/src/main/scala/fr/epicanard/mapsaver/MapSaverPlugin.scala

Jannyboy11 commented 1 year ago

Okay, thank you. I refactored the code for instantiation of ScalaPlugins only a little bit, so I am surprised by this exception. Also my test plugin that uses an object singleton for the main class succeeded to load, so I have to look at the bytecode differences as to why yours doesn't load.

Edit: I figured out why this happens: the loader tries to instantiate the companion class of the object. Edit2: Due to some personal circumstances, I will have to delay the v0.18.0 release till somewhere next week.

Jannyboy11 commented 1 year ago

Okay, I know what went wrong. This was caused by a change in behaviour by ScalaLoader on Paper: namely, the main attribute from the plugin.yml is no longer ignored. I made this change because on Paper, I'm not forced to use PluginDescriptionFile which requires a main attribute, but rather I can use any class that implements PluginMeta. So I wrote my own ScalaPluginMeta class which implements this interface and does not require a main attribute. I figured that, if a main class is defined in the plugin.yml, then the developer must really want to use this main class. The main class definition in your plugin.yml file is not the singleton object, but instead it is its companion class. The object's main class is fr.epicanard.mapsaver.MapSaverPlugin$ (the same as the original, but with a doller appended to the end). I created a fork to test it, and got a bit further in the loading process, but ran into some classloader-related issues which I will address next week. You can incorporate this change in your original repo, or you can decide to delete the main attribute entirely, if you don't care about compatibility with CraftBukkit and Spigot.

tl;dr: the main class defined in the plugin.yml was actually wrong (but this bug was ignored by the old loading mechanism), it is missing a dollar at the end. With the fix incorprated the plugin still doesn't load because of a classloader bug on my end.

EpiCanard commented 1 year ago

Now that you explain it, it really makes sense. I didn't think the problem could come from here.

Thanks for your analysis

Jannyboy11 commented 1 year ago

I changed the behaviour so that the main class from the (paper-)plugin.yml is only used as the main class when no class could be found with a @Scala or @CustomScala annotation. The classloader bug I mentioned is also fixed in this release. ScalaLoader-0.18.0-SNAPSHOT.zip Your original MapSaver plugin loads now, but commands didn't work in my test because I didn't set up a database. I have also yet to fix that PluginLoaderOrder bug, but I can only work on this again tomorrow.

EpiCanard commented 1 year ago

I tested and it works really well. Thank you for your work !

I still have an error but this one is on my side, maybe something has changed in 1.19.4 api.

Edit : False alert, this is normal behavior. Old database but new empty server so it couldn't work.

Jannyboy11 commented 1 year ago

Okay, nice! I leave this issue open until all functionality is properly implemented and tested on Paper.

Jannyboy11 commented 1 year ago

PluginLoadOrder bug fixed as of https://github.com/Jannyboy11/ScalaPluginLoader/commit/6b9af13d004b3b26e5306e75c2b7059ab6da88ae. All three of my test plugins now load and enable fine! Shutdowns and reloads also work as expected!

Release notes: https://www.spigotmc.org/resources/scalaloader.59568/update?update=494857