Small project to prevent cities from spawning right next to the map edge

[neroden]

This is something which has irritated me whenever I try to generate a new map, so I finally developed a patch for it.
New parameter in cityrules.tab.

edge_avoidance = 0 will reproduce the existing behavior

edge_avoidance = 16 will not generate cities within 16 tiles of the edge of the map, which is what I actually want to happen when making large pak128.britain maps

The default in my current implementation is to not generate cities within 8 tiles of the edge of the map, which I think is a small enough buffer to be sensible even for small maps.

The edge_avoidance parameter is saved in the savefile to deal with some tricky corner cases related to using the "expand map" button.  This requires a bump in the experimental minor revision version number (to 21).  If it is decided that this doesn't need to be saved in the savefile, I can take that stuff out and eliminate the version number bump.

Pull requests are at:
(the code)
https://github.com/jamespetts/simutrans-extended/pull/605

(the pak change for pak128.britain)
https://github.com/jamespetts/simutrans-pak128.britain/pull/146

Tested and appears to be working fine.

[jamespetts]

Excellent, thank you for this. This seems like a worthwhile addition. I doubt that this will be an issue as I do not think that the relevant parts of the code are engaged, but can I check whether you have tested whether this creates any merge conflicts with the ex-15 branch?

[neroden]

I have not checked on merging with the ex-15 branch yet.  In the meantime, I have another set of fixes, on a subsidiary branch, here:

https://github.com/jamespetts/simutrans-extended/pull/606/commits/8a40a4c933690684b25ae13819bd11f9449b9a14

Basically, the code to figure out where to place factories was totally hosed -- it was ONLY placing factories diagonally kitty-corner to roads, rivers, and seas, and it could never match for river_city or shore_city because it tried to find the river or shore on the same tile as the road -- it also would never build forest factories at all on a map with no trees.

I straightened it out so it's always placing the factories rectinlinearly to the roads, rivers, and seas, fixed the other bugs, and it's much more comprehensible now, too.  And I implemented edge_avoidance for factory placement too (for the same reason as for city placement).

I will check both of these simultaneously against the ex-15 branch when I get a chance.

-----

AND I made a final update with an additional set of changes, so that the "public-service-built" road connections from the factories look better and are more varied.  This includes all the previous patches.

https://github.com/jamespetts/simutrans-extended/pull/607

I have not checked it for merge safety against ex-15 yet.  Attempting that merge will be next on my list, but I've got some non-programming stuff I have to do now.

Please pull THIS pull request for master.  I have closed the previous pull requests because this includes all of them for master.

----
AND I have successfully merged it into ex-15, along with a collection of other little patches which were on master but not on ex-15.

The pull request for ex-15 is here:
https://github.com/jamespetts/simutrans-extended/pull/608

Please pull this one for ex-15, I solved the merge conflicts (which were all technical conflicts, no real conflicts) correctly in this one

----
Then when these pull requests are both pulled for the code, please pull this for pak128-britain
https://github.com/jamespetts/simutrans-pak128.britain/pull/146

[neroden]

Just one note, some of the stuff I fixed here is actually broken in Standard and has been broken in Standard for years. 

I only ever play Extended, and the divergences are so severe that I'm not willing to do the work to adapt the patch to Standard.  But someone who plays Standard might want to look into the entire series of patches from d16d1d7 to bac862d -- is_area_ok has been broken in Standard for years, it looks like.

[Matthew]

Nathaniel, thank you so much for contributing these patches.

Basically, the code to figure out where to place factories was totally hosed -- it was ONLY placing factories diagonally kitty-corner to roads, rivers, and seas, and it could never match for river_city or shore_city because it tried to find the river or shore on the same tile as the road

This puzzles me as I ran some tests which consistently placed city shore factories in the right places and with roads as well. But never mind, what's important is whether your code works going forward. Maybe I can test that on this branch this weekend but no promises.

I would also like to sneakily request a new feature though while this code is fresh in your mind. Previously, factories always faced the road (that is, the 0 orientation had the road to the south). Would it be possible to add the option that factories which must be built on the shore or by a river can instead have the orientation set towards the water/river? This would be useful for dock warehouses, fishing ports, etc.

[neroden]

If you read through the old code and understand it -- it was almost impossible for river_city or shore_city to find an implementation in is_area_ok (only if it was adjacent to a road/river crossing).  As a result there was special-case code to call the subroutine with City instead for river_city or shore_city pak objects (I have now removed this).

If you thought you were seeing factories orthogonal to roads it was an accident -- they were matching roads diagonally ONLY, but maybe there was an orthogonal road there coincidentally (since the roads are typically running in a grid).  This would show up repeatedly when generating crowded cities where the factories were squeezed into weird internal holes in the city grid where their only road access is diagonal -- that was my initial motivation for figuring this out, I wanted the factories to stop being in stupid locations.

There's a sort of backup code which activates if the game can't find an area which is OK, which goes "aw, forget it" and starts planting factories anywhere with disregard for the restrictions, which might have accounted for some of the apparent "successes" you were seing.  This was triggering far too often -- this was most frequently visible with the creation of fishing ports which aren't in cities or aren't on the seaside.  This should trigger much less.

The fishing ports are particularly difficult to get to behave properly.  They were my motivation to allow shore_city industries to be built on the shore in a city not next to a pre-existing road, with the road added after-the-fact, as this makes it more likely to be able to actually find a city location (and a shore location is *essential* since there are no live-fish trucks in early game, only ships).  A fishing port which is too far from a city becomes unusable if staff_shortage is turned on (as a manufacturing industry, the "rural" option doesn't apply to it and it stops operating without staff).

The fact that rural industries (which build a new road) always attached by the top left diagonal if possible was kind of subtle, but I started noticing it after generating a lot of maps, and it was there in the code.

I actually didn't even look at factory orientation code.  The code I altered is the code which tests whether the location was a reasonable location for a factory.  Deciding which way the factory faced, I left completely alone -- it's actually a totally different block of code and I haven't even read it.  I can point you to it, but I did not touch it.

[neroden]

And a followup patch to give the fishing port in pak128.britain "shore_city" placement, which should work after my patches to the code.

https://github.com/jamespetts/simutrans-pak128.britain/pull/148

That'll probably be it for me for a little while, I've got a bunch of other things I have to go do.

[jamespetts]

Excellent, thank you very much for this, and for merging with ex-15. One query about the latter, however (and apologies if this is a silly question from somebody whose knowledge of the complexities of GIT are limited): if I merge the master branch patch into the master branch and then the ex-15 patch into ex-15, how would I then merge further changes into ex-15 from master without it giving rise to the very merge conflicts that you have already fixed?

Thank you again for your work on this.

[neroden]

So, based on my knowledge of git.
ex-15 branched off from an old revision of master (call this the "origin").
master has some other changes, not mine from the origin (call this "current")
My more-varied-factory-connections branch is branched off of "current"
My merge-prettification-to-ex-15 branch was first branched off of ex-15, and then:
-- has "current" master merged into it (sucessfully, this is where the merge conflicts were)
-- has more-varied-factory-connections merged into it (successfully, in two goes since I did an update)
This was done *in this order*.

If you merge more-varied-factory-connections into master, the new master is now descended from it.
If you merge merge-prettification-to-ex-15 into ex-15, the new ex-15 is now descended from it, and therefore descended from the commit which resolved the merge conflicts when merging from "current" master.

So it's merged all those commits and won't ask you about them again if you merge from master again.

No promises, since git occasionally has bugs, but it should Just Work.

[jamespetts]

So, based on my knowledge of git.
ex-15 branched off from an old revision of master (call this the "origin").
master has some other changes, not mine from the origin (call this "current")
My more-varied-factory-connections branch is branched off of "current"
My merge-prettification-to-ex-15 branch was first branched off of ex-15, and then:
-- has "current" master merged into it (sucessfully, this is where the merge conflicts were)
-- has more-varied-factory-connections merged into it (successfully, in two goes since I did an update)
This was done *in this order*.

If you merge more-varied-factory-connections into master, the new master is now descended from it.
If you merge merge-prettification-to-ex-15 into ex-15, the new ex-15 is now descended from it, and therefore descended from the commit which resolved the merge conflicts when merging from "current" master.

So it's merged all those commits and won't ask you about them again if you merge from master again.

No promises, since git occasionally has bugs, but it should Just Work.

Thank you very much for this, and apologies for the delay. Now tested and incorporated on both master and ex-15.