From 0928b1bff1ad0687fb72d9282db2911fea0ebd3e Mon Sep 17 00:00:00 2001 From: Wayne Chen Date: Fri, 25 Jan 2019 11:36:58 -0800 Subject: [PATCH] code review feedback --- interface/src/ui/LoginDialog.cpp | 63 +++++++++---------- .../src/plugins/OculusPlatformPlugin.h | 4 +- plugins/oculus/src/OculusPlatformPlugin.cpp | 10 ++- plugins/oculus/src/OculusPlatformPlugin.h | 6 +- 4 files changed, 38 insertions(+), 45 deletions(-) diff --git a/interface/src/ui/LoginDialog.cpp b/interface/src/ui/LoginDialog.cpp index 74d6f7879c..f05219a8fb 100644 --- a/interface/src/ui/LoginDialog.cpp +++ b/interface/src/ui/LoginDialog.cpp @@ -160,8 +160,8 @@ void LoginDialog::linkOculus() { const QString LINK_OCULUS_PATH = "api/v1/user/oculus/link"; QJsonObject payload; - payload.insert("oculus_nonce", QJsonValue::fromVariant(QVariant(nonce))); - payload.insert("oculus_id", QJsonValue::fromVariant(QVariant(oculusID))); + payload["oculus_nonce"] = nonce; + payload["oculus_id"] = oculusID; auto accountManager = DependencyManager::get(); accountManager->sendRequest(LINK_OCULUS_PATH, AccountManagerAuth::Required, @@ -188,16 +188,16 @@ void LoginDialog::createAccountFromOculus(QString email, QString username, QStri const QString CREATE_ACCOUNT_FROM_OCULUS_PATH = "api/v1/user/oculus/create"; QJsonObject payload; - payload.insert("oculus_nonce", QJsonValue::fromVariant(QVariant(nonce))); - payload.insert("oculus_id", QJsonValue::fromVariant(QVariant(oculusID))); + payload["oculus_nonce"] = nonce; + payload["oculus_id"] = oculusID; if (!email.isEmpty()) { - payload.insert("email", QJsonValue::fromVariant(QVariant(email))); + payload["email"] = email; } if (!username.isEmpty()) { - payload.insert("username", QJsonValue::fromVariant(QVariant(username))); + payload["username"] = username; } if (!password.isEmpty()) { - payload.insert("password", QJsonValue::fromVariant(QVariant(password))); + payload["password"] = password; } auto accountManager = DependencyManager::get(); @@ -239,7 +239,7 @@ void LoginDialog::linkSteam() { const QString LINK_STEAM_PATH = "api/v1/user/steam/link"; QJsonObject payload; - payload.insert("steam_auth_ticket", QJsonValue::fromVariant(QVariant(ticket))); + payload["steam_auth_ticket"] = QJsonValue::fromVariant(QVariant(ticket)); auto accountManager = DependencyManager::get(); accountManager->sendRequest(LINK_STEAM_PATH, AccountManagerAuth::Required, @@ -266,9 +266,9 @@ void LoginDialog::createAccountFromSteam(QString username) { const QString CREATE_ACCOUNT_FROM_STEAM_PATH = "api/v1/user/steam/create"; QJsonObject payload; - payload.insert("steam_auth_ticket", QJsonValue::fromVariant(QVariant(ticket))); + payload["steam_auth_ticket"] = QJsonValue::fromVariant(QVariant(ticket)); if (!username.isEmpty()) { - payload.insert("username", QJsonValue::fromVariant(QVariant(username))); + payload["username"] = username; } auto accountManager = DependencyManager::get(); @@ -305,40 +305,35 @@ void LoginDialog::createFailed(QNetworkReply* reply) { return; } auto root = doc.object(); - auto data = root.value("data").toObject(); - auto error = data.value("error").toObject(); - auto oculusError = data.value("oculus"); - auto user = error.value("username"); - auto uid = error.value("uid"); - auto email = error.value("email"); - auto password = error.value("password"); + auto data = root["data"].toObject(); + auto error = data["error"].toObject(); + auto oculusError = data["oculus"]; + auto user = error["username"].toArray(); + auto uid = error["uid"].toArray(); + auto email = error["email"].toArray(); + auto password = error["password"].toArray(); QString reply; - QString emailError; - if (!uid.isNull() && !uid.isUndefined()) { - QJsonArray arr = uid.toArray(); - if (!arr.isEmpty()) { - emit handleCreateFailed("Oculus ID " + arr.at(0).toString() + "."); + if (!uid.isEmpty()) { + if (uid[0].isString()) { + emit handleCreateFailed("Oculus ID " + uid[0].toString() + "."); return; } } - if (!user.isNull() && !user.isUndefined()) { - QJsonArray arr = user.toArray(); - if (!arr.isEmpty()) { - reply = "Username " + arr.at(0).toString() + "."; + if (!user.isEmpty()) { + if (user[0].isString()) { + reply = "Username " + user[0].toString() + "."; } } - if (!email.isNull() && !email.isUndefined()) { - QJsonArray arr = email.toArray(); - if (!arr.isEmpty()) { + if (!email.isEmpty()) { + if (email[0].isString()) { reply.append((!reply.isEmpty()) ? "\n" : ""); - reply.append("Email " + arr.at(0).toString() + "."); + reply.append("Email " + email[0].toString() + "."); } } - if (!password.isNull() && !password.isUndefined()) { - QJsonArray arr = password.toArray(); - if (!arr.isEmpty()) { + if (!password.isEmpty()) { + if (password[0].isString()) { reply.append((!reply.isEmpty()) ? "\n" : ""); - reply.append("Password " + arr.at(0).toString() + "."); + reply.append("Password " + password[0].toString() + "."); } } if (!oculusError.isNull() && !oculusError.isUndefined()) { diff --git a/libraries/plugins/src/plugins/OculusPlatformPlugin.h b/libraries/plugins/src/plugins/OculusPlatformPlugin.h index 26939ee74b..390dd1af71 100644 --- a/libraries/plugins/src/plugins/OculusPlatformPlugin.h +++ b/libraries/plugins/src/plugins/OculusPlatformPlugin.h @@ -17,8 +17,8 @@ class OculusPlatformPlugin { public: virtual ~OculusPlatformPlugin() = default; - virtual const QString getName() const = 0; - virtual const QString getOculusUserID() const = 0; + virtual QString getName() = 0; + virtual QString getOculusUserID() = 0; virtual bool isRunning() = 0; diff --git a/plugins/oculus/src/OculusPlatformPlugin.cpp b/plugins/oculus/src/OculusPlatformPlugin.cpp index 0347ab08d8..516d5b99ee 100644 --- a/plugins/oculus/src/OculusPlatformPlugin.cpp +++ b/plugins/oculus/src/OculusPlatformPlugin.cpp @@ -14,7 +14,7 @@ #include "OculusHelpers.h" -const char* OculusAPIPlugin::NAME { "Oculus Rift" }; +QString OculusAPIPlugin::NAME { "Oculus Rift" }; OculusAPIPlugin::OculusAPIPlugin() { _session = hifi::ovr::acquireRenderSession(); @@ -40,9 +40,10 @@ void OculusAPIPlugin::handleOVREvents() { #ifdef OCULUS_APP_ID if (qApp->property(hifi::properties::OCULUS_STORE).toBool()) { // pop messages to see if we got a return for an entitlement check - ovrMessageHandle message = ovr_PopMessage(); + ovrMessageHandle message { nullptr }; - while (message) { + // pop the next message to check, if there is one + while ((message = ovr_PopMessage())) { switch (ovr_Message_GetType(message)) { case ovrMessage_Entitlement_GetIsViewerEntitled: { if (!ovr_Message_IsError(message)) { @@ -104,9 +105,6 @@ void OculusAPIPlugin::handleOVREvents() { // free the message handle to cleanup and not leak ovr_FreeMessage(message); - - // pop the next message to check, if there is one - message = ovr_PopMessage(); } } #endif diff --git a/plugins/oculus/src/OculusPlatformPlugin.h b/plugins/oculus/src/OculusPlatformPlugin.h index c4919dfbe4..1c8f090cfa 100644 --- a/plugins/oculus/src/OculusPlatformPlugin.h +++ b/plugins/oculus/src/OculusPlatformPlugin.h @@ -18,8 +18,8 @@ class OculusAPIPlugin : public OculusPlatformPlugin { public: OculusAPIPlugin(); virtual ~OculusAPIPlugin(); - const QString getName() const { return NAME; } - const QString getOculusUserID() const { return _user; }; + QString getName() { return NAME; } + QString getOculusUserID() { return _user; }; bool isRunning(); @@ -28,7 +28,7 @@ public: virtual void handleOVREvents(); private: - static const char* NAME; + static QString NAME; NonceUserIDCallback _nonceUserIDCallback; QString _nonce; bool _nonceChanged{ false };