Review fixes (non-working)

Trouble with:

> Script.getExternalPath(Script.ExternalPaths.Assets, "hi")

[UncaughtException evaluate] Error: cannot call getExternalPath(): argument 1 has unknown type `ExternalResource::Bucket' (register the type with qScriptRegisterMetaType()) in about:console:1 [Backtrace] <global>() at about:console:1
TypeError: cannot call getExternalPath(): argument 1 has unknown type `ExternalResource::Bucket' (register the type with qScriptRegisterMetaType())

interface/src/Application.cpp

@@ -5545,20 +5545,18 @@ void Application::loadSettings() {
 
     getMyAvatar()->loadData();
 
-
     auto bucketEnum = QMetaEnum::fromType<ExternalResource::Bucket>();
-    auto er = ExternalResource::getInstance();
+    auto externalResource = ExternalResource::getInstance();
 
-    for(int i=0;i<bucketEnum.keyCount();i++) {
-        const char *keyName = bucketEnum.key(i);
+    for (int i = 0; i < bucketEnum.keyCount(); i++) {
+        const char* keyName = bucketEnum.key(i);
         QString setting("ExternalResource/");
         setting += keyName;
         auto bucket = static_cast<ExternalResource::Bucket>(bucketEnum.keyToValue(keyName));
-        Setting::Handle<QString> url(setting, er->getBase(bucket));
-        er->setBase( bucket, url.get() );
+        Setting::Handle<QString> url(setting, externalResource->getBase(bucket));
+        externalResource->setBase(bucket, url.get());
     }
 
-
     _settingsLoaded = true;
 }
 
@@ -5577,16 +5575,16 @@ void Application::saveSettings() const {
     PluginManager::getInstance()->saveSettings();
 
     auto bucketEnum = QMetaEnum::fromType<ExternalResource::Bucket>();
-    auto er = ExternalResource::getInstance();
+    auto externalResource = ExternalResource::getInstance();
 
-    for(int i=0;i<bucketEnum.keyCount();i++) {
-        const char *keyName = bucketEnum.key(i);
+    for (int i = 0; i < bucketEnum.keyCount(); i++) {
+        const char* keyName = bucketEnum.key(i);
         QString setting("ExternalResource/");
         setting += keyName;
         auto bucket = static_cast<ExternalResource::Bucket>(bucketEnum.keyToValue(keyName));
-        Setting::Handle<QString> url(setting, er->getBase(bucket));
-        url.set(er->getBase(bucket));
-    }    
+        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) {
@@ -7568,8 +7566,8 @@ void Application::registerScriptEngineWithApplicationServices(const ScriptEngine
     scriptEngine->registerGlobalObject("ResourceRequestObserver", DependencyManager::get<ResourceRequestObserver>().data());
 
     
-    scriptEngine->registerGlobalObject("ExternalResource", ExternalResource::getInstance());
-    scriptEngine->registerEnum("ExternalResource", QMetaEnum::fromType<ExternalResource::Bucket>());
+    //scriptEngine->registerGlobalObject("ExternalResource", ExternalResource::getInstance());
+   // scriptEngine->registerEnum("Script.ExternalPaths", QMetaEnum::fromType<ExternalResource::Bucket>());
 
     registerInteractiveWindowMetaType(scriptEngine.data());
 

interface/src/scripting/ScreenshareScriptingInterface.cpp

@@ -129,7 +129,7 @@ static const uint8_t LOCAL_SCREENSHARE_WEB_ENTITY_FPS = 30;
 // The `z` value here is dynamic.
 static const glm::vec3 LOCAL_SCREENSHARE_WEB_ENTITY_LOCAL_POSITION(0.0128f, -0.0918f, 0.0f);
 static const glm::vec3 LOCAL_SCREENSHARE_WEB_ENTITY_DIMENSIONS(3.6790f, 2.0990f, 0.0100f);
-static const QString LOCAL_SCREENSHARE_WEB_ENTITY_URL = ExternalResource::getInstance()->getUrl(ExternalResource::Bucket::Content, 
+static const QString LOCAL_SCREENSHARE_WEB_ENTITY_URL = ExternalResource::getInstance()->getUrl(ExternalResource::Bucket::HF_Content,
     "Experiences/Releases/usefulUtilities/smartBoard/screenshareViewer/screenshareClient.html");
 static const QString LOCAL_SCREENSHARE_WEB_ENTITY_HOST_TYPE = "local";
 void ScreenshareScriptingInterface::startScreenshare(const QUuid& screenshareZoneID,

libraries/networking/src/ExternalResource.cpp

@@ -58,21 +58,22 @@ QString ExternalResource::getBase(Bucket bucket) {
     return _bucketBases.value(bucket).toString();
 };
 
-void ExternalResource::setBase(Bucket bucket, const QString& url) {
+bool ExternalResource::setBase(Bucket bucket, const QString& url) {
     QUrl new_url(url);
 
     if (!new_url.isValid() || new_url.isRelative()) {
         qCCritical(external_resource) << "Attempted to set bucket " << bucket << " to invalid URL " << url;
-        return;
+        return false;
     }
 
     if (!_bucketBases.contains(bucket)) {
         qCritical(external_resource) << "Invalid bucket " << bucket;
-        return;
+        return false;
     }
 
     qCDebug(external_resource) << "Setting base URL for " << bucket << " to " << new_url;
 
     std::lock_guard<std::mutex> guard(_bucketMutex);
     _bucketBases[bucket] = new_url;
+    return true;
 }

libraries/networking/src/ExternalResource.h

@@ -30,7 +30,8 @@
  * This class makes it possible to deal with this in a more flexible manner: rather than hardcoding 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
+ * similar to be used. The term should just be taken as referring to the name of a data set.
  */
 class ExternalResource : public QObject {
     Q_OBJECT
@@ -50,13 +51,13 @@ public:
     enum class Bucket
     {
         /** Assets that used to be in the hifi-public S3 bucket */
-        Public,
+        HF_Public,
 
         /** Assets that used to be in the hifi-content S3 bucket */
-        Content,
+        HF_Content,
 
         /** Assets that used to be in the mpassets S3 bucket (hifi marketplace) */
-        MPAssets,
+        HF_Marketplace,
 
         /** Vircadia assets */
         Assets
@@ -124,9 +125,24 @@ public:
         return ExternalResource::getQUrl(bucket, QUrl(relative_path)).toString();
     };
 
-    Q_INVOKABLE QString getBase(Bucket bucket);
+    /**
+     * Returns the base path for a bucket
+     * 
+     * @param bucket Bucket whose path to return
+     */
+    QString getBase(Bucket bucket);
 
-    Q_INVOKABLE void setBase(Bucket bucket, const QString& url);
+    /**
+     * Sets the base path for a bucket
+     *
+     * The \p url parameter will be validated, and the action will not be performed
+     * if the URL isn't a valid one, or if the bucket wasn't valid.
+     *
+     * @param bucket Bucket whose path to set
+     * @param url Base URL for the bucket.
+     * @returns Whether the base was set.
+     */
+    bool setBase(Bucket bucket, const QString& url);
 
 private:
     ExternalResource(QObject* parent = nullptr);
@@ -134,10 +150,10 @@ private:
     std::mutex _bucketMutex;
 
     QMap<Bucket, QUrl> _bucketBases{
-        { Bucket::Public, QUrl("https://public.vircadia.com") },
-        { Bucket::Content, QUrl("https://content.vircadia.com") },
+        { Bucket::HF_Public, QUrl("https://public.vircadia.com") },
+        { Bucket::HF_Content, QUrl("https://content.vircadia.com") },
         { Bucket::Assets, QUrl("https://assets.vircadia.com") },
-        { Bucket::MPAssets, QUrl("https://mpassets.vircadia.com") },
+        { Bucket::HF_Marketplace, QUrl("https://mpassets.vircadia.com") },
     };
 };
 

libraries/script-engine/src/ScriptEngine.cpp

@@ -769,6 +769,7 @@ void ScriptEngine::init() {
         require.setProperty("resolve", resolve, READONLY_PROP_FLAGS);
         resetModuleCache();
     }
+    registerEnum("Script.ExternalPaths", QMetaEnum::fromType<ExternalResource::Bucket>());
 
     registerGlobalObject("Audio", DependencyManager::get<AudioScriptingInterface>().data());
 
@@ -2869,3 +2870,7 @@ void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QS
         }
     }
 }
+
+QString ScriptEngine::getExternalPath(ExternalResource::Bucket bucket, const QString &relative_path) {
+    return ExternalResource::getInstance()->getUrl(bucket, relative_path);
+}

libraries/script-engine/src/ScriptEngine.h

@@ -20,6 +20,7 @@
 #include <QtCore/QSet>
 #include <QtCore/QWaitCondition>
 #include <QtCore/QStringList>
+#include <QMap>
 #include <QMetaEnum>
 
 #include <QtScript/QScriptEngine>
@@ -38,6 +39,7 @@
 #include "AssetScriptingInterface.h"
 #include "AudioScriptingInterface.h"
 #include "BaseScriptEngine.h"
+#include "ExternalResource.h"
 #include "Quat.h"
 #include "Mat4.h"
 #include "ScriptCache.h"
@@ -682,6 +684,8 @@ public:
 
     void setScriptEngines(QSharedPointer<ScriptEngines>& scriptEngines) { _scriptEngines = scriptEngines; }
 
+    Q_INVOKABLE QString getExternalPath(ExternalResource::Bucket bucket, const QString &relative_path);
+
 public slots:
 
     /**jsdoc