Engine - ECS: perform cycle assertion at method begin

friflo · Jul 21, 2024 · fa97d9f · fa97d9f
1 parent ef88805
commit fa97d9f
Showing 1 changed file with 14 additions and 21 deletions.
diff --git a/Engine/src/ECS/Entity/Store/NodeTree.cs b/Engine/src/ECS/Entity/Store/NodeTree.cs
@@ -100,8 +100,11 @@ internal TreeMembership  GetTreeMembership(int id)
         }
     }
 
-    private bool IsChildOf(int id, int parentId)
+    private bool WouldCreateCycle(int id, int parentId)
     {
+        if (id == parentId) {
+            return true;
+        }
         while (true) {
             var parent = GetTreeParentId(id);
             if (parent == Static.NoParentId) {
@@ -117,6 +120,9 @@ private bool IsChildOf(int id, int parentId)
 
     internal int AddChild (int parentId, int childId)
     {
+        if (WouldCreateCycle(parentId, childId)) {
+            throw OperationCycleException(parentId, childId);
+        }
         var curParentId = GetTreeParentId(childId);
         if (HasParent(curParentId)) {
             if (curParentId == parentId) {
@@ -127,14 +133,6 @@ internal int AddChild (int parentId, int childId)
         //  int curIndex = RemoveChildNode(ref localNodes[curParentId], curParentId, childId)
             int curIndex = RemoveChildNode(curParentId, childId);
             OnChildNodeRemove(curParentId, childId, curIndex);
-        } else {
-            if (parentId == childId) {
-                // case: tried to add entity to itself as a child
-                throw AddEntityAsChildToItselfException(parentId);
-            }
-        }
-        if (IsChildOf(parentId, childId)) {
-            throw OperationCycleException(parentId, childId);
         }
         // --- add entity with given id as child to this entity
         var parentEntity    = new Entity(this, parentId);
@@ -151,18 +149,14 @@ internal int AddChild (int parentId, int childId)
 
     internal void InsertChild (int parentId, int childId, int childIndex)
     {
+        if (WouldCreateCycle(parentId, childId)) {
+            throw OperationCycleException(parentId, childId);
+        }
         var parentEntity    = new Entity(this, parentId);
         ref var parent      = ref GetTreeNodeRef(parentEntity);
-
         if (childIndex > parent.childIds.count) {
             throw new IndexOutOfRangeException();
         }
-        if (parentId == childId) {
-            throw AddEntityAsChildToItselfException(parentId);
-        }
-        if (IsChildOf(parentId, childId)) {
-            throw OperationCycleException(parentId, childId);
-        }
         var curParentId = GetTreeParentId(childId);
         if (HasParent(curParentId))
         {
@@ -423,11 +417,10 @@ private bool HasCycle(int id, int childId, out InvalidOperationException excepti
         }
     }
 
-    private static InvalidOperationException AddEntityAsChildToItselfException(int id) {
-        return new InvalidOperationException($"operation would cause a cycle: {id} -> {id}");
-    }
-
     private InvalidOperationException OperationCycleException(int id, int other) {
+        if (id == other) {
+            return new InvalidOperationException($"operation would cause a cycle: {id} -> {id}");
+        }
         return CycleException("operation would cause a cycle: ", id, other);
     }
 
@@ -439,7 +432,7 @@ private InvalidOperationException CycleException(string message, int id, int oth
         sb.Append(id);
         while (true)
         {
-            cur = GetTreeParentId(cur);
+        cur = GetTreeParentId(cur);
             sb.Append(" -> ");
             sb.Append(cur);
             if (cur != other) {