Convoy length calculation bug

[Ranran]

Hi. (´・ω・｀)
If you change the order of vehicles, the convoy length (required total station tiles) may change.

Steps to reproduce:
1. Start the game from 1830

2. Construct tracks and train depot and make convoy in the following order

Code Select Expand


B&LR "Lancashire Witch" 0-4-0
Four wheel mail brake carriage (+ mail locker)
Four wheel brake carriage (first class)

NOTE: This order is very important


3. Change the order as follows

Code Select Expand


B&LR "Lancashire Witch" 0-4-0
Four wheel brake carriage (first class)
Four wheel mail brake carriage (+ mail locker)



4. Now Station tiles has changed from 1 to 2



This is thought to occur when "length = 0 vehicle" such as railway-mail-locker comes to the convoy end.

[ACarlotti]

This is the result of a deliberate fudge. The point at which it is applied (in this context) is in vehicle_summary_t::update_summary. The same fudge appears to be intended in convoi_t::get_tile_length(). However, I think the duplication of functionality may have allowed a bug to creep in, since these two methods use different constants for what I think is intended to be the same fudge.

convoi_t::get_tile_length() gives some hint as to what is going on.

Code Select Expand


// the last vehicle counts differently in stations and for reserving track
// (1) add 8 = 127/256 tile to account for the driving in stations in north/west direction
//     see at the end of vehicle_t::hop()
// (2) for length of convoi for loading in stations the length of the last vehicle matters
//     see convoi_t::hat_gehalten


So as far as I can tell, convoys wouldn't automatically end up correctly aligned with a station (or other stopping point) when travelling north or west. (Why? I don't know. I would love to hear an explanation.) So to fix this graphics issue, the convoy is halted (just under) halfway through the tile when arriving at a station travelling in those directions. But then there is a risk that the last vehicle is shown (and oriented) as if it were on the tile before the station. This wouldn't (usually) have been a trouble when all vehicles were exactly half a tile long. But now that some vehicles are shorted it is necessary to add a bit of padding to ensure that the last vehicle always reaches the last tile the train is on (or rather, to ensure that the tile the last vehicles is shown on is considered to be part of the train).
In practice this means the last vehicle is always assumed to be at least half a tile long.

[DrSuperGood]

In the case of the mail locker and such it does not matter if it is on the tile or not. I guess the solution would be to disable the "fudge" for 0 length?

[jamespetts]

This is a curious one - I did not write either versions of this code, and I cannot see that they are other than superficially different. I have tired to make them more consistent with one another, but I am not sure that this will have had any effect.

[ACarlotti]

I have tired to make them more consistent with one another,

I don't expect that to be causing any issues at present, although it's possible that there are (or, at least, were) assumptions being made that certain hard-coded or configurable settings are the same. I've not investigated where those parameters come from.

[Ranran]

@ACarlotti - thank you for detail information.

I noticed length calculation is unnatural regardless of mail-locker.
It seems that "padding" as ACarlotti said is inserted also in the depot.


The length of this image should be
2 + 2 + 3 + 0 + 3 = 10
But connecting more than this will exceed 16 (1 tile).

In my hypothesis, an invisible padding is inserted so that the last vehicle is 8 if length is less than 8
That is, 1 is inserted if the length of the last vehicle is 7 and 8 is inserted if the length of the last vehicle is 0.

Therefore, when the length of the last vehicle becomes small, the padding increases and the total convoy length increases.
This includes the case of reversing at the station.

I made a <a href="https://drive.google.com/open?id=1FSIa03XY29lVO_hpHixp_8MJNgWkv1C5">saved game</a> to reproduce the problem it caused. Please check.

.
.
.

It is just after departure from the depot when open the saved game.
You can see that this convoy's required station tiles is 1 in the convoy details window.
Please keep convoy detail window open.
Next, the convoy arrives at Bams station and reversing. At this time you can see that station tiles in the convoy detail window have changed to 2.
The convoy will arrive again at Fields station and reversing. Since the home length is 2 at this time, this convoy becomes "no route" status and can not reach Bam station despite being the station that it arrived last time.


There is a possibility that deadlock may be caused if convoy including short vehicle is reversed like this.
(´・ω・｀)

[ACarlotti]

n my hypothesis, an invisible padding is inserted so that the last vehicle is 8 if length is less than 8
That is, 1 is inserted if the length of the last vehicle is 7 and 8 is inserted if the length of the last vehicle is 0.

That is correct.

This includes the case of reversing at the station.

Bother. The easiest 'fix' in the short term will be to add more padding according the maximum required either before or after reversing. Since I'm unfamiliar with the precise rules for reversing I don't know how easy that will be, but in the worst case you could just add half a tile of padding to all trains. Except that will probably break existing games, so I think we need to make the padding as efficient as possible.

In the long run (particular with convoy recombination being developed) I would like to be able to remove the need for padding - it makes the code more complicated and seems like it shouldn't fundamentally be necessary. I also suspect it is related to funny graphics alignment when turning corners, However, the fact that this hasn't previously been implented in Standard suggests that it could be infeasible to actually change.

[jamespetts]

If it is of any assistance, here is the algorithm for reversing: essentially, the actual order of the vehicles in the train is simply swapped into a reversed order one by one:

Code Select Expand


void convoi_t::reverse_order(bool rev)
{
// Code snippet obtained and adapted from:
// http://www.cprogramming.com/snippets/show.php?tip=15&count=30&page=0
// by John Shao (public domain work)

uint8 a = 0;
    vehicle_t* reverse;
uint8 b  = vehicle_count;

working_method_t wm = drive_by_sight;
if(front()->get_waytype() == track_wt || front()->get_waytype() == tram_wt || front()->get_waytype() == maglev_wt || front()->get_waytype() == monorail_wt)
{
rail_vehicle_t* w = (rail_vehicle_t*)front();
wm = w->get_working_method();
}

if(rev)
{
front()->set_leading(false);
}
else
{
if(!back()->get_desc()->is_bidirectional())
{
// Do not change the order at all if the last vehicle is not bidirectional
return;
}

a++;
if(front()->get_desc()->get_power() > 0)
{
// If this is a locomotive, check for tenders/pair units.
if (front()->get_desc()->get_trailer_count() > 0 && vehicle[1]->get_desc()->get_leader_count() > 0)
{
a ++;
}

// Check for double-headed (and triple headed, etc.) tender locomotives
uint8 first = a;
uint8 second = a + 1;
while(vehicle_count > second && (vehicle[first]->get_desc()->get_power() > 0 || vehicle[second]->get_desc()->get_power() > 0))
{
if(vehicle[first]->get_desc()->get_trailer_count() > 0 && vehicle[second]->get_desc()->get_leader_count() > 0)
{
a ++;
}
first++;
second++;
}
if (vehicle_count > 1 && vehicle[1]->get_desc()->get_power() == 0 && vehicle[1]->get_desc()->get_trailer_count() == 1 && vehicle[1]->get_desc()->get_trailer(0) && vehicle[1]->get_desc()->get_trailer(0)->get_power() == 0 && vehicle[1]->get_desc()->get_trailer(0)->get_value() == 0)
{
// Multiple tenders or Garretts with powered front units.
a ++;
}
}

// Check whether this is a Garrett type vehicle (with unpowered front units).
if(vehicle[0]->get_desc()->get_power() == 0 && vehicle[0]->get_desc()->get_total_capacity() == 0)
{
// Possible Garrett
const uint8 count = front()->get_desc()->get_trailer_count();
if(count > 0 && vehicle[1]->get_desc()->get_power() > 0 && vehicle[1]->get_desc()->get_trailer_count() > 0)
{
// Garrett detected
a ++;
}
}

// Check for a goods train with a brake van
if((vehicle[vehicle_count - 2]->get_desc()->get_freight_type()->get_catg_index() > 1)
&& vehicle[vehicle_count - 2]->get_desc()->get_can_be_at_rear() == false)
{
b--;
}

for(uint8 i = 1; i < vehicle_count; i++)
{
if(vehicle[i]->get_desc()->get_power() > 0)
{
a++;
}
}
}

back()->set_last(false);

for( ; a<--b; a++) //increment a and decrement b until they meet each other
{
reverse = vehicle[a]; //put what's in a into swap spacekarte_t::load(
vehicle[a] = vehicle[b]; //put what's in b into a
vehicle[b] = reverse; //put what's in the swap (a) into b
}

if(!rev)
{
front()->set_leading(true);
}

back()->set_last(true);

reversed = !reversed;
if (rev)
{
re_ordered = !re_ordered;
}

for(const_iterator i = begin(); i != end(); ++i)
{
(*i)->set_reversed(reversed);
}

if(front()->get_waytype() == track_wt || front()->get_waytype()  == tram_wt || front()->get_waytype() == maglev_wt || front()->get_waytype() == monorail_wt)
{
rail_vehicle_t* w = (rail_vehicle_t*)front();
w->set_working_method(wm);
}

welt->set_dirty();
}


It would be splendid if padding could be eliminated - but working out how to do that would involve diving into the endless rabbit-hole that is the graphics code and I should not even know where to begin with that.

[Ranran]

I suppose I fixed this bug.
Please check my Github branch.

It seems to be working properly.


2 + 2 + 3 + 3 + 3 + 3 + 0 = 16 ... 16/16 = 1 tile    (OK)


2 + 2 + 3 + 3 + 3 + 3 + 0 + 3 = 19 ... 19/16 = 2 tiles   (OK)



2 * 8 = 16 ... 16/16 = 1 tile    (OK)



2 * 7 + 3 = 17 ... 17/16 = 2 tiles    (OK)




Also it does not fluctuate the convoy length when reversing.


Is there any problem with the code? (´・ω・｀)

[ACarlotti]

Is there any problem with the code?

I think so. Try creating a train with vehicles whose lengths sum to 16 and whose last vehicle has length less than 8, and run it north or west into a station of length 2, where the previous tile of track is a 90° bend. When the train arrives, I think it will still be occupying the previous tile of track (i.e. the bend), and the last vehicle (or vehicles) will be orientated incorrectly for a station stop.

[jamespetts]

Thank you both very much for your contributions on this: they are much appreciated.

[Ranran]

@ACarlotti - Thank you very much for checking. That is the information you pointed out in the previous post, now I understand - my apologies.



Apparently it still seems necessary to fix other codes. I tried some fixing on my Github branch again.

When the train arrives, I think it will still be occupying the previous tile of track (i.e. the bend), and the last vehicle (or vehicles) will be orientated incorrectly for a station stop.

I think that this issue was solved by deleting the following part in vehicle_t::hop().

Code Select Expand


if(  check_for_finish  &  leading && (direction==ribi_t::north  || direction==ribi_t::west))
{
steps_next = (steps_next/2)+1;
}


Then it came to hopping out from the station when reversing, so I commented out the movement process when reversing in convoi_t::vorfahren(), but I think that adjustment is necessary.

Although adjustment may be necessary, I suppose that it becomes somewhat decent behavior.
I am not confident that I can solve this problem altogether, so I hope if anyone could give your help for solve this.
Thank you.(´・ω・｀)

[prissi]

To alleviate the issue, the loading of vehicles is based on the reported convoi length, not if those are within the station. However, choosing singals assume the convoi fits into the station when it length is shorter than the tile count. Occupying the tile in front of the station may block following trains to enter. So it is adviseable to rather report a too short than a too long length, i.e. begin a new station tile every 16+8 train length, i.e. (tiles in srtation=(train length+8)/16). At rail signals (at least a few version ago) and at road crossings the vehicle always go to the end of a tile, at the border to the next tile. This is why some buses are happily inside crossings at read lights.

If one is ok with the vehicles jumping further in curves, one could align them differently and thus let them drive towards then end of a station. But this requires realignment of all vehicles in four of the eight directions and different offsets for four directional vehicles, i.e. eight images instead of four. Thus this was never done, although there was once a version where the special code for stations in those two directions was removed.

[prissi]

Here is the thread in question:
https://forum.simutrans.com/index.php/topic,3376.0.html

[jamespetts]

Gosh, 2009 - that is a long time ago.

[jamespetts]

Can I check - Ranran, do you believe that your changes on the fix-convoy-length branch are ready for integration, or does this need more work? If it does need more work, is there any worthwhile subset of changes/fixes that can be incorporated forthwith without further work?

[prissi]

The bug comes from a simple reason. With the zero length car not at the back, all cars are before tile 16*n+8 and thus the train needs n+1 tiles to fully load.

However, with the zero length car trailing, this car will be stopping at a position larger than 16*n+8. Hence this train will need n+2 tiles for loading, since the starting position of a car determines if it can load or not at a station. One must add extra logic which applies only to trailing cars of length zero for that. It may missing from standard as well, but is not used so extensively there.

[Ranran]

I tried testing that build again, but I think it would be better to redo the fix from scratch. (There's nothing more I can do.) (´・ω・｀)
The issue of the reference position of the convoy has not been solved and found some bugs.

[jamespetts]

Thank you for confirming - that is most helpful.

[Ranran]

I remembered that Ranran which translated Japanese help text pointed out inconsistencies in the content of the English help text.
depot.txt says:

Code Select Expand

The length is important because, if a convoy is too long for a stop, the vehicles at the back will be left out of the stop and will not be able to load/unload there.
Standard has such specifications, but Extended does not have such specifications and convoy is stuck if convoy exceeds station tile length . Therefore, this description seems to be incorrect.
By the way, point is, why does extended not allow exceeding station length? 
At worst, this bug should avoid getting stuck if the length of the station platform can be exceeded. (Note that the vehicle is still considered unexpectedly exceed the station. But think better than the stuck.)
And there are many examples in Japan and Asia that the train exceed the length of the station.
Actually, passengers can get off by moving to the previous vehicle. Or sometimes exceeding vehicles are mail and cargo vehicle that will not be load/unloaded at the station on the way.

[Vladki]

It happens occasionally on czech railways too. Passengers are told in advance that if they want to get off at some station, they have to go to the first car. But there is no such mechanism in extended. In standard, you at least know which cars are exceeding the platform, and arrange the train accordingly, but in extended it won't work due to reversal mechanics.

[Mariculous]

Same for Germany, at least these days, at some stations where platforms were not upgraded in time. This also happened in Britain with class 373 trains in GNER service from London to York and Leeds.

However, stucking because of being too long for a platform is still better than standards behavior, where people to a short station will stack up at cars that can't unload at the station and will never get out of it, effectively making that car useless ballast.

When allowing trains to be longer than the platform, we should in my opinion, at least warn players in lines status about "some vehicle is too long for some station in the line" and let passengers leave the train at increased loading times if a car does not fit in the station.
Also keep choose signals in mind. These should never route a train to a platform that is shorther than the scheduled one.

In addition, I don't think this special length calculation for the last car is a good idea. I already had some trains stucking after they reversed because of this.
If you think this is a good idea, we should really warn the player in the depot window if the train will be longer after reversing.

[jamespetts]

Thank you for noting this. The correct behaviour where a train is too long for a station should be that it takes longer to load/unload as passengers may have to move through the train to get off. I will have to check the code when I get home (eventually - there is an extremely long queue of things for me to do) to see whether this is what happens and, if so, amend the help text accordingly.

[Vladki]

Sorry to revive an old topic, but I was just wondering why modern british trams cannot fit in one tile, even if the lenghts add up to 16.

The bug comes from a simple reason. With the zero length car not at the back, all cars are before tile 16*n+8 and thus the train needs n+1 tiles to fully load.

However, with the zero length car trailing, this car will be stopping at a position larger than 16*n+8. Hence this train will need n+2 tiles for loading, since the starting position of a car determines if it can load or not at a station. One must add extra logic which applies only to trailing cars of length zero for that. It may missing from standard as well, but is not used so extensively there.

So if I understand correctly - any vehicle that starts in second half of tile will require one more tile for loading?. Lets say If I have a tram with lengths 9+7, it will need two tiles, but 8+8 will need only one? Is this logic used for anything else then loading? E.g. track reservations?

[Mariculous]

but 8+8 will need only one?

Exacly. 8 is the default car length if you don't specify anything btw.

Is this logic used for anything else then loading? E.g. track reservations?

Not sure from the codebase, but doesn't seem so from ingame observations as 9+7 trams seem to fit on a single tile e.g. when waiting at a signal.

[prissi]

Before car position (inside and outside of station) was used to determine if it can be loaded (because on old Simutrans one tile was enough to unload any train, like TTD).

Then it was noted that trains could only unload in one direction. and it changed when the map was rotated. Hence the unloading length calculation was seperated from the position of the car.

Putting a zero length car inside the train at the previous to last position is indeed asking for troubles.

[Vladki]

The problem is also with articulated trams, that are composed of many small parts, e.g. lengths 3+4+2+4+3 = 16, should fit on one tile.

[Mariculous]

We don't have zero length cars here.
Many trams are built up from many quite small sections of less than 15m each being slightly less than 30m (1 tile) length in total.

Such vehicles are an isue in simutrans as they will rquire longer platforms than they should.
There are very many examples for such trams in the real world. Especially, but not limited to trams from 2000 and later.

[prissi]

The calculation is consistent with the positioning of the cars that the first tile is only 8 long, all following tiles are 16 long.

effective length of station=(tile*16-8)

If you change this all vehicles need realignment and some special code for cornering and diagonal movement.

[Mariculous]

effective length of station=(tile*16-8)

That doesn't seem to be true.
A single car of length 16 fits on one tile stop.
Two cars of length 8+8 also fit on a single tile stop.
Two cars of length 9+7, however, don't fit on a single tile stop.

[prissi]

It is the start position of the car, as mentioned before. A single car will always on position 0. But if the length is longer than 8, the second car will be starting on the bend outside the stations. This is why it matters where a zero length car is. Because if it is inside the convoi, all cars are loaded. At the end, this car starts with the endposition, which could be outside of the stations.

[Ranran]

Drawing the vehicle in the correct position and calculating the correct convoy length are important to the convoy recombination system.
Currently, the system has many problems. When you mix reversed vehicles, the drawing position of convoy is chaos.

For example in Britain-ex, you may see a weird large space between the tender and the next coach on the return trip of the SL train. This is impossible to solve with current systems. Because standard took no account of reversing the vehicle at all, but extended inherited that system. Therefore, extended needs its own underlying solution.

Breaking true convoy length calculations due to small vehicles will now be visible graphically in the tile occupancy bar which is unincorporated patch I made. I tried to faithfully reproduce the formula.
It is sometimes seen when connecting small vehicles. Then players will find that strange behavior. But that is just a correct representation of the internal calculations. (´・ω・｀)

[jamespetts]

Drawing the vehicle in the correct position and calculating the correct convoy length are important to the convoy recombination system.
Currently, the system has many problems. When you mix reversed vehicles, the drawing position of convoy is chaos.

This may well need a considerable amount of work.

Does anyone have any ideas as to an efficient implementation of a better system that will not require re-rendering the graphics in the pakset, which is not realistically feasible for Pak128.Britain-Ex owing to the number of vehicles?

[Ranran]

This may well need a considerable amount of work.

Does anyone have any ideas as to an efficient implementation of a better system that will not require re-rendering the graphics in the pakset, which is not realistically feasible for Pak128.Britain-Ex owing to the number of vehicles?

This is a very important issue, so first I explain the mechanism by which it occurs to come up with a solution.

Look at the image.

This is an example of a vehicle drawn to current specifications. SW and NE, that is, one is the reverse one. I arranged them one above the other. Pay attention to the drawing position.


Reversed vehicles should be drawn in the same position if they may be mixed. Because the program just draws the same position for every vehicle.


Simutrans is not drawn in 3D technology. It simply puts a 2D picture at the calculated coordinates and overlays.

When the vehicles in the same position are made continuous, it is drawn like this.



Then connect the reversed vehicle in the same way



Next, connect the same vehicle as the first and second ones


Currently, this symptom can be seen only in a few combinations such as un-bidirectional locomotives and bidirectional coaches and wagons.
It is noticeable when they are a certain length.


So we have to take one of the measures;
- We rewrite the image to draw in the same position (Redraw the drawing position code after drawing at the center position.)
- Corresponding this in code
- Correspond to some extent with code and rewrite the image only in the part that can not be complemented


Issues to consider:
1) Difference in drawing position in length.
Larger vehicles appear to be drawn in the center, but are actually only fitted to the front or rear reference position. Small vehicles are skewed to either side.

2a) Difference in drawing position for each pakset.
The reference position for pakset may be different.

2b) We should also consider pak size.

3) Differences depending on the waytype.
Road vehicles are not the center of the road. May need different rules than railroad.


In any case, I think we can't avoid the confusion of drawing positions without a lot of work. (´・ω・｀)

[jamespetts]

Thank you for this summary. I wonder whether there might be any role for offsets, which can be specified in .dat files, and possibly for new systems applying offsets dynamically? More thought will definitely be needed on this.