Fix double-click handling in UI lists and improve test suite for missing MPQ assets

Source/DiabloUI/diabloui.cpp

@@ -18,7 +18,7 @@
 #include <SDL3/SDL_surface.h>
 #include <SDL3/SDL_timer.h>
 #else
 #include <SDL.h>
 #endif
 
 #include <function_ref.hpp>
@@ -111,6 +111,10 @@
 std::optional<TextInputState> UiTextInputState;
 bool allowEmptyTextInput = false;
 
+constexpr Uint32 ListDoubleClickTimeMs = 500;
+std::size_t lastListClickIndex = static_cast<std::size_t>(-1);
+Uint32 lastListClickTicks = 0;
+
 struct ScrollBarState {
 	bool upArrowPressed;
 	bool downArrowPressed;
@@ -144,7 +148,7 @@
 	if (fadeValue == 256) return;
 	if (fadeValue == 0 && fadeTc == 0) {
 		// Start the fade-in.
 		fadeTc = SDL_GetTicks();
 		fadeValue = 0;
 		BlackPalette();
 		// We can skip hardware cursor update for fade level 0 (everything is black).
@@ -165,7 +169,7 @@
 		return;
 	}
 
 	SDL_Color palette[256];
 	ApplyGlobalBrightness(palette, logical_palette.data());
 	ApplyFadeLevel(fadeValue, system_palette.data(), palette);
 
@@ -353,12 +357,12 @@
 
 void UiFocusPageDown()
 {
 	if (listOffset + ListViewportSize > static_cast<std::size_t>(SelectedItemMax)) {
 		UiFocus(SelectedItemMax, false, true);
 	} else {
 		const std::size_t relpos = SelectedItem - listOffset;
 		std::size_t nextPageEnd = SelectedItem + (ListViewportSize - relpos - 1);
 		if (nextPageEnd + ListViewportSize <= static_cast<std::size_t>(SelectedItemMax))
 			nextPageEnd += ListViewportSize;
 		else
 			nextPageEnd = SelectedItemMax;
@@ -417,7 +421,7 @@
 	RenderPresent();
 }
 
 void UiFocusNavigation(SDL_Event *event)
 {
 	switch (event->type) {
 	case SDL_EVENT_KEY_UP:
@@ -438,9 +442,9 @@
 	case SDL_EVENT_WINDOW_EXPOSED:
 #else
 #ifndef USE_SDL1
 	case SDL_WINDOWEVENT:
 #endif
 	case SDL_SYSWMEVENT:
 #endif
 		mainmenu_restart_repintro();
 		break;
@@ -495,7 +499,7 @@
 		return;
 	}
 
 	if (event->type == SDL_EVENT_KEY_DOWN && SDLC_EventKey(*event) == SDLK_RETURN) {
 		const auto *state = SDLC_GetKeyState();
 		if (state[SDLC_KEYSTATE_LALT] != 0 || state[SDLC_KEYSTATE_RALT] != 0) {
 			GetOptions().Graphics.fullscreen.SetValue(!IsFullScreen());
@@ -992,10 +996,6 @@
 	return true;
 }
 
-#ifdef USE_SDL1
-Uint32 dbClickTimer;
-#endif
-
 bool HandleMouseEventList(const SDL_Event &event, UiList *uiList)
 {
 	if (event.button.button != SDL_BUTTON_LEFT)
@@ -1016,25 +1016,32 @@
 	}
 
 	index += listOffset;
-
-	if (gfnListFocus != nullptr && SelectedItem != index) {
-		UiFocus(index, true, false);
-#ifdef USE_SDL1
-		dbClickTimer = SDL_GetTicks();
-	} else if (gfnListFocus == NULL || dbClickTimer + 500 >= SDL_GetTicks()) {
+	const bool hasFocusCallback = gfnListFocus != nullptr;
+	const Uint32 ticksNow = SDL_GetTicks();
+	const bool recentlyClickedSameItem = hasFocusCallback && lastListClickIndex == index && ticksNow - lastListClickTicks <= ListDoubleClickTimeMs;
+#ifndef USE_SDL1
+	const bool sdlReportedDoubleClick = event.button.clicks >= 2;
 #else
-	} else if (gfnListFocus == nullptr || event.button.clicks >= 2) {
-#endif
-		if (HasAnyOf(uiList->GetItem(index)->uiFlags, UiFlags::ElementHidden | UiFlags::ElementDisabled))
-			return false;
-		SelectedItem = index;
-		UiFocusNavigationSelect();
-#ifdef USE_SDL1
-	} else {
-		dbClickTimer = SDL_GetTicks();
+	const bool sdlReportedDoubleClick = false;
 #endif
+	const bool doubleClicked = recentlyClickedSameItem || sdlReportedDoubleClick;
+	lastListClickIndex = index;
+	lastListClickTicks = ticksNow;
+
+	if (hasFocusCallback && SelectedItem != index) {
+		UiFocus(index, true, false);
+		return true;
 	}
 
+	if (hasFocusCallback && !doubleClicked) {
+		return true;
+	}
+
+	if (HasAnyOf(uiList->GetItem(index)->uiFlags, UiFlags::ElementHidden | UiFlags::ElementDisabled))
+		return false;
+	SelectedItem = index;
+	UiFocusNavigationSelect();
+
 	return true;
 }
 

test/inv_test.cpp

@@ -23,8 +23,9 @@ class InvTest : public ::testing::Test {
 		LoadGameArchives();
 
 		// The tests need spawn.mpq or diabdat.mpq
-		// Please provide them so that the tests can run successfully
-		ASSERT_TRUE(HaveMainData());
+		if (!HaveMainData()) {
+			GTEST_SKIP() << "MPQ assets (spawn.mpq or DIABDAT.MPQ) not found - skipping test suite";
+		}
 
 		InitCursor();
 		LoadSpellData();

test/main.cpp

@@ -1,9 +1,81 @@
 #include <gtest/gtest.h>
 
+#include <iostream>
+#include <string>
+#include <unordered_map>
+
 #include "headless_mode.hpp"
 #include "options.h"
 #include "utils/paths.h"
 
+namespace {
+
+// Custom listener to track and report skipped tests with reasons
+class SkippedTestListener : public testing::EmptyTestEventListener {
+	std::unordered_map<std::string, int> skipReasons;
+	int totalSkipped = 0;
+
+	void OnTestPartResult(const testing::TestPartResult &test_part_result) override
+	{
+		if (test_part_result.skipped()) {
+			totalSkipped++;
+			std::string reason = test_part_result.message();
+			if (reason.empty()) {
+				reason = "No reason provided";
+			}
+			skipReasons[reason]++;
+		}
+	}
+
+	void OnTestProgramEnd(const testing::UnitTest & /*unit_test*/) override
+	{
+		if (totalSkipped > 0) {
+			std::cout << "\n";
+			std::cout << "========================================\n";
+			std::cout << "Test Skip Summary\n";
+			std::cout << "========================================\n";
+			std::cout << "Total tests skipped: " << totalSkipped << "\n\n";
+
+			// Show skip reasons, with most specific reasons first
+			bool hasMpqReason = false;
+			bool hasNoReason = false;
+			int mpqSkipCount = 0;
+			int noReasonCount = 0;
+
+			for (const auto &[reason, count] : skipReasons) {
+				if (reason.find("MPQ assets") != std::string::npos) {
+					hasMpqReason = true;
+					mpqSkipCount = count;
+				} else if (reason == "No reason provided") {
+					hasNoReason = true;
+					noReasonCount = count;
+				} else {
+					std::cout << "  • " << count << " test" << (count > 1 ? "s" : "") << " skipped: " << reason << "\n";
+				}
+			}
+
+			// Combine MPQ-related skips for clearer output
+			if (hasMpqReason) {
+				int totalMpqRelated = mpqSkipCount + (hasNoReason ? noReasonCount : 0);
+				std::cout << "  • " << totalMpqRelated << " test" << (totalMpqRelated > 1 ? "s" : "")
+				          << " skipped: MPQ assets (spawn.mpq or DIABDAT.MPQ) not found\n";
+				if (hasNoReason && noReasonCount > 0) {
+					std::cout << "    (" << noReasonCount << " test" << (noReasonCount > 1 ? "s" : "")
+					          << " automatically skipped due to test suite setup failure)\n";
+				}
+			} else if (hasNoReason) {
+				// Only "No reason provided" - show it as-is
+				std::cout << "  • " << noReasonCount << " test" << (noReasonCount > 1 ? "s" : "")
+				          << " skipped: " << "No reason provided" << "\n";
+			}
+
+			std::cout << "========================================\n";
+		}
+	}
+};
+
+} // namespace
+
 int main(int argc, char **argv)
 {
 	// Disable error dialogs.
@@ -20,5 +92,10 @@ int main(int argc, char **argv)
 #endif
 
 	testing::InitGoogleTest(&argc, argv);
+
+	// Add custom listener to track and report skipped tests
+	testing::TestEventListeners &listeners = testing::UnitTest::GetInstance()->listeners();
+	listeners.Append(new SkippedTestListener());
+
 	return RUN_ALL_TESTS();
 }

test/pack_test.cpp

@@ -977,8 +977,9 @@ class NetPackTest : public ::testing::Test {
 		LoadGameArchives();
 
 		// The tests need spawn.mpq or diabdat.mpq
-		// Please provide them so that the tests can run successfully
-		ASSERT_TRUE(HaveMainData());
+		if (!HaveMainData()) {
+			GTEST_SKIP() << "MPQ assets (spawn.mpq or DIABDAT.MPQ) not found - skipping test suite";
+		}
 
 		SetHellfireState(false);
 		InitCursor();

test/player_test.cpp

@@ -86,7 +86,9 @@ TEST(Player, PM_DoGotHit)
 {
 	LoadCoreArchives();
 	LoadGameArchives();
-	ASSERT_TRUE(HaveMainData());
+	if (!HaveMainData()) {
+		GTEST_SKIP() << "MPQ assets (spawn.mpq or DIABDAT.MPQ) not found - skipping test";
+	}
 	LoadPlayerDataFiles();
 
 	Players.resize(1);
@@ -191,8 +193,9 @@ TEST(Player, CreatePlayer)
 	LoadGameArchives();
 
 	// The tests need spawn.mpq or diabdat.mpq
-	// Please provide them so that the tests can run successfully
-	ASSERT_TRUE(HaveMainData());
+	if (!HaveMainData()) {
+		GTEST_SKIP() << "MPQ assets (spawn.mpq or DIABDAT.MPQ) not found - skipping test";
+	}
 
 	LoadPlayerDataFiles();
 	LoadMonsterData();

test/timedemo_test.cpp

@@ -48,8 +48,9 @@ void RunTimedemo(std::string timedemoFolderName)
 	LoadGameArchives();
 
 	// The tests need spawn.mpq or diabdat.mpq
-	// Please provide them so that the tests can run successfully
-	ASSERT_TRUE(HaveMainData());
+	if (!HaveMainData()) {
+		GTEST_SKIP() << "MPQ assets (spawn.mpq or DIABDAT.MPQ) not found - skipping test";
+	}
 
 	const std::string unitTestFolderCompletePath = paths::BasePath() + "test/fixtures/timedemo/" + timedemoFolderName;
 	paths::SetPrefPath(unitTestFolderCompletePath);

test/vendor_test.cpp

@@ -90,7 +90,9 @@ class VendorTest : public ::testing::Test {
 	{
 		LoadCoreArchives();
 		LoadGameArchives();
-		ASSERT_TRUE(HaveMainData());
+		if (!HaveMainData()) {
+			GTEST_SKIP() << "MPQ assets (spawn.mpq or DIABDAT.MPQ) not found - skipping test suite";
+		}
 		LoadPlayerDataFiles();
 		LoadItemData();
 		LoadSpellData();

test/writehero_test.cpp

@@ -368,8 +368,9 @@ TEST(Writehero, pfile_write_hero)
 	LoadGameArchives();
 
 	// The tests need spawn.mpq or diabdat.mpq
-	// Please provide them so that the tests can run successfully
-	ASSERT_TRUE(HaveMainData());
+	if (!HaveMainData()) {
+		GTEST_SKIP() << "MPQ assets (spawn.mpq or DIABDAT.MPQ) not found - skipping test";
+	}
 
 	const std::string savePath = paths::BasePath() + "multi_0.sv";
 	paths::SetPrefPath(paths::BasePath());