From e941dbb4a9971ae830aa226f944d0a425cb5056a Mon Sep 17 00:00:00 2001
From: Seth Alves <seth.alves@gmail.com>
Date: Thu, 10 Jan 2019 15:33:34 -0800
Subject: [PATCH] review

---
 .../plugins/src/plugins/PluginManager.cpp     | 40 ++++++-------------
 libraries/plugins/src/plugins/PluginManager.h |  7 ++++
 2 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/libraries/plugins/src/plugins/PluginManager.cpp b/libraries/plugins/src/plugins/PluginManager.cpp
index 854ae62ea1..9f481d04f1 100644
--- a/libraries/plugins/src/plugins/PluginManager.cpp
+++ b/libraries/plugins/src/plugins/PluginManager.cpp
@@ -47,45 +47,27 @@ PluginManagerPointer PluginManager::getInstance() {
 QString getPluginNameFromMetaData(const QJsonObject& object) {
     static const char* METADATA_KEY = "MetaData";
     static const char* NAME_KEY = "name";
-
     if (!object.contains(METADATA_KEY) || !object[METADATA_KEY].isObject()) {
         return QString();
     }
-
-    auto metaDataObject = object[METADATA_KEY].toObject();
-
-    if (!metaDataObject.contains(NAME_KEY) || !metaDataObject[NAME_KEY].isString()) {
-        return QString();
-    }
-
-    return metaDataObject[NAME_KEY].toString();
+    return object[METADATA_KEY][NAME_KEY].toString("");
 }
 
 QString getPluginIIDFromMetaData(const QJsonObject& object) {
     static const char* IID_KEY = "IID";
-
     if (!object.contains(IID_KEY) || !object[IID_KEY].isString()) {
         return QString();
     }
-
     return object[IID_KEY].toString();
 }
 
 int getPluginInterfaceVersionFromMetaData(const QJsonObject& object) {
-    static const char* METADATA_KEY = "MetaData";
-    static const char* NAME_KEY = "version";
-
+    static const QString METADATA_KEY = "MetaData";
+    static const QString NAME_KEY = "version";
     if (!object.contains(METADATA_KEY) || !object[METADATA_KEY].isObject()) {
         return 0;
     }
-
-    auto metaDataObject = object[METADATA_KEY].toObject();
-
-    if (!metaDataObject.contains(NAME_KEY) || !metaDataObject[NAME_KEY].isDouble()) {
-        return 0;
-    }
-
-    return (int)(metaDataObject[NAME_KEY].toDouble());
+    return object[METADATA_KEY][NAME_KEY].toInt(0);
 }
 
 
@@ -135,16 +117,18 @@ const LoaderList& getLoadedPlugins() {
                 QSharedPointer<QPluginLoader> loader(new QPluginLoader(pluginPath + plugin));
 
                 if (isDisabled(loader->metaData())) {
-                    qWarning() << "Plugin" << qPrintable(plugin) << "is disabled";
+                    qCWarning(plugins) << "Plugin" << qPrintable(plugin) << "is disabled";
                     // Skip this one, it's disabled
                     continue;
                 }
-
                 if (getPluginInterfaceVersionFromMetaData(loader->metaData()) != HIFI_PLUGIN_INTERFACE_VERSION) {
-                    qCDebug(plugins) << "Plugin" << qPrintable(plugin) << "interface version doesn't match, not loading:"
-                                     << getPluginInterfaceVersionFromMetaData(loader->metaData())
-                                     << "doesn't match" << HIFI_PLUGIN_INTERFACE_VERSION;
-                } else if (loader->load()) {
+                    qCWarning(plugins) << "Plugin" << qPrintable(plugin) << "interface version doesn't match, not loading:"
+                                       << getPluginInterfaceVersionFromMetaData(loader->metaData())
+                                       << "doesn't match" << HIFI_PLUGIN_INTERFACE_VERSION;
+                    continue;
+                }
+
+                if (loader->load()) {
                     qCDebug(plugins) << "Plugin" << qPrintable(plugin) << "loaded successfully";
                     loadedPlugins.push_back(loader);
                 } else {
diff --git a/libraries/plugins/src/plugins/PluginManager.h b/libraries/plugins/src/plugins/PluginManager.h
index a3305d8ee7..2a002577a4 100644
--- a/libraries/plugins/src/plugins/PluginManager.h
+++ b/libraries/plugins/src/plugins/PluginManager.h
@@ -62,4 +62,11 @@ private:
     InputPluginList _inputPlugins;
 };
 
+// TODO: we should define this value in CMake, and then use CMake
+// templating to generate the individual plugin.json files, so that we
+// don't have to update every plugin.json file whenever we update this
+// value.  The value should match "version" in
+//   plugins/*/src/plugin.json
+//   plugins/oculus/src/oculus.json
+//   etc
 static const int HIFI_PLUGIN_INTERFACE_VERSION = 1;