[RC 11.9] Loading fails too many intercity roads - also probably affects Standard

[pwhk]

Hm... I don't have any good 11.9003 savegame to load...

After loading 10.11+pak0.8.3 autosave in 11.9003 and re-save it as 11.9003+pak0.8.4 save file, loading that file fails.

The loaded save file is http://simutrans-germany.com/files/upload/autosave01.sve
And re-saved file is http://simutrans-germany.com/files/upload/1.sve

[jamespetts]

Thank you for checking that. Can you check whether you have the same issue with the latest version of Standard? Thank you!

[pwhk]

Simutrans-Standard 112.2 r6390M + pak64 112.2 r1185 does not exhibit this issue.

[jamespetts]

May I ask - do you use the single user install?

Edit: Also - would you mind sending me/uploading an uncompressed binary version of your corrupted save file based on the game loaded from an earlier version and re-saved without being changed so that I can look for the differences between the two with a hex editor?

Apologies, incidentally, for asking so much: I am afraid that it is very difficult for me to test this properly without being able to reproduce the problem myself, so I have to rely on those who can. You have been most helpful.

[prissi]

This rather indicates something is saved in another size than loded, if XML works.

And as mentioned before: The error is usually not in the city roads but in stuff saved before it. Could be even three objects before, with "luck".

[jamespetts]

Yes, indeed, you are correct: my tests show that all of the data in loading/saving the settings are shifted up by one. The problem originates before even loading/saving the settings, a the pakset string is corrupted. Instead of "Pak128.Britain-Ex-0.8.3" or the like, the pakset string is: "E:\Saved Games\Simutrans-Experimental\Pak128.Britain-Ex-0." (or something similar, depending on which of the various saved games that have been uploaded in this thread is used). Because I cannot reproduce the problem in saving the game itself, I have yet to work out why/how this is occurring.

[pwhk]

http://simutrans-germany.com/files/upload/save.zip
Contains both uncompressed 10.11 save and corrupted RC 11.9003 save, zipped with 7-zip (so when extracted with 7-zip they should give two uncompressed files)

My saves are stored in user directory.

btw, this is what I get when I open up the load dialog.

[Markohs]

mmm.... this is related to simutrans standard commit 6024 I think and if I'm not wrong the problem must be in how you merged loadsave_frame_t::get_info in experimental code.

But the detail of the bug might be somewere on that patch, because I experienced that bug when I developed the patch and fixed it before commiting. Something in experimental triggers the bug again, afaik.

Deleting save/_cached.xml *MIGHT* solve the bug too.

[jamespetts]

The oddest thing about this bug is that it appears to occur on some people's systems and not on others. May I ask - how long ago was this patch put into the code? I cannot find anything before March 2010 mentioning "loadsave_frame_t::get_info".

Edit: Also, I have yet to work out how to find a commit by SVN revision number in the Standard code - but I can find it on the Github mirrors by date.

Edit 2: I wonder whether this might be related to this issue...?

[jamespetts]

I have not found a definitive solution to this problem (which is very hard when I cannot reproduce it) but I think that I have managed to produce a workaround on my Github branch. Unfortunately, this will not be effective without a new pakset version release, as it uses the pakset version string (which is not defined in 0.8.4 owing to an oversight) instead of the pakset folder name where it detects corruption of the pakset string (where the second character is a colon: this fix will only work in Windows, therefore, but the problem only appears to have occurred in Windows so far in any event).

I have also incidentally fixed a number of coding errors, tracked down with Dr. Memory, that did not produce known bugs but that were theoretically capable of causing unpredictable behaviour, which it is just possible might fix the issue, although there is no easy way of testing it.

Edit: In the meantime, I recommend using the XML save as a workaround.

[jamespetts]

Can all those who have had the problem re-test with RC 11.9004 to check whether this recurs?

[killwater]

I'm sorry James mine is still crashing. Tried 3 times. Tried with 112.3 and 112.2 savegame version. 
zipped does not work
xml_zipped works
xml_bzip2 works but painfully slow

[jamespetts]

This is extremely odd. Can you upload your saved games from this latest version?

[pwhk]

RC 11.9004 Doesn't work for me too... :(

The following save does not touch any new game config, and used the old config taken from previous testing (i.e. the config files are not touched in the update)
http://simutrans-germany.com/files/upload/1.sve

[killwater]

Hello would you like zipped one?
Here it is:
http://simutrans-germany.com/files/upload/3.sve

[djgoldsmith]

Hi,

I am also having this problem on a fresh build of RC-11.9004.   Looks like I am the first with the problem on linux.

Any save from the new version give the "Too many (x) city roads" message.   However, Saves from the previous release (111.2.1 experimental 10.27) do load.

One thing I have noticed is the strings in the load game dialog box are munged:
Saves from previous release (that work) are <Name> <Date> <Pak> <Simutrans Version> <Ex-Version>
Saves from 11.9004 Are <Name> <Date> </home/dang/build>

So it looks like the Pakname is being replaced with the build / run directory

You can see this also in pwhks post

btw, this is what I get when I open up the load dialog.

Will be having a root around to see if I can track down the problem.  Will keep you updated.

EDIT:

XML Saves work fine for me.

[jamespetts]

Yes - that I have discovered: the trouble is that, because I cannot reproduce the issue, I have no idea what actually happens in the code to cause this at save time. Any investigation that you are able to undertake that might provide some illumination would be most worthwhile.

[Vonjo]

How about using another paksets? Does this problem occur with all paksets?
I didn't have this problem with pak128-standard. I haven't tried pak.brit-ex.

[djgoldsmith]

I have worked out the problem , now just have to write a fix  .

It appears that the pakset string being too long causes the issue.  Currently the pakset string in the save file is the full working directory of the pak, as the array to hold it is limited to 64 characters then if it goes longer than this you get an overrun.
If you move the working directory of simutrans, then the problem goes away (or you can replicate it)

For example if I moved simutrans (and thus the pakset) from:

Code Select Expand

$/home/dang/build/simutrans-experimental/simutrans/Pak128.Britain-Ex-0.8.3  (73 Chars)
to  $/home/dang/simutrans/Pak128.Britain-Ex-0.8.3/   (45 Chars)

I could save in binary.  James to replicate the bug you could try running from a few directories down the tree.

You can also edit the save file and remove a few characters from the first string which also made it work for me.


I am working on a quick fix now,  was intending to string the pakset string from its current form (the full path) to just the working directory.  However, James I see in other posts you were asking about this string, and where it comes from (copyright?) If this is solved in the pakset then the problem should resolve itself.

It shouldn't take too long, but its my first (I hope on many) trip into the simutrans source, so its taking a bit of time to get my head around things.  And I haven't coded in standard c++ for about 5 years (been wasting my time with Python and NesC) so its taking a bit of time to adjust back.

[jamespetts]

Hmm - this is rather curious. Thank you very much for your work on this. Have you managed to track down where in the code the full path appears? I do not seem to get the full path at all when I look at the relevant part of loadsave.cc with a debugger - but which lines are you looking at exactly? The problem seemed to me to be precisely that the full path was being used somewhere where it ought not to be used, but I could never reproduce the problem and find out where the full path ended up being used incorrectly. If you are able to shed any light on that, that would be most helpful.

[djgoldsmith]

Hi, sorry family called last night, so didn't make as much progress as I hoped.

Sticking some debug statements in it looks like the relevant code is in loadsave.cc lines
325 - 372
It appears to be the pakset_string variable written to file in line 358 that is the problem.

that's under the rc11-1004 tag

simutrans source is still only a day old as far as I am concerned, so it's all a bit of a mystery where function calls go, slowly starting to see bits of the big picture   I am interested in contributing, if there are any resources that could help me get started drop me a pm.  A quick heads up on what's what in the git repository would be useful, I am used to svn / bzr so haven't quite got that either.

[jamespetts]

I have just pushed one change to the Github repository which might conceivably make a difference to this issue: would you be so kind as to try re-compiling with the latest code and check whether the problem recurs?

More generally as to the code - what sort of thing did you want information about particularly? As you may know, Experimental is a fork of Standard, so I am rather unfamiliar myself with many parts of the code that have not changed significantly or at all from Standard to Experimental (the code for graphics, for example, or sound). The Github repository is what we use for Experimental - Standard primarily uses SVN, but there is an official Github mirror, which is used for merging the latest changes from Standard into Experimental. There are a number of branches. "Master" is always the latest released version (here, 10.27). There are a number of older branches for previous projects since merged, and a 10.x branch that will soon be superseded by a 11.x branch that deals with bug fixes based on the latest main version. The current branch on which development is occurring is the 112.x-private-car-merge branch, which is a merger of the 112.x-merge branch, itself a merger of Experimental and the changes in Standard 112.x, and the private-car-routing branch of Experimental, which introduced a number of new features.

In recent times, Bernd Gabriel has very helpfully done a lot of work on merging, and has also worked a great deal on the physics, which are different in Experimental to Standard (we have braking physics, for example). Information in this forum should give you some useful pointers about the Standard code, on which, of course, the Experimental code is closely based.

I hope that that is a helpful start - do let me know if you want more specific sorts of information and I shall try to assist.

[djgoldsmith]

I am afraid that your change didn't work for me. I still get a too many roads error

Ref the Code,  thanks for pointing me in the direction of the forum, looks like an excellent place to start.
Started to understand how git works, and have an idea of what branch to develop on now.
Just need to delve a bit deeper into the simutrans source to try to get an idea of how it all hangs together.

Regards.

[jamespetts]

You say that you still get a too many roads error: may I check - are you trying to load a game that you saved with the previous code, or are you trying to save/load a new game? As the error occurs on saving, not loading, only the latter will be a proper test.

[jamespetts]

I think that I have eventually found and fixed this issue, which was caused by too short a buffer in loadsave.cc (see this commit). In principle, this problem ought to affect Standard, too, so I am moving this to the Standard board, as the fix that I have applied in Experimental ought to work for Standard.

[prissi]

This cannot affect standard, as sizeof(SAVEGAME_PREFIX) is smaller than 77. If you want to play it save the buffer should have rather the size of char buf[sizeof(SAVEGAME_PREFIX)+sizeof(XML_SAVEGAME_PREFIX)+ 2 ]

[killwater]

Saving is working for me with RC11.9006

[jamespetts]

Excellent! Thank you for testing.