Clean up the OSD click callback code

Common/System/OSD.cpp

@@ -12,29 +12,12 @@ OnScreenDisplay g_OSD;
 // Effectively forever.
 constexpr double forever_s = 10000000000.0;
 
-OnScreenDisplay::~OnScreenDisplay() {
-	std::lock_guard<std::mutex> guard(mutex_);
-
-	double now = time_now_d();
-	for (auto &iter : entries_) {
-		if (iter.clickCallback) {
-			// Wasn't clicked, but let it free any data.
-			iter.clickCallback(false, iter.clickUserData);
-		}
-	}
-}
-
 void OnScreenDisplay::Update() {
 	std::lock_guard<std::mutex> guard(mutex_);
 
 	double now = time_now_d();
 	for (auto iter = entries_.begin(); iter != entries_.end(); ) {
 		if (now >= iter->endTime) {
-			if (iter->clickCallback) {
-				// Wasn't clicked, but let it free any data.
-				iter->clickCallback(false, iter->clickUserData);
-				iter->clickCallback = nullptr;
-			}
 			iter = entries_.erase(iter);
 		} else {
 			iter++;
@@ -61,9 +44,10 @@ float OnScreenDisplay::IngameAlpha() const {
 void OnScreenDisplay::ClickEntry(size_t index, double now) {
 	std::lock_guard<std::mutex> guard(mutex_);
 	if (index < entries_.size() && entries_[index].type != OSDType::ACHIEVEMENT_CHALLENGE_INDICATOR) {
-		entries_[index].endTime = std::min(now + FadeoutTime(), entries_[index].endTime);
 		if (entries_[index].clickCallback) {
-			entries_[index].clickCallback(true, entries_[index].clickUserData);
+			entries_[index].clickCallback();
+		} else {
+			entries_[index].endTime = std::min(now + FadeoutTime(), entries_[index].endTime);
 		}
 	}
 }
@@ -333,18 +317,17 @@ void OnScreenDisplay::ClearAchievementStuff() {
 	}
 }
 
-void OnScreenDisplay::SetClickCallback(const char *id, void (*callback)(bool, void *), void *userdata) {
+void OnScreenDisplay::SetClickCallback(std::string_view id, std::function<void()> callback) {
 	_dbg_assert_(callback != nullptr);
 	for (auto &ent : entries_) {
 		// protect against dupes.
-		if (ent.id == id && !ent.clickCallback) {
+		if (ent.id == id) {
 			ent.clickCallback = callback;
-			ent.clickUserData = userdata;
 		}
 	}
 }
 
-void OnScreenDisplay::SetFlags(const char *id, OSDMessageFlags flags) {
+void OnScreenDisplay::SetFlags(std::string_view id, OSDMessageFlags flags) {
 	for (auto &ent : entries_) {
 		// protect against dupes.
 		if (ent.id == id) {

Common/System/OSD.h

@@ -4,6 +4,7 @@
 #include <string_view>
 #include <vector>
 #include <mutex>
+#include <functional>
 
 #include "Common/Common.h"
 
@@ -49,8 +50,6 @@ ENUM_CLASS_BITOPS(OSDMessageFlags);
 // Data holder for on-screen messages.
 class OnScreenDisplay {
 public:
-	~OnScreenDisplay();
-
 	// If you specify 0.0f as duration, a duration will be chosen automatically depending on type.
 	void Show(OSDType type, std::string_view text, float duration_s = 0.0f, const char *id = nullptr) {
 		Show(type, text, "", duration_s, id);
@@ -90,8 +89,8 @@ public:
 	void ClearAchievementStuff();
 
 	// Can't add an infinite number of "Show" functions, so starting to offer post-modification.
-	void SetClickCallback(const char *id, void (*callback)(bool, void *), void *userdata);
-	void SetFlags(const char *id, OSDMessageFlags flag);
+	void SetFlags(std::string_view id, OSDMessageFlags flag);
+	void SetClickCallback(std::string_view id, std::function<void()> callback);
 
 	struct Entry {
 		OSDType type;
@@ -102,9 +101,7 @@ public:
 		std::string id;
 		OSDMessageFlags flags;
 
-		// We could use std::function, but prefer to do it the oldschool way.
-		void (*clickCallback)(bool, void *);
-		void *clickUserData;
+		std::function<void()> clickCallback;
 
 		int instanceID;
 		double startTime;

Core/System.cpp

@@ -403,22 +403,13 @@ static bool CPU_Init(FileLoader *fileLoader, IdentifiedFileType type, std::strin
 	if ((g_logManager.GetOutputsEnabled() & LogOutput::File) && !g_logManager.GetLogFilePath().empty()) {
 		auto dev = GetI18NCategory(I18NCat::DEVELOPER);
 
-		std::string logPath = g_logManager.GetLogFilePath().ToString();
-
-		// TODO: Really need a cleaner way to make clickable path notifications.
-		char *path = new char[logPath.size() + 1];
-		strcpy(path, logPath.data());
+		Path logPath = g_logManager.GetLogFilePath();
 
 		g_OSD.Show(OSDType::MESSAGE_INFO, ApplySafeSubstitutions("%1: %2", dev->T("Log to file"), g_logManager.GetLogFilePath().ToVisualString()), 0.0f, "log_to_file");
 		if (System_GetPropertyBool(SYSPROP_CAN_SHOW_FILE)) {
-			g_OSD.SetClickCallback("log_to_file", [](bool clicked, void *userdata) {
-				char *path = (char *)userdata;
-				if (clicked) {
-					System_ShowFileInFolder(Path(path));
-				} else {
-					delete[] path;
-				}
-			}, path);
+			g_OSD.SetClickCallback("log_to_file", [logPath]() {
+				System_ShowFileInFolder(logPath);
+			});
 		}
 	}
 
@@ -837,21 +828,11 @@ void DumpFileIfEnabled(const u8 *dataPtr, const u32 length, std::string_view nam
 	if (File::Exists(fullPath)) {
 		INFO_LOG(Log::sceModule, "%s already exists for this game, skipping dump.", filenameToDumpTo.c_str());
 
-		char *path = new char[strlen(fullPath.c_str()) + 1];
-		strcpy(path, fullPath.c_str());
-
-		g_OSD.Show(OSDType::MESSAGE_INFO, titleStr, fullPath.ToVisualString(), 5.0f);
+		g_OSD.Show(OSDType::MESSAGE_INFO, titleStr, fullPath.ToVisualString(), 5.0f, "file_dumped");
 		if (System_GetPropertyBool(SYSPROP_CAN_SHOW_FILE)) {
-			g_OSD.SetClickCallback("file_dumped", [](bool clicked, void *userdata) {
-				char *path = (char *)userdata;
-				if (clicked) {
-					System_ShowFileInFolder(Path(path));
-				} else {
-					delete[] path;
-				}
-			}, path);
-		} else {
-			delete[] path;
+			g_OSD.SetClickCallback("file_dumped", [fullPath]() {
+				System_ShowFileInFolder(fullPath);
+			});
 		}
 		return;
 	}
@@ -877,18 +858,11 @@ void DumpFileIfEnabled(const u8 *dataPtr, const u32 length, std::string_view nam
 
 	INFO_LOG(Log::sceModule, "Successfully wrote %s to %s", DumpFileTypeToString(type), fullPath.ToVisualString().c_str());
 
-	char *path = new char[strlen(fullPath.c_str()) + 1];
-	strcpy(path, fullPath.c_str());
-
-	// Re-suing the translation string here.
-	g_OSD.Show(OSDType::MESSAGE_SUCCESS, titleStr, fullPath.ToVisualString(), 5.0f);
+	// Re-using the translation string here.
+	g_OSD.Show(OSDType::MESSAGE_SUCCESS, titleStr, fullPath.ToVisualString(), 5.0f, "decr");
 	if (System_GetPropertyBool(SYSPROP_CAN_SHOW_FILE)) {
-		g_OSD.SetClickCallback("decr", [](bool clicked, void *userdata) {
-			char *path = (char *)userdata;
-			if (clicked) {
-				System_ShowFileInFolder(Path(path));
-			}
-			delete[] path;
-		}, path);
+		g_OSD.SetClickCallback("decr", [fullPath]() {
+			System_ShowFileInFolder(fullPath);
+		});
 	}
 }

UI/DeveloperToolsScreen.cpp

@@ -393,11 +393,9 @@ void DeveloperToolsScreen::CreateUITab(UI::LinearLayout *list) {
 	// This one is clickable
 	list->Add(new Choice(si->T("Success")))->OnClick.Add([&](UI::EventParams &) {
 		g_OSD.Show(OSDType::MESSAGE_SUCCESS, "Success", 0.0f, "clickable");
-		g_OSD.SetClickCallback("clickable", [](bool clicked, void *) {
-			if (clicked) {
-				System_LaunchUrl(LaunchUrlType::BROWSER_URL, "https://www.google.com/");
-			}
-		}, nullptr);
+		g_OSD.SetClickCallback("clickable", []() {
+			System_LaunchUrl(LaunchUrlType::BROWSER_URL, "https://www.google.com/");
+		});
 	});
 	list->Add(new Choice(sy->T("RetroAchievements")))->OnClick.Add([&](UI::EventParams &) {
 		g_OSD.Show(OSDType::MESSAGE_WARNING, "RetroAchievements warning", "", "I_RETROACHIEVEMENTS_LOGO");

UI/EmuScreen.cpp

@@ -1574,9 +1574,10 @@ void EmuScreen::HandleFlip() {
 		auto sy = GetI18NCategory(I18NCat::SYSTEM);
 		avi.Stop();
 		g_OSD.Show(OSDType::MESSAGE_INFO, sy->T("AVI Dump stopped."), 3.0f, "avi_dump");
-		g_OSD.SetClickCallback("avi_dump", [](bool, void *) {
-			System_ShowFileInFolder(avi.LastFilename());
-		}, nullptr);
+		Path lastFilename = avi.LastFilename();
+		g_OSD.SetClickCallback("avi_dump", [lastFilename]() {
+			System_ShowFileInFolder(lastFilename);
+		});
 		startDumping_ = false;
 	}
 #endif

UI/GameInfoCache.cpp

@@ -251,7 +251,11 @@ std::string GameInfo::GetMTime() const {
 // Not too meaningful if the object itself is a savedata directory...
 // Call this under lock.
 std::vector<Path> GameInfo::GetSaveDataDirectories() {
-	_dbg_assert_(hasFlags & GameInfoFlags::PARAM_SFO);  // so we know we have the ID.
+	if (!(hasFlags & GameInfoFlags::PARAM_SFO)) {
+		ERROR_LOG(Log::UI, "Can't get savedata directories if we don't have PARAM_SFO.");
+		return std::vector<Path>();
+	}
+
 	Path memc = GetSysDirectory(DIRECTORY_SAVEDATA);
 
 	std::vector<Path> directories;

UI/JitCompareScreen.h

@@ -18,11 +18,9 @@ private:
 	// Uses the current ListType
 	void FillBlockList();
 
-	UI::LinearLayout *comparisonView_ = nullptr;
 	UI::LinearLayout *leftDisasm_ = nullptr;
 	UI::LinearLayout *rightDisasm_ = nullptr;
 
-	UI::LinearLayout *blockListView_ = nullptr;
 	UI::LinearLayout *blockListContainer_ = nullptr;
 
 	void OnSelectBlock(UI::EventParams &e);

UI/NativeApp.cpp

@@ -846,12 +846,10 @@ bool NativeInitGraphics(GraphicsContext *graphicsContext) {
 		// This is a warning, not an error.
 		auto g = GetI18NCategory(I18NCat::GRAPHICS);
 		g_OSD.Show(OSDType::MESSAGE_WARNING, ApplySafeSubstitutions(g->T("Your display is set to a low refresh rate: %1 Hz. 60 Hz or higher is recommended."), (int)displayHz), 8.0f, "low_refresh");
-		g_OSD.SetClickCallback("low_refresh", [](bool clicked, void *) {
-			if (clicked) {
-				// Open the display settings.
-				System_OpenDisplaySettings();
-			}
-		}, nullptr);
+		g_OSD.SetClickCallback("low_refresh", []() {
+			// Open the display settings.
+			System_OpenDisplaySettings();
+		});
 	}
 #endif
 
@@ -1016,14 +1014,9 @@ static void TakeScreenshot(Draw::DrawContext *draw) {
 		if (success) {
 			g_OSD.Show(OSDType::MESSAGE_FILE_LINK, filename.ToVisualString(), 0.0f, "screenshot_link");
 			if (System_GetPropertyBool(SYSPROP_CAN_SHOW_FILE)) {
-				g_OSD.SetClickCallback("screenshot_link", [](bool clicked, void *data) -> void {
-					Path *path = reinterpret_cast<Path *>(data);
-					if (clicked) {
-						System_ShowFileInFolder(*path);
-					} else {
-						delete path;
-					}
-				}, new Path(filename));
+				g_OSD.SetClickCallback("screenshot_link", [filename]() -> void {
+					System_ShowFileInFolder(filename);
+				});
 			}
 		} else {
 			auto err = GetI18NCategory(I18NCat::ERRORS);