Vehicles are displayed in wrong depth order after rotating the world

[iv5343]

This is a bug in Simutrans r11015 and earlier, should affect all vehicles but is probably most evident with trains.  Wrong depth order means the leading vehicle in a convoi_t is displayed over a trailing one when they move upwards on screen or vice versa.  A front and back lane inversion is also possible.  The usual stuff.  The glitches are visible until the vehicles move to another tile.

The cause is that all kinds of things are rotated except the moving objects in an objlist_t which determines the drawing order.  I know this isn't hugely important but the sorting objlist_t::intern_add_moving() performs is reversible in practically all cases with moderate effort, the patch below demonstrates this.  It has a side effect I want to explain.

The job of intern_add_moving() is to sort vehicles back to front, in three steps.  1) It inserts them between any stationary foreground and background objects.  2) It appends vehicles in the order they enter a tile, so the last one going in is also the last one drawn and appears in front.  Vehicles moving downwards on the screen, NE < heading < SW clockwise, must be reversed, i.e. prepended.  3) The function sorts vehicles on different lanes by their disp_lane, these lane numbers always ascend back to front.  Step 2) applies to vehicles in the same lane, the lanes are parallel.  There are two loops which compute a start and end index, that takes care of 1) and 3).  Then it does something a little odd:

Code Select Expand

const uint8 direction = ((vehicle_base_t*)new_obj)->get_direction();
switch(lane) {
// pedestrians or road vehicles, back: either w/sw/s or n/ne/e
case 0:
case 1: {
// on right side to w,sw; on left to n: insert last
// on right side to s; on left to ne,e: insert first
if (direction == ribi_t::south  ||  (direction & ribi_t::east)) {
intern_insert_at(new_obj, start);
} else {
intern_insert_at(new_obj, end);
}
return true;
}
// middle land
case 2:
case 3:
// pedestrians, road vehicles, front lane
case 4: {
// going e/s: insert first, else last
if ( (direction & ribi_t::northwest)==0 ) {
intern_insert_at(new_obj, start);
} else {
intern_insert_at(new_obj, end);
}
return true;
}
}
return false;

The first conditional works as expected, it prepends if the direction is NE, E, SE, or S, and appends otherwise.  SW-NE is horizontal, it shouldn't really matter if the vehicles are appended or prepended there.  Whatever the rationale was, I'm in favor of covering four directions for rotational symmetry.  The second conditional prepends if the direction is E, SE, or S, unless the vehicle is on lane 0-1.  I see no reason for this exception, there's no comment explaining it, and my tests didn't reveal any problems when it is removed.

I also have to revisit road_vehicle_t::calc_disp_lane(): "disp_lane is valid for vehicles moving to the right side of the screen, must be mirrored if SE < heading < NW".  That's true in a strict sense, however the code still covers the directions SE, S, SW, W, and NW, that's actually SE <= heading <= NW.  NW-SE is vertical, the lane order shouldn't matter here as long as all moving objects are divided consistently, but as above the asymmetry is inconvenient and I'm unaware of a reason NW must be considered left side.

So the patch makes intern_add_moving() symmetric.  Thoughts?

[prissi]

You are right, however there is more to consider.

Rotation with a change of the display lane would mean to remove and readd all city cars, road vehicles and pedestrians to their current tile. I am not sure if this effort is worth it as it could slow down rotation a lot and the display is remended next tile. There is also another glitch after reloading, the overtaking cannot be calculated at the time a vehicle is added to the map. Hence, here two the display will be wrong until the next tile is entered. Same for changing the setting to drive on left.

One would probably need to test on how much extra time this would take on a map with lots of road vehciles and pedetrians, as the latter could easily reach 100000 or more.

[prissi]

Incorporated in r11022, thank you