In the current simutrans code, convoi_t and private_car_t inherits overtaker_t. For road_vehicle_t, can_overtake_function() is called through convoi_t. I think this code structure has some problems.
- Overtaking is the matter of the vehicle movement. The vehicle movements are generally handled by the classes derived from vehicle_base_t, but overtaking is currently handled by convoi_t.
- convoi_t is the class of convoy and it should be way type independent. However, overtaking is a feature only for road vehicles.
- convoi_t and private_car_t do not have a common ancestor class. Due to this, we have to write quite different codes for convoi_t and private_car_t.
I want to remove the overtaking related code from convoi_t and bring them to road_vehicle_t. road_vehicle_t should inherit overtaker_t instead of convoi_t. This makes all subclasses of overtaker_t being derived from vehicle_base_t, and the inheritance relationship of these classes becomes clearer.
If there is no problem for this code refactoring plan, I want to start the implementation. Please tell me your opinions. :)
For the congestion feature in extended, I implemented traffic_vehicle_t which is also inherited by both road_vehicle_t and private_car_t. It would be good to adapt this refactor into extended too so that traffic_vehicle_t can be merged into overtaker_t. This would reduce the amount of duplicate code.
Think about trailer, there can be one or more of it.
It is a convoi that overtake
In case of rail convoys, the leading rail_vehicle_t object of the convoy takes control of convoy movements. Thus, I think the leading road_vehicle_t objects should take the control of overtaking.
I'm afraid but you also have to face with many of the problems that your patch causes in Extended.
Quote from: THLeaderH on July 15, 2020, 08:06:47 AM
In case of rail convoys,
Not rail train. I mentioned, trailers of trucks on the road, there can be also more than one.
Quote from: makie on July 15, 2020, 10:35:41 AM
Not rail train. I mentioned, trailers of trucks on the road, there can be also more than one.
The issue is that there is only one convoi_t class, so overtaker_t is currently inherited by all convoy types, including trains, planes and boats.
Yesterday I tried the implementation to bring can_enter_tile() to road_vehicle_t. However, convoi_t still has to call set_tiles_overtaking() in some places and I could not exclude overtaking related code from convoi_t. Also, I could not reduce the size of can_enter_tile() function with this refactoring. Although road_vehicle_t should inherit overtaker_t instead of convoi_t, the benefits of this refactoring is not so huge. So it seems that we do not have do this refactoring.
The refactoring patch is attached just for reference.
I think this is more logical than the current implementation.
I think this refactoring does not do a harm but does not bring so much benefit, so both accepting and denying this patch is appropriate.