Skip to content

SDKTools: Add parameters to ForcePlayerSuicide native#1782

Merged
peace-maker merged 4 commits intoalliedmodders:masterfrom
Mikusch:commitsuicide-params
Jul 7, 2022
Merged

SDKTools: Add parameters to ForcePlayerSuicide native#1782
peace-maker merged 4 commits intoalliedmodders:masterfrom
Mikusch:commitsuicide-params

Conversation

@Mikusch
Copy link
Contributor

@Mikusch Mikusch commented Jun 16, 2022

The function CBasePlayer::CommitSuicide has two additional parameters which SourceMod silently passes false for when calling the ForcePlayerSuicide native. There are situations where you want to force the player to gib (explode) or want to bypass the 5 second suicide cooldown.
This PR allows users to pass the parameters to the underlying call.

native void ForcePlayerSuicide(int client, bool explode = false, bool force = false);

Tested on TF2 Windows.

@asherkin
Copy link
Member

or want to bypass the 5 second suicide cooldown.

Seems that should just be the default native behaviour here - it likely wasn't known what the boolean was for when this was added.

What's game support like for explode?

@Mikusch
Copy link
Contributor Author

Mikusch commented Jun 20, 2022 via email

@xerox8521
Copy link
Contributor

ZPS Supports exploding of models with their own gibs even and also requires the force parameter to be set to true to avoid the delay.

@Mikusch Mikusch requested a review from peace-maker June 21, 2022 09:51
@peace-maker
Copy link
Member

Seems that should just be the default native behaviour here - it likely wasn't known what the boolean was for when this was added.

It is weird that a native called "ForceSomething" isn't forcing something by default indeed. I don't know if changing the force default value to true would cause problems in some games now that it's been off for so long, but I'm inclined to try.

@asherkin
Copy link
Member

I don't think we should even expose force, just use true internally.

@Mikusch
Copy link
Contributor Author

Mikusch commented Jun 21, 2022

I don't think we should even expose force, just use true internally.

I'd agree if this native wasn't around for many years already. This might cause unexpected behavior for old plugins that expect the cooldown to be present. It's not likely many of them exist, but it's a possibility.

@Mikusch Mikusch requested a review from peace-maker June 24, 2022 10:29
@KyleSanderson
Copy link
Member

KickClient was expanded because it (well documented) crashed, RemoveEdict is still a nightmare in-frame where it's simply not safe. The Force* on these is a wart, I think defaulting these to safe is the appropriate way forward.

@Mikusch
Copy link
Contributor Author

Mikusch commented Jun 30, 2022

@peace-maker @asherkin I've set the force param to true by default. Is this good to go?

@peace-maker
Copy link
Member

KickClient was expanded because it (well documented) crashed, RemoveEdict is still a nightmare in-frame where it's simply not safe. The Force* on these is a wart, I think defaulting these to safe is the appropriate way forward.

The force parameter isn't removing entities or edicts when they shouldn't be. The game just prevents spamming CommitSuicide for some reason and ignores future calls for 5 seconds after the last call. Plugins should be able to override this.

@peace-maker peace-maker merged commit 278998f into alliedmodders:master Jul 7, 2022
@rtldg
Copy link
Contributor

rtldg commented Jul 7, 2022

Can this be backported / included in the 1.11 builds?

@Mikusch Mikusch deleted the commitsuicide-params branch July 7, 2022 18:02
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