Merge branch 'PrismLauncher:develop' into develop

CONTRIBUTING.md

@@ -2,16 +2,56 @@
 
 ## Code formatting
 
-Try to follow the existing formatting.
-If there is no existing formatting, you may use `clang-format` with our included `.clang-format` configuration.
+All files are formatted with `clang-format` using the configuration in `.clang-format`. Ensure it is run on changed files before committing!
 
-In general, in order of importance:
+We have no tool for enforcing names but please follow the following conventions for C++:
+- Class and type names should be formatted as `PascalCase`: `MyClass`.
+- Private or protected class data members should be formatted as `camelCase` prefixed with `m_`: `m_myCounter`.
+- Private or protected `static` class data members should be formatted as `camelCase` prefixed with `s_`: `s_instance`.
+- Public class data members should be formatted as `camelCase` without the prefix: `dateOfBirth`. 
+- Public, private or protected `static const` class data members should be formatted as `SCREAMING_SNAKE_CASE`: `MAX_VALUE`.
+- Class function members should be formatted as `camelCase` without a prefix: `incrementCounter`.
+- Global functions and non-`const` global variables should be formatted as `camelCase` without a prefix: `globalData`.
+- `const` global variables, macros, and enum constants should be formatted as `SCREAMING_SNAKE_CASE`: `LIGHT_GRAY`.
+- Avoid inventing acronyms or abbreviations especially for a name of multiple words - like `tp` for `texturePack`.
 
-- Make sure your IDE is not messing up line endings or whitespace and avoid using linters.
-- Prefer readability over dogma.
-- Keep to the existing formatting.
-- Indent with 4 space unless it's in a submodule.
-- Keep lists (of arguments, parameters, initializers...) as lists, not paragraphs. It should either read from top to bottom, or left to right. Not both.
+Here is what these conventions with the formatting configuration look like:
+
+```c++
+#define AWESOMENESS 10
+
+constexpr double PI = 3.14159;
+
+enum class PizzaToppings { HAM_AND_PINEAPPLE, OREO_AND_KETCHUP };
+
+struct Person {
+    QString name;
+    QDateTime dateOfBirth;
+
+    long daysOld() const { return dateOfBirth.daysTo(QDateTime::currentDateTime()); }
+};
+
+class ImportantClass {
+   public:
+    void incrementCounter()
+    {
+        if (m_counter + 1 > MAX_COUNTER_VALUE)
+            throw std::runtime_error("Counter has reached limit!");
+
+        ++m_counter;
+    }
+
+    int counter() const { return m_counter; }
+
+   private:
+    static constexpr int MAX_COUNTER_VALUE = 100;
+    int m_counter;
+};
+
+ImportantClass importantClassInstance;
+```
+
+If you see any names which do not follow these conventions, it is preferred that you leave them be - renames increase the number of changes therefore make reviewing harder and make your PR more prone to conflicts. However, if you're refactoring a whole class anyway, it's fine.
 
 ## Signing your work
 

launcher/Application.cpp

@@ -846,7 +846,7 @@ Application::Application(int& argc, char** argv) : QApplication(argc, argv)
                                     ":/icons/multimc/128x128/instances/", ":/icons/multimc/scalable/instances/" };
         m_icons.reset(new IconList(instFolders, setting->get().toString()));
         connect(setting.get(), &Setting::SettingChanged,
-                [&](const Setting&, QVariant value) { m_icons->directoryChanged(value.toString()); });
+                [this](const Setting&, QVariant value) { m_icons->directoryChanged(value.toString()); });
         qDebug() << "<> Instance icons initialized.";
     }
 
@@ -1081,11 +1081,11 @@ Application::Application(int& argc, char** argv) : QApplication(argc, argv)
 
 bool Application::createSetupWizard()
 {
-    bool javaRequired = [&]() {
-        if (BuildConfig.JAVA_DOWNLOADER_ENABLED && m_settings->get("AutomaticJavaDownload").toBool()) {
+    bool javaRequired = [this]() {
+        if (BuildConfig.JAVA_DOWNLOADER_ENABLED && settings()->get("AutomaticJavaDownload").toBool()) {
             return false;
         }
-        bool ignoreJavaWizard = m_settings->get("IgnoreJavaWizard").toBool();
+        bool ignoreJavaWizard = settings()->get("IgnoreJavaWizard").toBool();
         if (ignoreJavaWizard) {
             return false;
         }
@@ -1099,8 +1099,8 @@ bool Application::createSetupWizard()
         QString actualPath = FS::ResolveExecutable(currentJavaPath);
         return actualPath.isNull();
     }();
-    bool askjava = BuildConfig.JAVA_DOWNLOADER_ENABLED && !javaRequired && !m_settings->get("AutomaticJavaDownload").toBool() &&
-                   !m_settings->get("AutomaticJavaSwitch").toBool() && !m_settings->get("UserAskedAboutAutomaticJavaDownload").toBool();
+    bool askjava = BuildConfig.JAVA_DOWNLOADER_ENABLED && !javaRequired && !settings()->get("AutomaticJavaDownload").toBool() &&
+                   !settings()->get("AutomaticJavaSwitch").toBool() && !settings()->get("UserAskedAboutAutomaticJavaDownload").toBool();
     bool languageRequired = settings()->get("Language").toString().isEmpty();
     bool pasteInterventionRequired = settings()->get("PastebinURL") != "";
     bool validWidgets = m_themeManager->isValidApplicationTheme(settings()->get("ApplicationTheme").toString());
@@ -1507,7 +1507,7 @@ void Application::controllerSucceeded()
     // on success, do...
     if (controller->instance()->settings()->get("AutoCloseConsole").toBool()) {
         if (extras.window) {
-            extras.window->close();
+            QMetaObject::invokeMethod(extras.window, &QWidget::close, Qt::QueuedConnection);
         }
     }
     extras.controller.reset();

launcher/DataMigrationTask.cpp

@@ -27,7 +27,7 @@ void DataMigrationTask::executeTask()
 
     // 1. Scan
     // Check how many files we gotta copy
-    m_copyFuture = QtConcurrent::run(QThreadPool::globalInstance(), [&] {
+    m_copyFuture = QtConcurrent::run(QThreadPool::globalInstance(), [this] {
         return m_copy(true);  // dry run to collect amount of files
     });
     connect(&m_copyFutureWatcher, &QFutureWatcher<bool>::finished, this, &DataMigrationTask::dryRunFinished);
@@ -60,7 +60,7 @@ void DataMigrationTask::dryRunFinished()
         setProgress(m_copy.totalCopied(), m_toCopy);
         setStatus(tr("Copying %1…").arg(shortenedName));
     });
-    m_copyFuture = QtConcurrent::run(QThreadPool::globalInstance(), [&] {
+    m_copyFuture = QtConcurrent::run(QThreadPool::globalInstance(), [this] {
         return m_copy(false);  // actually copy now
     });
     connect(&m_copyFutureWatcher, &QFutureWatcher<bool>::finished, this, &DataMigrationTask::copyFinished);

launcher/FileSystem.cpp

@@ -341,7 +341,7 @@ bool copy::operator()(const QString& offset, bool dryRun)
         opt |= copy_opts::overwrite_existing;
 
     // Function that'll do the actual copying
-    auto copy_file = [&](QString src_path, QString relative_dst_path) {
+    auto copy_file = [this, dryRun, src, dst, opt, &err](QString src_path, QString relative_dst_path) {
         if (m_matcher && (m_matcher->matches(relative_dst_path) != m_whitelist))
             return;
 
@@ -428,7 +428,7 @@ void create_link::make_link_list(const QString& offset)
             m_recursive = true;
 
         // Function that'll do the actual linking
-        auto link_file = [&](QString src_path, QString relative_dst_path) {
+        auto link_file = [this, dst](QString src_path, QString relative_dst_path) {
             if (m_matcher && (m_matcher->matches(relative_dst_path) != m_whitelist)) {
                 qDebug() << "path" << relative_dst_path << "in black list or not in whitelist";
                 return;
@@ -523,7 +523,7 @@ void create_link::runPrivileged(const QString& offset)
 
     QString serverName = BuildConfig.LAUNCHER_APP_BINARY_NAME + "_filelink_server" + StringUtils::getRandomAlphaNumeric();
 
-    connect(&m_linkServer, &QLocalServer::newConnection, this, [&]() {
+    connect(&m_linkServer, &QLocalServer::newConnection, this, [this, &gotResults]() {
         qDebug() << "Client connected, sending out pairs";
         // construct block of data to send
         QByteArray block;
@@ -605,7 +605,7 @@ void create_link::runPrivileged(const QString& offset)
     }
 
     ExternalLinkFileProcess* linkFileProcess = new ExternalLinkFileProcess(serverName, m_useHardLinks, this);
-    connect(linkFileProcess, &ExternalLinkFileProcess::processExited, this, [&]() { emit finishedPrivileged(gotResults); });
+    connect(linkFileProcess, &ExternalLinkFileProcess::processExited, this, [this, gotResults]() { emit finishedPrivileged(gotResults); });
     connect(linkFileProcess, &ExternalLinkFileProcess::finished, linkFileProcess, &QObject::deleteLater);
 
     linkFileProcess->start();
@@ -1295,7 +1295,7 @@ bool clone::operator()(const QString& offset, bool dryRun)
     std::error_code err;
 
     // Function that'll do the actual cloneing
-    auto cloneFile = [&](QString src_path, QString relative_dst_path) {
+    auto cloneFile = [this, dryRun, dst, &err](QString src_path, QString relative_dst_path) {
         if (m_matcher && (m_matcher->matches(relative_dst_path) != m_whitelist))
             return;
 

launcher/InstanceCopyTask.cpp

@@ -91,7 +91,7 @@ void InstanceCopyTask::executeTask()
                     QEventLoop loop;
                     bool got_priv_results = false;
 
-                    connect(&folderLink, &FS::create_link::finishedPrivileged, this, [&](bool gotResults) {
+                    connect(&folderLink, &FS::create_link::finishedPrivileged, this, [&got_priv_results, &loop](bool gotResults) {
                         if (!gotResults) {
                             qDebug() << "Privileged run exited without results!";
                         }

launcher/InstanceList.cpp

@@ -487,7 +487,7 @@ InstanceList::InstListError InstanceList::loadList()
         int front_bookmark = -1;
         int back_bookmark = -1;
         int currentItem = -1;
-        auto removeNow = [&]() {
+        auto removeNow = [this, &front_bookmark, &back_bookmark, &currentItem]() {
             beginRemoveRows(QModelIndex(), front_bookmark, back_bookmark);
             m_instances.erase(m_instances.begin() + front_bookmark, m_instances.begin() + back_bookmark + 1);
             endRemoveRows();

launcher/Version.cpp

@@ -79,7 +79,7 @@ void Version::parse()
     if (m_string.isEmpty())
         return;
 
-    auto classChange = [&](QChar lastChar, QChar currentChar) {
+    auto classChange = [&currentSection](QChar lastChar, QChar currentChar) {
         if (lastChar.isNull())
             return false;
         if (lastChar.isDigit() != currentChar.isDigit())

launcher/filelink/FileLink.cpp

@@ -104,11 +104,11 @@ void FileLinkApp::joinServer(QString server)
 
     in.setDevice(&socket);
 
-    connect(&socket, &QLocalSocket::connected, this, [&]() { qDebug() << "connected to server"; });
+    connect(&socket, &QLocalSocket::connected, this, []() { qDebug() << "connected to server"; });
 
     connect(&socket, &QLocalSocket::readyRead, this, &FileLinkApp::readPathPairs);
 
-    connect(&socket, &QLocalSocket::errorOccurred, this, [&](QLocalSocket::LocalSocketError socketError) {
+    connect(&socket, &QLocalSocket::errorOccurred, this, [this](QLocalSocket::LocalSocketError socketError) {
         m_status = Failed;
         switch (socketError) {
             case QLocalSocket::ServerNotFoundError:
@@ -132,7 +132,7 @@ void FileLinkApp::joinServer(QString server)
         }
     });
 
-    connect(&socket, &QLocalSocket::disconnected, this, [&]() {
+    connect(&socket, &QLocalSocket::disconnected, this, [this]() {
         qDebug() << "disconnected from server, should exit";
         m_status = Succeeded;
         exit();

launcher/java/JavaUtils.cpp

@@ -405,7 +405,7 @@ QList<QString> JavaUtils::FindJavaPaths()
 {
     QList<QString> javas;
     javas.append(this->GetDefaultJava()->path);
-    auto scanJavaDir = [&](
+    auto scanJavaDir = [&javas](
                            const QString& dirPath,
                            const std::function<bool(const QFileInfo&)>& filter = [](const QFileInfo&) { return true; }) {
         QDir dir(dirPath);
@@ -424,7 +424,7 @@ QList<QString> JavaUtils::FindJavaPaths()
     };
     // java installed in a snap is installed in the standard directory, but underneath $SNAP
     auto snap = qEnvironmentVariable("SNAP");
-    auto scanJavaDirs = [&](const QString& dirPath) {
+    auto scanJavaDirs = [scanJavaDir, snap](const QString& dirPath) {
         scanJavaDir(dirPath);
         if (!snap.isNull()) {
             scanJavaDir(snap + dirPath);
@@ -546,12 +546,12 @@ QStringList getPrismJavaBundle()
 {
     QList<QString> javas;
 
-    auto scanDir = [&](QString prefix) {
+    auto scanDir = [&javas](QString prefix) {
         javas.append(FS::PathCombine(prefix, "jre", "bin", JavaUtils::javaExecutable));
         javas.append(FS::PathCombine(prefix, "bin", JavaUtils::javaExecutable));
         javas.append(FS::PathCombine(prefix, JavaUtils::javaExecutable));
     };
-    auto scanJavaDir = [&](const QString& dirPath) {
+    auto scanJavaDir = [scanDir](const QString& dirPath) {
         QDir dir(dirPath);
         if (!dir.exists())
             return;

launcher/launch/steps/PostLaunchCommand.cpp

@@ -65,7 +65,7 @@ void PostLaunchCommand::executeTask()
 
 void PostLaunchCommand::on_state(LoggedProcess::State state)
 {
-    auto getError = [&]() { return tr("Post-Launch command failed with code %1.\n\n").arg(m_process.exitCode()); };
+    auto getError = [this]() { return tr("Post-Launch command failed with code %1.\n\n").arg(m_process.exitCode()); };
     switch (state) {
         case LoggedProcess::Aborted:
         case LoggedProcess::Crashed:

launcher/launch/steps/PreLaunchCommand.cpp

@@ -65,7 +65,7 @@ void PreLaunchCommand::executeTask()
 
 void PreLaunchCommand::on_state(LoggedProcess::State state)
 {
-    auto getError = [&]() { return tr("Pre-Launch command failed with code %1.\n\n").arg(m_process.exitCode()); };
+    auto getError = [this]() { return tr("Pre-Launch command failed with code %1.\n\n").arg(m_process.exitCode()); };
     switch (state) {
         case LoggedProcess::Aborted:
         case LoggedProcess::Crashed:

launcher/meta/VersionList.cpp

@@ -159,8 +159,8 @@ Version::Ptr VersionList::getVersion(const QString& version)
 
 bool VersionList::hasVersion(QString version) const
 {
-    auto ver =
-        std::find_if(m_versions.constBegin(), m_versions.constEnd(), [&](Meta::Version::Ptr const& a) { return a->version() == version; });
+    auto ver = std::find_if(m_versions.constBegin(), m_versions.constEnd(),
+                            [version](Meta::Version::Ptr const& a) { return a->version() == version; });
     return (ver != m_versions.constEnd());
 }
 

launcher/minecraft/Library.cpp

@@ -65,7 +65,7 @@ void Library::getApplicableFiles(const RuntimeContext& runtimeContext,
 {
     bool local = isLocal();
     // Lambda function to get the absolute file path
-    auto actualPath = [&](QString relPath) {
+    auto actualPath = [this, local, overridePath](QString relPath) {
         relPath = FS::RemoveInvalidPathChars(relPath);
         QFileInfo out(FS::PathCombine(storagePrefix(), relPath));
         if (local && !overridePath.isEmpty()) {
@@ -115,7 +115,7 @@ QList<Net::NetRequest::Ptr> Library::getDownloads(const RuntimeContext& runtimeC
     bool local = isLocal();
 
     // Lambda function to check if a local file exists
-    auto check_local_file = [&](QString storage) {
+    auto check_local_file = [overridePath, &failedLocalFiles](QString storage) {
         QFileInfo fileinfo(storage);
         QString fileName = fileinfo.fileName();
         auto fullPath = FS::PathCombine(overridePath, fileName);
@@ -128,7 +128,7 @@ QList<Net::NetRequest::Ptr> Library::getDownloads(const RuntimeContext& runtimeC
     };
 
     // Lambda function to add a download request
-    auto add_download = [&](QString storage, QString url, QString sha1) {
+    auto add_download = [this, local, check_local_file, cache, stale, &out](QString storage, QString url, QString sha1) {
         if (local) {
             return check_local_file(storage);
         }
@@ -198,7 +198,7 @@ QList<Net::NetRequest::Ptr> Library::getDownloads(const RuntimeContext& runtimeC
             }
         }
     } else {
-        auto raw_dl = [&]() {
+        auto raw_dl = [this, raw_storage]() {
             if (!m_absoluteURL.isEmpty()) {
                 return m_absoluteURL;
             }

launcher/minecraft/MinecraftInstance.cpp

@@ -854,7 +854,7 @@ QStringList MinecraftInstance::verboseDescription(AuthSessionPtr session, Minecr
         out << "Libraries:";
         QStringList jars, nativeJars;
         profile->getLibraryFiles(runtimeContext(), jars, nativeJars, getLocalLibraryPath(), binRoot());
-        auto printLibFile = [&](const QString& path) {
+        auto printLibFile = [&out](const QString& path) {
             QFileInfo info(path);
             if (info.exists()) {
                 out << "  " + path;
@@ -874,7 +874,7 @@ QStringList MinecraftInstance::verboseDescription(AuthSessionPtr session, Minecr
     }
 
     // mods and core mods
-    auto printModList = [&](const QString& label, ModFolderModel& model) {
+    auto printModList = [&out](const QString& label, ModFolderModel& model) {
         if (model.size()) {
             out << QString("%1:").arg(label);
             auto modList = model.allMods();

launcher/minecraft/OneSixVersionFormat.cpp

@@ -176,7 +176,7 @@ VersionFilePtr OneSixVersionFormat::versionFileFromJson(const QJsonDocument& doc
         }
     }
 
-    auto readLibs = [&](const char* which, QList<LibraryPtr>& outList) {
+    auto readLibs = [&root, &out, &filename](const char* which, QList<LibraryPtr>& outList) {
         for (auto libVal : requireArray(root.value(which))) {
             QJsonObject libObj = requireObject(libVal);
             // parse the library

launcher/minecraft/PackProfile.cpp

@@ -746,7 +746,7 @@ bool PackProfile::removeComponent_internal(ComponentPtr patch)
     }
 
     // FIXME: we need a generic way of removing local resources, not just jar mods...
-    auto preRemoveJarMod = [&](LibraryPtr jarMod) -> bool {
+    auto preRemoveJarMod = [this](LibraryPtr jarMod) -> bool {
         if (!jarMod->isLocal()) {
             return true;
         }

launcher/minecraft/launch/LauncherPartLaunch.cpp

@@ -54,8 +54,8 @@ LauncherPartLaunch::LauncherPartLaunch(LaunchTask* parent)
 {
     if (parent->instance()->settings()->get("CloseAfterLaunch").toBool()) {
         std::shared_ptr<QMetaObject::Connection> connection{ new QMetaObject::Connection };
-        *connection =
-            connect(&m_process, &LoggedProcess::log, this, [=](const QStringList& lines, [[maybe_unused]] MessageLevel::Enum level) {
+        *connection = connect(
+            &m_process, &LoggedProcess::log, this, [connection](const QStringList& lines, [[maybe_unused]] MessageLevel::Enum level) {
                 qDebug() << lines;
                 if (lines.filter(QRegularExpression(".*Setting user.+", QRegularExpression::CaseInsensitiveOption)).length() != 0) {
                     APPLICATION->closeAllWindows();

launcher/minecraft/mod/ResourceFolderModel.cpp

@@ -311,7 +311,7 @@ bool ResourceFolderModel::update()
     connect(m_current_update_task.get(), &Task::failed, this, &ResourceFolderModel::onUpdateFailed, Qt::ConnectionType::QueuedConnection);
     connect(
         m_current_update_task.get(), &Task::finished, this,
-        [=] {
+        [this] {
             m_current_update_task.reset();
             if (m_scheduled_update) {
                 m_scheduled_update = false;
@@ -343,12 +343,14 @@ void ResourceFolderModel::resolveResource(Resource* res)
     m_active_parse_tasks.insert(ticket, task);
 
     connect(
-        task.get(), &Task::succeeded, this, [=] { onParseSucceeded(ticket, res->internal_id()); }, Qt::ConnectionType::QueuedConnection);
+        task.get(), &Task::succeeded, this, [this, ticket, res] { onParseSucceeded(ticket, res->internal_id()); },
+        Qt::ConnectionType::QueuedConnection);
     connect(
-        task.get(), &Task::failed, this, [=] { onParseFailed(ticket, res->internal_id()); }, Qt::ConnectionType::QueuedConnection);
+        task.get(), &Task::failed, this, [this, ticket, res] { onParseFailed(ticket, res->internal_id()); },
+        Qt::ConnectionType::QueuedConnection);
     connect(
         task.get(), &Task::finished, this,
-        [=] {
+        [this, ticket] {
             m_active_parse_tasks.remove(ticket);
             emit parseFinished();
         },