Weird timing problem in the ex-15 branch

[jamespetts]

I have begun recently to look into the code in the ex-15 branch (formerly vehicle-management) again; Ranran has very helpfully integrated a lot of the development that has occurred on the master branch in the last 4.5 years since this branch was last actively worked on to a significant extent (save for some interesting work that Ranran has done with the UI). I have also managed to fix a merge error that had been present until recently.

However, one problem that I have noticed is that, on occasions, such as when starting from depots, road vehicles appear to travel too fast. They are recorded as travelling the correct speed in km/h in their information window, but they are definitely going too fast: they are noticeably much faster than road vehicles travelling at the proper speed and rapidly catch up other road vehicles travelling at purportedly the same speed (48km/h, the town speed limit).

Moreover, the loading and reversing times seem to be accelerated by a similar factor, as if the problem is some fault in the computation of the time base. These problems disappear on reloading the game, but recur when restarting the vehicle from the depot.

I know very little about the code computing the time base, and do not think that the ex-15 branch has in any way (intentionally) modified any of this code, so I am entirely mystified as to what the cause of this problem could be. I did find one possible anomaly in the timing code, in which an sint32 variable was used to store a value from another variable with sint64 size, but this is present on the master branch, which does not have this problem, and rectifying this anomaly does nothing to address the behaviour.

If anyone who has any insight into the time base code and where I could sensibly look to locate this problem, I should be very grateful for any assistance.

[Matthew]

Two thoughts. I have noticed that timetabled convoys always seem to depart about 30s before the scheduled time. I wonder if that is related.

> I know very little about the code computing the time base

I have looked at it but you still probably know a lot more about it than me. It does seem that the key ticks: time ratio is based on a complex formula that looks like an invitation for rounding errors. But I am a novice programmer and know that odd things in Simutrans code have usually been written that way for very good reasons. I hope to look into this more once B-B4 reaches the present day.

[jamespetts]

Two thoughts. I have noticed that timetabled convoys always seem to depart about 30s before the scheduled time. I wonder if that is related.

I think that this is unlikely - unless you see the seconds counting down far faster than usual.

[Ranran(retired)]

As I reported before, I think one of the causes of the problem is the abnormal schedule data.
For example, if you check the timing data tab of times history of convoy, an hourglass mark is displayed at all stops.(fixed) So some convoys may be a data anomaly waiting for a new departure flag.

As previously reported, all convoys do not belong to the line as they first check if the line schedules are the same and return false.

Vehicles that have just departed from the depot certainly ignore the laws of the world, but when saved and loaded, they are back to sane. I have no idea why this happens. Does it have world distances and time scales for each vehicle or is it given when a convoy is created in the depot?


I fixed the problem with the livery scheme. I don't think there are any changes to the livery scheme, so I checked and overwrote the diffs in the related files on the master branch. The behavior seems to be normal, but I'm not sure if this was a change intended for a new feature.
Also fixed the cmakelist error.


EDIT:
You can see some differences by checking the diffs in void convoi_t::start(depot_t * dep).
I don't know how this makes a difference.

EDIT2:
The livery lag seems to have been fixed, but livery timeline doesn't seem to be working properly. So I put this fix on hold.

EDIT3:
I fixed the error in displaying times history, but there is a similar error in simconvoi.cc, and there may be other similar errors lurking.
https://github.com/Ranran-the-JuicyPork/simutrans-extended/commit/2f821ae710a55c769411f9ff00662e376ef12819

[jamespetts]

Thank you for your work on this. I noticed that you have closed your pull request - should I therefore wait before merging your work on this for the present?

[TurfIt]

Before looking at "time base code", I'd suggest checking the convois aren't being added to the synclist twice...
Standard ended up with this for citycars after it's recent synclist overhaul.

[Ranran(retired)]

I have attempted to narrow down the code that causes rocket starts from the depot from the simconvoi.cc's diff.
I found that removing the following part of the newly added code in simconvoi.cc in branch ex15 would not cause a rocket start from the depot.
https://github.com/Ranran-the-JuicyPork/simutrans-extended/commit/0dd11078c68ce394362276e2988b636f4d19341a

I also checked the differences in depot_t::start_convoi(), which is called here next. The differences here were slight except for the change to standard format.
In branch ex15, local_execution is missing from the if condition on line 594. As a result, subsequent processes are executed when the car departs from the depot. This seems to be causing the rocket start.

That was originally a statement that did not currently exist, but I don't know what the intent is. For example, it might be correct to pass true to local_execution instead of deleting that statement, or it might be necessary to change the processing of start_convoi().
I'm not familiar with the code associated with this, but I suppose you can dig into the cause here.

[jamespetts]

Thank you for that - I have now commented out the local_execution condition and that does indeed seem to fix the problem.

It was added with commit a9d1c0ea2a13c830a42ec939c159f5c410f28167, whose comment is, "CHANGE: Mechanics for vehicles visiting depots and then leaving again automatically". I cannot now recall why this was thought necessary for this mechanic; I shall have to look into this in more detail when I come to look at depot visiting mechanics again.

[Ranran(retired)]

Thank you for your work on this. I noticed that you have closed your pull request - should I therefore wait before merging your work on this for the present?

I canceled the pull request because the changes to the livery scheme were probably wrong, but now that I've removed the commit, you can merge it.
Changed to not restore the GUI that was opened in the save before 14 from the viewpoint of code maintenance. (Open dialogs of save data after 15.0 will be restored)