search

PaperMC / PaperLib

Plugin Library for interfacing with Paper Specific API's with graceful fallback that maintains Spigot Compatibility, such as Async Chunk Loading.
MIT License
267 stars 32 forks source link

Find a better approach to detecting the Environment #53

Open A248 opened 2 years ago

A248 commented 2 years ago

Currently, PaperLib checks for the presence of environment-specific configuration classes, namely com.destroystokyo.paper.PaperConfig and org.spigotmc.SpigotConfig. This may work at the moment, but it presents possibilities for environment-detection to malfunction should these internal classes ever be refactored. Neither PaperConfig nor SpigotConfig are exposed in their respective APIs and may be removed in a later release by either development team, that of Paper or Spigot.

Because PaperLib is most frequently shaded into dependent plugins, the current latest version of PaperLib will likely continue to be in use for a long time, living on in relocated classes. As such, the mechanisms PaperLib uses to determine the environment should be somewhat future-proof, so as to provide some stability when updating server versions and reduce the need to recompile and re-shade every plugin with the latest PaperLib version.

What's more, and this is not something PaperLib is responsible for but still pertinent, PaperLib often serves as a kind of unofficial reference implementation for checking whether Paper is in use. It's often recommended that users look at the PaperLib source code if they wish to implement Paper-specific features. PaperLib should ideally do so by the best means possible which would provide a good example to these developers.

electronicboy commented 2 years ago

It should probs really just look for the required features rather than just assuming based on the env, I think determining entirely based on environment is somewhat flawed as an overall strategy

BlackBeltPanda commented 1 year ago

The current impl does cause issues when a server's running paper but the method accessed doesn't exist: https://github.com/Slimefun/Slimefun4/issues/3649

electronicboy commented 1 year ago

~~That's not a running paper server, that method exists in paper, and has for years: https://jd.papermc.io/paper/1.19/org/bukkit/block/Block.html#getState(boolean)~~

Nevermind, that's nothing to do with paper, that's a compatibility issue with transportpipes not implementing methods used in paper, thus causing issues plugins due to the induced API contract violation, there's not much paperlib can do about this without tryna catch such exceptions and fall back to just returning a standard block state

BlackBeltPanda commented 1 year ago

As far as I can tell, there's no simple way to implement those methods in TransportPipes without breaking Spigot compatibility. If you do know of a method, please let me know; I'd love to get this fixed. =)

electronicboy commented 1 year ago

if you don't care about supporting the faster state snapshots, you can just create a method which delegates to the standard method and leave out the override annotation; ideally, you'd made that method call the proper method when running in paper, i.e. using reflection if needs be

BlackBeltPanda commented 1 year ago

Great, I'll give that a try, thank you =)