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

AIVehicle::SendVehicleToDepot behaves not as described #2801

Closed
DorpsGek opened this issue Apr 3, 2009 · 8 comments
Closed

AIVehicle::SendVehicleToDepot behaves not as described #2801

DorpsGek opened this issue Apr 3, 2009 · 8 comments
Labels
component: AI/Game script (squirrel) This issue is related to Squirrel (Scripting language) flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Apr 3, 2009

bilbo opened the ticket and wrote:

AIVehicle.StartStopVehicle does not "Sends the given vehicle to a depot." as the documentation says. It behaves more like the GUI button "send to depot" - first click/call sends to depot, second click/call cancels that order.

If the vehicle is already en-route to the depot, this command will cancel that order.

This is expecially bad if you want to send vehicle to depot but you don't know its state (like when loading from a savegame), as another call can cancel that order instead. And there is no function to check if the vehicle is already heading to a depot.

I think there are two solutions:

* Keep current behavior, but update documentation and add function that will check if vehicle is already en-route to depot, so you can examine the vehicle before calling the function.
* Split functionality to two explicit calls (send to depot, cancel order to send the depot) instead of current "toggle state of being sent to depot" call. Check for the state may be nice in this case too, but I think I can live without it :)

Also, there is missing function to send to depot only for servicing (available to humans when ctrl-clicking the depot button in gui). I think it can be useful to add to AI's, as sending vehicles explicitly for servicing can be useful - I use it sometimes to clear traffic jams if the road gets congested and the vehicles get stuck .... this little push if often enough to clear the jam unless it is really large.

Reported version: trunk
Operating system: All


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

Yexo wrote:

Proposal to fix this (and some related functions). Please comment if you miss any order-related functions.

New functions:

AIOrder::IsGotoStationOrder() <- returns true if a vehicle is going to a station/dock/airport
AIOrder::IsGotoDepotOrder() <- returns true if a vehicle is going to a depot/hangar
AIOrder::IsCurrentOrderPartOfOrderList() <- returns true if the current order is part of the order list.
AIVehicle::SendVehicleToDepotForServicing() <- Send a vehicle to a depot for servicing only

Functions to change:

GetOrderTypeByTile (in ai_order.cpp) <- should return OT_GOTO_DEPOT for airport hangar tiles
AIVehicle::SendVehicleToDepot() <- documentation should indicate that the order will be toggled instead of always set to goto depot


This comment was imported from FlySpray: https://bugs.openttd.org/task/2801#comment5939

@DorpsGek
Copy link
Member Author

bilbo wrote:

So code to "reliably" send vehicle to depot would look something like this?

if (AIOrder.IsGotoDepotOrder(VehicleID) && !AIOrder.IsCurrentOrderPartOfOrderList(VehicleID)) {
// Do nothing, vehicle was already sent to depot earlier
} else {
// Send it to the depot
AIVehicle.SendVehicleToDepot(vehicleID)
}

Your proposal is missing information about parameters ....


This comment was imported from FlySpray: https://bugs.openttd.org/task/2801#comment5940

@DorpsGek
Copy link
Member Author

frosch wrote:

The IsGoto...Order() are of course like IsConditionalOrder() :) So you would have to add an ORDER_CURRENT.


This comment was imported from FlySpray: https://bugs.openttd.org/task/2801#comment5942

@DorpsGek
Copy link
Member Author

Yexo wrote:

Ok, once more to prevent problems with missing parameters (I assumed those where clear):

AIOrder::IsGotoStationOrder(VehicleID, OrderPosition)
AIOrder::IsGotoDepotOrder(VehicleID, OrderPosition)
AIOrder::IsCurrentOrderPartOfOrderList(VehicleID)
AIVehicle::SendVehicleToDepotForServicing(VehicleID)


This comment was imported from FlySpray: https://bugs.openttd.org/task/2801#comment5943

@DorpsGek
Copy link
Member Author

bilbo wrote:

So the code to reliably send to depot would be:

if (AIOrder.IsGotoDepotOrder(VehicleID,AIOrder.ORDER_CURRENT) && !AIOrder.IsCurrentOrderPartOfOrderList(VehicleID)) {
// Do nothing, vehicle was already sent to depot earlier
} else {
// Send it to the depot
AIVehicle.SendVehicleToDepot(vehicleID)
}

I guess that's fine to fix it that way, though I think it would be slightly better to update SendVehicleToDepot() to match the documentation instead of updating the documentation to match the toggling reality :)


This comment was imported from FlySpray: https://bugs.openttd.org/task/2801#comment5947

@DorpsGek
Copy link
Member Author

Yexo wrote:

The problem with changing the behavior is that it'll then differ between trunk and 0.7.0, which is a problem at this point in time.


This comment was imported from FlySpray: https://bugs.openttd.org/task/2801#comment5948

@DorpsGek
Copy link
Member Author

frosch wrote:

Also it is consistent with StartStop this way :)


This comment was imported from FlySpray: https://bugs.openttd.org/task/2801#comment5949

@DorpsGek
Copy link
Member Author

Yexo closed the ticket.

Reason for closing: Implemented

In r16165.


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

@DorpsGek DorpsGek added flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) bug component: AI/Game script (squirrel) This issue is related to Squirrel (Scripting language) labels Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: AI/Game script (squirrel) This issue is related to Squirrel (Scripting language) flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Development

No branches or pull requests

1 participant