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

RV skips "go non-stop to nearest depot" order #4030

Closed
DorpsGek opened this issue Aug 13, 2010 · 3 comments
Closed

RV skips "go non-stop to nearest depot" order #4030

DorpsGek opened this issue Aug 13, 2010 · 3 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

Attila7 opened the ticket and wrote:

There is a bug where an RV will skip an order to go NON-STOP to the nearest depot. This happens if the RV was told to go to a drive-thru station, then to the nearest depot and stop, but upon leaving the station it must turn around at the end of a road. When the RV hits the end of the road, the order list changes to the next order (depot skipped).

The key is that the order must be "non-stop". If the order is just "go to nearest depot", the order will not be skipped.

I watched this for a number of trips of a bus, then I rearranged the roads so the RV could go to the depot without having to turn around and the order was not skipped.

See attached screenshot and savegame file.

Attachments

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/4030
@DorpsGek
Copy link
Member Author

Vitus wrote:

Very strange indeed. Attached savegame for easier testing.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4030#comment8514

@DorpsGek
Copy link
Member Author

Attila7 wrote:

I have done some digging in the code, and this bug seems to be a major performance hit, because of the following code at line 118 in file order_cmd.cpp:

bool Order::Equals(const Order &other) const
{
/* In case of go to nearest depot orders we need "only" compare the flags

  • with the other and not the nearest depot order bit or the actual
  • destination because those get clear/filled in during the order
  • evaluation. If we do not do this the order will continuously be seen as
  • a different order and it will try to find a "nearest depot" every tick. */
    if ((this->type == OT_GOTO_DEPOT && this->type == other.type) &&
    ((this->GetDepotActionType() & ODATFB_NEAREST_DEPOT) != 0 ||
    (other.GetDepotActionType() & ODATFB_NEAREST_DEPOT) != 0)) {
    return
    this->GetDepotOrderType() == other.GetDepotOrderType() &&
    (this->GetDepotActionType() & ~ODATFB_NEAREST_DEPOT) == (other.GetDepotActionType() & ~ODATFB_NEAREST_DEPOT);
    }

return
this->type == other.type &&
this->flags == other.flags &&
this->dest == other.dest;
}

I am not yet sure what the fix is, but what this code is trying to avoid is exactly what is happening if the order is non-stop goto nearest depot. The game tries to find the nearest depot on every tic (calling FindClosestDepot at line 1740 of order_cmd.cpp). When the bus gets to the reversing point, the pathfinder does not find a path to the nearest depot for some reason (another bug). This triggers the increment of the order number at line 1759 of order_cmd.

When the order does not contain "non-stop", the pathfinder is called and it finds the nearest depot as soon as the bus leaves the station and is not called again because the code above works as intended.

The bug in the pathfinder is that it cannot find the nearest depot when the bus is pointed directly South as it is reversing.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4030#comment8515

@DorpsGek
Copy link
Member Author

Rubidium closed the ticket.

Reason for closing: Fixed

In r20498


This comment was imported from FlySpray: https://bugs.openttd.org/task/4030

@DorpsGek DorpsGek added flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) Vehicles labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Development

No branches or pull requests

1 participant