AtriusX / Waystones

A small plugin that brings long-distance teleportation to Minecraft survival mode!
https://www.spigotmc.org/resources/waystones.93917/
11 stars 6 forks source link

Check Building Perms before Linking #4

Closed luna-skye closed 3 years ago

luna-skye commented 3 years ago

It would be nice to require the ability to build in order to link, this would prevent not only linking within lobbies/hubs of servers, but also stop linking to someone's waystone in a house claimed through some grief prevention plugin.

While I was hoping this could be done through linking into something such as Vault, it doesn't seem to support this specific type of permission, as it seems most plugins just cancel/override the event within themselves internally based on various things that aren't always so externally accessible, such as WorldGuard checking a specific flag within the region the player tries to place a block.

The most reliable way to do this without hardcoded support for certain plugins seems to be through a simulated block place event. When linking, simulate the placement of a block at the location of the waystone, if the placement was cancelled (likely by some other plugin) then they're not able to build there and the LinkEvent fails, likely with some sendErrorMessage being called to the inform the player they don't have permission.

I found a small forum post that briefly suggests this, I'll link it once I find it tomorrow.

AtriusX commented 3 years ago

It's an interesting proposition; I think we will need to consider it, but the level of hackiness of the solution has me a bit concerned about overall stability. Relying on events in such a passive way could be somewhat unpredictable if another plugin executes on the same event after ours.

With that said I don't currently have an alternative to this. We will need to review other approaches later on.

luna-skye commented 3 years ago

It does feel very hack-y and more of a sort of workaround, but we might be able to do something regarding Event priority or weight? I had found another post about listening to a BlockPlaceEvent, and setting priority there to define it should be executed last, although in the Javadocs I can't find anything regarding priority.

Although it does seem there's an [isCancelled method](https://hub.spigotmc.org/javadocs/bukkit/org/bukkit/event/block/BlockPlaceEvent.html#isCancelled()) on BlockPlaceEvent, which should return true if the event was cancelled at all, not sure how reliable this is and it'd still require simulating block placement it seems, but I might look into testing this.

Here's the original post that I found where I got the idea from. Someone does reference a part of the WorldGuard API that checks this relatively easily, but that'd require depending on WorldGuard. Short of hooking into every plugin that could possibly control block placement permissions, I feel this is the best bet, sadly.

AtriusX commented 3 years ago

BlockPlaceEvent does have an event.isCancelled() method in java, however kotlin simplifies this to property referencing via event.isCancelled. Priority flags are a way to tackle the problem but that still doesn't guarantee that another plugin won't execute after ours if they set their priority to the same level.

Maybe a better approach is to offer direct support for common plugins like Worldguard in addition to a secondary system that would allow other plugins to provide protected regions to us. Currently I do not have a solid idea of how we could implement this system, either through the introduction of a ProtectionService class and/or extra events perhaps?

AtriusX commented 3 years ago

This issue should be fixed now. Future plans for region or user based permissions can be considered later on.