[patch] Realistic Train Shunting

[TrueSatan]

Not sure if this was reported before, but I got a crash at my first tries.
Attached the game save starting one train with order one.
Let it run until the third station and it will crash.

Interesting patch. Have I missed the switch to get train reversing in stations back?

Cheers

[Karn]

Thank you for reports. I changed my mind and made a fix 0.8.2 before bigger update because those crashes are more often and serious than it initially looked. Enjoy.

[TrueSatan]

Thanks Karn for the fast fix

My dream is that one day this pacth is in nmf. Well
first things first. More bug hunting to make this patch great (without again haha)

Cheers

[wallyweb]

Thank you for reports. I changed my mind and made a fix 0.8.2 before bigger update because those crashes are more often and serious than it initially looked. Enjoy.

The fix is in!
Everything worked as intended.
Well ... almost ...

No crashes. Good!
The bug now is with graphics.
Bug 1:
1. Decoupled wagons turn gray when left in station.
2. When recoupled, those wagons remain gray.
3. In the depot those wagons show their true colours.
4. When leaving the depot, the wagons are gray again.

Bug 2:
1. On reversing, the articulated head end (steam engine+tender), the steam engine becomes a reversed tender, so the head end becomes (tender+reversed tender).
2. Reversing the reversed train, the head end returns to normal (steam engine+tender).

Re bug 2, I am not familiar with the internals, but I assume the train is laid out in an array and that the engine's cell governs the motion of the train and that when reversing the whole array flips, hence the need to swap graphics in the lead cell. Is this correct?
Is it possible to add some sort of variable (TRAIN_MOTION_DIRECTION ???) that is applied as a property of the array such that direction of motion can be applied without the need to flip the graphics array?

[Karn]

Thank you for reports. I changed my mind and made a fix 0.8.2 before bigger update because those crashes are more often and serious than it initially looked. Enjoy.

The fix is in!
Everything worked as intended.
Well ... almost ...

No crashes. Good!
The bug now is with graphics.
Bug 1:
1. Decoupled wagons turn gray when left in station.
2. When recoupled, those wagons remain gray.
3. In the depot those wagons show their true colours.
4. When leaving the depot, the wagons are gray again.

Bug 2:
1. On reversing, the articulated head end (steam engine+tender), the steam engine becomes a reversed tender, so the head end becomes (tender+reversed tender).
2. Reversing the reversed train, the head end returns to normal (steam engine+tender).

That's NewGRF specific. As I mentioned before, there are many ways to define graphics for articulated vehicles. You found a NewGRF with another way. Can you tell what NewGRF is having those problems with my patch? So then I can fix the NewGRF actions that cause this.

Re bug 2, I am not familiar with the internals, but I assume the train is laid out in an array and that the engine's cell governs the motion of the train and that when reversing the whole array flips, hence the need to swap graphics in the lead cell. Is this correct?
Is it possible to add some sort of variable (TRAIN_MOTION_DIRECTION ???) that is applied as a property of the array such that direction of motion can be applied without the need to flip the graphics array?

It's not array, it's double linked list. Each (part of) vehicle is node of this list and can do all the crazy stuff like checking other vehicles in chain, position in chain and anything you can think of (anything that is in NewGRF specs, there is a lot). Also each vehicle has same available variables, there is no real difference between wagons, articulated parts (if you are not NewGRF author) or engines, they have just variables set to different values. Motion is stored in every vehicle, such as everything important.

What my reversing does is linking whole list backwards, switching directions of each vehicle, swapping important variables between first and last vehicle and swapping variables in articulated parts and then telling each vehicle that it was flipped with VRF_REVERSE_DIRECTION flag. So then the graphical part can try to draw everything flipped. I was thinking about different approaches to flipping, but this one is most error proof and probably easiest to code, it shouldn't cause game crashes or infinite cycles. Downside is to deal with NewGRFs, but every approach to this has problems with NewGRFs afaik.

Train_motion_direction wouldn't work, because when you couple 2 trains of opposite direction, they have to reverse the old way to face the same directions no matter of what is set in Train_motion_direction, which would flip order of wagons during coupling.

[wallyweb]

Bug 2:
1. On reversing, the articulated head end (steam engine+tender), the steam engine becomes a reversed tender, so the head end becomes (tender+reversed tender).
2. Reversing the reversed train, the head end returns to normal (steam engine+tender).

That's NewGRF specific. As I mentioned before, there are many ways to define graphics for articulated vehicles. You found a NewGRF with another way. Can you tell what NewGRF is having those problems with my patch? So then I can fix the NewGRF actions that cause this.

The GRF is Canadian Train Set which was coded by OzTrans who is no longer available to describe his work.
I'll test with some other sets to get some comparisons.
Thanks.

[michael blunck]

GRF is Canadian Train Set which was coded by OzTrans [...]

There are ancient ways to code articulated vehicles, which coders had to use e.g. before implementation of "position within articulated vehicle" (var 4D).

regards
Michael

[Snail]

Ok, so I've found another bug (or, at least, an undesirable patch behavior). Let me precise I'm still using version 0.8 .

Background: my set uses the "property" callback (CB36) a lot, as well as the "recoloring" callback, in order to change properties (capacity, length, weight etc), recoloring scheme, and graphics of the wagons, according to the engine they're attached to.
It looks like this structure is not very well compatible with this otherwise wonderful patch.

My actions to reproduce this bug, and its effects, are the following:

1.
I create a consist pulled by a specific steamer, with three different types of wagons (a mail van, a mixed mail/passenger coach, and a passenger coach).
When pulled by this steamer, the first vehicle I mentioned will have its length shrunk from 4/8 to 3/8; the second vehicle, which is actually articulated and made of 3 parts, will be stretched from 4/8 to 6/8 length. All the vehicles will see their properties slightly change, in terms of capacity for example, as well as their graphics and recoloring (they will be recolored in brown, while a default wagon is recolored in green).

This is the initial stage, before reversing:

Normal train running forward

bug 20180718 0.png (17.64 KiB) Viewed 8639 times

2.
When this consist reaches the end of line, the patch reverses it. However, perhaps due to the changes in length occurred through CB36, I'm getting this error message (I actually get it twice, most likely because there are two vehicles -the mail van and the mixed coach- whose lengths changed):

Error messages...

bug 20180718 1.png (20.08 KiB) Viewed 8639 times

3.
Once the consist runs in reverse, all the changes to the coaches I made through CB36 are lost! Each coach reverts back to its default state. Even when the train reaches the depot, the in-depot view remains wrong.

Consist after reversing...

bug 20180718 2.png (12.18 KiB) Viewed 8639 times

It looks like that, when reversing a train, this patch flips all vehicles, so that the original engine is not the first vehicle in the consist anymore; therefore, CB36 and the other callbacks will lose their hook (I technically use the "engine()" syntax in m4nfo to link to the engine).

This is the same problem I saw when decoupling. Perhaps we should find a way for each vehicle to keep its characteristics as of when it left the depot, even after reversing or decoupling. I know that probably, if I avoided using CB36 like this, the issues would go away, but that's not really realistic (I couldn't achieve half of what I want without it).


Karn, please feel free to ask me any further questions if you need extra info. I could even send you the GRF I used for this.

[wallyweb]

Bug 2:
1. On reversing, the articulated head end (steam engine+tender), the steam engine becomes a reversed tender, so the head end becomes (tender+reversed tender).
2. Reversing the reversed train, the head end returns to normal (steam engine+tender).

That's NewGRF specific. As I mentioned before, there are many ways to define graphics for articulated vehicles. You found a NewGRF with another way. Can you tell what NewGRF is having those problems with my patch? So then I can fix the NewGRF actions that cause this.

The GRF is Canadian Train Set which was coded by OzTrans who is no longer available to describe his work.
I'll test with some other sets to get some comparisons.
Thanks.

I switched to NARS which graphically is the closest to the Canadian train set and it worked perfectly.

[Leanden]

Ok, so I've found another bug (or, at least, an undesirable patch behavior). Let me precise I'm still using version 0.8 .

Background: my set uses the "property" callback (CB36) a lot, as well as the "recoloring" callback, in order to change properties (capacity, length, weight etc), recoloring scheme, and graphics of the wagons, according to the engine they're attached to.
It looks like this structure is not very well compatible with this otherwise wonderful patch.

My actions to reproduce this bug, and its effects, are the following:

1.
I create a consist pulled by a specific steamer, with three different types of wagons (a mail van, a mixed mail/passenger coach, and a passenger coach).
When pulled by this steamer, the first vehicle I mentioned will have its length shrunk from 4/8 to 3/8; the second vehicle, which is actually articulated and made of 3 parts, will be stretched from 4/8 to 6/8 length. All the vehicles will see their properties slightly change, in terms of capacity for example, as well as their graphics and recoloring (they will be recolored in brown, while a default wagon is recolored in green).

This is the initial stage, before reversing:
bug 20180718 0.png

2.
When this consist reaches the end of line, the patch reverses it. However, perhaps due to the changes in length occurred through CB36, I'm getting this error message (I actually get it twice, most likely because there are two vehicles -the mail van and the mixed coach- whose lengths changed):
bug 20180718 1.png

3.
Once the consist runs in reverse, all the changes to the coaches I made through CB36 are lost! Each coach reverts back to its default state. Even when the train reaches the depot, the in-depot view remains wrong.
bug 20180718 2.png

It looks like that, when reversing a train, this patch flips all vehicles, so that the original engine is not the first vehicle in the consist anymore; therefore, CB36 and the other callbacks will lose their hook (I technically use the "engine()" syntax in m4nfo to link to the engine).

This is the same problem I saw when decoupling. Perhaps we should find a way for each vehicle to keep its characteristics as of when it left the depot, even after reversing or decoupling. I know that probably, if I avoided using CB36 like this, the issues would go away, but that's not really realistic (I couldn't achieve half of what I want without it).


Karn, please feel free to ask me any further questions if you need extra info. I could even send you the GRF I used for this.

Has anyone tested with BRTrains. In that set each “wagon” is individually coded for livery. I would assume that in that case the details of the vehicles are maintained?

[wallyweb]

I have a question of curiosity. I don't use timetables or scheduling in my games, but I do agree that for many players these are very important.
I am very happy with shunting as it is now, so the question is do timetables and scheduling need to be integrated into shunting? Or, without integration, does shunting break timetables and scheduling? As I mentioned, I do not use timetables and scheduling, so I really do not know. Are you adding a layer of complexity that might not be needed?

[ic111]

The key idea of using timetables is, that for every train movement, I specify when the departure and arrival is supposed to be. This way, I can synchronize different trains. E.g., on a one-track-line, I can specify that two trains meet at a defined time at the only two-track-station in the middle of the line, without any need that one of the train has to wait long for the other (just because they both are supposed to be at that place at an appropriate time).

What does this mean with regard to shunting?

IMHO, basically in such a game I would use shunting only, if I can specify when it happens. Because otherwise, it would just disturb my well-planned timetables, and as described above, the current possibilities in OpenTTD to recover from disturbed timetables are pretty limited; so in such a game my primary aim is that my timetables don't get disturbed in the first place.

How much code I would have to write in order to make this work with my timetables patch? I don't know at the moment; summer always is a season when my time for OpenTTD is quite limited... (BTW, this is also the main reason why I didn't make any effort to migrate my own patches (timetables, rivers) to Github yet).

If I would have time for such a project, maybe I would try to integrate shunting with my timetable patch and improved orders (as described above). E.g., my feeling is that if you start with shunting, maybe allowing a broader range of destinations for orders (not only stations and waypoints with straigh tracks without crossings) might be desirable as well. Wether I will have time for such a project is an open question, unfortunately...

[wallyweb]

...

I can see your reasoning ...
1. Decouple a train into two or more destinations (the decoupled trains would have to inherit power units and revised timetables)
2. Couple trains from two or more destinations (the coupled trains would have to surrender power units and inherit revised timetables)
A possible solution might be timetable blocks:
Block 1: point of origin to point of shunt
Block 2: point of shunt to point of destination
Can the Block changes be triggered by the shunt?
3. Couple (pick up) and decouple (drop off) wagons while in transit between destinations (power units and timetable revisions not involved )

Enjoy your summer and remember the first rule of summer ... Dreaming of solutions other than the appropriate sun block is forbidden.

[Eddi]

Ok, so I've found another bug (or, at least, an undesirable patch behavior). Let me precise I'm still using version 0.8 .

Background: my set uses the "property" callback (CB36) a lot, as well as the "recoloring" callback, in order to change properties (capacity, length, weight etc), recoloring scheme, and graphics of the wagons, according to the engine they're attached to.
It looks like this structure is not very well compatible with this otherwise wonderful patch.

this is exactly what i was talking about earlier in the thread (or maybe the previous thread), the use of CB36 is essentially incompatible with any shunting ability.

there are basically 2 ways out of this:

1. Disable shunting if CB36 indicates a change in capacity. this is not very desirable, but at least the game won't break.
2. somehow make a "ghost" version of the original front engine, and allow NewGRFs to access that. this will get complicated very quickly, like what happens if the front engine gets sold while the wagons are still out there somewhere?

[Snail]

Ok, so I've found another bug (or, at least, an undesirable patch behavior). Let me precise I'm still using version 0.8 .

Background: my set uses the "property" callback (CB36) a lot, as well as the "recoloring" callback, in order to change properties (capacity, length, weight etc), recoloring scheme, and graphics of the wagons, according to the engine they're attached to.
It looks like this structure is not very well compatible with this otherwise wonderful patch.

this is exactly what i was talking about earlier in the thread (or maybe the previous thread), the use of CB36 is essentially incompatible with any shunting ability.

there are basically 2 ways out of this:

1. Disable shunting if CB36 indicates a change in capacity. this is not very desirable, but at least the game won't break.
2. somehow make a "ghost" version of the original front engine, and allow NewGRFs to access that. this will get complicated very quickly, like what happens if the front engine gets sold while the wagons are still out there somewhere?

My point here was, that this doesn't only affect shunting. It also affects reversing, because the order of the vehicles is swapped, even if the original engine is still present in the consist.

[Karn]

Ok, so I've found another bug (or, at least, an undesirable patch behavior). Let me precise I'm still using version 0.8 .

Background: my set uses the "property" callback (CB36) a lot, as well as the "recoloring" callback, in order to change properties (capacity, length, weight etc), recoloring scheme, and graphics of the wagons, according to the engine they're attached to.
It looks like this structure is not very well compatible with this otherwise wonderful patch.

this is exactly what i was talking about earlier in the thread (or maybe the previous thread), the use of CB36 is essentially incompatible with any shunting ability.

there are basically 2 ways out of this:

1. Disable shunting if CB36 indicates a change in capacity. this is not very desirable, but at least the game won't break.
2. somehow make a "ghost" version of the original front engine, and allow NewGRFs to access that. this will get complicated very quickly, like what happens if the front engine gets sold while the wagons are still out there somewhere?

There are always more options. There is c) Update CB36 or part of it less often (only in depot arranging?) and move the part not compatible with shunting from cache to permanent variable.

[michael blunck]

[...] what happens if the front engine gets sold while the wagons are still out there somewhere?

This problem exists already now, without that shunting feature, for wagons in depot.

regards
Michael

[Eddi]

There is c) Update CB36 or part of it less often (only in depot arranging?) and move the part not compatible with shunting from cache to permanent variable.

This is not an option, because CB36 is also evaluated on loading a savegame, and it must at that point always return the same values as previously in the Depot. If you want to change that, you have to dig through a whole lot of cached values, and put them in the savegame.

[...] what happens if the front engine gets sold while the wagons are still out there somewhere?

This problem exists already now, without that shunting feature, for wagons in depot.

But it's far less of a problem while inside a depot.

[Karn]

There is c) Update CB36 or part of it less often (only in depot arranging?) and move the part not compatible with shunting from cache to permanent variable.

This is not an option, because CB36 is also evaluated on loading a savegame, and it must at that point always return the same values as previously in the Depot. If you want to change that, you have to dig through a whole lot of cached values, and put them in the savegame.

You are quite wrong. CB36 is not the problem, it's consequence. CB36 itself does nothing. It's depended on varaction 2.

CB36 change vehicle variables according to other vehicle variables (via Var2 a.k.a GetVariable()). Now this is mainly problem in parent scope, because that obviously change during decoupling and reversing. And this is heavily used by advanced NewGRFs.

Because of this:
a) is pointless, you can't disable reversing and shunting just because people use NewGRF.
b) is pointless too, it creates problem with virtual vehicle that returns broken values or real vehicle that returns invalid values.
c) should be discussed.

So there is question about c), how many variables should be stored from varaction 2 parent scope after leaving depot. Many of them don't make sense, because they return actual state on tracks, which is useless for parent scope anyway. There are some important like local_id, but list is quite long.

[TrainLover]

Could this be implemented in a huge patch pack like JGR's without breaking the patch pack?