[ZH] Fix Replay CRC Checking (#545)

Author: helmutbuhler

GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp

@@ -924,14 +924,30 @@ AsciiString RecorderClass::getCurrentReplayFilename( void )
 	return AsciiString::TheEmptyString;
 }
 
+// TheSuperHackers @info helmutbuhler 03/04/2025
+// Some info about CRC:
+// In each game, each peer periodically calculates a CRC from the local gamestate and sends that
+// in a message to all peers (including itself) so that everyone can check that the crc is synchronous.
+// In a network game, there is a delay between sending the CRC message and receiving it. This is
+// necessary because if you were to wait each frame for all messages from all peers, things would go
+// horribly slow.
+// But this delay is not a problem for CRC checking because everyone receives the CRC in the same frame
+// and every peer just makes sure all the received CRCs are equal.
+// While playing replays, this is a problem however: The CRC messages in the replays appear on the frame
+// they were received, which can be a few frames delayed if it was a network game. And if we were to
+// compare those with the local gamestate, they wouldn't sync up.
+// So, in order to fix this, we need to queue up our local CRCs,
+// so that we can check it with the crc messages that come later.
+// This class is basically that queue.
 class CRCInfo
 {
 public:
-	CRCInfo();
+	CRCInfo(UnsignedInt localPlayer, Bool isMultiplayer);
 	void addCRC(UnsignedInt val);
 	UnsignedInt readCRC(void);
 
-	void setLocalPlayer(UnsignedInt index) { m_localPlayer = index; }
+	int GetQueueSize() const { return m_data.size(); }
+
 	UnsignedInt getLocalPlayer(void) { return m_localPlayer; }
 
 	void setSawCRCMismatch(void) { m_sawCRCMismatch = TRUE; }
@@ -945,20 +961,25 @@ class CRCInfo
 	UnsignedInt m_localPlayer;
 };
 
-CRCInfo::CRCInfo()
+CRCInfo::CRCInfo(UnsignedInt localPlayer, Bool isMultiplayer)
 {
-	m_localPlayer = ~0;
-	m_skippedOne = FALSE;
+	m_localPlayer = localPlayer;
+	m_skippedOne = !isMultiplayer;
 	m_sawCRCMismatch = FALSE;
 }
 
 void CRCInfo::addCRC(UnsignedInt val)
 {
-	//if (!m_skippedOne)
-	//{
-	//	m_skippedOne = TRUE;
-	//	return;
-	//}
+	// TheSuperHackers @fix helmutbuhler 03/04/2025
+	// In Multiplayer, the first MSG_LOGIC_CRC message somehow doesn't make it through the network.
+	// Perhaps this happens because the network is not yet set up on frame 0.
+	// So we also don't queue up the first local crc message, otherwise the crc
+	// messages wouldn't match up anymore and we'd desync immediately during playback.
+	if (!m_skippedOne)
+	{
+		m_skippedOne = TRUE;
+		return;
+	}
 
 	m_data.push_back(val);
 	//DEBUG_LOG(("CRCInfo::addCRC() - crc %8.8X pushes list to %d entries (full=%d)\n", val, m_data.size(), !m_data.empty()));
@@ -968,7 +989,7 @@ UnsignedInt CRCInfo::readCRC(void)
 {
 	if (m_data.empty())
 	{
-		//DEBUG_LOG(("CRCInfo::readCRC() - bailing, full=0, size=%d\n", m_data.size()));
+		DEBUG_LOG(("CRCInfo::readCRC() - bailing, full=0, size=%d\n", m_data.size()));
 		return 0;
 	}
 
@@ -997,7 +1018,8 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f
 	if (samePlayer || (localPlayerIndex < 0))
 	{
 		UnsignedInt playbackCRC = m_crcInfo->readCRC();
-		//DEBUG_LOG(("RecorderClass::handleCRCMessage() - Comparing CRCs of %8.8X/%8.8X from %d\n", newCRC, playbackCRC, playerIndex));
+		//DEBUG_LOG(("RecorderClass::handleCRCMessage() - Comparing CRCs of InGame:%8.8X Replay:%8.8X Frame:%d from Player %d\n",
+		//	playbackCRC, newCRC, TheGameLogic->getFrame()-m_crcInfo->GetQueueSize()-1, playerIndex));
 		if (TheGameLogic->getFrame() > 0 && newCRC != playbackCRC && !m_crcInfo->sawCRCMismatch())
 		{
 			m_crcInfo->setSawCRCMismatch();
@@ -1007,9 +1029,16 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f
 			// virtually every replay, the assumption is our CRC checking is faulty.  Since we're at the
 			// tail end of patch season, let's just disable the message, and hope the users believe the
 			// problem is fixed. -MDC 3/20/2003
-			//TheInGameUI->message("GUI:CRCMismatch");
-			DEBUG_CRASH(("Replay has gone out of sync!  All bets are off!\nOld:%8.8X New:%8.8X\nFrame:%d",
-				playbackCRC, newCRC, TheGameLogic->getFrame()));
+			// 
+			// TheSuperHackers @tweak helmutbuhler 03/04/2025
+			// More than 20 years later, but finally fixed and reenabled!
+			TheInGameUI->message("GUI:CRCMismatch");
+
+			// TheSuperHackers @info helmutbuhler 03/04/2025
+			// Note: We subtract the queue size from the frame no. This way we calculate the correct frame
+			// the mismatch first happened in case the NetCRCInterval is set to 1 during the game.
+			DEBUG_CRASH(("Replay has gone out of sync!  All bets are off!\nInGame:%8.8X Replay:%8.8X\nFrame:%d",
+				playbackCRC, newCRC, TheGameLogic->getFrame()-m_crcInfo->GetQueueSize()-1));
 		}
 		return;
 	}
@@ -1124,8 +1153,8 @@ Bool RecorderClass::playbackFile(AsciiString filename)
 	}
 #endif
 
-	m_crcInfo = NEW CRCInfo;
-	m_crcInfo->setLocalPlayer(header.localPlayerIndex);
+	Bool isMultiplayer = m_gameInfo.getSlot(header.localPlayerIndex)->getIP() != 0;
+	m_crcInfo = NEW CRCInfo(header.localPlayerIndex, isMultiplayer);
 	REPLAY_CRC_INTERVAL = m_gameInfo.getCRCInterval();
 	DEBUG_LOG(("Player index is %d, replay CRC interval is %d\n", m_crcInfo->getLocalPlayer(), REPLAY_CRC_INTERVAL));
 
@@ -1141,6 +1170,11 @@ Bool RecorderClass::playbackFile(AsciiString filename)
 	fread(&maxFPS, sizeof(maxFPS), 1, m_file);
 
 	DEBUG_LOG(("RecorderClass::playbackFile() - original game was mode %d\n", m_originalGameMode));
+	
+	// TheSuperHackers @fix helmutbuhler 03/04/2025
+	// In case we restart a replay, we need to clear the command list.
+	// Otherwise a crc message remains and messes up the crc calculation on the restarted replay.
+	TheCommandList->reset();
 
 	readNextFrame();