Merge #6616

6616: fix timescore r=Robyt3 a=edg-l

Fixes the time displayed, this time i tested it quite thoroughly, i could reproduce the wrong ordering in scoreboard and the time not updating before, and fixed it now.

The previous way also had a issue with times that ended up with a value of -1, which is fixed now too

## Checklist

- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [x] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Edgar Luque <git@edgarluque.com>

src/game/server/player.cpp

@@ -119,7 +119,7 @@ void CPlayer::Reset()
 	m_DND = false;
 
 	m_LastPause = 0;
-	m_Score = -1;
+	m_Score.reset();
 
 	// Variable initialized:
 	m_Last_Team = 0;
@@ -183,7 +183,20 @@ void CPlayer::Tick()
 	if(m_ChatScore > 0)
 		m_ChatScore--;
 
-	Server()->SetClientScore(m_ClientID, m_Score);
+	int Score;
+	if(m_Score)
+	{
+		if(*m_Score == 9999)
+			Score = -10000;
+		else
+			Score = *m_Score;
+	}
+	else
+	{
+		Score = -9999;
+	}
+
+	Server()->SetClientScore(m_ClientID, Score);
 
 	if(m_Moderating && m_Afk)
 	{
@@ -336,11 +349,26 @@ void CPlayer::Snap(int SnappingClient)
 
 	int SnappingClientVersion = GameServer()->GetClientVersion(SnappingClient);
 	int Latency = SnappingClient == SERVER_DEMO_CLIENT ? m_Latency.m_Min : GameServer()->m_apPlayers[SnappingClient]->m_aCurLatency[m_ClientID];
-	int Score = m_Score;
+
+	int Score;
+	if(m_Score)
+	{
+		// -9999 stands for no time and isn't displayed in scoreboard, so
+		// shift the time by a second if the player actually took 9999
+		// seconds to finish the map.
+		if(*m_Score == 9999)
+			Score = -10000;
+		else
+			Score = -1 * absolute(*m_Score);
+	}
+	else
+	{
+		Score = -9999;
+	}
 
 	// send 0 if times of others are not shown
 	if(SnappingClient != m_ClientID && g_Config.m_SvHideScore)
-		Score = -1;
+		Score = -9999;
 
 	if(!Server()->IsSixup(SnappingClient))
 	{
@@ -349,10 +377,7 @@ void CPlayer::Snap(int SnappingClient)
 			return;
 
 		pPlayerInfo->m_Latency = Latency;
-		// -9999 stands for no time and isn't displayed in scoreboard, so
-		// shift the time by a second if the player actually took 9999
-		// seconds to finish the map.
-		pPlayerInfo->m_Score = Score == -1 ? -9999 : Score == 9999 ? -10000 : -Score;
+		pPlayerInfo->m_Score = Score;
 		pPlayerInfo->m_Local = (int)(m_ClientID == SnappingClient && (m_Paused != PAUSE_PAUSED || SnappingClientVersion >= VERSION_DDNET_OLD));
 		pPlayerInfo->m_ClientID = id;
 		pPlayerInfo->m_Team = m_Team;
@@ -375,7 +400,7 @@ void CPlayer::Snap(int SnappingClient)
 			pPlayerInfo->m_PlayerFlags |= protocol7::PLAYERFLAG_ADMIN;
 
 		// Times are in milliseconds for 0.7
-		pPlayerInfo->m_Score = Score == -1 ? -1 : Score * 1000;
+		pPlayerInfo->m_Score = Score == -9999 ? -1 : -Score * 1000;
 		pPlayerInfo->m_Latency = Latency;
 	}
 
@@ -887,7 +912,8 @@ void CPlayer::ProcessScoreResult(CScorePlayerResult &Result)
 			GameServer()->CallVote(m_ClientID, Result.m_Data.m_MapVote.m_aMap, aCmd, "/map", aChatmsg);
 			break;
 		case CScorePlayerResult::PLAYER_INFO:
-			GameServer()->Score()->PlayerData(m_ClientID)->Set(Result.m_Data.m_Info.m_Time, Result.m_Data.m_Info.m_aTimeCp);
+			if(Result.m_Data.m_Info.m_Time)
+				GameServer()->Score()->PlayerData(m_ClientID)->Set(*Result.m_Data.m_Info.m_Time, Result.m_Data.m_Info.m_aTimeCp);
 			m_Score = Result.m_Data.m_Info.m_Time;
 			Server()->ExpireServerInfo();
 			int Birthday = Result.m_Data.m_Info.m_Birthday;

src/game/server/player.h

@@ -11,6 +11,7 @@
 #include "teeinfo.h"
 
 #include <memory>
+#include <optional>
 
 class CCharacter;
 class CGameContext;
@@ -104,7 +105,7 @@ public:
 
 	int m_DieTick;
 	int m_PreviousDieTick;
-	int m_Score;
+	std::optional<int> m_Score;
 	int m_JoinTick;
 	bool m_ForceBalanced;
 	int m_LastActionTick;

src/game/server/scoreworker.cpp

@@ -40,7 +40,7 @@ void CScorePlayerResult::SetVariant(Variant v)
 		break;
 	case PLAYER_INFO:
 		m_Data.m_Info.m_Birthday = 0;
-		m_Data.m_Info.m_Time = -1;
+		m_Data.m_Info.m_Time.reset();
 		for(float &TimeCp : m_Data.m_Info.m_aTimeCp)
 			TimeCp = 0;
 	}

src/game/server/scoreworker.h

@@ -2,6 +2,7 @@
 #define GAME_SERVER_SCOREWORKER_H
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <utility>
 #include <vector>
@@ -45,10 +46,10 @@ struct CScorePlayerResult : ISqlResult
 		char m_aBroadcast[1024];
 		struct
 		{
-			float m_Time; // -1 for no time.
+			std::optional<float> m_Time;
 			float m_aTimeCp[NUM_CHECKPOINTS];
 			int m_Birthday; // 0 indicates no birthday
-		} m_Info;
+		} m_Info = {};
 		struct
 		{
 			char m_aReason[VOTE_REASON_LENGTH];

src/game/server/teams.cpp

@@ -818,7 +818,7 @@ void CGameTeams::OnFinish(CPlayer *Player, float Time, const char *pTimestamp)
 	}
 
 	int TTime = 0 - (int)Time;
-	if(Player->m_Score == -1 || Player->m_Score < TTime)
+	if(!Player->m_Score || -1 * absolute(*Player->m_Score) < TTime)
 	{
 		Player->m_Score = TTime;
 	}

src/test/score.cpp

@@ -183,7 +183,7 @@ TEST_P(SingleScore, LoadPlayerData)
 	ASSERT_FALSE(CScoreWorker::LoadPlayerData(m_pConn, &m_PlayerRequest, m_aError, sizeof(m_aError))) << m_aError;
 
 	EXPECT_EQ(m_pPlayerResult->m_MessageKind, CScorePlayerResult::PLAYER_INFO);
-	ASSERT_EQ(m_pPlayerResult->m_Data.m_Info.m_Time, -1.0);
+	ASSERT_FALSE(m_pPlayerResult->m_Data.m_Info.m_Time.has_value());
 	for(auto &Time : m_pPlayerResult->m_Data.m_Info.m_aTimeCp)
 	{
 		ASSERT_EQ(Time, 0);
@@ -194,7 +194,8 @@ TEST_P(SingleScore, LoadPlayerData)
 	ASSERT_FALSE(CScoreWorker::LoadPlayerData(m_pConn, &m_PlayerRequest, m_aError, sizeof(m_aError))) << m_aError;
 
 	EXPECT_EQ(m_pPlayerResult->m_MessageKind, CScorePlayerResult::PLAYER_INFO);
-	ASSERT_EQ(m_pPlayerResult->m_Data.m_Info.m_Time, 100.0);
+	ASSERT_TRUE(m_pPlayerResult->m_Data.m_Info.m_Time.has_value());
+	ASSERT_EQ(*m_pPlayerResult->m_Data.m_Info.m_Time, 100.0);
 	for(int i = 0; i < NUM_CHECKPOINTS; i++)
 	{
 		ASSERT_EQ(m_pPlayerResult->m_Data.m_Info.m_aTimeCp[i], i);
@@ -205,7 +206,7 @@ TEST_P(SingleScore, LoadPlayerData)
 	ASSERT_FALSE(CScoreWorker::LoadPlayerData(m_pConn, &m_PlayerRequest, m_aError, sizeof(m_aError))) << m_aError;
 
 	EXPECT_EQ(m_pPlayerResult->m_MessageKind, CScorePlayerResult::PLAYER_INFO);
-	ASSERT_EQ(m_pPlayerResult->m_Data.m_Info.m_Time, -1.0);
+	ASSERT_FALSE(m_pPlayerResult->m_Data.m_Info.m_Time.has_value());
 	for(int i = 0; i < NUM_CHECKPOINTS; i++)
 	{
 		ASSERT_EQ(m_pPlayerResult->m_Data.m_Info.m_aTimeCp[i], i);