#6145: PrimitiveSelectionTester is just using a AnySelector in its body

RadiantSelectionSystem is using the tester against the scene now.

include/iselectiontest.h

@@ -271,6 +271,12 @@ class Selector
      */
     virtual void addIntersection(const SelectionIntersection& intersection) = 0;
 
+    // Returns true if no selectable has been chosen
+    virtual bool empty() const = 0;
+
+    // Iterate over every selectable in the pool
+    virtual void foreachSelectable(const std::function<void(ISelectable*)>& functor) = 0;
+
     /// Add a selectable object and immediately commit it with a null intersection
     void addWithNullIntersection(ISelectable& selectable)
     {

libs/selection/EntitiesFirstSelector.h

@@ -89,14 +89,14 @@ class EntitiesFirstSelector :
         _currentSelectables.emplace(selectable, result);
     }
 
-    bool empty() const
+    bool empty() const override
     {
         return _entityPool.empty() && _primitivePool.empty();
     }
 
     // Visit each selectable, entities first, then primitives
-    void foreachSelectable(const std::function<void(ISelectable*)>& functor)
-    {
+    void foreachSelectable(const std::function<void(ISelectable*)>& functor) override
+    { 
         for (const auto& [_, selectable] : _entityPool)
         {
             functor(selectable);

libs/selection/OccludeSelector.h

@@ -20,17 +20,25 @@ class OccludeSelector :
 		_occluded = false;
 	}
 
-	void pushSelectable(ISelectable& selectable) {}
-	void popSelectable() {}
+	void pushSelectable(ISelectable& selectable) override {}
+	void popSelectable() override {}
 
-	void addIntersection(const SelectionIntersection& intersection)
+	void addIntersection(const SelectionIntersection& intersection) override
 	{
 		if (intersection.isCloserThan(_bestIntersection))
 		{
 			_bestIntersection = intersection;
 			_occluded = true;
 		}
 	}
+
+    bool empty() const override
+    {
+        return true;
+    }
+
+    void foreachSelectable(const std::function<void(ISelectable*)>& functor) override
+    {}
 };
 
 } // namespace

libs/selection/SelectionPool.h

@@ -121,10 +121,18 @@ class SelectionPool :
 		return _pool.end();
 	}
 
-	bool empty()
+	bool empty() const override
 	{
 		return _pool.empty();
 	}
+
+    void foreachSelectable(const std::function<void(ISelectable*)>& functor) override
+	{
+        for (const auto& [_, selectable] : _pool)
+        {
+            functor(selectable);
+        }
+	}
 };
 
 }

libs/selection/SingleItemSelector.h

@@ -60,6 +60,19 @@ class SingleItemSelector :
     {
         return _selectable;
     }
+
+    bool empty() const override
+    {
+        return !hasValidSelectable();
+    }
+
+    void foreachSelectable(const std::function<void(ISelectable*)>& functor) override
+	{
+        if (!empty())
+        {
+            functor(_selectable);
+        }
+	}
 };
 
 }

radiantcore/selection/BestSelector.h

@@ -25,7 +25,8 @@ class BestSelector :
 	std::list<ISelectable*> _bestSelectables;
 
 public:
-	BestSelector()
+	BestSelector() :
+        _selectable(nullptr)
 	{}
 
 	void pushSelectable(ISelectable& selectable) override
@@ -60,6 +61,19 @@ class BestSelector :
 	{
 		return _bestSelectables;
 	}
+
+    bool empty() const override
+    {
+        return _bestSelectables.empty();
+    }
+
+    void foreachSelectable(const std::function<void(ISelectable*)>& functor) override
+	{
+	    for (auto selectable : _bestSelectables)
+	    {
+            functor(selectable);
+	    }
+	}
 };
 
 }

radiantcore/selection/RadiantSelectionSystem.cpp

@@ -128,7 +128,6 @@ void RadiantSelectionSystem::testSelectScene(SelectablesList& targetList, Select
 {
     // The (temporary) storage pool
     SelectionPool selector;
-    SelectionPool sel2;
 
     switch(mode)
     {
@@ -144,52 +143,16 @@ void RadiantSelectionSystem::testSelectScene(SelectablesList& targetList, Select
 
         case SelectionMode::Primitive:
         {
-#if 1
-            // We have an orthoview, here, sorting entities before primitives
+            // Sort entities before primitives if required, by referencing the correct selector
             EntitiesFirstSelector sortedPool;
 
-            AnySelector anyTester(sortedPool, test);
-            GlobalSceneGraph().foreachVisibleNodeInVolume(view, anyTester);
+            auto& targetPool = !view.fill() && higherEntitySelectionPriority() ? 
+                static_cast<Selector&>(sortedPool) : selector;
 
-            //auto tester = createSceneSelectionTester(mode);
-            //tester->testSelectScene(view, test, selector);
-
-            sortedPool.foreachSelectable([&](ISelectable* s) { targetList.push_back(s); });
-#else
-            // Do we have a camera view (filled rendering?)
-            if (view.fill() || !higherEntitySelectionPriority())
-            {
-                // Test for any visible elements (primitives, entities), but don't select child primitives
-                AnySelector anyTester(selector, test);
-                GlobalSceneGraph().foreachVisibleNodeInVolume(view, anyTester);
-            }
-            else
-            {
-                // First, obtain all the selectable entities
-                EntitySelector entityTester(selector, test);
-                GlobalSceneGraph().foreachVisibleNodeInVolume(view, entityTester);
-
-                // Now retrieve all the selectable primitives
-                PrimitiveSelector primitiveTester(sel2, test);
-                GlobalSceneGraph().foreachVisibleNodeInVolume(view, primitiveTester);
-            }
-
-            // Add the first selection crop to the target vector
-            std::for_each(selector.begin(), selector.end(), [&](const auto& p) { targetList.push_back(p.second); });
+            auto tester = createSceneSelectionTester(mode);
+            tester->testSelectScene(view, test, targetPool);
 
-            // Add the secondary crop to the vector (if it has any entries)
-            for (SelectionPool::const_iterator i = sel2.begin(); i != sel2.end(); ++i) {
-                // Check for duplicates
-                SelectablesList::iterator j;
-                for (j = targetList.begin(); j != targetList.end(); ++j) {
-                    if (*j == i->second) break;
-                }
-                // Insert if not yet in the list
-                if (j == targetList.end()) {
-                    targetList.push_back(i->second);
-                }
-            }
-#endif
+            targetPool.foreachSelectable([&](ISelectable* s) { targetList.push_back(s); });
         }
         break;
 
@@ -222,7 +185,7 @@ void RadiantSelectionSystem::testSelectScene(SelectablesList& targetList, Select
             std::for_each(selector.begin(), selector.end(), [&](const auto& p) { targetList.push_back(p.second); });
         }
         break;
-    } // switch
+    }
 }
 
 /* greebo: This is true if nothing is selected (either in component mode or in primitive mode)

radiantcore/selection/SceneSelectionTesters.cpp

@@ -1,66 +1,15 @@
 #include "SceneSelectionTesters.h"
 
 #include "iscenegraph.h"
-#include "registry/registry.h"
 #include "SelectionTestWalkers.h"
-#include "selection/SelectionPool.h"
 
 namespace selection
 {
 
 void PrimitiveSelectionTester::testSelectScene(const VolumeTest& view, SelectionTest& test, Selector& selector)
 {
-    // Cameras don't sort entities higher
-    if (view.fill() || !higherEntitySelectionPriority())
-    {
-        // Test for any visible elements (primitives, entities), but don't select child primitives
-        AnySelector anyTester(selector, test);
-        GlobalSceneGraph().foreachVisibleNodeInVolume(view, anyTester);
-        return;
-    }
-
-    // We have an orthoview, select entities first
-    SelectionPool entityPool;
-    SelectionPool primitivePool;
-
-    // First, obtain all the selectable entities
-    EntitySelector entityTester(entityPool, test);
-    GlobalSceneGraph().foreachVisibleNodeInVolume(view, entityTester);
-
-    // Now retrieve all the selectable primitives
-    PrimitiveSelector primitiveTester(primitivePool, test);
-    GlobalSceneGraph().foreachVisibleNodeInVolume(view, primitiveTester);
-
-    // Transfer the items of both pools to the target selector, entities first
-    for (const auto& [intersection, selectable] : entityPool)
-    {
-        selector.pushSelectable(*selectable);
-        selector.addIntersection(intersection);
-        selector.popSelectable();
-    }
-
-    for (const auto& [intersection, selectable] : primitivePool)
-    {
-        // Check for duplicates
-        SelectionPool::const_iterator existing;
-
-        for (existing = entityPool.begin(); existing != entityPool.end(); ++existing)
-        {
-            if (existing->second == selectable) break;
-        }
-
-        if (existing == entityPool.end())
-        {
-            selector.pushSelectable(*selectable);
-            selector.addIntersection(intersection);
-            selector.popSelectable();
-        }
-    }
-}
-
-bool PrimitiveSelectionTester::higherEntitySelectionPriority() const
-{
-    return registry::getValue<bool>(RKEY_HIGHER_ENTITY_PRIORITY);
+    AnySelector anyTester(selector, test);
+    GlobalSceneGraph().foreachVisibleNodeInVolume(view, anyTester);
 }
 
 void EntitySelectionTester::testSelectScene(const VolumeTest& view, SelectionTest& test, Selector& selector)

radiantcore/selection/SceneSelectionTesters.h

@@ -6,16 +6,13 @@ namespace selection
 {
 
 /**
- * Tests entities and primitives in the scene, entities sorted first
+ * Tests entities and worldspawn primitives in the scene
  */
 class PrimitiveSelectionTester :
     public ISceneSelectionTester
 {
 public:
     void testSelectScene(const VolumeTest& view, SelectionTest& test, Selector& selector) override;
-
-private:
-    bool higherEntitySelectionPriority() const;
 };
 
 /**