Resolve #5151: Targeting projected lights: let the line point to the …

…light source instead of the light volume's midpoint

I considered introducing a new virtual scene::INode::getObjectCenter() method to be overridden by lights, but I settled to dynamic_cast the target(ed) nodes to fix this.

include/ilightnode.h

@@ -18,7 +18,7 @@ class ILightNode
     /** 
 	 * greebo: Get the AABB of the Light "Diamond" representation.
      */
-    virtual AABB getSelectAABB() = 0;
+    virtual AABB getSelectAABB() const = 0;
 };
 typedef std::shared_ptr<ILightNode> ILightNodePtr;
 

include/version.h

@@ -2,7 +2,7 @@
 #include <config.h>
 #define RADIANT_VERSION PACKAGE_VERSION
 #else
-#define RADIANT_VERSION "2.8.0pre1"
+#define RADIANT_VERSION "2.8.0pre3"
 #endif
 
 #define RADIANT_APPNAME "DarkRadiant"

radiant/entity/light/LightNode.cpp

@@ -62,18 +62,15 @@ void LightNode::snapto(float snap) {
 	_light.snapto(snap);
 }
 
-AABB LightNode::getSelectAABB() {
+AABB LightNode::getSelectAABB() const 
+{
 	AABB returnValue = _light.lightAABB();
 
 	default_extents(returnValue.extents);
 
 	return returnValue;
 }
 
-/*greebo: This is a callback function that gets connected in the constructor
-* Don't know exactly what it does, but it seems to notify the shader cache that the light has moved or
-* something like that.
-*/
 void LightNode::lightChanged() {
 	GlobalRenderSystem().lightChanged(*this);
 }

radiant/entity/light/LightNode.h

@@ -66,7 +66,7 @@ class LightNode :
 	/** greebo: Returns the AABB of the small diamond representation.
 	 *	(use this to select the light against an AABB selectiontest like CompleteTall or similar).
 	 */
-	AABB getSelectAABB() override;
+	AABB getSelectAABB() const override;
 
 	/*greebo: This is a callback function that gets connected in the constructor
 	* Don't know exactly what it does, but it seems to notify the shader cache that the light has moved or

radiant/entity/target/Target.h

@@ -2,6 +2,7 @@
 
 #include "inode.h"
 #include "ientity.h"
+#include "ilightnode.h"
 #include "math/Vector3.h"
 #include "math/AABB.h"
 
@@ -65,6 +66,14 @@ class Target :
 			return Vector3(0,0,0);
 		}
 
+		// Check if we're targeting a light, and use its center in that case (#5151)
+		auto lightNode = dynamic_cast<const ILightNode*>(node);
+
+		if (lightNode != nullptr)
+		{
+			return lightNode->getSelectAABB().getOrigin();
+		}
+
 		return node->worldAABB().getOrigin();
 	}
 };

radiant/entity/target/TargetLineNode.cpp

@@ -1,6 +1,7 @@
 #include "TargetLineNode.h"
 
 #include "../EntityNode.h"
+#include "ilightnode.h"
 
 namespace entity
 {
@@ -49,14 +50,24 @@ std::size_t TargetLineNode::getHighlightFlags()
 
 const Vector3& TargetLineNode::getOwnerPosition() const
 {
-	const AABB& bounds = _owner.worldAABB();
+    try
+    {
+        // Try to use the origin if this is a light
+        auto& light = dynamic_cast<ILightNode&>(_owner);
 
-	if (bounds.isValid())
+        return light.getSelectAABB().getOrigin();
+    }
+    catch (std::bad_cast&)
     {
-		return bounds.getOrigin();
-	}
+        const AABB& bounds = _owner.worldAABB();
+
+        if (bounds.isValid())
+        {
+            return bounds.getOrigin();
+        }
 
-	return _owner.localToWorld().t().getVector3();
+        return _owner.localToWorld().t().getVector3();
+    }
 }
 
 }

tools/innosetup/darkradiant.x64.iss

@@ -1,7 +1,7 @@
 ; Script generated by the Inno Setup Script Wizard.
 ; SEE THE DOCUMENTATION FOR DETAILS ON CREATING INNO SETUP SCRIPT FILES!
 
-#define DarkRadiantVersion "2.8.0pre1"
+#define DarkRadiantVersion "2.8.0pre3"
 
 [Setup]
 AppName=DarkRadiant