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

Clean Team Score After Player Disconnecting in FFA #589

Merged
merged 10 commits into from Mar 2, 2023
Merged

Clean Team Score After Player Disconnecting in FFA #589

merged 10 commits into from Mar 2, 2023

Conversation

ghost
Copy link

@ghost ghost commented Feb 3, 2023

Fixes #400
Player's team score will be cleaned after they disconnecting, so later joiners won't have trouble start from 0 score

Copy link
Contributor

@NoCatt NoCatt left a comment

Choose a reason for hiding this comment

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

The code seems a little too complicated for what this can do.
If we already have a table of the score of each player, wouldnt it also make sense to then save that score so if they re-join they get to keep their score.

Also a better way to do this, is to just save the score of a player in a table < string, int > . We dont need the struct anyway as the team is unique to each player.
I would suggest to just add a callback for player disconnect like this:

void function OnPlayerDisconnected(entity player)
{
	ffaPlayerScoreTable[player.GetUID()] <- GameRules_GetTeamScore( player.GetTeam() )

	// this prevents a new player to get someone who's left's score
	AddTeamScore( player.GetTeam(), -GameRules_GetTeamScore( player.GetTeam() ))

}

That way we can also eliminate the change to the OnPlayerKilled callback.
further we could adjust the client connect to:

void function OnClientConnected( entity player )
{
	string uid = player.GetUID()
	FFAScoreStruct emptyStruct
	if (  uid in file.ffaPlayerScoreTable ) 
	{
	AddTeamScore( player.GetTeam(), SaveScore[player.GetPlayerName()] )
	player.AddToPlayerGameStat( PGS_ASSAULT_SCORE, ffaPlayerScoreTable[player.GetUID()] )
	}

}

Then we have a system where the score is saved for a re joining player AND no one else can get the score

@ghost
Copy link
Author

ghost commented Feb 19, 2023

The code seems a little too complicated for what this can do. If we already have a table of the score of each player, wouldnt it also make sense to then save that score so if they re-join they get to keep their score.

Also a better way to do this, is to just save the score of a player in a table < string, int > . We dont need the struct anyway as the team is unique to each player. I would suggest to just add a callback for player disconnect like this:

void function OnPlayerDisconnected(entity player)
{
	ffaPlayerScoreTable[player.GetUID()] <- GameRules_GetTeamScore( player.GetTeam() )

	// this prevents a new player to get someone who's left's score
	AddTeamScore( player.GetTeam(), -GameRules_GetTeamScore( player.GetTeam() ))

}

That way we can also eliminate the change to the OnPlayerKilled callback. further we could adjust the client connect to:

void function OnClientConnected( entity player )
{
	string uid = player.GetUID()
	FFAScoreStruct emptyStruct
	if (  uid in file.ffaPlayerScoreTable ) 
	{
	AddTeamScore( player.GetTeam(), SaveScore[player.GetPlayerName()] )
	player.AddToPlayerGameStat( PGS_ASSAULT_SCORE, ffaPlayerScoreTable[player.GetUID()] )
	}

}

Then we have a system where the score is saved for a re joining player AND no one else can get the score

OnClientDisconnected() sometimes passes a null entity, and that's why I used player.WaitSignal( "OnDestroy" )
AddTeamScore( player.GetTeam(), -GameRules_GetTeamScore( player.GetTeam() )) should be pretty good
I made structs because I had also made a score system, I was thinking that a disconnecting player should get some punishment or something so I removed it

Copy link
Contributor

@NoCatt NoCatt left a comment

Choose a reason for hiding this comment

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

Looks good now, I have never experienced any issue with the Client disconnect callback but it really doesnt matter now. imo this is good to merge

Copy link
Contributor

@uniboi uniboi 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, I didn't test the changes though

@x3Karma
Copy link
Contributor

x3Karma commented Feb 27, 2023

Code looks good, I didn't test the changes though

sounds suspiciously like "LGTM!"

@uniboi
Copy link
Contributor

uniboi commented Feb 27, 2023

It isn't, the code has been improved from "why would you do that" to "this is a valid and efficient way of doing this"

@GeckoEidechse GeckoEidechse changed the title Clean Team Score After Player Disconneting in FFA Clean Team Score After Player Disconnecting in FFA Mar 2, 2023
@GeckoEidechse GeckoEidechse added needs testing Changes from the PR still need to be tested almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge labels Mar 2, 2023
@NoCatt
Copy link
Contributor

NoCatt commented Mar 2, 2023

Tested in private server with @uniboi. After I rejoined the game all my kills and point were gone 👍

Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

Still haven't managed to fix approval counting for reviewers so I'm just giving the approval sign-off to be able to get it merged.

Did not test/check code myself, signing it off as

@GeckoEidechse GeckoEidechse merged commit 85e2c14 into R2Northstar:main Mar 2, 2023
@uniboi uniboi removed the needs testing Changes from the PR still need to be tested label Mar 2, 2023
@GeckoEidechse GeckoEidechse removed the almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge label Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Fix potentially transferring score from leaving player to new joining player in FFA
4 participants