PaperLib icon indicating copy to clipboard operation
PaperLib copied to clipboard

Find a better approach to detecting the Environment

Open A248 opened this issue 2 years ago • 6 comments

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.

A248 avatar Aug 01 '21 10:08 A248

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

electronicboy avatar Aug 01 '21 10:08 electronicboy

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

BlackBeltPanda avatar Aug 13 '22 22:08 BlackBeltPanda

~~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

electronicboy avatar Aug 13 '22 22:08 electronicboy

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. =)

BlackBeltPanda avatar Aug 13 '22 22:08 BlackBeltPanda

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

electronicboy avatar Aug 13 '22 22:08 electronicboy

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

BlackBeltPanda avatar Aug 13 '22 23:08 BlackBeltPanda