code review feedback

This commit is contained in:
Wayne Chen 2019-01-25 11:36:58 -08:00
parent 9da7465698
commit 0928b1bff1
4 changed files with 38 additions and 45 deletions

View file

@ -160,8 +160,8 @@ void LoginDialog::linkOculus() {
const QString LINK_OCULUS_PATH = "api/v1/user/oculus/link"; const QString LINK_OCULUS_PATH = "api/v1/user/oculus/link";
QJsonObject payload; QJsonObject payload;
payload.insert("oculus_nonce", QJsonValue::fromVariant(QVariant(nonce))); payload["oculus_nonce"] = nonce;
payload.insert("oculus_id", QJsonValue::fromVariant(QVariant(oculusID))); payload["oculus_id"] = oculusID;
auto accountManager = DependencyManager::get<AccountManager>(); auto accountManager = DependencyManager::get<AccountManager>();
accountManager->sendRequest(LINK_OCULUS_PATH, AccountManagerAuth::Required, 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"; const QString CREATE_ACCOUNT_FROM_OCULUS_PATH = "api/v1/user/oculus/create";
QJsonObject payload; QJsonObject payload;
payload.insert("oculus_nonce", QJsonValue::fromVariant(QVariant(nonce))); payload["oculus_nonce"] = nonce;
payload.insert("oculus_id", QJsonValue::fromVariant(QVariant(oculusID))); payload["oculus_id"] = oculusID;
if (!email.isEmpty()) { if (!email.isEmpty()) {
payload.insert("email", QJsonValue::fromVariant(QVariant(email))); payload["email"] = email;
} }
if (!username.isEmpty()) { if (!username.isEmpty()) {
payload.insert("username", QJsonValue::fromVariant(QVariant(username))); payload["username"] = username;
} }
if (!password.isEmpty()) { if (!password.isEmpty()) {
payload.insert("password", QJsonValue::fromVariant(QVariant(password))); payload["password"] = password;
} }
auto accountManager = DependencyManager::get<AccountManager>(); auto accountManager = DependencyManager::get<AccountManager>();
@ -239,7 +239,7 @@ void LoginDialog::linkSteam() {
const QString LINK_STEAM_PATH = "api/v1/user/steam/link"; const QString LINK_STEAM_PATH = "api/v1/user/steam/link";
QJsonObject payload; QJsonObject payload;
payload.insert("steam_auth_ticket", QJsonValue::fromVariant(QVariant(ticket))); payload["steam_auth_ticket"] = QJsonValue::fromVariant(QVariant(ticket));
auto accountManager = DependencyManager::get<AccountManager>(); auto accountManager = DependencyManager::get<AccountManager>();
accountManager->sendRequest(LINK_STEAM_PATH, AccountManagerAuth::Required, 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"; const QString CREATE_ACCOUNT_FROM_STEAM_PATH = "api/v1/user/steam/create";
QJsonObject payload; QJsonObject payload;
payload.insert("steam_auth_ticket", QJsonValue::fromVariant(QVariant(ticket))); payload["steam_auth_ticket"] = QJsonValue::fromVariant(QVariant(ticket));
if (!username.isEmpty()) { if (!username.isEmpty()) {
payload.insert("username", QJsonValue::fromVariant(QVariant(username))); payload["username"] = username;
} }
auto accountManager = DependencyManager::get<AccountManager>(); auto accountManager = DependencyManager::get<AccountManager>();
@ -305,40 +305,35 @@ void LoginDialog::createFailed(QNetworkReply* reply) {
return; return;
} }
auto root = doc.object(); auto root = doc.object();
auto data = root.value("data").toObject(); auto data = root["data"].toObject();
auto error = data.value("error").toObject(); auto error = data["error"].toObject();
auto oculusError = data.value("oculus"); auto oculusError = data["oculus"];
auto user = error.value("username"); auto user = error["username"].toArray();
auto uid = error.value("uid"); auto uid = error["uid"].toArray();
auto email = error.value("email"); auto email = error["email"].toArray();
auto password = error.value("password"); auto password = error["password"].toArray();
QString reply; QString reply;
QString emailError; if (!uid.isEmpty()) {
if (!uid.isNull() && !uid.isUndefined()) { if (uid[0].isString()) {
QJsonArray arr = uid.toArray(); emit handleCreateFailed("Oculus ID " + uid[0].toString() + ".");
if (!arr.isEmpty()) {
emit handleCreateFailed("Oculus ID " + arr.at(0).toString() + ".");
return; return;
} }
} }
if (!user.isNull() && !user.isUndefined()) { if (!user.isEmpty()) {
QJsonArray arr = user.toArray(); if (user[0].isString()) {
if (!arr.isEmpty()) { reply = "Username " + user[0].toString() + ".";
reply = "Username " + arr.at(0).toString() + ".";
} }
} }
if (!email.isNull() && !email.isUndefined()) { if (!email.isEmpty()) {
QJsonArray arr = email.toArray(); if (email[0].isString()) {
if (!arr.isEmpty()) {
reply.append((!reply.isEmpty()) ? "\n" : ""); reply.append((!reply.isEmpty()) ? "\n" : "");
reply.append("Email " + arr.at(0).toString() + "."); reply.append("Email " + email[0].toString() + ".");
} }
} }
if (!password.isNull() && !password.isUndefined()) { if (!password.isEmpty()) {
QJsonArray arr = password.toArray(); if (password[0].isString()) {
if (!arr.isEmpty()) {
reply.append((!reply.isEmpty()) ? "\n" : ""); reply.append((!reply.isEmpty()) ? "\n" : "");
reply.append("Password " + arr.at(0).toString() + "."); reply.append("Password " + password[0].toString() + ".");
} }
} }
if (!oculusError.isNull() && !oculusError.isUndefined()) { if (!oculusError.isNull() && !oculusError.isUndefined()) {

View file

@ -17,8 +17,8 @@ class OculusPlatformPlugin {
public: public:
virtual ~OculusPlatformPlugin() = default; virtual ~OculusPlatformPlugin() = default;
virtual const QString getName() const = 0; virtual QString getName() = 0;
virtual const QString getOculusUserID() const = 0; virtual QString getOculusUserID() = 0;
virtual bool isRunning() = 0; virtual bool isRunning() = 0;

View file

@ -14,7 +14,7 @@
#include "OculusHelpers.h" #include "OculusHelpers.h"
const char* OculusAPIPlugin::NAME { "Oculus Rift" }; QString OculusAPIPlugin::NAME { "Oculus Rift" };
OculusAPIPlugin::OculusAPIPlugin() { OculusAPIPlugin::OculusAPIPlugin() {
_session = hifi::ovr::acquireRenderSession(); _session = hifi::ovr::acquireRenderSession();
@ -40,9 +40,10 @@ void OculusAPIPlugin::handleOVREvents() {
#ifdef OCULUS_APP_ID #ifdef OCULUS_APP_ID
if (qApp->property(hifi::properties::OCULUS_STORE).toBool()) { if (qApp->property(hifi::properties::OCULUS_STORE).toBool()) {
// pop messages to see if we got a return for an entitlement check // 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)) { switch (ovr_Message_GetType(message)) {
case ovrMessage_Entitlement_GetIsViewerEntitled: { case ovrMessage_Entitlement_GetIsViewerEntitled: {
if (!ovr_Message_IsError(message)) { if (!ovr_Message_IsError(message)) {
@ -104,9 +105,6 @@ void OculusAPIPlugin::handleOVREvents() {
// free the message handle to cleanup and not leak // free the message handle to cleanup and not leak
ovr_FreeMessage(message); ovr_FreeMessage(message);
// pop the next message to check, if there is one
message = ovr_PopMessage();
} }
} }
#endif #endif

View file

@ -18,8 +18,8 @@ class OculusAPIPlugin : public OculusPlatformPlugin {
public: public:
OculusAPIPlugin(); OculusAPIPlugin();
virtual ~OculusAPIPlugin(); virtual ~OculusAPIPlugin();
const QString getName() const { return NAME; } QString getName() { return NAME; }
const QString getOculusUserID() const { return _user; }; QString getOculusUserID() { return _user; };
bool isRunning(); bool isRunning();
@ -28,7 +28,7 @@ public:
virtual void handleOVREvents(); virtual void handleOVREvents();
private: private:
static const char* NAME; static QString NAME;
NonceUserIDCallback _nonceUserIDCallback; NonceUserIDCallback _nonceUserIDCallback;
QString _nonce; QString _nonce;
bool _nonceChanged{ false }; bool _nonceChanged{ false };