Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paper Plugin Future Planning #8859

Closed
4 of 6 tasks
Owen1212055 opened this issue Feb 18, 2023 · 40 comments
Closed
4 of 6 tasks

Paper Plugin Future Planning #8859

Owen1212055 opened this issue Feb 18, 2023 · 40 comments
Labels
type: feature Request for a new Feature.

Comments

@Owen1212055
Copy link
Member

Owen1212055 commented Feb 18, 2023

This is created to help forward possible more conversation and ideas that may be prompted under the whole new "paper plugin" umbrella.
With the paper plugin PR released, this allows us to ensure that we are able to remain compatible with previous spigot plugins.

These types of plugins are currently experimental, all API regarding it is subject to change.
Currently, paper plugins won't be offered with much new API, as those are still in the works.

This allows us to prompt for conversation for possible future new features:

General TODO

  1. build-pr-jar
  2. build-pr-jar
  3. 15 of 16
@Andre601
Copy link

My suggestion is a more direct support for bstats, so that plugin devs won't have to shade and relocate it into their plugin.

@Brokkonaut
Copy link
Contributor

What I would like to have is a change in the plugin loading system, so when a plugin cannot be loaded or enabled for any reason the server should not start at all. reason: a plugin that cannot be enabled is always a severe misconfiguration of the server and can easily cause additional damage if the server starts anyway.

@Andre601
Copy link

Something else that imo could deserve a rework is the api-version option.
Feels for me a bit annoying that you can only define a single API version rather than multiple ones (Given that your plugin isn't using stuff depending on one version).

Having the server yell that "Plugin X is made with an older version" or something like that because the dev keeps the api version low for backwards compat is a bit annoying...

So maybe make it a list? Or allow some format similar to mods where you f.e. can have >=1.19 defined?

@electronicboy
Copy link
Member

The notion of defining an API version is pointless

a) The entire system as it stands is practically just a boolean check for if it's defined or not, there is no branching there

b) Why should we determine what API versions the server determines as compatible? When you look at stuff like vault eco, the entire notion of API compatibility is stupid (especially defined by the server), given that they don't even really touch anything inside of bukkit outside of the services API stuff

c) The entire system is highly redundant, while maybe having some "built against" might be nice for future hacks, there is no usage for it outside of hacks, I'm not sure that the new system even really cares about that?

@Andre601
Copy link

The notion of defining an API version is pointless

a) The entire system as it stands is practically just a boolean check for if it's defined or not, there is no branching there

b) Why should we determine what API versions the server determines as compatible? When you look at stuff like vault eco, the entire notion of API compatibility is stupid (especially defined by the server), given that they don't even really touch anything inside of bukkit outside of the services API stuff

c) The entire system is highly redundant, while maybe having some "built against" might be nice for future hacks, there is no usage for it outside of hacks, I'm not sure that the new system even really cares about that?

Well, then either remove it, or perhaps make it kinda the oposite way of how it works?
Like allow a plugin dev to tell the server "Hey, this plugin is not supporting version(s) X(, Y, Z), so don't load it on those".
I feel like this could be a good idea to consider as a plugin may use features only present in more recent releases (or the oposite that a feature was removed and the plugin no longer works on those).

Right now this would obviously be pointless as that check wouldn't work on older Paper releases (Unless people would retroactively patch this into their 1.8 forks :P) obviously, but for the future could it be an interesting thing to have as not everyone has any clue how to obtain the current server version and check it against their supported versions...

@Andre601
Copy link

Something else that just came to mind: What about a change to the authors option to allow adding a personal link, description, etc?

Like f.e.

authors:
  - name: MainAuthor
    description: Core maintainer of the plugin
    website: https://example.com
  - name: SomeTranslator
    description: Provides translations

That info could then be included in /version <plugin> through hover text for the listed authors...

I think that could be useful in giving people proper credit for what they do instead of just being names in a list only.

@Akiranya
Copy link

I hope paper-plugins.yml can support the commands entries just like in the plugin.yml 🚀

it would make the migration to paper-plugin.yml easier

@Machine-Maker
Copy link
Member

Machine-Maker commented Feb 21, 2023

paper-plugin.yml is still very much in the experimental phase. I don't think we are expecting everyone to move to it now. commands will never be a part of it however. Commands will be registered at runtime using an upcoming brig API or currently you can manually extend Command/PluginIdentifiableCommand in a class and register your commands to the commandmap yourself.

@Andre601
Copy link

currently you can manually extend Command/PluginIdentifiableCommand in a class and register your commands to the commandmap yourself.

How would this look like in terms of code? 🤔

@Phoenix616
Copy link
Contributor

commands` will never be a part of it however. Commands will be registered at runtime using an upcoming brig API or currently you can manually extend Command/PluginIdentifiableCommand in a class and register your commands to the commandmap yourself.

Please add the ability to output all commands, usages and permissions in a human readable and obvious way then. Definining commands via the plugin.yml has the major benefit that users can just open the jar with any zip program and check the plugin.yml for the commands and permissions a plugin has. (And yes, I know a good chunk of people that do that, it's not just me)

Of course this breaks down in more complex setups and parts of it can already one by other plugins (e.g. easily getting all permissions with LuckPerms) but having an inbuilt way to get such info (potentially even on a sub command/command argument basis) would be pretty nice.

@electronicboy
Copy link
Member

The big issue is "how do we do that"

The entire reason that we don't wanna expose commands inside of the new system is that they'd be tied to bukkits dated command system which has far outstayed its welcome; I guess we could aim to have plugins provide some form of meta when they ask to register a command, but, idk how we'd expose that to users

@maestro-denery
Copy link
Contributor

So, my suggestions are:

  1. LevelStem API, WorldCreator API enhancements / a new API for the world creation. CraftServer#createWorld(WorldCreator) method does a lot of stuff internally which isn't configurable, especially the World.Environment enum which is internally converted to the LevelStem, which contains Holder<DimensionType>, which is configurable by datapacks and is synchronized with the client, I think we can use EnumCreators from my PR there as well, but tbh I think it's better to make a new API provided by a PluginProviderContext so created dimensions are created alongside datapack ones, and there will be configurable dimension types and more functionality.
  2. Tags API for the Registry API. For now my PR doesn't have Tag support, mainly because it's an another ton of work to do and generally is not as important as other stuff. I think when the Brigadier API is done or any other API which supports reloaded content I can do the Tag support.
  3. FeatureFlags API. It's a bit controversial, feature flags are stored in the WorldDataConfiguration -> LevelSettings -> PrimaryLevelData -> ServerLevel. It means they're per world, all checks based on them are made using the ServerLevel#enabledFeatures(), but it refers to the main PrimaryLevelData stored in the MinecraftServer, we can make this method return enabled feature of its own PrimaryLevelData, make ClientboundUpdateEnabledFeaturesPacket send to the clients when they cross dimensions, (client handler doesn't have any restrictions on sending it multiple times) so we can have per world feature flags and an API to create worlds with them / configure them for already existing worlds.

@kashike kashike pinned this issue Mar 6, 2023
@Andre601
Copy link

Andre601 commented Mar 6, 2023

Well, then either remove it, or perhaps make it kinda the oposite way of how it works? Like allow a plugin dev to tell the server "Hey, this plugin is not supporting version(s) X(, Y, Z), so don't load it on those". I feel like this could be a good idea to consider as a plugin may use features only present in more recent releases (or the oposite that a feature was removed and the plugin no longer works on those).

Right now this would obviously be pointless as that check wouldn't work on older Paper releases (Unless people would retroactively patch this into their 1.8 forks :P) obviously, but for the future could it be an interesting thing to have as not everyone has any clue how to obtain the current server version and check it against their supported versions...

I'm still in favour of adding some kind of way for a plugin dev to either define supported or unsupported server versions for their plugin. Maybe even extend this to dependency declarations?

It would allow an easier way of telling the user that plugin X doesn't work on server version Y or with plugin version Z, as those versions may lack required API methods a dev either has to make complicated workarounds for (i.e. NMS access) or not use at all.

For plugins that don't declare these versions should it just assume you accept any versions.

And if doable, allow declarations like >=1.19 to tell the server your plugin supports versions bigger or equal to 1.19.

Yes, yes, right now there is no use because it's legit only one version (Plugin dependencies on the other hand would have a bigger use out of this already), but it's better to build for the future and have it.

@Phoenix616
Copy link
Contributor

The big issue is "how do we do that"

The entire reason that we don't wanna expose commands inside of the new system is that they'd be tied to bukkits dated command system which has far outstayed its welcome; I guess we could aim to have plugins provide some form of meta when they ask to register a command, but, idk how we'd expose that to users

Simply having it part of some command like /help <pluginname> could help a lot. That could just output information based on the possible brigadier argument combinations.

@MelonHell
Copy link

Wow, looks very interesting! the main question that worries me, will the connection of libraries be somehow improved?

Problem example:
I have a plugin that is written in Kotlin, if I load Kotlin through libraries in plugin.yml or pack it in jar, then when I try to interact with another Kotlin plugin, I will get a LinkageError

Current Solution:
Create a plugin containing Kotlin, for example let's call it KotlinPlugin and my first and second plugin should be dependent on KotlinPlugin

Solution I want:
Ability to specify libraries in paper-plugin.yml (ideally not just from mavenCentral) and have their classes shared between plugins that use them

@Owen1212055
Copy link
Member Author

@MelonHell Take a look at the PluginLoader system. https://docs.papermc.io/paper/dev/getting-started/paper-plugins

This allows you to dynamically add kotlin, where your classloader will only be open to plugins that you explicitly define in your paper plugin yml.

@Andre601
Copy link

Paper should log libraries loaded by a Plugin through its PluginLoader, because as of right now is that not the case, which has the potential risk that someone could load dangerous libs without the user noticing.

Questions are:

  • How should it be logged? Similar to Spigot?
  • Should transistive dependencies used by the main lib be included? Would it be spammy?

If doable, a message like Loading <groupId>:<artifactId>:<version> from <repoName>... should be given here, but it's up to you.

@Machine-Maker
Copy link
Member

I think that really should be left up to the implementation of ClassPathLibrary. We can add it to the JarLibrary and MavenLibraryResolver impls, but otherwise the only information we have for logging is a "path". Which is less than optimal for something loaded from maven which should include the coordinates in the log msg.

@Andre601
Copy link

I just personally want it to be logged in one way or another. Because without that, server owners wouldn't know what libraries are loaded by a plugin, which would be a security risk as a plugin could be hijacked, have malicious code in form of a lib added and then published as a legit release.

While I hate Spigot's rather spammy load messages are they at least transparent here and don't hide the loaded libs.
Paper should try to implement something similar. And if the current API doesn't support this... why not make your own (a fork) that would allow it? Because from what I gather are you using an Apacha lib for this?

@Andre601
Copy link

Something I find a little tedious right now is that you have to define load-before/load-after separately to the dependency declarations, which can be annoying.

If dependencies are just - name: Plugin entries is the job a simple copy-paste, but when you also have required or bootsrap defined can it be a bit more work to copy over the entries and then remove the stuff not needed.

I personally would prefer having a load or load-order option for a dependency entry to set the order there, as it would reduce what essentially is duplicated text.

As an example:

dependencies:
  - name: RequiredPlugin
    required: true
  - name: OptionalPlugin
  - name: RequiredPluginLoadBefore
    required: true
    load-order: before
  - name: RequiredPluginLoadAfter
    required: true
    load-order: after
  - name: RequiredPluginLoadBeforeBootstrap
    required: true
    load-order: before-bootstrap
  - name: RequiredPluginLoadAfterBootstrap
    required: true
    load-order: after-bootstrap

Obviously, this is a minor thing, but I still think having to define the same plugins in your load-after/-before again is a bit tedious to do and only makes the YAML file larger than it needs to be.

@Andre601
Copy link

Andre601 commented Mar 22, 2023

Can't recall where I mentioned that before (I think in the paper plugin PR and on Discord?) but a replacement of the /reload command and its broken logic with an event-based system (PaperPluginReloadEvent) would be good.

Essentially, when a server admin runs /reload would Paper dispatch this event to plugins listening for it, so that those can do whatever is necessary to do a "reload" like refresh data used, clear caches, etc.

"/reload shouldn't be used!"

Yes, yet people still use it as a restart can take several minutes depending on the world, plugins used, hard/software behind the server, etc. which for the simple updating of some configuration or whatever is annoying, not to mention would a restart kick players, which in case of a single server and not a network with fallback-servers means you possibly lose players.

This would imo be a good compromise to have here: People have their reload command while plugins won't be broken left, right and center by it... given they use the paper-plugin system.
(Spigot plugins would perhaps still have the broken system for a while, just to not break existing setups... for now)

@Jannyboy11
Copy link
Contributor

Jannyboy11 commented Mar 23, 2023

Will there still be a PluginLoader api along the lines of PluginManager#registerInterface? I maintain a custom implementation of org.bukkit.plugin.PluginLoader and the PaperPlugin update caused me to have to completely reengineer how plugins supported by my api are loaded on Paper (See Jannyboy11/ScalaPluginLoader#20). I would love to work with a stable api, instead of having to rely on server internals which I'm currently doing in order to support Paper.

@Akiranya
Copy link

Akiranya commented Jun 9, 2023

Please introduce reload capability for paper plugins. This feature is intended for development use only, not for production. Currently, small changes require a full restart or /reload, which is time-consuming. Most well-structured plugins can support this feature. To prevent misuse, implement a configuration barrier so only developers can access it. Many developers are using plugman, but it's incompatible with paper plugins.

I don't think it's a feature that should be introduced by a server platform. You might want to know "hot swap" which is a native feature of some JVM implementation (see JBR) where you can swap methods ,classes, etc. at runtime. IDEA also natively support hot swap feature

@electronicboy
Copy link
Member

reloading is not exactly a mechanism supported by mojang, as such, it's not something that can be nicely exposed outside of the limited contexts that vanilla allows.

Reloading random plugins is not supported by the plugin manager, because you can't just safely unload random plugins at runtime, you'd need to effectively tear down the entire dependendents tree, which just creates cause for more unpredictable behavior. This is not something that java itself supports nicely in the context of these environments, nor is it something we really desire to waste our time on. Not to mention: The entire point of reloading is that it reloads all data, that means that you need to clear the servers state and properly call into the bootstrapers of plugins to actually load data, something which doing on running plugins is probably going to create more issues than it solves.

@MelonHell
Copy link

Please introduce reload capability for paper plugins. This feature is intended for development use only, not for production. Currently, small changes require a full restart or /reload, which is time-consuming. Most well-structured plugins can support this feature. To prevent misuse, implement a configuration barrier so only developers can access it. Many developers are using plugman, but it's incompatible with paper plugins.

I don't think it's a feature that should be introduced by a server platform. You might want to know "hot swap" which is a native feature of some JVM implementation (see JBR) where you can swap methods ,classes, etc. at runtime. IDEA also natively support hot swap feature

I think that this should be implemented in the server software, and not by an external plugin. If it is done in paper it will work much more stable and use less reflection and crutches

@Andre601
Copy link

Andre601 commented Jun 9, 2023

I mentioned a possible solution for reload, which is having an event plugins can listen to, to then decide what to do with that.
That way, it's open to the plugin to decide what it wants to do on a reload and the command stays imo relatively safe, no?

@MelonHell
Copy link

reloading is not exactly a mechanism supported by mojang, as such, it's not something that can be nicely exposed outside of the limited contexts that vanilla allows.

Reloading random plugins is not supported by the plugin manager, because you can't just safely unload random plugins at runtime, you'd need to effectively tear down the entire dependendents tree, which just creates cause for more unpredictable behavior. This is not something that java itself supports nicely in the context of these environments, nor is it something we really desire to waste our time on. Not to mention: The entire point of reloading is that it reloads all data, that means that you need to clear the servers state and properly call into the bootstrapers of plugins to actually load data, something which doing on running plugins is probably going to create more issues than it solves.

maybe I'm developing plugins wrong somehow, but I always use /plugman reload instead of hotswap during development, since hotswap is very limited

now I have to restart the server literally every 5-10 minutes, which takes 5 minutes and discourages motivation

@electronicboy
Copy link
Member

you can use the jetbrains runtime + hotswap agent to drastically improve the hotswapping capabilities

@MelonHell
Copy link

I mentioned a possible solution for reload, which is having an event plugins can listen to, to then decide what to do with that.
That way, it's open to the plugin to decide what it wants to do on a reload and the command stays imo relatively safe, no?

this solution will not help, since it will not differ in any way from the custom /myplugin reload. we need to reload the entire jar file

@Andre601
Copy link

Andre601 commented Jun 9, 2023

That's a you-problem honestly.
I for my part have no issues stoping my server, putting new jar in, delete old one and start server again.

Developing stuff can be time consuming and may not be easy, so why do you want a fundamentally broken system here?

@MelonHell
Copy link

this system does not prevent me from restarting the server in the same way, but when I know that my plugin does not break in any way from this "broken system" I can speed up development several times

@tal5
Copy link
Contributor

tal5 commented Jun 9, 2023

this system does not prevent me from restarting the server in the same way, but when I know that my plugin does not break in any way from this "broken system" I can speed up development several times

But you don't know that, the whole point is that it breaks in entirely unexpected ways and causes weird issues, especially with newer plugin API (I.e. would it re-call PluginBootstrap#createPlugin? that might cause even more unexpected issues with bootstrap code executing at weird times)
I see no reason for Paper devs to put time into creating and maintaining a fundamentally broken system, especially not when there are alternatives such as hot reload/just restarting

@Andre601
Copy link

Andre601 commented Jun 9, 2023

Additionally, this is - again - a you-problem.
Just because it would work for your plugin doesn't mean it wouldn't break other plugins 6 ways to sunday.

And don't give me the excuse about it being a config option or even env var for devs only.
People will find and use it because they can and because they are lazy.
It would break plugins, especially those more complex than others.

@MelonHell
Copy link

Additionally, this is - again - a you-problem.
Just because it would work for your plugin doesn't mean it wouldn't break other plugins 6 ways to sunday.

And don't give me the excuse about it being a config option or even env var for devs only.
People will find and use it because they can and because they are lazy.
It would break plugins, especially those more complex than others.

what's the problem with disabling this feature by default and putting it in the config with a huge warning?

@Malfrador
Copy link
Member

Additionally it's probably straight up impossible to make this work with the upcoming registry modification API.
A plugin being reloaded would try to redo it's registry modifications. At that point, the registries will be frozen however because the server is already running.
And considering several registries are synched with the client, there is no way around that (other than disconnecting all players on reload, which effectively would be a restart lol).

And with more and more things being backed up by a registry this means that a pretty significant part of future plugins won't be able to be reloaded anyways, making a reload feature even less useful.

@shubham8550
Copy link

now that paper got its one plugin respository

it will be great if they can make MPM -minecraft plugin manager

it ill be great specially mpm update will save pain
and versioning will be better

@electronicboy
Copy link
Member

There already is such software, no reason for us to create such a thing (nor is it relevant to this PR)

@Andre601
Copy link

Andre601 commented Jun 9, 2023

What kind of thing? Never heard of that (And I get the feeling it would be a useless thing anyways)

@electronicboy
Copy link
Member

electronicboy commented Jul 5, 2023 via email

@PukPukov
Copy link

PukPukov commented Apr 27, 2024

That's a you-problem honestly.

I just wont use paper plugins, lol.

@PaperMC PaperMC locked and limited conversation to collaborators Apr 28, 2024
@CodeByCam CodeByCam converted this issue into discussion #10561 Apr 28, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
type: feature Request for a new Feature.
Projects
None yet
Development

No branches or pull requests