Oxygen and Carbon Dioxide Rebalance #5693

adQuid · 2024-11-18T12:12:45Z

Brief Description of What This PR Does

Balances O2 and CO2 more tightly in the special case of that (mostly) closed loop between them. Also simplifies the diffusion simulation to let those compounds spread a little more, and modifies the auto-evo to make photosynthesisers more common to allow for more oxygen in the first place.

Related Issues

Progress Checklist

Note: before starting this checklist the PR should be marked as non-draft.

PR author has checked that this PR works as intended and doesn't
break existing features:
https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
(this is important as to not waste the time of Thrive team
members reviewing this PR)
Initial code review passed (this and further items should not be checked by the PR author)
Functionality is confirmed working by another person (see above checklist link)
Final code review is passed and code conforms to the
styleguide.

Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.

hhyyrylainen · 2024-11-18T12:34:44Z

src/general/world_effects/VolcanismEffect.cs

@@ -32,6 +33,8 @@ private void ApplyVolcanism()
    {
        foreach (var patchKeyValue in targetWorld.Map.Patches)
        {
+            GD.Print(patchKeyValue.Value + " volcanism");


This looks like leftover debugging code.

I knew I forgot something!

adQuid · 2024-11-21T01:13:56Z

I want to make a future PR doing the diffusion better, but right now I think this is good enough for surface patches. Would appreciate lots of testing, since that gets very time-intensive.

src/general/world_effects/VolcanismEffect.cs

hhyyrylainen · 2024-11-21T08:34:41Z

src/general/world_effects/CompoundDiffusionEffect.cs

@@ -27,14 +26,12 @@ public void OnTimePassed(double elapsed, double totalTimePassed)
        HandlePatchCompoundDiffusion();
    }

-    private static (float Ambient, float Density) CalculateWantedMoveAmounts(Patch sourcePatch, Patch adjacent,
+    private static (float Ambient, float Density) CalculateWantedMoveAmounts(Patch adjacent,
        KeyValuePair<Compound, BiomeCompoundProperties> compound)
    {
        // Apply patch distance to diminish how much to move (to make ocean bottoms receive less surface
        // resources like oxygen)


Isn't this comment now incorrect?

src/general/world_effects/CompoundDiffusionEffect.cs

hhyyrylainen · 2024-11-21T08:36:43Z

src/general/world_effects/PhotosynthesisProductionEffect.cs

+            {
+                if (co2Out == 0)
+                {
+                    // in the rare event we aren't making either compound, do nothing


But now consuming the compounds also doesn't do anything? I think this is a wrong conclusion to make in the code.

You are correct. If microbes had only bio processes that destroyed O2 or CO2 without producing anything in return, this code doesn't account for that. Right now the PhotosynthesisProductionEffect class is dependent on the special case of conservation of mass (and that CO2 and O2 are only being converted into each other)

The code shouldn't depend on that then as someone will make a mod eventually that breaks the laws of physics and will get confused why it doesn't do things correctly.

alright, I've modified the code to handle matter sinks, and while a matter generator will result in absurd numbers, I've protected against divide-by-zero exceptions.

hhyyrylainen · 2024-11-21T08:37:27Z

src/general/world_effects/PhotosynthesisProductionEffect.cs


-            if (co2Balance != 0)
-                changesToApply[Compound.Carbondioxide] = co2Balance;
+            changesToApply[Compound.Oxygen] = (oxygenTarget - existingOxygen.Ambient) / 2.0f;


It's more efficient to multiply by 0.5f than to do a float division.

hhyyrylainen · 2024-11-21T08:40:13Z

src/general/world_effects/PhotosynthesisProductionEffect.cs

+            }
+            else
+            {
+                oxygenTarget = oxygenOut / (oxygenIn + co2In) * total;


Could you add a comment here explaining the logic? I don't really understand this kind of scaling because ApplyLongTermCompoundChanges already does a relative applying of gases. So higher oxygen output reduces the effect of the co2 adding and vice versa. So to me it looks like this is now doing the same effect twice, which doesn't logically make a ton of sense as ApplyLongTermCompoundChanges needs to be given the absolute change wanted and then it automatically makes gases related to each other and caps it to a 100%.

Fair. Rather than explain here, I will add the comment and see if the explanation makes sense to you.

hhyyrylainen · 2024-11-26T13:22:04Z

src/general/world_effects/PhotosynthesisProductionEffect.cs

+            // Special (but common) case where zero oxygen is being produced
+            if (oxygenProduced == 0 && oxygenConsumed > 0)
+            {
+                oxygenTarget = 0.0f;


Doesn't this make the game even more harsh? Wasn't the goal of this PR to make the changes less extreme? I think whatever trace oxygen there is should be allowed to be slowly consumed over time instead of immediately forcing it to zero...

hhyyrylainen · 2024-11-26T13:22:53Z

src/general/world_effects/PhotosynthesisProductionEffect.cs

+            // Special (but common) case where zero carbon dioxide is being produced
+            if (co2Produced == 0 && co2Consumed > 0)
+            {
+                co2Target = 0.0f;


This is flat out wrong, there's the volcanism effect that generates co2 so this would override that impact. So even if my previous comment doesn't absolutely need to be addressed, this does have to.

hhyyrylainen · 2024-11-26T13:23:00Z

src/general/world_effects/PhotosynthesisProductionEffect.cs


-            if (oxygenBalance != 0)
-                changesToApply[Compound.Oxygen] = oxygenBalance;
+            // if both compounds are being produced, calculate an aproximate steady state value


Suggested change

// if both compounds are being produced, calculate an aproximate steady state value

// If both compounds are being produced, calculate an aproximate steady state value

hhyyrylainen · 2024-11-26T13:25:10Z

src/general/world_effects/PhotosynthesisProductionEffect.cs

-            if (co2Balance != 0)
-                changesToApply[Compound.Carbondioxide] = co2Balance;
+            changesToApply[Compound.Oxygen] = (oxygenTarget - existingOxygen.Ambient) * 0.5f;
+            changesToApply[Compound.Carbondioxide] = (co2Target - existingCo2.Ambient) * 0.5f;

            if (changesToApply.Count > 0)


This if no longer works correctly after your changes because even if the change to apply is less than EPSILON the value is added to the dictionary so this check no longer works. Either it needs to be reworked that small values (so close to target) are not processed or this needs to loop here to check if any of the changes are above EPSILON. I think the first approach will be easier to do and clearer to understand here.

hhyyrylainen

I'd like to see a few more small changes (I forgot to post my last comment as part of this review).

HexapodPhilosopher · 2024-11-27T16:14:51Z

On testing this today: the compounds are a lot more stable, but it's difficult to get the oxygen level to stay above 11%. My plant cells had to rely on hydrogenase to avoid using most of the oxygen in most patches. And even after 20 generations of maximizing my population in all surface patches, half of these had metabolosomes functioning worse than hydrogenase.

from adQuid's PR #5693

hhyyrylainen · 2024-12-12T13:31:50Z

I'll mark this as a draft pending changes.

I've taken the compound diffusion change and made it into a separate PR: #5743

* Updated the normal effect variant documentation text to not refer to the removed classes * Added the simpler compound move multiplier calculation from adQuid's PR #5693

adQuid added 5 commits November 10, 2024 08:58

make compound changes work on targets instead of raw additions

6d66165

rebalance world effects

01a1eed

fix return statement

eb051d6

more trimming

e887839

remove unneeded import

c87d669

hhyyrylainen added this to the Release 0.8.0 milestone Nov 18, 2024

hhyyrylainen reviewed Nov 18, 2024

View reviewed changes

adQuid added 4 commits November 18, 2024 08:02

remove debugging code

d773d72

remove unused parameter (might add back later)

7858768

Merge branch 'master' into compound_gen_rebalance

004a17a

Merge branch 'master' into compound_gen_rebalance

6f2277c

adQuid marked this pull request as ready for review November 21, 2024 01:13

adQuid added the review label Nov 21, 2024

hhyyrylainen reviewed Nov 21, 2024

View reviewed changes

src/general/world_effects/VolcanismEffect.cs Outdated Show resolved Hide resolved

hhyyrylainen requested changes Nov 21, 2024

View reviewed changes

adQuid added 6 commits November 23, 2024 18:24

minor review corrections

eee7449

rework equilibrium equations

47c6b9a

whoops

f505fdb

drop unneeded import

497880f

Merge branch 'master' into compound_gen_rebalance

7de5758

Merge branch 'master' into compound_gen_rebalance

7a29ea5

hhyyrylainen reviewed Nov 26, 2024

View reviewed changes

hhyyrylainen requested changes Nov 26, 2024

View reviewed changes

Merge branch 'master' into compound_gen_rebalance

854f37d

hhyyrylainen marked this pull request as draft December 12, 2024 13:23

hhyyrylainen added a commit that referenced this pull request Dec 12, 2024

Added the simpler compound move multiplier calculation

c5399c2

from adQuid's PR #5693

hhyyrylainen mentioned this pull request Dec 12, 2024

Faster diffuse of compounds between patches #5743

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Oxygen and Carbon Dioxide Rebalance #5693

Oxygen and Carbon Dioxide Rebalance #5693

adQuid commented Nov 18, 2024

hhyyrylainen Nov 18, 2024

adQuid Nov 18, 2024

adQuid commented Nov 21, 2024

hhyyrylainen Nov 21, 2024

hhyyrylainen Nov 21, 2024

adQuid Nov 21, 2024

hhyyrylainen Nov 21, 2024

adQuid Nov 24, 2024

hhyyrylainen Nov 21, 2024

hhyyrylainen Nov 21, 2024

adQuid Nov 21, 2024

hhyyrylainen Nov 26, 2024

hhyyrylainen Nov 26, 2024

hhyyrylainen Nov 26, 2024

hhyyrylainen Nov 26, 2024

hhyyrylainen left a comment

HexapodPhilosopher commented Nov 27, 2024

hhyyrylainen commented Dec 12, 2024

	// if both compounds are being produced, calculate an aproximate steady state value
	// If both compounds are being produced, calculate an aproximate steady state value

Oxygen and Carbon Dioxide Rebalance #5693

Are you sure you want to change the base?

Oxygen and Carbon Dioxide Rebalance #5693

Conversation

adQuid commented Nov 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adQuid commented Nov 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hhyyrylainen left a comment

Choose a reason for hiding this comment

HexapodPhilosopher commented Nov 27, 2024

hhyyrylainen commented Dec 12, 2024