Slimefun

Slimefun

Bukkit Plugins
3M Downloads

Merge pull #3431 broke ProtectionManager's API.

OmerBenGera opened this issue · 1 comments

commented

❗ Checklist

  • I am using the official english version of Slimefun and did not modify the jar.
  • I am using an up to date "DEV" (not "RC") version of Slimefun.
  • I am aware that issues related to Slimefun addons need to be reported on their bug trackers and not here.
  • I searched for similar open issues and could not find an existing bug report on this.

📍 Description

After merge request #3431 was accepted, the registerModule method's parameters were changed. Instead of having Server as the first parameter, you changed it to PluginManager, which broke the API.

I am asking you if it's possible to prevent such changes in the future, as these changes break plugins that support for Slimefun, and these changes require us to add a lot of workarounds in order to support different versions of the plugin.
For this example, you could just add another method that requires PluginManager as a parameter, and make the method's implementation that has a Server as a parameter to simply call the new method - this way nothing would break.

I know it's technically not a bug report, however I still see how it can be "fixed" or "treated" as a bug.

📑 Reproduction Steps

💡 Expected Behavior

📷 Screenshots / Videos

No response

📜 Server Log

No response

📂 /error-reports/ folder

No response

💻 Server Software

Spigot

🎮 Minecraft Version

1.18.x

⭐ Slimefun version

After merge request #3431

🧭 Other plugins

No response

commented
  1. This is not a "bug", please only use the Bug Tracker to report bugs. For other kinds of communication, use discord.
  2. This is the "Slimefun4" repository, you seem to be looking for dough
  3. Method parameters can change, the ProtectionManager is not primarily meant to be used from outside of dough.

Aside from that, I actually didn't notice this method was public in the first place. I could have sworn the one with Plugin as a parameter was. If not, then that will likely be corrected in the future. We can of course also add back the one with Server as a parameter.
Or you could just make a PR for it.

But please post these things on the right bug tracker next time or contact us on other ways.