search

elBukkit / PreciousStones

Self-service protection for Minecraft servers
http://dev.bukkit.org/server-mods/preciousstones/
10 stars 10 forks source link

Load needed chunks in TeleportationManager asynchronously #59

Closed A248 closed 3 years ago

A248 commented 3 years ago

Addresses #57

This PR uses PaperLib to load chunks asynchronously before checking the safety of the location and teleporting players.

I noticed a misconception about PaperLib here:

If anyone wants to make a PR to add this enhancement feel free, the main objective is to also keep compatibility with spigot so it must work with both. When I have time, Ill have a look into how to add this (as I have never added any custom paper stuff into a plugin)

Contrary to what the comment implies, this PR does NOT make PreciousStones require Paper to run. It will still run on Spigot as normal. In fact, this is a good example of the crucial distinction between a library and an API - If PreciousStones used the Paper API, it would only be able to run on Paper. Instead, PreciousStones will use PaperLib, which has support for both Spigot and Paper.

PaperLib detects the environment (Spigot or Paper) which the server is running and, accordingly, uses different behaviour. On Spigot, these improvements do not exist, so it functions just as if Player.teleport was called. On Paper, however, it will use async chunk loading.

The PaperLib dependency is shaded and relocated into PreciousStones using the maven-shade-plugin. The dependency is marked optional because it should not be transitive.

This is the first time I have used PaperLib, nor am I familiar with PreciousStones, so a code review would be appropriate, as reviews often are.

xtomyserrax commented 3 years ago

Thank you so much for this PR! Ill have a look at it as soon as I have time and Ill test it.

And thanks also for the explanation about how Paper works!

xtomyserrax commented 3 years ago

Im getting this NoClassDefFoundError in both spigot and paper, maybe there is something wrong with the pom file? Im not really experienced with Maven https://pastebin.com/yBjUecRe

Maybe @NathanWolf knows. My intention is to test how this works, if it works fine, then we will definitely move to this async method

A248 commented 3 years ago

Very sorry about that @xtomyserrax . I made a silly error and messed up the maven-shade-plugin configuration. It's corrected now.

xtomyserrax commented 3 years ago

I just tested it with that fix and works perfectly in both spigot and paper.

I will leave this up to @NathanWolf in case he has anything to say. If he agrees, Ill change the version number and fix another bug and merge it!

Thanks!

A248 commented 3 years ago

Added yet one more improvement.

xtomyserrax commented 3 years ago

Thank you so much for your collaboration! I hope we get more PRs from you!