The International Simutrans Forum

Simutrans Extended => Simutrans-Extended development => Topic started by: jamespetts on July 30, 2011, 11:50:06 PM

Title: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on July 30, 2011, 11:50:06 PM
Simutrans-Experimental 10.0 is nearly ready for release. New features, which come under the approximate theme of routing enhancements, include point-to-point average speeds, Inkelyad's spacing as loading and round trip time patches and the ability of passengers to walk between nearby halts (where one halt is in the catchment area of another) en route. It will also contain fixes for a number of reported bugs as well as optimisations and the removal of redundant code (10.0's Windows executable is 147Kb smaller than 9.12's; the first time in a very long time indeed that a subsequent version has had a smaller executable than a previous version).

There is a new 10.x branch on my Github repository (https://github.com/jamespetts/simutrans-experimental); I should be very grateful indeed if anyone with the ability to compile code could test to see whether it is ready for release. I should be very grateful for any pre-release feedback.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on July 31, 2011, 07:25:46 AM
Quote from: jamespetts on July 30, 2011, 11:50:06 PM
There is a new 10.x branch on my Github repository
I don't see it. Did you mean 'devel' branch?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: dustNbone on July 31, 2011, 11:08:43 AM
Yes that is where you will find it.

Linux x86 SDL and server builds:

http://www.megaupload.com/?d=FK9W2XQ6

Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on July 31, 2011, 11:14:17 AM
Ahh, apologies, I had forgot to push the 10.x branch. It is nearly identical to the devel branch, but I have just pushed one minor fix to it that is not on devel.

Edit: Note that the 10.x branch is now getting all of the bug fixes, rather than devel.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on July 31, 2011, 04:31:03 PM
I'm just about to have a proper play around with the pre-release version, but here's a graphical error I've noticed right away:

(http://dl.dropbox.com/u/61716/vehiclewindow.jpg)

The new "avg trip time" line has displaced the line-identifying section, and means that you can't follow the shortcut to the "list of lines" window.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on July 31, 2011, 05:07:29 PM
Ahh - this appears to be a problem only in Pak64 or similar sized sets. Inkelyad - do you think that you could look into fixing this, as it arose in your code?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: dustNbone on July 31, 2011, 07:06:56 PM
Ooops my bad on jumping the gun and making assumptions :)  I built new binaries a few minutes ago, 8PMish GMT.  Seems to still be a quickly moving target but here we go.

Linux x86 binaries, client and server packed together:

http://www.megaupload.com/?d=CX50WXRA

Dustin
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on July 31, 2011, 07:49:25 PM
I've found some pretty serious trouble with the new point-to-point average speed system, and more specifically with its impact on journey times. In short, the game is throwing up absurd combinations of journey times and passenger routing becomes very problematic as a result.

As far as I can see, there are three main problems.


1. Asymmetry of journey times

-- Journey times are severely asymmetric: there can be large differences between the A-to-B journey time and the B-to-A journey time. In theory these should not differ by more than a few minutes on a normal route, but in practice I've seen examples where A to B takes 25 mins and B to A takes 40 mins.

-- As a result of the above, journey times are not transitive. If a route takes 10 minutes to get from A to B and a further 10 minutes to get from B to C, then the A to C journey time should be approx 20 mins. But here's an example from my test map: A to C takes 125 mins, but B to A takes 54 mins and B to C takes 91 mins -- a total of 143 mins. Multiply this between lines (and interchange stations) and it  causes havoc for routing on a network of even moderate complexity.


2. End-to-end journey times are too short

-- Some journey times seem far too short. Here are two examples from my test map:

70kph vehicle can apparently travel 190km in 120 mins
45kph vehicle can apparently travel 55km in 54 mins

These are both absurd results. This problem is reminiscent of something that came up in a recent version -- 9.7/9.8?

In general, the end-to-end journey times are wildly different from those found in 9.12. In 9.12, the journey time calculations from one end of a route to the other were almost completely accurate -- the problems only came with intermediate stations. The fact that these are very different (and, in general, shorter), suggests that error has been introduced.


3. Penultimate station to terminus station journey times are too long

-- The journey time from the penultimate station on the line to the terminus station is usually excessively long. An examples: 44 mins for a 3km journey on a route with 55kph average speed. Could this be related to the fact that convoys on this line use "convoy spacing" and so spend a long time loading at the terminus station? That is -- could the game be counting the time between a convoy's leaving the penultimate station and its *leaving* the terminus station, rather than its *arriving* at the terminus?



I'll post more problems as and when I find them. The good news is that "walking connexions" appears to be working perfectly and the new system has no significant performance hit.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on July 31, 2011, 08:22:06 PM
Sorry for double-posting -- I've just found another serious problem which is unrelated to the above problems with journey times.

In short: on the "Via (amount)" display, vehicles are trying to take passengers to stops which they do not serve.

Here's a visual example:

(http://dl.dropbox.com/u/61716/via.jpg)

The trouble is that this train never visits "Budapest Ferenc Liszt Airport"! In order to get there, passengers would have to change. But the "Via (amount)" screen only shows the immediate destination for passengers, not final destinations. The upshot is that those Airport passengers will never leave the train.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on July 31, 2011, 10:14:07 PM
Carl,

thank you for the testing and feedback - much appreciated. Looking into these issues, I have not been able to reproduce your fourth problem (in the second post), but can reproduce no. 1 in so far as it applies to trains in the stop before the terminus, as you originally indicated. It is not confined to routes in which the spacing time is set, and I am looking into whether the reversing time is incorrectly taken into account. I have not been able to reproduce problem no. 1 other than in that situation as yet (and my results are far less extreme than yours - an extra few minutes, not 44 minutes for a 3km journey - might your termini be congested such as to lead to delays before the trains enter them?)

As to no. 2, I have not seen this effect. Perhaps you could upload a saved game? Also, have you been able to reproduce this with Pak128.Britain-Ex, or only with Pak64? I wonder whether the difficulties occur more with some distance per tile (or other) settings than others.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on July 31, 2011, 11:02:21 PM
Quote from: jamespetts on July 31, 2011, 10:14:07 PM
might your termini be congested such as to lead to delays before the trains enter them?

This occurred to me, but it seems to happen even on clear termini so I don't think it can be entirely explained this way.

I've only tested using pak64, I'm afraid, but I have tested on both 250m and 400m distance-per-tile settings.

Here are the two games I've used for checking for easy demonstration of these bugs.


Game 1 - (400m distance-per-tile)
Savegame: http://dl.dropbox.com/u/61716/Balkans10testing.sve
Addons folder: http://dl.dropbox.com/u/61716/carladdonsbalkans.rar

The train in the frame when the game loads -- vehicle (106) -- is carrying pax for a destination which it does not serve. 7 passengers wish to go to the Airport, which is not on the route of that train.

The "penultimate stations" bug is well demonstrated at Gyöngyöshalász (25 mins to terminus at Gyöngyös, 3 mins to equivalent-distance station in other direction) and Andornaktálya (62 mins to terminus at Eger, 4 mins to equivalent-distance station in other direction).

If you let the game run for a month (to "iron out" its journey times), you can find a good example of the "journey times are too short" worry by observing the Miskolc to Szeged journey. This is a 190km journey -- but apparently it only takes 130 mins, even on a train which averages just 62kph and takes a far-from-direct route.


Game 2 - (250m distance-per-tile)
Savegame: http://dl.dropbox.com/u/61716/UK10testing.sve
Addons folder: http://dl.dropbox.com/u/61716/carladdons2.rar

This map has been run for a month in 10.0 to update journey times. It does not seem to suffer from the "journey times are too short" worry, signalling a difference between 250m and 400m distance-per-tile values. I'm also unable to find any trains carrying passengers to the "wrong" destinations, like the airport passengers above.

However, the "penultimate stations" worry is definitely still present. See the 17-minute journey from Patterton to Neilston (compared to the 4-minute journey to the next station in the other direction) and the 13-minute journey from Chatelherault to Larkhall (compared to the 4-minute journey to Hamilton Central, the next station in the other direction).
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 01, 2011, 05:00:33 AM
Quote from: jamespetts on July 31, 2011, 05:07:29 PM
Ahh - this appears to be a problem only in Pak64 or similar sized sets. Inkelyad - do you think that you could look into fixing this, as it arose in your code?
I will look itho this.
Quote from: carlbaker on July 31, 2011, 07:49:25 PM
2. End-to-end journey times are too short

-- Some journey times seem far too short. Here are two examples from my test map:
70kph vehicle can apparently travel 190km in 120 mins
45kph vehicle can apparently travel 55km in 54 mins

3. Penultimate station to terminus station journey times are too long

-- The journey time from the penultimate station on the line to the terminus station is usually excessively long. An examples: 44 mins for a 3km journey on a route with 55kph average speed.

Here are we again. Where said minutes come from?
Are you using new Avg time? Or numbers from "line details" dialog?

EDIT:
@james: Do I understand correctly that
1) tiles/month speed does not depends on km/tile value?
2) journey_distance inside convoi_t::laden() does not depends on it too.
3) journey_time inside convoi_t::laden() does not depends on bits_per_month

1 -> it is useless for player to do any calculation using distance (in km) and speed (km/h)?
2 & 3 -> what is stored inside  average_speed table?

Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: dustNbone on August 01, 2011, 07:01:37 AM
Brief summary of my findings so far:

There are some errors in the journey times listed in the station details, mainly with my ships.  In one example the average speed in the line details is correctly shown as 20, but the journey times for 44.5km and 55.75km are 6 and 12 minutes respectively. It's a dogleg route serving three stops, and the 55.75km stop is actually quite alot more distant time wise than then 44.5km one.  This is a pretty extreme example, mainly seems to be the ships.  The only thing special about that line is that it's serviced quite frequently, 24 convoys a month leaving each stop. 

One other thing that is different, when I assemble a train at a depot and send it out to it's first stop, it sits at the station loading until I manually set its schedule to the next stop on the line, then it leaves, and next time it comes back to the end of the line reverses and carries on normally.  This isn't necessarily a problem, it's actually kind of handy for spacing trains out once you realize that it's doing that, but it's different behaviour from 9.12.

I'm going to spend some time today testing, it's a holiday Monday here.  Also, I couldn't compile the last commit on the github, the one described "FIX: Passengers would route to their final stop with a walk, but would not actually walk the final leg." It croaked on simhalt.cc, forgot to save the compiler output.  I'll run it again and post the exact build error.

Dustin
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 01, 2011, 08:38:41 AM
Quote from: inkelyad on August 01, 2011, 05:00:33 AM
I will look itho this.
Here are we again. Where said minutes come from? Are you using new Avg time? Or numbers from "line details" dialog?
Journey time and distance (km) come from the station details dialog. The average speed comes from the line details dialog.


Journey time from one end of a line to the other -- from terminus to terminus -- should always be at least approximately measurable by dividing total distance by total average speed. If it's not anywhere near this value then surely something's gone wrong.


Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 01, 2011, 09:46:23 AM
A brief reply, as my time is currently limited. Firstly, Inkelyad, to answer your questions:

(1) yes;
(2) yes;
(3) yes.

However, it is not useless for players to undertake any calculation in km and km/h, as the average speeds, the distances and the times shown in the stations are all calculated on a commensurate basis (that is, taking into account distance per tile but not bits per month). One thing about which I do wonder is whether it would be better for your round trip time and loading time displays to be expressed in minutes (i.e., the same minutes as used for journey times) rather than the internal measure currently used, so that players could also use those times for calculation purposes and compare them against other things (as presently, it might be considered somewhat confusing).

Dustin - I have not noticed in my testing the problem that you describe with trains waiting at their first stop, and this is not intended, nor have I changed any part of the code that I can think might account for this. As to the compiling - can you see if you can post a compiler output? What platform/compiler are you using?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 01, 2011, 09:50:13 AM
Quote from: jamespetts on August 01, 2011, 09:46:23 AM
One thing about which I do wonder is whether it would be better for your round trip time and loading time displays to be expressed in minutes (i.e., the same minutes as used for journey times) rather than the internal measure currently used, so that players could also use those times for calculation purposes and compare them against other things (as presently, it might be considered somewhat confusing).

I'll second this -- that would be extremely useful.


Quote
Dustin - I have not noticed in my testing the problem that you describe with trains waiting at their first stop, and this is not intended, nor have I changed any part of the code that I can think might account for this. As to the compiling - can you see if you can post a compiler output? What platform/compiler are you using?

I came across this problem on a brief test of Pak.Britain-Ex last night. A new convoy was apparently "scheduled" to wait at its first station for 320 hours before departing -- even though its line was not subject to any spacing settings. I'll see if I can recreate this later and upload an example.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 01, 2011, 10:16:14 AM
Quote from: jamespetts on August 01, 2011, 09:46:23 AM
However, it is not useless for players to undertake any calculation in km and km/h, as the average speeds, the distances and the times shown in the stations are all calculated on a commensurate basis (that is, taking into account distance per tile but not bits per month).
Are you sure about average speed information? At first glance I can't find where tiles/km is used.

EDIT: but still..
Vehicle speed is 100 tiles/month.
tiles/km == 1 => vehicle speed is 100km/month.
tiles/km == 3 => vehicle speed is 25km/month.

Do speed from pakfile scaled to tiles/km when printed?

Quote
One thing about which I do wonder is whether it would be better for your round trip time and loading time displays to be expressed in minutes (i.e., the same minutes as used for journey times) rather than the internal measure currently used, so that players could also use those times for calculation purposes and compare them against other things (as presently, it might be considered somewhat confusing).
It is goal of my next patch -- it wil create new date/time printing mode, where journey minutes are used.
It will make month of weird length, but it can't be helped.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: dustNbone on August 01, 2011, 07:30:47 PM
Here is the compiler output for my build attempt of f92927f (Latest Git):

===> CXX simhalt.cc
simhalt.cc: In member function 'uint16 haltestelle_t::get_average_waiting_time(halthandle_t, uint8) const':
simhalt.cc:1170: warning: taking address of temporary
simhalt.cc: In member function 'ware_t haltestelle_t::hole_ab(const ware_besch_t*, uint32, const schedule_t*, const spieler_t*, convoi_t*, bool)':
simhalt.cc:1504: warning: comparison between signed and unsigned integer expressions
simhalt.cc: In member function 'uint32 haltestelle_t::liefere_an(ware_t)':
simhalt.cc:1936: error: 'class ware_t' has no member named 'get_zeil'
simhalt.cc:1936: error: 'class ware_t' has no member named 'get_zeil'
simhalt.cc:1936: error: 'class ware_t' has no member named 'get_zeil'
simhalt.cc:1939: error: 'class ware_t' has no member named 'get_zeil'
simhalt.cc: In member function 'void haltestelle_t::rdwr(loadsave_t*)':
simhalt.cc:2580: warning: comparison between signed and unsigned integer expressions
simhalt.cc:2704: warning: comparison between signed and unsigned integer expressions
simhalt.cc:2737: warning: comparison between signed and unsigned integer expressions
make: *** [build/default/simhalt.o] Error 1

Compiler is GCC 4.4.5 on 32 bit Linux.  Maybe it just hates me :)

Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 01, 2011, 09:50:57 PM
Inkelyad - see my post in the other (http://forum.simutrans.com/index.php?topic=7735.new#new) thread in answer to your question.

Dustin - I have realised that this problem is because I forgot to push the fix to my typing errors causing this problem with the latest commit - apologies!

Edit: Inkelyad, incidentally, what do you mean when you write that printing loading/waiting times in terms of journey minutes will make the month length weird? The two should not be connected (see the post to which I refer above on the deliberate distinction between the first and second measures of time).
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 02, 2011, 06:11:29 PM
Quote from: carlbaker on July 31, 2011, 04:31:03 PM
The new "avg trip time" line has displaced the line-identifying section, and means that you can't follow the shortcut to the "list of lines" window.
Fixed in my round_trip_time (https://github.com/inkelyad/simutrans-experimental/tree/round_trip_time) branch.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 02, 2011, 11:10:43 PM
Thank you very much!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 03, 2011, 10:09:04 PM
I have found the cause of the "penultimate station" bug, and also possibly the "passengers for the wrong destination" bug: my alterations to the pathfinding system to accommodate point to point journey times, there was an error that caused the journey times to be recorded backwards: for example, the journey time from A to B would instead be recorded as the journey time from B to A. If convoys did not use linear routes, it is possible that passengers/goods might be loaded for the wrong destination (although I am not sure whether this is the cause of the problem).

The "penultimate station" issue was caused by a reversal (due to the above bug) of a phenomenon which one might expect to result from reversing times: because reversing is counted as part of the journey time for departing convoys (because the convoy arrives at the station, unloads, loads, then tries to depart, and then incurs the reversing delay when, having tried to depart, it realises that it has to go in the opposite direction), the journey time for departing from stations at which reversing occurs to the next stop is higher than the journey time in the opposite direction.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 04, 2011, 07:41:45 AM
Hi James, many thanks for looking into this.

Given that reversing only takes a few minutes for most vehicles, I don't think that reversal time can fully explain many of the discrepancies -- many seem to be out by about 20 minutes in the first map I uploaded, for example.

However, the "backwards journey times" problem sheds some light on what *could* be causing the problem here. I conjecture that journey times are being counted from the point when a passenger joins a service, rather than from the point when that service departs. Where convoy spacing is in effect, a passenger may join a service (e.g.) half-an-hour before it departs. If it's a five-minute journey to the next stop, then the game will erroneously record a 35-minute journey time to that stop.

If this is right then it marks a change on 9.x versions, where convoy spacing has no effect on journey times. Arguably, what's happening is that time which should be counted as part of waiting time is being counted as part of travelling time. Travelling time should only record how long it takes to get between stations, and waiting time should record the time between arriving at a station and leaving that station on another convoy. So the ideal solution would be for journey times to "start" only when a convoy leaves, and for all loading-time up until that point to be counted as waiting time.

Edit: Some data in favour of my conjecture: the severe instances of the "penultimate stations" bug on the first map occur in all and only the termini where convoy spacing is in effect.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 04, 2011, 08:26:04 AM
Quote from: carlbaker on August 04, 2011, 07:41:45 AM
So the ideal solution would be for journey times to "start" only when a convoy leaves, and for all loading-time up until that point to be counted as waiting time.
There is logic in current setup:
Waiting time is "how long we wait for bus/train".
Traveling time is "how long we are sitting inside". Eg. "It take (in average) 2 hours from boarding to getting off".
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 04, 2011, 09:25:21 AM
Yes, there is that.

But I still feel that this behaviour creates a strange discrepancy between lines with convoy spacing enabled and lines without it enabled. Even within the same line there are odd discrepancies. In the example I gave before, imagine that A is the terminus and B the next stop along. For journeys from A to B the game will display something like "35 mins travelling, 2 mins waiting", but for journeys from B to A it will display "5 mins travelling, 30 mins waiting". I think it's confusing for the player for these journey times to be so different when visually they take the same amount of time.

However, I recognise that the question here is "routeing-neutral": either the time in question gets counted as waiting time or as travelling time, and either way connections will work out the same -- so passenger behaviour will be the same either way.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 04, 2011, 09:47:33 AM
Carl,

the behaviour of counting spacing time as travelling time is not different to 9.x, but was is less noticeable in 9.x, as it was all merged into the general average speed there. Inkelyad is right about the logic of it, and there is another inescapable problem: if passengers board at stop A intending to go to stop C, and the train/bus/boat/etc. calls in the meantime at stop B, and there is a spacing time in force at stop B, then there is no sense in which the time that the passengers from stop A sitting on the train/bus/etc. at stop B can be said to be waiting time - it must be travelling time; there is no reason for the same not to apply to passengers boarding at stop B.

Just as in real life, one must be very careful with spacing times with passengers or other cargoes that want fast transport, as having to sit a long time waiting at a stop is likely to increase the total journey time considerably.

However, one workaround for this at terminus stations is to specify the terminus twice on the schedule; for example, in the A-B-C route that we postulated above, if A and C are spacing points, the schedule might be A-A-B-C-C, with the mirrored option enabled and the spacing time set on the first A and the last C; passengers will not board at A if the next stop is A, nor at C if the next stop is C, so the convoy will sit empty whilst it waits for its spacing time to be filled. This cannot, however, be used at intermediate halts (such as B), and nor can it accommodate the spacing feature's function of ending the spacing time if the convoy becomes full.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 04, 2011, 09:58:33 AM
Quote from: jamespetts on August 04, 2011, 09:47:33 AM
there is another inescapable problem: if passengers board at stop A intending to go to stop C, and the train/bus/boat/etc. calls in the meantime at stop B, and there is a spacing time in force at stop B, then there is no sense in which the time that the passengers from stop A sitting on the train/bus/etc. at stop B can be said to be waiting time - it must be travelling time; there is no reason for the same not to apply to passengers boarding at stop B.
Ah yes, that is a big problem -- I hadn't considered the matter of spacing at intermediate stops at all.

Quote
However, one workaround for this at terminus stations is to specify the terminus twice on the schedule; for example, in the A-B-C route that we postulated above, if A and C are spacing points, the schedule might be A-A-B-C-C, with the mirrored option enabled and the spacing time set on the first A and the last C; passengers will not board at A if the next stop is A, nor at C if the next stop is C, so the convoy will sit empty whilst it waits for its spacing time to be filled.

That's a very useful workaround. I think I'll be adopting it! (Another option is to have a vehicle "stable" in a siding and wait out its spacing time there rather than in a platform.)

Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 05, 2011, 12:23:22 AM
I have been investigating changing the loading sequence from:

arrive > unload > load > reverse > depart

to:

arrive > unload > reverse > load > depart

but this is very difficult, as the reversing is tightly bound up with the departing in the code, which is not much changed from Standard. I am wondering whether it is necessary to go as far as introduce a completely new convoy state called "UNLOADING", but this might require some fairly major code surgery. I'd be interested in any views on the matter.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 05, 2011, 03:27:48 AM
Quote from: jamespetts on August 05, 2011, 12:23:22 AM
arrive > unload > load > reverse > depart
to:
arrive > unload > reverse > load > depart
I think it is unnecessary. But you can join reversing time with waiting/loading time too.

Now (Reversing time is 30 min):

LOADING state          REVERSING state
|--------------------|-------------------------------|
    20 min                        30 min

After:

  LOADING            REVERSING
|--------------------|----------|
  20 min                 10min


Should be more or less easy. We have convoi_t::arrival_time now. Tricky part will be not using it when convoy need to reverse between stations(schedule change etc).

Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 05, 2011, 09:29:45 AM
Quote from: inkelyad on August 05, 2011, 03:27:48 AM
I think it is unnecessary. But you can join reversing time with waiting/loading time too.

Now (Reversing time is 30 min):

LOADING state          REVERSING state
|--------------------|-------------------------------|
    20 min                        30 min

After:

  LOADING            REVERSING
|--------------------|----------|
  20 min                 10min


Should be more or less easy. We have convoi_t::arrival_time now. Tricky part will be not using it when convoy need to reverse between stations(schedule change etc).

I must confess, I am not sure that I fully understand, as the only difference between the two diagrams is that, in the second, reversing takes less time; nothing appears to be joined. Also, it is not so much tricky as more or less impossible, I think, to distinguish between this being in intermediate and non-intermediate stations.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 05, 2011, 09:45:04 AM
Quote from: jamespetts on August 05, 2011, 09:29:45 AM
I must confess, I am not sure that I fully understand, as the only difference between the two diagrams is that, in the second, reversing takes less time; nothing appears to be joined.
Convoy reversing time is 30 min on both pictures. On second picture first 20 minutes are shared between loading and reversing.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 05, 2011, 10:07:42 AM
Ahh, I see - this is a totally different thing, then, to not registering the arrival time until reversing has taken place?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 05, 2011, 10:30:05 AM
Now I don't understand you. You are asking how to implement this?

After recent patches we have convoi_t::arrival_time.
Don't touch anything related to LOADING state, but use it in REVERSING state processing.
"if (now - arrival_time > loading_time) { ... }"


Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 05, 2011, 10:39:37 AM
Ahh, no, implementing this should be easy: add a flag as to whether the convoy has hopped since its last time in the loading state, and, if it has not, deduct the longest loading time from the reverse time (although, actually, your system seems even better, as it does away with the extra bool flag).

I was more making the point (in the previous post) that this is a different thing to taking the reversing delay (which is often much longer than the loading delay) out of the time computation for passengers/goods who/which are not on the vehicle at the time of reversing.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 05, 2011, 10:43:10 AM
Quote from: jamespetts on August 05, 2011, 10:39:37 AM
taking the reversing delay (which is often much longer than the loading delay) out of the time computation for passengers/goods who/which are not on the vehicle at the time of reversing.
Then said time will be added to waiting time. I see no difference. Final routing decision is based on total journey time (traveling time + waiting time).

EDIT:

const sint64 departure_time = (arrival_time + longest_loading_time) - reversing_time;
...
go_on_ticks_spacing = (wait_from_ticks + spacing * queue_pos) - reversing_time;

I am not sure why you are doing it that way. (Why not take in account loading time inside REVERSING state processing?) But it seems you forget subtract reversing_time here:

go_on_ticks_waiting = welt->get_zeit_ms() + (welt->ticks_per_world_month >> (16-fpl->get_current_eintrag().waiting_time_shift));
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 05, 2011, 06:54:32 PM
Ahh, it's not that simple: if the loading takes place after reversing, then passengers/goods who/that have arrived after the convoy has started to reverse will be loaded, which will have waited less than the reversing time. Also, to simulate the fact that, in real life, people have timetables, and thus do not turn up at the station five minutes after the train to their destination has departed as often as they turn up to the station five minutes before their train is due to leave, waiting times are halved relative to travelling times, so the distinction between travelling and waiting is important (and not least because waiting time is for all convoys from one stop to a particular destination, whereas travelling time is specific to the one particular line/convoy, and is used for determining which is the best line/convoy to take from that stop).
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 05, 2011, 07:25:20 PM
I still think it is unnecessary.
Quote from: jamespetts on August 05, 2011, 06:54:32 PM
Also, to simulate the fact that, in real life, people have timetables, and thus do not turn up at the station five minutes after the train to their destination has departed as often as they turn up to the station five minutes before their train is due to leave
Hmm. we have  haltestelle_t::loading_here
You can use this in passenger generations.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 05, 2011, 07:32:32 PM
What do you mean?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 05, 2011, 07:37:56 PM
It contains list of loaded convoys. We can generate more passengers when station is not empty. Or even (but it will be processor-heavy) look into connexion table for current convoy.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 13, 2011, 05:43:58 PM
I think that I have found a sensible workaround to the reversing/travelling/loading time issues - have a look at the latest 10.x branch. Note that the saved game format has changed, so games previously saved with 10.x will not load with this new version unless first saved with Experimental version 9 or lower. Further testing would be much appreciated!

Edit: I have now updated this to take Inkelyad's most recent suggestion relating to the loading_here list, such that convoys now continue to load whilst they are reversing.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 14, 2011, 10:44:36 PM
Might I ask those who had problems with the initial run to re-test with a version compiled from the latest 10.x branch?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: dustNbone on August 15, 2011, 12:42:03 AM
Bit of weirdness here with convoys not following their schedules, seemingly at random.  It seems to happen with trains and ships, that's all I've tested.  They leave their terminal station, seem to stutter on the map and in their window it shows them as "reversing", even though they are travelling, they cycle through stops in their schedule and then seem to stop on one much further down the roster, and head directly there.  Savegame: http://dl.dropbox.com/u/38204997/simutrans/June1898.sve

There are 3 ship routes on the map, and it's prominent in all of them, I can't seem to pin down the cause.  I'm building the map to test some of the new features so I've "cheated" a little and many of the stations have huge coverage areas, so I don't need to worry about local transport to generate passenger volume.  Any insight would be great, thanks in advance.  Good work on 10.x

Dustin
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 15, 2011, 02:42:21 PM
I'm getting some similar weirdness. When a train is ready to reverse, it displays "Reversing. left" before teleporting two squares along the track, displaying "Reversing. left" again, teleporting again...and this is repeated three or four times before the train eventually heads off.

Also, as dustNbone reports, trains don't always appear to be following their assigned schedules.

Edit: A further observation on the previously reported bug where passengers would display an initial destination which was not on their vehicle's schedule. This still occurs, but seems only to happen when a game is loaded into 10.0. That is, nobody will board a convoy headed for the wrong destination; this is a problem related to mistaken categorisation of passengers who are already on vehicles at the time the game is loaded.

Edit 2: Can I ask -- what is the '[R]' flag on stations in the Schedule window? Does it concern reversing? On many lines, every stop has this flag -- and on almost all lines, many stations have it which are not reversing points.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: dustNbone on August 15, 2011, 06:46:11 PM
The [R] does signify a stop at which reversing is (supposed) to take place.  Behaviour you describe is identical, reversing,left caption, jumping a couple squares, etc.  I did have the same thing on a train schedule as well, but I withdrew the train service temporarily so that I could try to isolate the problem.  It's not all convoys on the line either, some (maybe even most) leave normally and head to the next station on the schedule, but some just don't behave.

Dustin
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 15, 2011, 06:48:14 PM
Quote from: dustNbone on August 15, 2011, 06:46:11 PM
It's not all convoys on the line either, some (maybe even most) leave normally and head to the next station on the schedule, but some just don't behave.


Yep, that's the same as what I've witnessed.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 15, 2011, 10:04:22 PM
Thank you for the report. I'm looking into this, but it's rather elusive so far. I have fixed another bug in the process (which might have resulted in stations/stops being marked as "[R]" (a stop at which the convoy reverses)) incorrectly.

Any help in tracking down the main issue would be appreciated!

Edit: One thing that would be very helpful in tracking down the problem is if somebody could manufacture a saved game in which there is one single convoy that reliably misbehaves in the way described.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 16, 2011, 08:23:35 AM
Quote from: jamespetts on August 15, 2011, 10:04:22 PM
Edit: One thing that would be very helpful in tracking down the problem is if somebody could manufacture a saved game in which there is one single convoy that reliably misbehaves in the way described.
I'll upload such a save-game later on today.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 16, 2011, 09:23:58 AM
Thank you; that's very kind.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 16, 2011, 08:21:23 PM
Here's a savegame: http://dl.dropbox.com/u/61716/balkansreversing.sve (uses previously uploaded addons set: http://dl.dropbox.com/u/61716/carladdons3.rar)

The trains leaving 'Cegled sidings' will reliably display this behaviour. That is: every other train will display this behaviour. Each time it 'reverses'/teleports it skips a stop on its route, meaning that when it gets out of the siding it's headed for the wrong station. You can see this behaviour repeated by a number of other lines which terminate in the vicinity (e.g. at Kecskemet, slightly to the south).

Edit: Why does this only happen with alternate trains at Cegled sidings? My only guess is that it's explained by the fact that trains alternate platforms at Cegled sidings because of the choose-signals at the entrance to those sidings. So only trains leaving from one of these platforms is subject to the problems. The plot thickens...

I haven't yet compiled the very latest updates to the 10.x branch -- I'll do that now and report any changes in behaviour.
Edit 2: I've compiled the latest sources and the above behaviour is unchanged. Also, some lines still have an '[R]' flag at every stop -- for example, the 'R Szeged - Kecskemet - Szolnok' line.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: dustNbone on August 16, 2011, 08:54:51 PM
Yeah i'm getting the same behavior in my latest build too.  This happens with ships as well, so it's not isolated to platforms with choose signals.  I noticed that if I don't set any convoy spacing or any waiting conditions at all, it doesn't seem to occur but I haven't had enough testing time to say for sure that this is the case. 

Dustin
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 16, 2011, 10:21:40 PM
I think that I have managed to confirm the hypothesis suggested by dustNbone that this does not happen with convoy spacing/waiting: I took Carl's saved game and, on the line where the problem occurs reproducibly, removed all of the waiting time (etc.) settings. This caused the convoys to behave normally. Another thing that I noticed in testing was that, when this problem occurs, the convoy will try to reserve its entire path to its next destination, not just to the next signal, which is odd.

However, in order to be able to fix this, I need to be able to isolate the behaviour in a debugger, which means running a map where there is only one convoy, and that reproducibly causes the problem. It needs to be one convoy because I need to be able to set a breakpoint in the debugger in the code relating to advancing through the schedule and be sure that it is set for the right convoy. My attempts to produce such a saved game have not so far met with success. Is anyone able to assist with producing such a saved game?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Bernd Gabriel on August 16, 2011, 10:48:23 PM
@james: can you set a conditional breakpoint? If yes, give the convoy a unique name, if it hasn't one yet and break only, if the convoy with this name is processed.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 17, 2011, 07:41:38 AM
Quote from: jamespetts on August 16, 2011, 10:21:40 PM
Another thing that I noticed in testing was that, when this problem occurs, the convoy will try to reserve its entire path to its next destination, not just to the next signal, which is odd.

Is this because it's waiting at a choose-signal (which ordinarily will try to reserve a path all the way to the next station)? In my experience this is in line with the normal behaviour of choose-signals.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: chs on August 17, 2011, 11:39:47 AM
I would like to test it.. if anyone could provide windows binaries, that would be appreciated.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 17, 2011, 09:55:02 PM
I think that I might well have fixed the erratic behaviour issue - can people re-test?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: dustNbone on August 18, 2011, 12:37:23 AM
It seems that you did :) Good work James.

Dustin
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 18, 2011, 09:30:37 AM
Do people think that 10.0 is ready for release yet?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 18, 2011, 11:13:30 AM
It seems that the most recent bug has been fixed -- thanks James!

The one main lingering trouble is journey times and their relation to distances/average speeds. The question of release-readiness depends on how important fixing these bugs is perceived to be.

I've uncovered some new data on this problem which is very interesting. When playing with a 400m distance-per-tile setting (that is, a value of 40 in simuconf), the game still displays impossibly short journey times for all routes. For instance: a 20km journey in a 60kph-average vehicle apparently takes only 13 minutes. All things being equal, we should expect the journey to take around 20 minutes. However, when playing with a 250m distance-per-tile setting (that is, a value of 25 in simuconf.tab), the journey time calculations appear to be sensible.  

What's more, I've an idea of how the incorrect journey times at 400m distance-per-tile is being reached. The 13-minute journey time quoted above is wrong -- but it *would* be correct if the game were instead being played under a 250m distance-per-tile setting rather than a 400m setting. At a value of 400m-per-tile, 20.8km (the exact distance in question) is equivalent to 52 squares. 52 squares at 250-per-tile is 13 kilometres. Since our vehicle averages 60kph, we would expect it to travel 13 kilometres in 13 minutes. And, lo and behold, 13 minutes is the value given for journey-time!

What appears to be happening is the following. At 400m distance-per-tile, the game displays the correct km value for journey distances but uses an incorrect km value for its journey time calculations. In the journey time calculations it seems to be assuming that distance-per-tile is 250m, even though it is set at 400m by simuconf.tab (and even though the game's displayed km distances are measured by 400m-per-tile).

Does this make sense? I've run this against a couple of test maps and the hypothesis seems to check out. The erroneous journey times all seem to be entirely sensible if we assume that the game is mistakenly taking a 250m per tile rather than 400m per tile value in journey-time calculation.

Edit: I've just tested this on an 800m-per-tile map (i.e. with a value of 80 for distance-per-tile in simuconf.tab), and my hypothesis has been further confirmed. The game says that a 92kph vehicle can travel 44km in 9 minutes, which is absurd. Now 44km is equivalent to 52 tiles at 800m-per-tile, and 52 tiles at 250m-per-tile is 13km. And 92kph vehicle should take around 9 minutes to travel 13km -- and 9 minutes is the journey time given by the game. At some point the game is mistakenly assuming that distance-per-tile is 250m rather than 800m.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 18, 2011, 01:14:36 PM
Quote from: carlbaker on August 18, 2011, 11:13:30 AM
What appears to be happening is the following. At 400m distance-per-tile, the game displays the correct km value for journey distances but uses an incorrect km value for its journey time calculations. In the journey time calculations it seems to be assuming that distance-per-tile is 250m, even though it is set at 400m by simuconf.tab (and even though the game's displayed km distances are measured by 400m-per-tile).
I thought we no longer use speed and distance in journey time calculations?

I slowly (it is hot here, and it is not healthy for my computer and me) do cleaning in related code so I will look into it. Again.

Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 18, 2011, 04:20:01 PM
Quote from: inkelyad on August 18, 2011, 01:14:36 PM
I thought we no longer use speed and distance in journey time calculations?
I'm afraid I'm completely ignorant of how it works in the code -- but the point is that whatever measure the code uses to calculate journey times isn't sensitive to changes in distance-per-tile value. So the higher the value the bigger the error in journey times.


Quote
I slowly (it is hot here, and it is not healthy for my computer and me) do cleaning in related code so I will look into it. Again.
Thanks for looking into this, inkelyad. Sorry to keep going on about it -- I hope that the new data I've mentioned helps in the search for a solution.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 18, 2011, 04:53:29 PM
Quote from: carlbaker on August 18, 2011, 04:20:01 PM
I'm afraid I'm completely ignorant of how it works in the code -- but the point is that whatever measure the code uses to calculate journey times isn't sensitive to changes in distance-per-tile value. So the higher the value the bigger the error in journey times.
It is supposed to be that way. Printing/output functions should care about all conversions.
But it is very unclear when you read code.

Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 18, 2011, 08:25:38 PM
Journey times must still be adjusted for distance per tile, because neither speed nor distance are so adjusted (or ought be).

Edit: I have now pushed what I hope is a fix for the journey times/distance per tile issue, along with some code cleanup to standardise conversions from ticks to minutes to use the distance per tile setting properly. Further, I have changed the time displayed in the tool-tip for reversing/loading, and in the depot display, to use the same time scale as journey and waiting times, which is more useful than using the scale of time used for passing months/years.

I should be grateful if people could check my mathematics and test whether this works and successfully fixes the reported bugs. Thank you all for testing!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 19, 2011, 08:31:01 AM
Many thanks for this James! I've done some "straight-line testing" -- that is, simple toy lines with only a couple of stops, to abstract from the effects that complex networks can have on journey times -- and the initial results suggest that the bug is still present. Here's some data paired with simple savegames:

Distance-per-tile (dpt): 25
Journey distance=23.5km
Avg speed=88kph
Expected journey time (calculated manually from distance and speed)=16 mins
Game's actual displayed journey time=16 mins
Comments: As ever, correct result given at 25 dpt.

Dpt=40
Journey=33.6km
Avg speed=63kph
Expected=32 mins
Actual=21 mins
Savegame: http://dl.dropbox.com/u/61716/dpt40.sve
Comments: Out by a factor which is close to 40/25.*

Dpt=50
Journey=32km
Avg speed=50kph
Expected = 38.5mins
Actual = 20 mins
Savegame: http://dl.dropbox.com/u/61716/dpt50.sve
Comments: Out by a factor of approximately 50/25 (i.e. a factor of 2)

Dpt=75
Journey = 67.5km
Avg speed = 36kph
Expected = 112 mins
Actual = 38 mins
Savegame: http://dl.dropbox.com/u/61716/dpt75.sve
Comments: Out by a factor of approximately 75/25 (i.e. a factor of 3)


Thanks for making the change regarding time displayed for loading/reversing/etc. I have one question about this. When the game displays (for example) "Loading 1:00", does this denote one minute or one hour? If it denotes one minute then (if my previous calculations are correct) then these displayed times seem right. If "1:00" denotes an hour then I think they are out by a factor of around 60 -- convoys  load for around "2:00" before leaving each station, which would be absurd if "2:00" denotes 2 hours.




* Confusingly, I've found much more ambiguous results on the Balkans test map that I uploaded previously -- on that map, where dpt=40, the journey times seem far more plausible and certainly don't seem to be out by a factor of 40/25. I'll have to play around with this map to verify this.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 19, 2011, 09:32:29 AM
Carl,

thank you for the tests - I am not sure what the problem is, but shall look into those as soon as possible. Are those all saved in pak.ee (the same as the Balkans setup)? As for the loading times, they are indeed intended to be mm:ss rather than hh:mm, as a loading time of several hours is quite implausible.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 19, 2011, 10:11:34 AM
Quote from: jamespetts on August 19, 2011, 09:32:29 AM
Are those all saved in pak.ee (the same as the Balkans setup)?
Yes, they are -- sorry, I forgot to mention that in the previous post.

Quote
As for the loading times, they are indeed intended to be mm:ss rather than hh:mm, as a loading time of several hours is quite implausible.
Great -- thanks for the clarification.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 20, 2011, 11:27:23 AM
I think that I have found the trouble - the average speed was calculated without taking into account the distance scale. I have pushed a fix - I should be grateful if people could re-test.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 20, 2011, 12:32:08 PM
James,

Initial testing suggests that the bug has been fixed. Many thanks for your work on this! :)
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 20, 2011, 12:44:49 PM
Thank you for testing!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: wlindley on August 20, 2011, 02:09:40 PM
I built a new Fedora system and have done this to create a build environment:

sudo yum install git gcc gcc-c++ libzip-devel bzip2-devel libpng-devel allegro-devel SDL-devel SDL_mixer-devel
git clone https://github.com/jamespetts/simutrans-experimental


edited the config file appropriately (BACKEND=sdl, COLOUR_DEPTH=16, OSTYPE=linux) but compile fails with this error:

simhalt.cc: In member function 'uint16 haltestelle_t::get_average_waiting_time(halthandle_t, uint8) const':
simhalt.cc:1378:55: error: taking address of temporary [-fpermissive]


Suggestions?  And... am I actually compiling 10.0 this way -- or where exactly do I find 10.0 if not?  I looked at the github page but there's nothing there about different versions. Nor does the Git tutorial linked off the wiki even mention how the "git clone" command or explain how to access different versions. 

p.s., I have been writing C programs since 1981 but there are so many unknowns with a new project; I edited the wiki (http://simutrans-germany.com/wiki/wiki/tiki-index.php?page=en_CompilingSimutrans#How_to_compile) with the actual, exact names of the compilers and libraries requried, after spending half an hour trying to find them.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 20, 2011, 02:44:15 PM
This compiler "error" is actually a warning treated as an error by default, which can be disabled by using the -fpermissive switch. However, I am confused by the line number references: line 1378 of simhalt.cc is:


void haltestelle_t::add_pax_too_slow(int n)


That does not appear, however, to be the line to which the compiler is referring. Can I ask - are you using the master branch from Github? If so, that is the latest release version (9.12). You will need to switch to the 10.x branch to compile the pre-release 10.0.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 20, 2011, 02:58:42 PM
Quote from: jamespetts on August 18, 2011, 08:25:38 PM
Further, I have changed the time displayed in the tool-tip for reversing/loading, and in the depot display, to use the same time scale as journey and waiting times, which is more useful than using the scale of time used for passing months/years.
My branch internal_minutes (https://github.com/inkelyad/simutrans-experimental/tree/internal_minutes) add new show_month value. It will display internal minutes as left corner clock.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 20, 2011, 04:30:16 PM
This is interesting - thank you for that. Is this in HH:MM or MM:SS? Either way, it seems to need a further layer: in the first instance, DD:HH:MM or the second HH:MM:SS, as showing 191 hours (or minutes, as the case may be) is not ideal.

Also, if this is in HH:MM, it would seem inconsistent with the other timings that use MM:SS (inconsistent both in format and in actual duration).
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 20, 2011, 05:00:39 PM
Quote from: jamespetts on August 20, 2011, 04:30:16 PM
This is interesting - thank you for that. Is this in HH:MM or MM:SS?
It is direct usage of karte_t::sprintf_ticks.
Quote
Either way, it seems to need a further layer: in the first instance, DD:HH:MM or the second HH:MM:SS, as showing 191 hours (or minutes, as the case may be) is not ideal.
It is deliberately so. To avoid confusion with calendar hours/minutes.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 20, 2011, 05:22:22 PM
I think that it is clearer to use hours as well as minutes - given your new format, it should not cause confusion with calendar days. See my latest push to the journey-minutes-display branch.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 20, 2011, 06:27:59 PM
Well, we need more responses.
What is better:
Month is 191:59 long
or
Month is 3:11:59 long.
Remember, it will be used for spacing/spacing shift values too.

EDIT:
59 seconds is rounding error.
If you use exact formula:

tenths_of_minutes = get_settings().get_meters_per_tile() * ticks * 30L / (4096L * 1000L)
and
seconds = get_settings().get_meters_per_tile() * ticks * 30L * 6L / (4096L * 1000L);
You will get 192:00 or 3:12:00
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 20, 2011, 07:24:18 PM
Thank you for the fix - most helpful.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 20, 2011, 07:31:11 PM
Can you fix my English and put calculations from here (http://forum.simutrans.com/index.php?topic=7735.0) into commentaries? Somewhere in simunits.h?
It will be very helpful for new developers.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 20, 2011, 07:40:02 PM
Didn't we come to the conclusion that those calculations weren't correct; or am I mis-remembering or mis-understanding?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 20, 2011, 07:55:59 PM
I was not sure about them. But when you made karte_t::ticks_to_tenths_of_minutes you prove they are correct.
Compare
tenth of minute = 4096/30 ticks when meters_per_tile == 1000
and
get_settings().get_meters_per_tile() * ticks * 30L / (4096L * 1000L)
and
get_settings().get_meters_per_tile() * ticks * 1/136533L
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 20, 2011, 08:03:45 PM
Hmm, I see. I'm not quite sure how this works, I have to say, but if the calculations are equivalent, then there's no arguing with that. Would it not be better to put the comments in karte_t::ticks_to_tenths_of_minutes ?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 20, 2011, 08:05:51 PM
At least put in simunits.h "look to karte_t::ticks_to_tenths_of_minutes".
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 21, 2011, 12:00:47 AM
I have added comments based on the calculations to simnuits.h - can you check that these are correct?

I have also pushed some changes to remove bugs and increase accuracy in the calculation of departure/loading/reversing/journey/waiting times; I should be grateful if people could re-test.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 21, 2011, 07:09:17 AM
Quote from: jamespetts on August 21, 2011, 12:00:47 AM
I have added comments based on the calculations to simnuits.h - can you check that these are correct?
I think we must explain 104857600/1280 too.
Something like this:

*      -- (express for journey, waiting, reversing, loading and spacing times in Experimental)
*      100 tiles = 256000 steps = 104857600 yards
*      100 km/h = (100 << 10) / 80 "yards"/tick = 1280 "yards"/tick. (see macro kmh_to_speed below)
*      -- Assuming 1000 meters per tile, 1h = 104857600/1280 = 81920 ticks;
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 21, 2011, 08:44:47 AM
Quote from: jamespetts on August 21, 2011, 12:00:47 AM
I have also pushed some changes to remove bugs and increase accuracy in the calculation of departure/loading/reversing/journey/waiting times; I should be grateful if people could re-test.

For the most part everything seems to be running like clockwork. Just two issues to report:

Firstly, although almost all of the new point-to-point journey times seem to be spot on, in my test game I've found two isolated cases where they cannot possibly be right. While it may seem obtuse to get too worried about a couple of isolated cases, I'm reporting them in case they're symptomatic of some broader error. I think the problem may be related to waypoints.

Here's a savegame for demonstration: http://dl.dropbox.com/u/61716/Balkansnewtesting.sve (requires the previously-uploaded addons folder).

The displayed journey time from Cegled to Budapest Nagy is just 16 minutes for a 66km journey. This would require a point-to-point average speed of around 250kph, but the vehicles on this route have a top speed of only 130kph. This error occurs at only one other place on the map that I can find -- the journey from Pusztaszabolcs to Budapest Rakoczi. What the two cases have in common is that both lines visit a waypoint between the two named stations. Perhaps the presence of the waypoint is interfering with the correct calculation of point-to-point average speed.

Edit: Here's a toy map which I think confirms my suspicion here: http://dl.dropbox.com/u/61716/waypointtoy.sve
Journey time from A to B (without waypoint) - 26 mins
Journey time from B to A (with waypoint approx halfway) -- 14 mins

Edit 2: Looks like this toy map might have uncovered another bug. Check out the vehicle's behaviour when it passes the level crossing going eastbound. It slows down to 16kph and won't speed up again until it reaches the waypoint. This doesn't occur when going westbound. This is very reminiscent of the bug where road vehicles won't go faster than 16kph at certain distance-per-tile values.

---

Secondly, a very minor aesthetic request. Currently when a convoy is waiting for over an hour it displays a loading time of (for example) "1:3:15". Would it be possible to reformat this so it reads "1:03:15", i.e. with an extra zero in the minutes column?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 21, 2011, 12:27:08 PM
Carl,

thank you very much for those reports. I have fixed those errors - would you mind re-testing? I have also added the leading zero before the minutes when there are hours - thank you for that suggestion.

Inkelyad,

I have added this comment as suggested; thank you for the suggestion.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 21, 2011, 01:04:34 PM
Those bugs all seem to be fixed now -- thanks very much! As far as I can see, everything seems to be working swimmingly now with journey times, loading times, reversing times, etc.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: wlindley on August 21, 2011, 03:37:50 PM
OK I got past the error by changing the last line of config.default to:

FLAGS = -DNDEBUG -fpermissive

And I successfully switched to the 10.x branch --

$ git branch -a
$ git checkout remotes/origin/10.x


Everything compiles now, and runs my existing pak128.Britain savegames... everything looks good so far, thanks folks!

Is it correct that  $ git pull will keep me up to date, like svn update would on Subversion?  (Still struggling to convert my mindset from svn... we have come a long way from rcs.)  Maybe we can add those few bits into the header topic of this thread, or is there a wiki page I should edit...
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 21, 2011, 03:54:03 PM
Glad that it works for you! I'd not considered maintaining our own Git documentation here - is it not enough to refer to Git's own documentation? However, Git Pull is not enough, since you will have to pull and then merge with your local copy.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Junna on August 21, 2011, 04:06:57 PM
Are we likely to get a release today, then, as things are looking rather well?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 21, 2011, 04:27:56 PM
I'm running out of time to-day, I'm afraid, as I have some work to do for Monday; also, I'd like to make sure that the Nettool (http://forum.simutrans.com/index.php?topic=7862.new#new) issue is fixed before release. However, I am anticipating a release within a week if all goes well.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 21, 2011, 05:33:52 PM
Quote from: carlbaker on August 21, 2011, 01:04:34 PM
Those bugs all seem to be fixed now -- thanks very much! As far as I can see, everything seems to be working swimmingly now with journey times, loading times, reversing times, etc.
Not so fast.

1) See spacing savegame. It is convoy with 1cnv/month spacing and zero shift. But it departs not at 00:00.
Difference is small, but it exists.

2) See waiting savegame. Now it is plain old 'wait for 1/32 month'. In current timescale it is 6:00. Reversing time is about 2:30. So convoy must leave station in 6:00minutes after arrival. But it wait about 2:00 more.

EDIT:
James, can you comment on reverse-related variables?

bool reversable;
bool reversed;

Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 23, 2011, 10:02:19 PM
Inkelyad,

thank you very much for your reports. I think that I have fixed the second problem (please re-test), but the first is more illusive: it is not clear to me at present what is causing it (I can reproduce it, however). It seems to be a relatively minor problem (especially if all spacing shifts are off by exactly the same amount). Do you have any idea what might be behind it?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: wlindley on August 24, 2011, 02:18:29 AM
A little git help please.  I have googled and googled this, read the O'Reilly git book, and tried to figure what's going on, but I am more confused more now than ever.  'git pull' does nothing.  It all seems to start with this problem:

$ git status
# Not currently on any branch.
nothing to commit (working directory clean)
$


OK so apparently I wound up in "detached HEAD" state.  This returns me to the 'master' branch:

$ git checkout master
Previous HEAD position was 5e66af2... CODE: Minor formatting changes.
Switched to branch 'master'
$ git status
# On branch master
nothing to commit (working directory clean)


OK. Now I want to go to the 10.x branch, so let's verify where I am going:

$ git branch -r
  origin/10.x
  origin/8.x
  origin/9.x
  origin/HEAD -> origin/master
...


Alright, there it is.  Let's switch to it:

$ git checkout origin/10.x
Note: checking out 'origin/10.x'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b new_branch_name

HEAD is now at 5e66af2... CODE: Minor formatting changes.
$ git status
# Not currently on any branch.
nothing to commit (working directory clean)


Aaaaarrrrrrghhhh...  I listed the remote branches, and tried to switch to one, but it didn't!!! Why don't I get "On branch origin/10.x" instead of "Not currently on any branch" ...? 

(bashing head against desk)


Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 24, 2011, 10:59:07 PM
I've found a crash on the 10.x branch.


FATAL ERROR:
array_tpl<T>::[]
index out of bounds: -1
not in 0..3, T=class
vehikel_t *


This crash is tied to a particular vehicle, as demonstrated in the following savegame: http://dl.dropbox.com/u/61716/NewCrash1.sve. The addons folder (which has been updated since I last uploaded a save) is here: http://dl.dropbox.com/u/61716/carladdons4.rar.

Upon loading the game you'll see a train leave a depot. When it arrives at the station, the game will crash. Some bullet points:

-- The crash doesn't occur under the same conditions in 9.12.
-- The crash only occurs with this type of vehicle, and only when the train is formed of one car. When it's formed of two or more cars the crash won't occur.
-- The vehicle in question is one I've made myself, but since this crash doesn't occur in 9.12 I'm hoping that the trouble isn't *entirely* down to my lack of techincal nous.  :)


(Edited to upload a simpler savegame so as to make it easier to isolate the effect)
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on August 25, 2011, 03:30:04 PM
Hi

If you can help I have compiled the simutrans experimental (update 24/08/2011) using Mingw.
The file is compiled for windows and you can download at this address.

http://simutrans-germany.com/files/upload/simutrans-experimental_20110824.zip

More information on how I compile it using mingw can be found in this thread.

http://forum.simutrans.com/index.php?topic=6512.0

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 25, 2011, 09:16:40 PM
Carl,

I think that I have fixed the crash - do you want to re-test? Thank you for the report.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 26, 2011, 12:08:15 PM
Thanks James, that's fixed now.

Just wanted to note a change since the last time I compiled. Previously vehicles on lines with convoy spacing would display how long they were waiting, like so:

(http://dl.dropbox.com/u/61716/loading1.jpg)

Now, however, the "time left" is gone and the vehicles just say "Loading":

(http://dl.dropbox.com/u/61716/loading2.jpg)

Was this change intentional? The "Time left" information is very useful, so if it were possible to have that feature reinstated I'd be grateful.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on August 26, 2011, 12:56:06 PM
Hello

both pieces of information seem interesting ... you can leave them both, or line becomes too long?

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on August 26, 2011, 01:09:15 PM
Hello

I have compiled "experimental simutrans 10.0" (github updated 24/08), I downloaded the 0.4.9 pak96c to prove it. Launching the game I get the error reported in this thread: http://forum.simutrans.com/index.php?topic=7711.msg75000; topicseen # msg75000.
But I tried with the version "9.12" and the game started without any problems ...

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 26, 2011, 07:14:55 PM
Quote from: Milko on August 26, 2011, 12:56:06 PM
Hello

both pieces of information seem interesting ... you can leave them both, or line becomes too long?

Giuseppe
I agree in prinicple, but if only one piece of information can be displayed in practice then (IMO) it should be the "time left". The loading level can be easily determined by clicking on the convoy and seeing how many people are on-board. The "time left" is much harder to calculate this way in many cases.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: dustNbone on August 26, 2011, 07:26:14 PM
+1 on this.  Loading time left seems more valuable to have at a glance.

Dustin
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on August 27, 2011, 12:00:58 AM
Hello

I have this problem in the goods list

the names are duplicates of the goods and the revenue is no longer visible

second problem: in the version "9.12" changing the value of the speed bonus (timeline on) did not change the revenue obtained by transporting the good, correct? I could not see what happens with version "10.0".

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 27, 2011, 03:16:46 PM
Giuseppe,

thank you for the report on the goods list problem. I have fixed that, and also added a little more functionality to the goods list. I should be grateful if you could re-test.

As to the loading times issue: this is an error, not an intended feature, which I shall look into fixing. Edit: Fixed - please re-test. Thank you for the report!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 27, 2011, 10:49:31 PM
The loading times issue appears to be fixed -- many thanks once again, James!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 27, 2011, 10:50:57 PM
Thank you for re-testing! Glad that it's fixed.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 28, 2011, 03:40:26 PM
The new 'show_month=8' option allows for verification of the journey times displayed in the Stop Details window. After quite a lot of testing, I've found some reasons to think that these journey times are not always quite accurate.

Here's a toy map for demonstration: http://dl.dropbox.com/u/61716/journeytimes.sve (requires addons folder http://dl.dropbox.com/u/61716/carladdons4.rar, as ever).

The headline findings from this map are as follows: point to point lines with only two stops give accurate journey times, but more complex lines of several stops give journey times which are too short by a factor of between 10% and 35%. A more detailed analysis follows:

There are two lines on this map. Line (2) has only two stations. The GUI displays a journey time of around 12-13 minutes. If we set show_month to 8 and follow the convoy, we will see that it does indeed take around 12 minutes from end to end.

Line (1) has several stations. The GUI displays a total journey time of around 30-31 minutes. However, if we follow the convoy, we see that it reliably takes around 38-39 minutes to get from end to end. (Note that the "avg trip time" displayed in the vehicle window for Line (1) appears to be accurate, since it displays a value of 1h 24m.)

I can confirm that this effect occurs on a larger scale. On my larger map, this effect seems to occur on more or less all lines. For instance: there are cases where a journey advertised as being 2.5 hours long reliably takes 3 hours.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: sdog on August 28, 2011, 03:55:14 PM
I did some superficial testing, it compiled and ran without crash on my system (ubuntu 64bit). Didn't look deeply enough to check things like journey times etc.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 28, 2011, 06:39:08 PM
Quote from: carlbaker on August 28, 2011, 03:40:26 PM
Here's a toy map for demonstration: http://dl.dropbox.com/u/61716/journeytimes.sve (requires addons folder http://dl.dropbox.com/u/61716/carladdons4.rar, as ever).
Line (1) has several stations. The GUI displays a total journey time of around 30-31 minutes. However, if we follow the convoy, we see that it reliably takes around 38-39 minutes to get from end to end. (Note that the "avg trip time" displayed in the vehicle window for Line (1) appears to be accurate, since it displays a value of 1h 24m.)
Simutrans don't use real waiting time to compute total journey time.
from haltestelle_t::get_waiting_minutes:

// Waiting time is reduced (2* ...) instead of (3 * ...) because, in real life, people
// can organise their journies according to timetables, so waiting is more efficient.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 28, 2011, 06:41:13 PM
Yes -- sorry, the times I was referring to are the travelling times rather than the complete journey times. I take it that travelling times don't make any reference to waiting times.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 28, 2011, 06:43:25 PM
I've found the problem with this, which is that, currently, the only actual journey times booked are journey times between consecutive halts: otherwise, timings are based on the fallback average speed system. This was a mistaken omission from the original design of point to point average timings, which I am currently working to correct: however, this will require a hashtable of departure halts | times for each convoy, which will increase memory consumption somewhat.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: dustNbone on August 28, 2011, 07:05:20 PM
I don't think memory consumption is really a huge issue as long as it's within reason.  Considering the complexity of the game and in comparison to other modern games, Simutrans has a very small footprint.  CPU usage gets a bit tight sometimes but memory is the one thing that I think most of us have in ample supply.  I can't speak for everyone however.

Dustin
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 28, 2011, 09:49:00 PM
I have now pushed a fix for this to Github - can people test?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 28, 2011, 10:44:30 PM
1. The problem appears to persist, as demonstrated by the following savegame which was created and saved in the newest version: http://dl.dropbox.com/u/61716/journeytimes1.sve

There are two lines with multiple stops in this savegame.  Line (1) has nine stops, and line (2) has four. The error in the line (1)'s travelling time is much larger -- it's displayed as 14 mins when it is in fact 28 mins. Line (2)'s times are much closer to the truth; 12 minutes when they are in fact 15 mins. I've no idea whether this will be helpful but I thought I should mention it!


2. The new version is unable to open any savegames from previous 10.x versions. The following error is given:


FATAL ERROR:
loadsave_t::rdwr_str()
string longer (2820) than
allowed size (128)


Is this an unavoidable consequence of the changes made?

Even a 10.x game set to save in the 9.x save-format via the settings menu cannot be opened in the new version -- although this yields a standard Windows crash, not the error quoted above.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 28, 2011, 10:58:49 PM
Carl,

thank you very much for testing this. Taking the second point first, I needed to update the saved game format in order to accommodate the new data. This will inevitably break previous saves with the same version number but a different format. I have checked, and it loads without difficulty games saved in 9.12 (and other 9.x versions), so I am a little confused that it is having trouble loading games saved as 9.x with previous pre-release 10.x builds - have you tried loading those builds in 9.12?

As to the main issue, I shall have to look into this further. Do you notice any change in behaviour between the latest build and the previous situation?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 28, 2011, 11:24:44 PM
Quote from: jamespetts on August 28, 2011, 10:58:49 PM
I have checked, and it loads without difficulty games saved in 9.12 (and other 9.x versions), so I am a little confused that it is having trouble loading games saved as 9.x with previous pre-release 10.x builds - have you tried loading those builds in 9.12?

Loading the savegames in 9.12 is generally successful. On further testing, I have been able to create a file in a previous 10.x build, save it as a 9.x file, and load it in the latest 10.x build. However, not all such games are so lucky. The following file cannot be opened in the latest build, despite having been saved as a 9.x file:  http://dl.dropbox.com/u/61716/canisavein9.sve (http://dl.dropbox.com/u/61716/canisavein9.sve). Have you any idea why this might be? (Edit: Note that this file appears to load fine in 9.12.)


Quote
Do you notice any change in behaviour between the latest build and the previous situation?

Since I can't use the same savegame it's difficult to make a perfect comparison. But I've just set up a new savegame in the previous build which is similar in format to "journeytimes1.sve". In this savegame, the line with 9 stops has a more accurate travelling time than in the latest build -- displayed as 21 mins when it is in fact 29. So there might be reason to think that the latest build is less accurate than the previous situation.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 28, 2011, 11:37:03 PM
Hmm - this is odd. The game that you still can't get to work - does it open in 9.12, but fail to open in 10.x even though saved in 9.12; or does it fail to open in 9.12?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 28, 2011, 11:41:20 PM
It can be opened in 9.12, and once opened in 9.12 and saved with 9.12 it can be opened with the latest 10.x build. So it seems that 10.x is failing to save "properly" in 9.x format, somehow. (Although that doesn't explain why 9.12 can open it but the latest build can't.)
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on August 29, 2011, 12:02:27 PM
Hello

I built a simutrans-exp for windows. Updated 29/08.

I publish the file if someone wants to try and not be able to compile it. http://simutrans-germany.com/files/upload/simutrans-experimental_2908.zip

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 02:14:51 PM
I think that I have fixed the issue with the journey times: would anyone care to re-test? Note that the correct journey times will only appear after the path explorer has refreshed the connexions data once at least one convoy in the line has recorded average journey times for the route in question. The problem with incorrect reverse markers has also been fixed.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 03:31:10 PM
Something odd has happened to the journey times in this version. See attached savegame: http://dl.dropbox.com/u/61716/journeytimes2.sve

In this save all of the "travelling times" lists read like multiplication tables. For instance:

From Station A:

to Station B, 6:06 mins
to Station C, 12:12 mins
to Station D, 18:18 mins
to Station E, 24:24 mins
to Station F, 30:30 mins
to Station G, 36:36 mins
to Station H, 42:42 mins



I can confirm that this happens in both newly-created maps and older maps. It's a different multiplier each time, but the journey times on each line take this times-table structure. The stations in question are not equally far apart, so this couldn't be right even with the average speed-based system.

Note that this "multiples" phenomenon seems to occur even before the path-to-path average speeds could have been calculated -- i.e. when a line has just been built.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 03:32:57 PM
Carl,

thank you. This suggests that there might be a problem with the fallback system for calculating journey times. Can you run the game fast-forward for about two months and see whether the problem persists? If not, the problem will be only with the fallback system. If it does persist, the problem may be more serious.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 03:35:22 PM
Hi James,

This pattern of times occurs both before and after the two-month point. The uploaded game has been run for almost a year and nevertheless displays this behaviour.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 04:04:53 PM
Carl,

thank you for that report. The problem was indeed with the fallback mechanism. I have uploaded a fix; can you re-test? (Note that I have added a new simuconf.tab setting, which is saved: your saved games will not work unless you save them as 9.x first, or use a debugger and skip line 1257 of einstellungen.cc when loading).
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 04:28:51 PM
Quote from: jamespetts on August 29, 2011, 04:04:53 PM
Carl,

thank you for that report. The problem was indeed with the fallback mechanism. I have uploaded a fix; can you re-test? (Note that I have added a new simuconf.tab setting, which is saved: your saved games will not work unless you save them as 9.x first, or use a debugger and skip line 1257 of einstellungen.cc when loading).

Hi James,

Thanks for this. At the moment I can't get this version to run at all -- I get the following error:

loadsave_t::rdwr_str() expected "<i16>", got "</ei>"

This error occurs when trying to run the program.

Do I need to change/add something to simuconf.tab in order to get it to work?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 04:35:14 PM
Carl,

yes - you need to delete settings-experimental.xml (or settings-experimental-debug.xml if you're running without -DNDEBUG defined), as the settings format has been incremented without incrementing the version number.

Giuseppe,

I have split your post into a new topic so that we can keep this topic devoted to testing 10.x.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 04:54:52 PM
James,

Thanks -- I should have thought to try that!

The point-to-point average speeds system still doesn't seem to be kicking in -- I think all travelling times are being given by the fallback system. Here's another map: http://dl.dropbox.com/u/61716/journeytimes3.sve

This map has been run for almost a year, so I guess that point-to-point speeds should have kicked in by now. But line (2) demonstrates that they can't have. Half of its route is on track with a low speed limit, so we'd expect that half of the route to take longer. But the travelling times given don't reflect this -- rather, it's clear they have both been calculated using the same average speed. That is, the behaviour here resembles that found in 9.12 and previous versions.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 05:46:00 PM
Carl,

thank you for that testing. The problem was, I think, that the data were being over-written because they were reset on being read, but for some reason were read twice, and had been reset by the second time. I think that I have fixed this now - can you re-test?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 06:21:28 PM
I'm afraid that there doesn't seem to be any change in this version -- the situation I described in my previous post is still present, even after the game has been run for a further in-game year.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 07:02:19 PM
Hmm, I can't reproduce this. After running through two path refreshes, the debugger confirms that the fall-back code is not accessed by the second run. The timing from Frankenthal branch station to St. Johann branch station (fast line, no intermediate stops, 12km) is 9:00 minutes. The timing from Frankenthal land 1 station to Mönchengladbach branch station (slow line, two intermediate stops, 13.25km) is 22:18 minutes. This seems correct to me.

Are these timings consistent with your results?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 08:08:38 PM
Those timings seem exactly right. However, I can't get the game to display that pattern in the Stop Details window, which is very strange. :-\ I've recompiled the sources and I still can't get this to work.

How long does a path refresh take? I'm guessing it can't be longer than a couple of months.

Edit: I've been compiling a release version rather than a debug version -- could that make any difference?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 09:00:38 PM
Hmm - that is very odd. It shouldn't make a difference if you're using a release build, but can you test with a debug build just to be sure? As to the path explorer , you can see when it refreshes in the "display" menu: look at the very bottom indicator, which shows "Status:" - it usually says "stand-by", but will very quickly go through a series of categories every so often. If you keep that window open and fast forward, you will be able to see when it refreshes by looking carefully at the "status" indicator.

May I ask - what operating system are you using?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 09:26:57 PM
I've compiled a Debug version, and intriguingly the point-to-point journey times are working perfectly in this version! However, the Release version compiled from the same sources appears only to use the fallback system. Very strange!

I'm using Windows 7 64-bit.

An aside: the Debug version itself runs into a crash with 9.x games:

minivec_tpl<T>:resize()
new size 256 too large
(>255).


I can't reproduce this in the Release version, so I daresay it isn't too serious. But I thought it worth mentioning anyway!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 09:36:49 PM
Hmm, this is very odd indeed. Are you using MinGW or MSVC++? Problems present only in the release version indicate undefined behaviour somewhere, but are extraordinarily difficult to track down because it's not possible to use a debugger with the release version (which is a large part of the reason why the debug version exists in the first place).

The minivec problem, however, can easily be solved by replacing the minivec with a full vector.

I shall have to think about how to deal with this when I know whether you are using MinGW or MSVC++.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 09:40:03 PM
I'm using MSVC++ 2010 Express.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 09:42:20 PM
Hmm, same as me. Are you using the project files from the Github repository?

Incidentally, I have pushed a fix for the minivec crash.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 09:44:24 PM
Yes, each time I've been downloading the source files from the 10.x branch on github. When I compile, I usually select "Release" from the Configuration menu, and "Release" in the Configuration Manager window. Is this right? I see that there's also a "Release (SDL)" option, and I don't know what the difference between these two is.

Thanks for the minivec fix -- I'll check that out now.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on August 29, 2011, 09:48:21 PM
Hello

A reflection: the average speed is calculated by dividing the distance between the two stations (note, the distance is calculated as the crow flies) and the time it takes to travel the convoy route. The convoy, however, use a path that will in most cases longer than the crow flies. This difference can cause the problem?
Consider these four stations:

A-B
  I
D-C

The distance between "A" and "B", "B" and "C", "C" and "D" is one tile.
The distance between "A" and "D" is for the convoy of three tiles (A-B-C-D), but the crow flies is one tile.
It may be that in some cases simutrans not using the correct values ​​of the distance?

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 09:55:23 PM
The SDL version is now deprecated, so don't worry about that.

May I ask - what timings are you getting exactly with the release version and your test map? There have been changes to the calculation of distances in 10.x - we now use the "shortest_distance" method written by Knightly, which calculates the shortest possible distance using only the 8 directions available in Simutrans (the previous Experimental method was to use the Euclidian distance, which is the shortest physical distance which may be shorter than the shortest distance achievable in Simutrans, and the Standard method is to use the Manhattan distance, which is the shortest distance using only 4 directions). However, I don't think that this is likely, since the distance method is the same for the GUI and for the calculation of the speed, and unless you're seeing a different distance display in the GUI between debug and release, it won't be that.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 10:03:33 PM
Here's a complete account of the timings I get in the Release version on the test map.

From St Johann branch station (line 2):

Frankenthal branch (12km)
12:30 mins

<slow line kicks in here>

Frankenthal external (19km)
19:30 mins

Bezau branch (28km)
29:00 mins



From Monchengladbach branch (line 1):

Monchengladbach external (2.75km)
4:12 mins

Frankenthal land (5.75km)
8:06 mins

Frankenthal outer station (9.50km)
12:54 mins

Frankenthal land 1 (13.25km)
17:42 mins

<slow line ends here>

Frankenthal land 2 (17km)
22:30 mins

Frankenthal land 3 (21km)
27:42 mins

Frankenthal land 4 (23.5km)
30:54 mins


These vary a small amount from month to month depending on the average speed of the vehicle.


I can confirm that the distances are the same in the debug version.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 10:16:08 PM
And you get the same timings as I do in debug...?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 29, 2011, 10:19:17 PM
Yes, I get almost identical times to yours in debug.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 10:23:03 PM
Very, very odd. I'm not sure that I can quite work out how to track an error that is present in release but not debug builds. Any ideas anyone (Inkelyad, for example)?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 29, 2011, 11:09:16 PM
Hmm - one thing that you could try is saving a game in the debug version after you have given it enough time to accumulate the timing values than loading it in a release build and seeing whether the values are maintained, or whether it eventually reverts to the fallback?

Edit: Incidentally, I have now been able to reproduce this issue. The trouble is, I can't debug the release version. This is a bizarre and difficult problem indeed.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 30, 2011, 08:18:21 AM
Quote from: jamespetts on August 29, 2011, 11:09:16 PM
Hmm - one thing that you could try is saving a game in the debug version after you have given it enough time to accumulate the timing values than loading it in a release build and seeing whether the values are maintained, or whether it eventually reverts to the fallback?

On initial load of the debug game in the release version, the correct timing values are overwritten -- with one exception. The time from Frankenthal branch to St Johann branch is retained at 8:36 mins -- the broadly correct value. This appears to be the only retained value. Even the reverse journey (St Johann to Frankenthal branch) has reverted to the fallback level of 12:30 mins.

After a path refresh, even this value reverts back to the fallback mechanism.

This is indeed bizarre -- what are the principal differences between the debug and release version that could case this kind of problem?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 30, 2011, 09:28:31 AM
The only significant differences are: (1) compiler optimisations; (2) nothing in "#ifdef DEBUG... #endif" blocks are compiled, and (3) asserts are not called. However, (2) and (3) can be ruled out, as there is no relevant code in #ifdef DEBUG blocks, and I have not used any assertions. Compiler optimisations can sometimes cause code with undefined behaviour that works by chance in a non-optimised build to stop working, but I don't think that I've written anything with undefined behaviour, either, and I'm not sure how to go about testing whether I have.

Edit: Your test, incidentally, narrows down the problem to the new code in path_explorer.cc, as that is the only relevant code called at the very start of a game.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 30, 2011, 11:10:24 AM
Update: I have made a very minor change, but not had chance to test it yet. There is a small chance that this might fix the problem - I should be grateful if this could be tested.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 30, 2011, 11:59:52 AM
The central issue is, I'm afraid, not fixed in this version. The Debug version still gives the correct times while the Release version does not.

However, there is one significant change: a Debug-savegame loaded in the Release version *does* display the correct times upon loading. But the incorrect fallback times return after a path refresh.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 30, 2011, 12:21:44 PM
Here's a potential lead: a problem in the debug version.
http://dl.dropbox.com/u/61716/journeytimes3.2.sve

This game is saved in the latest debug version.

At Warendorf External station we're told that both of its destinations will take 6:48 mins to travel to despite the fact that these destinations are a couple of kms apart on a linear line. This anomaly has remained despite several path refreshes. (Note that the other line on this map seems to be functioning normally.)
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on August 30, 2011, 02:39:09 PM
Hello

Quote from: jamespetts on August 29, 2011, 10:23:03 PM
Very, very odd. I'm not sure that I can quite work out how to track an error that is present in release but not debug builds. Any ideas anyone (Inkelyad, for example)?

It can be an uninitialized variable?

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 30, 2011, 06:15:26 PM
Quote from: Milko on August 30, 2011, 02:39:09 PM
It can be an uninitialized variable?
Uninitialized elements in container or unexpected temporary variable, i think.
I suppose I can run Simutrans through Valgrind.

Edit:
Result for debug version:

==27940== Conditional jump or move depends on uninitialised value(s)
==27940==    at 0x8289110: vehikel_besch_t::set_scale(unsigned short) (vehikel_besch.h:647)
==27940==    by 0x82755C8: karte_t::set_scale() (simworld.cc:1822)
==27940==    by 0x8275019: karte_t::karte_t() (simworld.cc:1788)
==27940==    by 0x823B4CE: simu_main(int, char**) (simmain.cc:981)
==27940==    by 0x82C0EF8: main (simsys_s.cc:682)
==27940==
==27940== Conditional jump or move depends on uninitialised value(s)
==27940==    at 0x8289147: vehikel_besch_t::set_scale(unsigned short) (vehikel_besch.h:651)
==27940==    by 0x82755C8: karte_t::set_scale() (simworld.cc:1822)
==27940==    by 0x8275019: karte_t::karte_t() (simworld.cc:1788)
==27940==    by 0x823B4CE: simu_main(int, char**) (simmain.cc:981)
==27940==    by 0x82C0EF8: main (simsys_s.cc:682)
==27940==

max_loading_time_seconds and min_loading_time_seconds are used before initialisation.

Edit2:
One more suspicious place:

simhalt.cc: In member function 'uint16 haltestelle_t::get_average_waiting_time(halthandle_t, uint8) const':
simhalt.cc:1174: warning: taking address of temporary
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 30, 2011, 07:47:43 PM
Inkelyad,

uninitialised variables are unlikely to be the cause of this trouble, since they are usually equally problematic in debug builds as release builds. The first location shows that there are cases (namely, when a non-Experimental pakset is read) where the minimum and maximum loading time values are not initialised; I have now fixed that on the 10.x branch, but this will not affect the issue in question.

The second issue identified, line 1174 of simhalt.cc is not something that can affect this issue, since it is related to the code for waiting times, not journey times, and is in any event not an error:


if(&waiting_times[category].get(halt.get_id()) != NULL)


The address of the temporary is taken only to check whether it is NULL or not and is not further used, so no problems should result.

Carl's leads are helpful, and I shall look into those further. Thank you both for your helpful input on this!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 30, 2011, 08:00:56 PM
Quote from: carlbaker on August 30, 2011, 12:21:44 PM
Here's a potential lead: a problem in the debug version.
http://dl.dropbox.com/u/61716/journeytimes3.2.sve

At Warendorf External station we're told that both of its destinations will take 6:48 mins to travel to despite the fact that these destinations are a couple of kms apart on a linear line. This anomaly has remained despite several path refreshes. (Note that the other line on this map seems to be functioning normally.)
I can't reproduce this. My version (debug and release, compiled on Debian 6.0.2 32 bit) give me 6:30 and 6:48 mins.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 30, 2011, 08:05:47 PM
That's what I get upon loading in the release version. After a path refresh, though, it reverts to 2:30 and 5:36.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 30, 2011, 08:10:13 PM
Quote from: jamespetts on August 30, 2011, 07:47:43 PM
The address of the temporary is taken only to check whether it is NULL or not and is not further used, so no problems should result.
It is temporary variable. it was created while expression was computed. Its address can't be NULL.

I think it should be written as

if (waiting_times[category].is_contained(halt.get_id())
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: inkelyad on August 30, 2011, 08:13:21 PM
Quote from: carlbaker on August 30, 2011, 08:05:47 PM
That's what I get upon loading in the release version. After a path refresh, though, it reverts to 2:30 and 5:36.
In my version there is no such revert. Tricky bug.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: prissi on August 30, 2011, 09:17:24 PM
Uninitialized variables are initialized to a certain value in debug builds (pointer for instance to 0x7dddddddd in MSVC); to find out that those are actually unitialized. In release build they could rather contain random stuff.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 30, 2011, 10:32:40 PM
I have found an issue using Carl's latest test game, and pushed a fix for it; can people re-test? I am not sure whether this solves the original problem (it is possible), but it certainly solves a problem.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 30, 2011, 11:05:45 PM
Thanks James. Your suspicions were correct, as this version fixes that one oddity but doesn't seem to impact upon the main issue.

(By the way, I'm not sure that the "loading time range" fix has had the desired effect yet. At a distance_per_tile of 40, trains seem to always take 1:06 mins to load, regardless of the amount of people joining/leaving the service.)
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 30, 2011, 11:34:14 PM
Carl,

thank you for testing - I shall have to look into the (rather difficult) principal issue further. As to loading times; do the loading times in practice correspond to the loading times displayed in the depot?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 31, 2011, 08:12:29 AM
Quote from: jamespetts on August 30, 2011, 11:34:14 PM
As to loading times; do the loading times in practice correspond to the loading times displayed in the depot?

Yes, they do -- and having looked in the depot I've found the root of this issue, and it's not so much of a concern as I had thought:

-- All non-Experimental vehicles display the 0:20--1:00 range that you're looking for.

-- Vehicles I've built using the Experimental MakeObj -- but which don't have a loading time range specified in the .dat yet -- aren't subject to this default. Rather, most display a range of 01:10--01:10 (or occasionally 0:35-0:35). Note that many of these have *no* loading time specified in their .dat files.

---
Edit:
I've just tested this out with Standard vehicles. While vehicles are loading for the correct times, what's displayed on-screen is not quite right. See the following save: http://dl.dropbox.com/u/61716/loadingtimes.sve

The vehicle here displays a funny mix of loading times. Sometimes it will say "Loading 17:00", but it will leave after one minute of this time has passed. Similarly, sometimes it will say "Loading 3:30", but leave long before it reaches zero. I take it it's the displayed times that are mistaken here. This is a little difficult to explain -- I hope that the save is self-explanatory!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 31, 2011, 09:49:05 AM
Carl,

thank you for that. The difference between vehicles built with Standard and Experimetnal makeobjs is not a bug: it has to work that way to maintain compatibility with older Experimental vehicles built using the original "loading_time" specification. Standard vehicles are not subject to such a constraint.

The second part, however, is a bug, and I shall have to look into that when I get a chance. Thank you for your report.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on August 31, 2011, 09:51:10 AM
Hello

I have problems opening the files. rar, I can have the addon of Carl in zip format?

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on August 31, 2011, 10:11:42 AM
Here it is: http://dl.dropbox.com/u/61716/carladdons4.zip
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 31, 2011, 10:26:38 PM
Carl,

I think that I've fixed the loading time error - can you re-test? The other issue is rather more tricky, and will need furhter investigation.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on August 31, 2011, 10:49:38 PM
Hello

I'm testing http://dl.dropbox.com/u/61716/journeytimes3.2.sve

I have built sim-exp using all update to the code 31/08

I'm looking the first stop of the line 1 (Hoyersweda outer station). In init.png you may look travel time to second (Hoyersweda branch station) stop and to third stop (Hoyersweda external station), times appear ok.

On game loading the train is between third and fourth stop.

When the train reach first stop of line 1 the travel time is not updated and rimain 9:36, it's correct...

When the train reach second stop of the line travel time is updated, but is not correct (look second.png, travel time 17:18, the double)

It appear that when a train reach the first stop of a line the travel time is not applied to the correct point of the line and not resetted.

If you look the second stop when the train reach the first stop the travel time from second to one don't change.

May be the same for the last stop of the line?

Giuseppe






Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 31, 2011, 10:52:00 PM
Some diagnostic progress in relation to the main problem: on loading a game saved in the release build in a debug build, it is apparent that the average journey time records are all empty, which is why the fallback mechanism is used. The average objects are present in the hashtable at the correct locations, but the values are zero, as if they had been incorrectly reset. Loading in debug will slowly restore the averages, but loading, running, and saving again in release will clear them again. I wonder whether this is a problem associated with resetting the averages after being read, especially as the minor change that I made to the code that had some effect was relating to that part of the code. I shall investigate further.

Giuseppe - are you using the debug or release build?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on August 31, 2011, 11:11:52 PM
I have found a possible cause of this problem, and have pushed a fix - can people re-test?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on September 01, 2011, 07:02:58 AM
Hello

Quote from: jamespetts on August 31, 2011, 10:52:00 PM
Giuseppe - are you using the debug or release build?

I use MingW and the procedure I described in one of my previous post ... Sorry but I do not know if this is a debug version or release.  :)

EDIT:
I enclose the report on the compilation using MingW; is reported something about "-SYNC_VECTOR".
I do not understand if something important ....

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 01, 2011, 08:04:35 AM
Giuseppe,

-SYNC-VECTOR is a new preprocessor definition added to Standard, which, when enabled, changes the sync steppable objects collection from an slist to a vector, which, according to the commit comment, can increase speed by up to 5%, and is also network safe. I am not sure why MinGW is reporting that it is not recognised - perhaps because it is not present in those particular files?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on September 01, 2011, 08:32:13 AM
Quote from: jamespetts on August 31, 2011, 11:11:52 PM
I have found a possible cause of this problem, and have pushed a fix - can people re-test?

James,

Initial testing suggests that the bug has indeed been fixed in the Release version!  :)

The lines in the test maps are now displaying journey times which are (a) commensurate with point-to-point times rather than fallback times, and (b) are in line with how long travel takes according to the GUI clock.

Many thanks!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 01, 2011, 08:48:21 AM
Aha - excellent!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on September 01, 2011, 08:51:01 AM
Quote from: jamespetts on September 01, 2011, 08:04:35 AM
-SYNC-VECTOR is a new preprocessor definition added to Standard, which, when enabled, changes the sync steppable objects collection from an slist to a vector, which, according to the commit comment, can increase speed by up to 5%, and is also network safe. I am not sure why MinGW is reporting that it is not recognised - perhaps because it is not present in those particular files?


EDIT2:
I'll try to see if the problem also exists in the standard version. If the problem is shared I will insert a message in the forum of the standard version.
"SYNC_VECTOR" does not produce reports compiling the standard version ...

EDIT:
Enclosed you will find sim-exp with the latest bug fixes by James
http://simutrans-germany.com/files/upload/simutrans-experimental_0109.zip

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: wlindley on September 01, 2011, 10:57:39 AM
I have done a 'git pull' and am on commit a52bf63b71f56c7143017deaa0ff07bc6429aa21 ... 64-bit linux.  Using Pak128.Britain-Ex-0.8 ... everything worked up until yesterday's build and now I get:

$ ./simutrans-experimental
Use work dir /home/bill/simutrans/sim-exp/
Reading low level config data ...
FATAL ERROR: loadsave_t::rdwr_str()
expected "<i16>", got "</ei>"

FATAL ERROR: loadsave_t::rdwr_str()
expected "<i16>", got "</ei>"
Aborted (core dumped)


Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 01, 2011, 11:06:38 AM
WLindley,

you will need to delete settings-experimental.xml (or settings-experimental-debug.xml if you are using a debug build).
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on September 01, 2011, 07:05:15 PM
I've just observed another vector crash which I assume only affects larger maps:


vector_tpl<T>::[]
class koord3d: index out
of bounds: 13 not in 0..-1


Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on September 01, 2011, 08:20:59 PM
Hello

I have another problem loading Pak96comic 0.4.9 using Exp10.0.

Using 9.12 there are no problem.

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 01, 2011, 08:27:46 PM
Carl,

what were the conditions in which that occurred? Can you upload a saved game?

Giuseppe,

the problem appears to be with the saved game demo.sve - can you delete demo.sve in the pakset folder and try it again?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on September 01, 2011, 08:33:19 PM
Hello

Deleting demo.sve all is ok.

But I have a similar problem using Pak German Exp - 20382.

demo.sve all seem to create loading problems with version 10.0

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on September 01, 2011, 08:35:28 PM
Quote from: jamespetts on September 01, 2011, 08:27:46 PM
what were the conditions in which that occurred? Can you upload a saved game?

No special conditions -- the crash simply occurred while I was running the following (9.x) game in the background, checking the travelling times fix. It took perhaps a quarter or half a month from loading for the crash to occur.

Savegame: http://dl.dropbox.com/u/61716/UKJuly.sve -- it's a big one, i'm afraid
I think this is the right addons folder: http://dl.dropbox.com/u/61716/carladdons2.rar

Many thanks!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 02, 2011, 12:27:25 AM
Hmm, I can't reproduce this with the debug build, I'm afraid. Can you re-check with the SYNC_VECTOR option disabled?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on September 02, 2011, 08:56:44 AM
Hmm -- I can't reproduce the crash, either. Very strange! I'll let you know if this comes up again.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Dwachs on September 02, 2011, 09:24:31 AM
Quote from: jamespetts on September 01, 2011, 08:04:35 AM
-SYNC-VECTOR is a new preprocessor definition added to Standard ...
you should have written
Quote
-DSYNC-VECTOR
in the config.* file
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on September 03, 2011, 11:20:22 AM
Oh dear -- it seems I may have jumped the gun a little in thinking that the journey-times bug was fixed.

As far as I can work out, the situation is as follows: the latest release version is can read point-to-point average speeds from the hashtable (which is why I thought the problem was fixed on the previous test maps), but it cannot write them.

Here's a new map, created in the latest version, in which point-to-point journey times haven't kicked in. I guess you'll be able to verify in debug whether the hashtable is empty or not: http://dl.dropbox.com/u/61716/journeytimes3.5.sve

Further evidence comes from the previous test map, journeytimes3.2. Although the point-to-point journey times are correctly displayed upon loading, they never change from month to month. Normally I'd expect to see random variation of 6/12 seconds here and there, but there is none. This suggests that the release version is only reading, and not writing, point-to-point average speeds from the hashtable.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 08, 2011, 11:54:52 PM
Carl,

thank you very much for this report, and apologies most sincere for not having reverted to you: I have been especially busy at work this week and have had very little time for Simutrans, unfortunately.

I have managed to do some testing this evening, however, and have found that the problem manifests in the id_pair representing the pair of two halthandle ids used as the hashtable key (the x being the destination and the y the origin): on loading the debug version, all of the y values in the pairs are zero. There are no id_pairs with y-values of anything other than zero. This is corrected when the debug version is run, and a full complement of id_pair values are generated.

Testing further, and using testing code to trigger the built-in error messages in place of the debugger in the release build, it appears that no pairs are put in to the hashtables with a y-value of zero. However, clearly, they are coming out with a value of zero. This is odd. I have not had time to test further yet, but I should be grateful if anyone has any ideas on this issue in the interim.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: sdog on September 09, 2011, 04:38:09 PM
Just a thought on load times: At a trains route during normal stops only a part of the pax unloads, at terminal stops, all unload. Perhaps it would be sensible to double (or more) load times on terminal stops to reflect this?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on September 09, 2011, 09:28:59 PM
Hello

I was trying the pak96c, I noticed that cities are created (while leaving the default settings and no timeline) are all large and with few inhabitants. All buildings generated are small, have not been generated, high-density buildings.
If I use the standard version instead of the buildings are of varying density and smaller cities are created equal in population.
pak96c has not been developed for experimental but it seems to me that in any case "experimental" has a rather strange behavior as he does not consider the medium and high density buildings.

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 09, 2011, 09:55:15 PM
Sdog,

this should be accounted for by the fact that the new system measures the actual number of passengers (etc.) loading/unloading.

Giuseppe,

when using a non-Experimental pakset, you should increase the "rennovation_percentage" in cityrules.tab to prevent this.

Edit: The problem reported by Carl is especially tricky and bizarre. See the latest testing code in simconvoi.cc in the 10.x branch - the second of the fatal error calls is triggered in release but not debug builds. The id_pair values are inserted into the hash table, and, on insertion, the numbers are correct. In debug builds, they are retrieved from the hashtable correctly. However, in release builds, all of the y (but not the x) values are immediately read as zero whenever they are removed from the hashtable. This is rather beyond me, I am afraid: does anyone have any idea(s) as to what this might be?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: sdog on September 10, 2011, 12:26:54 AM
oh, didn't notice that new feature, should have read the thread more thoroughly, instead of only browsing it.

is the load time specified in the dat file the maximum load time if all pax would alight and a full load get picked up?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: infernalmachine on September 10, 2011, 09:51:45 AM
Hi.  I spent a while looking at the code, and I noticed a possible reason for the problem.

But first I should say I've never seen simutrans code before, I haven't coded in 15 years, and I never quite "got" templates in c++.  So if this is a silly suggestion, I'll understand  :-[  :)

My first guess was that y is being bit-shifted into zeroness, either accidentally when you retrieve it, or possibly when you create the hash number.

In the tpl directory your template for koordhashtable_tpl has the hash function:

static uint32 hash(const key_t key)
    {
uint32 hash = key.y << 16;
hash |= key.x;
return hash;
    }


Now, assuming that key.x and key.y are both uint16, it struck me that key.y might need to be typecast as uint32 before being bitshifted, or else risk becoming (uint16) 0 before you store it in hash and then or in key.x.  If that happened then hash is just equal to (uint32) key.x

So you'd only be dealing with the first 64K possible entries in the hash table for both lookup and updating.  And you wouldn't get junk back on lookup, because you have to hash your present and past location ids before you can access what's stored in the table. The expected matrix would effectively be just a vector.

Don't know how a debugger could hide that, though.... Unless it casts everything under 32 bits as 32-bit?

Anyway, as I say, I'm very rusty on when typecasting is needed, so this may be a silly suggestion and that code may be working as intended.

ps Does every line have its own hashtable? And the expected max size is the square of the number of stops on the line?

Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: isidoro on September 11, 2011, 12:17:24 AM
I don't think there is a problem with that.  Automatic integer promotion should take place.  Please see, for instance:
https://www.securecoding.cert.org/confluence/display/seccode/INT02-C.+Understand+integer+conversion+rules (https://www.securecoding.cert.org/confluence/display/seccode/INT02-C.+Understand+integer+conversion+rules)
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: infernalmachine on September 11, 2011, 03:26:47 AM
Okay.  Thanks for that.  You convinced me.  So key.y is turned into sint32 before the bit shift. 

I guess I should try to find out where key.x and key.y values are retrieved from the the whole key.  The link you gave does warn about potential problems when going from unsigned to signed ints.

Beyond that, I'm stumped.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 11, 2011, 11:09:02 AM
Infernal Machine,

thank you very much for your suggestion and looking into the code in this way - that is very kind. Thank you also Isidoro for your feedback. I am wondering whether the problem is that I am putting objects of the class "id_pair" into a koordhashtable, which is designed for objects of the class "koord". "Koord" objects have two sint16 members, whereas id_pair objects have two uint16 members. I notice that there is discussion above about difficulties when converting between signed and unsigned integer types in this context. Might that be an issue, do people think? If so - how would a hashtable class for pairs of unsigned integers work differently to one for signed integers?

Edit: This does not appear to be the problem - when I substitute koord objects for the id_pair objects, the exact same problem still occurs. This is rather bizarre.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 11, 2011, 01:58:35 PM
After much testing, I think that I might have fixed this latest issue with the average journey times (although the cause is still somewhat opaque to me) - can people re-test?
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on September 11, 2011, 02:28:37 PM
It seems that the bug is fixed!

Since I jumped the gun in saying it was fixed last time, this time I started an entirely new test map using the latest version. After leaving it to run for a few months the stops display the correct journey times -- times that could not have been given by the fallback system.

That said, I can't get the correct times to display on the 'journeytimes3.5' map from my previous post. However, that might just be because it was saved in the previous (defective) version. The fact that this bug doesn't occur in newly-created maps suggests that it's fixed.

Many thanks! :)
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 11, 2011, 03:34:14 PM
Thank you very much for testing - much appreciated!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on September 12, 2011, 04:44:50 PM
Hello James

The problems of "SYNC_VECTOR" decreased but still I believe we need to revise some things, I am attaching the report of MingW.

EDIT
When compiling: almost all files. "cc" to report the same message about "SYNC_VECTOR", not just those that show the image that I attached it.

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 17, 2011, 05:52:41 PM
Giuseppe,

thank you for reporting this: the problem was that I had not realised that SYNC_VECTOR was already defined in the code, so the definition in the makefile produced the duplication warning. I have now removed these redefinitions, and it should now compile without these warnings.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on September 17, 2011, 06:37:15 PM
Hello

Now I have this warning, in many ".cc" files. Were not there before.

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 17, 2011, 06:41:08 PM
Don't worry about that warning - the code from Standard is full of signed/unsigned integer comparison warnings.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on September 17, 2011, 06:51:07 PM
Hello

Last "nightly"...

http://simutrans-germany.com/files/upload/simutrans-experimental_1709.zip

Giuseppe
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 18, 2011, 11:28:17 AM
10.0 has now been released - I should like to thank very much all those who have helped with testing it.
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Carl on September 18, 2011, 12:05:59 PM
Many thanks for your hard work too, James -- 10.0 is a truly excellent release!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: jamespetts on September 18, 2011, 12:32:53 PM
Glad that you like it!
Title: Re: Simutrans-Experimental 10.0 - pre-release testing
Post by: Milko on September 19, 2011, 01:02:28 PM
Hello

Quote from: jamespetts on September 17, 2011, 06:41:08 PM
Don't worry about that warning - the code from Standard is full of signed/unsigned integer comparison warnings.

You can eliminate this warning?  :)

I agree that the warnings signs are weak and potentially harmful.
The problem is that if it causes so many warning signals may create confusion.
Among the many warning it is difficult to understand if some of them are important or not.
I had noticed the problem due to the fact "SYNC_VECTOR" just based on the number of warnings at compilation.

Giuseppe