#5885: IMatrixTransform's method is now setLocalToParent()

Assigning to a non-const reference return value is horrible for debugging (no easy way to set a breakpoint on value change) and similarly inconvenient for IDE navigation (const and non-const methods are mixed up in search results). This is now a fully fledged setter method to improve clarity.
codereader · Feb 2, 2022 · 0ffea8f · 0ffea8f
1 parent 7e728cb
commit 0ffea8f
Show file tree

Hide file tree

Showing 8 changed files with 18 additions and 25 deletions.
diff --git a/include/itransformnode.h b/include/itransformnode.h
@@ -20,6 +20,6 @@ class IMatrixTransform: public ITransformNode
 {
 public:
 
-    /// Return a modifiable reference to a contained transformation matrix
-	virtual Matrix4& localToParent() = 0;
+    /// Set the value of the contained transformation matrix
+	virtual void setLocalToParent(const Matrix4& localToParent) = 0;
 };
diff --git a/radiantcore/entity/EntityNode.cpp b/radiantcore/entity/EntityNode.cpp
@@ -145,7 +145,7 @@ void EntityNode::createAttachedEntities()
 
             // Set the attached entity's transform matrix according to the
             // required offset
-            attachedEnt->localToParent() = Matrix4::getTranslation(a.offset);
+            attachedEnt->setLocalToParent(Matrix4::getTranslation(a.offset));
         }
     );
 }

diff --git a/radiantcore/entity/EntityNode.h b/radiantcore/entity/EntityNode.h
@@ -111,7 +111,10 @@ class EntityNode :
 
     // IMatrixTransform implementation
     Matrix4 localToParent() const override { return _localToParent; }
-    Matrix4& localToParent() override { return _localToParent; }
+    void setLocalToParent(const Matrix4& localToParent) override
+    {
+        _localToParent = localToParent;
+    }
 
     // IComparableNode implementation
     std::string getFingerprint() override;

diff --git a/radiantcore/entity/doom3group/StaticGeometryNode.cpp b/radiantcore/entity/doom3group/StaticGeometryNode.cpp
@@ -674,16 +674,13 @@ void StaticGeometryNode::modelChanged(const std::string& value)
 
 void StaticGeometryNode::updateTransform()
 {
-	localToParent() = Matrix4::getIdentity();
+    if (isModel())
+        setLocalToParent(Matrix4::getTranslation(m_origin) * m_rotation.getMatrix4());
+    else
+        setLocalToParent(Matrix4::getIdentity());
 
-	if (isModel())
-	{
-		localToParent().translateBy(m_origin);
-		localToParent().multiplyBy(m_rotation.getMatrix4());
-	}
-
-	// Notify the Node about this transformation change	to update the local2World matrix
-	transformChanged();
+    // Notify the Node about this transformation change	to update the local2World matrix
+    transformChanged();
 }
 
 void StaticGeometryNode::translateChildren(const Vector3& childTranslation)

diff --git a/radiantcore/entity/eclassmodel/EclassModelNode.cpp b/radiantcore/entity/eclassmodel/EclassModelNode.cpp
@@ -159,10 +159,7 @@ const Vector3& EclassModelNode::getUntransformedOrigin()
 
 void EclassModelNode::updateTransform()
 {
-	localToParent() = Matrix4::getIdentity();
-	localToParent().translateBy(_origin);
-
-	localToParent().multiplyBy(_rotation.getMatrix4());
+    setLocalToParent(Matrix4::getTranslation(_origin) * _rotation.getMatrix4());
 
 	EntityNode::transformChanged();
 }

diff --git a/radiantcore/entity/generic/GenericEntityNode.cpp b/radiantcore/entity/generic/GenericEntityNode.cpp
@@ -190,7 +190,7 @@ void GenericEntityNode::_freezeTransform()
 
 void GenericEntityNode::updateTransform()
 {
-	localToParent() = Matrix4::getTranslation(m_origin);
+	setLocalToParent(Matrix4::getTranslation(m_origin));
 
 	if (_allow3Drotations)
 	{

diff --git a/radiantcore/entity/light/LightNode.cpp b/radiantcore/entity/light/LightNode.cpp
@@ -663,9 +663,7 @@ void LightNode::updateOrigin() {
         projectionChanged();
 
     // Update the transformation matrix
-    localToParent() = Matrix4::getIdentity();
-    localToParent().translateBy(_originTransformed);
-    localToParent().multiplyBy(m_rotation.getMatrix4());
+    setLocalToParent(Matrix4::getTranslation(_originTransformed) * m_rotation.getMatrix4());
 
     // Notify all child nodes
     m_transformChanged();
@@ -794,9 +792,7 @@ void LightNode::rotationChanged()
     m_rotation = m_useLightRotation ? m_lightRotation : m_rotationKey.m_rotation;
 
     // Update the transformation matrix
-    localToParent() = Matrix4::getIdentity();
-    localToParent().translateBy(_originTransformed);
-    localToParent().multiplyBy(m_rotation.getMatrix4());
+    setLocalToParent(Matrix4::getTranslation(_originTransformed) * m_rotation.getMatrix4());
 
     // Notify owner about this
     m_transformChanged();

diff --git a/radiantcore/entity/speaker/SpeakerNode.cpp b/radiantcore/entity/speaker/SpeakerNode.cpp
@@ -265,7 +265,7 @@ void SpeakerNode::translate(const Vector3& translation)
 
 void SpeakerNode::updateTransform()
 {
-	localToParent() = Matrix4::getTranslation(m_origin);
+	setLocalToParent(Matrix4::getTranslation(m_origin));
 	transformChanged();
 }