Thank you for your remarks, Zuu.
The answers I can give so far:
I wonder though, why adding a new OT_WAITING?
I knew that this was going to be, eventually, the most discussed point. It's a quite a change. I'll try to explain my reasons for this choice.
As I understand it, stations don't have a separate OT_* enum value for when to wait at the station. So why do this for depots?
Actually, they do have specific orders for stations. When a train stops in station, it triggers some order-type changing. Specifically: when arriving, order type becomes OT_LOADING and after that, when the train is going to leave, it becomes OT_LEAVESTATION. So, internally, what looks as a single goto station order actually changes in at least 3 order-types during its processing (all this orders are listed in order_base.h, l. 37).
A lot of my choices were mainly guided exactly by what you mention as important: trying to make the in-depot behaviour as consistent as possible with what happens in stations. Some other choices also results from that. The HandleWaiting() function, i.e. (in vehicle.cpp), is largely inspired from the HandleLoading() function.
Is it not enough that the depot order has the depot flag set to ODATF_SERVICE_ONLY which I assume correspond to "go always to depot (but not stop there)", and set order->wait_time to a value > 0 for you as condition on when to wait?
Well. Actually, when I decided to introduce the OT_WAITING order, I had the feeling that this was necessary to make things properly. Now that I worked a lot more on the code, and that I better understand it, I think it is probably possible to make the thing without it. I'm trying to do that right at the moment and I'll be able to answer you more consistently.
But I also already see that avoiding this order seems to bring to some inconsistencies. A lot of tests can be better achieved if there is actually a clear way to check whether the train is waiting or not (example: when processing orders; when recalculating the arrival/departure board; also in the skip orders function). My thought also was that a such kind of order could be interesting to be used in a lot of cases other than depots (waiting at Waypoints ?), thus useful for future development.
You can find the OpenTTD coding style documented here: https://wiki.openttd.org/Coding_style
. You have spaces as indentation and deviate from how multi-line comments should be made. Also if-statements with code on the next line should use curly brackets. This is all explained in the wiki. Your code is already readable as such, but before going into trunk it need to follow the coding style. Though this is less important than how you actually solved things in your patch.
Yes, I tried to comply the best but obviously, I read that part a little bit to fast. Will fix that.