Skip to content

Add permission check for spawn protection#10335

Closed
TheMeinerLP wants to merge 1 commit into
PaperMC:masterfrom
TheMeinerLP:feature/add-permission-check-for-spawn-protection
Closed

Add permission check for spawn protection#10335
TheMeinerLP wants to merge 1 commit into
PaperMC:masterfrom
TheMeinerLP:feature/add-permission-check-for-spawn-protection

Conversation

@TheMeinerLP
Copy link
Copy Markdown
Contributor

Problem:
If you want to allow builders or other members to build at the spawn but don't want to give them an operator. So Vanilla does not currently offer any possibility.

Solution:
Without doing anything hacky, the PR adds a configuration option to Misc on Paper which overrides this problem in combination with a permission.

Test possibility:

  • 2 players join the server
  • One of two gets OP, player 2 can no longer mine within the spawn protection

Here is the JoinEvent test how to test this:

    @EventHandler
    public void onJoin(PlayerJoinEvent event) {
        PermissionAttachment permissionAttachment = event.getPlayer().addAttachment(this);
        permissionAttachment.setPermission("bukkit.environment.spawnprotection.ignore", true);
    }

@TheMeinerLP TheMeinerLP force-pushed the feature/add-permission-check-for-spawn-protection branch from 5fd843a to 3ac6c93 Compare March 17, 2024 19:41
@TheMeinerLP TheMeinerLP marked this pull request as ready for review March 17, 2024 19:42
@TheMeinerLP TheMeinerLP requested a review from a team as a code owner March 17, 2024 19:42
@lynxplay
Copy link
Copy Markdown
Contributor

What is the gain of this over just disabling spawn protection and e.g. locking it down with world guard?

@TheMeinerLP
Copy link
Copy Markdown
Contributor Author

TheMeinerLP commented Mar 17, 2024

To keep the vanilla style and not be dependent on 2 plugins.
In the problem section it is described that it is specifically intended for vanilla without having to build up your vanilla server.

Comment thread patches/server/1057-Add-permission-check-for-spawn-protection.patch Outdated
@electronicboy
Copy link
Copy Markdown
Member

Not fond of the perm node, we're not bukkit, it's it's a bypass, not an ignore
Configuration file changes go into the configuration patch
and the comment needs updating

I really dislike the notion of having a 1 liner patch for based around mechanism we generally do not encourage people to use, bleh

Update paper comment for references
@TheMeinerLP TheMeinerLP force-pushed the feature/add-permission-check-for-spawn-protection branch from 3ac6c93 to 5424ec2 Compare March 17, 2024 19:57
@lynxplay
Copy link
Copy Markdown
Contributor

Yea I mean "vanilla does not have that ability" I guess is true, but this isn't vanilla and spawn protection is far from a best practice to use when trying to limit build access. It doesn't have a flag system like world guard nor can you deploy it in different regions.

It is also easily bypassed by pistons, had weird quirks like not being active without an operator etc.
Mob griefing also iirc works in there? It's far far from a good solution for servers. A bandaid permission that people can use when the proper way is a protection plugin seem more like a false sense of security than anything.

@TheMeinerLP
Copy link
Copy Markdown
Contributor Author

The goal is not to be the "best solution", but simply to give players the opportunity to build on the spawn without having to come straight to them with operators.
Operators could then ban players or do other things. In this sense, a permission here would make more sense than coming around the corner with the operator or the whole plugin horde.

@TheMeinerLP
Copy link
Copy Markdown
Contributor Author

I would like to point out that either nobody is allowed to have OP or the spawn protection must be set to 0. There is no in-between. This would be a way to allow this intermediate path and keep the server vanilla as much as possible. Because WorldEdit/FAWE and WorldGuard on top only to allow people to build on spawn via "flags". I think that's excessive from my admin and dev point of view

@lynxplay
Copy link
Copy Markdown
Contributor

I mean, this way you are adding a permissions plugin into the mix.
Something that may also not be present on the "vanilla" server. The plugin "horde" is now 1 plugin instead of 2. Still with the same previously stated issues and now you install one permissions plugin instead of two (WG + WE) to allow builders to make use of a barely functioning protection mechanism.

I digress however, while I personally don't care for this patch, other members might so we can leave this up for discussion.

@lynxplay
Copy link
Copy Markdown
Contributor

With a heavy heart, I'll close this PR after some more discussion.
The spawn protection logic exists, but is by far enough to protect your spawn.

If you want builders to be the only ones building and changing your spawn, you'll need a proper plugin like WG anyway.
The fact that WG requires WorldEdit is, to that degree, annoying and I am unsure if you can lobby them to bundle some form of "minimal region selection" bs, but it doesn't change the fact that this permission flag is not useful for anything beyond the false hope that spawn protection is a proper region protection concept.

This patch makes sense in forks that care for such a niche usecase on their specific server setup, but on the broad spectrum that is paper I don't think it is worth adding 👍

@lynxplay lynxplay closed this Mar 20, 2024
@TheMeinerLP
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback. I will hold it in my own fork

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

Successfully merging this pull request may close these issues.

4 participants