[Csantucci]

I'm quite puzzled, but I believe I found a bug in Traveller.DistanceTo(Traveller traveller, TrackNode trackNode, int tileX, int tileZ, float x, float y, float z, float maxDistance), which should be quite a consolidated method. This method should return the distance from the initial position of the traveller to the point which has the coordinates which are the argument of the call. If the point hasn't been reached within maxDistance, a -1 must be returned.
This is the first part of the central while loop:

           while (accumulatedDistance < maxDistance)
            {
                if (traveller.IsTrack)
                {
                    var initialOffset = traveller.trackOffset;
                    var radius = traveller.IsTrackCurved ? traveller.trackSection.SectionCurve.Radius : 1;
                    if (traveller.TN == trackNode || trackNode == null)
                    {
                        var direction = traveller.Direction == TravellerDirection.Forward ? 1 : -1;
                        if (InitTrackSectionSucceeded(traveller, targetLocation))
                        {
                            // If the new offset is EARLIER, the target is behind us!
                            if (traveller.trackOffset * direction < initialOffset * direction)
                                break;
                            // Otherwise, accumulate distance from offset change and we're done.
                            accumulatedDistance += (traveller.trackOffset - initialOffset) * direction * radius;
                            return accumulatedDistance;
                        }
                    }

So note that the check for accumulatedDistance < maxDistance is made within the while clause, but at the end of this excerpt there is this

                            accumulatedDistance += (traveller.trackOffset - initialOffset) * direction * radius;
                            return accumulatedDistance;

which increases accumulated distance and returns it, without checking whether this increase causes leads to accumulatedDistance being bigger than maxDistance. So a check should be added in the return line: if accumulated distance is bigger than maxDistance, -1 must be returned.

Is the actual behaviour wanted? Because being a TrackNode also very long, the distance could be returned even if it is some Km higher than maxDistance.

I found it because this is the cause of some occasional flickering in camera position in the new Shift-4 camera, that locates it preferentially at platforms and level crossings.

[James Ross]

 Csantucci, on 03 February 2019 - 02:37 AM, said:

So note that the check for accumulatedDistance < maxDistance is made within the while clause, but at the end of this excerpt there is this

                            accumulatedDistance += (traveller.trackOffset - initialOffset) * direction * radius;
                            return accumulatedDistance;

which increases accumulated distance and returns it, without checking whether this increase causes leads to accumulatedDistance being bigger than maxDistance. So a check should be added in the return line: if accumulated distance is bigger than maxDistance, -1 must be returned.

Is the actual behaviour wanted? Because being a TrackNode also very long, the distance could be returned even if it is some Km higher than maxDistance.

I found it because this is the cause of some occasional flickering in camera position in the new Shift-4 camera, that locates it preferentially at platforms and level crossings.

Hmm, that's interesting. I believe that in general the max distance is only intended to prevent the code searching forever (i.e. to set an arbitrary limit), but I can see that it might be more important for certain cases to never return distances >= max distance.

Does the flickering stop if you add a condition here, to return -1 if the accumulated distance is >= max distance? If so, I'd be okay with a patch for it - though we will have to be a bit wary that others things could be affected.

[Csantucci]

Hi James
yes, flickering stops in that case. Waiting for your reply, I inserted the check outside the DistanceTo method. I've also had a check in all places where this is used. I didn't find problems in introducing the correction (even, as I said, I solved the problem outside). There's a point (in activity.cs) where the coder seemed to be aware of the issue

           var distance = trainFrontPosition.DistanceTo(e.TileX, e.TileZ, e.X, trainFrontPosition.Y, e.Z, e.RadiusM);
            if (distance == -1)
            {
                trainFrontPosition.ReverseDirection();
                distance = trainFrontPosition.DistanceTo(e.TileX, e.TileZ, e.X, trainFrontPosition.Y, e.Z, e.RadiusM);
                if (distance == -1)
                    return triggered;
            }
            if (distance < e.RadiusM) { triggered = true; }
            return triggered;
        }