-
-
Notifications
You must be signed in to change notification settings - Fork 511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Oxygen and Carbon Dioxide Rebalance #5693
base: master
Are you sure you want to change the base?
Changes from 5 commits
6d66165
01a1eed
eb051d6
e887839
c87d669
d773d72
7858768
004a17a
6f2277c
eee7449
47c6b9a
f505fdb
497880f
7de5758
7a29ea5
854f37d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,12 +29,9 @@ public void OnTimePassed(double elapsed, double totalTimePassed) | |
private void ApplyCompoundsAddition() | ||
{ | ||
// These affect the final balance | ||
var outputModifier = 1.5f; | ||
var outputModifier = 1.0f; | ||
var inputModifier = 1.0f; | ||
|
||
// This affects how fast the conditions change, but also the final balance somewhat | ||
var modifier = 0.00015f; | ||
|
||
List<TweakedProcess> microbeProcesses = []; | ||
|
||
var cloudSizes = new Dictionary<Compound, float>(); | ||
|
@@ -49,8 +46,10 @@ private void ApplyCompoundsAddition() | |
if (patch.SpeciesInPatch.Count < 1) | ||
continue; | ||
|
||
float oxygenBalance = 0; | ||
float co2Balance = 0; | ||
float oxygenIn = 0; | ||
float oxygenOut = 0; | ||
float co2In = 0; | ||
float co2Out = 0; | ||
|
||
foreach (var species in patch.SpeciesInPatch) | ||
{ | ||
|
@@ -101,11 +100,11 @@ private void ApplyCompoundsAddition() | |
{ | ||
if (input.Key.ID is Compound.Oxygen) | ||
{ | ||
oxygenBalance -= input.Value * inputModifier * effectiveSpeed * species.Value; | ||
oxygenIn += input.Value * inputModifier * effectiveSpeed * species.Value; | ||
} | ||
else if (input.Key.ID is Compound.Carbondioxide) | ||
{ | ||
co2Balance -= input.Value * inputModifier * effectiveSpeed * species.Value; | ||
co2In += input.Value * inputModifier * effectiveSpeed * species.Value; | ||
} | ||
} | ||
|
||
|
@@ -114,27 +113,48 @@ private void ApplyCompoundsAddition() | |
{ | ||
if (output.Key.ID is Compound.Oxygen) | ||
{ | ||
oxygenBalance += output.Value * outputModifier * effectiveSpeed * species.Value; | ||
oxygenOut += output.Value * outputModifier * effectiveSpeed * species.Value; | ||
} | ||
else if (output.Key.ID is Compound.Carbondioxide) | ||
{ | ||
co2Balance += output.Value * outputModifier * effectiveSpeed * species.Value; | ||
co2Out += output.Value * outputModifier * effectiveSpeed * species.Value; | ||
} | ||
} | ||
} | ||
} | ||
|
||
// Scale the balances to make the changes less drastic | ||
oxygenBalance *= modifier; | ||
co2Balance *= modifier; | ||
float oxygenTarget; | ||
float co2Target; | ||
|
||
patch.Biome.TryGetCompound(Compound.Oxygen, CompoundAmountType.Biome, out var existingOxygen); | ||
patch.Biome.TryGetCompound(Compound.Carbondioxide, CompoundAmountType.Biome, out var existingCo2); | ||
|
||
float total = existingOxygen.Ambient + existingCo2.Ambient; | ||
|
||
changesToApply.Clear(); | ||
if (oxygenOut == 0) | ||
{ | ||
if (co2Out == 0) | ||
{ | ||
// in the rare event we aren't making either compound, do nothing | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But now consuming the compounds also doesn't do anything? I think this is a wrong conclusion to make in the code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
continue; | ||
} | ||
|
||
if (oxygenBalance != 0) | ||
changesToApply[Compound.Oxygen] = oxygenBalance; | ||
oxygenTarget = 0; | ||
co2Target = total; | ||
} | ||
else if (co2Out == 0) | ||
{ | ||
oxygenTarget = total; | ||
co2Target = 0; | ||
} | ||
else | ||
{ | ||
oxygenTarget = oxygenOut / (oxygenIn + co2In) * total; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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%. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair. Rather than explain here, I will add the comment and see if the explanation makes sense to you. |
||
co2Target = co2Out / (oxygenIn + co2In) * total; | ||
} | ||
|
||
if (co2Balance != 0) | ||
changesToApply[Compound.Carbondioxide] = co2Balance; | ||
changesToApply[Compound.Oxygen] = (oxygenTarget - existingOxygen.Ambient) / 2.0f; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's more efficient to multiply by 0.5f than to do a float division. |
||
changesToApply[Compound.Carbondioxide] = (co2Target - existingCo2.Ambient) / 2.0f; | ||
|
||
if (changesToApply.Count > 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
patch.Biome.ApplyLongTermCompoundChanges(patch.BiomeTemplate, changesToApply, cloudSizes); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using Godot; | ||
using Newtonsoft.Json; | ||
|
||
/// <summary> | ||
|
@@ -32,6 +33,8 @@ private void ApplyVolcanism() | |
{ | ||
foreach (var patchKeyValue in targetWorld.Map.Patches) | ||
{ | ||
GD.Print(patchKeyValue.Value + " volcanism"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like leftover debugging code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I knew I forgot something! |
||
|
||
// TODO: make these configured by the biomes.json file | ||
if (patchKeyValue.Value.BiomeType == BiomeType.Vents) | ||
{ | ||
|
@@ -72,7 +75,7 @@ private void ProduceCO2(Patch patch, float co2Strength, float threshold) | |
return; | ||
|
||
// TODO: should this clamp or not? | ||
addedCo2[Compound.Carbondioxide] = Math.Clamp(amount.Ambient + co2Strength, 0, threshold); | ||
addedCo2[Compound.Carbondioxide] = Math.Clamp(co2Strength, 0, threshold); | ||
hhyyrylainen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
patch.Biome.ApplyLongTermCompoundChanges(patch.BiomeTemplate, addedCo2, cloudSizesDummy); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this comment now incorrect?