Re: ✨ add t+-.tpll.newchunks permission node for non-generated chunks - #91
Re: ✨ add t+-.tpll.newchunks permission node for non-generated chunks#91MaxLananas wants to merge 8 commits into
Conversation
Zoriot
left a comment
Zoriot
left a comment
There was a problem hiding this comment.
Other than that LGTM & it works fine.
|
Following the discussion in #74 and feedback from @Zoriot and @Agrrox, I reworked the approach. Original design had Changelog
Permissions
|
| * linked worlds are checked, because the final destination depends on the computed height and may | ||
| * differ from {@code world}. | ||
| */ | ||
| private static boolean isChunkGeneratedInAnyRelevantWorld(int chunkX, int chunkZ, @NonNull World world) { |
There was a problem hiding this comment.
Can you clarify why this method is needed at all? Instead, you could also Just add the check inside getHeightFromMapsAndTeleportIfThere or not?
| @@ -32,6 +36,8 @@ paper-api = { module = "io.papermc.paper:paper-api", version.ref = "paper-api" } | |||
| pluginupdater-paper = { module = "org.lushplugins.pluginupdater:updater.paper-api", version.ref = "pluginupdater" } | |||
| pluginupdater-common = { module = "org.lushplugins.pluginupdater:updater.common-api", version.ref = "pluginupdater" } | |||
| terraminusminus = { module = "net.buildtheearth.terraminusminus:terraminusminus-bukkit", version.ref = "terraminusminus" } | |||
| jspecify = { module = "org.jspecify:jspecify", version.ref = "jspecify" } | |||
| jetbrains-annotations = { module = "org.jetbrains:annotations", version.ref = "jetbrains-annotations" } | |||
There was a problem hiding this comment.
You currently have a lot of unrelated changes in your pr. Please remove these change and rebase / merge (i prefer a rebase and force push in such cases)
You should also not remove comments or change the formatting of code which is not related to your changes.
If you need help with that lmk
| register("t+-.admin") { | ||
| description = "Grants all Terraplusminus permissions" | ||
| default = BukkitPluginDescription.Permission.Default.OP | ||
| children = mapOf( | ||
| "t+-.tpll" to true, | ||
| "t+-.forcetpll" to true, | ||
| "t+-.where" to true, | ||
| "t+-.offset" to true, | ||
| "t+-.distortion" to true, | ||
| "t+-.notify.update" to true | ||
| ) | ||
| } |
There was a problem hiding this comment.
You have now changed the behaviour of t+-.admin without documenting that inside of the Readme.
I'm generally fine with introducing permpacks, please then also supply a builder and visitor permpack and i think a permission node like t+-.permpack.visitor would make more sense.
Then the permission behaviour is also not changed.
| } | ||
| register("t+-.tpll") { | ||
| description = "Allows usage of /tpll" | ||
| default = BukkitPluginDescription.Permission.Default.TRUE |
There was a problem hiding this comment.
Please don't change the behaviour with defaulting to true. This is unexpected tp server owners and a breaking change we shouldn't do yet.
If you think that make long term sense please make a issue and we will incude that in v2.
There was a problem hiding this comment.
Your commits are poorly named. Please make proper commits out of it. If you need help lmk.
It seems like you are not using IntelliJ. That's generally fine though I may not be familiar with your setuo then.
2 participants
Implements the permission node
t+-.tpll.newchunksrequested in #74.Players without this permission can still use
/tpllto already-generated areas, but cannot trigger new chunk generation. Useful for servers where Visitors should be able to navigate built areas without accidentally loading new terrain.Changes
TPLL_NEW_CHUNKStoPermission.javaTpllCommand#execute()usingWorld#isChunkGenerated()Permission
t+-.tpll.newchunksfalse/tpllto ungenerated chunksNotes
t+-.admin) bypass the check implicitly via their permission setup