Merge pull request #3341 from IvanSavenko/fix_extraction_progress

Fix extraction progress display for mod installation

launcher/modManager/cmodlistview_moc.cpp

@@ -591,7 +591,7 @@ void CModListView::downloadFile(QString file, QString url, QString description,
 			this, SLOT(downloadFinished(QStringList,QStringList,QStringList)));
 		
 		connect(manager.get(), SIGNAL(extractionProgress(qint64,qint64)),
-			this, SLOT(downloadProgress(qint64,qint64)));
+			this, SLOT(extractionProgress(qint64,qint64)));
 		
 		connect(modModel, &CModListModel::dataChanged, filterModel, &QAbstractItemModel::dataChanged);
 
@@ -613,6 +613,14 @@ void CModListView::downloadProgress(qint64 current, qint64 max)
 	ui->progressBar->setValue(current / (1024 * 1024));
 }
 
+void CModListView::extractionProgress(qint64 current, qint64 max)
+{
+	// display progress, in extracted files
+	ui->progressBar->setVisible(true);
+	ui->progressBar->setMaximum(max);
+	ui->progressBar->setValue(current);
+}
+
 void CModListView::downloadFinished(QStringList savedFiles, QStringList failedFiles, QStringList errors)
 {
 	QString title = tr("Download failed");

launcher/modManager/cmodlistview_moc.h

@@ -98,6 +98,7 @@ private slots:
 	void dataChanged(const QModelIndex & topleft, const QModelIndex & bottomRight);
 	void modSelected(const QModelIndex & current, const QModelIndex & previous);
 	void downloadProgress(qint64 current, qint64 max);
+	void extractionProgress(qint64 current, qint64 max);
 	void downloadFinished(QStringList savedFiles, QStringList failedFiles, QStringList errors);
 	void modelReset();
 	void hideProgressBar();

launcher/modManager/cmodmanager.cpp

@@ -24,7 +24,9 @@ namespace
 {
 QString detectModArchive(QString path, QString modName, std::vector<std::string> & filesToExtract)
 {
-	filesToExtract = ZipArchive::listFiles(qstringToPath(path));
+	ZipArchive archive(qstringToPath(path));
+
+	filesToExtract = archive.listFiles();
 
 	QString modDirName;
 
@@ -285,14 +287,23 @@ bool CModManager::doInstallMod(QString modname, QString archivePath)
 	if(!modDirName.size())
 		return addError(modname, "Mod archive is invalid or corrupted");
 	
-	auto futureExtract = std::async(std::launch::async, [&archivePath, &destDir, &filesToExtract]()
+	std::atomic<int> filesCounter = 0;
+
+	auto futureExtract = std::async(std::launch::async, [&archivePath, &destDir, &filesCounter, &filesToExtract]()
 	{
-		return ZipArchive::extract(qstringToPath(archivePath), qstringToPath(destDir), filesToExtract);
+		ZipArchive archive(qstringToPath(archivePath));
+		for (auto const & file : filesToExtract)
+		{
+			if (!archive.extract(qstringToPath(destDir), file))
+				return false;
+			++filesCounter;
+		}
+		return true;
 	});
 	
-	while(futureExtract.wait_for(std::chrono::milliseconds(50)) != std::future_status::ready)
+	while(futureExtract.wait_for(std::chrono::milliseconds(10)) != std::future_status::ready)
 	{
-		emit extractionProgress(0, 0);
+		emit extractionProgress(filesCounter, filesToExtract.size());
 		qApp->processEvents();
 	}
 	

lib/filesystem/CZipLoader.cpp

@@ -150,22 +150,11 @@ static bool extractCurrent(unzFile file, std::ostream & where)
 	return false;
 }
 
-std::vector<std::string> ZipArchive::listFiles(const boost::filesystem::path & filename)
+std::vector<std::string> ZipArchive::listFiles()
 {
 	std::vector<std::string> ret;
 
-	CDefaultIOApi zipAPI;
-	auto zipStructure = zipAPI.getApiStructure();
-
-	unzFile file = unzOpen2_64(filename.c_str(), &zipStructure);
-
-	if (file == nullptr)
-	{
-		logGlobal->error("Failed to open file '%s'! Unable to list files!", filename.string());
-		return {};
-	}
-
-	int result = unzGoToFirstFile(file);
+	int result = unzGoToFirstFile(archive);
 
 	if (result == UNZ_OK)
 	{
@@ -174,73 +163,66 @@ std::vector<std::string> ZipArchive::listFiles(const boost::filesystem::path & f
 			unz_file_info64 info;
 			std::vector<char> zipFilename;
 
-			unzGetCurrentFileInfo64 (file, &info, nullptr, 0, nullptr, 0, nullptr, 0);
+			unzGetCurrentFileInfo64 (archive, &info, nullptr, 0, nullptr, 0, nullptr, 0);
 
 			zipFilename.resize(info.size_filename);
 			// Get name of current file. Contrary to docs "info" parameter can't be null
-			unzGetCurrentFileInfo64(file, &info, zipFilename.data(), static_cast<uLong>(zipFilename.size()), nullptr, 0, nullptr, 0);
+			unzGetCurrentFileInfo64(archive, &info, zipFilename.data(), static_cast<uLong>(zipFilename.size()), nullptr, 0, nullptr, 0);
 
 			ret.emplace_back(zipFilename.data(), zipFilename.size());
 
-			result = unzGoToNextFile(file);
+			result = unzGoToNextFile(archive);
 		}
 		while (result == UNZ_OK);
-
-		if (result != UNZ_OK && result != UNZ_END_OF_LIST_OF_FILE)
-		{
-			logGlobal->error("Failed to list file from '%s'! Error code %d", filename.string(), result);
-		}
-	}
-	else
-	{
-		logGlobal->error("Failed to list files from '%s'! Error code %d", filename.string(), result);
 	}
-
-	unzClose(file);
-
 	return ret;
 }
 
-bool ZipArchive::extract(const boost::filesystem::path & from, const boost::filesystem::path & where)
-{
-	// Note: may not be fast enough for large archives (should NOT happen with mods)
-	// because locating each file by name may be slow. Unlikely slower than decompression though
-	return extract(from, where, listFiles(from));
-}
-
-bool ZipArchive::extract(const boost::filesystem::path & from, const boost::filesystem::path & where, const std::vector<std::string> & what)
+ZipArchive::ZipArchive(const boost::filesystem::path & from)
 {
 	CDefaultIOApi zipAPI;
 	auto zipStructure = zipAPI.getApiStructure();
 
-	unzFile archive = unzOpen2_64(from.c_str(), &zipStructure);
+	archive = unzOpen2_64(from.c_str(), &zipStructure);
 
-	auto onExit = vstd::makeScopeGuard([&]()
-	{
-		unzClose(archive);
-	});
+	if (archive == nullptr)
+		throw std::runtime_error("Failed to open file" + from.string() + "'%s'! Unable to list files!");
+}
 
+ZipArchive::~ZipArchive()
+{
+	unzClose(archive);
+}
+
+bool ZipArchive::extract(const boost::filesystem::path & where, const std::vector<std::string> & what)
+{
 	for (const std::string & file : what)
-	{
-		if (unzLocateFile(archive, file.c_str(), 1) != UNZ_OK)
+		if (!extract(where, file))
 			return false;
 
-		const boost::filesystem::path fullName = where / file;
-		const boost::filesystem::path fullPath = fullName.parent_path();
+	return true;
+}
 
-		boost::filesystem::create_directories(fullPath);
-		// directory. No file to extract
-		// TODO: better way to detect directory? Probably check return value of unzOpenCurrentFile?
-		if (boost::algorithm::ends_with(file, "/"))
-			continue;
+bool ZipArchive::extract(const boost::filesystem::path & where, const std::string & file)
+{
+	if (unzLocateFile(archive, file.c_str(), 1) != UNZ_OK)
+		return false;
 
-		std::fstream destFile(fullName.c_str(), std::ios::out | std::ios::binary);
-		if (!destFile.good())
-			return false;
+	const boost::filesystem::path fullName = where / file;
+	const boost::filesystem::path fullPath = fullName.parent_path();
 
-		if (!extractCurrent(archive, destFile))
-			return false;
-	}
+	boost::filesystem::create_directories(fullPath);
+	// directory. No file to extract
+	// TODO: better way to detect directory? Probably check return value of unzOpenCurrentFile?
+	if (boost::algorithm::ends_with(file, "/"))
+		return true;
+
+	std::fstream destFile(fullName.c_str(), std::ios::out | std::ios::binary);
+	if (!destFile.good())
+		return false;
+
+	if (!extractCurrent(archive, destFile))
+		return false;
 	return true;
 }
 

lib/filesystem/CZipLoader.h

@@ -61,16 +61,17 @@ public:
 	std::unordered_set<ResourcePath> getFilteredFiles(std::function<bool(const ResourcePath &)> filter) const override;
 };
 
-namespace ZipArchive
+class DLL_LINKAGE ZipArchive : boost::noncopyable
 {
-	/// List all files present in archive
-	std::vector<std::string> DLL_LINKAGE listFiles(const boost::filesystem::path & filename);
+	unzFile archive;
 
-	/// extracts all files from archive "from" into destination directory "where". Directory must exist
-	bool DLL_LINKAGE extract(const boost::filesystem::path & from, const boost::filesystem::path & where);
+public:
+	ZipArchive(const boost::filesystem::path & from);
+	~ZipArchive();
 
-	///same as above, but extracts only files mentioned in "what" list
-	bool DLL_LINKAGE extract(const boost::filesystem::path & from, const boost::filesystem::path & where, const std::vector<std::string> & what);
-}
+	std::vector<std::string> listFiles();
+	bool extract(const boost::filesystem::path & where, const std::vector<std::string> & what);
+	bool extract(const boost::filesystem::path & where, const std::string & what);
+};
 
 VCMI_LIB_NAMESPACE_END