FS#6410 - YAPF and Road Depots

Attached to Project: OpenTTD
Opened by Juanjo (juanjo) - Thursday, 07 January 2016, 21:01 GMT
Last edited by frosch (frosch) - Sunday, 29 May 2016, 14:18 GMT
Type Bug
Category Core
Status Closed
Assigned To No-one
Operating System All
Severity Low
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


See topic:

It seems that this line in yapf_road.cpp:
if (max_distance > 0 && n->m_cost > max_distance * YAPF_TILE_LENGTH) return false;
causes the problem.

m_cost -> cost of the best path (penalty length, not tile length)
max_distance -> usually 2000. It is also penalty length (2000 with penalty is about 20 tiles)

With r27488, if a vehicle needs servicing and it finds a depot at a distance less than 2000 tiles, it will go there skipping all other orders. Changing it to:
if (max_distance > 0 && n->m_cost > max_distance) return false;
will do it if the depot is at a distance less than 20 tiles.

This is the easiest way to solve the bug.

This task depends upon

Closed by  frosch (frosch)
Sunday, 29 May 2016, 14:18 GMT
Reason for closing:  Fixed
Additional comments about closing:  in r27586. Thanks for the patch!
Comment by Juanjo (juanjo) - Thursday, 07 January 2016, 21:02 GMT
As I was trying to solve it, I came across that:
- YAPF for rail uses max_distance in a more efficient way: it discards paths longer than max_distance while searching a path. Road and ship do not do it.
For some reason it hasn't been done. Probably, something about efficiency. I could try extending it to road and ship depots if other developers consider it appropriate. If that could be done, I could try with ships finding depots in a better way (FS#5713 - Boats sent to impossible depots).

- FindDepot in YAPF calculates the cost of the best path, but doesn't return it. If a depot is found, it returns a cost of max_distance/2.
I think that returning a FindDepotData object (tile, cost and reversed) makes the code easier to read. Maybe returning a bool is faster, but I don't know if that is really necessary. The attached patch is for changing this, that could also be changed for YAPF rail.
Comment by frosch (frosch) - Sunday, 29 May 2016, 14:18 GMT
Yes, adding "discarding long paths" to road- and ship pathfinder would be nice.
I referenced your comment in FS#5713.