[9.x] bug - became: Reverse route bug

[sdog]

Code Select Expand


Program received signal SIGSEGV, Segmentation fault.
0x00000000005ece3a in haltestelle_t::update_alternative_seats (this=0x67c1070,
   cnv=...) at simhalt.cc:2462
2462 if ( here_convoys[i] == cnv || ! here_convoys[i]->get_free_seats())
(gdb) bt
#0  0x00000000005ece3a in haltestelle_t::update_alternative_seats (
   this=0x67c1070, cnv=...) at simhalt.cc:2462
#1  0x00000000005cc51a in convoi_t::hat_gehalten (this=0x1ecb880, k=...,
   halt=...) at simconvoi.cc:3772
#2  0x00000000005ca6a0 in convoi_t::laden (this=0x1ecb880) at simconvoi.cc:3337
#3  0x00000000005c1a9e in convoi_t::step (this=0x1ecb880) at simconvoi.cc:1177
#4  0x0000000000643bb6 in karte_t::step (this=0xe48d60) at simworld.cc:3503
#5  0x000000000064eab1 in karte_t::interactive (this=0xe48d60,
   quit_month=2147483647) at simworld.cc:6046
#6  0x000000000060038c in simu_main (argc=1, argv=0x7fffffffe2d8)
   at simmain.cc:1141
#7  0x000000000068a3be in main (argc=1, argv=0x7fffffffe2d8) at simsys_s.cc:731


tested with 9.x branch, commit ae283a16ebbeafca08877607c4f339b45ebc883b, pak.german-exp.20382
in linux 64, gcc 4.4.3

while it hapened some lines hat 'withdraw all' orders and i sent out new convoys (opel blitz bus) from a depot for those lines.

[jamespetts]

SDog,

thank you for the report. Do you have a saved game?

[inkelyad]

Sdog. Can you merge latest commit from my loading_queue branch and try again?

[jamespetts]

After applying Inkelyad's fix, I cannot reproduce this (at least, not with Pak128.Britain-Ex - I assume that the pakset makes no difference to this).

[sdog]

the bug is ideosyncratic, i couldn't reproduce it.

[jamespetts]

Hmm, those are extremely hard to fix. Has it happened again since the first occurrence?

[sdog]

no, only once.

we don't have a safety relevant application here, if it doesn't happen frequently enough to find it. It might also be unimportant. Until it's fixed without knowing, or it get's really bad through some other changes.

On another note, inkelyad, did you remove one feature of your reverse route system? When a train gets to a different platform of a track first it will call at the first one it encounters. Necessary for reversing on a double line track with double stations. This does not seem to work anymore, trains will go the whole cycle now to get to the correct platform when reversing.

[inkelyad]

Reverse route is not my patch.

[sdog]

Must have been yobbobananda's then, sorry for the confusion caused!

[jamespetts]

no, only once.

we don't have a safety relevant application here, if it doesn't happen frequently enough to find it. It might also be unimportant. Until it's fixed without knowing, or it get's really bad through some other changes.

On another note, inkelyad, did you remove one feature of your reverse route system? When a train gets to a different platform of a track first it will call at the first one it encounters. Necessary for reversing on a double line track with double stations. This does not seem to work anymore, trains will go the whole cycle now to get to the correct platform when reversing.

Is this a problem in 9.x or in Inkelyad's branch or both?

[sdog]

i didn't get around to try inkleyad's branch yet. so it was only in your 9.x

i'll test it more thoroughly though, if you think there might be a bug.

[jamespetts]

I'd be grateful if you could test it more - this would indeed be a rather troubling bug.

[sdog]

i've checked it again, when "reverse route" is set, trains will try to get to the platform set in the schedule instead of calling at the first platform they pass of the same station.

[inkelyad]

I think relevant code should be same in Standard and Experimental. Can you test Standard too?
Edit:
Merge 90f31f27786c7aa4f129edd016316ed795b0589d
breaks patches from mesilliac (starting from b99ca8a71bf1ab4cb9a1e4f555dd13b016fd1870)

[jamespetts]

Inkelyad - haven't had time to test, but, to clarify, do you mean that the code is broken in Experimental but working in Standard, or broken in both Standard and Experimental?

[inkelyad]

Standard have no 'reverse route'. It was my error.
90f31f27786c7aa4f129edd016316ed795b0589d is first "bad" commit.

[sdog]

posting this for completness, see remarks below.

Program received signal SIGSEGV, Segmentation fault.
0x00000000005ece3a in haltestelle_t::update_alternative_seats (this=0x9878100,
    cnv=...) at simhalt.cc:2462
2462         if ( here_convoys == cnv || ! here_convoys->get_free_seats())
(gdb) bt
#0  0x00000000005ece3a in haltestelle_t::update_alternative_seats (
    this=0x9878100, cnv=...) at simhalt.cc:2462
#1  0x00000000005cc51a in convoi_t::hat_gehalten (this=0x8eabce0, k=...,
    halt=...) at simconvoi.cc:3772
#2  0x00000000005ca6a0 in convoi_t::laden (this=0x8eabce0) at simconvoi.cc:3337
#3  0x00000000005c1a9e in convoi_t::step (this=0x8eabce0) at simconvoi.cc:1177
#4  0x0000000000643bb6 in karte_t::step (this=0x471fcf0) at simworld.cc:3503
#5  0x000000000064eab1 in karte_t::interactive (this=0x471fcf0,
    quit_month=2147483647) at simworld.cc:6046
#6  0x000000000060038c in simu_main (argc=1, argv=0x7fffffffe2e8)
    at simmain.cc:1141
#7  0x000000000068a3be in main (argc=1, argv=0x7fffffffe2e8) at simsys_s.cc:731

It happened when a single wood barge loaded at a canal goods quay. Fast time, no user interaction.

tested with 9.x branch from james git repository, state 6 November. So before merge with inkleyads fixes.

[jamespetts]

Sdog,

do you have a saved game? Are you able to reproduce this now?

[sdog]

just use the sheffield ring line anywhere (line 2)
i've tried it near the southern harrogate junction station

http://dl.dropbox.com/u/1876190/103.sve

[jamespetts]

SDog,

your URL gives me a 404 error - can you re-upload your saved game?

[sdog]

114.sve
check line 2 at Sheffield Railway Station, the clock-wise train (on reverse route) will drive through the station to turn somewhere else to get on the counter-clock-wise track to call into the platform specified in it's schedule.

update it seems the savegame is not compatible with the latest version, you pushed to github

FATAL ERROR: haltestelle_t::rdwr()
invalid halt co-ordinate at 0, 4096, 0

[sdog]

90f31f27786c7aa4f129edd016316ed795b0589d#diff-10
vehicle/simvehikel.cc

3555   
-  // if an early platform was found, stop there
-  if(early_platform_index!=INVALID_INDEX) {
-    next_signal_index = early_platform_index;
-    // directly modify the route
-    route->truncate_from(early_platform_index);
-  }
-  // if next stop is further away then next crossing, return next crossing
-  if(next_signal_index>next_crossing_index) {
-    next_signal_index = next_crossing_index;
-  }

+    if(  gr  &&  gr->is_halt()  ) {
+      next_signal_index = route->get_count();
+    }

I think here there's the problem.

[jamespetts]

Sdog,

hmm - these are changes from Standard, which are necessary in order to make choose signals work. I'm not sure that the "early platform" is quite the same as the implementation of the reverse route code here, and, in any event, if removing this was the cause of the problem, I couldn't just put it back, as it would break the current choose signal implementation.

The trouble is, because this (that is, the reverse route algorithm) was not my code, it's harder for me to maintain, as I'm not familiar with it, and my time is very limited this week-end. Do you think that you could test this by reverting your local branch to before that commit and seeing whether that makes the difference?

Thank you for your help with this!

Edit: The latest save games are indeed incompatible: see here for an explanation of why an additional save parameter was necessary. It is possible to hack the code by commenting out the part that loads the additional parameter, loading the game with the hacked binary, saving it, then reverting the code to get to a position where you have a compatible saved game.

[sdog]

inkelyad reported this commit (merge with standard) broke that feature. i haven't verified it. i'll have a look in yoyobanana's changes and see if it was a part. but i won't be able to do so within the next 1 1/2 weeks or so.

[inkelyad]

Reverse route flag is not used here. yoyobanana modified waggon_t::block_reserver so first encountered platform will be used. Recent changes in Standard broke this. And I not sure we should place it back. I want convoy to use platform in schedule, not some 'unnatural AI' decision.

EDIT:
Github branch with fix

[sdog]

without it, the reverse route won't work :-)

you've been pretty quick, just two hours later you fix it already.

i shall test this as soon as possible.

Code Select Expand


3639:    && cnv->get_line()->get_schedule()->is_mirrored()

it doesn't apply for circle routes, so it should either check if the convoy is set to "reverse route" or
if convoy is_mirrored or is_bidirectional

Code Select Expand

&& (cnv->get_line()->get_schedule()->is_mirrored() || cnv->get_line()->get_schedule()->is_bidirectional())

https://github.com/sdog/simutrans-experimental

[inkelyad]

Merged.

[jamespetts]

This fix has now been incorporated into Simutrans-Experimental 9.0. Thank you very much, SDog and Inkelyad!