Merge pull request #19432 from hrydgard/more-zip-install-work

Some refactoring of zip install code

Core/Util/GameManager.cpp

@@ -167,7 +167,11 @@ void GameManager::Update() {
 				return;
 			}
 			// Game downloaded to temporary file - install it!
-			InstallGameOnThread(Path(curDownload_->url()), fileName, true);
+			ZipFileTask task;
+			task.url = Path(curDownload_->url());
+			task.fileName = fileName;
+			task.deleteAfter = true;
+			InstallZipOnThread(task);
 		} else {
 			ERROR_LOG(Log::HLE, "Expected HTTP status code 200, got status code %d. Install cancelled, deleting partial file '%s'",
 				curDownload_->ResultCode(), fileName.c_str());
@@ -199,14 +203,15 @@ static void countSlashes(const std::string &fileName, int *slashLocation, int *s
 	}
 }
 
-ZipFileContents DetectZipFileContents(const Path &fileName, ZipFileInfo *info) {
+bool DetectZipFileContents(const Path &fileName, ZipFileInfo *info) {
 	struct zip *z = ZipOpenPath(fileName);
 	if (!z) {
-		return ZipFileContents::UNKNOWN;
+		info->contents = ZipFileContents::UNKNOWN;
+		return false;
 	}
-	ZipFileContents retVal = DetectZipFileContents(z, info);
+	DetectZipFileContents(z, info);
 	zip_close(z);
-	return retVal;
+	return true;
 }
 
 inline char asciitolower(char in) {
@@ -215,7 +220,7 @@ inline char asciitolower(char in) {
 	return in;
 }
 
-ZipFileContents DetectZipFileContents(struct zip *z, ZipFileInfo *info) {
+void DetectZipFileContents(struct zip *z, ZipFileInfo *info) {
 	int numFiles = zip_get_num_files(z);
 
 	// Verify that this is a PSP zip file with the correct layout. We also try
@@ -280,83 +285,96 @@ ZipFileContents DetectZipFileContents(struct zip *z, ZipFileInfo *info) {
 
 	// If a ZIP is detected as both, let's let the memstick game interpretation prevail.
 	if (isPSPMemstickGame) {
-		return ZipFileContents::PSP_GAME_DIR;
+		info->contents = ZipFileContents::PSP_GAME_DIR;
 	} else if (isZippedISO) {
-		return ZipFileContents::ISO_FILE;
+		info->contents = ZipFileContents::ISO_FILE;
 	} else if (isTexturePack) {
 		info->stripChars = stripCharsTexturePack;
 		info->ignoreMetaFiles = true;
-		return ZipFileContents::TEXTURE_PACK;
+		info->contents = ZipFileContents::TEXTURE_PACK;
 	} else {
-		return ZipFileContents::UNKNOWN;
+		info->contents = ZipFileContents::UNKNOWN;
 	}
 }
 
 // Parameters need to be by value, since this is a thread func.
-bool GameManager::InstallGame(const Path &url, const Path &fileName, bool deleteAfter) {
-	SetCurrentThreadName("InstallGame");
+void GameManager::InstallZipContents(ZipFileTask task) {
+	SetCurrentThreadName("InstallZipContents");
 
 	if (installDonePending_) {
 		ERROR_LOG(Log::HLE, "Cannot have two installs in progress at the same time");
-		return false;
+		return;
 	}
 
 	AndroidJNIThreadContext context;  // Destructor detaches.
-	if (!File::Exists(fileName)) {
-		ERROR_LOG(Log::HLE, "Game file '%s' doesn't exist", fileName.c_str());
-		return false;
+	if (!File::Exists(task.fileName)) {
+		ERROR_LOG(Log::HLE, "Game file '%s' doesn't exist", task.fileName.c_str());
+		return;
 	}
 
 	auto st = GetI18NCategory(I18NCat::STORE);
 	auto di = GetI18NCategory(I18NCat::DIALOG);
 	auto sy = GetI18NCategory(I18NCat::SYSTEM);
 
-	std::string extension = url.GetFileExtension();
+	std::string urlExtension = task.url.GetFileExtension();
 	// Examine the URL to guess out what we're installing.
-	if (extension == ".cso" || extension == ".iso" || extension == ".chd") {
+	// TODO: Bad idea due to Android content api where we don't always get the filename.
+	if (urlExtension == ".cso" || urlExtension == ".iso" || urlExtension == ".chd") {
 		// It's a raw ISO or CSO file. We just copy it to the destination.
-		std::string shortFilename = url.GetFilename();
-		bool success = InstallRawISO(fileName, shortFilename, deleteAfter);
-		return success;
+		std::string shortFilename = task.url.GetFilename();
+		bool success = InstallRawISO(task.fileName, shortFilename, task.deleteAfter);
+		if (!success) {
+			ERROR_LOG(Log::HLE, "Raw ISO install failed");
+			// This shouldn't normally happen at all (only when putting ISOs in a store, which is not a normal use case), so skipping the translation string
+			SetInstallError("Failed to install raw ISO");
+		}
+		return;
 	}
 
 	Path pspGame = GetSysDirectory(DIRECTORY_GAME);
 	Path dest = pspGame;
 	int error = 0;
 
-	struct zip *z = ZipOpenPath(fileName);
+	struct zip *z = ZipOpenPath(task.fileName);
 	if (!z) {
 		g_OSD.RemoveProgressBar("install", false, 0.5f);
 		SetInstallError(sy->T("Unable to open zip file"));
-		return false;
+		return;
 	}
 
-	ZipFileInfo info;
-	ZipFileContents contents = DetectZipFileContents(z, &info);
 	bool success = false;
-	switch (contents) {
+
+	ZipFileInfo zipInfo;
+	if (task.zipFileInfo) {
+		// The normal case
+		zipInfo = task.zipFileInfo.value();
+	} else {
+		DetectZipFileContents(task.fileName, &zipInfo);
+	}
+
+	switch (zipInfo.contents) {
 	case ZipFileContents::PSP_GAME_DIR:
-		INFO_LOG(Log::HLE, "Installing '%s' into '%s'", fileName.c_str(), pspGame.c_str());
+		INFO_LOG(Log::HLE, "Installing '%s' into '%s'", task.fileName.c_str(), pspGame.c_str());
 		// InstallMemstickGame contains code to close (and delete) z.
-		success = InstallMemstickGame(z, fileName, pspGame, info, false, deleteAfter);
+		success = InstallMemstickGame(z, task.fileName, pspGame, zipInfo, false, task.deleteAfter);
 		break;
 	case ZipFileContents::ISO_FILE:
-		INFO_LOG(Log::HLE, "Installing '%s' into its containing directory", fileName.c_str());
+		INFO_LOG(Log::HLE, "Installing '%s' into its containing directory", task.fileName.c_str());
 		// InstallZippedISO contains code to close z.
-		success = InstallZippedISO(z, info.isoFileIndex, fileName, deleteAfter);
+		success = InstallZippedISO(z, zipInfo.isoFileIndex, task.fileName, task.deleteAfter);
 		break;
 	case ZipFileContents::TEXTURE_PACK:
 		// InstallMemstickGame contains code to close z, and works for textures too.
-		if (DetectTexturePackDest(z, info.textureIniIndex, dest)) {
-			INFO_LOG(Log::HLE, "Installing texture pack '%s' into '%s'", fileName.c_str(), dest.c_str());
+		if (DetectTexturePackDest(z, zipInfo.textureIniIndex, dest)) {
+			INFO_LOG(Log::HLE, "Installing texture pack '%s' into '%s'", task.fileName.c_str(), dest.c_str());
 			File::CreateFullPath(dest);
 			// Install as a zip file if textures.ini is in the root. Performs better on Android.
-			if (info.stripChars == 0) {
-				success = InstallMemstickZip(z, fileName, dest / "textures.zip", info, deleteAfter);
+			if (zipInfo.stripChars == 0) {
+				success = InstallMemstickZip(z, task.fileName, dest / "textures.zip", zipInfo, task.deleteAfter);
 			} else {
 				// TODO: Can probably remove this, as we now put .nomedia in /TEXTURES directly.
 				File::CreateEmptyFile(dest / ".nomedia");
-				success = InstallMemstickGame(z, fileName, dest, info, true, deleteAfter);
+				success = InstallMemstickGame(z, task.fileName, dest, zipInfo, true, task.deleteAfter);
 			}
 		} else {
 			zip_close(z);
@@ -368,12 +386,12 @@ bool GameManager::InstallGame(const Path &url, const Path &fileName, bool delete
 		SetInstallError(sy->T("Not a PSP game"));
 		zip_close(z);
 		z = nullptr;
-		if (deleteAfter)
-			File::Delete(fileName);
+		if (task.deleteAfter) {
+			File::Delete(task.fileName);
+		}
 		break;
 	}
 	g_OSD.RemoveProgressBar("install", success, 0.5f);
-	return success;
 }
 
 bool GameManager::DetectTexturePackDest(struct zip *z, int iniIndex, Path &dest) {
@@ -769,11 +787,12 @@ bool GameManager::InstallZippedISO(struct zip *z, int isoFileIndex, const Path &
 	return true;
 }
 
-bool GameManager::InstallGameOnThread(const Path &url, const Path &fileName, bool deleteAfter) {
+bool GameManager::InstallZipOnThread(ZipFileTask task) {
 	if (InstallInProgress() || installDonePending_) {
 		return false;
 	}
-	installThread_ = std::thread(std::bind(&GameManager::InstallGame, this, url, fileName, deleteAfter));
+
+	installThread_ = std::thread(std::bind(&GameManager::InstallZipContents, this, task));
 	return true;
 }
 

Core/Util/GameManager.h

@@ -24,6 +24,7 @@
 
 #include <thread>
 #include <atomic>
+#include <optional>
 
 #include "Common/Net/HTTPClient.h"
 #include "Common/File/Path.h"
@@ -34,6 +35,29 @@ enum class GameManagerState {
 	INSTALLING,
 };
 
+enum class ZipFileContents {
+	UNKNOWN,
+	PSP_GAME_DIR,
+	ISO_FILE,
+	TEXTURE_PACK,
+};
+
+struct ZipFileInfo {
+	ZipFileContents contents;
+	int numFiles;
+	int stripChars;  // for PSP game
+	int isoFileIndex;  // for ISO
+	int textureIniIndex;  // for textures
+	bool ignoreMetaFiles;
+};
+
+struct ZipFileTask {
+	std::optional<ZipFileInfo> zipFileInfo;
+	Path url;  // Same as filename if installing from disk. Probably not really useful.
+	Path fileName;
+	bool deleteAfter;
+};
+
 struct zip;
 class FileLoader;
 struct ZipFileInfo;
@@ -74,12 +98,14 @@ public:
 	}
 
 	// Only returns false if there's already an installation in progress.
-	bool InstallGameOnThread(const Path &url, const Path &tempFileName, bool deleteAfter);
+	bool InstallZipOnThread(ZipFileTask task);
+
+	// Separate kind of functionality from InstallZipOnThread, so doesn't re-use the task struct.
 	bool UninstallGameOnThread(const std::string &name);
 
 private:
-	// TODO: The return value on this is a bit pointless, we can't get at it.
-	bool InstallGame(const Path &url, const Path &tempFileName, bool deleteAfter);
+	void InstallZipContents(ZipFileTask task);
+
 	bool InstallMemstickGame(struct zip *z, const Path &zipFile, const Path &dest, const ZipFileInfo &info, bool allowRoot, bool deleteAfter);
 	bool InstallMemstickZip(struct zip *z, const Path &zipFile, const Path &dest, const ZipFileInfo &info, bool deleteAfter);
 	bool InstallZippedISO(struct zip *z, int isoFileIndex, const Path &zipfile, bool deleteAfter);
@@ -109,20 +135,5 @@ private:
 
 extern GameManager g_GameManager;
 
-enum class ZipFileContents {
-	UNKNOWN,
-	PSP_GAME_DIR,
-	ISO_FILE,
-	TEXTURE_PACK,
-};
-
-struct ZipFileInfo {
-	int numFiles;
-	int stripChars;  // for PSP game
-	int isoFileIndex;  // for ISO
-	int textureIniIndex;  // for textures
-	bool ignoreMetaFiles;
-};
-
-ZipFileContents DetectZipFileContents(struct zip *z, ZipFileInfo *info);
-ZipFileContents DetectZipFileContents(const Path &fileName, ZipFileInfo *info);
+void DetectZipFileContents(struct zip *z, ZipFileInfo *info);
+bool DetectZipFileContents(const Path &fileName, ZipFileInfo *info);

UI/GameScreen.cpp

@@ -541,8 +541,11 @@ UI::EventReturn GameScreen::OnDeleteGame(UI::EventParams &e) {
 	if (info->Ready(GameInfoFlags::PARAM_SFO)) {
 		auto di = GetI18NCategory(I18NCat::DIALOG);
 		auto ga = GetI18NCategory(I18NCat::GAME);
+		std::string prompt;
+		prompt = di->T("DeleteConfirmGame", "Do you really want to delete this game\nfrom your device? You can't undo this.");
+		prompt += "\n\n" + gamePath_.ToVisualString(g_Config.memStickDirectory.c_str());
 		screenManager()->push(
-			new PromptScreen(gamePath_, di->T("DeleteConfirmGame", "Do you really want to delete this game\nfrom your device? You can't undo this."), ga->T("ConfirmDelete"), di->T("Cancel"),
+			new PromptScreen(gamePath_, prompt, ga->T("ConfirmDelete"), di->T("Cancel"),
 			std::bind(&GameScreen::CallbackDeleteGame, this, std::placeholders::_1)));
 	}
 	return UI::EVENT_DONE;

UI/InstallZipScreen.cpp

@@ -50,10 +50,9 @@ void InstallZipScreen::CreateViews() {
 	std::string shortFilename = zipPath_.GetFilename();
 	
 	// TODO: Do in the background?
-	ZipFileInfo zipInfo{};
-	ZipFileContents contents = DetectZipFileContents(zipPath_, &zipInfo);
+	DetectZipFileContents(zipPath_, &zipFileInfo_);  // Even if this fails, it sets zipInfo->contents.
 
-	if (contents == ZipFileContents::ISO_FILE || contents == ZipFileContents::PSP_GAME_DIR) {
+	if (zipFileInfo_.contents == ZipFileContents::ISO_FILE || zipFileInfo_.contents == ZipFileContents::PSP_GAME_DIR) {
 		std::string_view question = iz->T("Install game from ZIP file?");
 		leftColumn->Add(new TextView(question, ALIGN_LEFT, false, new AnchorLayoutParams(10, 10, NONE, NONE)));
 		leftColumn->Add(new TextView(shortFilename, ALIGN_LEFT, false, new AnchorLayoutParams(10, 60, NONE, NONE)));
@@ -67,7 +66,7 @@ void InstallZipScreen::CreateViews() {
 		rightColumnItems->Add(new CheckBox(&deleteZipFile_, iz->T("Delete ZIP file")));
 
 		returnToHomebrew_ = true;
-	} else if (contents == ZipFileContents::TEXTURE_PACK) {
+	} else if (zipFileInfo_.contents == ZipFileContents::TEXTURE_PACK) {
 		std::string_view question = iz->T("Install textures from ZIP file?");
 		leftColumn->Add(new TextView(question, ALIGN_LEFT, false, new AnchorLayoutParams(10, 10, NONE, NONE)));
 		leftColumn->Add(new TextView(shortFilename, ALIGN_LEFT, false, new AnchorLayoutParams(10, 60, NONE, NONE)));
@@ -102,7 +101,12 @@ bool InstallZipScreen::key(const KeyInput &key) {
 }
 
 UI::EventReturn InstallZipScreen::OnInstall(UI::EventParams &params) {
-	if (g_GameManager.InstallGameOnThread(zipPath_, zipPath_, deleteZipFile_)) {
+	ZipFileTask task;
+	task.url = zipPath_;
+	task.fileName = zipPath_;
+	task.deleteAfter = deleteZipFile_;
+	task.zipFileInfo = zipFileInfo_;
+	if (g_GameManager.InstallZipOnThread(task)) {
 		installStarted_ = true;
 		if (installChoice_) {
 			installChoice_->SetEnabled(false);

UI/InstallZipScreen.h

@@ -43,6 +43,7 @@ private:
 	UI::ProgressBar *progressBar_ = nullptr;
 	UI::TextView *doneView_ = nullptr;
 	Path zipPath_;
+	ZipFileInfo zipFileInfo_{};
 	bool returnToHomebrew_ = true;
 	bool installStarted_ = false;
 	bool deleteZipFile_ = false;