Potential new KSP bugfix : ReRootPreserveSurfaceAttach

[gotmachine]

Our dear friend Lisias has uncovered a strange behavior on re-rooting crafts.

In KSP 1.8.0, a change was introduced to the Part.SetHierarchyRoot() method. This method is called when the user uses the re-rooting tool in the editor, but also in flight when docking/undocking. It's a recursive method responsible for reshuffling the part hierarchy according to the modified root part.

The implementation was pretty straightforward prior to 1.8.0, but for some reason (the Fix re-rooting of surface attach nodes entry in the KSP changelog doesn't explain much) they added a weird behavior to it altering surface attachment nodes position and orientation. When the re-rooting operation results in an inversion of the parent-child relationship, the intent seems to be to alter the nodes as if the craft was initially built in that reversed order. There are multiple issues with that :

• The implementation of that "fix" just doesn't work. The code flow and logic is questionable, and the result is systematically a new node position/orientation that doesn't make any sense, the geometry transformations just don't make any sense. Moreover, even surface attached nodes that aren't actually used are affected (in a way that doesn't make sense either).
• Supposedly, the intent is to somewhat "mirror" the surface nodes so after a re-root operation resulting in a hierarchy inversion, the user can detach and re-attach surface attached parts without having to deal with the offset resulting from the inversion. There is a valid concern that due to editor offset limits, an user that tries to detach and re-attach a surface attached part after a re-root might not be able to do it because the induced offset is greater than allowed.
• Fundamentally, there is no "fix" to the fact that inverting the hierarchy by re-rooting lead to an additional position and rotation offset between the "visual" attachment point and the config-defined surface attachment. It's an inherent side effect of parts being a hierarchical tree. And while it can be argued that moving the node makes a better user experience, it just hide the inconsistency behind a less obvious one, which is that you end up with parts having inconsistent (and seemingly random from the user PoV) surface attachment points.
• Outside of legit cases where the part shape is dynamic (mesh switching, part scaling, robotics...), there is nothing altering attach nodes in the stock codebase, so this break the assumption that mods can rely on the nodes position/orientation as defined in the part config. In the editor, this results in parts whose surface attachment position and orientation is permanently altered, resulting in a weird discrepancy in how they snap when trying to attach them compared to a freshly instantiated part.
• Why this is also applied in flight is another mystery. It might just be an oversight (which wouldn't be surprising), but attach nodes positions do have an influence over stuff like aerodynamics and thermodynamics, and while I haven't checked, it would make somewhat sense to ensure their position is "logical" after a docking/undocking induced re-root.

So that leaves us with two options :

1. "Fix" the node displacement logic so it actually does what it is (supposedly) trying to do. That would at least prevent surface attachments from becoming total nonsense and would be an improvement in terms of UX, but is questionable from a consistency perspective, and has the potential to cause issues for mods making the assumption that attach nodes are somewhat "immutable".
2. Just disable the whole feature, reverting to pre-1.8 behavior. In practice, this will still be a UX improvement since at least you get consistent nodes instead of borked nodes that don't help with the original UX issue in any way, and this will fix potential issues for mods relying on attach nodes non-mutability.

I'm quite tempted to just push solution 2 (which is what this PR currently does), but I fear I might have missed something about what this whole thing is trying to do, especially because it is also applied in flight. So I'm gonna sleep on it for a while.

Example of the surface attach node being altered even when the part isn't actually surface attached :

2023-04-27.17-50-18-999.mp4

Another example showing how the nodes are moved to nonsensical positions/directions when re-rooting :

2023-04-27.17-54-54-162.mp4