Fixed entity ID mismatches and desync kicks caused by gap creation/removal in Structure.SetDamage. Creating the gaps on damaged walls wasn't guaranteed to happen in the same order client-side as on the server, causing the IDs to get assigned mismatching IDs and in some cases also affecting the IDs of other types of entities (see #528).

Now the structure gaps simply don't have IDs. They're never accessed by ID so I don't think there's the need to make the creation/removal go through entityspawner.

Barotrauma/BarotraumaShared/Source/Map/Entity.cs

@@ -20,6 +20,8 @@ namespace Barotrauma
 
         protected AITarget aiTarget;
 
+        private bool idFreed;
+
         public virtual bool Removed
         {
             get;
@@ -52,7 +54,8 @@ namespace Barotrauma
                     DebugConsole.Log("The id of " + this + " is now " + value);
                 }
 
-                id = value;                             
+                id = value;
+                idFreed = false;
                 dictionary.Add(id, this);
             }
         }
@@ -199,21 +202,24 @@ namespace Barotrauma
             dictionary.Clear();
         }
 
-        public virtual void Remove()
+        /// <summary>
+        /// Removes the entity from the entity dictionary and frees up the ID it was using.
+        /// </summary>
+        public void FreeID()
         {
             DebugConsole.Log("Removing entity " + ToString() + " (" + ID + ") from entity dictionary.");
             if (!dictionary.TryGetValue(ID, out Entity existingEntity))
             {
                 DebugConsole.Log("Entity " + ToString() + " (" + ID + ") not present in entity dictionary.");
                 GameAnalyticsManager.AddErrorEventOnce(
-                    "Entity.Remove:EntityNotFound" + ID,
+                    "Entity.FreeID:EntityNotFound" + ID,
                     GameAnalyticsSDK.Net.EGAErrorSeverity.Error,
                     "Entity " + ToString() + " (" + ID + ") not present in entity dictionary.\n" + Environment.StackTrace);
             }
             else if (existingEntity != this)
             {
                 DebugConsole.Log("Entity ID mismatch in entity dictionary. Entity " + existingEntity + " had the ID " + ID + " (expecting " + ToString() + ")");
-                GameAnalyticsManager.AddErrorEventOnce("Entity.Remove:EntityMismatch" + ID,
+                GameAnalyticsManager.AddErrorEventOnce("Entity.FreeID:EntityMismatch" + ID,
                     GameAnalyticsSDK.Net.EGAErrorSeverity.Error,
                     "Entity ID mismatch in entity dictionary. Entity " + existingEntity + " had the ID " + ID + " (expecting " + ToString() + ")");
 
@@ -224,6 +230,13 @@ namespace Barotrauma
             }
 
             dictionary.Remove(ID);
+            id = 0;
+            idFreed = true;
+        }
+
+        public virtual void Remove()
+        {
+            if (!idFreed) FreeID();
             Removed = true;
         }
 

Barotrauma/BarotraumaShared/Source/Map/Structure.cs

@@ -18,11 +18,7 @@ namespace Barotrauma
         public Rectangle rect;
         public float damage;
         public Gap gap;
-
+        
-        public int GapID;
-
-        //public float lastSentDamage;
-
         public WallSection(Rectangle rect)
         {
             System.Diagnostics.Debug.Assert(rect.Width > 0 && rect.Height > 0);
@@ -687,7 +683,7 @@ namespace Barotrauma
             return new AttackResult(damageAmount, 0.0f);
         }
 
-        private void SetDamage(int sectionIndex, float damage, Character attacker = null)
+        private void SetDamage(int sectionIndex, float damage, Character attacker = null, bool createNetworkEvent = true)
         {
             if (Submarine != null && Submarine.GodMode) return;
             if (!prefab.Body) return;
@@ -695,7 +691,7 @@ namespace Barotrauma
 
             damage = MathHelper.Clamp(damage, 0.0f, prefab.Health);
 
-            if (GameMain.Server != null && damage != sections[sectionIndex].damage)
+            if (GameMain.Server != null && createNetworkEvent && damage != sections[sectionIndex].damage)
             {
                 GameMain.Server.CreateEntityEvent(this);
             }
@@ -720,7 +716,9 @@ namespace Barotrauma
                         GameServer.Log((sections[sectionIndex].gap.IsRoomToRoom ? "Inner" : "Outer") + " wall repaired by " + attacker.Name, ServerLog.MessageType.ItemInteraction);
                     }
 
-                    //remove existing gap if damage is below 50%
+                    DebugConsole.Log("Removing gap (ID " + sections[sectionIndex].gap.ID + ", section: " + sectionIndex + ") from wall " + ID);
+                    //remove existing gap if damage is below leak threshold
+                    sections[sectionIndex].gap.Open = 0.0f;
                     sections[sectionIndex].gap.Remove();
                     sections[sectionIndex].gap = null;
 #if CLIENT
@@ -730,17 +728,21 @@ namespace Barotrauma
             }
             else
             {
-
                 if (sections[sectionIndex].gap == null)
                 {
-
                     Rectangle gapRect = sections[sectionIndex].rect;
                     gapRect.X -= 10;
                     gapRect.Y += 10;
                     gapRect.Width += 20;
                     gapRect.Height += 20;
                     sections[sectionIndex].gap = new Gap(gapRect, !isHorizontal, Submarine);
+                    //free the ID, because if we give gaps IDs we have to make sure they always match between the clients and the server and
+                    //that clients create them in the correct order along with every other entity created/removed during the round
+                    //which COULD be done via entityspawner, but it's unnecessary because we never access these gaps by ID
+                    sections[sectionIndex].gap.FreeID();
+                    sections[sectionIndex].gap.ShouldBeSaved = false;
                     sections[sectionIndex].gap.ConnectedWall = this;
+                    DebugConsole.Log("Created gap (ID " + sections[sectionIndex].gap.ID + ", section: " + sectionIndex + ") on wall " + ID);
                     //AdjustKarma(attacker, 300);
 
                     //the structure didn't have any other gaps yet, log the breach
@@ -856,9 +858,8 @@ namespace Barotrauma
         {
             for (int i = 0; i < sections.Length; i++)
             {
-                float damage = msg.ReadRangedSingle(0.0f, 1.0f, 8) * Health;
+                float damage = msg.ReadRangedSingle(0.0f, 1.0f, 8) * Health;                
-
+                SetDamage(i, damage);                
-                SetDamage(i, damage);
             }
         }
         public override void FlipX()
@@ -900,10 +901,12 @@ namespace Barotrauma
             }
 
             Rectangle rect = element.GetAttributeRect("rect", Rectangle.Empty);
-            Structure s = new Structure(rect, prefab, submarine);
+            Structure s = new Structure(rect, prefab, submarine)
-            s.Submarine = submarine;
+            {
-            s.ID = (ushort)int.Parse(element.Attribute("ID").Value);
+                Submarine = submarine,
-            
+                ID = (ushort)int.Parse(element.Attribute("ID").Value)
+            };
+
             foreach (XElement subElement in element.Elements())
             {
                 switch (subElement.Name.ToString())
@@ -911,12 +914,7 @@ namespace Barotrauma
                     case "section":
                         int index = subElement.GetAttributeInt("i", -1);
                         if (index == -1) continue;
-
+                        s.sections[index].damage = subElement.GetAttributeFloat("damage", 0.0f);
-                        s.sections[index].damage = 
-                            subElement.GetAttributeFloat("damage", 0.0f);
-
-                        s.sections[index].GapID = subElement.GetAttributeInt("gap", -1);
-
                         break;
                 }
             }
@@ -938,17 +936,10 @@ namespace Barotrauma
             for (int i = 0; i < sections.Length; i++)
             {
                 if (sections[i].damage == 0.0f) continue;
-
                 var sectionElement =
                     new XElement("section",
                         new XAttribute("i", i),
                         new XAttribute("damage", sections[i].damage));
-
-                if (sections[i].gap != null)
-                {
-                    sectionElement.Add(new XAttribute("gap", sections[i].gap.ID));
-                }
-
                 element.Add(sectionElement);
             }
 
@@ -961,14 +952,10 @@ namespace Barotrauma
 
         public override void OnMapLoaded()
         {
-            foreach (WallSection s in sections)
+            for (int i = 0; i < sections.Length; i++)
             {
-                if (s.GapID == -1) continue;
+                SetDamage(i, sections[i].damage, createNetworkEvent: false);
-
-                s.gap = FindEntityByID((ushort)s.GapID) as Gap;
-                if (s.gap != null) s.gap.ConnectedWall = this;
             }
         }
-        
     }
 }

Barotrauma/BarotraumaShared/Source/Map/Submarine.cs

@@ -322,7 +322,7 @@ namespace Barotrauma
             DockedTo = new List<Submarine>();
 
             ID = ushort.MaxValue;
-            base.Remove();
+            FreeID();
         }
 
         public bool HasTag(SubmarineTag tag)