From 3b806088ba656f21442a316dafdb4a6b4b90e6c1 Mon Sep 17 00:00:00 2001
From: Matthew Mott <orbweaver3d@gmail.com>
Date: Wed, 24 Nov 2021 21:14:24 +0000
Subject: [PATCH] scene::forEachTransformable takes node by reference

Not sure why this was ever using a shared_ptr, since the INode was never
stored: the only thing the function ever did with it was immediately
dereference it. Switching to a simple reference avoids the need for several
shared_from_this() calls in Doom3GroupNode.
---
 libs/transformlib.h                           | 38 +++++++++++--------
 .../entity/doom3group/Doom3GroupNode.cpp      | 10 ++---
 2 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/libs/transformlib.h b/libs/transformlib.h
index dc20157a5f..9b61e30e56 100644
--- a/libs/transformlib.h
+++ b/libs/transformlib.h
@@ -21,23 +21,29 @@ namespace scene
 {
 
 /**
- * Visit each transformable that is child of the given node with the given functor.
+ * @brief Visit each Transformable child node
+ *
+ * @tparam Func
+ * Functor object accepting a single parameter consisting of an ITransformable&.
+ *
+ * @param node
+ * Node to start traversal from.
+ *
+ * @param functor
+ * Functor to invoke.
  */
-inline void foreachTransformable(const scene::INodePtr& node, const std::function<void(ITransformable&)>& functor)
+template<typename Func>
+void forEachTransformable(const INode& node, Func functor)
 {
-	if (!node) return;
-
-	node->foreachNode([&] (const scene::INodePtr& child)->bool
-	{
-		ITransformablePtr transformable = Node_getTransformable(child);
-
-		if (transformable)
-		{
-			functor(*transformable);
-		}
-
-		return true;
-	});
+    node.foreachNode(
+        [&](const scene::INodePtr& child) -> bool
+        {
+            ITransformablePtr transformable = Node_getTransformable(child);
+            if (transformable)
+                functor(*transformable);
+
+            return true;
+        }
+    );
 }
-
 }
diff --git a/radiantcore/entity/doom3group/Doom3GroupNode.cpp b/radiantcore/entity/doom3group/Doom3GroupNode.cpp
index a972482688..256e179cd4 100644
--- a/radiantcore/entity/doom3group/Doom3GroupNode.cpp
+++ b/radiantcore/entity/doom3group/Doom3GroupNode.cpp
@@ -370,7 +370,7 @@ void Doom3GroupNode::_onTransformationChanged()
 	// If this is a container, pass the call to the children and leave the entity unharmed
 	if (!isModel())
 	{
-		scene::foreachTransformable(shared_from_this(), [] (ITransformable& child)
+		scene::forEachTransformable(*this, [] (ITransformable& child)
 		{
 			child.revertTransform();
 		});
@@ -491,7 +491,7 @@ void Doom3GroupNode::rotate(const Quaternion& rotation)
 	if (!isModel())
 	{
 		// Rotate all child nodes too
-		scene::foreachTransformable(shared_from_this(), [&] (ITransformable& child)
+		scene::forEachTransformable(*this, [&] (ITransformable& child)
 		{
 			child.setType(TRANSFORM_PRIMITIVE);
 			child.setRotation(rotation);
@@ -512,7 +512,7 @@ void Doom3GroupNode::scale(const Vector3& scale)
 	if (!isModel())
 	{
 		// Scale all child nodes too
-		scene::foreachTransformable(shared_from_this(), [&] (ITransformable& child)
+		scene::forEachTransformable(*this, [&] (ITransformable& child)
 		{
 			child.setType(TRANSFORM_PRIMITIVE);
 			child.setScale(scale);
@@ -554,7 +554,7 @@ void Doom3GroupNode::freezeTransform()
 
 	if (!isModel())
 	{
-		scene::foreachTransformable(shared_from_this(), [] (ITransformable& child)
+		scene::forEachTransformable(*this, [] (ITransformable& child)
 		{
 			child.freezeTransform();
 		});
@@ -694,7 +694,7 @@ void Doom3GroupNode::translateChildren(const Vector3& childTranslation)
 	if (inScene())
 	{
 		// Translate all child nodes too
-		scene::foreachTransformable(shared_from_this(), [&] (ITransformable& child)
+		scene::forEachTransformable(*this, [&] (ITransformable& child)
 		{
 			child.setType(TRANSFORM_PRIMITIVE);
 			child.setTranslation(childTranslation);