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

Add OnKickClient() forward #1749

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

Conversation

GabenManPowered
Copy link
Contributor

This forward allow kick to be detected (ie catch kick reason) or block kicking process.

This patch allow ROOT flag to call votes even if a delay is set. Its nice to have the possibility to set a delay betweens votes to limit public votes. Generally, we do this when we have lots of admins. But as a ROOT flag this limit shall no be set.
This patch allow ROOT flag to call votes even if a delay is set. Its nice to have the possibility to set a delay betweens votes to limit public votes. Generally, we do this when we have lots of admins. But as a ROOT flag this limit shall no be set.
This forward allow kick to be detected (ie catch kick reason) or block kicking process.
This forward allow kick to be detected (ie catch kick reason) or block kicking process.
Use KickClient instead of kickid
@Wend4r
Copy link
Contributor

Wend4r commented Apr 18, 2022

I think it is possible to hook the common PerformKick() (kick and kicked commands) (from engine/host_cmd.cpp) to call not only from the SourceMod abstraction trigger

@Wend4r
Copy link
Contributor

Wend4r commented Apr 18, 2022

Or even more common, can use CBaseClient::Disconnect() with a reason

@GabenManPowered
Copy link
Contributor Author

I think it is possible to hook the common PerformKick() (kick and kicked commands) (from engine/host_cmd.cpp) to call not only from the SourceMod abstraction trigger

@Wend4r I have used the same design as BanClient (ie banid with forwards OnBanClient, OnBanIdentity, OnRemoveBan)

Or even more common, can use CBaseClient::Disconnect() with a reason

@Wend4r For CBaseClient::Disconnect(), we dont want to have the disconnect reason! Here we want to detect a kick and if needed catch the reason when a player/bot/tv etc is kicked

Regards

@Wend4r
Copy link
Contributor

Wend4r commented Apr 18, 2022

For CBaseClient::Disconnect(), we dont want to have the disconnect reason! Here we want to detect a kick and if needed catch the reason when a player/bot/tv etc is kicked

@GabenManPowered In this regard, I suggest adding an reason argument to OnClientDisconnect()/OnClientDisconnected() by CBaseClient::Disconnect()

@Wend4r
Copy link
Contributor

Wend4r commented Apr 18, 2022

From the kick follows disconnecting with the kick reason

@GabenManPowered
Copy link
Contributor Author

I think its fine for the moment to used the same design as BanClient.

Copy link

@xerom999 xerom999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me, can this request be approved please? Will be very usefull...

@KyleSanderson
Copy link
Member

I apologize for the delay on this.

Functionally this looks ok, but what's the use-case? In what situation would you need to deny a kick? We have immunity rules that should take care of this so I'm coming to this from a place of ignorance.

@sapphonie
Copy link
Contributor

sapphonie commented Feb 13, 2023

I dont have a reason to block a kick, but it would be wonderful to have at least a fwd for when it gets called instead of rolling my own impl

@Kenzzer
Copy link
Member

Kenzzer commented Feb 14, 2023

I'm also against the idea to block kicks on the fly, as stated by Kyle the permission system more than cover this aspect.
If one truly wants control over the game kick system, then they should do what one would when it comes to anything game related, and dhook the engine kick function.

But I agree with sappho, having a notification forwards for kicks could have its benefits, but I can't think of any right now.

@KyleSanderson
Copy link
Member

Can you make this a Post notification for now?

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.

6 participants