From cae4ed9ce063cb98ac9444aa9f97adc3237b4aca Mon Sep 17 00:00:00 2001
From: Alexander Bock <alexander.bock@me.com>
Date: Mon, 9 Apr 2018 10:20:36 -0400
Subject: [PATCH] Sort nodes in GlobeBrowsing UI by whether they have URLs
 associated with them or not (closes #580)

---
 modules/globebrowsing/globebrowsingmodule.cpp |  5 ++
 modules/globebrowsing/globebrowsingmodule.h   |  1 +
 .../imgui/src/guiglobebrowsingcomponent.cpp   | 59 ++++++++++++++++---
 3 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/modules/globebrowsing/globebrowsingmodule.cpp b/modules/globebrowsing/globebrowsingmodule.cpp
index c6900b915e..eeeec0f331 100644
--- a/modules/globebrowsing/globebrowsingmodule.cpp
+++ b/modules/globebrowsing/globebrowsingmodule.cpp
@@ -628,6 +628,11 @@ GlobeBrowsingModule::urlInfo(const std::string& globe) const
     return res;
 }
 
+bool GlobeBrowsingModule::hasUrlInfo(const std::string& globe) const {
+    return _urlList.find(globe) != _urlList.end();
+}
+
+
 #endif // GLOBEBROWSING_USE_GDAL
 
 } // namespace openspace
diff --git a/modules/globebrowsing/globebrowsingmodule.h b/modules/globebrowsing/globebrowsingmodule.h
index fcb3aacdf7..64da904daf 100644
--- a/modules/globebrowsing/globebrowsingmodule.h
+++ b/modules/globebrowsing/globebrowsingmodule.h
@@ -79,6 +79,7 @@ public:
     Capabilities capabilities(const std::string& name);
 
     std::vector<UrlInfo> urlInfo(const std::string& globe) const;
+    bool hasUrlInfo(const std::string& globe) const;
 
     void removeWMSServer(const std::string& name);
 
diff --git a/modules/imgui/src/guiglobebrowsingcomponent.cpp b/modules/imgui/src/guiglobebrowsingcomponent.cpp
index 988435433a..83e2a12f5d 100644
--- a/modules/imgui/src/guiglobebrowsingcomponent.cpp
+++ b/modules/imgui/src/guiglobebrowsingcomponent.cpp
@@ -79,9 +79,9 @@ void GuiGlobeBrowsingComponent::render() {
         std::remove_if(
             nodes.begin(),
             nodes.end(),
-            [](SceneGraphNode* n) {
-                return !(n->renderable() &&
-                         n->renderable()->identifier() == "RenderableGlobe");
+            [module](SceneGraphNode* n) {
+                Renderable* r = n->renderable();
+                return !r || r->identifier() != "RenderableGlobe";
             }
         ),
         nodes.end()
@@ -89,13 +89,40 @@ void GuiGlobeBrowsingComponent::render() {
     std::sort(
         nodes.begin(),
         nodes.end(),
-        [](SceneGraphNode* lhs, SceneGraphNode* rhs) {
+        [module](SceneGraphNode* lhs, SceneGraphNode* rhs) {
+            bool lhsHasUrl = module->hasUrlInfo(lhs->identifier());
+            bool rhsHasUrl = module->hasUrlInfo(rhs->identifier());
+
+            if (lhsHasUrl && !rhsHasUrl) {
+                return true;
+            }
+            if (!lhsHasUrl && rhsHasUrl) {
+                return false;
+            }
+
             return lhs->guiName() < rhs->guiName();
         }
     );
+
+
+    auto firstWithoutUrl = std::find_if(
+        nodes.begin(),
+        nodes.end(),
+        [module](SceneGraphNode* n) {
+            return !module->hasUrlInfo(n->identifier());
+        }
+    );
+    nodes.insert(firstWithoutUrl, nullptr);
+
     std::string nodeNames;
     for (SceneGraphNode* n : nodes) {
-        nodeNames += n->identifier() + '\0';
+        // Add separator between nodes with URL and nodes without
+        if (n) {
+            nodeNames += n->identifier() + '\0';
+        }
+        else {
+            nodeNames += std::string("===== =====") + '\0';
+        }
     }
 
     int iNode = -1;
@@ -116,7 +143,7 @@ void GuiGlobeBrowsingComponent::render() {
             nodes.cbegin(),
             nodes.cend(),
             [this](SceneGraphNode* lhs) {
-                return lhs->identifier() == _currentNode;
+                return lhs && (lhs->identifier() == _currentNode);
             }
         );
         iNode = static_cast<int>(std::distance(nodes.cbegin(), it));
@@ -141,6 +168,12 @@ void GuiGlobeBrowsingComponent::render() {
         // or if there are no nodes
         return;
     }
+
+    if (!nodes[iNode]) {
+        // The user selected the separator
+        return;
+    }
+
     _currentNode = nodes[iNode]->identifier();
 
     if (nodeChanged) {
@@ -225,7 +258,7 @@ void GuiGlobeBrowsingComponent::render() {
 
     }
 
-    if (iServer == -1) {
+    if (iServer < 0) {
         return;
     }
     _currentServer = urlInfo[iServer].name;
@@ -299,16 +332,26 @@ void GuiGlobeBrowsingComponent::render() {
         auto addFunc = [this, &l](const std::string& type) {
             std::string layerName = l.name;
             std::replace(layerName.begin(), layerName.end(), '.', '-');
+            layerName.erase(
+                std::remove(layerName.begin(), layerName.end(), ' '),
+                layerName.end()
+            );
             OsEng.scriptEngine().queueScript(
                 fmt::format(
                     "openspace.globebrowsing.addLayer(\
                         '{}', \
                         '{}', \
-                        {{ Name = '{}', FilePath = '{}', Enabled = true }}\
+                        {{ \
+                            Identifier = '{}',\
+                            Name = '{}',\
+                            FilePath = '{}',\
+                            Enabled = true\
+                        }}\
                     );",
                     _currentNode,
                     type,
                     layerName,
+                    l.name,
                     l.url
                 ),
                 scripting::ScriptEngine::RemoteScripting::Yes