comments and thoughts about component merge order

src/main/java/baritone/builder/DependencyGraphScaffoldingOverlay.java

@@ -222,6 +222,7 @@ public class DependencyGraphScaffoldingOverlay {
             }
             components.remove(child.id);
             child.deleted = true;
+            child.deletedInto = parent.id;
             //System.out.println("Debug child contains: " + child.positions.contains(963549069314L) + " " + parent.positions.contains(963549069314L));
             return parent;
         }
@@ -320,6 +321,7 @@ public class DependencyGraphScaffoldingOverlay {
             // if i change ^^ that "Set" to "ObjectOpenHashSet" it actually makes the bench about 15% SLOWER?!?!?
             private int y = -1;
             private boolean deleted;
+            private int deletedInto;
             private final Set<CollapsedDependencyGraphComponent> unmodifiableOutgoing = Collections.unmodifiableSet(outgoingEdges);
             private final Set<CollapsedDependencyGraphComponent> unmodifiableIncoming = Collections.unmodifiableSet(incomingEdges);
 
@@ -350,6 +352,13 @@ public class DependencyGraphScaffoldingOverlay {
                 return deleted;
             }
 
+            public int deletedIntoRecursive() { // what cid was this merged into that caused it to be deleted
+                if (!deleted) {
+                    return id;
+                }
+                return components.get(deletedInto).deletedIntoRecursive();
+            }
+
             public LongSet getPositions() {
                 return LongSets.unmodifiable(positions);
             }

src/main/java/baritone/builder/DijkstraScaffolder.java

@@ -44,7 +44,7 @@ public enum DijkstraScaffolder implements IScaffolderStrategy {
             ScaffoldingSearchNode node = openSet.poll();
             CollapsedDependencyGraphComponent tentativeComponent = overlayGraph.getCollapsedGraph().getComponentLocations().get(node.pos);
             if (tentativeComponent != null) {
-                if (exclusiveDescendents.contains(tentativeComponent)) { // TODO is exclusiveDescendants even valid? returning a route into one of the descendants, if it's on the top of the heap, is valid because it closes a loop and the next dijkstra can start from there? perhaps there's no need to treat descendant interactions differently from any other non-root component?
+                if (exclusiveDescendents.contains(tentativeComponent)) { // TODO is exclusiveDescendants even valid? returning a route into one of the descendants, if it's on the top of the heap, is valid because it closes a loop and the next dijkstra can start from there? perhaps there's no need to treat descendant interactions differently from any other non-root component? EDIT: maybe it's good to prevent adding useless scaffolding that closes loops for no good reason?
                     // have gone back onto a descendent of this node
                     // sadly this can happen even at the same Y level even in Y_STRICT mode due to orientable blocks forming a loop
                     continue; // TODO does this need to be here? can I expand THROUGH an unrelated component? probably requires testing, this is quite a mind bending possibility

src/main/java/baritone/builder/Scaffolder.java

@@ -112,8 +112,26 @@ public class Scaffolder {
                 rootComponents.add(components.get(i));
             }
         }
-        // this works because as we add new components and connect them up, we can say that
-        rootComponents.removeIf(CollapsedDependencyGraphComponent::deleted);
+        // why is this valid?
+        // for this to be valid, we need to be confident that no component from cid 0 to old lastcid could have had incomings become empty
+        // consider the case root -> descendant
+        // what if scaffolding created descendant -> root, then they were merged together, but descendant won?
+        // then, descendant would have cid less than last cid, and it wouldn't be added to rootComponents by the previous line perhaps?
+        // but, dijkstra strategy skips merging roots with their descendants intentionally since it's useless to do so
+        rootComponents.removeIf(root -> {
+            if (root.deleted()) {
+                if (root.deletedIntoRecursive() <= cid) {
+                    throw new IllegalStateException(); // sanity check the above - if this throws, i suspect it would mean that a root component was merged into one of its descendants by useless scaffolding
+                    // if this ends up being unavoidable, then iterating over all deletedIntoRecursive of rootComponents should find all new rootComponents
+                    // this is because all new scaffoldings have their own component, so the only way for an old component to have no incomings is if it was merged "the wrong way" with the root, which is easily locatable by deletedIntoRecursive
+                }
+                return true;
+            }
+            return false;
+        });
+        /*rootComponents.clear();
+        rootComponents.addAll(calcRoots());*/
+
         if (Main.DEBUG) {
             if (!rootComponents.equals(calcRoots())) {
                 throw new IllegalStateException();