TechnicallyCoded / FoliaLib

This is a wrapper library for aiding in supporting the Folia Paper Fork. This library adds multiple scheduler options to use instead of the Bukkit or Folia native schedulers.
MIT License
70 stars 11 forks source link

Change JavaPlugin to Plugin #26

Closed failutee closed 1 month ago

failutee commented 3 months ago

Please, I'm begging you 😭

TechnicallyCoded commented 3 months ago

Please explain your reasoning

failutee commented 3 months ago

Please explain your reasoning

There is no major reason, simply compatibility with other projects, everyone uses the Plugin interface and today, when I wanted to use your lib, I was surprised when I had to make a slight change and use JavaPlugin instead of Plugin

jquery-package commented 1 month ago

+1, I find this change quite useful too. When refactoring lots of legacy plugins I had to cast to JavaPlugin without any reason. With this change it will be just simpler to use and give more freedom to the developers. Can't see any reason to keep the only one constructor, we can add another one with Plugin

jquery-package commented 1 month ago

@failutee Would you mind to update this PR to make two constructors? For Plugin and JavaPlugin to maintain compatibility. If not, I'll make a separate PR with those changes

TechnicallyCoded commented 1 month ago

@failutee Would you mind to update this PR to make two constructors? For Plugin and JavaPlugin to maintain compatibility. If not, I'll make a separate PR with those changes

Unless someone is using reflection to instantiate FoliaLib, Plugin being a supertype of JavaPlugin should allow it to just work with no modifications to constructors. Did I miss something?

jquery-package commented 1 month ago

Unless someone is using reflection to instantiate FoliaLib, Plugin being a supertype of JavaPlugin should allow it to just work with no modifications to constructors. Did I miss something?

Nah, this will change byte code and cause NoSuchMethodError: java.lang.NoSuchMethodError: 'void xx.folialib.FoliaLib.(org.bukkit.plugin.java.JavaPlugin)'

TechnicallyCoded commented 1 month ago

You would be recompiling with the library attached. There is no case in which you compiled the plugin with folialib of a different version (idk how that would happen). The relocations that are required avoid this exact issue. Any dev that relocates the lib will not be interacting with anyone else's loaded classes.

jquery-package commented 1 month ago

You would be recompiling with the library attached. There is no case in which you compiled the plugin with folialib of a different version (idk how that would happen). The relocations that are required avoid this exact issue. Any dev that relocates the lib will not be interacting with anyone else's loaded classes.

That's right, but that error is possible if I compile FoliaLib as independent plugin to use it as a library for another plugins, so I don't have to shade it every time

TechnicallyCoded commented 1 month ago

It's not a plugin nor an API, and has no intention of being so, such behavior will not be supported.