Use a habitability factor when deciding on planet populations#6317
Use a habitability factor when deciding on planet populations#6317sam52796 wants to merge 2 commits into
Conversation
sturnclaw
left a comment
There was a problem hiding this comment.
Please note that the use of float operations in sector/system generation code is strongly discouraged at this time. While it can be assumed that supported platforms provide IEEE754-compliant basic operations, determinism is not guaranteed with the use of transcendentals and exponentiation functions such as std::exp which may have different implementations on different platforms/compilers.
I'd encourage you to write the equivalent code using the fixed.h header for fixed-point arithmetic. An approximation of std::exp may need to be implemented for 32.32 fixed-point numbers.
That aside, I do like the idea of this PR. The habitability score is fine for determining the quantity of people living "on the surface" of the planet, but an extension may be needed to approximate e.g. subsurface colonies on moons or otherwise inhospitable terrestrial planets.
Keep in mind that atmospheric surface pressure and atmospheric density are two different quantities - Earth's atmospheric density is 1.225kg/m^3, while its atmospheric pressure (returned from GetAtmSurfacePressure) is ~1.0atm. I see a confusion of those quantities in your habitability function.
As a note to other reviewers, we may have the ability to reduce our dependence on fixed-point computations - SSE(2) is fully deterministic (as long as the compiler can be forced to output correctly-ordered operations), but we'd need to provide our own approximations of transcendentals and ban the use of stdlib versions to maintain determinism.
|
SSE2 might be fully deterministic, but ARM cpus are becoming much more popular and we need cross-architecture determinability too. I'd imagine that RISC-V CPU will eventually get performant enough for desktop usage too |
|
I've just noticed that a previous save game now crashes when I load it using the new habitability factor code. It's trying to reference system body number 23 in a system that only has 22 system bodies. I assume what's happening is that changing the populations of some worlds has caused different branches in the procedural generation to be followed, affecting the random number consumption, and it ends up creating a slightly different universe compared to the one that was generated for that save file's game session. I'll see if I can tweak the system generator code so that it calls the random number function in the same order that it did before adding the habitability factor, so that the result is the same universe with only population numbers differing. |
|
The save file incompatibility was caused by the starport adding code, which adds more starports to planets with larger populations. Changing the population figures therefore changes how many starports are generated, which breaks anything that indexed them. I've fixed this issue by bumping up the galaxy generator version to 2. So the new adjusted population figures will only appear when someone starts a new game with this version. However, if this is not the intended use of the galaxy generation version number, then let me know and I'll try to find an alternative approach. |
This is expected behavior for changing the system generation code. Typically we merge PRs that affect procedural generation integrity in one batch sometime before the next release and bump the savefile version to invalidate old saves (rather than crashing). You can potentially seed a new Random object to roll the habitability values from the current state of the RNG object without affecting later uses of it, but as you mentioned later the population value is used "downstream" in flow control so you're not going to be able to avoid breaking compatibility. We do not typically use the galaxy generator version to version changes to the generators. Instead, we will increment the saved game version number. It's decidedly not worth the effort to maintain multiple versions of the system generator codepaths and select between them based upon which "version" of the galaxy is being loaded. |
|
Thanks. I've reverted the galaxy generation version increase, so it's back to how it was, being incompatible with previous saves. We could potentially maintain compatibility by only applying the habitability factor after all the other economy and station generation code has run, so that all uses the old numbers, while the new numbers end up mostly being for display purposes. But that seems to defeat the point somewhat. Not sure how often you do a save bump, but hopefully this can be included then. From a previous commit, the code uses fixed instead of float wherever it can. Approximating a normal distribution by using a Lorentz curve to the power of 9 seems to give broadly the same output. I've reviewed a few systems and I'm happy with the assigned populations, but if anyone is running this code and spots some numbers that need adjusting then let me know. Also happy to help with adding additional factors to the habitability if the opportunity arises... |
We typically release a major content update once per year on 03 Feb. We'll certainly merge the PR before then, though I personally prefer to break save compatibility late in the yearly development cycle (a few months before release) to allow saved games in bug reports to be loaded more easily during the earlier part of the development cycle.
That seems fine - perfect accuracy is not required. If you know you're working with extremely small numbers < 65536, you can potentially look at using a non-default fixed-point representation for increased accuracy in part or all of the computation, e.g.
Congratulations, you're the first person to materially change this specific code in ~10 years. You own it now. Happy maintaining! 😛 On a more serious note, the Pioneer "core team" (for lack of a better term) typically are happy to allow interested contributors to take over responsibility for their areas of interest in the codebase, so long as the contributions are in line with the overall design direction of the game and comply with specific technical guidelines and requirements. If you find something that the code is missing, or want to completely revamp how system generation assigns population values and generates settlements, feel free to do so! (I know a long-standing defect is that custom systems cannot set the population of starports or bodies directly and has to rely on the randomly-generated values which are often wildly different than the system's lore would imply...) |
|
At your leisure, please squash all "fixup" commits into the base commit and split the changes to the
Using the common subset of fully-specified operations that are compliant with the IEEE754 requirements for 32- and 64-bit precision floating-point numbers should allow for cross-platform determinism (in the absence of compiler reordering). The problem is more that |
|
In an effort to become best friends with @sturnclaw I've removed all trace of floating point from the habitability factor calculation, which meant I had to create CalcSurfaceGravityAsFixed() for SystemBody, among other things. I haven't rolled up the commits yet, so that you can see what I changed since the last time you had a look at this. If you find it all acceptable then let me know and I'll do that. |
sturnclaw
left a comment
There was a problem hiding this comment.
In an effort to become best friends with @sturnclaw I've removed all trace of floating point from the habitability factor calculation, which meant I had to create CalcSurfaceGravityAsFixed() for SystemBody, among other things.
The effort is much appreciated! I'd like to apologize if I've come off as a total grouch in prior interactions - I greatly value the effort being put forth here and your contribution of your time and effort towards the project.
As a note towards the future, the expression fixed(N, 1) is equivalent to writing the integer N in most expressions, as the fixed-point class has automatic promotion from signed integers to the equivalent fixed-point representation for most basic math operations.
069171f to
b37f242
Compare
Fixes #6229 by adding a habitability factor when calculating populations, so that they scale better with planetary conditions. Earth now has ~13bn population, while Deimos and Phobos have a few thousand each.
Also fixed some issues with the planet specifications in the Sol system:
After the new habitability factor, the following planets needed tweaking in Epsilon Eridani:
Finally, I added two more checks at the points where planets are rejected for settlement population: