Skip to content

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Anooxy17
Copy link
Contributor

It's tested. Maybe needs to optimize, you can check, greetings.

@jakjus
Copy link
Owner

jakjus commented Mar 13, 2025

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.

@Anooxy17
Copy link
Contributor Author

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
click haxball admin.

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.

@jakjus
Copy link
Owner

jakjus commented Mar 13, 2025

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)
@Anooxy17
Copy link
Contributor Author

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 ;)

Copy link
Owner

@jakjus jakjus left a 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);
Copy link
Owner

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.

@Anooxy17
Copy link
Contributor Author

Anooxy17 commented Apr 6, 2025

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 🫣

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