GUI: Better integration for the unknown game dialog when adding games

* The list of candidates now includes unknown variants. When an unknown
variant is selected, the unknown game dialog is shown.
* On the unknown game dialog, users are given the choice to add the game
when that is possible, or to cancel.

The goal of those changes is to make the unknown game dialog less
confusing for users, especially when both known and unknown games
variants are found.

6 files changed, 109 insertions, 75 deletions

diff --git a/base/plugins.cpp b/base/plugins.cpp
index 55c99b1eda..ac217ab960 100644
--- a/base/plugins.cpp
+++ b/base/plugins.cpp
@@ -553,6 +553,7 @@ DetectionResults EngineManager::detectGames(const Common::FSList &fslist) const
 			for (uint i = 0; i < engineCandidates.size(); i++) {
 				engineCandidates[i].engineName = metaEngine.getName();
 				engineCandidates[i].path = fslist.begin()->getParent().getPath();
+				engineCandidates[i].shortPath = fslist.begin()->getParent().getDisplayName();
 				candidates.push_back(engineCandidates[i]);
 			}
 
diff --git a/engines/game.cpp b/engines/game.cpp
index bb5f4ae6cb..24e125825f 100644
--- a/engines/game.cpp
+++ b/engines/game.cpp
@@ -152,8 +152,16 @@ DetectedGames DetectionResults::listRecognizedGames() const {
 	return candidates;
 }
 
+DetectedGames DetectionResults::listDetectedGames() const {
+	return _detectedGames;
+}
+
 Common::String DetectionResults::generateUnknownGameReport(bool translate, uint32 wordwrapAt) const {
-	assert(!_detectedGames.empty());
+	return ::generateUnknownGameReport(_detectedGames, translate, false, wordwrapAt);
+}
+
+Common::String generateUnknownGameReport(const DetectedGames &detectedGames, bool translate, bool fullPath, uint32 wordwrapAt) {
+	assert(!detectedGames.empty());
 
 	const char *reportStart = _s("The game in '%s' seems to be an unknown game variant.\n\n"
 	                             "Please report the following data to the ScummVM team at %s "
@@ -162,7 +170,8 @@ Common::String DetectionResults::generateUnknownGameReport(bool translate, uint3
 	const char *reportEngineHeader = _s("Matched game IDs for the %s engine:");
 
 	Common::String report = Common::String::format(
-			translate ? _(reportStart) : reportStart, _detectedGames[0].path.c_str(),
+			translate ? _(reportStart) : reportStart,
+			fullPath ? detectedGames[0].path.c_str() : detectedGames[0].shortPath.c_str(),
 			"https://bugs.scummvm.org/"
 	);
 	report += "\n";
@@ -170,8 +179,8 @@ Common::String DetectionResults::generateUnknownGameReport(bool translate, uint3
 	FilePropertiesMap matchedFiles;
 
 	const char *currentEngineName = nullptr;
-	for (uint i = 0; i < _detectedGames.size(); i++) {
-		const DetectedGame &game = _detectedGames[i];
+	for (uint i = 0; i < detectedGames.size(); i++) {
+		const DetectedGame &game = detectedGames[i];
 
 		if (!game.hasUnknownFiles) continue;
 
@@ -215,17 +224,9 @@ Common::String DetectionResults::generateUnknownGameReport(bool translate, uint3
 	return report;
 }
 
-Common::StringArray DetectionResults::getUnknownGameEngines() const {
-	Common::StringArray engines;
-	const char *currentEngineName = nullptr;
-	for (uint i = 0; i < _detectedGames.size(); i++) {
-		const DetectedGame &game = _detectedGames[i];
-		if (!game.hasUnknownFiles)
-			continue;
-		if (!currentEngineName || strcmp(currentEngineName, game.engineName) != 0) {
-			currentEngineName = game.engineName;
-			engines.push_back(Common::String(currentEngineName));
-		}
-	}
-	return engines;
+Common::String generateUnknownGameReport(const DetectedGame &detectedGame, bool translate, bool fullPath, uint32 wordwrapAt) {
+	DetectedGames detectedGames;
+	detectedGames.push_back(detectedGame);
+
+	return generateUnknownGameReport(detectedGames, translate, fullPath, wordwrapAt);
 }
diff --git a/engines/game.h b/engines/game.h
index fcaae62e5c..8a678cfd2c 100644
--- a/engines/game.h
+++ b/engines/game.h
@@ -142,6 +142,7 @@ struct DetectedGame {
 	Common::Language language;
 	Common::Platform platform;
 	Common::String path;
+	Common::String shortPath;
 	Common::String extra;
 
 	/**
@@ -202,6 +203,14 @@ public:
 	DetectedGames listRecognizedGames() const;
 
 	/**
+	 * List all the games that were detected
+	 *
+	 * That includes entries that don't have enough information to be added to the
+	 * configuration manager.
+	 */
+	DetectedGames listDetectedGames() const;
+
+	/**
 	 * Were unknown game variants found by the engines?
 	 *
 	 * When unknown game variants are found, an unknown game report can be generated.
@@ -209,21 +218,26 @@ public:
 	bool foundUnknownGames() const;
 
 	/**
-	 * Generate a report that we found an unknown game variant, together with the file
-	 * names, sizes and MD5 sums.
+	 * Generate a report that we found an unknown game variant.
 	 *
-	 * @param translate translate the report to the currently active GUI language
-	 * @param wordwrapAt word wrap the text part of the report after a number of characters
+	 * @see ::generateUnknownGameReport
 	 */
 	Common::String generateUnknownGameReport(bool translate, uint32 wordwrapAt = 0) const;
 
-	/**
-	 * Get the list of engines for which an unknown game variant was found.
-	 */
-	Common::StringArray getUnknownGameEngines() const;
-
 private:
 	DetectedGames _detectedGames;
 };
 
+/**
+ * Generate a report that we found an unknown game variant, together with the file
+ * names, sizes and MD5 sums.
+ *
+ * @param translate translate the report to the currently active GUI language
+ * @param fullPath include the full path where the files are located, otherwise only the name
+ *                 of last component of the path is included
+ * @param wordwrapAt word wrap the text part of the report after a number of characters
+ */
+Common::String generateUnknownGameReport(const DetectedGames &detectedGames, bool translate, bool fullPath, uint32 wordwrapAt = 0);
+Common::String generateUnknownGameReport(const DetectedGame &detectedGame, bool translate, bool fullPath, uint32 wordwrapAt = 0);
+
 #endif
diff --git a/gui/launcher.cpp b/gui/launcher.cpp
index 14e0e96a3b..b09f650fc9 100644
--- a/gui/launcher.cpp
+++ b/gui/launcher.cpp
@@ -563,12 +563,9 @@ bool LauncherDialog::doGameDetection(const Common::String &path) {
 	if (detectionResults.foundUnknownGames()) {
 		Common::String report = detectionResults.generateUnknownGameReport(false, 80);
 		g_system->logMessage(LogMessageType::kInfo, report.c_str());
-
-		UnknownGameDialog dialog(detectionResults);
-		dialog.runModal();
 	}
 
-	Common::Array<DetectedGame> candidates = detectionResults.listRecognizedGames();
+	Common::Array<DetectedGame> candidates = detectionResults.listDetectedGames();
 
 	int idx;
 	if (candidates.empty()) {
@@ -583,16 +580,37 @@ bool LauncherDialog::doGameDetection(const Common::String &path) {
 	} else {
 		// Display the candidates to the user and let her/him pick one
 		StringArray list;
-		for (idx = 0; idx < (int)candidates.size(); idx++)
-			list.push_back(candidates[idx].description);
+		for (idx = 0; idx < (int)candidates.size(); idx++) {
+			Common::String description = candidates[idx].description;
+
+			if (candidates[idx].hasUnknownFiles) {
+				description += " - ";
+				description += _("Unknown variant");
+			}
+
+			list.push_back(description);
+		}
 
 		ChooserDialog dialog(_("Pick the game:"));
 		dialog.setList(list);
 		idx = dialog.runModal();
 	}
+
 	if (0 <= idx && idx < (int)candidates.size()) {
 		const DetectedGame &result = candidates[idx];
 
+		if (result.hasUnknownFiles) {
+			UnknownGameDialog dialog(result);
+
+			bool cancel = dialog.runModal() == -1;
+			if (cancel) {
+				idx = -1;
+			}
+		}
+	}
+
+	if (0 <= idx && idx < (int)candidates.size()) {
+		const DetectedGame &result = candidates[idx];
 		Common::String domain = EngineMan.createTargetForGame(result);
 
 		// Display edit dialog for the new entry
diff --git a/gui/unknown-game-dialog.cpp b/gui/unknown-game-dialog.cpp
index d96791742e..0aa8cc5750 100644
--- a/gui/unknown-game-dialog.cpp
+++ b/gui/unknown-game-dialog.cpp
@@ -38,14 +38,21 @@ enum {
 	kCopyToClipboard = 'cpcl',
 	kOpenBugtrackerURL = 'ourl',
 	kClose = 'clse',
+	kAddAnyway = 'adda',
 	kScrollContainerReflow = 'SCRf'
 };
 
-UnknownGameDialog::UnknownGameDialog(const DetectionResults &detectionResults) :
+UnknownGameDialog::UnknownGameDialog(const DetectedGame &detectedGame) :
 		Dialog(30, 20, 260, 124),
-		_detectionResults(detectionResults) {
+		_detectedGame(detectedGame) {
 	// For now place the buttons with a default place and size. They will be resized and moved when rebuild() is called.
-	_closeButton = new ButtonWidget(this, 0, 0, 0, 0, _("Close"), 0, kClose);
+	_closeButton = new ButtonWidget(this, 0, 0, 0, 0, detectedGame.canBeAdded ? _("Cancel") : _("Close"), 0, kClose);
+
+	if (detectedGame.canBeAdded) {
+		_addAnywayButton = new ButtonWidget(this, 0, 0, 0, 0, _("Add anyway"), 0, kAddAnyway);
+	} else {
+		_addAnywayButton = nullptr;
+	}
 
 	//Check if we have clipboard functionality
 	if (g_system->hasFeature(OSystem::kFeatureClipboardSupport)) {
@@ -76,6 +83,8 @@ void UnknownGameDialog::reflowLayout() {
 }
 
 void UnknownGameDialog::rebuild() {
+	// TODO: Use a theme layout dialog definition
+
 	// First remove the old text widgets
 	for (uint i = 0; i < _textWidgets.size() ; i++) {
 		_textContainer->removeWidget(_textWidgets[i]);
@@ -91,7 +100,7 @@ void UnknownGameDialog::rebuild() {
 	int buttonHeight = g_gui.xmlEval()->getVar("Globals.Button.Height", 0);
 	int buttonWidth = g_gui.xmlEval()->getVar("Globals.Button.Width", 0);
 
-	Common::String reportTranslated = _detectionResults.generateUnknownGameReport(true);
+	Common::String reportTranslated = generateUnknownGameReport(_detectedGame, true, true);
 
 	// Check if we have clipboard functionality and expand the reportTranslated message if needed...
 	if (g_system->hasFeature(OSystem::kFeatureClipboardSupport)) {
@@ -116,7 +125,9 @@ void UnknownGameDialog::rebuild() {
 	_h = MIN(screenH - 20, lineCount * kLineHeight + kLineHeight + buttonHeight + 24);
 
 	int closeButtonWidth = MAX(buttonWidth, g_gui.getFont().getStringWidth(_closeButton->getLabel()) + 10);
-	int copyToClipboardButtonWidth = 0, openBugtrackerURLButtonWidth = 0, totalButtonWidth = closeButtonWidth;
+	int copyToClipboardButtonWidth = 0, openBugtrackerURLButtonWidth = 0, addAnywayButtonWidth = 0;
+	int totalButtonWidth = closeButtonWidth;
+
 	if (_copyToClipboardButton) {
 		copyToClipboardButtonWidth = MAX(buttonWidth, g_gui.getFont().getStringWidth(_copyToClipboardButton->getLabel()) + 10);
 		totalButtonWidth += copyToClipboardButtonWidth + 10;
@@ -125,6 +136,10 @@ void UnknownGameDialog::rebuild() {
 		openBugtrackerURLButtonWidth = MAX(buttonWidth, g_gui.getFont().getStringWidth(_openBugTrackerUrlButton->getLabel()) + 10);
 		totalButtonWidth += openBugtrackerURLButtonWidth + 10;
 	}
+	if (_addAnywayButton) {
+		addAnywayButtonWidth = MAX(buttonWidth, g_gui.getFont().getStringWidth(_addAnywayButton->getLabel()) + 10);
+		totalButtonWidth += addAnywayButtonWidth + 10;
+	}
 
 	_w = MAX(MAX(maxlineWidth, 0) + 16 + scrollbarWidth, totalButtonWidth) + 20;
 
@@ -133,7 +148,12 @@ void UnknownGameDialog::rebuild() {
 	_y = (g_system->getOverlayHeight() - _h) / 2;
 
 	// Now move the buttons and text container to their proper place
-	int buttonPos = _w - closeButtonWidth - 10;
+	int buttonPos = _w - 10;
+	if (_addAnywayButton) {
+		buttonPos -= addAnywayButtonWidth + 5;
+		_addAnywayButton->resize(buttonPos, _h - buttonHeight - 8, addAnywayButtonWidth, buttonHeight);
+	}
+	buttonPos -= closeButtonWidth + 5;
 	_closeButton->resize(buttonPos, _h - buttonHeight - 8, closeButtonWidth, buttonHeight);
 	if (_copyToClipboardButton) {
 		buttonPos -= copyToClipboardButtonWidth + 5;
@@ -156,7 +176,7 @@ void UnknownGameDialog::rebuild() {
 	}
 }
 
-Common::String UnknownGameDialog::encodeUrlString(const Common::String& string) {
+Common::String UnknownGameDialog::encodeUrlString(const Common::String &string) {
 	Common::String encoded;
 	for (uint i = 0 ; i < string.size() ; ++i) {
 		char c = string[i];
@@ -170,34 +190,10 @@ Common::String UnknownGameDialog::encodeUrlString(const Common::String& string)
 }
 
 Common::String UnknownGameDialog::generateBugtrackerURL() {
-	Common::String report = _detectionResults.generateUnknownGameReport(false);
-	// Remove the filesystem path from the bugtracker report.
-	// The path is on the first line between single quotes. We strip everything except the last level.
-	int path_start = -1, path_size = 0;
-	for (uint i = 0 ; i < report.size() ; ++i) {
-		char c = report[i];
-		if (c == '\'') {
-			if (path_start == -1)
-				path_start = i + 1;
-			else
-				break;
-		} else if (path_start != -1 && (c == '/' || c == '\\')) {
-			path_size = 1 + i - path_start;
-		} else if (c == '\n') {
-			path_size = 0;
-			break;
-		}
-	}
-	if (path_size > 0)
-		report.erase(path_start, path_size);
+	Common::String report = generateUnknownGameReport(_detectedGame, false, false);
 	report = encodeUrlString(report);
 
-	// Pass engine name if there is only one.
-	Common::String engineName = "unknown";
-	Common::StringArray engines = _detectionResults.getUnknownGameEngines();
-	if (engines.size() == 1)
-		engineName = engines.front();
-	engineName = encodeUrlString(engineName);
+	Common::String engineName = encodeUrlString(_detectedGame.engineName);
 
 	return Common::String::format(
 		"https://www.scummvm.org/unknowngame?"
@@ -210,8 +206,7 @@ Common::String UnknownGameDialog::generateBugtrackerURL() {
 void UnknownGameDialog::handleCommand(CommandSender *sender, uint32 cmd, uint32 data) {
 	switch(cmd) {
 	case kCopyToClipboard: {
-		// TODO: Remove the filesystem path from the report
-		Common::String report = _detectionResults.generateUnknownGameReport(false);
+		Common::String report = generateUnknownGameReport(_detectedGame, false, false);
 
 		if (g_system->setTextInClipboard(report)) {
 			g_system->displayMessageOnOSD(
@@ -222,8 +217,12 @@ void UnknownGameDialog::handleCommand(CommandSender *sender, uint32 cmd, uint32
 		break;
 	}
 	case kClose:
+		// The user cancelled adding the game
+		setResult(-1);
+		close();
+		break;
+	case kAddAnyway:
 		// When the detection entry comes from the fallback detector, the game can be added / launched anyways.
-		// TODO: Add a button to cancel adding the game. And make it clear that launching the game may not work properly.
 		close();
 		break;
 	case kOpenBugtrackerURL:
diff --git a/gui/unknown-game-dialog.h b/gui/unknown-game-dialog.h
index 30ac13cc84..24e4179bf4 100644
--- a/gui/unknown-game-dialog.h
+++ b/gui/unknown-game-dialog.h
@@ -35,9 +35,9 @@ class ButtonWidget;
 
 class UnknownGameDialog : public Dialog {
 public:
-	UnknownGameDialog(const DetectionResults &detectionResults);
+	UnknownGameDialog(const DetectedGame &detectedGame);
 
-	void handleMouseWheel(int x, int y, int direction);
+	void handleMouseWheel(int x, int y, int direction) override;
 private:
 	void rebuild();
 
@@ -46,14 +46,15 @@ private:
 	void reflowLayout() override;
 
 	Common::String generateBugtrackerURL();
-	Common::String encodeUrlString(const Common::String&);
+	static Common::String encodeUrlString(const Common::String &string);
 
-	const DetectionResults &_detectionResults;
+	const DetectedGame &_detectedGame;
 	ScrollContainerWidget *_textContainer;
 	Common::Array<StaticTextWidget *> _textWidgets;
-	ButtonWidget* _openBugTrackerUrlButton;
-	ButtonWidget* _copyToClipboardButton;
-	ButtonWidget* _closeButton;
+	ButtonWidget *_openBugTrackerUrlButton;
+	ButtonWidget *_copyToClipboardButton;
+	ButtonWidget *_closeButton;
+	ButtonWidget *_addAnywayButton;
 };
 
 } // End of namespace GUI