Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YAPF can't find road depot, but NPF can #7001

Closed
SamuXarick opened this issue Dec 29, 2018 · 6 comments · Fixed by #7009

Comments

@SamuXarick
Copy link
Contributor

@SamuXarick SamuXarick commented Dec 29, 2018

fruntborough transport 1966-01-09 1
fruntborough transport 1966-01-09

@James103

This comment has been minimized.

Copy link
Contributor

@James103 James103 commented Dec 29, 2018

To Reproduce (OTTD 1.8.0, Win 7 Ultimate):

  1. Build the above setup or download the attached savegame (unzip first).
  2. Make sure that the road vehicle is north-east of the depot and facing north-east or that the road vehicle is south-west of the depot and facing south-west (both work).
  3. Pause game.
  4. Try to send vehicle to depot with YAPF pathfinder. Result: "Unable to find local depot" error.
  5. Try to send vehicle to depot with NPF pathfinder. Result: Success
  6. While the game is paused and the vehicle is being sent to the depot, change it's pathfinder to YAPF.
  7. The vehicle successfully goes to the depot without reporting in as lost

.

@nielsmh

This comment has been minimized.

Copy link
Contributor

@nielsmh nielsmh commented Dec 30, 2018

Reproduced per the above in a recent development build. The issue is not specific to NE-SW directions, it also reproduces in NW-SE directions.

@James103

This comment has been minimized.

Copy link
Contributor

@James103 James103 commented Jan 3, 2019

Possible Fix: Instead of checking just the pathfinder defined in the game settings (ethier YAPF or NPF), also check the other pathfinder for a valid go-to-depot location/route before returning the error (YAPF and then NPF if YAPF is selected or vice versa). Only if both pathfinders fail to find a suitable depot location for the go-to-nearest-depot order will the "Unable to find [route to] local depot" error be thrown.

@J0anJosep

This comment has been minimized.

Copy link
Contributor

@J0anJosep J0anJosep commented Jan 3, 2019

Possible Fix: Instead of checking just the pathfinder defined in the game settings (ethier YAPF or NPF), also check the other pathfinder for a valid go-to-depot location/route before returning the error (YAPF and then NPF if YAPF is selected or vice versa). Only if both pathfinders fail to find a suitable depot location for the go-to-nearest-depot order will the "Unable to find [route to] local depot" error be thrown.

That is not acceptable.

@J0anJosep

This comment has been minimized.

Copy link
Contributor

@J0anJosep J0anJosep commented Jan 3, 2019

There are several problems here:

  • Pathfinders don't detect that normal road stations (non-drive-through road stations) are tiles where road vehicles can reverse.
  • NPF returns a depot, but this is due to a bug, as it should not find it. It uses function NPFRouteToDepotBreadthFirstTwoWay(*), as trains do. Trains can reverse and look two ways, but road vehicles shouldn't.
  • There is a max. penalty setting which is not used.

So, the three of them should be addressed.

I attach two savegames where the bus shouldn't find any depot. NPF finds depot#1, but depot#2 is closest. YAPF returns depot#2, which is the closest one, but I think it shouldn't have found it as it is too far away.

FindRoadDepot.zip

Something like this on follow_track.hpp would solve the first problem (probably there are better ways of writing it):

 		/* single tram bits cause reversing */
-		if (IsTram() && GetSingleTramBit(m_old_tile) == ReverseDiagDir(m_exitdir)) {
+		if ((IsTram() && GetSingleTramBit(m_old_tile) == ReverseDiagDir(m_exitdir)) ||
+				(IsRoadTT() && IsStandardRoadStopTile(m_old_tile) && GetRoadStopDir(m_old_tile) == ReverseDiagDir(m_exitdir))) {
 			/* reverse */
 			m_new_tile = m_old_tile;
 			m_new_td_bits = TrackdirToTrackdirBits(ReverseTrackdir(m_old_td));
 			m_exitdir = ReverseDiagDir(m_exitdir);

The other two issues should be considered as well. I have a patch for the max. distance problem and I will also look for a solution for the two-way problem with NPF.

@andythenorth

This comment has been minimized.

Copy link
Contributor

@andythenorth andythenorth commented Jan 3, 2019

This would be a nice one to fix 👍

Inability for RVs to find depots is the principal reason I play with breakdowns off. That in turn distorts how I design newgrf vehicles 🤦‍♂️ 🤦‍♀️

Might be interesting to re-run train pathfinder when it needs to find a depot on a PBS route, but eh, different issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.