ORTS.Processes.ThreadHangException at ORTS.Traveller.DistanceTo (X3036, Vanchiglia - Nizza, Drivethrough1_notraffic, balloon loop (case 2))

[Carlo Santucci]

When re-entering the balloon loop the program freezes.
Route can be downloaded from here http://www.trainsimhobby.net/infusions/pro_download_panel/download.php?did=761 .
Logfile is attached, as well as the packaged activity. Within file GTT7000.con the trainset has to be replaced with another one, as I am no more able to provide a download link for the original trainset.
This bug is solved with patch traveller.cs.patch present in bug report https://bugs.launchpad.net/or/+bug/1447222 but not with patch activity.cs.patch.

[jeroenp (j-paasschens)]

Dear all,
I confirmed this.

As James already mentioned in https://bugs.launchpad.net/or/+bug/1447222 different places that use Traveller.DistanceTo should preferably define a maximum distance to search for to prevent loops. But it is also clear that not all of these uses have a default or natural search distance. In 1447222 there is a natural distance.

So I find it very difficult to create such a distance for all possible uses (both from within Activity.cs and in other places like train.cs). But I agree with James that the traveller loopchecking is probably too complex for 1.0.

So I can fix this bug (I think), see patch. But, other places where hanging can occur due to loops are still possible.

Note, that for this bug, the DistanceTo is called in places where the activity checks on whether it is on a station or platform. I take a maximum distance of 10000 (I am assuming this means 10km). Which I would assume would cover all platforms (1km might be too small for all I know). But I cannot be sure.

A better solution for this bug would be to refactor the code for finding whether you are in a platform/station or not, and perhaps use the size of the platform/station as a natural limit. But that is certainly for after 1.0.

As said, this fix is less natural then the one in 1447222.

Note, this patch is independent of the 1447222 patch (and does not solve that bug).

Best regards. Jeroen.

[Carlo Santucci (carlosanit1)]

Hi Jeroen,
I was investigating this in parallel with you, and came to the same patch as you (only, I put 5000 instead of 10000 meters and gave another name to the constant max checking distance :) ).
I have made some tests and in my opinion the patch is OK, so if James has nothing about this I kindly ask you to commit it .

[Carlo Santucci (carlosanit1)]

By the way such a patch spares also computing time.

[r.roeterdink (r-roeterdink)]

I don't quite agree with this patch.
It is a solution based on a particular situation rather than general assumptions.
I know plenty of routes and locations where platforms are more than 10km apart.
What happens in such a situation with the information in the Station Stop window (F10)?
Has that been checked?

Also, it does not save as much computing time as you might think. The check if the max distance has been reached is done for all situations and that will actually cost computing time.

[r.roeterdink (r-roeterdink)]

Furhter thoughts on this : a possible solution which would avoid the loop at all time regardless of the distance to the next platform is to use the train's path to search for the next platform, rather than the tracknodes.
The train's path will not loop by definition.
The Subroute class has a method "GetDistanceAlongRoute" which could be easily used for this purpose.

[r.roeterdink (r-roeterdink)]

And yet some more thoughts : if a general maximum distance for the "DistanceTo" is required, a very usefull value could be the "EndOfRoute" distance which by definition is available for all trains, as it is always set as distance-triggered action.
Only exception where this value is not available is in Explorer and Manual modes, but a general maximum could be set in those modes as no search for platforms or sidings is required in those modes anyway.
All what would be required is a new method for the distance-triggered actions to get the value for EndOfRoute action without actually triggering or removing that action.

[Carlo Santucci (carlosanit1)]

Rob,
the patch works well. The calculation of the distance to next station that is displayed in the F10 window is done in another place. I have built a test route in the Northeastern Corridor. You can see from picture Distantstation.jpg that the distance of next station is shown correctly. When the train has correctly (and automatically, with autopilot mode on) stopped and left the first station, the distance to next one is shown, see picture Distantstation1.jpg. The change performed affects tests that have sense only and are used only near to the station.
And I remark that computing time is spared, because you give up much sooner looking for a station; and the test for a maxdistance is done also now, only it is done with the maximum floating value.
So, the test is OK. This does not prevent me to say again that I preferred the solution based on the loop test in the Traveller method, because it is more general and protects against all hangups caused by distance calculations in loops. (By the way, the two patches can co-exist). It is also true that your suggestions too probably work well. But jeroen's solution is the one that minimizes patch dimension while providing a solution. So, it best fits into the actual rules for bug fix.

[jeroenp (j-paasschens)]

Dear all

First, let me mention that this part of the code for which I created a possible patch is not calculating the distance to the next platform. It is determining whether a traveller is between either two platform ends or two siding ends. So we are talking about the size of a platform/siding, not a distance from platform to platform.

The choice of 10km max platform size seems reasonable, but it is not failsafe.

A second remark is that the check for maxDistance is already in the DistanceTo method, but if gets a value of float.MaxValue (1e38 or so). So adding a different maxDistance can only save time. The loop checking, in contrast, will cost some time for every situation.

As James mentioned (http://www.elvastower.com/forums/index.php?/topic/24176-orts-simply-freezes-up-no-error-message-sound-keeps-running/page__st__60, April 26th) it is probably a good idea that all 'users' of a DistanceTo try to limit their search as much as possible. This will save time. This is probably a good idea whether or not there is a general loop detection mechanism. But, determining that max value is a bit more tricky. DistanceTo is used in activities (the source of this bug), but also in signals.cs, train.cs and some other places. That being said, I can imagine that for particular cases there are other possibilities to determine the distance and I, for sure do not have the overview.

I did try to look at 'EndOfRoute', but only found a boolean. Did you mean NextStopDistanceM?

Personally, I think the particular part of Activity.cs we are talking about in this bug should probably be rewritten to use the distance between the two platform ends as a more natural maxDistance. But this is for after 1.0 for certain. A possible solution is attached (but, like I said, not for now). But also in this case I can imagine a very long curved platform or siding where the distance along the track is much much larger than the distance from end-to-end.

[r.roeterdink (r-roeterdink)]

The problem with Activity.cs is that it was created before the signalling/train control update, and therefor does not use the new data structures as were made available in that update.
For all platforms, the position of the platform with relation to the TrackCircuitSection in which the platform is located, is calculated at program start and stored with the platform data.
For AI trains, platform stop handling has been moved to the train itself. The station stop position is calculated as the train starts, using the train's path information, and this is set as a 'distance-triggered' action. When that action is triggered and the train stops at that location, it is at the station - there is no further check required.
The timetable player train uses TrackCircuitSection information - this does not calculate any distances at all, but just compares the train's position (as available in OccupiedSections) with the platform location. If the train occupies the section which holds the platform, it must be at the station. Some further refinement is still required for unsignalled platforms, using the position within the section. But that will not require a search either - it just can compare the train's position within the section with the platform position.

This patch may do for now, to avoid the loop.
But the long-term solution should be along the same lines as for the timetable player train, using the TrackCircuitSection Information. This requires no search at all, and therefor can never loop - and by not having to search through the TDB data it saves even more load. Alos, the check should be moved from Activity.cs to the train itself.

[James Ross (twpol)]

I'm happy with any of the fixes which set a max distance for DistanceTo. Anything more complex I'd like to check it first.

After the stable release, we should certainly see about removing any use of the traveller to locate objects that are already located on the path, but we do need to make sure that any such solutions work in explore.

[jeroenp (j-paasschens)]

Dear all,
I committed the fix with only the DistanceTo part (3050). But it is clearly not the final solution.
I agree with Rob that parts of activity.cs should be rewritten, but I do not think I am currently capable of doing that.
I am not sure whether that needs to be tracked in this ticket though. So I did not set it to fixed yet.
Jeroen

[James Ross (twpol)]

Let's consider this bug fixed and open a new one for the Activity.cs work.