[gpz]

For the comment in the code, actually the previous line does the job:

if (forsight >= list.Count) SearchTrainInfo(forsight, type);
// <CSComment> Doesn't : list.Count - 1 lead to wrong returns if forsight > 0? </CSComment>
return list[forsight < list.Count ? forsight : list.Count - 1];

The SearchTrainInfo() function extends the list to the requested "forsight". That last line is just a protection, if a too big "forsight" number was asked for, it still returns some value. I didn't want to give back a null value, since the script writer is not expected to always do null-checks on interface functions, it would be a valid expectation from him, that these functions always return something. You may be right, that it was not the best choice to just repeat the last signal (most probably a STOP) to any bigger numbers, there could have been a dummy signal object defined with aspect none, speed limit float.max, etc., and return that one. Yes, this might be a valid change.

In my code I am about to change the TrainInfo object to something like

        Train.TrainInfo TrainInfo {
            get 
            {
                if (!TrainInfoUpdatedFlag) _trainInfo = Locomotive.Train.GetTrainInfo();
                TrainInfoUpdatedFlag = true;
                return _trainInfo;
            }
        }
        Train.TrainInfo _trainInfo = new Train.TrainInfo();
        bool TrainInfoUpdatedFlag;

where the TrainInfoUpdatedFlag = false can be set in the ClearParams() function, so this way the TrainInfo is updated only once per Update() call, but for sure gets updated at the first use.

I am also about to add the collecting of the generic signals and the trailing switches to the TrainInfo. If we want long-term maintainability, or make our successor developers to understand what we've done, that is the correct solution I think. It also makes the code more clear, since the current functions are mainly just copies from the TrainInfo with slightly modified functionality. (Anyway, they are well coded functions, imho.) TrainInfo is there mainly for feeding the MSTS-style track monitor, and e.g. the ETCS track monitor is just a more advanced track monitor. So these additions naturally belong there. In long term the OR track monitor probably should also be updated to have the missing functions that other TCS system track monitors have.

[gpz]

Just a note about my original code: Probably my original SearchTrainInfo() function is also overcomplicated. I was coded it to build other List()-s of different type of objects based on hits in TrainInfo. But it is absolutely unnecessary. The simplest way is just returning directly the found TrainInfo member's properties, without additional magic... TrainInfo is the master database, that (should) contain all the necessary data. I must add the altitudes there as well...

[Csantucci]

I have introduced some of the suggestions by Peter about TrainInfo into rev. 75 of OR NewYear MG.

[YoRyan]

So, what's the status on these proposed changes? Are we doing anything to the Locomotive handle? Are we going to upstream the TrainInfo improvements?

[gpz]

I've finished with 80-90% of the coding, but only 5-10% of the testing... One may take a look on the code here on Github, but it is not worth tying to download it yet, currently it is not even in a compilable state. I am yet to figure out how to query the altitudes of different signalling elements. (I'm sorry, I have less time for coding than it would be ideal.)

The to-be-PR of this code will not contain the removal of the Locomotive handle to allow people to make the transition smooth, but that handle must be removed not later than just before the unstable-testing branch goes stable.

[Csantucci]

Peter,
Have you checked that your revised version handles the usage of all locomotive class items actually used by the TCS developers?
Apart from the work needed because of the deletion of the Locomotive handle, does your revision require other changes to the TCS scripts?

[gpz]

Hi Carlo,

By my intention it handles all classes, and I tried to cover all the use cases that came up in this thread, and that I am aware of, if I hadn't forgotten something. There's a list of variables I thought to add in the first round as built-ins at the beginning of this file. I don't think most of the existing ones need to be removed, even those I tried to give a cleaner alternative for here, because beyond the Locomotive() one neither of them makes any harm. When it will be ready, and before merge, actually a feedback from people, especially from ones familiar with scripting of other simulators, would be highly appreciated, to tell whether this approach is any better than the previous one I started years ago. Eventually it should be good enough to support scripting of not only the TCS, but also of other fancy features of a locomotive.

[cesarbl]

I am so excited with this feature!! This will finally allow me to create scripts to have more realistic controls (customized brake blending, cruise speed...) If this is approved, I think people looking for a realistic driving experience will come to OR.

With respect to signal altitudes, I only mentioned them as an example of weird usage of OR internal structures. My ETCS is still experimental and I haven't released it, so they are not a problem. I used them to show that ETCS can be used in OR with full functionality, but I don't mind if I have to set gradients to zero for a while after the Locomotive() hook is removed.

I have looked at the interface and it suits my needs, so as soon as it is merged I will remove all Locomotive() references in my script.

Some suggestions: It might be worth adding this variables: line voltage (for use by the circuit breaker, not the one shown in cabview), "TCSEmergency" and "TCSFullService". Also, save/restore functionality would be interesting for controller scripts.

[Csantucci]

I attach here the script lines where I use the Locomotive() hook. I wonder if they are all covered with this new interface.
 UseOfLocomotive.txt (1.77K)
Number of downloads: 484
Anyhow, as already written, I'll keep the hook in OR NewYear MG and am still not convinced of the need to remove it.

[Csantucci]

As a compromise I propose to do as Cédric has done with SetCustomizedTCSControlString.
When a call to Locomotive() is executed, a log line is generated, stating "Locomotive() is deprecated". Of course this should be generated only at the first call, to avoid flooding the logfile.

[Csantucci]

I've just inserted a small modification to the TCS code, increasing to 48 the maximum amount of generic TCS commands available. The modification should now be present in the Unstable release. Trellobox https://trello.com/c...of-tcs-commands .

[gpz]

Csantucci, on 15 October 2020 - 11:12 AM, said:

I attach here the script lines where I use the Locomotive() hook. I wonder if they are all covered with this new interface.
UseOfLocomotive.txt
Anyhow, as already written, I'll keep the hook in OR NewYear MG and am still not convinced of the need to remove it.

As I see, your uses are not even for the Locomotive class itself, but jumping over to the Train class. Most of them are for finding out some signalling information that the current interface doesn't cover, many of them going into deep querying into the Track Circuit Sections, using also the internal route data. None of these have any reasonable right to exist in a script. Something such programmed is no longer a "script", rather it is an internal part of the OpenRails. But the code is spreaded to 100-1000 places, so noone is able to fix them if something changes inside the OpenRails internal code. You are not building this script for the future, rather just for the near present. Do you think this "script" will remain compatible in 2 years timeframe? What about 10 years timeframe? Or more...

You must also be aware of that the Train class became such a mess over the time, so it is the primary subject of a refactoring and splitting up to 5-10 pieces. How can you reason with an argument that you don't foresee any changes here in the future? Anyway, nobody can calculate the exact probability of a change, especially given that a change anywhere can potentially break the script outsourced internal code. But the exact probability doesn't even matter, because there IS a probability, despite of that no technical aspect denies us creating an interface with ZERO probability. By the way, this is the basic principle of a scripting interface: The interface functions must be designed in a way that they are guaranteed to remain compatible over time. Let me quote your own words again from the first page of this thread:

Csantucci, on 04 February 2020 - 07:24 AM, said:

These hooks can't create problems neither to the scripts nor to the OR code, because (...) if the (...) setup will be changed, the since long time existing (...).cs script won't be changed, and the (...).cs script I'm publishing won't be changed. What will have to be changed is the code of the script hooks in TrainControlSystem.cs, but this really is a minor task in redoing the (...) setup: it's enough to see how few code lines are used for such script setup.

Just think about your own reasoning, in contrast how the current "interface" looks like.

[Csantucci]

Peter,
the concept I expressed on Februarh 4th is evidently correct: using hooks hides what is behind them. However between February 4th and March 31st, which is the date where I created the Locomotive() handle, I was confronted with so many needs (by myself and others) to add new, sometimes quite particular, hooks, that I considered more important not to fill the code with a plethora of hooks, some of them used only in particular cases.
By the way I don't permit myself to say that parts of the OR code are a mess; by the way I'm not eager to spend my time in refactoring working things and prefer to write new features. Apparently I have a different character than you.
Anyhow I see that here I am in minority on my position, and therefore I made the proposal to generate the deprecate log line.
I also generated and tested a patch with the additional handles I need to avoid the Locomotive() handle in the TCS scripts I wrote, and here it is
 New_TCS_handles.zip (2.98K)
Number of downloads: 504
The most part of the additional handles is related to signals, and they are all needed. I have the intention to add these to the Unstable release and they should remain also after your refactoring, minor changes apart if reasonable.
As already said, Locomotive() will anyhow remain in OR NewYear MG.

[gpz]

Csantucci, on 18 October 2020 - 10:21 AM, said:

the concept I expressed on Februarh 4th is evidently correct: using hooks hides what is behind them. However between February 4th and March 31st, which is the date where I created the Locomotive() handle, I was confronted with so many needs (by myself and others) to add new, sometimes quite particular, hooks, that I considered more important not to fill the code with a plethora of hooks, some of them used only in particular cases.

With little thinking the needs can be fulfilled with general functions connecting to the internal databases, and leaving the particularities to the script. A good example of this is the "DoesNextNormalSignalHaveTwoAspects() and the two aspects of each head are STOP and ( CLEAR_2 or CLEAR_1 or RESTRICTING)" function. You should just generally provide the list of available aspects from the interface, and leave the special searching in the list to the script...

Csantucci, on 18 October 2020 - 10:21 AM, said:

By the way I don't permit myself to say that parts of the OR code are a mess; by the way I'm not eager to spend my time in refactoring working things and prefer to write new features. Apparently I have a different character than you.

Why? Nothing dishonored is in that. It is a natural progress, that a code is written in functionality bearing in mind first, and then making it more and more structured, with rewritings and refactorings, mergings. All this is for keeping the long-term maintainability in mind. Preferably even the first published version of a code piece should have at least a little structuring and sensible connection and integration with the rest of the code. For example I very much dislike the style of a new feature programming, where one copies a hundred lines of code to a new function and modifies some lines in it to fulfill his needs. Instead of adding his mods to the original function, even though if he would have to test the original feature for avoiding degradation. IMO actually this copying is the thing that is dishonorable, because this doesn't respect the successor programmers trying to add further features or maintain the code, it is short-sighted. And OpenRails has a lot of places where such code-copies appear. Also there are a lot of other places where several not closely related functions are just grouped to a class (e.g. the Train). Nothing new in this, these types of codes are "mess". Why to be ashamed saying this? Generally saying, not specifically to you, going forward with adding new functions without perceiving this, leads to less and less maintainable code, also passing over the less-enjoyable part of the work to the others. And this is why adding new functions should always be done in a structured, well thought-over way.

Csantucci, on 18 October 2020 - 10:21 AM, said:

As already said, Locomotive() will anyhow remain in OR NewYear MG.

Originally I didn't want to comment on this, but as you are dropping this "bomb" on me multiple times now, it looks like you are awaiting some answer from me. I think this wildcard sentence is the thing that you should not permit yourself, because it actually sounds quite arrogant, because with this you are saying you are not respecting anyone's arguments trying to discuss how OpenRails should progress. Seems to me you use this to weigh down anybody you disagree with, and from the forum history I see that it is also a good tool to deteriorate your relationship with anyone. But keep it in OR NewYear MG, I don't care. (Which btw is at least in 70-80% percent based on my own work. :) ) The future is yours anyway, because you are willing to put more work into OpenRails, than me for example.

[Serana]

On my side, I use the Locomotive function in two locations:


1) In order to get the DriverClosingOrder of the CircuitBreaker, which will be fixed with this future PR: https://github.com/S...-battery-switch
This PR is the addition of the master key, battery switch and other power supply related things.
I still have to make some changes (a renaming of DieselElectricPowerSupply to DieselPowerSupply and some refactoring using generics for the PowerSupply class in order to simplify a bit the power supply classes).
These power supply classes will be really useful when creating scripted pantograph selectors and power limit selectors... and also for dual mode locomotives/multiple units.

2) In order to release the speed limit precisely after a turnout (I am using the FrontTDBTraveller).
I currently don't know how to integrate this in the TCS interface.