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

Change PermissionCheckEvent to use phases and add an additional phase #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ExcessiveAmountsOfZombies

This would add a phase that would run before any permission handlers.

Useful if someone has explicitly set permissions for users to false, but would want a content modifying mod to give them permissions if they meet certain conditions.

a real use case is for my professions mod where server owners could give users permission if they reach a certain level in their occupation

@lucko
Copy link
Owner

lucko commented Sep 5, 2022

Hmm interesting. My intention with these events is that they would only ever have one handler/listener registered - "the" permission handler.

If mods want to give players permissions, in my opinion they should do that via the permission handler. This enables lots of useful functionality, like the ability for permission handlers to offer things like LuckPerms' verbose command.

I'm not against the idea, or saying that fabric-permissions-api shouldn't provide the functionality, just that maybe a different approach would be better. (e.g. providing an API that allows mods to add/remove permission assignments from a player).

@ExcessiveAmountsOfZombies
Copy link
Author

I just went for something similar to bukkits perm api, where you could have plugins that directly added to the permission map, (like towny? been a while since I looked). was also the most simple thing to PR that I thought would 'just work' while being compatible with what already exists and no extra work for anyone who has written a perm handler

If this method is a no-go I think a nice alternative, would be as you mentioned an API to add/remove permissions from a player.

I would like to avoid having to add support to individual permission handlers with this mod as it'd be way more work to make sure the appropriate permissions are applied or removed from players.

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.

2 participants