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 NoAccess as an AccessLevel #2412

Closed
wants to merge 1 commit into from

Conversation

TheKnobGoblin
Copy link

Accounts set to AccessLevel 0 will now be rejected.

0 - NoAccess
1 - Player
2 - Advocate
3 - Sentinel
4 - Envoy
5 - Developer
6 - Admin

Useful for servers that require email validation before a player can login.

Accounts set to AccessLevel 0 will now be rejected.

0 - NoAccess
1 - Player
2 - Advocate
3 - Sentinel
4 - Envoy
5 - Developer
6 - Admin

Useful for servers that require email validation before a player can login.
@Mag-nus
Copy link
Member

Mag-nus commented Nov 12, 2019

I like the idea, but, i don't think shifting the AccessLevel by 1 will work. I think that would demote existing characters and account levels.

Furthermore, it's probably not a good idea to drop the entire table in
DROP TABLE IF EXISTS accesslevel;
Instead, it would probably be safer to update existing rows, and add the new row.

@TheKnobGoblin
Copy link
Author

It works, you need to run the sql so that existing account levels are converted to the new ones.

Also you can't simply update the rows in the accesslevel table due to one of them being a primary key. I see nothing wrong with dropping this table and remaking it but your call.

@Mag-nus
Copy link
Member

Mag-nus commented Nov 12, 2019

I like the idea of this, but the implementation needs to be thought out some more.

For example, if a server adds this PR without the mysql update, their accounts won't have the appropriate access level, and many will be rejected.

if the do the SQL update before the PR, then their players have more access than they should.

ALso, the UPDATE statement should be a single query that adds 1 to all existing values that are between 0-5

@TheKnobGoblin
Copy link
Author

I've tested this and it works perfectly. You're defeating the entire purpose of mandatory sql updates when they're made. Server owners always need to install them.

Also who cares about whether the sql is on one line or not? It's just a simple update script that will be eventually moved to the base anyway.

Anyway, I'll keep this to myself and make sure not to contribute to this project anymore.

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.

None yet

2 participants