From 7c0b72abc0bbd7b89570ee3cd0c0704bc44db3ce Mon Sep 17 00:00:00 2001
From: Gleb Mazovetskiy <glex.spb@gmail.com>
Date: Sun, 24 Jul 2022 21:32:48 +0100
Subject: [PATCH] Improve demo test error message

Example:

```
file "game" is different at byte 54251:
Expected: ... 00 00 00 00 00 00 00 00 30 00 00 00 50 00 00 00 ...
  Actual: ... FF FF FF FF 00 00 00 00 30 00 00 00 50 00 00 00 ...
```
---
 Source/engine/demomode.cpp |  4 ++--
 Source/pfile.cpp           | 37 ++++++++++++++++++++++++++-----------
 Source/pfile.h             | 12 ++++++++----
 test/timedemo_test.cpp     |  3 ++-
 4 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/Source/engine/demomode.cpp b/Source/engine/demomode.cpp
index fa29da061..fa5409652 100644
--- a/Source/engine/demomode.cpp
+++ b/Source/engine/demomode.cpp
@@ -280,7 +280,7 @@ void NotifyGameLoopEnd()
 		gbRunGame = false;
 
 		HeroCompareResult compareResult = pfile_compare_hero_demo(DemoNumber);
-		switch (compareResult) {
+		switch (compareResult.status) {
 		case HeroCompareResult::ReferenceNotFound:
 			SDL_Log("Timedemo: No final comparision cause reference is not present.");
 			break;
@@ -288,7 +288,7 @@ void NotifyGameLoopEnd()
 			SDL_Log("Timedemo: Same outcome as inital run. :)");
 			break;
 		case HeroCompareResult::Difference:
-			SDL_Log("Timedemo: Different outcome then inital run. ;(");
+			Log("Timedemo: Different outcome then inital run. ;(\n{}", compareResult.message);
 			break;
 		}
 	}
diff --git a/Source/pfile.cpp b/Source/pfile.cpp
index 7346d9cb6..fb17738da 100644
--- a/Source/pfile.cpp
+++ b/Source/pfile.cpp
@@ -21,6 +21,7 @@
 #include "utils/file_util.h"
 #include "utils/language.h"
 #include "utils/paths.h"
+#include "utils/stdcompat/abs.hpp"
 #include "utils/stdcompat/string_view.hpp"
 #include "utils/str_cat.hpp"
 #include "utils/utf8.hpp"
@@ -187,7 +188,7 @@ bool ArchiveContainsGame(MpqArchive &hsArchive)
 	return IsHeaderValid(hdr);
 }
 
-bool CompareSaves(const std::string &actualSavePath, const std::string &referenceSavePath)
+HeroCompareResult CompareSaves(const std::string &actualSavePath, const std::string &referenceSavePath)
 {
 	std::vector<std::string> possibleFileNamesToCheck;
 	possibleFileNamesToCheck.emplace_back("hero");
@@ -202,7 +203,6 @@ bool CompareSaves(const std::string &actualSavePath, const std::string &referenc
 	auto actualArchive = *MpqArchive::Open(actualSavePath.c_str(), error);
 	auto referenceArchive = *MpqArchive::Open(referenceSavePath.c_str(), error);
 
-	bool compareResult = true;
 	for (const auto &fileName : possibleFileNamesToCheck) {
 		size_t fileSizeActual;
 		auto fileDataActual = ReadArchive(actualArchive, fileName.c_str(), &fileSizeActual);
@@ -212,15 +212,31 @@ bool CompareSaves(const std::string &actualSavePath, const std::string &referenc
 			continue;
 		}
 		if (fileSizeActual != fileSizeReference) {
-			compareResult = false;
-			break;
+			return { HeroCompareResult::Difference, StrCat("file \"", fileName, "\" is different size. Expected: ", fileSizeReference, "Actual: ", fileSizeActual) };
 		}
-		if (memcmp(fileDataReference.get(), fileDataActual.get(), fileSizeActual) != 0) {
-			compareResult = false;
-			break;
+
+		const byte *reference = fileDataReference.get();
+		const byte *referenceEnd = reference + fileSizeActual;
+		const byte *actual = fileDataActual.get();
+		while (reference != referenceEnd) {
+			if (*reference != *actual) {
+				const size_t diffSize = std::min<size_t>(16, referenceEnd - reference);
+				return {
+					HeroCompareResult::Difference,
+					fmt::format(
+					    "file \"{}\" is different at byte {}:\n"
+					    "Expected: ... {:02X} ...\n"
+					    "  Actual: ... {:02X} ...",
+					    fileName, reference - fileDataReference.get(),
+					    fmt::join(reference, reference + diffSize, " "),
+					    fmt::join(actual, actual + diffSize, " "))
+				};
+			}
+			++reference;
+			++actual;
 		}
 	}
-	return compareResult;
+	return { HeroCompareResult::Same, {} };
 }
 
 void pfile_write_hero(MpqWriter &saveWriter, bool writeGameData)
@@ -298,7 +314,7 @@ HeroCompareResult pfile_compare_hero_demo(int demo)
 	std::string referenceSavePath = GetSavePath(gSaveNumber, StrCat("demo_", demo, "_reference_"));
 
 	if (!FileExists(referenceSavePath.c_str()))
-		return HeroCompareResult::ReferenceNotFound;
+		return { HeroCompareResult::ReferenceNotFound, {} };
 
 	std::string actualSavePath = GetSavePath(gSaveNumber, StrCat("demo_", demo, "_actual_"));
 	{
@@ -306,8 +322,7 @@ HeroCompareResult pfile_compare_hero_demo(int demo)
 		pfile_write_hero(saveWriter, true);
 	}
 
-	bool compareResult = CompareSaves(actualSavePath, referenceSavePath);
-	return compareResult ? HeroCompareResult::Same : HeroCompareResult::Difference;
+	return CompareSaves(actualSavePath, referenceSavePath);
 }
 
 void sfile_write_stash()
diff --git a/Source/pfile.h b/Source/pfile.h
index 3455f4593..5aa4634f5 100644
--- a/Source/pfile.h
+++ b/Source/pfile.h
@@ -17,10 +17,14 @@ extern bool gbValidSaveFile;
 /**
  * @brief Comparsion result of pfile_compare_hero_demo
  */
-enum class HeroCompareResult {
-	ReferenceNotFound,
-	Same,
-	Difference,
+struct HeroCompareResult {
+	enum Status {
+		ReferenceNotFound,
+		Same,
+		Difference,
+	};
+	Status status;
+	std::string message;
 };
 
 std::optional<MpqArchive> OpenSaveArchive(uint32_t saveNum);
diff --git a/test/timedemo_test.cpp b/test/timedemo_test.cpp
index f727eac6c..686cd2285 100644
--- a/test/timedemo_test.cpp
+++ b/test/timedemo_test.cpp
@@ -52,7 +52,8 @@ void RunTimedemo(std::string timedemoFolderName)
 
 	StartGame(false, true);
 
-	ASSERT_EQ(pfile_compare_hero_demo(demoNumber), HeroCompareResult::Same);
+	HeroCompareResult result = pfile_compare_hero_demo(demoNumber);
+	ASSERT_EQ(result.status, HeroCompareResult::Same) << result.message;
 	ASSERT_FALSE(gbRunGame);
 	gbRunGame = false;
 	init_cleanup();