The International Simutrans Forum

 

Author Topic: Code refactoring of road vehicle overtaking  (Read 927 times)

0 Members and 1 Guest are viewing this topic.

Offline THLeaderH

  • Coder/patcher
  • Devotee
  • *
  • Posts: 424
  • Languages: JP,EN
Code refactoring of road vehicle overtaking
« on: July 15, 2020, 05:56:18 AM »
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.  :)

Offline freddyhayward

  • Devotee
  • *
  • Posts: 681
  • Languages: EN
Re: Code refactoring of road vehicle overtaking
« Reply #1 on: July 15, 2020, 06:42:04 AM »
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.

Online makie

  • Devotee
  • *
  • Posts: 313
    • Homepage PAK128-German
  • Languages: DE
Re: Code refactoring of road vehicle overtaking
« Reply #2 on: July 15, 2020, 07:42:13 AM »
Think about trailer, there can be one or more of it.
It is a  convoi that overtake

Offline THLeaderH

  • Coder/patcher
  • Devotee
  • *
  • Posts: 424
  • Languages: JP,EN
Re: Code refactoring of road vehicle overtaking
« Reply #3 on: July 15, 2020, 08:06:47 AM »
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.

Offline Ranran

  • Devotee
  • *
  • Posts: 1548
  • Languages: ja
Re: Code refactoring of road vehicle overtaking
« Reply #4 on: July 15, 2020, 08:28:48 AM »
I'm afraid but you also have to face with many of the problems that your patch causes in Extended.

Online makie

  • Devotee
  • *
  • Posts: 313
    • Homepage PAK128-German
  • Languages: DE
Re: Code refactoring of road vehicle overtaking
« Reply #5 on: July 15, 2020, 10:35:41 AM »
In case of rail convoys,
Not rail train. I mentioned, trailers of trucks on the road, there can be also more than one.

Offline freddyhayward

  • Devotee
  • *
  • Posts: 681
  • Languages: EN
Re: Code refactoring of road vehicle overtaking
« Reply #6 on: July 15, 2020, 10:58:22 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.

Offline THLeaderH

  • Coder/patcher
  • Devotee
  • *
  • Posts: 424
  • Languages: JP,EN
Re: Code refactoring of road vehicle overtaking
« Reply #7 on: July 16, 2020, 07:14:09 AM »
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.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10670
  • Languages: De,EN,JP
Re: Code refactoring of road vehicle overtaking
« Reply #8 on: July 16, 2020, 07:25:41 AM »
I think this is more logical than the current implementation.

Offline THLeaderH

  • Coder/patcher
  • Devotee
  • *
  • Posts: 424
  • Languages: JP,EN
Re: Code refactoring of road vehicle overtaking
« Reply #9 on: July 16, 2020, 07:37:48 AM »
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.
« Last Edit: July 17, 2020, 04:53:27 AM by THLeaderH »