Fixed several issues with the implementation

Author: xfractalino

src/auto-evo/RunResults.cs

@@ -440,6 +440,9 @@ public void ApplyResults(GameWorld world, bool skipMutations)
 
             if (!skipMutations && entry.Value.MutatedProperties != null)
             {
+                if (entry.Key is MicrobeSpecies microbe)
+                    microbe.Organelles.Approve();
+
                 entry.Key.ApplyMutation(entry.Value.MutatedProperties);
             }
 

src/auto-evo/mutation_strategy/MoveOrganelleBack.cs

@@ -28,18 +28,20 @@ public MoveOrganelleBack(Func<OrganelleDefinition, bool> criteria)
         var workMemory2 = new List<Hex>();
         var workMemory3 = new HashSet<Hex>();
 
-        foreach (OrganelleTemplate organelle in baseSpecies.Organelles.Where(x => allOrganelles.Contains(x.Definition)))
+        int organelleCount = baseSpecies.Organelles.Count;
+        for (int i = 0; i < organelleCount; i++)
         {
-            MicrobeSpecies newSpecies = (MicrobeSpecies)baseSpecies.Clone();
+            var organelle = baseSpecies.Organelles[i];
+
+            if (!allOrganelles.Contains(organelle.Definition))
+                continue;
 
+            MicrobeSpecies newSpecies = (MicrobeSpecies)baseSpecies.Clone();
             newSpecies.Organelles.Remove(organelle);
 
             if (CommonMutationFunctions.AddOrganelle(organelle.Definition, CommonMutationFunctions.Direction.Rear,
                     newSpecies, workMemory1, workMemory2, workMemory3, random))
             {
-                // Add mutation attempt only if was able to place the organelle
-                // TODO: maybe this should add the attempt anyway as this may act as a separate remove organelle step
-                // for things that cannot be moved?
                 mutated.Add(Tuple.Create(newSpecies, mp - Constants.ORGANELLE_MOVE_COST));
             }
         }

src/auto-evo/steps/ModifyExistingSpecies.cs

@@ -169,7 +169,7 @@ public bool RunStep(RunResults results)
                     bool isEqual = m.ParentSpecies == m.MutatedSpecies;
 
                     if (isEqual)
-                        m.MutatedSpecies.Organelles.Approve(false);
+                        m.MutatedSpecies.Organelles.Reject();
 
                     return isEqual;
                 });
@@ -180,7 +180,7 @@ public bool RunStep(RunResults results)
                 while (mutationsToTry.Count > TotalMutationsToTry)
                 {
                     var toRemove = mutationsToTry[^1];
-                    toRemove.MutatedSpecies.Organelles.Approve(false);
+                    toRemove.MutatedSpecies.Organelles.Reject();
 
                     mutationsToTry.RemoveAt(mutationsToTry.Count - 1);
                 }
@@ -241,10 +241,12 @@ public bool RunStep(RunResults results)
                             new KeyValuePair<Patch, long>(patch, newPopulation), mutation.AddType,
                             mutation.ParentSpecies);
 
-                        mutation.MutatedSpecies.Organelles.Approve(true);
+                        mutation.MutatedSpecies.Organelles.Approve();
+                    }
+                    else
+                    {
+                        mutation.MutatedSpecies.Organelles.Reject();
                     }
-
-                    mutation.MutatedSpecies.Organelles.Approve(false);
                 }
 
                 return true;

src/general/HexLayout.cs

@@ -42,7 +42,7 @@ public HexLayout()
 
     public HexLayout(List<T> hexes)
     {
-        existingHexes = new HexLayoutView(hexes, true, false);
+        existingHexes = new HexLayoutView(hexes, true, true);
     }
 
     /// <summary>
@@ -325,18 +325,16 @@ public bool Contains(T item)
         return existingHexes.Contains(item);
     }
 
-    public void Approve(bool isApproved = true)
+    public void Approve()
     {
-        if (isApproved)
-        {
-            existingHexes.Commit(false);
-        }
-        else
-        {
-            // This species will probably be discarded by auto-evo in this situation, so this returns the allocated
-            // memory to the pool.
-            existingHexes.Drop();
-        }
+        existingHexes.Commit(false);
+    }
+
+    public void Reject()
+    {
+        // This species will probably be discarded by auto-evo in this situation, so this returns the allocated
+        // memory to the pool.
+        existingHexes.Drop();
     }
 
     // TODO: remove this bit of boxing here. https://nede.dev/blog/preventing-unnecessary-allocation-in-net-collections
@@ -488,12 +486,11 @@ protected struct HexLayoutView : IReadOnlyList<T>
         private const int MEMORY_ALLOC_SIZE = 16;
 
         private T?[]? diffHexes;
-        private int diffIndex;
+        private int diffIndex = 0;
 
         public HexLayoutView(List<T> parent, bool shared, bool initDiffHexes = false)
         {
             MainHexes = parent;
-
             Shared = shared;
 
             if (initDiffHexes)
@@ -516,8 +513,7 @@ public T this[int index]
                     {
                         var item = diffHexes[pendingIndex];
 
-                        return item is null ? throw new ArgumentOutOfRangeException(nameof(index), "Out of range.") :
-                            diffHexes[pendingIndex]!;
+                        return item ?? throw new ArgumentOutOfRangeException(nameof(index), "Out of range.");
                     }
                 }
 
@@ -550,19 +546,15 @@ public void Add(T item)
             if (diffHexes == null)
             {
                 // No diff hexes, so we add directly to the layout.
-                if (Shared)
-                {
-                    // This is a shared hex layout. Since we're diverging from it, we need to allocate a brand-new list.
 
-                    MainHexes = [];
-                    Shared = false;
-                }
+                // This is a shared hex layout. Since we're diverging from it, we need to allocate a brand-new list.
+                IsolateIfShared();
 
                 MainHexes.Add(item);
                 return;
             }
 
-            if (diffIndex == diffHexes.Length - 1)
+            if (diffIndex == diffHexes.Length)
             {
                 ArrayPool<T?>.Shared.Resize(ref diffHexes, diffHexes.Length + MEMORY_ALLOC_SIZE);
             }
@@ -572,53 +564,40 @@ public void Add(T item)
 
         public void Remove(T item)
         {
-            if (diffHexes == null)
+            if (diffHexes != null)
             {
-                if (Shared)
+                for (int i = 0; i < diffIndex; ++i)
                 {
-                    MainHexes = [];
-                    Shared = false;
-                }
+                    if (!ReferenceEquals(item, diffHexes[i]))
+                        continue;
 
-                MainHexes.Remove(item);
-                return;
+                    diffHexes[i] = diffHexes[--diffIndex];
+                    diffHexes[diffIndex] = null;
+                    return;
+                }
             }
 
-            for (int i = 0; i < diffIndex; ++i)
-            {
-                var toTest = diffHexes[i];
+            IsolateIfShared();
 
-                if (!ReferenceEquals(item, toTest))
-                    continue;
-
-                diffHexes[i] = diffHexes[--diffIndex];
-                diffHexes[diffIndex] = null;
-
-                break;
-            }
+            MainHexes.Remove(item);
         }
 
         /// <summary>
         ///   This method clears the diff layout and loads the modifications into the permanent layout.
         /// </summary>
         /// <remarks>
-        /// <para>
-        ///   This is the method that allocates memory. Only when this is called and the diff is not empty the
-        ///   organelles get deep cloned.
-        /// </para>
-        /// </remarks>>
-        /// <returns>true if this layout was in diff mode.</returns>>
+        ///   <para>
+        ///     This is the method that allocates memory. Only when this is called and the diff is not empty the
+        ///     organelles get deep cloned.
+        ///   </para>
+        /// </remarks>
+        /// <returns>true if this layout was in diff mode.</returns>
         public bool Commit(bool reallocate)
         {
             if (diffHexes is null || diffIndex == 0)
                 return false;
 
-            if (Shared)
-            {
-                // Mark this not as shared now and allocate a new list.
-                MainHexes = new List<T>(MainHexes);
-                Shared = false;
-            }
+            IsolateIfShared();
 
             for (int i = 0; i < diffIndex; ++i)
             {
@@ -644,12 +623,32 @@ public bool Commit(bool reallocate)
             return true;
         }
 
+        public void IsolateIfShared()
+        {
+            if (!Shared)
+                return;
+
+            var count = MainHexes.Count;
+            var newList = new List<T>(count);
+
+            for (int i = 0; i < count; i++)
+            {
+                newList.Add((T)((ICloneable)MainHexes[i]).Clone());
+            }
+
+            MainHexes = newList;
+            Shared = false;
+        }
+
         public void Drop()
         {
             if (diffHexes is null)
                 return;
 
             ArrayPool<T?>.Shared.Return(diffHexes, true);
+
+            diffHexes = null;
+            diffIndex = 0;
         }
 
         public IEnumerator<T> GetEnumerator()
@@ -662,7 +661,7 @@ public IEnumerator<T> GetEnumerator()
             if (diffHexes == null)
                 yield break;
 
-            for (int i = 0; i < diffHexes.Length; ++i)
+            for (int i = 0; i < diffIndex; ++i)
             {
                 var item = diffHexes[i];
 

src/microbe_stage/OrganelleLayout.cs

@@ -10,7 +10,7 @@
 /// </summary>
 /// <typeparam name="T">The type of organelle contained in this layout</typeparam>
 public class OrganelleLayout<T> : HexLayout<T>, IArchivable, IReadOnlyOrganelleLayout<T>
-    where T : class, IPositionedOrganelle, ICloneable
+    where T : class, IPositionedOrganelle
 {
     public OrganelleLayout(Action<T> onAdded, Action<T>? onRemoved = null) : base(onAdded, onRemoved)
     {
@@ -156,6 +156,12 @@ public bool RepositionToOrigin()
         if (centerOfMass.Q == 0 && centerOfMass.R == 0)
             return false;
 
+        // The following repositioning is unsafe to share, so let's isolate.
+        // TODO:
+        // Maybe, instead of this, use an offset Hex in the iterator and adjust the positions on-fly instead of
+        // allocating?
+        existingHexes.IsolateIfShared();
+
         foreach (var organelle in Organelles)
         {
             // This calculation aligns the center of mass with the origin by moving every organelle of the microbe.