Address more review comments

* Make Validation a singleton instead
* Wording changes for error messages
* Smart pointers for Ui members
* Other minor nitpicks

src/citra_qt/configuration/config.cpp

@@ -238,8 +238,12 @@ void Config::ReadValues() {
     UISettings::values.port = qt_config->value("port", Network::DefaultRoomPort).toString();
     UISettings::values.room_nickname = qt_config->value("room_nickname", "").toString();
     UISettings::values.room_name = qt_config->value("room_name", "").toString();
-    UISettings::values.room_port = qt_config->value("room_port", 24872).toString();
+    UISettings::values.room_port = qt_config->value("room_port", "24872").toString();
-    UISettings::values.host_type = qt_config->value("host_type", 0).toString();
+    bool ok;
+    UISettings::values.host_type = qt_config->value("host_type", 0).toUInt(&ok);
+    if (!ok) {
+        UISettings::values.host_type = 0;
+    }
     UISettings::values.max_player = qt_config->value("max_player", 8).toUInt();
     UISettings::values.game_id = qt_config->value("game_id", 0).toULongLong();
     qt_config->endGroup();

src/citra_qt/main.cpp

@@ -133,8 +133,6 @@ GMainWindow::GMainWindow() : config(new Config()), emu_thread(nullptr) {
 
     SetupUIStrings();
 
-    setWindowTitle(QString("Citra %1| %2-%3")
-                       .arg(Common::g_build_name, Common::g_scm_branch, Common::g_scm_desc));
     show();
 
     game_list->LoadCompatibilityList();

src/citra_qt/main.h

@@ -55,10 +55,11 @@ public:
     void filterBarSetChecked(bool state);
     void UpdateUITheme();
 
-    GameList* game_list;
     GMainWindow();
     ~GMainWindow();
 
+    GameList* game_list;
+
 signals:
 
     /**
@@ -180,8 +181,6 @@ private:
 
     GRenderWindow* render_window;
 
-    QFutureWatcher<Service::AM::InstallStatus>* watcher = nullptr;
-
     // Status bar elements
     QProgressBar* progress_bar = nullptr;
     QLabel* message_label = nullptr;

src/citra_qt/multiplayer/chat_room.cpp

@@ -22,7 +22,7 @@ class ChatMessage {
 public:
     explicit ChatMessage(const Network::ChatEntry& chat, QTime ts = {}) {
         /// Convert the time to their default locale defined format
-        static QLocale locale;
+        QLocale locale;
         timestamp = locale.toString(ts.isValid() ? ts : QTime::currentTime(), QLocale::ShortFormat);
         nickname = QString::fromStdString(chat.nickname);
         message = QString::fromStdString(chat.message);
@@ -60,12 +60,12 @@ public:
     }
 
 private:
-    const QString system_color = "#888888";
+    static constexpr const char system_color[] = "#888888";
     QString timestamp;
     QString message;
 };
 
-ChatRoom::ChatRoom(QWidget* parent) : QWidget(parent), ui(new Ui::ChatRoom) {
+ChatRoom::ChatRoom(QWidget* parent) : QWidget(parent), ui(std::make_unique<Ui::ChatRoom>()) {
     ui->setupUi(this);
 
     // set the item_model for player_view
@@ -148,7 +148,7 @@ void ChatRoom::OnChatReceive(const Network::ChatEntry& chat) {
                                    return member.nickname == chat.nickname;
                                });
         if (it == members.end()) {
-            LOG_INFO(Network, "Chat message received from unknown player. Ignoring it.");
+            NGLOG_INFO(Network, "Chat message received from unknown player. Ignoring it.");
             return;
         }
         auto player = std::distance(members.begin(), it);
@@ -175,7 +175,7 @@ void ChatRoom::OnSendChat() {
                                    return member.nickname == chat.nickname;
                                });
         if (it == members.end()) {
-            LOG_INFO(Network, "Chat message received from unknown player");
+            NGLOG_INFO(Network, "Cannot find self in the player list when sending a message.");
         }
         auto player = std::distance(members.begin(), it);
         ChatMessage m(chat);

src/citra_qt/multiplayer/chat_room.h

@@ -49,7 +49,7 @@ private:
     void AppendChatMessage(const QString&);
     bool ValidateMessage(const std::string&);
     QStandardItemModel* player_list;
-    Ui::ChatRoom* ui;
+    std::unique_ptr<Ui::ChatRoom> ui;
 };
 
 Q_DECLARE_METATYPE(Network::ChatEntry);

src/citra_qt/multiplayer/client_room.cpp

@@ -19,7 +19,7 @@
 
 ClientRoomWindow::ClientRoomWindow(QWidget* parent)
     : QDialog(parent, Qt::WindowTitleHint | Qt::WindowCloseButtonHint | Qt::WindowSystemMenuHint),
-      ui(new Ui::ClientRoom) {
+      ui(std::make_unique<Ui::ClientRoom>()) {
     ui->setupUi(this);
 
     // setup the callbacks for network updates
@@ -49,39 +49,39 @@ void ClientRoomWindow::OnRoomUpdate(const Network::RoomInformation& info) {
 void ClientRoomWindow::OnStateChange(const Network::RoomMember::State& state) {
     switch (state) {
     case Network::RoomMember::State::Idle:
-        LOG_INFO(Network, "State: Idle");
+        NGLOG_INFO(Network, "State: Idle");
         break;
     case Network::RoomMember::State::Joining:
-        LOG_INFO(Network, "State: Joining");
+        NGLOG_INFO(Network, "State: Joining");
         break;
     case Network::RoomMember::State::Joined:
-        LOG_INFO(Network, "State: Joined");
+        NGLOG_INFO(Network, "State: Joined");
         ui->chat->Clear();
         ui->chat->AppendStatusMessage(tr("Connected"));
         break;
     case Network::RoomMember::State::LostConnection:
         NetworkMessage::ShowError(NetworkMessage::LOST_CONNECTION);
-        LOG_INFO(Network, "State: LostConnection");
+        NGLOG_INFO(Network, "State: LostConnection");
         break;
     case Network::RoomMember::State::CouldNotConnect:
         NetworkMessage::ShowError(NetworkMessage::UNABLE_TO_CONNECT);
-        LOG_INFO(Network, "State: CouldNotConnect");
+        NGLOG_INFO(Network, "State: CouldNotConnect");
         break;
     case Network::RoomMember::State::NameCollision:
         NetworkMessage::ShowError(NetworkMessage::USERNAME_IN_USE);
-        LOG_INFO(Network, "State: NameCollision");
+        NGLOG_INFO(Network, "State: NameCollision");
         break;
     case Network::RoomMember::State::MacCollision:
         NetworkMessage::ShowError(NetworkMessage::MAC_COLLISION);
-        LOG_INFO(Network, "State: MacCollision");
+        NGLOG_INFO(Network, "State: MacCollision");
         break;
     case Network::RoomMember::State::WrongPassword:
         NetworkMessage::ShowError(NetworkMessage::WRONG_PASSWORD);
-        LOG_INFO(Network, "State: WrongPassword");
+        NGLOG_INFO(Network, "State: WrongPassword");
         break;
     case Network::RoomMember::State::WrongVersion:
         NetworkMessage::ShowError(NetworkMessage::WRONG_VERSION);
-        LOG_INFO(Network, "State: WrongVersion");
+        NGLOG_INFO(Network, "State: WrongVersion");
         break;
     default:
         break;

src/citra_qt/multiplayer/client_room.h

@@ -34,5 +34,5 @@ private:
     void UpdateView();
 
     QStandardItemModel* player_list;
-    Ui::ClientRoom* ui;
+    std::unique_ptr<Ui::ClientRoom> ui;
 };

src/citra_qt/multiplayer/direct_connect.cpp

@@ -23,7 +23,7 @@ enum class ConnectionType : u8 { TraversalServer, IP };
 
 DirectConnectWindow::DirectConnectWindow(QWidget* parent)
     : QDialog(parent, Qt::WindowTitleHint | Qt::WindowCloseButtonHint | Qt::WindowSystemMenuHint),
-      ui(new Ui::DirectConnect) {
+      ui(std::make_unique<Ui::DirectConnect>()) {
 
     ui->setupUi(this);
 
@@ -31,11 +31,11 @@ DirectConnectWindow::DirectConnectWindow(QWidget* parent)
     watcher = new QFutureWatcher<void>;
     connect(watcher, &QFutureWatcher<void>::finished, this, &DirectConnectWindow::OnConnection);
 
-    ui->nickname->setValidator(Validation::nickname);
+    ui->nickname->setValidator(Validation::get().nickname);
     ui->nickname->setText(UISettings::values.nickname);
-    ui->ip->setValidator(Validation::ip);
+    ui->ip->setValidator(Validation::get().ip);
     ui->ip->setText(UISettings::values.ip);
-    ui->port->setValidator(Validation::port);
+    ui->port->setValidator(Validation::get().port);
     ui->port->setText(UISettings::values.port);
 
     // TODO(jroweboy): Show or hide the connection options based on the current value of the combo

src/citra_qt/multiplayer/direct_connect.h

@@ -34,5 +34,5 @@ private:
     void EndConnecting();
 
     QFutureWatcher<void>* watcher;
-    Ui::DirectConnect* ui;
+    std::unique_ptr<Ui::DirectConnect> ui;
 };

src/citra_qt/multiplayer/host_room.cpp

@@ -26,13 +26,13 @@
 HostRoomWindow::HostRoomWindow(QWidget* parent, QStandardItemModel* list,
                                std::shared_ptr<Core::AnnounceMultiplayerSession> session)
     : QDialog(parent, Qt::WindowTitleHint | Qt::WindowCloseButtonHint | Qt::WindowSystemMenuHint),
-      ui(new Ui::HostRoom), announce_multiplayer_session(session), game_list(list) {
+      ui(std::make_unique<Ui::HostRoom>()), announce_multiplayer_session(session), game_list(list) {
     ui->setupUi(this);
 
     // set up validation for all of the fields
-    ui->room_name->setValidator(Validation::room_name);
+    ui->room_name->setValidator(Validation::get().room_name);
-    ui->username->setValidator(Validation::nickname);
+    ui->username->setValidator(Validation::get().nickname);
-    ui->port->setValidator(Validation::port);
+    ui->port->setValidator(Validation::get().port);
     ui->port->setPlaceholderText(QString::number(Network::DefaultRoomPort));
 
     // Create a proxy to the game list to display the list of preferred games
@@ -49,8 +49,8 @@ HostRoomWindow::HostRoomWindow(QWidget* parent, QStandardItemModel* list,
     ui->room_name->setText(UISettings::values.room_name);
     ui->port->setText(UISettings::values.room_port);
     ui->max_player->setValue(UISettings::values.max_player);
-    int index = ui->host_type->findData(UISettings::values.host_type);
+    int index = UISettings::values.host_type;
-    if (index != -1) {
+    if (index < ui->host_type->count()) {
         ui->host_type->setCurrentIndex(index);
     }
     index = ui->game_list->findData(UISettings::values.game_id, GameListItemPath::ProgramIdRole);
@@ -94,7 +94,7 @@ void HostRoomWindow::Host() {
                                         ui->max_player->value(), game_name.toStdString(), game_id);
             if (!created) {
                 NetworkMessage::ShowError(NetworkMessage::COULD_NOT_CREATE_ROOM);
-                LOG_ERROR(Network, "Could not create room!");
+                NGLOG_ERROR(Network, "Could not create room!");
                 ui->host->setEnabled(true);
                 return;
             }
@@ -109,7 +109,7 @@ void HostRoomWindow::Host() {
             ui->game_list->currentData(GameListItemPath::ProgramIdRole).toLongLong();
         UISettings::values.max_player = ui->max_player->value();
 
-        UISettings::values.host_type = ui->host_type->currentText();
+        UISettings::values.host_type = ui->host_type->currentIndex();
         UISettings::values.room_port = (ui->port->isModified() && !ui->port->text().isEmpty())
                                            ? ui->port->text()
                                            : QString::number(Network::DefaultRoomPort);
@@ -136,7 +136,7 @@ void HostRoomWindow::OnConnection() {
                 if (auto session = announce_multiplayer_session.lock()) {
                     session->Start();
                 } else {
-                    LOG_ERROR(Network, "Starting announce session failed");
+                    NGLOG_ERROR(Network, "Starting announce session failed");
                 }
             }
             auto parent = static_cast<MultiplayerState*>(parentWidget());

src/citra_qt/multiplayer/host_room.h

@@ -52,7 +52,7 @@ private:
     std::weak_ptr<Core::AnnounceMultiplayerSession> announce_multiplayer_session;
     QStandardItemModel* game_list;
     ComboBoxProxyModel* proxy;
-    Ui::HostRoom* ui;
+    std::unique_ptr<Ui::HostRoom> ui;
 };
 
 /**

src/citra_qt/multiplayer/lobby.cpp

@@ -22,7 +22,7 @@
 Lobby::Lobby(QWidget* parent, QStandardItemModel* list,
              std::shared_ptr<Core::AnnounceMultiplayerSession> session)
     : QDialog(parent, Qt::WindowTitleHint | Qt::WindowCloseButtonHint | Qt::WindowSystemMenuHint),
-      ui(new Ui::Lobby), announce_multiplayer_session(session), game_list(list) {
+      ui(std::make_unique<Ui::Lobby>()), announce_multiplayer_session(session), game_list(list) {
     ui->setupUi(this);
 
     // setup the watcher for background connections
@@ -48,7 +48,7 @@ Lobby::Lobby(QWidget* parent, QStandardItemModel* list,
     ui->room_list->setExpandsOnDoubleClick(false);
     ui->room_list->setContextMenuPolicy(Qt::CustomContextMenu);
 
-    ui->nickname->setValidator(Validation::nickname);
+    ui->nickname->setValidator(Validation::get().nickname);
     ui->nickname->setText(UISettings::values.nickname);
 
     // UI Buttons
@@ -74,7 +74,7 @@ Lobby::Lobby(QWidget* parent, QStandardItemModel* list,
     RefreshLobby();
 }
 
-const QString Lobby::PasswordPrompt() {
+QString Lobby::PasswordPrompt() {
     bool ok;
     const QString text =
         QInputDialog::getText(this, tr("Password Required to Join"), tr("Password:"),

src/citra_qt/multiplayer/lobby.h

@@ -97,7 +97,7 @@ private:
      * Prompts for a password. Returns an empty QString if the user either did not provide a
      * password or if the user closed the window.
      */
-    const QString PasswordPrompt();
+    QString PasswordPrompt();
 
     QStandardItemModel* model;
     QStandardItemModel* game_list;
@@ -105,7 +105,7 @@ private:
 
     std::future<AnnounceMultiplayerRoom::RoomList> room_list_future;
     std::weak_ptr<Core::AnnounceMultiplayerSession> announce_multiplayer_session;
-    Ui::Lobby* ui;
+    std::unique_ptr<Ui::Lobby> ui;
     QFutureWatcher<void>* watcher;
 };
 

src/citra_qt/multiplayer/message.cpp

@@ -19,20 +19,22 @@ const ConnectionError PORT_NOT_VALID(QT_TR_NOOP("Port must be a number between 0
 const ConnectionError NO_INTERNET(
     QT_TR_NOOP("Unable to find an internet connection. Check your internet settings."));
 const ConnectionError UNABLE_TO_CONNECT(
-    QT_TR_NOOP("Unable to connect to the host. Verify that the connection settings are correct."));
+    QT_TR_NOOP("Unable to connect to the host. Verify that the connection settings are correct. If "
+               "you still cannot connect, contact the room host and verify that the host is "
+               "properly configured with the external port forwarded."));
 const ConnectionError COULD_NOT_CREATE_ROOM(
     QT_TR_NOOP("Creating a room failed. Please retry. Restarting Citra might be necessary."));
 const ConnectionError HOST_BANNED(
     QT_TR_NOOP("The host of the room has banned you. Speak with the host to unban you "
                "or try a different room."));
 const ConnectionError WRONG_VERSION(
-    QT_TR_NOOP("You are using a different version of Citra-Local-Wifi(tm) then the room "
+    QT_TR_NOOP("Version mismatch! Please update to the latest version of citra. If the problem "
-               "you are trying to connect to."));
+               "persists, contact the room host and ask them to update the server."));
-const ConnectionError WRONG_PASSWORD(QT_TR_NOOP("Wrong password."));
+const ConnectionError WRONG_PASSWORD(QT_TR_NOOP("Incorrect password."));
 const ConnectionError GENERIC_ERROR(QT_TR_NOOP("An error occured."));
 const ConnectionError LOST_CONNECTION(QT_TR_NOOP("Connection to room lost. Try to reconnect."));
 const ConnectionError MAC_COLLISION(
-    QT_TR_NOOP("MAC-Address is already in use. Please choose another."));
+    QT_TR_NOOP("MAC address is already in use. Please choose another."));
 
 static bool WarnMessage(const std::string& title, const std::string& text) {
     return QMessageBox::Ok == QMessageBox::warning(nullptr, QObject::tr(title.c_str()),

src/citra_qt/multiplayer/state.cpp

@@ -82,11 +82,14 @@ void MultiplayerState::OnNetworkStateChanged(const Network::RoomMember::State& s
 
 void MultiplayerState::OnAnnounceFailed(const Common::WebResult& result) {
     announce_multiplayer_session->Stop();
-    QMessageBox::warning(this, tr("Error"),
+    QMessageBox::warning(
-                         tr("Failed to announce the room to the public lobby.\nThe room will not "
+        this, tr("Error"),
-                            "get listed publicly.\nError: ") +
+        tr("Failed to announce the room to the public lobby. In order to host a room publicly, you "
-                             QString::fromStdString(result.result_string),
+           "must have a valid Citra account configured in Emulation -> Configure -> Web. If you do "
-                         QMessageBox::Ok);
+           "not want to publish a room in the public lobby, then select Unlisted instead.\n"
+           "Debug Message: ") +
+            QString::fromStdString(result.result_string),
+        QMessageBox::Ok);
 }
 
 static void BringWidgetToFront(QWidget* widget) {

src/citra_qt/multiplayer/validation.h

@@ -7,22 +7,38 @@
 #include <QRegExp>
 #include <QValidator>
 
-namespace Validation {
+class Validation {
-/// room name can be alphanumeric and " " "_" "." and "-"
+public:
-static const QRegExp room_name_regex("^[a-zA-Z0-9._- ]+$");
+    static Validation get() {
-static const QValidator* room_name = new QRegExpValidator(room_name_regex);
+        static Validation validation;
+        return validation;
+    }
 
-/// nickname can be alphanumeric and " " "_" "." and "-"
+    ~Validation() {
-static const QRegExp nickname_regex("^[a-zA-Z0-9._- ]+$");
+        delete room_name;
-static const QValidator* nickname = new QRegExpValidator(nickname_regex);
+        delete nickname;
+        delete ip;
+        delete port;
+    }
 
-/// ipv4 address only
+    /// room name can be alphanumeric and " " "_" "." and "-"
-// TODO remove this when we support hostnames in direct connect
+    QRegExp room_name_regex = QRegExp("^[a-zA-Z0-9._- ]+$");
-static const QRegExp ip_regex(
+    const QValidator* room_name = new QRegExpValidator(room_name_regex);
-    "(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|"
-    "2[0-4][0-9]|25[0-5])");
-static const QValidator* ip = new QRegExpValidator(ip_regex);
 
-/// port must be between 0 and 65535
+    /// nickname can be alphanumeric and " " "_" "." and "-"
-static const QValidator* port = new QIntValidator(0, 65535);
+    QRegExp nickname_regex = QRegExp("^[a-zA-Z0-9._- ]+$");
-}; // namespace Validation
+    const QValidator* nickname = new QRegExpValidator(nickname_regex);
+
+    /// ipv4 address only
+    // TODO remove this when we support hostnames in direct connect
+    QRegExp ip_regex = QRegExp(
+        "(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|"
+        "2[0-4][0-9]|25[0-5])");
+    const QValidator* ip = new QRegExpValidator(ip_regex);
+
+    /// port must be between 0 and 65535
+    const QValidator* port = new QIntValidator(0, 65535);
+
+private:
+    Validation() = default;
+};

src/citra_qt/ui_settings.h

@@ -65,7 +65,7 @@ struct Values {
     QString room_name;
     quint32 max_player;
     QString room_port;
-    QString host_type;
+    uint host_type;
     qulonglong game_id;
 };