From 638e91841ca9235cd0063ec0828d36962cefd5bf Mon Sep 17 00:00:00 2001
From: codereader <greebo@angua.at>
Date: Sun, 17 May 2020 12:14:11 +0200
Subject: [PATCH] #5231: Move connect entities algorithm from EntityCreator
 interface to namespace selection::algorithm.

---
 include/ientity.h                      |  9 ++----
 radiant/entity/EntityCreator.cpp       | 43 --------------------------
 radiant/entity/EntityCreator.h         |  1 -
 radiant/selection/algorithm/Entity.cpp | 43 +++++++++++++++++++++++---
 4 files changed, 42 insertions(+), 54 deletions(-)

diff --git a/include/ientity.h b/include/ientity.h
index 6ebbd0ce9a..dd6f8806b7 100644
--- a/include/ientity.h
+++ b/include/ientity.h
@@ -317,7 +317,7 @@ class ITargetManager
 };
 typedef std::shared_ptr<ITargetManager> ITargetManagerPtr;
 
-const std::string MODULE_ENTITYCREATOR("Doom3EntityCreator");
+const char* const MODULE_ENTITYCREATOR("Doom3EntityCreator");
 
 /**
  * \brief
@@ -332,15 +332,12 @@ class EntityCreator :
     /// Create an entity node with the given entity class.
     virtual IEntityNodePtr createEntity(const IEntityClassPtr& eclass) = 0;
 
-    /// Connect the two given entity nodes using the "target" system.
-    virtual void connectEntities(const scene::INodePtr& source,
-                                 const scene::INodePtr& target) = 0;
-
     // Constructs a new targetmanager instance (used by root nodes)
     virtual ITargetManagerPtr createTargetManager() = 0;
 };
 
-inline EntityCreator& GlobalEntityCreator() {
+inline EntityCreator& GlobalEntityCreator()
+{
     // Cache the reference locally
     static EntityCreator& _entityCreator(
         *std::static_pointer_cast<EntityCreator>(
diff --git a/radiant/entity/EntityCreator.cpp b/radiant/entity/EntityCreator.cpp
index dd9c052c87..28c0a07365 100644
--- a/radiant/entity/EntityCreator.cpp
+++ b/radiant/entity/EntityCreator.cpp
@@ -102,49 +102,6 @@ IEntityNodePtr Doom3EntityCreator::createEntity(const IEntityClassPtr& eclass)
 	return node;
 }
 
-/* Connect two entities using a "target" key.
- */
-void Doom3EntityCreator::connectEntities(const scene::INodePtr& source,
-                                         const scene::INodePtr& target)
-{
-	// Obtain both entities
-	Entity* e1 = Node_getEntity(source);
-	Entity* e2 = Node_getEntity(target);
-
-	// Check entities are valid
-	if (e1 == NULL || e2 == NULL) {
-		rError() << "entityConnectSelected: both of the selected instances must be an entity\n";
-		return;
-	}
-
-	// Check entities are distinct
-	if (e1 == e2) {
-		rError() << "entityConnectSelected: the selected instances must not both be from the same entity\n";
-		return;
-	}
-
-	// Start the scoped undo session
-	UndoableCommand undo("entityConnectSelected");
-
-	// Find the first unused target key on the source entity
-	for (int i = 0; i < 1024; ++i) {
-
-		// Construct candidate key by appending number to "target"
-		std::string targetKey = fmt::format("target{0:d}", i);
-
-		// If the source entity does not have this key, add it and finish,
-		// otherwise continue looping
-		if (e1->getKeyValue(targetKey).empty())
-		{
-			e1->setKeyValue(targetKey, e2->getKeyValue("name"));
-			break;
-		}
-	}
-
-	// Redraw the scene
-	SceneChangeNotify();
-}
-
 ITargetManagerPtr Doom3EntityCreator::createTargetManager()
 {
     return std::make_shared<TargetManager>();
diff --git a/radiant/entity/EntityCreator.h b/radiant/entity/EntityCreator.h
index ef0f7011f4..f94b97af01 100644
--- a/radiant/entity/EntityCreator.h
+++ b/radiant/entity/EntityCreator.h
@@ -14,7 +14,6 @@ class Doom3EntityCreator :
 
     // EntityCreator implementation
 	IEntityNodePtr createEntity(const IEntityClassPtr& eclass) override;
-	void connectEntities(const scene::INodePtr& source, const scene::INodePtr& target) override;
     ITargetManagerPtr createTargetManager() override;
 
 	// RegisterableModule implementation
diff --git a/radiant/selection/algorithm/Entity.cpp b/radiant/selection/algorithm/Entity.cpp
index 0c16cef92d..603d9e6bd6 100644
--- a/radiant/selection/algorithm/Entity.cpp
+++ b/radiant/selection/algorithm/Entity.cpp
@@ -1,5 +1,6 @@
 #include "Entity.h"
 
+#include <limits>
 #include "i18n.h"
 #include "itransformable.h"
 #include "selectionlib.h"
@@ -178,10 +179,44 @@ void connectSelectedEntities(const cmd::ArgumentList& args)
 {
 	if (GlobalSelectionSystem().countSelected() == 2)
 	{
-		GlobalEntityCreator().connectEntities(
-			GlobalSelectionSystem().penultimateSelected(),	// source
-			GlobalSelectionSystem().ultimateSelected()		// target
-		);
+		// Obtain both entities
+		Entity* e1 = Node_getEntity(GlobalSelectionSystem().penultimateSelected()); // source
+		Entity* e2 = Node_getEntity(GlobalSelectionSystem().ultimateSelected()); // target
+
+		// Check entities are valid
+		if (e1 == nullptr || e2 == nullptr)
+		{
+			rError() << "connectSelectedEntities: both of the selected instances must be entities" << std::endl;
+			return;
+		}
+
+		// Check entities are distinct
+		if (e1 == e2) 
+		{
+			rError() << "connectSelectedEntities: the selected entities must be different" << std::endl;
+			return;
+		}
+
+		// Start the scoped undo session
+		UndoableCommand undo("entityConnectSelected");
+
+		// Find the first unused target key on the source entity
+		for (unsigned int i = 0; i < std::numeric_limits<unsigned int>::max(); ++i)
+		{
+			// Construct candidate key by appending number to "target"
+			auto targetKey = fmt::format("target{0:d}", i);
+
+			// If the source entity does not have this key, add it and finish,
+			// otherwise continue looping
+			if (e1->getKeyValue(targetKey).empty())
+			{
+				e1->setKeyValue(targetKey, e2->getKeyValue("name"));
+				break;
+			}
+		}
+
+		// Redraw the scene
+		SceneChangeNotify();
 	}
 	else
 	{