Merge pull request #751 from ctrlaltdavid/fix/external-resource

Fix/external resource

interface/src/Application.cpp

@@ -3295,7 +3295,6 @@ void Application::initializeUi() {
     qmlRegisterType<ResourceImageItem>("Hifi", 1, 0, "ResourceImageItem");
     qmlRegisterType<Preference>("Hifi", 1, 0, "Preference");
     qmlRegisterType<WebBrowserSuggestionsEngine>("HifiWeb", 1, 0, "WebBrowserSuggestionsEngine");
- //   qmlRegisterType<ExternalResource>("ExternalResource", 1, 0, "ExternalResource");
 
     {
         auto tabletScriptingInterface = DependencyManager::get<TabletScriptingInterface>();
@@ -5574,6 +5573,9 @@ void Application::saveSettings() const {
     getMyAvatar()->saveData();
     PluginManager::getInstance()->saveSettings();
 
+    // Don't save external resource paths until such time as there's UI to select or set alternatives. Otherwise new default
+    // values won't be used unless Interface.json entries are manually remove or Interface.json is deleted.
+    /*
     auto bucketEnum = QMetaEnum::fromType<ExternalResource::Bucket>();
     auto externalResource = ExternalResource::getInstance();
 
@@ -5585,6 +5587,7 @@ void Application::saveSettings() const {
         Setting::Handle<QString> url(setting, externalResource->getBase(bucket));
         url.set(externalResource->getBase(bucket));
     }
+    */
 }
 
 bool Application::importEntities(const QString& urlOrFilename, const bool isObservable, const qint64 callerId) {
@@ -7565,10 +7568,6 @@ void Application::registerScriptEngineWithApplicationServices(const ScriptEngine
     scriptEngine->registerGlobalObject("HifiAbout", AboutUtil::getInstance());  // Deprecated.
     scriptEngine->registerGlobalObject("ResourceRequestObserver", DependencyManager::get<ResourceRequestObserver>().data());
 
-    
-    //scriptEngine->registerGlobalObject("ExternalResource", ExternalResource::getInstance());
-   // scriptEngine->registerEnum("Script.ExternalPaths", QMetaEnum::fromType<ExternalResource::Bucket>());
-
     registerInteractiveWindowMetaType(scriptEngine.data());
 
     auto pickScriptingInterface = DependencyManager::get<PickScriptingInterface>();

interface/src/scripting/TestScriptingInterface.cpp

@@ -68,7 +68,7 @@ bool TestScriptingInterface::loadTestScene(QString scene) {
     }
 
     static const QString TEST_ROOT = "https://raw.githubusercontent.com/hifi-archive/hifi_tests/master/";
-    static const QString TEST_BINARY_ROOT = NetworkingConstants::HF_CONTENT_CDN_URL + "/test_scene_data/";
+    static const QString TEST_BINARY_ROOT = NetworkingConstants::HF_CONTENT_CDN_URL + "test_scene_data/";
     static const QString TEST_SCRIPTS_ROOT = TEST_ROOT + "scripts/";
     static const QString TEST_SCENES_ROOT = TEST_ROOT + "scenes/";
     

libraries/networking/src/ExternalResource.cpp

@@ -4,7 +4,7 @@
 //  Created by Dale Glass on 6 Sep 2020
 //  Copyright 2020 Vircadia contributors.
 //
-//  Flexible management for external resources (eg, on S3)
+//  Flexible management for external resources (e.g., on S3).
 //
 //  Distributed under the Apache License, Version 2.0.
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
@@ -23,25 +23,24 @@ ExternalResource* ExternalResource::getInstance() {
     return &instance;
 }
 
-QUrl ExternalResource::getQUrl(Bucket bucket, const QUrl& relative_path) {
+QUrl ExternalResource::getQUrl(Bucket bucket, const QUrl& path) {
-    qCDebug(external_resource) << "Requested URL for bucket " << bucket << ", path " << relative_path;
+    qCDebug(external_resource) << "Requested URL for bucket " << bucket << ", path " << path;
 
     if (!_bucketBases.contains(bucket)) {
-        qCCritical(external_resource) << "External resource " << relative_path << " was requested from unrecognized bucket "
+        qCCritical(external_resource) << "External resource " << path << " was requested from unrecognized bucket " << bucket;
-                                      << bucket;
         return QUrl();
     }
 
-    if (!relative_path.isValid()) {
+    if (!path.isValid()) {
-        qCCritical(external_resource) << "External resource " << relative_path << " was requested from bucket " << bucket
+        qCCritical(external_resource) << "External resource " << path << " was requested from bucket " << bucket
-                                      << " with an invalid path. Error: " << relative_path.errorString();
+                                      << " with an invalid path. Error: " << path.errorString();
         return QUrl();
     }
 
-    if (!relative_path.isRelative()) {
+    if (!path.isRelative()) {
-        qCWarning(external_resource) << "External resource " << relative_path << " was requested from bucket " << bucket
+        qCWarning(external_resource) << "External resource " << path << " was requested from bucket " << bucket
                                      << " without using a relative path, returning as-is.";
-        return relative_path;
+        return path;
     }
 
     QUrl base;
@@ -50,13 +49,13 @@ QUrl ExternalResource::getQUrl(Bucket bucket, const QUrl& relative_path) {
         base = _bucketBases[bucket];
     }
 
-    QUrl merged = base.resolved(relative_path).adjusted(QUrl::NormalizePathSegments);
+    QUrl merged = base.resolved(path).adjusted(QUrl::NormalizePathSegments);
 
     if ( merged.isValid() ) {
         qCDebug(external_resource) << "External resource resolved to " << merged;
     } else {
-        qCCritical(external_resource) << "External resource resolved to invalid URL " << merged << "; Error " << merged.errorString()
+        qCCritical(external_resource) << "External resource resolved to invalid URL " << merged << "; Error " 
-                                      << "; base = " << base << "; relative_path = " << relative_path;
+                                      << merged.errorString() << "; base = " << base << "; path = " << path;
     }
 
     return merged;
@@ -68,9 +67,9 @@ QString ExternalResource::getBase(Bucket bucket) {
 };
 
 bool ExternalResource::setBase(Bucket bucket, const QString& url) {
-    QUrl new_url(url);
+    QUrl newURL(url);
 
-    if (!new_url.isValid() || new_url.isRelative()) {
+    if (!newURL.isValid() || newURL.isRelative()) {
         qCCritical(external_resource) << "Attempted to set bucket " << bucket << " to invalid URL " << url;
         return false;
     }
@@ -80,9 +79,9 @@ bool ExternalResource::setBase(Bucket bucket, const QString& url) {
         return false;
     }
 
-    qCDebug(external_resource) << "Setting base URL for " << bucket << " to " << new_url;
+    qCDebug(external_resource) << "Setting base URL for " << bucket << " to " << newURL;
 
     std::lock_guard<std::mutex> guard(_bucketMutex);
-    _bucketBases[bucket] = new_url;
+    _bucketBases[bucket] = newURL;
     return true;
 }

libraries/networking/src/ExternalResource.h

@@ -4,7 +4,7 @@
 //  Created by Dale Glass on 6 Sep 2020
 //  Copyright 2020 Vircadia contributors.
 //
-//  Flexible management for external resources (eg, on S3)
+//  Flexible management for external resources (e.g., on S3).
 //
 //  Distributed under the Apache License, Version 2.0.
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
@@ -25,11 +25,11 @@
  *
  * With the death of the original High Fidelity and the transition of the project to a community-managed
  * one, it became necessary to deal with that the various assets that used to be located under the
- * highfidelity.com domain will disappear, and there won't be a fixed place for them afterwards. Data
+ * highfidelity.com domain, and there won't be a fixed place for them afterwards. Data
  * hosted by community members may not remain forever, reorganization may be necessary, people may want
- * to run mirrors, and some might want to run without external internet access at all.
+ * to run mirrors, and some might want to run without external Internet access at all.
  *
- * This class makes it possible to deal with this in a more flexible manner: rather than hardcoding URLs
+ * This class makes it possible to deal with this in a more flexible manner: rather than hard-coding URLs
  * all over the codebase, now it's possible to easily change where all those things are downloaded from.
  *
  * The term 'bucket' refers to the buckets used on Amazon S3, but there's no requirement for S3 or anything
@@ -42,25 +42,42 @@ public:
     static ExternalResource* getInstance();
     ~ExternalResource(){};
 
-    /**
+    /**jsdoc
-     * Bucket from which to retrieve the resource
+     * <p>An external resource bucket.</p>
-     *
+     * <p>The original High Fidelity used "Public", "Content", and "MPAssets" Amazon S3 buckets. The intention is that the
-     * The original High Fidelity used the Public, Content and MPAssets buckets. The intention is that the
+     * community-run versions of these will keep the original data and structure, and any new additions will be made to 
-     * community-run versions of these will keep the original data and structure, and any new additions
+     * Vircadia's "Assets" bucket. This should ease the transition from High Fidelity and ensure a clean separation.</p>
-     * will be done to the Assets bucket instead. This should ease the transition and ensure a clean
+     * @typedef {object} Script.ResourceBuckets
-     * separation.
+     * @property {Script.ResourceBucket} Assets - Vircadia assets.
+     * @property {Script.ResourceBucket} HF_Public - Assets that used to be in High Fidelity's <code>hifi-public</code> Amazon 
+     *     S3 bucket.
+     * @property {Script.ResourceBucket} HF_Content - Assets that used to be in High Fidelity's <code>hifi-content</code> Amazon
+     *     S3 bucket.
+     * @property {Script.ResourceBucket} HF_Marketplace - Assets that used to be in the High Fidelity's <code>mpassets</code>
+     *     Amazon S3 bucket. (High Fidelity marketplace.)
+     */
+    /**jsdoc
+     * <p>An external resource bucket.</p>
+     * <table>
+     *   <thead>
+     *     <tr><th>Value</th><th>Name</th><th>Description</th>
+     *   </thead>
+     *   <tbody>
+     *     <tr><td><code>0</code></td><td>HF_Public</td><td>Assets that used to be in High Fidelity's <code>hifi-public</code>
+     *       Amazon S3 bucket.</td></tr>
+     *     <tr><td><code>1</code></td><td>HF_Content</td><td>Assets that used to be in High Fidelity's <code>hifi-content</code>
+     *       Amazon S3 bucket.</td></tr>
+     *     <tr><td><code>2</code></td><td>HF_Marketplace</td><td>Assets that used to be in the High Fidelity's 
+     *       <code>mpassets</code> Amazon S3 bucket. (High Fidelity marketplace.)</td></tr>
+     *     <tr><td><code>3</code></td><td>Assets</td><td>Vircadia assets.</td></tr>
+     *   </tbody>
+     * </table>
+     * @typedef {number} Script.ResourceBucket
      */
     enum class Bucket {
-        /** Assets that used to be in the hifi-public S3 bucket */
         HF_Public,
-
-        /** Assets that used to be in the hifi-content S3 bucket */
         HF_Content,
-
-        /** Assets that used to be in the mpassets S3 bucket (hifi marketplace) */
         HF_Marketplace,
-
-        /** Vircadia assets */
         Assets
     };
     Q_ENUM(Bucket)
@@ -77,52 +94,10 @@ public:
      * @param relative_path The path of the resource within the bucket
      * @returns The resulting URL as a QUrl
      */
-    QUrl getQUrl(Bucket bucket, const QUrl& relative_path);
+    QUrl getQUrl(Bucket bucket, const QUrl& path);
 
-    /**
+    QString getUrl(Bucket bucket, const QString& path) {
-     * Returns the location of a resource as a QUrl
+       return ExternalResource::getQUrl(bucket, QUrl(path)).toString();
-     *
-     * Returns the location of the resource \p relative_path in bucket \p bucket
-     *
-     * @note The resulting path will be sanitized by condensing multiple instances of '/' to one.
-     * This is done for easier usage with Amazon S3 and compatible systems.
-     *
-     * @param bucket The bucket in which the resource is found
-     * @param relative_path The path of the resource within the bucket
-     * @returns The resulting URL as a QUrl
-     */
-    QUrl getQUrl(Bucket bucket, QString path) { return getQUrl(bucket, QUrl(path)); }
-
-    /**
-     * Returns the location of a resource as a QString
-     *
-     * Returns the location of the resource \p relative_path in bucket \p bucket
-     *
-     * @note The resulting path will be sanitized by condensing multiple instances of '/' to one.
-     * This is done for easier usage with Amazon S3 and compatible systems.
-     *
-     * @param bucket The bucket in which the resource is found
-     * @param relative_path The path of the resource within the bucket
-     * @returns The resulting URL as a QString
-     */
-    QString getUrl(Bucket bucket, const QUrl& relative_path) {
-        return ExternalResource::getQUrl(bucket, relative_path).toString();
-    };
-
-    /**
-     * Returns the location of a resource as a QString
-     *
-     * Returns the location of the resource \p relative_path in bucket \p bucket
-     *
-     * @note The resulting path will be sanitized by condensing multiple instances of '/' to one.
-     * This is done for easier usage with Amazon S3 and compatible systems.
-     *
-     * @param bucket The bucket in which the resource is found
-     * @param relative_path The path of the resource within the bucket
-     * @returns The resulting URL as a QString
-     */
-    Q_INVOKABLE QString getUrl(Bucket bucket, const QString& relative_path) {
-        return ExternalResource::getQUrl(bucket, QUrl(relative_path)).toString();
     };
 
     /**

libraries/networking/src/NetworkingConstants.h

@@ -45,11 +45,11 @@ namespace NetworkingConstants {
     const QUrl MASTER_BUILDS_XML_URL("https://highfidelity.com/dev-builds.xml");
 
     // CDN URLs
-    const QString HF_CONTENT_CDN_URL = "https://cdn-1.vircadia.com/eu-c-1/vircadia-content";
+    const QString HF_CONTENT_CDN_URL = "https://cdn-1.vircadia.com/eu-c-1/vircadia-content/";
-    const QString HF_MPASSETS_CDN_URL = "https://cdn-1.vircadia.com/eu-c-1/vircadia-mpassets";
+    const QString HF_MPASSETS_CDN_URL = "https://cdn-1.vircadia.com/eu-c-1/vircadia-mpassets/";
-    const QString HF_PUBLIC_CDN_URL = "https://cdn-1.vircadia.com/eu-c-1/vircadia-public";
+    const QString HF_PUBLIC_CDN_URL = "https://cdn-1.vircadia.com/eu-c-1/vircadia-public/";
     const QString HF_MARKETPLACE_CDN_HOSTNAME = "mpassets.highfidelity.com";
-    const QString VIRCADIA_CONTENT_CDN_URL = "https://cdn-1.vircadia.com/us-e-1";
+    const QString VIRCADIA_CONTENT_CDN_URL = "https://cdn-1.vircadia.com/us-e-1/";
 
 #if USE_STABLE_GLOBAL_SERVICES
     const QString ICE_SERVER_DEFAULT_HOSTNAME = "ice.vircadia.com";

libraries/script-engine/src/ScriptEngine.cpp

@@ -2883,6 +2883,6 @@ void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QS
     }
 }
 
-QString ScriptEngine::getExternalPath(ExternalResource::Bucket bucket, const QString& relativePath) {
+QString ScriptEngine::getExternalPath(ExternalResource::Bucket bucket, const QString& path) {
-    return ExternalResource::getInstance()->getUrl(bucket, relativePath);
+    return ExternalResource::getInstance()->getUrl(bucket, path);
 }

libraries/script-engine/src/ScriptEngine.h

@@ -121,6 +121,7 @@ public:
  *       <li><code>"agent"</code>: An assignment client script.</li>
  *     </ul>
  *     <em>Read-only.</em>
+ * @property {Script.ResourceBuckets} ExternalPaths - External resource buckets.
  */
 class ScriptEngine : public BaseScriptEngine, public EntitiesScriptEngineProvider {
     Q_OBJECT
@@ -235,12 +236,12 @@ public:
 
     /**jsdoc
      * @function Script.registerEnum
-     * @param {string} enumName - Name.
+     * @param {string} name - Name.
-     * @param {object} newEnum - Enumeration to be added
+     * @param {object} enum - Enum.
-     * @warning This function must be called after a registerGlobalObject that creates the namespace this enum is located in,
-     * or the globalObject won't function. Eg, if you have a Foo object and a Foo.FooType enum, Foo must be registered first.
      * @deprecated This function is deprecated and will be removed.
      */
+    // WARNING: This function must be called after a registerGlobalObject that creates the namespace this enum is located in, or\
+    // the globalObject won't function. E.g., if you have a Foo object and a Foo.FooType enum, Foo must be registered first.
     /// registers a global enum
     Q_INVOKABLE void registerEnum(const QString& enumName, QMetaEnum newEnum);
 
@@ -684,7 +685,23 @@ public:
 
     void setScriptEngines(QSharedPointer<ScriptEngines>& scriptEngines) { _scriptEngines = scriptEngines; }
 
-    Q_INVOKABLE QString getExternalPath(ExternalResource::Bucket bucket, const QString& relativePath);
+    /**jsdoc
+     * Gets the URL for an asset in an external resource bucket. (The location where the bucket is hosted may change over time
+     * but this method will return the asset's current URL.)
+     * @function Script.getExternalPath
+     * @param {Script.ResourceBucket} bucket - The external resource bucket that the asset is in.
+     * @param {string} path - The path within the external resource bucket where the asset is located. 
+     *     <p>Normally, this should start with a path or filename to be appended to the bucket URL.
+     *     Alternatively, it can be a relative path starting with <code>./</code> or <code>../</code>, to navigate within the 
+     *     resource bucket's URL. Or it can be an absolute path starting with <code>/</code>, in which case the bucket's path
+     *     is discarded when calculating the asset's URL.</p>
+     * @Returns {string} The URL of an external asset.
+     * @example <caption>Report the URL of a default particle.</caption>
+     * print(Script.getExternalPath(Script.ExternalPaths.Assets, "Bazaar/Assets/Textures/Defaults/Interface/default_particle.png"));
+     * @example <caption>Report the root directory where the Vircadia assets are located.</caption>
+     * print(Script.getExternalPath(Script.ExternalPaths.Assets, "."));
+     */
+    Q_INVOKABLE QString getExternalPath(ExternalResource::Bucket bucket, const QString& path);
 
 public slots: