Geometry functions returning null Variants in GDScript returns (possibly) false values in C#

[PerMalmberg]

Tested versions

• v4.2.1.stable.mono.official [b09f793]

System information

Godot v4.2.1.stable.mono - Windows 10.0.22631 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2070 SUPER (NVIDIA; 31.0.15.5176) - AMD Ryzen 7 3800XT 8-Core Processor (16 Threads)

Issue description

There may be other instances where this happens, but these are the ones I've found:

• https://docs.godotengine.org/en/stable/classes/class_geometry2d.html#class-geometry2d-method-segment-intersects-segment
• https://docs.godotengine.org/en/stable/classes/class_geometry2d.html#class-geometry2d-method-line-intersects-line
• https://docs.godotengine.org/en/stable/classes/class_geometry3d.html#class-geometry3d-method-ray-intersects-triangle

Vector2/3.Zero may be the actual intersection point so there is no way can check if an intersection actually exists or not using these functions in C#.

Steps to reproduce

using static Godot.Geometry2D;
....
// Correctly returns (0,0)
var undeterminable = SegmentIntersectsSegment(new Vector2(-1,0), new Vector2(1, 0), new Vector2(0, -1), new Vector2(0, 1)).AsVector2();

// Erroneously returns (0,0)
undeterminable = SegmentIntersectsSegment(new Vector2(-1, 0), new Vector2(0, 0), new Vector2(1, 1), new Vector2(1, 2)).AsVector2();

// 
// CS0019	Operator '==' cannot be applied to operands of type 'Variant' and '<null>'
// if(undeterminable == null) { 
//
// }

The two first vectors intersect at (0,0), but we can't determine that since it might also be a default constructed Vector2, i.e. null. The second two also results in (0,0)

Minimal reproduction project (MRP)

See steps to reproduce.

Edit: Make example code more explicit for Vector2.

[PerMalmberg]

Here's a MRP showing the problem and the entire C# code for those that don't want to open the actual project.

Godot-89850.mp4

Godot-89850.zip

using Godot;
using System.Linq;
using static Godot.Geometry2D;

public partial class Node2D : Godot.Node2D
{
    private Vector2[] crossing = [new Vector2(100, 100), new Vector2(200, 100),
                                  new Vector2(150, 50), new Vector2(150, 150)];

    private Vector2[] notCrossing = [new Vector2(100, 100), new Vector2(100, 200),
                                  new Vector2(150, 100), new Vector2(150, 200)];

    private Vector2[] current;

    // Called when the node enters the scene tree for the first time.
    public override void _Ready()
	{
        Toggle();
    }

    private void Toggle()
    {
        current = current == notCrossing ? crossing : notCrossing;
        var l1 = GetNode<Line2D>("%Line1");
        var l2 = GetNode<Line2D>("%Line2");

        var points1 = current.Take(2).ToArray();
        var points2 = current.TakeLast(2).ToArray();

        l1.Points = points1;
        l2.Points = points2;
        var intersection = SegmentIntersectsSegment(points1[0], points1[1], points2[0], points2[1]).As<Vector2>();
        GetNode<Label>("%Label").Text = intersection.ToString();
    }


    public void on_pressed()
    {
        Toggle();        
    }
}

[raulsntos]

Vector2/3.Zero may be the actual intersection point so there is no way can check if an intersection actually exists or not using these functions in C#.

If a Variant is null, you can check the type:

Variant intersection = SegmentIntersectSegment(fromA, toA, fromB, toB);
if (intersection.VariantType == Variant.Type.Nil)
{
    // Intersection is null.
}
else
{
    // Intersection is not null.
    Vector2 intersectionVector = intersection.As<Vector2>();
}

[PerMalmberg]

Ok, that's a step in the right direction.

For now we can wrap it like this:

private Vector2? SegmentIntersectsSegment2(Vector2 a1, Vector2 a2, Vector2 b1, Vector2 b2)
{
    var intersection = SegmentIntersectsSegment(a1, a2, b1, b2); ;
    return intersection.VariantType == Variant.Type.Nil ? null : intersection.AsVector2();        
}

May I suggest that these functions are changed to return a Vector2? instead of a Variant, essentially providing the above wrapper as part of the API? That would bring them in line with what the documentation states and be more what is expected, imho.

[raulsntos]

These methods are generated from the information in ClassDB and ClassDB only says that the return type is Variant. We have no way of knowing if that means Vector2 | null or Vector2 | Vector3 or any other combination of types.

This information may be added to ClassDB if this proposal gets implemented:

• Add support for nullable static types in GDScript godot-proposals#162

But keep in mind that changing the return type of a method breaks binary compatibility in C#.

[PerMalmberg]

Yes, it is a breaking change. And needs to be treated accordingly. It has been done previously though, see #32670