[devel-new] Expanding the map crashes if stops exist at certain locations

[zhaop]

I'm happy to file my first bug report here ☺

These steps should guarantee a crash on any map:
1: Load any map.
2: Create a stop at the bottom-right (max x, max y) corner (if there isn't already one).
3: Expand the map by 1 in either horizontally or vertically.

These steps also cause a crash in Standard 120.1.3. I would have liked to attach the save where I first noticed this problem, but 444 KB was apparently too much to post.

More generally, I guess this happens any time a stop exists...
... either within the station coverage square of the bottom-right edge of the map, and the map is vertically expanded (y-direction) by some small amount
... or within the station coverage distance of the bottom (max y) edge of the map, and the map is horizontally expanded (x-direction) by some small amount

I believe the reason is faulty bounds checking in simworld.cc:2364-2368,2381-2385, which updates haltlists in tiles in the expanded area. I will post a link to a pull request as soon as a fix is ready.

[jamespetts]

Thank you very much for the report: this is helpful. I confirm that I can reproduce it on the latest devel-new branch of Experimental, and the crash appears to emanate from Standard code. Given that this can be reproduced in Standard, I have moved it to the Standard bug reports forum, and hopefully any fix can be applied both to Standard and Experimental.

[zhaop]

Thank you James. One thing to note about reproducing this in Standard 120.1.3: the crash can only be reliably reproduced on small maps (e.g. 64x64 -> 65x64), I'm guessing due to the smaller station coverage relative to Experimental.

Also, should one attach a patch or make a github pull request to propose a fix for Standard? My proposed fix is attached in any case.

[Dwachs]

Thanks for the patch. Incorporated in r7860

[jamespetts]

Thank you very much for that fix. For Standard, the developers like patch files, whereas for Experimental, Github is the best way to go. Is this already on Github, may I ask?

[prissi]

Actually, patch has nothing to do with svn at all. TThatprogram is even used internally by git.

[DrSuperGood]

Patch files apply changes to files and so technically have nothing to do with SVN. For GIT one would first apply the patch to the working file set and then commit the file changes the patch makes as a normal GIT commit. Using GitHub to merge the changes is only recommended because it applies correct crediting of the commit origin (the patch maker's account, as opposed to your account).

[zhaop]

Thank you all for your explanations. My question was to understand policy differences between Standard and Experimental for contributing code (for "non-core developers"). I understand that pull requests on Github are the usual practice to submit contributions for Experimental, whereas since the official repo for Standard is on svn, I don't think an analogous "pull request" process exists, so I guess submitting patch files is the standard way (like James explains).

@jamespetts I have also separately committed this fix on my fork of experimental. The pull request is on Github, so unless there are special processes for porting Standard fixes & changes to Experimental, this should be ready to merge!

[jamespetts]

Excellent, thank you very much: now merged. That is very helpful.