Paper Plugins, Bootstrappers, Loaders and More?

[Owen1212055]

This implements paper plugins, which can be created by paper-plugin.yml in your plugin resources. At runtime, they are all in the end the same "JavaPlugins". However, in the future, it is possible that this can be expanded. Note: You can have both paper plugin and spigot plugin ymls and paper will prioritize the paper yml. The goal is to also have paper and spigot plugins work fine together (because again, they are the same at run time).

This was opened to collect a lot of feedback Difference between paper plugins:

• library loading is done at runtime by plugin loaders, not anymore in the config.
• They have a cool bootstrapper field, allowing code to be executed even before a Minecraft server is created!
• Cannot be pre-1.19
• Cannot be loaded on spigot servers (shocker)

TODO:

• [x] Reload logic
• [x] Througly test classloader stuff, it certainly doesn't work right now.
• [x] Duplicate dependency loading logic in the ServerPluginProviderStorage
• [x] Connect the api for loading plugins and what not
• [x] Test api stuff
• [X] Make /plugins cooler
• [ ] Make /version cooler
• [x] Switch everything to use the java logger, I decided to use the slf4j one and it bit me later.
• [x] Ensure exceptions are similar for failures and whatnot.
• [x] Make it so paper plugins cannot be legacy.
• [x] Implement yucky bytecode plug-in fixer
• [x] Document, document, document!!
• [ ] General diff cleanup
• [ ] Make a standard for plugin logging. What should be an error? What should be a warning?
• [ ] Test plugin updater
• [ ] Move classloader stuff out of API, cleanup and in general make some of the classloader groups wrap a classloader group rather than the list.

Resolves: https://github.com/PaperMC/Paper/issues/5992 https://github.com/PaperMC/Paper/issues/7961 https://github.com/PaperMC/Paper/issues/7955

---

Download the paperclip jar for this pull request: paper-8108.zip

[lokka30]

Although I like the library loading feature, I've heard it's against Maven Central's TOS. I believe Paper should adhere to that in order to avoid potential issues down the line. It's also important to factor in that lots of servers are floating around, hopefully they aren't already putting too much strain on Central. For now, I am quite certain only a small amount of plugins use library-loading, so the damage shouldn't be too bad, I assume.

[electronicboy]

the library loader is fine, bar the fact that it's pointed directly to central, ideally what we do is just flip it over to our own repo at some point, just some pending stuff in the works, etc

[underscore11code]

While I'm fully aware of how non-trivial it is, having checks on all the adventure-deprecated methods (Player#sendMessage(String) and such) to see if the caller is a paper plugin or not, and if so, in some way flagging it (cancel it, console warn, shutdown server, something), seems like a good idea to try to help force people off legacy code. Same goes for ChatColor etc.

[Owen1212055]

The /plugins command has received some changes Clicking on a plugin name will now automatically run /version The version command will be redone in the future as well.

Descriptions will probably be moved to the doc website. Changes:

• Added custom paper plugins command
• If an initializer fails, the corresponding plugin provider will be skipped.

[lokka30]

Looks awesome Owen!

[Owen1212055]

Plugin Loaders

public class TestPluginLoader implements PluginLoader {
    
    @Override
    public void classloader(PluginClasspathBuilder classpathBuilder) {
        classpathBuilder.addLibrary(new JarLibrary(Path.of("bob.jar")));

        MavenLibraryResolver resolver = new MavenLibraryResolver();
        resolver.addDependency(new Dependency(new DefaultArtifact("com.owen1212055:particlehelper:1.0.0-SNAPSHOT"), null));
        resolver.addRepository(new RemoteRepository.Builder("bytecode", "default", "https://repo.bytecode.space/repository/maven-public/").build());

        resolver.setFilter(new DependencyFilter() {
            @Override
            public boolean accept(DependencyNode node, List<DependencyNode> parents) {
                return true;
            }
        });

        classpathBuilder.addLibrary(resolver);
    }
}

Plugin loaders allow you to configure certain things about your plugin before it is created at all. In this case, a new robust way of adding libraries to your plugin! This allows you to add jars/dependencies before your plugin is created, making it a bit nicer for plugins that may require a lot of complex library logic.

Plugin Initializers have also been renamed to PluginBootstrappers.

• Should it be bootstrapper or just bootstrap?

[TheFruxz]

Is it is possible to disable the advertisement (like Hangar), in the server properties?

[Owen1212055]

Is it is possible to disable the advertisement (like Hangar), in the server properties?

It might be moved to the paper docs instead? Does that sound fine? I can make a config option to disable these types of links tho.

[TheFruxz]

Is it is possible to disable the advertisement (like Hangar), in the server properties?

It might be moved to the paper docs instead? Does that sound fine? I can make a config option to disable these types of links tho.

That would be great, thank you!

[Owen1212055]

Is it is possible to disable the advertisement (like Hangar), in the server properties?

It might be moved to the paper docs instead? Does that sound fine? I can make a config option to disable these types of links tho.

That would be great, thank you!

For clarification, the config or the moving it to the docs? (I’m fine with both)

[TheFruxz]

Is it is possible to disable the advertisement (like Hangar), in the server properties?

It might be moved to the paper docs instead? Does that sound fine? I can make a config option to disable these types of links tho.

That would be great, thank you!

For clarification, the config or the moving it to the docs? (I’m fine with both)

For me personally, the text can stand as it is, including the link to Hangar. For many inexperienced players or server owners, this could be a good help. But in my opinion, it should be possible to disable it optionally via the configuration. It doesn't have to move somewhere else, this seems to be a good place for that type of information

[jpenilla]

I don't think linking to our own site like this is "advertising". We can choose to link Hanagar or not, personally I think it's a good idea, but a config option for this seems like totally unnecessary bloat.

[TheFruxz]

I don't think linking to our own site like this is "advertising". We can choose to link Hanagar or not, personally I think it's a good idea, but a config option for this seems like totally unnecessary bloat.

I would definitely say that promoting another product/service (no matter how strongly related to the manufacturer) is advertisement.

[jpenilla]

In the literal sense sure, hence the quotes in my comment, but using that word to describe this is somewhat disingenuous in my opinion and implies unfair comparisons. You could also argue /ver is advertising our downloads site. Additionally there is the precedent of Bukkit, Spigot, and Paper linking to their various sites such as forums, discord, irc, etc. in their config files.

[electronicboy]

I think in the config files makes sense, or maybe if there's 0 plugins installed, but, otherwise, chat is a limited space and it just feels like extra pointless fluff which should be on the "getting started" guide

[jpenilla]

It's a hover text, not a chat message, which I would agree would be too much. Again I don't feel super strongly for or against linking Hangar here, just pointing out the "advertising" point is not really relevant to that decision in my mind.

[Owen1212055]

I've implemented dependency loading, would appreciate if anyone would like to look over that.

There are now two "plugin loaders", legacy and modern. Modern plugin loading (default) does not allow infinite dependency loops, as this caused issues in the past. If infinite dependency loops are detected the servers shuts down.

Optionally, this can be switched by using the LEGACY plugin loader in paper global config (misc.plugin-loading-strategy)

Please feel free to test if plugins correctly load in order. Also /plugins now split every 10 plugins

[Owen1212055]

The test plugin has been relocated to io.papermc.testplugin inorder to properly block plugins loading in the io.papermc.paper namespace.

This can be reverted, but currently applies for:

  "net.minecraft.",
   "org.bukkit.",
   [NEW] "io.papermc.paper.",
   [NEW] "com.destroystokoyo.paper."

[Owen1212055]

I'm going to mark this as ready for review, most of the larger requirements have been resolved.

The api support has been finished, and PluginLoader has been deprecated.

[Andre601]

I like the improved plugin thing, tho it feels a bit much...

Maybe keep it closer to the original but like this?

<white>Server Plugins (3):
<green>PaperPlugin</green><grey>, <gold>LegacyPlugin</gold>, <red>DisabledPlugin</red>

Then have hover text displaying plugin info and what plugin type it is. Or have the asterisk next to the plugin for more info.

Just an idea tho.

Also regarding the yaml file: Maybe allow api-version for paper to allow multiple versions, to A) avoid legacy warning on load and B) allow a plugin to load on newer versions without older versions failing.

---

Example I had imagined:

https://user-images.githubusercontent.com/11576465/183115519-0b2e2676-c1f1-4ba5-a824-807091f54b70.mp4

[Owen1212055]

@Andre601 Thanks for the feedback, I was hesitant at putting info like that in the hover because I was worried about making the message too large. I might need to split it up significantly more (I added logic to split every 10 plugins), as I want to make sure I don't kick people because of the message being too large.

But yeah, I defo understand what you mean here. If you have any ideas here, I'm open.

[maestro-denery]

(I hope Owen won't hate me for writing such essays) Let's talk about the problems of PluginBootstrap usage I discovered while trying to use it for registering custom things with plugins. (I've chosen DimensionTypes as an example) First of all, Imo there are 3 types of registries in Minecraft:

1. Common registries - registries which needed both on a server and on a client and which aren't sychronized between them. (Blocks, entities, items etc.)
2. Server-side registries - registries which server uses and don't need to be sychronized with a client. (LevelStem, MemoryModuleType, ConfiguredFeature etc. As you can see, they may appear both in Registry and BuiltInRegistries classes)
3. Synchronizable registries - registries which content sychronizes with a client in the ClientboundLoginPacket class by serializing RegistryAccess in NBT with Codecs. (For now there are only three of them: Biome, DimensionType and ChatType.)

Second of all, I've suggested Owen to move PluginBootstrap initialization above the Bootstrap.bootStrap()method in the review because in the future we may need to change a content of "common" registries. By changing I don't mean adding or removing blocks, entities etc. I mean, for example, changing BlockProperties of a block which is already exist in Minecraft to have higher resistance value. (There is already an Explosion Resistance API PR by PeterCrawley with such functional) So to handle such operation more safely we can allow changing these properties only if a content hasn't registered yet.

But there is another problem: We need bukkit events to work when Bukkit API hasn't initialized yet, and even when a server instance hasn't created. And we all know that this event API is controlled by PluginManager, from which Owen even makes its new implementation!!! (PaperPluginManagerImpl) And also Owen makes the PaperEventManager, which has four public methods copied from the SimplePluginManager. And I found that it isn't that hard to make these four methods independent from Bukkit API, so in my fork I've made my own isPrimaryThread() method and some others in there, also good to mention that I moved CraftServer's Bukkit configuration loading to the n.m.s.Main.main(), so I can receive WarningState in there. (Everything this configuration thing needs is OptionSet passed in n.m.s.Main.main(), so it was kinda easy. It all works without exceptions, but I highly doubt that there are no deeper issues) And after all of that I made my own EventManager interface in the API and made Owen's PaperEventManager implement it, so I could call and receive events even before a server instance has created. But there is another problem: Which plugin instance do I pass to register an event listener from PluginBootstrap? For now I don't know an ideal solution for it, but a simple one is to make MinecraftInternalPlugin instance creation way before and use it in PluginBootstraps, so did I.

Now we have our independent EventManager API instance, but where do we store this singleton to use it in the API? I think it is kinda a huge problem, but for Paper I think the best way is to move all this "pre-server" stuff into a separate gradle subproject. (:paper-preserver-api like you did with the :paper-mojangapi?) (including PluginBootstrap). And make in there a "Paper" class (like Bukkit class) and have a "PreServerAPI" interface instance in it. (like the Server class in a Bukkit class) If you know a better solution where to store these API singletons, then I'd be interested to hear it.

So, why did I need events to register my custom dimension? Why didn't I registered it right in the PluginBootstrap.bootstrap() method? That's because we placed registries before the Bootstrap.bootStrap() to handle common registries content properties, so if we try to register our new DimensionType before registries are initialized we'll get an IllegalArgumentException from Bootstrap.checkBootstrapCalled(), and it's totally understandable, we'd break an order of how registries initialize and we don't know which bugs would it cause. So, I made RegistriesLoadEvent which fires in Bootstrap.bootStrap() after mc's registries are loaded, but before they are frozen. (before the Registry.freezeBuiltins() method executes) Now, let's test if our DimensionType is registered properly!

    public static void bootstrap() {
        WaitingRoom.WAITING_ROOM_DIM_KEY = WaitingRoom.register("toky:waiting_room");

        WaitingRoom.WAITING_ROOM_DIM = Toky.secure(BuiltinRegistries.register(
                BuiltinRegistries.DIMENSION_TYPE,
                WaitingRoom.WAITING_ROOM_DIM_KEY,
                new DimensionType(OptionalLong.empty(), false, true, false, false, 256.0D, true, true,
                        0, 16, 16, BlockTags.INFINIBURN_END, BuiltinDimensionTypes.OVERWORLD_EFFECTS, 0.0F,
                        new DimensionType.MonsterSettings(true, false, UniformInt.of(0, 7), 0))
        ));

        BuiltinRegistries.DIMENSION_TYPE.entrySet().forEach(entry -> {
            System.out.println("key: " + entry.getKey().toString() + " :: value: " + entry.getValue().toString());
        });
    }

This method is placed in a class called "WaitingRoom" (I called the dimension type this way) I run it when the RegistriesLoadedEvent fires which listener is registered in my PluginBootstrap, so in a console I'll get:

Starting org.bukkit.craftbukkit.Main
System Info: Java 17 (OpenJDK 64-Bit Server VM 17.0.1+12-jvmci-21.3-b05) Host: Linux 4.18.0-408.el8.x86_64 (amd64)
Loading libraries, please wait...
2022-09-04 16:51:31,690 ServerMain WARN Advanced terminal features are not available in this environment
[16:51:36 INFO]: Building unoptimized datafixer
key: ResourceKey[minecraft:dimension_type / toky:waiting_room] :: value: DimensionManager[fixedTime=OptionalLong.empty, hasSkyLight=false, hasCeiling=true, ultraWarm=false, natural=false, coordinateScale=256.0, bedWorks=true, respawnAnchorWorks=true, minY=0, height=16, logicalHeight=16, infiniburn=TagKey[minecraft:block / minecraft:infiniburn_end], effectsLocation=minecraft:overworld, ambientLight=0.0, monsterSettings=a[piglinSafe=true, hasRaids=false, monsterSpawnLightTest=[0-7], monsterSpawnBlockLightLimit=0]]
key: ResourceKey[minecraft:dimension_type / minecraft:overworld_caves] :: value: DimensionManager[fixedTime=OptionalLong.empty, hasSkyLight=true, hasCeiling=true, ultraWarm=false, natural=true, coordinateScale=1.0, bedWorks=true, respawnAnchorWorks=false, minY=-64, height=384, logicalHeight=384, infiniburn=TagKey[minecraft:block / minecraft:infiniburn_overworld], effectsLocation=minecraft:overworld, ambientLight=0.0, monsterSettings=a[piglinSafe=false, hasRaids=true, monsterSpawnLightTest=[0-7], monsterSpawnBlockLightLimit=0]]
key: ResourceKey[minecraft:dimension_type / minecraft:the_end] :: value: DimensionManager[fixedTime=OptionalLong[6000], hasSkyLight=false, hasCeiling=false, ultraWarm=false, natural=false, coordinateScale=1.0, bedWorks=false, respawnAnchorWorks=false, minY=0, height=256, logicalHeight=256, infiniburn=TagKey[minecraft:block / minecraft:infiniburn_end], effectsLocation=minecraft:the_end, ambientLight=0.0, monsterSettings=a[piglinSafe=false, hasRaids=true, monsterSpawnLightTest=[0-7], monsterSpawnBlockLightLimit=0]]
key: ResourceKey[minecraft:dimension_type / minecraft:overworld] :: value: DimensionManager[fixedTime=OptionalLong.empty, hasSkyLight=true, hasCeiling=false, ultraWarm=false, natural=true, coordinateScale=1.0, bedWorks=true, respawnAnchorWorks=false, minY=-64, height=384, logicalHeight=384, infiniburn=TagKey[minecraft:block / minecraft:infiniburn_overworld], effectsLocation=minecraft:overworld, ambientLight=0.0, monsterSettings=a[piglinSafe=false, hasRaids=true, monsterSpawnLightTest=[0-7], monsterSpawnBlockLightLimit=0]]
key: ResourceKey[minecraft:dimension_type / minecraft:the_nether] :: value: DimensionManager[fixedTime=OptionalLong[18000], hasSkyLight=false, hasCeiling=true, ultraWarm=true, natural=false, coordinateScale=8.0, bedWorks=false, respawnAnchorWorks=true, minY=0, height=256, logicalHeight=128, infiniburn=TagKey[minecraft:block / minecraft:infiniburn_nether], effectsLocation=minecraft:the_nether, ambientLight=0.1, monsterSettings=a[piglinSafe=true, hasRaids=false, monsterSpawnLightTest=11, monsterSpawnBlockLightLimit=15]]

You see that I have properly registered my custom DimensionType, so I can make LevelStem with it, load it and enjoy my custom dimension with the custom DimensionType without datapacks!

Now I've proven that making our future "Registry API" is totally possible without breaking many aspects of CraftBukkit's initialization, but again, I used NMS registries in here and we still don't know will it break an actual bukkit API or not. So, Owen, you have a choice:

1. Extend this API with "PreServer" things in the separate subproject I described in here.
2. Remove PluginBootstrap from this PR for now and return it in the complete "PreServer" API PR in the separate subproject.

I hope this "essay" will check MachineMaker / papermc's core team and answer if such concept has a place to be in Paper or not. Thank you for all of your work!

[Owen1212055]

@maestro-denery I have made changes that should allow adding to all registries. See RegistryDebuggingImpl, does anything look wrong with this?

[maestro-denery]

Let's summarize the talk from discord. To begin with, I think the correctness of PluginBootstrap initialization is really important, because it'd cause many problems in the future if we do it incorrectly in this PR. For example, if we do it incorrectly and then start making an API exposing registries, we'd have to do breaking changes for users who used PluginBootstrap before for some reason, which is bad if we can avoid them from the beginning. (even if we mark all of this stuff as experimental and users would use NMS) First of all, I think that the PluginBootstrap should be moved to another gradle subproject (aka module) like mojang-api. There are several reasons to do that:

1. We can create a simple API contract: "People can interact with everything in this subproject in the PluginBootstrap#bootstrap() method without worrying if something will throw an NPE like a Bukkit API usage"
2. People who are new to the concept of registries, how minecraft initializes and would want to use some new fancy API based on it would ask why Paper moved these fancy things to a separate project, and they'd more likely to find the reason, which is important to understand because of the API contract I mentioned above. (dare I say, 80% of plugin devs don't know what a registry is.)
3. More correct API designs which depend on registries. For example, there is the Brain API, which can be split in two parts: 1) Brain management. 2) Creation of Brain components. (Memories, Sensors, Activities) The first one stays in the same place, what it basically provides is to interact with vanilla Brain stuff and do various operations with an entity's brain. The second one should go into the subproject and the thing it does is the creation of those Brain components, so a user can register those things in a PluginBootstrap, pass this data to their plugin constructor and do management with the first part of the API. Hence, a user can use the Brain API without depending on that subproject, but if a user wants to create their own memories, activities, they'll have to learn at least briefly why this thing is like that. That's all for the subproject idea.

Now let's discuss how actually PluginBootstrap initialization should look like. Owen asked me why all the event stuff was needed. And, first of all what we need to understand, modding systems like Forge and Fabric which deal with problems of initialization like in this PR have a concept of "lifecycle events". The concept is simple: an API has events which tell people at what point of initialization they should perform what they need. These events are mostly global ones like: "ServerStartEvent" or something like that, but registries also have their lifecycles and stages how they initialize. This is the problem, we can't put a PluginBootstrap entrypoint in some concrete place of initialization so everything would work, and if we make more methods in the PluginBootstrap class for every initialization stage it would look bad, because a user can't choose whether they should listen to some initialization stages or not, and what if there would be a lot of these stages to listen to? I hope you've got the point. So, what are concrete examples of such initialization stages? The most relevant example I can tell is the RegistryAccess creation, the thing it does is making new Registry instances which are based on BuiltInRegistries instances. These new instances are empty and later all of the datapack data is put in them. Mojang created such concept as RegistryAccess to control where is a datapack content and where is a builtIn content. The thing we don't want to do is register a user's content directly in the Registry instances of BuiltInRegistries because in this case we would put our not-builtIn content alongside builtIn one, and both MachineMaker and I think that it is bad to do. (You did so in your RegistryDebiggingImpl) Hence, the best thing to do would be to put a user's content together with a datapack content into a newly created RegistryAccess. But the problem is that this RegistryAccess is created later than the registries initialize in the Registry class. So we need to have an event for that to register custom biomes, dimension types, etc. It was only one of those examples, there are even more little and less relevant ones. Anyway, I am a fan of a solution where you call the PluginBootstrap entrypoint method at the earliest moment you can do, and then make lifecycle events for the most important stages and later when the time comes to implement our "Registry API" we can expand these lifecycle events without doing any breaking changes and worrying how did we do initialization in this PR. That's all I guess, thanks for reading!

[Owen1212055]

Hmm okay, I see. So basically you are saying that this might also be helpful for future plans, as it would allow us to add additional entry points?

[maestro-denery]

Hmm okay, I see. So basically you are saying that this might also be helpful for future plans, as it would allow us to add additional entry points?

Yes, I think that some kind of an event system is needed there, but there is one question remaining, which event system would you use? Because I remember during the discord conversation on my previous comment people agreed that what I did with the bukkit even system in my test fork is a hack and looks bad, and they agreed that it is better to create a new event system than reusing bukkit one for that purpose. Sounds kinda edgy ngl

[DaRacci]

Hmm okay, I see. So basically you are saying that this might also be helpful for future plans, as it would allow us to add additional entry points?

Yes, I think that some kind of an event system is needed there, but there is one question remaining, which event system would you use? Because I remember during the discord conversation on my previous comment people agreed that what I did with the bukkit even system in my test fork is a hack and looks bad, and they agreed that it is better to create a new event system than reusing bukkit one for that purpose. Sounds kinda edgy ngl

In the idea of a new event system are you referencing a system which would be used only for this early bootstrapping stage events or to deprecate and phase out the bukkit event system. Personally I've been experimenting with some abstraction layers over the bukkit event system in my api plugin and what i found to be most intuitive and clean to use was a monad like system for filtering, and launching in specific contexts / threads and so on.

[maestro-denery]

Personally I've been experimenting with some abstraction layers over the bukkit event system in my api plugin and what i found to be most intuitive and clean to use was a monad like system for filtering, and launching in specific contexts / threads and so on.

Yeah, I see, so we'll have an interface which would look like something like this?

public interface LifecycleProvider {
    LifecycleProvider registryAccessInitialization(Consumer<RegistryAccessContext> context);
    
    /**
    * Registers all the methods above so their consumers can be executed on proper stages.
    */
    void build();
}

In the idea of a new event system are you referencing a system which would be used only for this early bootstrapping stage events or to deprecate and phase out the bukkit event system.

I don't think there is a need to deprecate and phase out the bukkit event system, at least before the "hard fork". As I said in previous comments I am a fan of separating APIs which interact with minecraft before the bukkit API initializes into another gradle subproject like Paper-MojangAPI and I don't wanna these preserver things to affect the main API... So I think I like more your solution with a tiny monad-like system for managing lifecycles than an entire new event system...

[darbyjack]

One concern that I would like to bring up is that from what I can tell, there is no jar hash checking for libraries/dependencies.

This becomes a problem due to the fact that we are moving from all dependencies coming from a hard-coded location to opening it up to plugin developers to specify their own repositories to download from.

How it plays out currently:

1. A plugin is made with a runtime downloaded dependency that is a SNAPSHOT artifact
2. That plugin is released to the public and everything is normal
3. Down the road, if the developer for some reason wants to put a backdoor in the plugin, they can push another snapshot to the maven repo with the same versioned snapshot and that would start being pulled instead of the non-backdoored (or even consider someone could manually replace the jar being downloaded)

So this is why jar hashing should be encouraged, if not required, due to the fact that this would open the door to potential problems down the road.

[electronicboy]

jar hashing only works when you're using release dependencies or hard-versioned snapshots, so that would be more a false sense of security than anything else; Sadly, there comes some level of risks when allowing devs to pull stuff from random sources, but, it's not like plugins aren't already doing this sorta thing, etc; I think adding validation makes sense, but, it's not a magical solution for anything

I do think that one thing though is that we really should have the maven repo as part of the libraries path if we don't already, just because I know many people whom have their own repos building stuff which others also build into their own repos which could create some conflicts if we're not already