Merge #2502

2502: Improved dummy switching r=def- a=Fireball-Teeworlds

- Fix a use-after-free when there are no new snapshots for the cl_dummy tee after the switch: #2499 .
- When one of the tees is hooked, cycling through them will no longer show phantom hook for the other tee: https://youtu.be/mxVT4pdyGnU.
- Dummy switch might happen a bit quicker since it doesn't depend on receiving new snapshots.
- Simplified code: m_LastDummy2 is no more.



Co-authored-by: Fireball <fireball.teeworlds@gmail.com>

src/engine/client.h

@@ -221,6 +221,7 @@ public:
 	virtual void OnRconType(bool UsernameReq) = 0;
 	virtual void OnRconLine(const char *pLine) = 0;
 	virtual void OnInit() = 0;
+	virtual void InvalidateSnapshot() = 0;
 	virtual void OnNewSnapshot() = 0;
 	virtual void OnEnterGame() = 0;
 	virtual void OnShutdown() = 0;

src/engine/client/client.cpp

@@ -339,7 +339,6 @@ CClient::CClient() : m_DemoPlayer(&m_SnapshotDelta)
 	m_CurrentInput[0] = 0;
 	m_CurrentInput[1] = 0;
 	m_LastDummy = 0;
-	m_LastDummy2 = 0;
 
 	mem_zero(&m_aInputs, sizeof(m_aInputs));
 
@@ -505,12 +504,6 @@ void CClient::SendInput()
 	if(m_PredTick[g_Config.m_ClDummy] <= 0)
 		return;
 
-	if(m_LastDummy != (bool)g_Config.m_ClDummy)
-	{
-		m_LastDummy = g_Config.m_ClDummy;
-		GameClient()->OnDummySwap();
-	}
-
 	bool Force = false;
 	// fetch input
 	for(int Dummy = 0; Dummy < 2; Dummy++)
@@ -848,6 +841,9 @@ void CClient::DummyDisconnect(const char *pReason)
 	m_NetClient[CLIENT_DUMMY].Disconnect(pReason);
 	g_Config.m_ClDummy = 0;
 	m_RconAuthed[1] = 0;
+	m_aSnapshots[1][SNAP_CURRENT] = 0;
+	m_aSnapshots[1][SNAP_PREV] = 0;
+	m_ReceivedSnapshots[1] = 0;
 	m_DummyConnected = false;
 	GameClient()->OnDummyDisconnect();
 }
@@ -2574,8 +2570,15 @@ void CClient::Update()
 			Disconnect();
 		}
 	}
-	else if(State() == IClient::STATE_ONLINE && m_ReceivedSnapshots[g_Config.m_ClDummy] >= 3)
+	else if(State() == IClient::STATE_ONLINE)
 	{
+		if(m_LastDummy != (bool)g_Config.m_ClDummy)
+		{
+			// Invalidate references to !m_ClDummy snapshots
+			GameClient()->InvalidateSnapshot();
+			GameClient()->OnDummySwap();
+		}
+
 		if(m_ReceivedSnapshots[!g_Config.m_ClDummy] >= 3)
 		{
 			// switch dummy snapshot
@@ -2605,12 +2608,21 @@ void CClient::Update()
 			}
 		}
 
+		if(m_ReceivedSnapshots[g_Config.m_ClDummy] >= 3)
+		{
 			// switch snapshot
 			int Repredict = 0;
 			int64 Freq = time_freq();
 			int64 Now = m_GameTime[g_Config.m_ClDummy].Get(time_get());
 			int64 PredNow = m_PredictedTime.Get(time_get());
 
+			if(m_LastDummy != (bool)g_Config.m_ClDummy && m_aSnapshots[g_Config.m_ClDummy][SNAP_CURRENT] && m_aSnapshots[g_Config.m_ClDummy][SNAP_PREV])
+			{
+				// Load snapshot for m_ClDummy
+				GameClient()->OnNewSnapshot();
+				Repredict = 1;
+			}
+
 			while(1)
 			{
 				CSnapshotStorage::CHolder *pCur = m_aSnapshots[g_Config.m_ClDummy][SNAP_CURRENT];
@@ -2628,7 +2640,7 @@ void CClient::Update()
 						m_CurGameTick[g_Config.m_ClDummy] = m_aSnapshots[g_Config.m_ClDummy][SNAP_CURRENT]->m_Tick;
 						m_PrevGameTick[g_Config.m_ClDummy] = m_aSnapshots[g_Config.m_ClDummy][SNAP_PREV]->m_Tick;
 
-					if(m_LastDummy2 == (bool)g_Config.m_ClDummy && m_aSnapshots[g_Config.m_ClDummy][SNAP_CURRENT] && m_aSnapshots[g_Config.m_ClDummy][SNAP_PREV])
+						if(m_aSnapshots[g_Config.m_ClDummy][SNAP_CURRENT] && m_aSnapshots[g_Config.m_ClDummy][SNAP_PREV])
 						{
 							GameClient()->OnNewSnapshot();
 							Repredict = 1;
@@ -2641,10 +2653,6 @@ void CClient::Update()
 					break;
 			}
 
-		if(m_LastDummy2 != (bool)g_Config.m_ClDummy)
-		{
-			m_LastDummy2 = g_Config.m_ClDummy;
-		}
 
 			if(m_aSnapshots[g_Config.m_ClDummy][SNAP_CURRENT] && m_aSnapshots[g_Config.m_ClDummy][SNAP_PREV])
 			{
@@ -2693,6 +2701,9 @@ void CClient::Update()
 			}
 		}
 
+		m_LastDummy = (bool)g_Config.m_ClDummy;
+	}
+
 	// STRESS TEST: join the server again
 #ifdef CONF_DEBUG
 	if(g_Config.m_DbgStress)

src/engine/client/client.h

@@ -186,7 +186,6 @@ class CClient : public IClient, public CDemoPlayer::IListener
 
 	int m_CurrentInput[2];
 	bool m_LastDummy;
-	bool m_LastDummy2;
 	bool m_DummySendConnInfo;
 
 	// graphs

src/game/client/gameclient.cpp

@@ -511,10 +511,10 @@ void CGameClient::OnConnected()
 
 void CGameClient::OnReset()
 {
-	// clear out the invalid pointers
 	m_LastNewPredictedTick[0] = -1;
 	m_LastNewPredictedTick[1] = -1;
-	mem_zero(&g_GameClient.m_Snap, sizeof(g_GameClient.m_Snap));
+
+	InvalidateSnapshot();
 
 	for(int i = 0; i < MAX_CLIENTS; i++)
 		m_aClients[i].Reset();
@@ -1084,13 +1084,18 @@ static CGameInfo GetGameInfo(const CNetObj_GameInfoEx *pInfoEx, int InfoExSize,
 	return Info;
 }
 
-void CGameClient::OnNewSnapshot()
+void CGameClient::InvalidateSnapshot()
 {
-	m_NewTick = true;
-
-	// clear out the invalid pointers
+	// clear all pointers
 	mem_zero(&g_GameClient.m_Snap, sizeof(g_GameClient.m_Snap));
 	m_Snap.m_LocalClientID = -1;
+}
+
+void CGameClient::OnNewSnapshot()
+{
+	InvalidateSnapshot();
+
+	m_NewTick = true;
 
 	// secure snapshot
 	{

src/game/client/gameclient.h

@@ -346,6 +346,7 @@ public:
 	virtual void OnConsoleInit();
 	virtual void OnStateChange(int NewState, int OldState);
 	virtual void OnMessage(int MsgId, CUnpacker *pUnpacker, bool IsDummy = 0);
+	virtual void InvalidateSnapshot();
 	virtual void OnNewSnapshot();
 	virtual void OnPredict();
 	virtual void OnActivateEditor();