Global glaciation event #5974

Patryk26g · 2025-03-02T21:08:54Z

Brief Description of What This PR Does

Adds global glaciation event. Still needs the new icon and translation files

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.

simulation_parameters/microbe_stage/compounds.json

src/general/world_effects/WorldEffectVisuals.cs

simulation_parameters/Constants.cs

src/microbe_stage/BiomeConditions.cs

src/general/world_effects/patch_events/GlobalGlaciationEvent.cs

simulation_parameters/Constants.cs

hhyyrylainen

I started commenting and ended up doing an almost full review... though once I saw this major architecture problem I stopped as I think that needs to be the first thing to be actually fixed before it makes sense to polish the rest of the code.

src/general/world_effects/patch_events/GlobalGlaciationEvent.cs

# Conflicts: # locale/af.po # locale/ar.po # locale/be.po # locale/bg.po # locale/bn.po # locale/ca.po # locale/cs.po # locale/da.po # locale/de.po # locale/el.po # locale/en.po # locale/eo.po # locale/es.po # locale/es_AR.po # locale/et.po # locale/fi.po # locale/fr.po # locale/frm.po # locale/gsw.po # locale/he.po # locale/hr.po # locale/hu.po # locale/id.po # locale/it.po # locale/ja.po # locale/ka.po # locale/ko.po # locale/la.po # locale/lb_LU.po # locale/lt.po # locale/lv.po # locale/messages.pot # locale/mk.po # locale/nb_NO.po # locale/nl.po # locale/nl_BE.po # locale/pl.po # locale/pt_BR.po # locale/pt_PT.po # locale/ro.po # locale/ru.po # locale/si_LK.po # locale/sk.po # locale/sr_Cyrl.po # locale/sr_Latn.po # locale/sv.po # locale/th_TH.po # locale/tok.po # locale/tr.po # locale/tt.po # locale/uk.po # locale/vi.po # locale/zh_CN.po # locale/zh_TW.po

Remove caves from the event

hhyyrylainen · 2025-03-10T09:31:50Z

assets/textures/gui/bevel/GlobalGlaciationEvent.svg

I think this is not the latest icon posted on the discord?

Yes, I didn't have time to update it yet

hhyyrylainen · 2025-03-10T09:33:10Z

locale/en.po

+"Global glaciation event\n"
+"The event has been caused by rising concentration of oxygen in the atmosphere.\n"
+"It will persist for several generations, reducing sunlight and temperature.\n"
+"Snow chunks also appear in these patches now."


Suggested change

"Snow chunks also appear in these patches now."

"Ice chunks also appear in these patches now."

hhyyrylainen

I only saw one major problem left, which is the modification of the biome template which needs to be gotten rid of. Other than that I commented on mostly just smaller details that should be easier to fix. Though I suppose me asking for patch snapshot system support for more events at once is a bit complicated change to make but makes it much more manageable for future event writers.

hhyyrylainen · 2025-03-10T09:33:55Z

simulation_parameters/microbe_stage/compounds.json

@@ -209,7 +209,7 @@
    "Volume": 1,
    "IsCloud": false,
    "IsAlwaysUseful": false,
-    "IsEnvironmental": true,
+    "IsEnvironmental": false,


This will break photosynthesis entirely... as the process system won't anymore pick up the sunlight from the environment.

hhyyrylainen · 2025-03-10T09:34:25Z

src/general/world_effects/WorldEffectVisuals.cs

@@ -4,6 +4,6 @@
    ///   Used as a placeholder value instead of null
    /// </summary>
    None,
-


Isn't this blank line required by the formatter as there's an XML comment so None is a actually a multiline block meaning that there must be a blank line after it?

hhyyrylainen · 2025-03-10T09:35:31Z

src/microbe_stage/editor/MicrobeEditor.cs

@@ -257,6 +257,7 @@ protected override void ElapseEditorEntryTime()
    {
        // TODO: select which units will be used for the master elapsed time counter
        CurrentGame.GameWorld.OnTimePassed(1);
+        cellEditorTab.UpdateBackgroundImage(CurrentPatch.BiomeTemplate);


I think a comment here would be helpful about the events being able to change the properties of the current patch.

hhyyrylainen · 2025-03-10T09:36:32Z

src/multicellular_stage/editor/MulticellularEditor.cs

@@ -304,6 +304,7 @@ protected override void ElapseEditorEntryTime()
    {
        // TODO: select which units will be used for the master elapsed time counter
        CurrentGame.GameWorld.OnTimePassed(1);
+        cellEditorTab.UpdateBackgroundImage(CurrentPatch.BiomeTemplate);


Or if every single editor needs to implement this, then I think it might be worth considering putting this into the base editor class.

hhyyrylainen · 2025-03-10T09:38:00Z

src/microbe_stage/Patch.cs

+        BiomeTemplate.Background = currentSnapshot.Background;
+        BiomeTemplate.Sunlight.Colour = currentSnapshot.LightColour;


Hang on, this cannot be safe, right? This is modifying the Biome class which is a registry type, which is absolutely forbidden from being modified after loading by SimulationParameters.

hhyyrylainen · 2025-03-10T09:43:42Z

src/general/world_effects/patch_events/GlobalGlaciationEvent.cs

+        var changes = new Dictionary<Compound, float>
+        {
+            [Compound.Temperature] = currentTemperature.Ambient,
+            [Compound.Sunlight] = currentSunlight.Ambient,
+        };
+
+        patch.Biome.ApplyLongTermCompoundChanges(patch.BiomeTemplate, changes, new Dictionary<Compound, float>());


As this method is applied to multiple patches the changes and the blank created dictionary here should both be class fields so that they don't need to be re-allocated for each method call.

hhyyrylainen · 2025-03-10T09:45:03Z

src/general/world_effects/patch_events/GlobalGlaciationEvent.cs

+        foreach (var index in modifiedPatchesIds)
+        {
+            if (!targetWorld.Map.Patches.TryGetValue(index, out var patch))
+                continue;


Shouldn't this give a warning that a patch has exited the world as that's not expected to happen?

hhyyrylainen · 2025-03-10T09:48:01Z

src/general/world_effects/patch_events/GlobalGlaciationEvent.cs

+            PatchSnapshot patchSnapshot = patch.History[eventDuration];
+
+            ResetBackground(patch, patchSnapshot);
+            ResetEnvironment(patch, patchSnapshot);
+            RemoveChunks(patch);


For multiple events to work concurrently, I think we need like a flag in the history snapshot to tell which events are active, that way the history can be scanned for when no event is active. Imagine that we add like a volcano event, and things go like:

gen 1 no event

gen 2 volcano event starts

gen 3 glaciation starts

gen 4 volcano ends and it reverts things (this can be figured out later but I think the history format should be fixed now)

gen 6 glaciation ends and it reverts things back to gen 2 which is incorrect as the light values will be lower than the default in gen 1

So I think the history needs a flag for active events and probably a helper method for finding conditions without events to revert to by looking backwards.

hhyyrylainen · 2025-03-10T09:48:10Z

src/general/world_effects/patch_events/GlobalGlaciationEvent.cs

+    {
+        var biomeBackground = patchSnapshot.Background;
+        var biomeLightColour = patchSnapshot.LightColour;
+        patch.BiomeTemplate.Background = biomeBackground;


More template modifying.

hhyyrylainen · 2025-03-10T09:49:23Z

src/general/world_effects/patch_events/GlobalGlaciationEvent.cs

+        currentTemperature.Ambient = previousTemperature.Ambient - currentTemperature.Ambient;
+        currentSunlight.Ambient = previousSunlight.Ambient - currentSunlight.Ambient;
+
+        var changes = new Dictionary<Compound, float>


I think it would be much easier to use the ModifyLongTermCondition(Compound compound, BiomeCompoundProperties newValue) overload here to just apply the old values back.

Patryk26g · 2025-03-10T10:08:05Z

I only saw one major problem left, which is the modification of the biome template which needs to be gotten rid of. Other than that I commented on mostly just smaller details that should be easier to fix. Though I suppose me asking for patch snapshot system support for more events at once is a bit complicated change to make but makes it much more manageable for future event writers.

How would you then approach the modifIcation o the background image if the template cannot be changed? The only solution I see is getting the background image from the current snapshot, is that the right approach?

hhyyrylainen · 2025-03-10T10:09:39Z

I would make it so that the template variable can be swapped. So the template objects are immutable, but which template is in use can be changed. So the patches would swap their template from whatever to the ice shelf to apply the background etc. properties.

Patryk26g · 2025-03-10T18:37:11Z

I would make it so that the template variable can be swapped. So the template objects are immutable, but which template is in use can be changed. So the patches would swap their template from whatever to the ice shelf to apply the background etc. properties.

Right, but that would also change the icon of the patch and besides that I want to create a custom light color, maybe Biome should implement ICloneable so that I can clone it and change its properties?

hhyyrylainen · 2025-03-10T19:45:26Z

I think the map icons changing is fine. And if you want a custom light property then a new biome template could be added that is only used for the glaciation event (and not generated on the map by default).

Patryk26g · 2025-03-10T21:56:04Z

I think I would prefer the icons to stay as they are but if there is no simple solution then I'll do it as you saying. The only other solutuion I have in mind is creating a new template based on the base one this way:

private void CreateGlaciatedBiomeTemplate(Biome biomeTemplate)
    {
        Biome glaciatedBiomeTemplate = new Biome
        {
            ActiveMusicContexts = biomeTemplate.ActiveMusicContexts,
            Background = "iceshelf",
            Conditions = (BiomeConditions)biomeTemplate.Conditions.Clone(),
            Icon = biomeTemplate.Icon,
            InternalName = biomeTemplate.InternalName,
            Name = biomeTemplate.Name,
            Sunlight = new Biome.LightDetails
            {
                Colour = new Color(0.8f, 0.8f, 1, 1),
            },
            Panorama = biomeTemplate.Panorama,
        };
    }

Is that an acceptable approach?

hhyyrylainen · 2025-03-10T22:25:14Z

Cloning the biome template again has the issue that it will make save compatibility a nightmare as any slight change to the biomes data will not propagate that config change to the cloned templates. So the choice is either a full template switch system where patches can swap between templates due to events, or an addition of data to the snapshots that allow overriding the properties (background image, sunlight, though there will probably be even more so this will bloat the snapshots).

If the snapshots approach is taken even the background images need to be carefully designed how they are saved as it is once again bad for save compatibility if they store the exact paths to the background images. So another layer of indirection will be needed there. That's also one reason why I think that allowing the template type to switch dynamically is the easiest solution in terms of allowing everything to change but at the same time not introducing save compatibility gotchas.

Patryk26g added 3 commits March 1, 2025 18:27

Global glaciation event

f75c672

Name change

74fceef

Save fix

d2958a6

Patryk26g marked this pull request as draft March 2, 2025 21:09

hhyyrylainen added this to the Release 0.8.2 milestone Mar 3, 2025