The International Simutrans Forum

Simutrans Extended => Simutrans-Extended development => Topic started by: neroden on June 18, 2013, 08:25:56 PM

Title: OK, solved a bug with dying trees in dingliste.cc -- but there are more
Post by: neroden on June 18, 2013, 08:25:56 PM
Commits:
eca594049807861
and
ca27d50c054ba (fixing a compilation error)
on the "dinglist-cleanup-light" branch

Also found another bug in dingliste_t, a fencepost error, fixed in commit
8f256fcf265565f

Both of these fixes should probably be merged into standard as well as into experimental; the bugs are in standard.

These *seemed* like they could have been the bug causing the server crashes.  But they weren't, because I managed to reproduce it again.

Something else is causing null pointers to get into the dingliste_t lists.  I've set up an assertion for null pointers being passed to "add()", and that isn't happening.  This means there's something else going wrong, probably in the internal memory management code.

On the ncn-devel branch, in hopes of squashing these bugs, I cherry-picked a bunch of bug fixes from standard, avoiding new features.  Experimental had diverged from standard in the simgraph*.cc code, which was clearly unintentional; I'm hoping rhat restoring the standard version there will do the trick.

I'm going to try again to track down the crash bug.  This one is hard.
Title: Re: OK, solved a bug with dying trees in dingliste.cc -- but there are more
Post by: jamespetts on June 18, 2013, 11:29:06 PM
Thank you very much indeed for this work - it is appreciated. I have duly merged your new code and briefly tested it. One small graphical anomaly arises:  the scrolling text at the bottom of the screen has patches where it is not updated unless the map is scrolled or a window is moved around behind that area. They appear to be in consistent places on different occasions of starting the game.

Incidentally, I have also taken the opportunity to remove the "COMMAND_LINE_SERVER" preprocessor definition fully and use COLOUR_DEPTH=0 instead.

Your work in tracking down this elusive and difficult yet rather serious bug is very much appreciated.

Edit: I don't see you having a dinglist-cleanup-light branch - have you pushed this? I only see a dinglist-cleanup branch.
Title: Re: OK, solved a bug with dying trees in dingliste.cc -- but there are more
Post by: neroden on June 19, 2013, 01:41:50 AM
Quote from: jamespetts on June 18, 2013, 11:29:06 PM
Thank you very much indeed for this work - it is appreciated. I have duly merged your new code and briefly tested it. One small graphical anomaly arises:  the scrolling text at the bottom of the screen has patches where it is not updated unless the map is scrolled or a window is moved around behind that area. They appear to be in consistent places on different occasions of starting the game.

That popped up earlier; it relates to "dirty bit" handling.  I fixed it with some cherrypicking of patches from standard (well, actually, a lot of cherrypicking of patches), which is now on my ncn-devel branch as well as on my dinglist-cleanup branch.

QuoteIncidentally, I have also taken the opportunity to remove the "COMMAND_LINE_SERVER" preprocessor definition fully and use COLOUR_DEPTH=0 instead.

Yay!  Thank you.

Quote
Your work in tracking down this elusive and difficult yet rather serious bug is very much appreciated.
My current belief (...and we'll find out if I'm wrong, if we get another crash of the same type) is that there were a large number of subtle errors in dingliste_t related to confusion between the meanings of "capacity" and "top", and failure to update both of them accurately. 

In order to fix this I defined a specific meaning for each variable -- which is now written in the header file -- and then went through the entire .cc file changing the game logic to match the meanings which I had defined, and changing different routines to have parallel structure so that it's easier to tell that they follow the meanings accurately.  These are the "heavy" fixes which I mention below.

QuoteEdit: I don't see you having a dinglist-cleanup-light branch - have you pushed this? I only see a dinglist-cleanup branch.

Whoops.  I thought I had pushed that.  Many apologies.

The dinglist-cleanup-light branch had the "small, safe" fixes. 
The dinglist-cleanup branch also has the *heavy* fixes, which at the time I last posted, were not really working quite right.  I believe they are working right now.

You may want to test the dinglist-cleanup branch now, as that is my newest version.  If you've already merged an earlier version of it, I suggest you go ahead and merge the revised and better-functioning version.