Selector_add() is replaced with non-virtual methods on Selector

These needlessly-separate functions are now replaced by two new Selector
methods: addWithNullIntersection() and addWithIntersection(). These are
non-virtual helper methods implemented entirely in terms of the pure
virtual interface.

include/iselectiontest.h

@@ -238,28 +238,56 @@ class SelectionTest
 };
 typedef std::shared_ptr<SelectionTest> SelectionTestPtr;
 
+/**
+ * @brief Abstract interface for an object which collects possibly-selected
+ * objects, keeping track of the best intersection which typically determines
+ * which candidate object should be selected when more than one is possible.
+ *
+ * This object exposes a stateful interface. A call to pushSelectable() sets the
+ * given ISelectable object as the "current" selectable. Subsequent calls to
+ * addIntersection() apply to this current selectable, replacing the current
+ * intersection if the newly-submitted one is better. The operation is completed
+ * when popSelectable() is called, and the current selectable along with its
+ * best intersection is added to the internal list(s).
+ *
+ * Despite the usage of "push" and "pop" terminology, there is only one current
+ * selectable, and no internal stack.
+ */
 class Selector
 {
 public:
-  virtual ~Selector() {}
-  virtual void pushSelectable(ISelectable& selectable) = 0;
-  virtual void popSelectable() = 0;
-  virtual void addIntersection(const SelectionIntersection& intersection) = 0;
-};
+    virtual ~Selector() {}
 
-inline void Selector_add(Selector& selector, ISelectable& selectable)
-{
-  selector.pushSelectable(selectable);
-  selector.addIntersection(SelectionIntersection(0, 0));
-  selector.popSelectable();
-}
+    /// Set the given object as the current selectable
+    virtual void pushSelectable(ISelectable& selectable) = 0;
 
-inline void Selector_add(Selector& selector, ISelectable& selectable, const SelectionIntersection& intersection)
-{
-  selector.pushSelectable(selectable);
-  selector.addIntersection(intersection);
-  selector.popSelectable();
-}
+    /// Commit the current selectable, storing it along with its best intersection
+    virtual void popSelectable() = 0;
+
+    /**
+     * @brief Add a candidate intersection for the current selectable.
+     *
+     * The candidate intersection is only stored if it is a better fit than the
+     * best intersection seen so far.
+     */
+    virtual void addIntersection(const SelectionIntersection& intersection) = 0;
+
+    /// Add a selectable object and immediately commit it with a null intersection
+    void addWithNullIntersection(ISelectable& selectable)
+    {
+        pushSelectable(selectable);
+        addIntersection(SelectionIntersection(0, 0));
+        popSelectable();
+    }
+
+    /// Add a selectable object and immediately commit it with the given intersection
+    void addWithIntersection(ISelectable& selectable, const SelectionIntersection& intersection)
+    {
+        pushSelectable(selectable);
+        addIntersection(intersection);
+        popSelectable();
+    }
+};
 
 class VolumeTest;
 class SelectionTestable

libs/dragplanes.h

@@ -85,47 +85,47 @@ class DragPlanes
         if (planes[0].normal().dot(corners[1]) > 0 && planes[0].normal().dot(corners[2]) > 0 &&
             planes[0].normal().dot(corners[5]) > 0 && planes[0].normal().dot(corners[6]) > 0)
         {
-            Selector_add(selector, _selectableRight);
+            selector.addWithNullIntersection(_selectableRight);
             selectedPlaneCallback(planes[0]);
             //rMessage() << "right\n";
         }
 
         if (planes[1].normal().dot(corners[0]) > 0 && planes[1].normal().dot(corners[3]) > 0 && 
             planes[1].normal().dot(corners[4]) > 0 && planes[1].normal().dot(corners[7]) > 0)
         {
-            Selector_add(selector, _selectableLeft);
+            selector.addWithNullIntersection(_selectableLeft);
             selectedPlaneCallback(planes[1]);
             //rMessage() << "left\n";
         }
 
         if (planes[2].normal().dot(corners[0]) > 0 && planes[2].normal().dot(corners[1]) > 0 && 
             planes[2].normal().dot(corners[4]) > 0 && planes[2].normal().dot(corners[5]) > 0)
         {
-            Selector_add(selector, _selectableFront);
+            selector.addWithNullIntersection(_selectableFront);
             selectedPlaneCallback(planes[2]);
             //rMessage() << "front\n";
         }
 
         if (planes[3].normal().dot(corners[2]) > 0 && planes[3].normal().dot(corners[3]) > 0 && 
             planes[3].normal().dot(corners[6]) > 0 && planes[3].normal().dot(corners[7]) > 0)
         {
-            Selector_add(selector, _selectableBack);
+            selector.addWithNullIntersection(_selectableBack);
             selectedPlaneCallback(planes[3]);
             //rMessage() << "back\n";
         }
 
         if (planes[4].normal().dot(corners[0]) > 0 && planes[4].normal().dot(corners[1]) > 0 && 
             planes[4].normal().dot(corners[2]) > 0 && planes[4].normal().dot(corners[3]) > 0)
         {
-            Selector_add(selector, _selectableTop);
+            selector.addWithNullIntersection(_selectableTop);
             selectedPlaneCallback(planes[4]);
             //rMessage() << "top\n";
         }
 
         if (planes[5].normal().dot(corners[4]) > 0 && planes[5].normal().dot(corners[5]) > 0 && 
             planes[5].normal().dot(corners[6]) > 0 && planes[5].normal().dot(corners[7]) > 0)
         {
-            Selector_add(selector, _selectableBottom);
+            selector.addWithNullIntersection(_selectableBottom);
             selectedPlaneCallback(planes[5]);
             //rMessage() << "bottom\n";
         }
@@ -140,32 +140,32 @@ class DragPlanes
 
         if (selectedPlanes.contains(-planes[0]))
         {
-            Selector_add(selector, _selectableRight);
+            selector.addWithNullIntersection(_selectableRight);
         }
 
         if (selectedPlanes.contains(-planes[1]))
         {
-            Selector_add(selector, _selectableLeft);
+            selector.addWithNullIntersection(_selectableLeft);
         }
 
         if (selectedPlanes.contains(-planes[2]))
         {
-            Selector_add(selector, _selectableFront);
+            selector.addWithNullIntersection(_selectableFront);
         }
 
         if (selectedPlanes.contains(-planes[3]))
         {
-            Selector_add(selector, _selectableBack);
+            selector.addWithNullIntersection(_selectableBack);
         }
 
         if (selectedPlanes.contains(-planes[4]))
         {
-            Selector_add(selector, _selectableTop);
+            selector.addWithNullIntersection(_selectableTop);
         }
 
         if (selectedPlanes.contains(-planes[5]))
         {
-            Selector_add(selector, _selectableBottom);
+            selector.addWithNullIntersection(_selectableBottom);
         }
     }
 

radiantcore/brush/EdgeInstance.h

@@ -54,7 +54,7 @@ class EdgeInstance : public ISelectable {
 		SelectionIntersection best;
 		m_edge->testSelect(test, best);
 		if (best.isValid()) {
-			Selector_add(selector, *this, best);
+			selector.addWithIntersection(*this, best);
 		}
 	}
 };

radiantcore/brush/FaceInstance.cpp

@@ -175,7 +175,7 @@ void FaceInstance::testSelect(Selector& selector, SelectionTest& test) {
 	SelectionIntersection best;
 	testSelect(test, best);
 	if (best.isValid()) {
-		Selector_add(selector, m_selectable, best);
+		selector.addWithIntersection(m_selectable, best);
 	}
 }
 
@@ -184,7 +184,7 @@ void FaceInstance::testSelect_centroid(Selector& selector, SelectionTest& test)
 		SelectionIntersection best;
 		m_face->testSelect_centroid(test, best);
 		if (best.isValid()) {
-			Selector_add(selector, m_selectable, best);
+			selector.addWithIntersection(m_selectable, best);
 		}
 	}
 }
@@ -199,14 +199,14 @@ void FaceInstance::selectPlane(Selector& selector, const Line& line, PlanesItera
 		}
 	}
 
-	Selector_add(selector, m_selectable);
+	selector.addWithNullIntersection(m_selectable);
 
 	selectedPlaneCallback(getFace().plane3());
 }
 
 void FaceInstance::selectReversedPlane(Selector& selector, const SelectedPlanes& selectedPlanes) {
 	if (selectedPlanes.contains(-(getFace().plane3()))) {
-		Selector_add(selector, m_selectable);
+		selector.addWithNullIntersection(m_selectable);
 	}
 }
 

radiantcore/brush/VertexInstance.h

@@ -57,7 +57,7 @@ class VertexInstance :
 		SelectionIntersection best;
 		m_vertex->testSelect(test, best);
 		if (best.isValid()) {
-			Selector_add(selector, *this, best);
+			selector.addWithIntersection(*this, best);
 		}
 	}
 };

radiantcore/entity/VertexInstance.h

@@ -56,7 +56,7 @@ class VertexInstance :
 
 		if (best.isValid()) {
 			// Add the selectable to the given selector > this should trigger the callbacks
-			Selector_add(selector, *this, best);
+			selector.addWithIntersection(*this, best);
 		}
 	}
 };
@@ -86,7 +86,7 @@ class VertexInstanceRelative : public VertexInstance {
 
 		if (best.isValid()) {
 			// Add the selectable to the given selector > this should trigger the callbacks
-			Selector_add(selector, *this, best);
+			selector.addWithIntersection(*this, best);
 		}
 	}
 };

radiantcore/entity/curve/CurveEditInstance.cpp

@@ -25,7 +25,7 @@ void CurveEditInstance::testSelect(Selector& selector, SelectionTest& test)
 		test.TestPoint(*p, best);
 		if (best.isValid())
 		{
-			Selector_add(selector, *i, best);
+			selector.addWithIntersection(*i, best);
 		}
     }
 }

radiantcore/entity/doom3group/StaticGeometryNode.cpp

@@ -327,7 +327,7 @@ void StaticGeometryNode::testSelect(Selector& selector, SelectionTest& test)
 
 	// If the selectionIntersection is non-empty, add the selectable to the SelectionPool
 	if (best.isValid()) {
-		Selector_add(selector, *this, best);
+		selector.addWithIntersection(*this, best);
 	}
 }
 

radiantcore/patch/PatchControlInstance.h

@@ -41,7 +41,7 @@ class PatchControlInstance :
 		// If there is a control point that can be selected, add the Selectable to the selector
 		if (best.isValid())
 		{
-			Selector_add(selector, *this, best);
+			selector.addWithIntersection(*this, best);
 		}
 	}
 

radiantcore/selection/textool/FaceNode.cpp

@@ -120,7 +120,7 @@ void FaceNode::testSelect(Selector& selector, SelectionTest& test)
 
     if (best.isValid())
     {
-        Selector_add(selector, *this);
+        selector.addWithNullIntersection(*this);
     }
 }
 

radiantcore/selection/textool/Node.cpp

@@ -70,7 +70,7 @@ void Node::testSelectComponents(Selector& selector, SelectionTest& test)
 
         if (intersection.isValid())
         {
-            Selector_add(selector, vertex);
+            selector.addWithNullIntersection(vertex);
         }
     }
 }

radiantcore/selection/textool/PatchNode.cpp

@@ -104,7 +104,7 @@ void PatchNode::testSelect(Selector& selector, SelectionTest& test)
 
     if (best.isValid())
     {
-        Selector_add(selector, *this);
+        selector.addWithNullIntersection(*this);
     }
 }
 

radiantcore/selection/textool/TextureToolRotateManipulator.cpp

@@ -148,7 +148,7 @@ void TextureToolRotateManipulator::testSelect(SelectionTest& test, const Matrix4
 
     if (best.isValid())
     {
-        Selector_add(selector, _selectableZ);
+        selector.addWithNullIntersection(_selectableZ);
     }
 
     if (!selector.empty())