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

Added linking bypass perm #1510

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

supercoolspy
Copy link

based on #1491

@Scarsz
Copy link
Member

Scarsz commented Mar 3, 2023

Seems unnecessary to have a toggle for the permission, that's the permission plugin's job

That said, the reason this has never been implemented is because the link requirement check happens before the player has even fully joined the server... this means that (most?) permissions plugins don't have the player's permissions loaded just yet.

Have you tested this? Which permissions plugin?

@supercoolspy
Copy link
Author

Seems unnecessary to have a toggle for the permission, that's the permission plugin's job

That said, the reason this has never been implemented is because the link requirement check happens before the player has even fully joined the server... this means that (most?) permissions plugins don't have the player's permissions loaded just yet.

Have you tested this? Which permissions plugin?

Haven't tested it yet, I've seen some perm loading go fast enough but it just depends what you have loaded. I'll try it right now with a server with 30ish plugins

@supercoolspy
Copy link
Author

supercoolspy commented Mar 3, 2023

Yeah, doesn't load fast enough, in theory could the login event be pushed to nonpre if they enable the perm?

@underscore11code
Copy link
Contributor

Did some poking around, currently DSRV defaults to listening to AsyncPlayerPreLogin at LOWEST, which is before LuckPerms loads at LOW. By PlayerLogin at LOW, LP will kick players if their data isn't loaded, so any priority after LOW on non-async should guarantee permission data, and I believe even anything after LOW on async should be fine too (I think LP blocks the async event?). So, switching the event priority to NORMAL should be sufficient to make this work.

Obviously this only applies to LP, but given it's got the vast majority of the marketshare, I figure that's probably all that really matters.

@NullTxt
Copy link

NullTxt commented Mar 3, 2023

Did some poking around, currently DSRV defaults to listening to AsyncPlayerPreLogin at LOWEST, which is before LuckPerms loads at LOW. By PlayerLogin at LOW, LP will kick players if their data isn't loaded, so any priority after LOW on non-async should guarantee permission data, and I believe even anything after LOW on async should be fine too (I think LP blocks the async event?). So, switching the event priority to NORMAL should be sufficient to make this work.

Obviously this only applies to LP, but given it's got the vast majority of the marketshare, I figure that's probably all that really matters.

(My alt)
Yeah I think if we either force set it to normal or higher that would be good. Either that or we warn/error and make them manually set it if they turn the perm on in the config

@gerolndnr
Copy link

gerolndnr commented May 29, 2024

Seems unnecessary to have a toggle for the permission, that's the permission plugin's job

That said, the reason this has never been implemented is because the link requirement check happens before the player has even fully joined the server... this means that (most?) permissions plugins don't have the player's permissions loaded just yet.

Have you tested this? Which permissions plugin?

If I understand this correctly, this feature would surely be appreciated by many server owners. I looked for a way to enforce discord linking at a later point than the first join to make the first join as user-friendly as possible. The plan was to give beginner ranks the bypass permission and as soon as they got used to the server concept and ranked up, they would need the discord linking.
Is this possible with existing features? I didn't find anything in the docs on the linking.yml page.

@Dinty1
Copy link
Member

Dinty1 commented May 29, 2024

You could lock things behind the discordsrv:linked luckperms context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants