From c2ceeb3d7696a4d982ecf3b7327da8b56114b691 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Fri, 4 Jan 2019 09:41:31 -0800 Subject: [PATCH] Apply AvatarPackager code review cleanup --- .../avatarPackager/AvatarUploadStatusItem.qml | 17 +++++++++++++---- .../qml/hifi/avatarPackager/ClickableArea.qml | 2 +- .../qml/hifi/avatarPackager/RalewayButton.qml | 2 +- interface/src/avatar/AvatarProject.cpp | 8 ++++---- interface/src/avatar/AvatarProject.h | 2 +- .../src/avatar/MarketplaceItemUploader.cpp | 11 ++++++++--- interface/src/avatar/MarketplaceItemUploader.h | 17 +++++++---------- libraries/fbx/src/FST.cpp | 6 ++++-- libraries/fbx/src/FST.h | 4 ++-- libraries/networking/src/AccountManager.h | 7 +++---- 10 files changed, 44 insertions(+), 32 deletions(-) diff --git a/interface/resources/qml/hifi/avatarPackager/AvatarUploadStatusItem.qml b/interface/resources/qml/hifi/avatarPackager/AvatarUploadStatusItem.qml index 4749e912c6..1e48264b3a 100644 --- a/interface/resources/qml/hifi/avatarPackager/AvatarUploadStatusItem.qml +++ b/interface/resources/qml/hifi/avatarPackager/AvatarUploadStatusItem.qml @@ -14,25 +14,34 @@ Item { property int uploaderState; property var uploader; +/* state: root.uploader === undefined ? "" : (root.uploader.state > uploaderState ? "success" : (root.uploader.error !== 0 ? "fail" : (root.uploader.state === uploaderState ? "running" : ""))) + */ states: [ State { - name: "running" + name: "" + when: root.uploader === null + }, + State { + name: "success" + when: root.uploader.state > uploaderState PropertyChanges { target: stepText; color: "white" } - PropertyChanges { target: runningImage; visible: true; playing: true } + PropertyChanges { target: successGlyph; visible: true } }, State { name: "fail" + when: root.uploader.error !== 0 PropertyChanges { target: stepText; color: "#EA4C5F" } PropertyChanges { target: failGlyph; visible: true } }, State { - name: "success" + name: "running" + when: root.uploader.state === uploaderState PropertyChanges { target: stepText; color: "white" } - PropertyChanges { target: successGlyph; visible: true } + PropertyChanges { target: runningImage; visible: true; playing: true } } ] diff --git a/interface/resources/qml/hifi/avatarPackager/ClickableArea.qml b/interface/resources/qml/hifi/avatarPackager/ClickableArea.qml index ebe38364ec..0f7b201f72 100644 --- a/interface/resources/qml/hifi/avatarPackager/ClickableArea.qml +++ b/interface/resources/qml/hifi/avatarPackager/ClickableArea.qml @@ -60,4 +60,4 @@ Item { } ] } -} \ No newline at end of file +} diff --git a/interface/resources/qml/hifi/avatarPackager/RalewayButton.qml b/interface/resources/qml/hifi/avatarPackager/RalewayButton.qml index edbc31b24f..18cce8138f 100644 --- a/interface/resources/qml/hifi/avatarPackager/RalewayButton.qml +++ b/interface/resources/qml/hifi/avatarPackager/RalewayButton.qml @@ -25,4 +25,4 @@ RalewaySemiBold { onClicked: root.clicked() } -} \ No newline at end of file +} diff --git a/interface/src/avatar/AvatarProject.cpp b/interface/src/avatar/AvatarProject.cpp index e049dc0ba3..34da80587b 100644 --- a/interface/src/avatar/AvatarProject.cpp +++ b/interface/src/avatar/AvatarProject.cpp @@ -162,7 +162,7 @@ AvatarProject* AvatarProject::createAvatarProject(const QString& projectsFolder, return new AvatarProject(fst); } -QStringList AvatarProject::getScriptPaths(const QDir& scriptsDir) { +QStringList AvatarProject::getScriptPaths(const QDir& scriptsDir) const { QStringList result{}; constexpr auto flags = QDir::Files | QDir::NoSymLinks | QDir::NoDotAndDotDot | QDir::Hidden; if (!scriptsDir.exists()) { @@ -244,6 +244,8 @@ MarketplaceItemUploader* AvatarProject::upload(bool updateExisting) { } void AvatarProject::openInInventory() { + constexpr int TIME_TO_WAIT_FOR_INVENTORY_TO_OPEN_MS { 1000 }; + auto tablet = dynamic_cast( DependencyManager::get()->getTablet("com.highfidelity.interface.tablet.system")); tablet->loadQMLSource("hifi/commerce/wallet/Wallet.qml"); @@ -251,9 +253,7 @@ void AvatarProject::openInInventory() { auto name = getProjectName(); // I'm not a fan of this, but it's the only current option. - QTimer::singleShot(1000, [name]() { - auto tablet = dynamic_cast( - DependencyManager::get()->getTablet("com.highfidelity.interface.tablet.system")); + QTimer::singleShot(TIME_TO_WAIT_FOR_INVENTORY_TO_OPEN_MS, [name, tablet]() { tablet->sendToQml(QVariantMap({ { "method", "updatePurchases" }, { "filterText", name } })); }); } diff --git a/interface/src/avatar/AvatarProject.h b/interface/src/avatar/AvatarProject.h index c422c14d62..2a655409e1 100644 --- a/interface/src/avatar/AvatarProject.h +++ b/interface/src/avatar/AvatarProject.h @@ -103,7 +103,7 @@ private: void refreshProjectFiles(); void appendDirectory(const QString& prefix, const QDir& dir); - QStringList getScriptPaths(const QDir& scriptsDir); + QStringList getScriptPaths(const QDir& scriptsDir) const; FST* _fst; diff --git a/interface/src/avatar/MarketplaceItemUploader.cpp b/interface/src/avatar/MarketplaceItemUploader.cpp index 8b97358ba4..543617fc56 100644 --- a/interface/src/avatar/MarketplaceItemUploader.cpp +++ b/interface/src/avatar/MarketplaceItemUploader.cpp @@ -36,7 +36,10 @@ MarketplaceItemUploader::MarketplaceItemUploader(QString title, QUuid marketplaceID, QList filePaths) : _title(title), - _description(description), _rootFilename(rootFilename), _marketplaceID(marketplaceID), _filePaths(filePaths) { + _description(description), + _rootFilename(rootFilename), + _marketplaceID(marketplaceID), + _filePaths(filePaths) { } void MarketplaceItemUploader::setState(State newState) { @@ -299,11 +302,13 @@ void MarketplaceItemUploader::doWaitForInventory() { if (success) { setState(State::Complete); } else { + constexpr int MAX_INVENTORY_REQUESTS { 8 }; + constexpr int TIME_BETWEEN_INVENTORY_REQUESTS_MS { 5000 }; qDebug() << "Failed to find item in inventory"; - if (_numRequestsForInventory > 8) { + if (_numRequestsForInventory > MAX_INVENTORY_REQUESTS) { setError(Error::Unknown); } else { - QTimer::singleShot(5000, [this]() { doWaitForInventory(); }); + QTimer::singleShot(TIME_BETWEEN_INVENTORY_REQUESTS_MS, [this]() { doWaitForInventory(); }); } } }); diff --git a/interface/src/avatar/MarketplaceItemUploader.h b/interface/src/avatar/MarketplaceItemUploader.h index 4fb1713b7d..998413da88 100644 --- a/interface/src/avatar/MarketplaceItemUploader.h +++ b/interface/src/avatar/MarketplaceItemUploader.h @@ -30,21 +30,19 @@ class MarketplaceItemUploader : public QObject { Q_PROPERTY(Error error READ getError NOTIFY errorChanged) Q_PROPERTY(QString responseData READ getResponseData) public: - enum class Error - { + enum class Error { None, - Unknown + Unknown, }; Q_ENUM(Error); - enum class State - { + enum class State { Idle, GettingCategories, UploadingAvatar, WaitingForUploadResponse, WaitingForInventory, - Complete + Complete, }; Q_ENUM(State); @@ -63,7 +61,6 @@ public: State getState() const { return _state; } bool getComplete() const { return _state == State::Complete; } - QUuid getMarketplaceID() const { return _marketplaceID; } Error getError() const { return _error; } @@ -86,8 +83,8 @@ private: QNetworkReply* _reply; - State _state{ State::Idle }; - Error _error{ Error::None }; + State _state { State::Idle }; + Error _error { Error::None }; QString _title; QString _description; @@ -98,7 +95,7 @@ private: QString _responseData; - int _numRequestsForInventory{ 0 }; + int _numRequestsForInventory { 0 }; QString _rootFilePath; QList _filePaths; diff --git a/libraries/fbx/src/FST.cpp b/libraries/fbx/src/FST.cpp index 9510ca5962..7828037c74 100644 --- a/libraries/fbx/src/FST.cpp +++ b/libraries/fbx/src/FST.cpp @@ -15,6 +15,8 @@ #include #include +constexpr float DEFAULT_SCALE { 1.0f }; + FST::FST(QString fstPath, QVariantHash data) : _fstPath(std::move(fstPath)) { auto setValueFromFSTData = [&data] (const QString& propertyID, auto &targetProperty) mutable { @@ -55,7 +57,7 @@ FST* FST::createFSTFromModel(const QString& fstPath, const QString& modelFilePat mapping.insert(TEXDIR_FIELD, "textures"); // mixamo/autodesk defaults - mapping.insert(SCALE_FIELD, 1.0); + mapping.insert(SCALE_FIELD, DEFAULT_SCALE); QVariantHash joints = mapping.value(JOINT_FIELD).toHash(); joints.insert("jointEyeLeft", hfmModel.jointIndices.contains("jointEyeLeft") ? "jointEyeLeft" : (hfmModel.jointIndices.contains("EyeLeft") ? "EyeLeft" : "LeftEye")); @@ -161,7 +163,7 @@ void FST::setModelPath(const QString& modelPath) { emit modelPathChanged(modelPath); } -QVariantHash FST::getMapping() { +QVariantHash FST::getMapping() const { QVariantHash mapping; mapping.unite(_other); mapping.insert(NAME_FIELD, _name); diff --git a/libraries/fbx/src/FST.h b/libraries/fbx/src/FST.h index 6104130512..0f4c1ecd3a 100644 --- a/libraries/fbx/src/FST.h +++ b/libraries/fbx/src/FST.h @@ -45,9 +45,9 @@ public: QStringList getScriptPaths() const { return _scriptPaths; } void setScriptPaths(QStringList scriptPaths) { _scriptPaths = scriptPaths; } - QString getPath() { return _fstPath; } + QString getPath() const { return _fstPath; } - QVariantHash getMapping(); + QVariantHash getMapping() const; bool write(); diff --git a/libraries/networking/src/AccountManager.h b/libraries/networking/src/AccountManager.h index 77f20472fa..b8c3bf0cc4 100644 --- a/libraries/networking/src/AccountManager.h +++ b/libraries/networking/src/AccountManager.h @@ -40,11 +40,10 @@ public: }; namespace AccountManagerAuth { -enum Type -{ +enum Type { None, Required, - Optional + Optional, }; } @@ -157,7 +156,7 @@ private: bool _isWaitingForTokenRefresh{ false }; bool _isAgent{ false }; - bool _isWaitingForKeypairResponse{ false }; + bool _isWaitingForKeypairResponse { false }; QByteArray _pendingPrivateKey; QUuid _sessionID{ QUuid::createUuid() };