DrawNodeCircle has bug/redundancy. but it works.

[kianzarrin]

TrafficManagerTool.DrawNodeCircle has a bug. but the bug is in redundant code so it works.

//the debug logs does not exist in original code
public void DrawNodeCircle(RenderManager.CameraInfo cameraInfo,
                            ushort nodeId,
                            Color color,
                            bool alpha = false) {
    NetNode[] nodesBuffer = Singleton<NetManager>.instance.m_nodes.m_buffer;
    Debug.Log($"m_segment0={nodesBuffer[nodeId].m_segment0}"); 
    NetSegment segment =
        Singleton<NetManager>.instance.m_segments.m_buffer[ nodesBuffer[nodeId].m_segment0];//BUG!!!!!!!!!!!
    Debug.Log($"vectors = {segment.m_startDirection} {segment.m_endDirection}");

    Vector3 pos = nodesBuffer[nodeId].m_position;
    float terrainY = Singleton<TerrainManager>.instance.SampleDetailHeightSmooth(pos);
    if (terrainY > pos.y) {
        pos.y = terrainY;
    }

    Bezier3 bezier;
    bezier.a = pos;
    bezier.d = pos;

////Redundant!!!!!!  instead: *** bezier.a=bezier.b=bezier.c=bezier.d=pos ***
    NetSegment.CalculateMiddlePoints(
        bezier.a,
        segment.m_startDirection,
        bezier.d,
        segment.m_endDirection, 
        false,
        false,
        out bezier.b,
        out bezier.c);

    Debug.Log($"bezier = {bezier.a} {bezier.b} {bezier.c} {bezier.d}");


    DrawOverlayBezier(cameraInfo, bezier, color, alpha);
}

m_segment0 in the code above can be zero (screenshot 1). this is similar to #549.
Fortunately here the code that utilizes m_segment0 is redundant at least to the best of my knowledge. In screenshot 2 m_segment0 is 0 and its directional vectors are subsequently zero as well. Nevertheless the final bezier result is accurate. screenshot 3 shows a valid m_segment0 will result in the same bezier result as invalid m_segment0.

The call to CalculateMiddlePoints()

Why are we calling CalculateMiddlePoints on a single node anyway. node cannot collide with node?

Risk

• If CalculateMiddlePoints is really redundant then we are getting away with the bug because the bug is in redundant part of the code. In future skylines upgrades this might result in null reference exceptions.
• If I am missing something and there are circumstances in which CalculateMiddlePoints() is not redundant then we have a real bug.

Action

we do not need segment or a call to CalculateMiddlePoints (AFIK). Instead bezier.a=bezier.b=bezier.c=bezier.d=pos because its a simple node.
DrawOverlayCircle(cameraInfo, color, pos, 20, alpha);

[kianzarrin]

Am I correct? is CalculateMiddlePoints() redundant?

[kvakvs]

Seems to make sense. If it works without that call, just remove it.

[kianzarrin]

If you want this fixed then please assign it to me.

[kvakvs]

You will get it assigned at some point, during the review of your pull request usually. On small tasks like this, especially reported by you and fixed by you, assigning yourself to it has no effect.

[kianzarrin]

I think its better if we calculate the overlay circle radius as the average radius of all connected segments.