Centralize the check for valid external geometry to ensure consistency

src/Mod/Sketcher/App/SketchObject.cpp

@@ -68,6 +68,7 @@
 
 #include <Mod/Part/App/Geometry.h>
 #include <Mod/Part/App/DatumFeature.h>
+#include <Mod/Part/App/BodyBase.h>
 
 #include "SketchObject.h"
 #include "SketchObjectPy.h"
@@ -1700,6 +1701,34 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point)
     return -1;
 }
 
+bool SketchObject::isExternalAllowed(App::Document *pDoc, App::DocumentObject *pObj) const
+{
+    // Externals outside of the Document are NOT allowed
+    if (this->getDocument() != pDoc)
+        return false;
+
+    App::DocumentObject *support = this->Support.getValue();
+    Part::BodyBase* body = Part::BodyBase::findBodyOf(support);
+    if (body != NULL) {
+        if (Part::BodyBase::findBodyOf(pObj) != body) {
+            // Selection outside of body not allowed if flag is not set
+            if (!this->allowOtherBody)
+                return false;
+        }
+
+        // Datum features are always allowed
+        if(pObj->getTypeId().isDerivedFrom(App::Plane::getClassTypeId()) ||
+           pObj->getTypeId().isDerivedFrom(Part::Datum::getClassTypeId()))
+            return true;
+    } else {
+        // Legacy parts - don't allow selection outside of the support
+        if (pObj != support)
+            return false;
+    }
+
+    return true;
+}
+
 int SketchObject::addSymmetric(const std::vector<int> &geoIdList, int refGeoId, Sketcher::PointPos refPosId/*=Sketcher::none*/)
 {
     const std::vector< Part::Geometry * > &geovals = getInternalGeometry();
@@ -2718,9 +2747,7 @@ int SketchObject::DeleteUnusedInternalGeometry(int GeoId)
 int SketchObject::addExternal(App::DocumentObject *Obj, const char* SubName)
 {
     // so far only externals to the support of the sketch and datum features
-    if (!allowOtherBody && (Support.getValue() != Obj))
+    if (!isExternalAllowed(Obj->getDocument(), Obj))
-        if (!Obj->getTypeId().isDerivedFrom(App::Plane::getClassTypeId()) &&
-            !Obj->getTypeId().isDerivedFrom(Part::Datum::getClassTypeId()))
        return -1;
 
     // get the actual lists of the externals

src/Mod/Sketcher/App/SketchObject.h

@@ -259,8 +259,10 @@ public:
     /// gets the solved sketch as a reference
     inline Sketch &getSolvedSketch(void) {return solvedSketch;}
 
-    // Flag to allow external geometry from other bodies than the one this sketch belongs to
+    /// Flag to allow external geometry from other bodies than the one this sketch belongs to
     bool allowOtherBody;
+    /// Return true if this object is allowed as external geometry for the sketch
+    bool isExternalAllowed(App::Document *pDoc, App::DocumentObject *pObj) const;
 
 protected:
     /// get called by the container when a property has changed

src/Mod/Sketcher/Gui/CommandCreateGeo.cpp

@@ -4474,33 +4474,15 @@ namespace SketcherGui {
 
         bool allow(App::Document *pDoc, App::DocumentObject *pObj, const char *sSubName)
         {
-            // Selection outside of the Document is NOT allowed
             Sketcher::SketchObject *sketch = static_cast<Sketcher::SketchObject*>(object);
-            if (sketch->getDocument() != pDoc)
+            if (!sketch->isExternalAllowed(pDoc, pObj))
                 return false;
 
             App::DocumentObject *support = sketch->Support.getValue();
             Part::BodyBase* body = Part::BodyBase::findBodyOf(support);
-            if (body != NULL) {
+            if ((body != NULL) && (Part::BodyBase::findBodyOf(pObj) == body) && body->isAfterTip(pObj))
-                if (Part::BodyBase::findBodyOf(pObj) == body) {
                 // Don't allow selection after the Tip feature in the same body
-                    if (body->isAfterTip(pObj))
                 return false;
-                } else {
-                    // Selection outside of body not allowed if flag is not set
-                    if (!sketch->allowOtherBody)
-                        return false;
-                }
-
-                // Datum features are always allowed
-                if(pObj->getTypeId().isDerivedFrom(App::Plane::getClassTypeId()) ||
-                   pObj->getTypeId().isDerivedFrom(Part::Datum::getClassTypeId()))
-                    return true;
-            } else {
-                // Legacy parts - don't allow selection outside of the support
-                if (pObj != support)
-                    return false;
-            }
 
             if (!sSubName || sSubName[0] == '\0')
                 return false;