Add support for paper-plugin.yml

[rainbowdashlabs]

I finally finished my implementation. I saw the other PR too late, so here is my proposal as well.

What I changed:

• Registered the paper plugin in build file
• Updated properties
• Added Paper Plugin which builds to paper-plugin.yml
• I also added contributors already as proposed in Add support for contributors #29
• Marked api-version as required since it is.

I thought about using the BukkitPlugin as a parent, but there are just too many differences and I would probably expect that there will be more differences in the future. Maybe a further refactoring could still extract common classes.

I also thought about extracting the command and permission class to reuse those declarations for spigot and paper plugins. Maybe you have an opinion on this.

I also removed the YAMLGenerator.Feature.INDENT_ARRAYS because it renders object like this:

dependencies:
 -
  name: FastAsyncWorldEdit
  required: false
  bootstrap: true
 -
  name: Essentials
  required: true
  bootstrap: false

instead of this

dependencies:
- name: FastAsyncWorldEdit
  required: false
  bootstrap: true
- name: Essentials
  required: true
  bootstrap: false
load-before:
- name: FastAsyncWorldEdit
  bootstrap: false

Not sure what the reason was behind it, but I am quite certain it wont break something. Of course both cases are valid yaml, but I see no gain from the setting.

dependencies and loaders are defined like this:

    depends {
        register("WorldEdit") {
            required = true
        }
        register("Essentials") {
            required = true
        }
    }

    loadBefore {
        register("BrokenPlugin") {
            bootstrap = true
        }
    }

Paper Libraries work different. The library field in paper was removed, therefore no straight way of importing classes exist. Instead a file called plugin-libraries.json is generated when enabled.

paper {
    generatePluginLibraries = true
}

This file can be loaded from resources.

public class Loader implements PluginLoader {
    @Override
    public void classloader(@NotNull PluginClasspathBuilder classpathBuilder) {
        MavenLibraryResolver resolver = new MavenLibraryResolver();
        PluginLibraries pluginLibraries = load();
        pluginLibraries.asDependencies().forEach(resolver::addDependency);
        pluginLibraries.asRepositories().forEach(resolver::addRepository);
        classpathBuilder.addLibrary(resolver);
    }

    public PluginLibraries load() {
        try (var in = getClass().getResourceAsStream("/plugin-libraries.json")) {
            return new Gson().fromJson(new String(in.readAllBytes()), PluginLibraries.class);
        } catch (IOException e) {
            throw new RuntimeException(e);
        }
    }

    private record PluginLibraries(List<String> repositories, List<String> dependencies) {
        public List<Dependency> asDependencies() {
            return dependencies.stream()
                    .map(d -> new Dependency(new DefaultArtifact(d), null))
                    .toList();
        }

        public List<RemoteRepository> asRepositories() {
            AtomicInteger integer = new AtomicInteger();
            return repositories.stream()
                    .map(d -> new RemoteRepository.Builder("maven" + integer.getAndIncrement(), "default", d).build())
                    .toList();
        }
    }
}

Fixes #27

[stephan-gh]

@TheMeinerLP @rainbowdashlabs Could you coordinate/agree on one of the two PRs adding support for the paper-plugin.yml and close the other one? I would appreciate if I just need to review one PR with the set of changes you both consider to be best.

[rainbowdashlabs]

@stephan-gh We combined our two PRs. @TheMeinerLP did great work on the dynamic class generation to provide libraries and repositories inside the plugin.

The plugin is ready for review and I checked that it works on paper servers. I also updated the readme to include information about the paper module.

[TheMeinerLP]

Can you add also for normal "bukkit" plugins the folia support ?

[rainbowdashlabs]

I did this in #31 since it is not scope of this PR

[rainbowdashlabs]

Implementing this would basically involve removing library support from the Paper plugin (see Nukkit as example). Perhaps one could then define net.minecrell.plugin-yml.libraries like an additional PlatformPlugin. It doesn't have to be though, could also be a separate plugin class if needed.

Since this feature is required for paper anyway, I would include it since it is somehow a fixed dependency.

There are of course a lot of points still to improve, but I would prefer to stick to the initial scope of the PR which is simply "add support for paper plugins" and not "add a plugin for library exporting".

Stuff I would set on my list for future goals of this gradle plugin would be:

1. Add another plugin module for library export independent of the used platform. Whether we remove the library loading from paper after this or not has to be decided in the future.
2. Unify Permission and command classes. There are a lot of duplicated in terms of data classes at least between paper and spigot now, which could be unified to make better use of dsl. E.g. defining permissions once for both modules. That is currently not possible because the classes are different in their path.
3. Develop a companion lib which provides a default PluginLoader to use with the plugin-libraries.json

But that is future stuff and I would prefer to tackle this in future PRs.

[stephan-gh]

There are of course a lot of points still to improve, but I would prefer to stick to the initial scope of the PR which is simply "add support for paper plugins" and not "add a plugin for library exporting".

This sounds good and is likely indeed a good idea to speed up the review!

Since this feature is required for paper anyway, I would include it since it is somehow a fixed dependency.

But I'm confused about this part: How come is the library functionality a fixed dependency of the Paper plugin?

The main purpose of plugin-yml is to generate the plugin description file (paper-plugin.yml), with whatever contents are supported. For example, Nukkit does not have a built-in library mechanism in the nukkit.yml so we don't have that functionality there. That does not mean that it is impossible to load additional libraries there, it just needs to be implemented using additional code in the Nukkit plugin.

The way I understand it the same applies to Paper: They offer an API to load additional libraries but plugins are free to implement the library selection in any way they like. We can offer help with this, by generating a custom plugin-libraries.json and providing example code. In theory, the same JSON file and related example code could also be provided for Nukkit, since the functionality is not really specific to Paper. This is why I suggested that it should not be part of the Paper plugin, but an independent optional extension. People can decide to use it, or decide not to use it - for example if they would rather use the code generation approach or something entirely different.

It's probably easiest if we do it like you suggested, focus on "adding support for paper plugins" first. But then Paper should be set up like Nukkit and don't offer the "library exporting" at all in the first step.

[rainbowdashlabs]

That might be the viewpoint from a development side. However I would also take a look at all this from the majority of the user side.

Nearly every person person usually has the normal plugin yml. Adding a version replacement is not that much work. However most of the people decide to use this plugin once they want to load libraries. Imo that is the main selling point of this plugin. For me as well. It is simply a feature that users of the bukkit and in the future the paper module expect.

I dont know what capabilities nukkit has, but from my view it doesnt seem to be important, otherwise this feature would exist already.

My approach would be to implement a generic approach to create such a file for any platform by simply extending a barebone class which allows this. This would then be added for the required platforms and enabled those who are supposed to use it to use it properly.

Making this a general dependency might cause some other issues:

1. File name conflicts
2. Multireleases for nukkit and paper for example, where we probably need different libraries. how do we tell which dependency belongs in which file if both are using the same plugin in the same file?
3. People might expect this to do something on bukkit as well since reading documentation is hard.

But also honestly I grew a bit tired of this PR already. The first suggestions were reasonable, but I feel like we are starting to argue a lot about personal preferences.

[lordofpipes]

Thanks this looks much better. Given that the plugin-libraries.json are not directly related to Paper I now wonder if this could be moved into a separate net.minecrell.plugin-yml.libraries plugin that can be used either additionally to the Paper plugin, but also independently of it. Some people might want to use it on entirely different platforms just to get the plugin-libraries.json file generated. Basically, I'm thinking of something like

plugins {
    id("net.minecrell.plugin-yml.paper") version "0.5.3"
    id("net.minecrell.plugin-yml.libraries") version "0.5.3"
}

[...]

I'm not sure why this implementation detail needs to be exposed to the user. Sharing the json generation code between platforms is already easy — we already have generic-icity between the different platform types in the form of PlatformPlugin being the base class for the other plugin types. So just having plugin { generatePluginLibraries = true and having the gradle plugin figure it out for you is slightly more obvious from a usability perspective, and also prevents the user from generating such a file for platforms where it doesn't make sense. The way I see it, Gradle plugins are supposed to introduce some degree of magic hand-wavey stuff without the need to be explicit about everything.

However, it may be justified if specifying the libraries plugin can also automatically pull in the appropriate runtime helper code. But this is straying close to being codegen-like, and I lean towards the principle of separating the runtime code out into a separate implementation() library (or for now, a README example suffices)

Anyways, I like the current state of the PR, and would also be fine with any small tweak to the idea. Should emphasize that the differences between these approaches are very minor and may not be consequential. (But on the other hand I guess there is time to debate and get it right the first time, since Paper Plugins are still an alpha feature of PaperMC.)

[stephan-gh]

That might be the viewpoint from a development side. However I would also take a look at all this from the majority of the user side.

Nearly every person person usually has the normal plugin yml. Adding a version replacement is not that much work. However most of the people decide to use this plugin once they want to load libraries. Imo that is the main selling point of this plugin. For me as well. It is simply a feature that users of the bukkit and in the future the paper module expect.

For me the reason I made this plugin is that I was "annoyed" by having to write several subtly different template YAML files with placeholders for version, description, author, website. This is also why the plugin tries to fill out those automatically with information from the Gradle project. I haven't used the "library exporting" feature much myself. However, I understand how this can be very useful as well and I think it's perfectly fine if everyone finds different value in using the plugin.

The difficult part for libraries on Paper is that we don't have a "standard" solution to work with. The various options we have already discussed, i.e.

• Code generation
• Data file with repositories + dependencies
  • JSON vs YAML
  • Example code vs prebuilt library

make it obvious that this part is very subjective and everyone has slightly different personal preferences. Perhaps some project setups work better with the code generation approach, while for others the JSON file is more simple.

Ideally I would like plugin-yml to be as "impartial" as possible. The core functionality right now is defined exactly by the specification of the plugin-y(a)ml files, with convenient integration into Gradle where appropriate. For Bukkit/Bungee, we can output libraries in plugin.yml/bungee.yml. For Paper(/Nukkit) we can't - we need something custom. But no matter how one personally handles libraries on Paper, you still need the standard paper-plugin.yml. And that is - for me - the main purpose of plugin-yml.

Providing opinionated defaults for convenience of most users is also nice, but in my opinion those should be clearly separated and optional. And this is the part I'm not entirely sure about yet.

If Bukkit/Bungee did not already have the library functionality I probably would have suggested "implement this in a separate plugin + project" right from the start, since it goes beyond generating the plugin-y(a)ml file. And this would not be a bad thing at all - Gradle has a flexible plugin architecture that allows combining plugin-yml with any number of other plugins.

We do however have that functionality for Bukkit/Bungee, and I also understand the argument that people coming from Bukkit expect the same functionality to work on Paper. It's not that easy though because Paper is simply different, and people still need to copy the example code, set up the loader correctly etc. Otherwise they will just find the plugin-libraries.json doing nothing.

I perfectly understand if you find my thoughts a bit "overengineered", but for me personally a clear separation of the purposes and responsibilities on the design level is important. I believe this makes the end result easier to understand and also easier to maintain (which will be my job).

---

TL;DR: I probably just need some more time to think about this and maybe experiment with some options myself.

-> As mentioned before I suggest we focus on the paper-plugin.yml part first and do the "library exporting" in the second step. I started integrating the changes in this PR and have some purely technical comments: If you prefer I can fix them myself but I didn't want to change all this without asking for your opinion first. :)

[stephan-gh]

I'm still not fully sure if supporting libraries for Paper is in scope for plugin-yml.. If someone wants more features than just the simple plugin-libraries.json this would definitely fit better in a separate Gradle plugin.

However, after playing with the changes a bit locally it seems easy enough to support generating the JSON file. I made some minor refactoring to the changes:

• The option is now called generateLibrariesJson and is supported for all plugins (Nukkit, even Bukkit/Bungee although I don't think anyone will use it there)
  • The implementation is a bit nicer that way because it doesn't require special-casing Paper
  • To avoid conflicting files as mentioned here earlier the generated file name is now paper-libraries.json (or nukkit-libraries.json etc)
  • I changed the repositories from a List to a Map, since Gradle supports repository names we can use (so there is no need to use the AtomicInteger numbering)
• To avoid duplication I just reuse the BukkitPluginDescription.{Permission,PluginLoaderOrder}, seems to work nicely

Could you look through the commits I pushed and look and/or test that this is still okay? Any feedback is welcome. Thanks.

[lordofpipes]

I ported a plugin to use it and it seems to work well. Make sure README.md is changed back to BukkitPluginDescription

Here: d66b3da4ff0a31f1d61675981181e43b0e33bb39

diff --git a/README.md b/README.md
index 252d86f..0a2776a 100644
--- a/README.md
+++ b/README.md
@@ -276,11 +276,11 @@ paper {
     apiVersion = "1.19"
 
     // Other possible properties from plugin.yml (optional)
-    load = PaperPluginDescription.PluginLoadOrder.STARTUP // or POSTWORLD
+    load = BukkitPluginDescription.PluginLoadOrder.STARTUP // or POSTWORLD
     authors = listOf("Notch", "Notch2")
 
     prefix = "TEST"
-    defaultPermission = PaperPluginDescription.Permission.Default.OP // TRUE, FALSE, OP or NOT_OP
+    defaultPermission = BukkitPluginDescription.Permission.Default.OP // TRUE, FALSE, OP or NOT_OP
     provides = listOf("TestPluginOldName", "TestPlug")
 
     depends {

Should it have a Kotlin example for the loader as well?

[stephan-gh]

Make sure README.md is changed back to BukkitPluginDescription

Thanks for noticing this!

Should it have a Kotlin example for the loader as well?

Hm you mean the PluginLibrariesLoader? Does Paper include Kotlin already? Otherwise I'd expect that you need a Java Loader first that downloads the kotlin-stdlib and the rest can then be Kotlin.

[lordofpipes]

Make sure README.md is changed back to BukkitPluginDescription

Thanks for noticing this!

Should it have a Kotlin example for the loader as well?

Hm you mean the PluginLibrariesLoader? Does Paper include Kotlin already? Otherwise I'd expect that you need a Java Loader first that downloads the kotlin-stdlib and the rest can then be Kotlin.

In theory, could the loader be written without requiring the kotlin standard library? Not actually sure how heavily tied kotlin is to its standard library. If it does try to pull in a bunch of stuff no matter what, then yeah, shouldn't bother adding a README example.

[lordofpipes]

Heads up that Paper might be changing the paper-plugin.yml format, it's not confirmed yet but has been discussed here: https://discord.com/channels/289587909051416579/925530366192779286/1097207295777194054 (PaperMC Discord). Seems like it will be a pretty small change, just a restructuring of the dependencies section. I can help implement this when the time comes. They've said that if they add this, they won't be dropping support for the old format until 1.20 — so this isn't an immediate concern for plugin-yml.

Anyways, I've been using this pull request for a bit and it seems to work great. No complaints so far!

[rainbowdashlabs]

Sorry for the long delay. Didnt really had time to test this. No objections from my side. This seems to be functional and working.

[lordofpipes]

Here is the 1.20 update on paper-plugin.yml formatting, from Owen:

2023-06-07, PaperMC discord #dev-announcements

Dependency Format Update

Dependency declaration has been update for paper plugins to better represent the different lifecycles in Paper plugins.
Note, the previous format is scheduled for removal in 1.21.

Most noteably with this new format is that specifying load order is now relative to the dependency rather than the plugin as a whole.

So for example, marking a dependency with load: BEFORE will now cause the dependency to load BEFORE your plugin.

Before

load-before:
  - name: RequiredPlugin
    bootstrap: false
load-after:
  - name: RegistryPlugin
    bootstrap: true
  - name: OtherPlugin
    bootstrap: false
dependencies:
  - name: OtherPlugin
    required: false
    bootstrap: false
  - name: RegistryPlugin
    required: true
    bootstrap: true

After

dependencies:
  bootstrap:
    # Lets say that RegistryPlugin has some registry elements that this plugin requires.
    # We don't need this during runtime, so it's not required in the server section. However
    # can be added to both if needed
    RegistryPlugin:
      load: BEFORE 
      required: true
      # (this is default)
      join-classpath: true
  server:
    # Add a required "RequiredPlugin" dependency, which will load AFTER your plugin.
    RequiredPlugin:
      load: AFTER
      required: true
      # This means that this plugin won't have access to classpath
      join-classpath: false
    # Add "OtherPlugin" dependency, which will load BEFORE your plugin. WILL join classpath (by default)
    OtherPlugin:
      load: BEFORE
      required: false
      join-classpath: true
    # Load order can be omitted to cause it to be ignored... or specified by load-order: OMIT
    SpecialDependency:
      required: true
      join-classpath: true

[lordofpipes]

Here is a version that supports the new dependencies format introduced in 1.19.4 build #549 and intended to become the standard for 1.20. However, it drops support for Paper 1.19.3 build #405 to 1.19.4 build #548.

lordofpipes@a08522c

@Minecrell @rainbowdashlabs @stephan-gh @TheMeinerLP what do you think?

Here is the syntax:

kotlin

paper {
    [...]
    bootstrapDependencies {
        // Required dependency during bootstrap
        register("WorldEdit")

        // During bootstrap, load BeforePlugin's bootstrap code before ours
        register("BeforePlugin") {
            required = false
            load = PaperPluginDescription.RelativeLoadOrder.BEFORE
        }
        // During bootstrap, load AfterPlugin's bootstrap code after ours
        register("AfterPlugin") {
            required = false
            load = PaperPluginDescription.RelativeLoadOrder.AFTER
        }
    }

    serverDependencies {
        // During server run time, require LuckPerms, add it to the classpath, and load it before us
        register("LuckPerms") {
            load = PaperPluginDescription.RelativeLoadOrder.BEFORE
        }

        // During server run time, require WorldEdit, add it to the classpath, and load it before us
        register("WorldEdit") {
            load = PaperPluginDescription.RelativeLoadOrder.BEFORE
        }

        // Optional dependency, add it to classpath if it is available
        register("ProtocolLib") {
            required = false
        }

        // During server run time, optionally depend on Essentials but do not add it to the classpath
        register("Essentials") {
            required = false
            joinClasspath = false
        }
    }
}

groovy

paper {
    [...]
    bootstrapDependencies {
        // Required dependency during bootstrap
        'WorldEdit' {}

        // During bootstrap, load BeforePlugin's bootstrap code before ours
        'BeforePlugin' {
            load = PaperPluginDescription.RelativeLoadOrder.BEFORE
            required = false
            joinClasspath = false
        }
        // During bootstrap, load AfterPlugin's bootstrap code after ours
        'AfterPlugin' {
            load = PaperPluginDescription.RelativeLoadOrder.AFTER
            required = false
            joinClasspath = false
        }
    }

    serverDependencies {
        // During server run time, require LuckPerms, add it to the classpath, and load it before us
        'LuckPerms' {
            load = PaperPluginDescription.RelativeLoadOrder.BEFORE
        }

        // During server run time, require WorldEdit, add it to the classpath, and load it before us
        'WorldEdit' {
            load = PaperPluginDescription.RelativeLoadOrder.BEFORE
        }

        // Optional dependency, add it to classpath if it is available
        'ProtocolLib' {
            required = false
        }

        // During server run time, optionally depend on Essentials but do not add it to the classpath
        'Essentials' {
            required = false
            joinClasspath = false
        }
    }
}

[stephan-gh]

@lordofpipes Thanks! I merged the previous state for now, can you make a new PR with the needed changes?

[lordofpipes]

Here: #34