fix sequence node Except() (#2610)

Co-authored-by: metalgearsloth <31366439+metalgearsloth@users.noreply.github.com>

Robust.Shared/Physics/Dynamics/Fixture.cs

@@ -54,9 +54,19 @@ namespace Robust.Shared.Physics.Dynamics
         /// </remarks>
         /// </summary>
         [ViewVariables(VVAccess.ReadWrite)]
-        [DataField("id")]
-        public string ID { get; set; } = string.Empty;
+        public string ID
+        {
+            get => _id ?? AutoGeneratedId;
+            set => _id = value;
+        }
 
+        // Because Ids are optional and will be auto generated, this means every component that doesn't specify an ID
+        // will get its fixture component saved to the map. This is a shitty workaround:
+        public string AutoGeneratedId = string.Empty;
+
+        [DataField("id")]
+        private string? _id;
+        
         [ViewVariables]
         [field: NonSerialized]
         public FixtureProxy[] Proxies { get; set; } = Array.Empty<FixtureProxy>();

Robust.Shared/Physics/FixtureSystem.cs

@@ -55,7 +55,7 @@ namespace Robust.Shared.Physics
             // (probably change this someday for perf reasons?)
             foreach (var fixture in component.SerializedFixtures)
             {
-                fixture.ID = GetFixtureName(component, fixture);
+                EnsureFixtureId(component, fixture);
 
                 if (component.Fixtures.TryAdd(fixture.ID, fixture)) continue;
 
@@ -93,6 +93,21 @@ namespace Robust.Shared.Physics
 
         #region Public
 
+        /// <summary>
+        ///     Attempts to add a new fixture. Will do nothing if a fixture with the requested ID already exists.
+        /// </summary>
+        public bool TryCreateFixture(PhysicsComponent body, Fixture fixture, bool updates = true, FixturesComponent? manager = null, TransformComponent? xform = null)
+        {
+            if (!Resolve(body.Owner, ref manager, ref xform, false))
+                return false;
+
+            if (!string.IsNullOrEmpty(fixture.ID) && manager.Fixtures.ContainsKey(fixture.ID))
+                return false;
+
+            CreateFixture(body, fixture, updates, manager, xform);
+            return true;
+        }
+
         public void CreateFixture(PhysicsComponent body, Fixture fixture, bool updates = true, FixturesComponent? manager = null, TransformComponent? xform = null)
         {
             if (!Resolve(body.Owner, ref manager, ref xform))
@@ -101,7 +116,7 @@ namespace Robust.Shared.Physics
                 return;
             }
 
-            fixture.ID = GetFixtureName(manager, fixture);
+            EnsureFixtureId(manager, fixture);
             manager.Fixtures.Add(fixture.ID, fixture);
             fixture.Body = body;
 
@@ -120,7 +135,7 @@ namespace Robust.Shared.Physics
             {
                 FixtureUpdate(manager, body);
                 body.ResetMassData();
-                manager.Dirty();
+                Dirty(manager);
             }
             // TODO: Set newcontacts to true.
         }
@@ -350,7 +365,10 @@ namespace Robust.Shared.Physics
             }
         }
 
-        private string GetFixtureName(FixturesComponent component, Fixture fixture)
+        /// <summary>
+        ///     Return the fixture's id if it has one. Otherwise, this automatically generates a fixture id and stores it.
+        /// </summary>
+        private string EnsureFixtureId(FixturesComponent component, Fixture fixture)
         {
             if (!string.IsNullOrEmpty(fixture.ID)) return fixture.ID;
 
@@ -364,6 +382,7 @@ namespace Robust.Shared.Physics
 
                 if (!found)
                 {
+                    fixture.AutoGeneratedId = name;
                     return name;
                 }
             }

Robust.Shared/Serialization/Markdown/DataNode.cs

@@ -1,4 +1,5 @@
 using Robust.Shared.Serialization.Manager;
+using System;
 
 namespace Robust.Shared.Serialization.Markdown
 {
@@ -18,6 +19,16 @@ namespace Robust.Shared.Serialization.Markdown
 
         public abstract DataNode? Except(DataNode node);
 
+        public override bool Equals(object? obj)
+        {
+            if (obj is not DataNode other)
+                return false;
+
+            // mapping and sequences nodes are equal if removing duplicate entires leaves us with nothing. Value nodes
+            // override this and directly check equality.
+            return Except(other) == null;
+        }
+
         public T CopyCast<T>() where T : DataNode
         {
             return (T) Copy();

Robust.Shared/Serialization/Markdown/Sequence/SequenceDataNode.cs

@@ -112,11 +112,10 @@ namespace Robust.Shared.Serialization.Markdown.Sequence
 
         public override SequenceDataNode? Except(SequenceDataNode node)
         {
-            var set = new HashSet<DataNode>(node._nodes);
             var newList = new List<DataNode>();
-            foreach (var nodeNode in node._nodes)
+            foreach (var nodeNode in _nodes)
             {
-                if (!set.Contains(nodeNode)) newList.Add(nodeNode);
+                if (!node._nodes.Contains(nodeNode)) newList.Add(nodeNode);
             }
 
             if (newList.Count > 0)