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

Force player respawn to match vanilla #554

Merged
merged 25 commits into from
Sep 2, 2023

Conversation

ASpoonPlaysGames
Copy link
Contributor

Forces player respawn after 5 seconds of the respawn button being available (idk if that's accurate but can be changed whenever)

This should probably be made a private match setting or something? Some servers might want it off, but it is kinda necessary for gamemodes like MFD

@ghost
Copy link

ghost commented Jan 2, 2023

respawning a player may have many things to consider, like, player will have to watch replays, and they're always respawn as pilot if timer runs out.
I've wrote a function that does the same in Make Northstar Works Better pr, it looks like this

void function ForcedRespawnThink( entity player, float waitBeforeRespawn )
{
	player.EndSignal( "RespawnMe" )
	table result = {}
	result.repsawnIsForced <- false

	OnThreadEnd(
		function(): ( player, result )
		{
			if( GetGameState() != eGameState.Playing ) // no forced respawn
				return 
			if( IsValid( player ) )
			{
				if( expect bool( result.repsawnIsForced ) ) // reset player's persistent var if respawn is forced
					player.SetPersistentVar( "spawnAsTitan", false )
				player.Signal( "RespawnMe" )
			}
		}
	)
	
	WaitFrame()
	if( !IsValid( player ) )
		return
	if( player.IsWatchingKillReplay() )
		player.WaitSignal( "KillCamOver" )
	if( waitBeforeRespawn > 0 )
		wait waitBeforeRespawn
	result.repsawnIsForced = true
}

@ASpoonPlaysGames
Copy link
Contributor Author

respawning a player may have many things to consider, like, player will have to watch replays, and they're always respawn as pilot if timer runs out. I've wrote a function that does the same in Make Northstar Works Better pr, it looks like this

Honestly I think you have overengineered this a lot. Waiting for kill cam to be over first I agree with, but you shouldn't need to set the persistent var or check the gamestate? Players are forced to respawn no matter the gamestate (if they can respawn), and the persistent var should really be being set elsewhere.

catornot

This comment was marked as outdated.

@F1F7Y F1F7Y added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels Apr 20, 2023
Copy link
Member

@F1F7Y F1F7Y left a comment

Choose a reason for hiding this comment

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

Works as expected, code looks good

As discussed in the review by @catornot there should be a ns_force_respawn to disable this. Might also be worth using it to get the delay ( eg ns_force_respawn 0 would disable this behaviour, ns_force_respawn 5 would wait 5 seconds before respawning )

@ASpoonPlaysGames
Copy link
Contributor Author

eg ns_force_respawn 0 would disable this behaviour, ns_force_respawn 5 would wait 5 seconds before respawning

I agree, but ns_force_respawn 0 should be instant force respawn, anything below 0 should be disabled

Copy link
Member

@F1F7Y F1F7Y left a comment

Choose a reason for hiding this comment

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

works as expected, code looks good. I was able to join servers without this with no problems

NOTE: I contributed to this PR so would be cool if someone else reviewed this

@catornot catornot self-requested a review July 26, 2023 03:09
Copy link
Member

@catornot catornot left a comment

Choose a reason for hiding this comment

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

code looks good and everything worked in testing

5 second cool down

Untitled.1.mp4

0 second cool down :)

Untitled.3.mp4

-1 second cool down worked as expected

@ASpoonPlaysGames ASpoonPlaysGames added READY TO MERGE This mergeable right now and removed needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels Aug 25, 2023
@F1F7Y F1F7Y merged commit f16af28 into R2Northstar:main Sep 2, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
READY TO MERGE This mergeable right now
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants