Fix processes' toggle buttons not matching their actual state (#5632)

* Fix process toggle buttons not matching their actual states

* Fix button flickering

* Make processes save their enabled-state when updated

* Remove obsolete comment

* Remove a redundant function call

* Remove a redundant 'using'

* Style fix

* Fix processes not being togglable on early multicellular organisms

* Style fix

* Remove debug prints from ProcessStatistics

* Make TweakedProcess a struct (it was unnecessary for it to be a class)

* Revert a line break

* Style fix

* Add an error print

* Add a debug exception throw

* Fix wrong code revert

* Remove some new comparisons

* Reduce temporary memory allocations

* Move memory allocations out of ProcessSystem

* Save some more memory allocations

* Fix style

* Revert temporary memory; move to a new approach

* Remove an unneeded tempMemory allocation

* Change unmarked process removal algorithm

* Style fix

* Improve removal algorithm

* Some code cleanup

* Add some clarifying comments, remove the initial process unmarking cycle

* Change the way the process button is toggled

* Fix the order of process panel processes changing

when toggling on/off state

* Fixed toggle button status no longer working after

the last commit

* Improved efficiency of process list computation for production effect

so that it doesn't uselessly try to combine processes

* Fix bug in process calculation where old rates where added to new ones

causing process speed to just go up, and fixed process order being able to change based on the old data which is not very good for reliability as it'd be very hard to track down problems that only occur when old cached data is in specific order

---------

Co-authored-by: Henri Hyyryläinen <[email protected]>

src/general/world_effects/PhotosynthesisProductionEffect.cs

@@ -67,6 +67,8 @@ private void ApplyCompoundsAddition()
                 if (balance.TotalConsumption < balance.TotalProduction)
                     balanceModifier = balance.TotalConsumption / balance.TotalProduction;
 
+                // Cleared for efficiency
+                microbeProcesses.Clear();
                 ProcessSystem.ComputeActiveProcessList(microbeSpecies.Organelles, ref microbeProcesses);
 
                 foreach (var process in microbeProcesses)

src/gui_common/ChemicalEquation.cs

@@ -59,8 +59,6 @@ public partial class ChemicalEquation : VBoxContainer
     /// </summary>
     private bool hasNoInputs;
 
-    private bool lastToggle = true;
-
     [Signal]
     public delegate void ToggleProcessPressedEventHandler(ChemicalEquation thisEquation);
 
@@ -102,14 +100,22 @@ public bool ShowToggle
 
     public bool ProcessEnabled
     {
-        get => lastToggle;
+        get
+        {
+            if (EquationFromProcess == null)
+                return true;
+
+            return EquationFromProcess.Enabled;
+        }
         set
         {
-            if (value == lastToggle)
+            if (EquationFromProcess == null)
+            {
+                GD.PrintErr("Display info isn't assigned to a ChemicalEquation, can't toggle process");
                 return;
+            }
 
-            lastToggle = value;
-            ApplyProcessToggleValue();
+            EmitSignal(SignalName.ToggleProcessPressed, this, value);
         }
     }
 
@@ -149,7 +155,6 @@ public bool ShowFullSecondText
     public override void _Ready()
     {
         UpdateEquation();
-        ApplyProcessToggleValue();
     }
 
     public override void _EnterTree()
@@ -245,6 +250,8 @@ private void UpdateEquation()
 
         // Environment conditions
         UpdateEnvironmentPart(environmentalInputs);
+
+        ApplyProcessToggleValue();
     }
 
     private void UpdateHeader()
@@ -373,11 +380,9 @@ private string GetEnvironmentLabelText()
         return Localization.Translate("PROCESS_ENVIRONMENT_SEPARATOR");
     }
 
-    private void ToggleButtonPressed(bool toggled)
+    private void ToggleButtonPressed()
     {
-        ProcessEnabled = toggled;
-
-        EmitSignal(SignalName.ToggleProcessPressed, this);
+        ProcessEnabled = !ProcessEnabled;
     }
 
     private void ApplyProcessToggleValue()

src/gui_common/ChemicalEquation.tscn

@@ -64,4 +64,4 @@ layout_mode = 2
 mouse_filter = 2
 theme = SubResource("1")
 
-[connection signal="toggled" from="HBoxContainer/ToggleProcess" to="." method="ToggleButtonPressed"]
+[connection signal="pressed" from="HBoxContainer/ToggleProcess" to="." method="ToggleButtonPressed"]

src/microbe_stage/IProcessDisplayInfo.cs

@@ -43,6 +43,8 @@ public interface IProcessDisplayInfo : IEquatable<IProcessDisplayInfo>
     /// </summary>
     public float CurrentSpeed { get; }
 
+    public bool Enabled { get; }
+
     /// <summary>
     ///   The limiting compounds in speed. Or null if not set
     /// </summary>

src/microbe_stage/MicrobeHUD.cs

@@ -841,7 +841,7 @@ private void OnTranslationsChanged()
         UpdateColonySizeForMacroscopic();
     }
 
-    private void ToggleProcessPressed(ChemicalEquation equation)
+    private void ToggleProcessPressed(ChemicalEquation equation, bool enabled)
     {
         if (!stage!.HasAlivePlayer || !stage.Player.Has<BioProcesses>())
             return;
@@ -852,9 +852,26 @@ private void ToggleProcessPressed(ChemicalEquation equation)
             return;
         }
 
-        ref var processes = ref stage.Player.Get<BioProcesses>();
+        if (stage.Player.Has<MicrobeColony>())
+        {
+            ref var colony = ref stage.Player.Get<MicrobeColony>();
+
+            foreach (var colonyMember in colony.ColonyMembers)
+            {
+                ref var bioProcesses = ref colonyMember.Get<BioProcesses>();
+                ToggleProcessOnEntity(equation, enabled, ref bioProcesses);
+            }
+        }
+        else
+        {
+            ref var bioProcesses = ref stage.Player.Get<BioProcesses>();
+            ToggleProcessOnEntity(equation, enabled, ref bioProcesses);
+        }
+    }
 
-        var activeProcesses = processes.ActiveProcesses;
+    private void ToggleProcessOnEntity(ChemicalEquation equation, bool enabled, ref BioProcesses bioProcesses)
+    {
+        var activeProcesses = bioProcesses.ActiveProcesses;
 
         if (activeProcesses == null)
             return;
@@ -864,10 +881,10 @@ private void ToggleProcessPressed(ChemicalEquation equation)
         for (int i = 0; i < processesCount; ++i)
         {
             // Update speed of the process controlled by the GUI control that signaled this change
-            if (equation.EquationFromProcess.MatchesUnderlyingProcess(activeProcesses[i].Process))
+            if (equation.EquationFromProcess!.MatchesUnderlyingProcess(activeProcesses[i].Process))
             {
                 var process = activeProcesses[i];
-                process.SpeedMultiplier = equation.ProcessEnabled ? 1 : 0;
+                process.SpeedMultiplier = enabled ? 1 : 0;
                 activeProcesses[i] = process;
             }
         }

src/microbe_stage/ProcessList.cs

@@ -88,8 +88,6 @@ private ChemicalEquation CreateEquation(StrictProcessDisplayInfoEquality process
 
         equation.Connect(SignalName.ToggleProcessPressed, new Callable(this, nameof(HandleToggleProcess)));
 
-        equation.ProcessEnabled = process.DisplayInfo.CurrentSpeed > 0;
-
         if (ProcessesTitleColour != null)
             equation.DefaultTitleFont = ProcessesTitleColour;
 
@@ -99,8 +97,8 @@ private ChemicalEquation CreateEquation(StrictProcessDisplayInfoEquality process
         return equation;
     }
 
-    private void HandleToggleProcess(ChemicalEquation equation)
+    private void HandleToggleProcess(ChemicalEquation equation, bool enabled)
     {
-        EmitSignal(SignalName.ToggleProcessPressed, equation);
+        EmitSignal(SignalName.ToggleProcessPressed, equation, enabled);
     }
 }

src/microbe_stage/ProcessPanel.cs

@@ -64,8 +64,8 @@ protected override void Dispose(bool disposing)
         base.Dispose(disposing);
     }
 
-    private void ToggleProcessToggled(ChemicalEquation equation)
+    private void ToggleProcessToggled(ChemicalEquation equation, bool enabled)
     {
-        EmitSignal(SignalName.ToggleProcessPressed, equation);
+        EmitSignal(SignalName.ToggleProcessPressed, equation, enabled);
     }
 }

src/microbe_stage/ProcessSpeedInformation.cs

@@ -39,6 +39,8 @@ public ProcessSpeedInformation(BioProcess process)
 
     public float CurrentSpeed { get; set; }
 
+    public bool Enabled => true;
+
     /// <summary>
     ///   Efficiency is a measure of how well the environment is favorable to the process.
     /// </summary>

src/microbe_stage/ProcessStatistics.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using Godot;
 using Newtonsoft.Json;
 using Nito.Collections;
@@ -15,24 +16,19 @@ public class ProcessStatistics
     ///   a ThreadLocal in <see cref="Systems.ProcessSystem"/> as that was causing the game process to lock up in
     ///   Godot 4 (for unknown reasons). See: https://github.com/Revolutionary-Games/Thrive/issues/4989 for context.
     /// </summary>
-    private List<BioProcess>? temporaryRemovedItems;
+    private List<TweakedProcess>? temporaryRemovedItems;
 
     /// <summary>
     ///   The processes and their associated speed statistics
     /// </summary>
     /// <remarks>
     ///   <para>
-    ///     This uses <see cref="BioProcess"/> rather than <see cref="TweakedProcess"/> as the key so that equality
-    ///     comparison matches based on the process type not the process type and speed. All processables should
-    ///     combine their processes to run correctly with speed tracking.
-    ///   </para>
-    ///   <para>
     ///     This is JSON ignore to ensure that this object can exist in saves, but won't store non-savable information
     ///     like the process statistics object. That's the situation now but maybe some other design would be better...
     ///   </para>
     /// </remarks>
     [JsonIgnore]
-    public Dictionary<BioProcess, SingleProcessStatistics> Processes { get; } = new();
+    public Dictionary<TweakedProcess, SingleProcessStatistics> Processes { get; } = new();
 
     public void MarkAllUnused()
     {
@@ -49,7 +45,7 @@ public void RemoveUnused()
     {
         lock (Processes)
         {
-            temporaryRemovedItems ??= new List<BioProcess>();
+            temporaryRemovedItems ??= new List<TweakedProcess>();
 
             foreach (var entry in Processes)
             {
@@ -71,10 +67,10 @@ public void RemoveUnused()
         }
     }
 
-    public SingleProcessStatistics GetAndMarkUsed(BioProcess forProcess)
+    public SingleProcessStatistics GetAndMarkUsed(TweakedProcess forProcess)
     {
 #if DEBUG
-        if (forProcess == null!)
+        if (forProcess.Process == null!)
             throw new ArgumentException("Invalid process marked used");
 #endif
 
@@ -109,7 +105,7 @@ public class SingleProcessStatistics : IProcessDisplayInfo
 
     private Dictionary<Compound, float>? precomputedEnvironmentInputs;
 
-    public SingleProcessStatistics(BioProcess process,
+    public SingleProcessStatistics(TweakedProcess process,
         float keepSnapshotTime = Constants.DEFAULT_PROCESS_STATISTICS_AVERAGE_INTERVAL)
     {
         this.keepSnapshotTime = keepSnapshotTime;
@@ -120,11 +116,11 @@ public SingleProcessStatistics(BioProcess process,
     /// <summary>
     ///   The process these statistics are for
     /// </summary>
-    public BioProcess Process { get; }
+    public TweakedProcess Process { get; private set; }
 
     public bool Used { get; internal set; }
 
-    public string Name => Process.Name;
+    public string Name => Process.Process.Name;
 
     public IEnumerable<KeyValuePair<Compound, float>> Inputs =>
         LatestSnapshot?.Inputs.Where(p => !IProcessDisplayInfo.IsEnvironmental(p.Key)) ??
@@ -135,7 +131,7 @@ public SingleProcessStatistics(BioProcess process,
         throw new InvalidOperationException("No snapshot set");
 
     public IReadOnlyDictionary<Compound, float> FullSpeedRequiredEnvironmentalInputs =>
-        precomputedEnvironmentInputs ??= Process.Inputs
+        precomputedEnvironmentInputs ??= Process.Process.Inputs
             .Where(p => IProcessDisplayInfo.IsEnvironmental(p.Key.ID))
             .ToDictionary(p => p.Key.ID, p => p.Value);
 
@@ -154,6 +150,8 @@ public float CurrentSpeed
         }
     }
 
+    public bool Enabled => Process.SpeedMultiplier > 0;
+
     public IReadOnlyList<Compound>? LimitingCompounds => LatestSnapshot?.LimitingCompounds;
 
     private SingleProcessStatisticsSnapshot? LatestSnapshot =>
@@ -294,9 +292,24 @@ public void Clear()
         precomputedEnvironmentInputs = null;
     }
 
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public void UpdateProcessDataIfNeeded(in TweakedProcess process)
+    {
+        if (!MatchesUnderlyingProcess(process.Process))
+        {
+            GD.PrintErr("Wrong process info passed to SingleProcessStatistics for data update");
+            return;
+        }
+
+        // Copying the whole struct is needed here, even though really we'd just need to copy the speed multiplier as
+        // that's the only thing that is allowed to change
+        Process = process;
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public bool MatchesUnderlyingProcess(BioProcess process)
     {
-        return Process == process;
+        return Process.Process == process;
     }
 
     public bool Equals(IProcessDisplayInfo? other)
@@ -394,9 +407,13 @@ public AverageProcessStatistics(SingleProcessStatistics owner)
     public float CurrentSpeed { get; set; }
     public IReadOnlyList<Compound> LimitingCompounds => WritableLimitingCompounds;
 
+    public TweakedProcess Process => owner.Process;
+
+    public bool Enabled => owner.Enabled;
+
     public bool MatchesUnderlyingProcess(BioProcess process)
     {
-        return owner.Process == process;
+        return owner.Process.Process == process;
     }
 
     public bool Equals(IProcessDisplayInfo? other)

src/microbe_stage/StrictProcessDisplayInfoEquality.cs

@@ -2,7 +2,8 @@
 using System.Linq;
 
 /// <summary>
-///   A strict equality comparison for <see cref="IProcessDisplayInfo"/>
+///   A strict equality comparison for <see cref="IProcessDisplayInfo"/>, done as some displays must be much more
+///   strict than normal related to the equality of the data.
 /// </summary>
 public class StrictProcessDisplayInfoEquality : IEquatable<StrictProcessDisplayInfoEquality>
 {
@@ -39,6 +40,11 @@ public bool Equals(StrictProcessDisplayInfoEquality? other)
         if (Math.Abs(our.CurrentSpeed - theirs.CurrentSpeed) > MathUtils.EPSILON)
             return false;
 
+        // If process toggle state doesn't match, cannot be equal (needed to properly update process panel state when
+        // enable / disable button is pressed)
+        if (our.Enabled != theirs.Enabled)
+            return false;
+
         if (ReferenceEquals(our.Inputs, null) != ReferenceEquals(theirs.Inputs, null))
             return false;
         if (our.Inputs != null && !our.Inputs.DictionaryEquals(theirs.Inputs!))

src/microbe_stage/TweakedProcess.cs

@@ -6,7 +6,7 @@
 /// </summary>
 /// <remarks>
 ///   <para>
-///     This is a struct as this just packs a few floats and a single object reference in here. This allows much tighter
+///     This is a struct as this just packs a few values and a single object reference in here. This allows much tighter
 ///     data packing when this is used in lists.
 ///   </para>
 /// </remarks>
@@ -19,6 +19,16 @@ public struct TweakedProcess : IEquatable<TweakedProcess>
 
     public float SpeedMultiplier = 1;
 
+    /// <summary>
+    ///   Indicates if this process is marked for a specific operation.
+    /// </summary>
+    /// <remarks>
+    ///   <para>
+    ///     Should be reset to false by any function using this field.
+    ///   </para>
+    /// </remarks>
+    internal bool Marked;
+
     [JsonConstructor]
     public TweakedProcess(BioProcess process, float rate = 1.0f)
     {
@@ -49,7 +59,8 @@ public bool Equals(TweakedProcess other)
         if (Process != other.Process)
             return false;
 
-        // ReSharper disable once CompareOfFloatsByEqualityOperator
+        // This equality check may not be very strict, because otherwise the ProcessPanel breaks! Specifically
+        // ProcessStatistics.GetAndMarkUsed doesn't return the correct entry
         return Rate == other.Rate && ReferenceEquals(Process, other.Process);
     }
 
@@ -69,7 +80,7 @@ public override int GetHashCode()
     public override string ToString()
     {
         if (SpeedMultiplier != 1)
-            return $"{Process} at {Rate}x (mult: #{SpeedMultiplier})";
+            return $"{Process} at {Rate}x (mult: {SpeedMultiplier})";
 
         return $"{Process} at {Rate}x";
     }

src/microbe_stage/components/OrganelleContainer.cs

@@ -347,8 +347,7 @@ public static void RecalculateOrganelleBioProcesses(this ref OrganelleContainer
     {
         if (container.Organelles != null)
         {
-            ProcessSystem.ComputeActiveProcessList(container.Organelles.Organelles,
-                ref bioProcesses.ActiveProcesses);
+            ProcessSystem.ComputeActiveProcessList(container.Organelles.Organelles, ref bioProcesses.ActiveProcesses);
         }
     }