-
Notifications
You must be signed in to change notification settings - Fork 2
Added saving admin even after room restarts #3
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
base: master
Are you sure you want to change the base?
Conversation
Thanks for your contribution. Code looks good to me. I just wonder, what if I just want to give someone admin to take care of players for 5 minutes, but that person exits room? If he comes back after few days, he would still have the admin (and possibly I'm not in the room to revoke it). How can a room admin revoke his admin privileges then? He would have to open .sqlite manually, which most of the people cannot do. Also, it cannot be done on database thats currently running transaction, so it would require turning off the room to revoke the admin. Also, it could be counter-intuitive. People expect that restarting a room process will reset the admins so that only person who has process control can reassign admins. Think of a league room (there are leagues that use this script already). League admin gives temporary admin to the team captains and turns off the room after game. During next league match, when previous game's captains want to spectate, they would reappear with admin. |
You're right, I will try to add today removing player admin from database by auth or something. If someone wants temporary (for 5 minutes), just give him admin by single I was hosting a lot of league rooms, admins are for captains, if you want to give all of them, the function about giving admins is completely wrong, if its still for "league admins", it should stay at it is, can add option into settings saveAdmins or anything, I was mostly thinking about pub room. |
Ah, my mistake, this PR is only to reapply admins to people who got admin through !admin command. I overlooked that. Sounds good. |
I've added 2 new commands due to our previous chat !deladmin <auth> that can remove player admin by auth !ladmin with admin list, syntax name - [auth] Yes, I know it's gonna be problematic for now until there's no master rank or something for "owners".
Updated db file (support for deladmin and ladmin commands)
I've updated code, will prepare master rank soon aswell, so there wont be problem with random players removing, but still its open-source, so people can choose how they want to remove admins, etc. Or maybe someone has any other idea ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not sure about this... I'm not sure if it's a good solution for majority of people. Just a bit counterintuitive, also that the main host can get admin deleted (the control will be overtaken) after he shares admin password. And to retake it he would have to grant himself admin again and be FASTER than the other admin that could revoke it from him immediately. Especially that main admin possible would have the same auth, so he could spam !deladmin auth
even after room restart. I need to think a bit longer about this one.
While it works well on your pub I don't know if it's a good idea to merge it.
); | ||
return; | ||
} | ||
await getAdminsList(p); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Output will be too long if it lists all admin's auths. It should be truncated into multiple messages.
You're right, we trust our people at server, so even with 15 admins there, still none is abusing it. But you can keep it in mind to add stable admins etc, command is still not the best option, same as my PR here. One leak and room can be gone 🫣 |
It's tested. Maybe needs to optimize, you can check, greetings.