City growth changes in the GIT passenger-generation branch

[Philip]

I've been playing with the passenger-generation branch, and I've noticed that there are quite drastic changes to the city expansion/growth code (which is currently disabled and causing segfaults when forced on, though this might be a local issue): cities-within-cities and overlapping cities are gone, and in fact all cities are constrained to non-overlapping squares. In actual games with fast growth, that results in extremely ugly maps, a little like a lost Tetris game, and feels like an unnatural limitation. Is the intention to come back and restore these features, or has a decision been made to remove them for code simplicity? IMHO, it would be best to keep overlapping cities

While I'm boldly requesting things, would it be possible to save/restore a city's wachstum in the next version of experimental save games? I've made that change locally, for very experimental local code that uses more than the last four bits of it, but would like to avoid incompatible SVEs. (Maybe it would also be a good idea to save next_growth_step and incoming/outgoing_private_cars, if the change is made?).

(I hope this is the right place, as it is a request, not a bug report. I'll get back to those soon!)

Code Select Expand


diff --git a/simcity.cc b/simcity.cc
index f06d28d..8b9d9e9 100644
--- a/simcity.cc
+++ b/simcity.cc
@@ -2240,6 +2240,12 @@ void stadt_t::rdwr(loadsave_t* file)
check_road_connexions = false;
}
}
+
+ if(file->get_experimental_version() >= 12 && file->get_version() >= 110000)
+ {
+ file->rdwr_long(wachstum);
+ }
+
}




[Philip]

Oh no, something appears to have gone wrong in the GIT repository: the rdwr_long(wachstum) moved into an if(file->is_loading()) block, so saved games can't be loaded. Suggest this change:

Code Select Expand


diff --git a/simcity.cc b/simcity.cc
index 54ac3e4..789e667 100644
--- a/simcity.cc
+++ b/simcity.cc
@@ -2240,11 +2240,13 @@ void stadt_t::rdwr(loadsave_t* file)
                        check_road_connexions = false;
                }

-               if(file->get_experimental_version() >= 12 && file->get_version() >= 110000)
-               {
-                       file->rdwr_long(wachstum);
-               }
        }
+
+       if(file->get_experimental_version() >= 12 && file->get_version() >= 110000)
+       {
+              file->rdwr_long(wachstum);
+       }
+
}




[jamespetts]

Thank you for your thoughts on this. A little explanation of the relevant branches might assist here: the passenger-generation branch is derived from my devel-new branch, into which is merged my 11.x branch, as well as another branch by a coder who was quite active a short while ago but seems to have taken a break from Simutrans recently, Nathaneal ("Neroden" on the forums), called the "fix-city-limits" branch. It is the code in the fix-city-limits branch that I think that you have found, and I think that overlapping cities were disabled on purpose because of the problems to which they led.

However, I did not realise that that branch also disabled the city growth code, which, having just tested it, it appears to do. This is most unfortunate, as city growth will have to be restored before the next major version can be released, and I do not know in detail the rationale behind the changes, and the person who does is not currently working on Simutrans-Experimental. I had wondered whether the issue was that I had forgotten to merge in the latest changes in the fix-city-limits branch, which I now have done, but, on testing, city growth has not been restored.

As to saving "wachstum", I have now added this to "devel-new" and "passenger-generation" (and fixed the bug that you identified in your most recent post - my apologies for getting that wrong), although may I ask why you added the second of the two checks?

Thank you very much for your work and testing on this - it is much appreciated.

[Philip]

Thank you, both for the code inclusion and the explanation. I take it the fix-city-limits branch is in another repository?

As for your last question, upon closer inspection, it appears the check is unnecessary, sorry about that (copy and paste strikes again).

[jamespetts]

Yes - the fix-city-limits branch is part of Neroden's repository. My devel-new branch has merged the fix-city-limits branch, however.

Thank you for clarifying in relation to the second check - as you will see, I have removed it.

[jamespetts]

I have just found and fixed the issue with this - Neroden had changed the system for upgrading city buildings. I had partly reverted this in order to try to track down the cause of crashes that I was getting some time ago, but it seems that my reversion was incompatible with Neroden's more recent code, which broke city growth entirely. I have now de-reverted to Neroden's original code, which now works.

[neroden]

Ah.

Um.

Saving "wachstum" (under a new name, and with more bits of precision, and allowing for negative numbers, and so on) was the *entire point* of the city-growth branch.  If you've gone ahead and saved it with the old number of bits of precision, you'll have to undo that in order to merge my city-growth branch.  Messy messy versioning.

It turns out that it's necessary to save the growth to many, many bits of precision in order to create a more carefully calibrated, slower growth system.  I think I got it saved properly on the city-growth branch, though I haven't actually created a new growth system.

[jamespetts]

Thank you for letting me know - I will look into that.