# HG changeset patch
# User Teemu Piippo <teemu.s.piippo@gmail.com>
# Date 1654947030 -10800
# Node ID 27259810da6d1c7a4a11c0b30f9e741ae2f20748
# Parent  b27b90fb993f7629794e9b284a649d3097d3d559
Rewrite dependency loading

diff -r b27b90fb993f -r 27259810da6d src/basics.h
--- a/src/basics.h	Thu Jun 09 19:11:27 2022 +0300
+++ b/src/basics.h	Sat Jun 11 14:30:30 2022 +0300
@@ -22,6 +22,7 @@
 #include <compare>
 #include <memory>
 #include <optional>
+#include <set>
 #include <type_traits>
 #include <QDataStream>
 #include <QDebug>
@@ -118,6 +119,7 @@
 template<typename T, typename R, typename K>
 R* findInMap(std::map<T, R>& map, K&& key)
 {
+	static_assert(std::is_convertible_v<K, T>, "bad type for key parameter");
 	auto pair = map.find(key);
 	if (pair != map.end())
 	{
@@ -304,3 +306,20 @@
 {
 	return qHash(key.x) ^ rotl10(qHash(key.y)) ^ rotl20(qHash(key.z));
 }
+
+template<typename K, typename V, typename Fn>
+void forValueInMap(const std::map<K, V>& map, Fn&& fn)
+{
+	for (const auto& it : map) {
+		fn(it.second);
+	}
+}
+
+inline QString joined(QString value, const QString& separator, const QString& element)
+{
+	if (not value.isEmpty()) {
+		value += separator;
+	}
+	value += element;
+	return value;
+}
diff -r b27b90fb993f -r 27259810da6d src/documentmanager.cpp
--- a/src/documentmanager.cpp	Thu Jun 09 19:11:27 2022 +0300
+++ b/src/documentmanager.cpp	Sat Jun 11 14:30:30 2022 +0300
@@ -23,12 +23,7 @@
 #include "documentmanager.h"
 #include "parser.h"
 
-/**
- * @brief Constructs a new document manager
- * @param parent Parent object
- */
-DocumentManager::DocumentManager(QObject* parent) :
-	QObject{parent}
+DocumentManager::DocumentManager()
 {
 }
 
@@ -39,40 +34,27 @@
 ModelId DocumentManager::newModel()
 {
 	const ModelId modelId{++this->modelIdCounter};
-	const QString name = makeNewModelName();
-	this->openModels[modelId] = ModelInfo{
-		.model = std::make_unique<Model>(this),
-		.id = modelId,
-		.opentype = OpenType::ManuallyOpened,
-	};
+	this->openModels[modelId].id = modelId;
+	this->openModels[modelId].opentype = OpenType::ManuallyOpened;
 	this->makePolygonCacheForModel(modelId);
 	return modelId;
 }
 
-/**
- * @brief Looks for a model by name
- * @param name Name of the model
- * @returns model or null
- */
 Model* DocumentManager::findDependencyByName(const ModelId modelId, const QString& name)
 {
 	const auto modelsIterator = this->openModels.find(modelId);
-	if (modelsIterator != std::end(this->openModels))
-	{
+	if (modelsIterator != std::end(this->openModels)) {
 		const auto& dependencies = modelsIterator->second.dependencies;
 		const auto dependenciesIterator = dependencies.find(name);
-		if (dependenciesIterator != dependencies.end())
-		{
+		if (dependenciesIterator != dependencies.end()) {
 			ModelInfo& modelInfo = this->openModels[dependenciesIterator->second];
 			return modelInfo.model.get();
 		}
-		else
-		{
+		else {
 			return nullptr;
 		}
 	}
-	else
-	{
+	else {
 		return nullptr;
 	}
 }
@@ -131,7 +113,7 @@
 	QFile file{path};
 	const QString name = pathToName(path);
 	file.open(QFile::ReadOnly | QFile::Text);
-	std::unique_ptr<Model> newModel = std::make_unique<Model>(this);
+	std::unique_ptr<Model> newModel = std::make_unique<Model>(nullptr);
 	QTextStream textStream{&file};
 	Parser parser{file};
 	parser.parseBody(*newModel);
@@ -139,7 +121,13 @@
 	if (file.error() == QFile::NoError)
 	{
 		const ModelId modelId{++this->modelIdCounter};
-		this->openModels[modelId] = {std::move(newModel), modelId, path, openType};
+		this->openModels[modelId] = {
+			.model = std::move(newModel),
+			.id = modelId,
+			.path = path,
+			.opentype = openType,
+			.polygonCache = {},
+		};
 		this->makePolygonCacheForModel(modelId);
 		result = modelId;
 	}
@@ -150,49 +138,6 @@
 	return result;
 }
 
-QString DocumentManager::makeNewModelName()
-{
-	untitledNameCounter += 1;
-	return "untitled-" + QString::number(untitledNameCounter);
-}
-
-void DocumentManager::loadDependenciesForAllModels(const LibraryManager& libraries, QTextStream& errorStream)
-{
-	for (const auto& modelInfoPair : this->openModels)
-	{
-		this->loadDependenciesForModel(modelInfoPair.first, modelInfoPair.second.path, libraries, errorStream);
-	}
-}
-
-struct DocumentManager::LoadDepedenciesBag
-{
-	const LibraryManager& libraries;
-	QStringList missing;
-	QSet<ModelId> processed;
-	QTextStream& errorStream;
-};
-
-void DocumentManager::loadDependenciesForModel(
-	const ModelId modelId,
-	const QString& path,
-	const LibraryManager& libraries,
-	QTextStream& errorStream)
-{
-	LoadDepedenciesBag bag {
-		.libraries = libraries,
-		.missing = {},
-		.processed = {},
-		.errorStream = errorStream,
-	};
-	this->loadDependenciesForModel(modelId, path, bag);
-	if (not bag.missing.empty())
-	{
-		bag.missing.sort(Qt::CaseInsensitive);
-		errorStream << tr("The following files could not be opened: %1")
-			.arg(bag.missing.join(", "));
-	}
-}
-
 void DocumentManager::closeDocument(const ModelId modelId)
 {
 	ModelInfo* modelInfo = findInMap(this->openModels, modelId);
@@ -216,63 +161,51 @@
 	}
 }
 
-/**
- * @brief Changes the path of the specified model. Since the name of the file may change,
- * changing the path can cause dependencies to be resolved differently. As such, dependencies
- * need to be resolved for all files after this operation.
- * @param modelId Model to change the path of
- * @param newPath New path
- * @param libraries Library manager for the purpose of dependency resolving
- * @param errorStream Where to write any errors regarding dependency resolving
- */
+//! \brief Changes the path of the specified model. This can cause dependencies
+//! to be resolved differently. As such, dependencies need to be resolved for
+//! all files after this operation.
 void DocumentManager::setModelPath(
 	const ModelId modelId,
 	const QString &newPath,
 	const LibraryManager &libraries,
 	QTextStream &errorStream)
 {
-	auto modelInfoPair = this->openModels.find(modelId);
-	if (true
-		and modelInfoPair != this->openModels.end()
-		and modelInfoPair->second.opentype == OpenType::ManuallyOpened
-	) {
-		modelInfoPair->second.path = newPath;
-		this->loadDependenciesForAllModels(libraries, errorStream);
+	ModelInfo* info = findInMap(this->openModels, modelId);
+	if (info != nullptr and info->opentype == OpenType::ManuallyOpened) {
+		info->path = newPath;
+		const MissingDependencies missing = this->loadDependenciesForAllModels(libraries);
+		if (not missing.empty()) {
+			errorStream << errorStringFromMissingDependencies(missing);
+		}
 	}
 }
 
 bool DocumentManager::saveModel(const ModelId modelId, QTextStream &errors)
 {
-	const QString* const path = this->modelPath(modelId);
-	if (path != nullptr)
+	ModelInfo* info = findInMap(this->openModels, modelId);
+	if (info != nullptr)
 	{
-		QSaveFile file{*path};
+		QSaveFile file{info->path};
 		file.setDirectWriteFallback(true);
-		if (file.open(QSaveFile::WriteOnly))
-		{
-			// if path is not nullptr, getModelById will always return a value as well
-			::save(*this->getModelById(modelId), &file);
+		if (file.open(QSaveFile::WriteOnly)) {
+			::save(info->model.get(), &file);
 			const bool commitSucceeded = file.commit();
-			if (not commitSucceeded)
-			{
-				errors << tr("Could not save: %1").arg(file.errorString());
+			if (not commitSucceeded) {
+				errors << QObject::tr("Could not save: %1").arg(file.errorString());
 				return false;
 			}
-			else
-			{
+			else {
 				return true;
 			}
 		}
-		else
-		{
-			errors << tr("Could not open %1 for writing: %2")
+		else {
+			errors << QObject::tr("Could not open %1 for writing: %2")
 				.arg(file.fileName(), file.errorString());
 			return false;
 		}
 	}
-	else
-	{
-		errors << tr("Bad model ID %1").arg(modelId.value);
+	else {
+		errors << QObject::tr("Bad model ID %1").arg(modelId.value);
 		return false;
 	}
 }
@@ -298,17 +231,30 @@
 
 PolygonCache *DocumentManager::getPolygonCacheForModel(ModelId modelId)
 {
-	auto it = this->polygonCaches.find(modelId);
-	if (it != this->polygonCaches.end())
-	{
-		return &it->second;
+	ModelInfo* info = findInMap(this->openModels, modelId);
+	if (info != nullptr) {
+		return &info->polygonCache;
 	}
-	else
-	{
+	else {
 		return nullptr;
 	}
 }
 
+const DocumentManager::ModelInfo *DocumentManager::infoForModel(ModelId modelId) const
+{
+	return findInMap(this->openModels, modelId);
+}
+
+QString errorStringFromMissingDependencies(const DocumentManager::MissingDependencies& missing)
+{
+	QString missingString;
+	forValueInMap(missing, [&missingString](const QString& path){
+		missingString = joined(missingString, QStringLiteral(", "), path);
+	});
+	return QObject::tr("The following files could not be opened: %1")
+		.arg(missingString);
+}
+
 /**
  * @brief Cleans up and erases models that are no longer required.
  */
@@ -321,17 +267,12 @@
 			and it->second.opentype == OpenType::AutomaticallyOpened
 			and not this->isReferencedByAnything(it->first)
 		) {
-			// Remove its polygon cache
-			const auto polygonCache = this->polygonCaches.find(it->first);
-			if (polygonCache != this->polygonCaches.end())
-			{
-				this->polygonCaches.erase(polygonCache);
-			}
 			// Remove the model
 			this->openModels.erase(it);
-			// We need to start over now. It is possible that other models that previously
-			// were referenced by the model we just erased have become prunable.
-			// Moreover, our iterator is invalid now and we cannot continue in this for loop.
+			// We need to start over now. It is possible that other models that
+			// previously were referenced by the model we just erased have
+			// become prunable. Moreover, our iterator is invalid now and we
+			// cannot continue in this loop.
 			this->prune();
 			break;
 		}
@@ -366,27 +307,29 @@
 	Model* model = this->getModelById(modelId);
 	if (model != nullptr)
 	{
-		this->polygonCaches[modelId] = {};
-		connect(model, &Model::dataChanged, this, &DocumentManager::modelModified);
-		connect(model, &Model::rowsInserted, this, &DocumentManager::modelModified);
-		connect(model, &Model::rowsRemoved, this, &DocumentManager::modelModified);
+		const auto modelModified = [this, model]{
+			const std::optional<ModelId> modelId = this->findIdForModel(model);
+			if (modelId.has_value()) {
+				ModelInfo* info = findInMap(this->openModels, *modelId);
+				if (info != nullptr) {
+					info->polygonCache.needRecache = true;
+				}
+			}
+		};
+		QObject::connect(model, &Model::dataChanged, modelModified);
+		QObject::connect(model, &Model::rowsInserted, modelModified);
+		QObject::connect(model, &Model::rowsRemoved, modelModified);
 	}
 }
 
-void DocumentManager::modelModified()
-{
-	Model* const model = qobject_cast<Model*>(this->sender());
-	const std::optional<ModelId> modelId = this->findIdForModel(model);
-	if (modelId.has_value()) {
-		this->polygonCaches[*modelId].needRecache = true;
-	}
-}
-
-static QString findFile(QString referenceName, const QString& path, const LibraryManager& libraries)
+static QString findFile(
+	QString referenceName,
+	const QString& modelPath,
+	const LibraryManager& libraries)
 {
 	// Try to find the file in the same place as the model itself
 	referenceName.replace("\\", "/");
-	const QDir dir = QFileInfo{path}.dir();
+	const QDir dir = QFileInfo{modelPath}.dir();
 	QString referencedFilePath = dir.filePath(referenceName);
 	if (not QFileInfo{referencedFilePath}.exists())
 	{
@@ -396,72 +339,87 @@
 	return referencedFilePath;
 }
 
-template<typename T>
-void iterate(const Model& model, std::function<void(const T&)> fn)
+static std::set<QString> referenceNames(const Model* model)
+{
+	std::set<QString> result;
+	iterate<Colored<SubfileReference>>(*model, [&result](const SubfileReference& ref){
+		result.insert(ref.name);
+	});
+	return result;
+}
+
+struct Dependency
+{
+	QString name;
+	QString path;
+	bool operator<(const Dependency& other) const
+	{
+		if (this->name != other.name) {
+			return this->name < other.name;
+		}
+		else {
+			return this->path < other.path;
+		}
+	}
+};
+
+static std::set<Dependency> resolveReferencePaths(
+	const DocumentManager::ModelInfo* modelInfo,
+	const LibraryManager* libraries)
 {
-	for (int i = 0; i < model.size(); ++i) {
-		if (std::holds_alternative<T>(model[i])) {
-			fn(std::get<T>(model[i]));
+	std::set<Dependency> result;
+	const std::set<QString> refNames = referenceNames(modelInfo->model.get());
+	if (modelInfo != nullptr) {
+		for (const QString& name : refNames) {
+			const QString path = findFile(name, modelInfo->path, *libraries);
+			if (not path.isEmpty()) {
+				result.insert(Dependency{.name = name, .path = path});
+			}
+		}
+	}
+	return result;
+}
+
+static void loadDependenciesForModel(
+	DocumentManager::ModelInfo* info,
+	DocumentManager* documents,
+	const LibraryManager* libraries,
+	std::map<QString, QString>& missing)
+{
+	bool repeat = true;
+	info->dependencies.clear();
+	while (repeat) {
+		repeat = false;
+		const std::set<Dependency> dependencies = resolveReferencePaths(info, libraries);
+		for (const Dependency& dep : dependencies) {
+			if (not info->dependencies.contains(dep.name) and not missing.contains(dep.path)) {
+				QString loadErrorString;
+				QTextStream localErrorStream{&loadErrorString};
+				const std::optional<ModelId> modelIdOpt = documents->openModel(
+					dep.path,
+					localErrorStream,
+					OpenType::AutomaticallyOpened);
+				if (not modelIdOpt.has_value()) {
+					const QString& errorMessage = QObject::tr("could not load '%1': %2")
+						.arg(dep.path, loadErrorString);
+					missing[dep.path] = errorMessage;
+				}
+				else {
+					info->dependencies[dep.name] = modelIdOpt.value();
+					repeat = true;
+				}
+			}
 		}
 	}
 }
 
-void DocumentManager::loadDependenciesForModel(
-	const ModelId modelId,
-	const QString &path,
-	LoadDepedenciesBag& bag)
+std::map<QString, QString> DocumentManager::loadDependenciesForAllModels(const LibraryManager& libraries)
 {
-	QSet<QString> failedToOpen;
-	struct LoadingError
+	std::map<QString, QString> missing;
+	for (auto& modelInfoPair : this->openModels)
 	{
-		QString message;
-	};
-	bag.processed.insert(modelId);
-	if (not this->openModels.contains(modelId))
-	{
-		bag.errorStream << tr("bad model ID %1").arg(modelId.value);
-		return;
+		loadDependenciesForModel(&modelInfoPair.second, this, &libraries, missing);
 	}
-	ModelInfo& modelInfo = this->openModels[modelId];
-	modelInfo.dependencies.clear();
-	iterate<Colored<SubfileReference>>(*modelInfo.model, [&](const SubfileReference& ref) {
-		const QString referenceName = ref.name;
-		if (not referenceName.isEmpty()
-			and modelInfo.dependencies.count(referenceName) == 0
-			and not failedToOpen.contains(referenceName))
-		{
-			try
-			{
-				const QString referencedFilePath = ::findFile(referenceName, path, bag.libraries);
-				if (referencedFilePath.isEmpty())
-				{
-					throw LoadingError{tr("could not find '%1'").arg(referenceName)};
-				}
-				QString loadErrorString;
-				QTextStream localErrorStream{&loadErrorString};
-				const std::optional<ModelId> modelIdOpt = this->openModel(
-					referencedFilePath,
-					localErrorStream,
-					OpenType::AutomaticallyOpened);
-				if (not modelIdOpt.has_value())
-				{
-					const QString& errorMessage = tr("could not load '%1': %2")
-						.arg(referencedFilePath)
-						.arg(loadErrorString);
-					throw LoadingError{errorMessage};
-				}
-				modelInfo.dependencies[referenceName] = modelIdOpt.value();
-				if (not bag.processed.contains(modelIdOpt.value()))
-				{
-					this->loadDependenciesForModel(modelIdOpt.value(), referencedFilePath, bag);
-				}
-			}
-			catch(const LoadingError& error)
-			{
-				bag.errorStream << error.message << "\n";
-				failedToOpen.insert(referenceName);
-				bag.missing.append(referenceName);
-			}
-		}
-	});
+	this->prune();
+	return missing;
 }
diff -r b27b90fb993f -r 27259810da6d src/documentmanager.h
--- a/src/documentmanager.h	Thu Jun 09 19:11:27 2022 +0300
+++ b/src/documentmanager.h	Sat Jun 11 14:30:30 2022 +0300
@@ -21,32 +21,33 @@
 #include "model.h"
 #include "polygoncache.h"
 
-class DocumentManager : public QObject
+enum OpenType
 {
-	Q_OBJECT
+	//! \brief Document was opened manually by the user
+	ManuallyOpened,
+	//! \brief Document was opened automatically in order to resolve subfile references
+	AutomaticallyOpened,
+};
+
+class DocumentManager
+{
 public:
-	enum OpenType
+	struct ModelInfo
 	{
-		/**
-		  * Document was opened manually by the user
-		  */
-		ManuallyOpened,
-		/**
-		  * Document was opened automatically in order to resolve subfile references
-		  */
-		AutomaticallyOpened,
+		std::unique_ptr<Model> model;
+		ModelId id;
+		QString path;
+		OpenType opentype;
+		std::map<QString, ModelId> dependencies = {};
+		PolygonCache polygonCache;
 	};
-    DocumentManager(QObject* parent = nullptr);
+	using MissingDependencies = std::map<QString, QString>;
+	DocumentManager();
 	ModelId newModel();
 	Model* findDependencyByName(const ModelId modelId, const QString& name);
 	Model* getModelById(ModelId modelId);
 	std::optional<ModelId> openModel(const QString& path, QTextStream& errorStream, const OpenType openType);
-	QString makeNewModelName();
-	void loadDependenciesForAllModels(const LibraryManager &libraries, QTextStream &errorStream);
-	void loadDependenciesForModel(const ModelId modelId,
-		const QString& path,
-		const LibraryManager& libraries,
-		QTextStream& errorStream);
+	std::map<QString, QString> loadDependenciesForAllModels(const LibraryManager &libraries);
 	void closeDocument(const ModelId modelId);
 	const QString* modelPath(ModelId modelId) const;
 	void setModelPath(
@@ -57,27 +58,16 @@
 	bool saveModel(const ModelId modelId, QTextStream& errors);
 	std::optional<ModelId> findIdForModel(const Model* model) const;
 	PolygonCache* getPolygonCacheForModel(ModelId modelId);
+	const ModelInfo* infoForModel(ModelId modelId) const;
 private:
-	struct ModelInfo
-	{
-		std::unique_ptr<Model> model;
-		ModelId id;
-		QString path;
-		OpenType opentype;
-		std::map<QString, ModelId> dependencies = {};
-	};
-	struct LoadDepedenciesBag;
 	int modelIdCounter = 0;
-	int untitledNameCounter = 0;
 	std::map<ModelId, ModelInfo> openModels;
-	std::map<ModelId, PolygonCache> polygonCaches;
-	void loadDependenciesForModel(const ModelId modelId, const QString& path, LoadDepedenciesBag& bag);
 	void collectReferences(QSet<QString> &referenced, const QString& name, const Model* model);
 	void updateDependencies(ModelInfo* model);
 	void prune();
-	Q_SLOT void modelModified();
 	bool isReferencedByAnything(const ModelId modelId) const;
 	void makePolygonCacheForModel(const ModelId modelId);
 };
 
+QString errorStringFromMissingDependencies(const DocumentManager::MissingDependencies& missing);
 QString pathToName(const QFileInfo& path);
diff -r b27b90fb993f -r 27259810da6d src/main.cpp
--- a/src/main.cpp	Thu Jun 09 19:11:27 2022 +0300
+++ b/src/main.cpp	Sat Jun 11 14:30:30 2022 +0300
@@ -50,14 +50,14 @@
 	const std::optional<ModelId> modelIdOpt = documents->openModel(
 		path,
 		errorStream,
-		DocumentManager::OpenType::ManuallyOpened);
+		OpenType::ManuallyOpened);
 	if (modelIdOpt.has_value()) {
-		documents->loadDependenciesForModel(modelIdOpt.value(), path, *libraries, errorStream);
-		if (not errorString.isEmpty()) {
+		const DocumentManager::MissingDependencies missing = documents->loadDependenciesForAllModels(*libraries);
+		if (not missing.empty()) {
 			QMessageBox::warning(
 				parent,
 				QObject::tr("Problem loading references"),
-				errorString);
+				errorStringFromMissingDependencies(missing));
 		}
 	}
 	else {
@@ -286,7 +286,7 @@
 	QApplication app{argc, argv};
 	QMainWindow mainWindow;
 	Ui_MainWindow ui;
-	DocumentManager documents{&mainWindow};
+	DocumentManager documents;
 	QString currentLanguage = "en";
 	QTranslator translator{&mainWindow};
 	Configuration settings;
diff -r b27b90fb993f -r 27259810da6d src/model.h
--- a/src/model.h	Thu Jun 09 19:11:27 2022 +0300
+++ b/src/model.h	Sat Jun 11 14:30:30 2022 +0300
@@ -226,3 +226,13 @@
 
 void save(const Model& model, QIODevice *device);
 void updateHeaderNameField(Model& model, const QString &name);
+
+template<typename T>
+void iterate(const Model& model, std::function<void(const T&)> fn)
+{
+	for (int i = 0; i < model.size(); ++i) {
+		if (std::holds_alternative<T>(model[i])) {
+			fn(std::get<T>(model[i]));
+		}
+	}
+}