From 1b884d0a9dc28d8bca38fe8756482d991d0ea850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Mr=C3=A1zek?= Date: Mon, 4 May 2015 01:20:48 +0200 Subject: GH-907 improve Java testing and PermGen deprecation handling --- application/CMakeLists.txt | 7 +- application/JavaCommon.cpp | 107 +++++++++++++++++++++++++++++ application/JavaCommon.h | 46 +++++++++++++ application/MainWindow.cpp | 8 +-- application/MultiMC.cpp | 2 + application/NagUtils.cpp | 38 ---------- application/NagUtils.h | 23 ------- application/pages/InstanceSettingsPage.cpp | 41 ++++------- application/pages/InstanceSettingsPage.h | 11 +-- application/pages/global/JavaPage.cpp | 43 ++++-------- application/pages/global/JavaPage.h | 9 ++- application/pages/global/JavaPage.ui | 80 +++++++++++---------- application/pages/global/MinecraftPage.cpp | 12 ---- application/pages/global/MultiMCPage.cpp | 15 +--- 14 files changed, 241 insertions(+), 201 deletions(-) create mode 100644 application/JavaCommon.cpp create mode 100644 application/JavaCommon.h delete mode 100644 application/NagUtils.cpp delete mode 100644 application/NagUtils.h (limited to 'application') diff --git a/application/CMakeLists.txt b/application/CMakeLists.txt index 2518e52e..7ec96e5d 100644 --- a/application/CMakeLists.txt +++ b/application/CMakeLists.txt @@ -156,10 +156,9 @@ SET(MULTIMC_SOURCES InstancePageProvider.h InstancePageProvider.cpp - # Annoying nag screen logic - NagUtils.h - NagUtils.cpp - + # Common java checking UI + JavaCommon.h + JavaCommon.cpp # GUI - page dialog pages pages/BasePage.h diff --git a/application/JavaCommon.cpp b/application/JavaCommon.cpp new file mode 100644 index 00000000..d0a3cba3 --- /dev/null +++ b/application/JavaCommon.cpp @@ -0,0 +1,107 @@ +#include "JavaCommon.h" +#include "dialogs/CustomMessageBox.h" +#include + +bool JavaCommon::checkJVMArgs(QString jvmargs, QWidget *parent) +{ + if (jvmargs.contains("-XX:PermSize=") || jvmargs.contains(QRegExp("-Xm[sx]"))) + { + CustomMessageBox::selectable( + parent, QObject::tr("JVM arguments warning"), + QObject::tr("You tried to manually set a JVM memory option (using " + " \"-XX:PermSize\", \"-Xmx\" or \"-Xms\") - there" + " are dedicated boxes for these in the settings (Java" + " tab, in the Memory group at the top).\n" + "Your manual settings will be overridden by the" + " dedicated options.\n" + "This message will be displayed until you remove them" + " from the JVM arguments."), + QMessageBox::Warning)->exec(); + return false; + } + return true; +} + +void JavaCommon::TestCheck::javaWasOk(JavaCheckResult result) +{ + QString text; + text += tr("Java test succeeded!
Platform reported: %1
Java version " + "reported: %2
").arg(result.realPlatform, result.javaVersion); + if (result.stderr.size()) + { + auto htmlError = result.stderr; + htmlError.replace('\n', "
"); + text += tr("
Warnings:
%1").arg(htmlError); + } + CustomMessageBox::selectable(m_parent, tr("Java test success"), text, + QMessageBox::Information)->show(); +} + +void JavaCommon::TestCheck::javaArgsWereBad(JavaCheckResult result) +{ + auto htmlError = result.stderr; + QString text; + htmlError.replace('\n', "
"); + text += tr("The specified java binary didn't work with the arguments you provided:
"); + text += tr("%1").arg(htmlError); + CustomMessageBox::selectable(m_parent, tr("Java test failure"), text, QMessageBox::Warning) + ->show(); +} + +void JavaCommon::TestCheck::javaBinaryWasBad(JavaCheckResult result) +{ + QString text; + text += tr( + "The specified java binary didn't work.
You should use the auto-detect feature, " + "or set the path to the java executable.
"); + CustomMessageBox::selectable(m_parent, tr("Java test failure"), text, QMessageBox::Warning) + ->show(); +} + +void JavaCommon::TestCheck::run() +{ + if (!JavaCommon::checkJVMArgs(m_args, m_parent)) + { + emit finished(); + return; + } + checker.reset(new JavaChecker()); + connect(checker.get(), SIGNAL(checkFinished(JavaCheckResult)), this, + SLOT(checkFinished(JavaCheckResult))); + checker->m_path = m_path; + checker->performCheck(); +} + +void JavaCommon::TestCheck::checkFinished(JavaCheckResult result) +{ + if (!result.valid) + { + javaBinaryWasBad(result); + emit finished(); + return; + } + checker.reset(new JavaChecker()); + connect(checker.get(), SIGNAL(checkFinished(JavaCheckResult)), this, + SLOT(checkFinishedWithArgs(JavaCheckResult))); + checker->m_path = m_path; + checker->m_args = m_args; + checker->m_minMem = m_minMem; + checker->m_maxMem = m_maxMem; + if (Strings::naturalCompare(result.javaVersion, "1.8", Qt::CaseInsensitive) < 0) + { + checker->m_permGen = m_permGen; + } + checker->performCheck(); +} + +void JavaCommon::TestCheck::checkFinishedWithArgs(JavaCheckResult result) +{ + if (result.valid) + { + javaWasOk(result); + emit finished(); + return; + } + javaArgsWereBad(result); + emit finished(); +} diff --git a/application/JavaCommon.h b/application/JavaCommon.h new file mode 100644 index 00000000..b100c213 --- /dev/null +++ b/application/JavaCommon.h @@ -0,0 +1,46 @@ +#pragma once +#include + +class QWidget; + +/** + * Common UI bits for the java pages to use. + */ +namespace JavaCommon +{ + bool checkJVMArgs(QString args, QWidget *parent); + + class TestCheck : public QObject + { + Q_OBJECT + public: + TestCheck(QWidget *parent, QString path, QString args, int minMem, int maxMem, int permGen) + :m_parent(parent), m_path(path), m_args(args), m_minMem(minMem), m_maxMem(maxMem), m_permGen(permGen) + { + } + virtual ~TestCheck() {}; + + void run(); + + signals: + void finished(); + + private: + void javaBinaryWasBad(JavaCheckResult result); + void javaArgsWereBad(JavaCheckResult result); + void javaWasOk(JavaCheckResult result); + + private slots: + void checkFinished(JavaCheckResult result); + void checkFinishedWithArgs(JavaCheckResult result); + + private: + std::shared_ptr checker; + QWidget *m_parent = nullptr; + QString m_path; + QString m_args; + int m_minMem = 0; + int m_maxMem = 0; + int m_permGen = 64; + }; +} diff --git a/application/MainWindow.cpp b/application/MainWindow.cpp index 055a38c9..608aca9c 100644 --- a/application/MainWindow.cpp +++ b/application/MainWindow.cpp @@ -381,7 +381,7 @@ namespace Ui { #include "BaseInstance.h" #include "BaseProcess.h" #include "java/JavaUtils.h" -#include "NagUtils.h" +#include "JavaCommon.h" #include "InstancePageProvider.h" #include "minecraft/SkinUtils.h" @@ -1545,7 +1545,7 @@ void MainWindow::instanceActivated(QModelIndex index) if (!inst) return; - NagUtils::checkJVMArgs(inst->settings().get("JvmArgs").toString(), this); + JavaCommon::checkJVMArgs(inst->settings().get("JvmArgs").toString(), this); doLaunch(); } @@ -1554,7 +1554,7 @@ void MainWindow::on_actionLaunchInstance_triggered() { if (m_selectedInstance) { - NagUtils::checkJVMArgs(m_selectedInstance->settings().get("JvmArgs").toString(), this); + JavaCommon::checkJVMArgs(m_selectedInstance->settings().get("JvmArgs").toString(), this); doLaunch(); } } @@ -1563,7 +1563,7 @@ void MainWindow::on_actionLaunchInstanceOffline_triggered() { if (m_selectedInstance) { - NagUtils::checkJVMArgs(m_selectedInstance->settings().get("JvmArgs").toString(), this); + JavaCommon::checkJVMArgs(m_selectedInstance->settings().get("JvmArgs").toString(), this); doLaunch(false); } } diff --git a/application/MultiMC.cpp b/application/MultiMC.cpp index b993aa5e..aae8d154 100644 --- a/application/MultiMC.cpp +++ b/application/MultiMC.cpp @@ -453,6 +453,8 @@ void MultiMC::initGlobalSettings(bool test_mode) // Java Settings m_settings->registerSetting("JavaPath", ""); + m_settings->registerSetting("JavaTimestamp", 0); + m_settings->registerSetting("JavaVersion", ""); m_settings->registerSetting("LastHostname", ""); m_settings->registerSetting("JavaDetectionHack", ""); m_settings->registerSetting("JvmArgs", ""); diff --git a/application/NagUtils.cpp b/application/NagUtils.cpp deleted file mode 100644 index 41e2f63e..00000000 --- a/application/NagUtils.cpp +++ /dev/null @@ -1,38 +0,0 @@ -/* Copyright 2013-2015 MultiMC Contributors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "NagUtils.h" -#include "dialogs/CustomMessageBox.h" - -namespace NagUtils -{ -void checkJVMArgs(QString jvmargs, QWidget *parent) -{ - if (jvmargs.contains("-XX:PermSize=") || jvmargs.contains(QRegExp("-Xm[sx]"))) - { - CustomMessageBox::selectable( - parent, QObject::tr("JVM arguments warning"), - QObject::tr("You tried to manually set a JVM memory option (using " - " \"-XX:PermSize\", \"-Xmx\" or \"-Xms\") - there" - " are dedicated boxes for these in the settings (Java" - " tab, in the Memory group at the top).\n" - "Your manual settings will be overridden by the" - " dedicated options.\n" - "This message will be displayed until you remove them" - " from the JVM arguments."), - QMessageBox::Warning)->exec(); - } -} -} diff --git a/application/NagUtils.h b/application/NagUtils.h deleted file mode 100644 index d757703a..00000000 --- a/application/NagUtils.h +++ /dev/null @@ -1,23 +0,0 @@ -/* Copyright 2013-2015 MultiMC Contributors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include - -namespace NagUtils -{ -void checkJVMArgs(QString args, QWidget *parent); -} diff --git a/application/pages/InstanceSettingsPage.cpp b/application/pages/InstanceSettingsPage.cpp index 1e571eff..4fc812b2 100644 --- a/application/pages/InstanceSettingsPage.cpp +++ b/application/pages/InstanceSettingsPage.cpp @@ -6,10 +6,11 @@ #include #include "dialogs/VersionSelectDialog.h" -#include "NagUtils.h" -#include "java/JavaVersionList.h" +#include "JavaCommon.h" #include "MultiMC.h" +#include + InstanceSettingsPage::InstanceSettingsPage(BaseInstance *inst, QWidget *parent) : QWidget(parent), ui(new Ui::InstanceSettingsPage), m_instance(inst) { @@ -100,7 +101,7 @@ void InstanceSettingsPage::applySettings() if(javaArgs) { m_settings->set("JvmArgs", ui->jvmArgsTextBox->toPlainText().replace("\n", " ")); - NagUtils::checkJVMArgs(m_settings->get("JvmArgs").toString(), this->parentWidget()); + JavaCommon::checkJVMArgs(m_settings->get("JvmArgs").toString(), this->parentWidget()); } else { @@ -187,30 +188,18 @@ void InstanceSettingsPage::on_javaBrowseBtn_clicked() void InstanceSettingsPage::on_javaTestBtn_clicked() { - checker.reset(new JavaChecker()); - connect(checker.get(), SIGNAL(checkFinished(JavaCheckResult)), this, - SLOT(checkFinished(JavaCheckResult))); - checker->path = ui->javaPathTextBox->text(); - checker->performCheck(); + if(checker) + { + return; + } + checker.reset(new JavaCommon::TestCheck( + this, ui->javaPathTextBox->text(), ui->jvmArgsTextBox->toPlainText().replace("\n", " "), + ui->minMemSpinBox->value(), ui->maxMemSpinBox->value(), ui->permGenSpinBox->value())); + connect(checker.get(), SIGNAL(finished()), SLOT(checkerFinished())); + checker->run(); } -void InstanceSettingsPage::checkFinished(JavaCheckResult result) +void InstanceSettingsPage::checkerFinished() { - if (result.valid) - { - QString text; - text += "Java test succeeded!\n"; - if (result.is_64bit) - text += "Using 64bit java.\n"; - text += "\n"; - text += "Platform reported: " + result.realPlatform; - QMessageBox::information(this, tr("Java test success"), text); - } - else - { - QMessageBox::warning( - this, tr("Java test failure"), - tr("The specified java binary didn't work. You should use the auto-detect feature, " - "or set the path to the java executable.")); - } + checker.reset(); } diff --git a/application/pages/InstanceSettingsPage.h b/application/pages/InstanceSettingsPage.h index 60cc1898..55ae69db 100644 --- a/application/pages/InstanceSettingsPage.h +++ b/application/pages/InstanceSettingsPage.h @@ -19,7 +19,9 @@ #include "java/JavaChecker.h" #include "BaseInstance.h" +#include #include "BasePage.h" +#include "JavaCommon.h" #include "MultiMC.h" class JavaChecker; @@ -53,21 +55,20 @@ public: return "Instance-settings"; } virtual bool shouldDisplay() const; + private slots: void on_javaDetectBtn_clicked(); - void on_javaTestBtn_clicked(); - void on_javaBrowseBtn_clicked(); - void checkFinished(JavaCheckResult result); - void applySettings(); void loadSettings(); + void checkerFinished(); + private: Ui::InstanceSettingsPage *ui; BaseInstance *m_instance; SettingsObject *m_settings; - std::shared_ptr checker; + QObjectPtr checker; }; diff --git a/application/pages/global/JavaPage.cpp b/application/pages/global/JavaPage.cpp index 1b480376..3daf35dc 100644 --- a/application/pages/global/JavaPage.cpp +++ b/application/pages/global/JavaPage.cpp @@ -14,6 +14,7 @@ */ #include "JavaPage.h" +#include "JavaCommon.h" #include "ui_JavaPage.h" #include @@ -22,15 +23,11 @@ #include -#include "NagUtils.h" - -#include "Platform.h" #include "dialogs/VersionSelectDialog.h" #include #include "java/JavaUtils.h" #include "java/JavaVersionList.h" -#include "java/JavaChecker.h" #include "settings/SettingsObject.h" #include "MultiMC.h" @@ -69,7 +66,7 @@ void JavaPage::applySettings() // Java Settings s->set("JavaPath", ui->javaPathTextBox->text()); s->set("JvmArgs", ui->jvmArgsTextBox->text()); - NagUtils::checkJVMArgs(s->get("JvmArgs").toString(), this->parentWidget()); + JavaCommon::checkJVMArgs(s->get("JvmArgs").toString(), this->parentWidget()); // Custom Commands s->set("PreLaunchCommand", ui->preLaunchCmdTextBox->text()); @@ -113,33 +110,21 @@ void JavaPage::on_javaBrowseBtn_clicked() ui->javaPathTextBox->setText(dir); } } + void JavaPage::on_javaTestBtn_clicked() { - checker.reset(new JavaChecker()); - connect(checker.get(), SIGNAL(checkFinished(JavaCheckResult)), this, - SLOT(checkFinished(JavaCheckResult))); - checker->path = ui->javaPathTextBox->text(); - checker->performCheck(); + if(checker) + { + return; + } + checker.reset(new JavaCommon::TestCheck( + this, ui->javaPathTextBox->text(), ui->jvmArgsTextBox->text(), + ui->minMemSpinBox->value(), ui->maxMemSpinBox->value(), ui->permGenSpinBox->value())); + connect(checker.get(), SIGNAL(finished()), SLOT(checkerFinished())); + checker->run(); } -void JavaPage::checkFinished(JavaCheckResult result) +void JavaPage::checkerFinished() { - if (result.valid) - { - QString text; - text += "Java test succeeded!\n"; - if (result.is_64bit) - text += "Using 64bit java.\n"; - text += "\n"; - text += "Platform reported: " + result.realPlatform + "\n"; - text += "Java version reported: " + result.javaVersion; - QMessageBox::information(this, tr("Java test success"), text); - } - else - { - QMessageBox::warning( - this, tr("Java test failure"), - tr("The specified java binary didn't work. You should use the auto-detect feature, " - "or set the path to the java executable.")); - } + checker.reset(); } diff --git a/application/pages/global/JavaPage.h b/application/pages/global/JavaPage.h index 2af85280..4dd2b306 100644 --- a/application/pages/global/JavaPage.h +++ b/application/pages/global/JavaPage.h @@ -17,10 +17,10 @@ #include #include - -#include "java/JavaChecker.h" #include "pages/BasePage.h" +#include "JavaCommon.h" #include +#include class SettingsObject; @@ -64,10 +64,9 @@ slots: void on_javaDetectBtn_clicked(); void on_javaTestBtn_clicked(); void on_javaBrowseBtn_clicked(); - - void checkFinished(JavaCheckResult result); + void checkerFinished(); private: Ui::JavaPage *ui; - std::shared_ptr checker; + QObjectPtr checker; }; diff --git a/application/pages/global/JavaPage.ui b/application/pages/global/JavaPage.ui index 6ae41a49..f9c629c2 100644 --- a/application/pages/global/JavaPage.ui +++ b/application/pages/global/JavaPage.ui @@ -161,45 +161,6 @@ - - - - - 0 - 0 - - - - Auto-detect... - - - - - - - - 0 - 0 - - - - Test - - - - - - - - 0 - 0 - - - - JVM arguments: - - - @@ -229,6 +190,45 @@ + + + + + 0 + 0 + + + + JVM arguments: + + + + + + + + 0 + 0 + + + + Auto-detect... + + + + + + + + 0 + 0 + + + + Test + + + @@ -292,8 +292,6 @@ permGenSpinBox javaPathTextBox javaBrowseBtn - javaDetectBtn - javaTestBtn jvmArgsTextBox preLaunchCmdTextBox postExitCmdTextBox diff --git a/application/pages/global/MinecraftPage.cpp b/application/pages/global/MinecraftPage.cpp index f5e7e57f..d1f1fb08 100644 --- a/application/pages/global/MinecraftPage.cpp +++ b/application/pages/global/MinecraftPage.cpp @@ -23,18 +23,6 @@ #include #include "Platform.h" -#include "dialogs/VersionSelectDialog.h" -#include "dialogs/CustomMessageBox.h" - -#include "NagUtils.h" - -#include "java/JavaUtils.h" -#include "java/JavaVersionList.h" -#include "java/JavaChecker.h" - -#include "updater/UpdateChecker.h" - -#include "tools/BaseProfiler.h" #include "settings/SettingsObject.h" #include "MultiMC.h" diff --git a/application/pages/global/MultiMCPage.cpp b/application/pages/global/MultiMCPage.cpp index 5f56fb89..83c1ccd5 100644 --- a/application/pages/global/MultiMCPage.cpp +++ b/application/pages/global/MultiMCPage.cpp @@ -22,22 +22,9 @@ #include #include - -#include "Platform.h" -#include "dialogs/VersionSelectDialog.h" -#include "dialogs/CustomMessageBox.h" #include - -#include "NagUtils.h" - -#include "java/JavaUtils.h" -#include "java/JavaVersionList.h" -#include "java/JavaChecker.h" - #include "updater/UpdateChecker.h" -#include "tools/BaseProfiler.h" - #include "settings/SettingsObject.h" #include "MultiMC.h" @@ -456,4 +443,4 @@ void MultiMCPage::refreshFontPreview() workCursor.insertText(tr("[Something/WARN] A not so spooky warning."), format); workCursor.insertBlock(); } -} \ No newline at end of file +} -- cgit v1.2.3