Code refactoring of road vehicle overtaking

[THLeaderH]

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. 

[freddyhayward]

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.

[makie]

Think about trailer, there can be one or more of it.
It is a  convoi that overtake

[THLeaderH]

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.

[Ranran(retired)]

I'm afraid but you also have to face with many of the problems that your patch causes in Extended.

[makie]

In case of rail convoys,

Not rail train. I mentioned, trailers of trucks on the road, there can be also more than one.

[freddyhayward]

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.

[THLeaderH]

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.

[prissi]

I think this is more logical than the current implementation.

[THLeaderH]

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.