Support multiple loaders for plugin updates via Modrinth API#288
Conversation
|
@Zoriot Thanks for the PR, before I go deeper into it, do you mind explaining your issue in more detail and how exactly this fixes it? |
| return res; | ||
| } | ||
| public SearchResult searchUpdatePlugin(MinecraftPlugin plugin, String mcVersion) { //TODO: probably don't hardcode spigot and papermc | ||
| return searchUpdate("spigot\",\"paper", plugin.getModrinthId(), mcVersion, plugin.getInstallationPath(), false); |
There was a problem hiding this comment.
@Osiris-Team Plugin updating worked only for spigot/paper because it was hardcoded here. Now it also works for velocity and specific paper forks like Folia. It get's the loader either via server-updater.loader or if smt is specified plugins-updater.loaders where you can specify multiple ones.
There was a problem hiding this comment.
Okay it seems counterintuitive to be able to provide multiple mod loaders from a user-perspective, because usually the server only has one loader, thus I'd like to keep these options away from the user and under the hood as API implementation details.
Instead it would be great to enhance the automatic detection in InstalledModLoader.java to include the detection for new mod loaders.
There was a problem hiding this comment.
When you want a general logic it can be complicated quite quickly, e.g you would have only supply folia for folia, Purpur > Paper > Spigot for Purpur.
Not sure if that's really feasible in that PR.
It would be nice when it correctly updates FAWE through with the paper build on paper.
What's your (simple) idea for that?
There was a problem hiding this comment.
I also wonder if you are open in merging that and creating an issue for moving that to internal again. With proper handling
There was a problem hiding this comment.
@Zoriot I'd like to see this implemented in this PR directly 👍. I'm not sure the implementation is that complicated. We could even remove or simplify the auto detection to instead just check what the user entered in the server-updater software section of the config, and use that to determine the compatible mod loader.
There was a problem hiding this comment.
Understandable. What's your proposed solution to make preferred option/handling possible?
Add a (optional) fiel to the field where we safe the data about plugins? E.g. https://modrinth.com/plugin/fastasyncworldedit/versions needs that, on a Paper (Fork) it should always install the paper version not the spigot version.
There was a problem hiding this comment.
@Zoriot Why not just install the best match automatically? For example If the server is paper we prefer paper mod/plugin builds and only fallback on spigot if needed.
| plugins_updater_loaders = put(name, "plugins-updater", "loaders") | ||
| .setComments("The list of software's to update minecraft plugins.", | ||
| "If left empty, taken from server-updater.software above. I multiple values are specified it will prioritize the fist one.", | ||
| "Currently only relevant for Modrinth plugins."); |
There was a problem hiding this comment.
Is there an API endpoint that returns the available loaders that we can include here, so the user can view a valid list of available loaders to use?
There was a problem hiding this comment.
There is https://api.modrinth.com/v3/tag/loader but without parsing & filtering it's not shown nicely.
Probably specifying manually would make sense if we go that route
| return res; | ||
| } | ||
| public SearchResult searchUpdatePlugin(MinecraftPlugin plugin, String mcVersion) { //TODO: probably don't hardcode spigot and papermc | ||
| return searchUpdate("spigot\",\"paper", plugin.getModrinthId(), mcVersion, plugin.getInstallationPath(), false); |
There was a problem hiding this comment.
Okay it seems counterintuitive to be able to provide multiple mod loaders from a user-perspective, because usually the server only has one loader, thus I'd like to keep these options away from the user and under the hood as API implementation details.
Instead it would be great to enhance the automatic detection in InstalledModLoader.java to include the detection for new mod loaders.
Needed to make updating from modrinth work the right way.