[James Ross]

Roadmap: https://trello.com/c...ify-inheritance
Previous private discussion: http://www.elvastowe...post__p__270537
Proposal:

Quote

We currently have a lot of code spread over "TrainCar", "MSTSWagon", "MSTSLocomotive", and the traction-specific subclasses

To advance the simulation, and particularly support non-MSTS formats, we need to factor out all the core code (e.g., steam traction) into non-MSTS classes

This will allow all the subclasses under "TrainCar" to be replaced by composition of classes, like how diesel and electric power supplies currently work

The affected files are:

• Source\Orts.Simulation\Simulation\RollingStocks\TrainCar.cs
  
• Source\Orts.Simulation\Simulation\RollingStocks\MSTSWagon.cs
  
• Source\Orts.Simulation\Simulation\RollingStocks\MSTSLocomotive.cs
  
• Source\Orts.Simulation\Simulation\RollingStocks\MSTSDieselLocomotive.cs
  
• Source\Orts.Simulation\Simulation\RollingStocks\MSTSElectricLocomotive.cs
  
• Source\Orts.Simulation\Simulation\RollingStocks\MSTSSteamLocomotive.cs

To set the stage before this work begins, here are the top 10 source files (by lines of code in the Testing Version) with those six highlighted:

--------------------------------------------------------------------------------------------
Language                              Files     Lines   Blanks  Comments     Code Complexity
--------------------------------------------------------------------------------------------
C#                                      671    326448    38044     49024   239380      42246
--------------------------------------------------------------------------------------------
~rts.Simulation\Simulation\Physics\Train.cs     21431     3089      3097    15245       4094
~imulation\Simulation\Timetables\TTTrain.cs     15181     1932      1741    11508       3016
~imulation\Simulation\Signalling\Signals.cs     15294     2385      2005    10904       2669
~ation\RollingStocks\MSTSSteamLocomotive.cs      7681      963       857     5861       1015 <--
Orts.Simulation\Simulation\AIs\AITrain.cs        7033      779       738     5516       1639
~Simulation\RollingStocks\MSTSLocomotive.cs      5416      563       428     4425       1221 <--
~wer3D\RollingStock\MSTSLocomotiveViewer.cs      3762       81        66     3615        133
Orts.Simulation\MultiPlayer\Message.cs           3889      217       129     3543        542
~tion\Simulation\RollingStocks\MSTSWagon.cs      4363      526       532     3305        724 <--
~\Simulation\Timetables\ProcessTimetable.cs      4081      538       476     3067        485
Orts.Parsers.Msts\STFReader.cs                   3861      269      1036     2556        541
~ation\Simulation\RollingStocks\TrainCar.cs      3412      487       457     2468        578 <--
Tests\Orts.Parsers.Msts\StfReader.cs             2879      255       255     2369          8
RunActivity\Viewer3D\Cameras.cs                  2736      281       225     2230        476
Menu\Options.Designer.cs                         2679        6       506     2167          3
...
~tion\RollingStocks\MSTSDieselLocomotive.cs      1346      182       171      993        243 <--
~on\RollingStocks\MSTSElectricLocomotive.cs       463       61        85      317         55 <--

As you can see, four of the six are in the top 10 - and the two which aren't ("MSTSDieselLocomotive" and "MSTSElectricLocomotive") are the ones we have already spent a lot of effort on refactoring into smaller components.

The plan is to refactor the six files, continuing the good work for diesel and electric locomotives, to the point that it is possible to combine them in the following order (most likely):

• "MSTSElectricLocomotive" + "MSTSDieselLocomotive" + "MSTSSteamLocomotive" --> "MSTSLocomotive"
  
• "MSTSLocomotive" --> "MSTSWagon"
  
• "MSTSWagon" --> "TrainCar"

Throughout this process (first refactoring, then simplifying), train behaviour and performance will be tested before and after each change to ensure that no unexpected changes occur, using the replay function.

Although the work will be processed in small chunks (including PRs and getting merged) to reduce the chance of conflicts or other bad interactions with other people's work, if anyone is considering or working on any other large changes to any of these six files, please let me know so we can coordinate to avoid big problems.

[steamer_ctn]

James Ross, on 02 March 2022 - 11:20 AM, said:

The plan is to refactor the six files, continuing the good work for diesel and electric locomotives, to the point that it is possible to combine them in the following order (most likely):

• "MSTSElectricLocomotive" + "MSTSDieselLocomotive" + "MSTSSteamLocomotive" --> "MSTSLocomotive"
• "MSTSLocomotive" --> "MSTSWagon"
• "MSTSWagon" --> "TrainCar"

How would a car with a control only Cab be included in this architecture? It will not have any power, etc, but should "inherit" all the potential controls of a locomotive (depending upon the type of locomotive that it is paired with), so that the user can select what controls will operate from the Cab.

[Laci1959]

Quote

The plan is to refactor the six files, continuing the good work for diesel and electric locomotives, to the point that it is possible to combine them in the following order (most likely):

"MSTSElectricLocomotive" + "MSTSDieselLocomotive" + "MSTSSteamLocomotive" --> "MSTSLocomotive"
"MSTSLocomotive" --> "MSTSWagon"
"MSTSWagon" --> "TrainCar"

Helló.

Hogyan kerülnek ebbe az olyan speciális járművek mint a vonat energia ellátására áramot fejlesztő kocsik? Jelenleg az MSTS-ből örökölt megoldást használjuk annak minden hátrányával.

Tisztelettel Laci 1959

[Csantucci]

Hi James,
the code of this blueprint https://blueprints.l...et/or/+spec/eot , which is ready and running for ORNYMG and which I intend to insert in an official PR if and when the blueprint is approved, defines the EOT as a subclass of MSTSWagon.

[gpz]

James Ross, on 02 March 2022 - 11:20 AM, said:

The plan is to refactor the six files, continuing the good work for diesel and electric locomotives, to the point that it is possible to combine them

This is a very welcome effort, I can say!

[James Ross]

steamer_ctn, on 02 March 2022 - 10:20 PM, said:

How would a car with a control only Cab be included in this architecture? It will not have any power, etc, but should "inherit" all the potential controls of a locomotive (depending upon the type of locomotive that it is paired with), so that the user can select what controls will operate from the Cab.

The same way all the other train car types will be supported: by delegating the functions to other classes, the choice of which is made during loading (and other key events).

At a very high level, a control car would have the same classes for cabs and control input as a normal locomotive, but would have the same traction classes as an unpowered wagon.

The details of exactly which new classes we will have and how the code for different features will be moved is not going to be known until I have actually looked at all the relevant bits of code.

Pull request #611 adds the current architecture of the "TrainCar" and related classes, and subsequent PRs to change the architecture (by moving code) will update that documentation.

Laci1959, on 02 March 2022 - 11:33 PM, said:

Hogyan kerülnek ebbe az olyan speciális járművek mint a vonat energia ellátására áramot fejlesztő kocsik? Jelenleg az MSTS-ből örökölt megoldást használjuk annak minden hátrányával.

It will make it simpler to add new special vehicles and improve the code of the ones we already have.

Combined with some articulated locomotive work, many more options will be possible without MSTS-style hacks.

Csantucci, on 03 March 2022 - 01:23 AM, said:

the code of this blueprint https://blueprints.l...et/or/+spec/eot , which is ready and running for ORNYMG and which I intend to insert in an official PR if and when the blueprint is approved, defines the EOT as a subclass of MSTSWagon.

Thanks for the heads-up, you should proceed normally with this work. It is unlikely I will be in a position to make any significant changes to MSTSWagon before your PR is merged.

I've also approved the blueprint :)