Android memstick folder moves: Do recursive moves if in the same devi…

…ce. Drastically faster.

Common/File/FileUtil.cpp

@@ -628,13 +628,12 @@ bool Rename(const Path &srcFilename, const Path &destFilename) {
 		break;
 	case PathType::CONTENT_URI:
 		// Content URI: Can only rename if in the same folder.
-		// TODO: Fallback to move + rename? Or do we even care about that use case?
+		// TODO: Fallback to move + rename? Or do we even care about that use case? We have MoveIfFast for such tricks.
 		if (srcFilename.GetDirectory() != destFilename.GetDirectory()) {
 			INFO_LOG(COMMON, "Content URI rename: Directories not matching, failing. %s --> %s", srcFilename.c_str(), destFilename.c_str());
 			return false;
 		}
 		INFO_LOG(COMMON, "Content URI rename: %s --> %s", srcFilename.c_str(), destFilename.c_str());
-
 		return Android_RenameFileTo(srcFilename.ToString(), destFilename.GetFilename()) == StorageError::SUCCESS;
 	default:
 		return false;
@@ -752,30 +751,26 @@ bool Copy(const Path &srcFilename, const Path &destFilename) {
 
 // Will overwrite the target.
 bool Move(const Path &srcFilename, const Path &destFilename) {
-	// Try a shortcut in Android Storage scenarios.
-	if (srcFilename.Type() == PathType::CONTENT_URI && destFilename.Type() == PathType::CONTENT_URI && srcFilename.CanNavigateUp() && destFilename.CanNavigateUp()) {
-		// We do not handle simultaneous renames here.
-		if (srcFilename.GetFilename() == destFilename.GetFilename()) {
-			Path srcParent = srcFilename.NavigateUp();
-			Path dstParent = destFilename.NavigateUp();
-			if (Android_MoveFile(srcFilename.ToString(), srcParent.ToString(), dstParent.ToString()) == StorageError::SUCCESS) {
-				return true;
-			}
-			// If failed, fall through and try other ways.
-		}
-	}
-
-	if (Rename(srcFilename, destFilename)) {
+	bool fast = MoveIfFast(srcFilename, destFilename);
+	if (fast) {
 		return true;
-	} else if (Copy(srcFilename, destFilename)) {
+	}
+	// OK, that failed, so fall back on a copy.
+	if (Copy(srcFilename, destFilename)) {
 		return Delete(srcFilename);
 	} else {
 		return false;
 	}
 }
 
 bool MoveIfFast(const Path &srcFilename, const Path &destFilename) {
-	if (srcFilename.Type() == PathType::CONTENT_URI && destFilename.Type() == PathType::CONTENT_URI && srcFilename.CanNavigateUp() && destFilename.CanNavigateUp()) {
+	if (srcFilename.Type() != destFilename.Type()) {
+		// No way it's gonna work.
+		return false;
+	}
+
+	// Only need to check one type here, due to the above check.
+	if (srcFilename.Type() == PathType::CONTENT_URI && srcFilename.CanNavigateUp() && destFilename.CanNavigateUp()) {
 		if (srcFilename.GetFilename() == destFilename.GetFilename()) {
 			Path srcParent = srcFilename.NavigateUp();
 			Path dstParent = destFilename.NavigateUp();
@@ -787,11 +782,7 @@ bool MoveIfFast(const Path &srcFilename, const Path &destFilename) {
 		}
 	}
 
-	if (srcFilename.Type() != destFilename.Type()) {
-		// No way it's gonna work.
-		return false;
-	}
-
+	// Try a traditional rename operation.
 	return Rename(srcFilename, destFilename);
 }
 

Common/File/FileUtil.h

@@ -97,7 +97,7 @@ bool DeleteDir(const Path &filename);
 // Deletes the given directory and anything under it. Returns true on success.
 bool DeleteDirRecursively(const Path &directory);
 
-// Renames file srcFilename to destFilename, returns true on success 
+// Renames/moves file srcFilename to destFilename, returns true on success 
 // Will usually only work with in the same partition or other unit of storage,
 // so you might have to fall back to copy/delete.
 bool Rename(const Path &srcFilename, const Path &destFilename);

Core/Util/GameManager.cpp

@@ -345,6 +345,7 @@ bool GameManager::InstallGame(const Path &url, const Path &fileName, bool delete
 			if (info.stripChars == 0) {
 				success = InstallMemstickZip(z, fileName, dest / "textures.zip", info, 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);
 			}

Core/Util/MemStick.cpp

@@ -62,7 +62,6 @@ bool SwitchMemstickFolderTo(Path newMemstickFolder) {
 	return true;
 }
 
-
 // Keep the size with the file, so we can skip overly large ones in the move.
 // The user will have to take care of them afterwards, it'll just take too long probably.
 struct FileSuffix {
@@ -100,6 +99,28 @@ static bool ListFileSuffixesRecursively(const Path &root, const Path &folder, st
 	return true;
 }
 
+bool MoveChildrenFast(const Path &moveSrc, const Path &moveDest, MoveProgressReporter &progressReporter) {
+	std::vector<File::FileInfo> files;
+	progressReporter.SetStatus("Starting move...");
+	if (!File::GetFilesInDir(moveSrc, &files)) {
+		return false;
+	}
+
+	for (auto file : files) {
+		// Construct destination path
+		Path fileSrc = file.fullName;
+		Path fileDest = moveDest / file.name;
+		progressReporter.SetStatus(file.name);
+		INFO_LOG(SYSTEM, "About to move PSP data from '%s' to '%s'", fileSrc.c_str(), fileDest.c_str());
+		bool result = File::MoveIfFast(fileSrc, fileDest);
+		if (!result) {
+			// TODO: Should we try to move back anything that succeeded before this one?
+			return false;
+		}
+	}
+	return true;
+}
+
 MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressReporter &progressReporter) {
 	auto ms = GetI18NCategory(I18NCat::MEMSTICK);
 	if (moveSrc.GetFilename() != "PSP") {
@@ -112,6 +133,20 @@ MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressR
 
 	INFO_LOG(SYSTEM, "About to move PSP data from '%s' to '%s'", moveSrc.c_str(), moveDest.c_str());
 
+	// First, we try the cheapest and safest way to move: Can we move files directly within the same device?
+	// We loop through the files/dirs in the source directory and just try to move them, it should work.
+	if (MoveChildrenFast(moveSrc, moveDest, progressReporter)) {
+		INFO_LOG(SYSTEM, "Quick-move succeeded");
+		progressReporter.SetStatus(ms->T("Done!"));
+		return new MoveResult{
+			true, ""
+		};
+	}
+
+	// If this doesn't work, we'll fall back on a recursive *copy* (disk space is less of a concern when
+	// moving from device to device, other than that everything fits on the destination).
+	// Then we verify the results before we delete the originals.
+
 	// Search through recursively, listing the files to move and also summing their sizes.
 	std::vector<FileSuffix> fileSuffixesToMove;
 	std::vector<std::string> directorySuffixesToCreate;
@@ -121,7 +156,7 @@ MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressR
 		// TODO: Handle failure listing files.
 		std::string error = "Failed to read old directory";
 		INFO_LOG(SYSTEM, "%s", error.c_str());
-		progressReporter.Set(ms->T(error.c_str()));
+		progressReporter.SetStatus(ms->T(error.c_str()));
 		return new MoveResult{ false, error };
 	}
 
@@ -144,14 +179,14 @@ MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressR
 				INFO_LOG(SYSTEM, "dry run: Would have created dir '%s'", dir.c_str());
 			} else {
 				INFO_LOG(SYSTEM, "Creating dir '%s'", dir.c_str());
-				progressReporter.Set(dirSuffix);
+				progressReporter.SetStatus(dirSuffix);
 				// Just ignore already-exists errors.
 				File::CreateDir(dir);
 			}
 		}
 
 		for (auto &fileSuffix : fileSuffixesToMove) {
-			progressReporter.Set(StringFromFormat("%s (%s)", fileSuffix.suffix.c_str(), NiceSizeFormat(fileSuffix.fileSize).c_str()));
+			progressReporter.SetStatus(StringFromFormat("%s (%s)", fileSuffix.suffix.c_str(), NiceSizeFormat(fileSuffix.fileSize).c_str()));
 
 			Path from = moveSrc / fileSuffix.suffix;
 			Path to = moveDest / fileSuffix.suffix;
@@ -163,8 +198,7 @@ MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressR
 				} else {
 					if (!File::MoveIfFast(from, to)) {
 						INFO_LOG(SYSTEM, "Skipped moving file '%s' to '%s' (%s)", from.c_str(), to.c_str(), NiceSizeFormat(fileSuffix.fileSize).c_str());
-						skippedFiles++;
-					} else {
+						skippedFiles++; 
 						INFO_LOG(SYSTEM, "Moved file '%s' to '%s'", from.c_str(), to.c_str());
 					}
 				}
@@ -193,7 +227,7 @@ MoveResult *MoveDirectoryContentsSafe(Path moveSrc, Path moveDest, MoveProgressR
 				INFO_LOG(SYSTEM, "dry run: Would have deleted dir '%s'", dir.c_str());
 			} else {
 				INFO_LOG(SYSTEM, "Deleting dir '%s'", dir.c_str());
-				progressReporter.Set(dirSuffix);
+				progressReporter.SetStatus(dirSuffix);
 				if (File::Exists(dir)) {
 					File::DeleteDir(dir);
 				}

Core/Util/MemStick.h

@@ -8,7 +8,7 @@
 
 class MoveProgressReporter {
 public:
-	void Set(const std::string &value) {
+	void SetStatus(const std::string &value) {
 		std::lock_guard<std::mutex> guard(mutex_);
 		progress_ = value;
 	}

GPU/Common/TextureReplacer.cpp

@@ -87,8 +87,10 @@ void TextureReplacer::NotifyConfigChanged() {
 
 		// If we're saving, auto-create the directory.
 		if (g_Config.bSaveNewTextures && !File::Exists(newTextureDir_)) {
+			INFO_LOG(G3D, "Creating new texture directory: '%s'", newTextureDir_.ToVisualString().c_str());
 			File::CreateFullPath(newTextureDir_);
-			File::CreateEmptyFile(newTextureDir_ / ".nomedia");
+			// We no longer create a nomedia file here, since we put one
+			// in the TEXTURES root.
 		}
 
 		enabled_ = File::IsDirectory(basePath_);

UI/MemStickScreen.cpp

@@ -552,12 +552,12 @@ void ConfirmMemstickMoveScreen::update() {
 
 		if (result) {
 			if (result->success) {
-				progressReporter_.Set(iz->T("Done!"));
+				progressReporter_.SetStatus(iz->T("Done!"));
 				INFO_LOG(SYSTEM, "Move data task finished successfully!");
 				// Succeeded!
 				FinishFolderMove();
 			} else {
-				progressReporter_.Set(iz->T("Failed to move some files!"));
+				progressReporter_.SetStatus(iz->T("Failed to move some files!"));
 				INFO_LOG(SYSTEM, "Move data task failed!");
 				// What do we do here? We might be in the middle of a move... Bad.
 				RecreateViews();
@@ -575,7 +575,7 @@ UI::EventReturn ConfirmMemstickMoveScreen::OnConfirm(UI::EventParams &params) {
 	// If the directory itself is called PSP, don't go below.
 
 	if (moveData_) {
-		progressReporter_.Set(T(I18NCat::MEMSTICK, "Starting move..."));
+		progressReporter_.SetStatus(T(I18NCat::MEMSTICK, "Starting move..."));
 
 		moveDataTask_ = Promise<MoveResult *>::Spawn(&g_threadManager, [&]() -> MoveResult * {
 			Path moveSrc = g_Config.memStickDirectory;
@@ -594,6 +594,8 @@ UI::EventReturn ConfirmMemstickMoveScreen::OnConfirm(UI::EventParams &params) {
 void ConfirmMemstickMoveScreen::FinishFolderMove() {
 	auto ms = GetI18NCategory(I18NCat::MEMSTICK);
 
+	Path oldMemstickFolder = g_Config.memStickDirectory;
+
 	// Successful so far, switch the memstick folder.
 	if (!SwitchMemstickFolderTo(newMemstickFolder_)) {
 		// TODO: More precise errors.
@@ -603,6 +605,12 @@ void ConfirmMemstickMoveScreen::FinishFolderMove() {
 
 	// If the chosen folder already had a config, reload it!
 	g_Config.Load();
+
+	// If the current browser directory is the old memstick folder, drop it.
+	if (g_Config.currentDirectory == oldMemstickFolder) {
+		g_Config.currentDirectory = g_Config.defaultCurrentDirectory;
+	}
+
 	PostLoadConfig();
 
 	if (!initialSetup_) {