Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find a better approach to detecting the Environment #53

Open
A248 opened this issue Aug 1, 2021 · 6 comments
Open

Find a better approach to detecting the Environment #53

A248 opened this issue Aug 1, 2021 · 6 comments

Comments

@A248
Copy link

A248 commented Aug 1, 2021

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
Copy link
Member

electronicboy commented Aug 1, 2021

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
Copy link

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

@electronicboy
Copy link
Member

electronicboy commented Aug 13, 2022

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
Copy link

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
Copy link
Member

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants