From a404b7dfd3baca309a9149fc506f4c5786a48e10 Mon Sep 17 00:00:00 2001 From: gotmachine <24925209+gotmachine@users.noreply.github.com> Date: Sat, 6 May 2023 21:25:48 +0200 Subject: [PATCH] Potential new KSP bugfix : ReRootPreserveSurfaceAttach (#142) * New bugfix patch : ReRootPreserveSurfaceAttach * ReRootPreserveSurfaceAttach : prepare for release --- GameData/KSPCommunityFixes/Settings.cfg | 5 +- .../BugFixes/ReRootCloneSurfaceAttach.cs | 65 +++++++++++++++++++ .../BugFixes/ReRootPreserveSurfaceAttach.cs | 56 ++++------------ KSPCommunityFixes/KSPCommunityFixes.csproj | 1 + 4 files changed, 82 insertions(+), 45 deletions(-) create mode 100644 KSPCommunityFixes/BugFixes/ReRootCloneSurfaceAttach.cs diff --git a/GameData/KSPCommunityFixes/Settings.cfg b/GameData/KSPCommunityFixes/Settings.cfg index e6d4d44..3fff32c 100644 --- a/GameData/KSPCommunityFixes/Settings.cfg +++ b/GameData/KSPCommunityFixes/Settings.cfg @@ -174,7 +174,10 @@ KSP_COMMUNITY_FIXES // Disable the stock behavior of altering surface attachment nodes on re-rooting, which is // unnecessary and doesn't work correctly, leading to permanently borked attachement nodes. - ReRootPreserveSurfaceAttach = true + // Replaces ReRootPreserveSurfaceAttach with some better (but more complex) logic. + // If this patch causes an issue, try turning it off and turning on ReRootPreserveSurfaceAttach + ReRootCloneSurfaceAttach = true + ReRootPreserveSurfaceAttach = false // Fix leaking a camera and spotlight created by the thumbnail system on certain failures ThumbnailSpotlight = true diff --git a/KSPCommunityFixes/BugFixes/ReRootCloneSurfaceAttach.cs b/KSPCommunityFixes/BugFixes/ReRootCloneSurfaceAttach.cs new file mode 100644 index 0000000..4866adf --- /dev/null +++ b/KSPCommunityFixes/BugFixes/ReRootCloneSurfaceAttach.cs @@ -0,0 +1,65 @@ +using HarmonyLib; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using UnityEngine; + +namespace KSPCommunityFixes.BugFixes +{ + class ReRootCloneSurfaceAttach : BasePatch + { + protected override void ApplyPatches(List patches) + { + patches.Add(new PatchInfo( + PatchMethodType.Prefix, + AccessTools.Method(typeof(AttachNode), nameof(AttachNode.ReverseSrfNodeDirection)), + this)); + + patches.Add(new PatchInfo( + PatchMethodType.Prefix, + AccessTools.Method(typeof(AttachNode), nameof(AttachNode.ChangeSrfNodePosition)), + this)); + } + + // In stock, this function is called after reversing a surface attachment during a re-root operation. + // it tries to alter a part's surface attachment so that it mirrors the surface attach node of its parent. + // But that's not a great idea, because a lot of things depend on the surface attach node never changing. + // For example, if the user then picks the part back up, it won't attach the same way to anything else + // To fix this, instead of using the child's actual srfAttachNode we create a new surface attach node and + // just stick it in the regular AttachNode list. + static bool AttachNode_ReverseSrfNodeDirection_Prefix(AttachNode __instance, AttachNode fromNode) + { + // note that instead of cloning the child's srfAttachNode and using its properties, we use the fromNode + // because we want to mirror the previous state as much as possible - this node WAS the other part's srfAttachNode + AttachNode newSrfAttachNode = AttachNode.Clone(fromNode); + newSrfAttachNode.owner = __instance.owner; + newSrfAttachNode.attachedPart = fromNode.owner; + // the name here isn't important, but if anyone is debugging an issue I'd like to make it clear where it came from. + // I'm pretty sure the empty string has some special meaning, and we need to be sure it doesn't collide with any existing node IDs + newSrfAttachNode.id = "KSPCF-reroot-srfAttachNode"; + + // convert the position, orientation from the other part's local space into ours + Vector3 positionWorld = fromNode.owner.transform.TransformPoint(fromNode.position); + Vector3 orientationWorld = fromNode.owner.transform.TransformDirection(fromNode.orientation); + newSrfAttachNode.originalPosition = newSrfAttachNode.owner.transform.InverseTransformPoint(positionWorld); + newSrfAttachNode.originalOrientation = -newSrfAttachNode.owner.transform.InverseTransformDirection(orientationWorld); + newSrfAttachNode.position = newSrfAttachNode.originalPosition; + newSrfAttachNode.orientation = newSrfAttachNode.originalOrientation; + newSrfAttachNode.owner.attachNodes.Add(newSrfAttachNode); + + // now clear the srfAttachNodes from both parts + __instance.attachedPart = null; + fromNode.attachedPart = null; + + return false; + } + + // this function is just horribly broken and no one could call it, ever + static bool AttachNode_ChangeSrfNodePosition_Prefix() + { + return false; + } + } +} diff --git a/KSPCommunityFixes/BugFixes/ReRootPreserveSurfaceAttach.cs b/KSPCommunityFixes/BugFixes/ReRootPreserveSurfaceAttach.cs index e9aa1f6..78dd04b 100644 --- a/KSPCommunityFixes/BugFixes/ReRootPreserveSurfaceAttach.cs +++ b/KSPCommunityFixes/BugFixes/ReRootPreserveSurfaceAttach.cs @@ -6,8 +6,6 @@ using System.Collections.Generic; using System.Reflection; using System.Reflection.Emit; -using UnityEngine.UIElements; -using UnityEngine; #if REROOT_DEBUG_MODULE using UnityEngine; @@ -20,53 +18,23 @@ class ReRootPreserveSurfaceAttach : BasePatch protected override void ApplyPatches(List patches) { patches.Add(new PatchInfo( - PatchMethodType.Prefix, - AccessTools.Method(typeof(AttachNode), nameof(AttachNode.ReverseSrfNodeDirection)), - this)); - - patches.Add(new PatchInfo( - PatchMethodType.Prefix, - AccessTools.Method(typeof(AttachNode), nameof(AttachNode.ChangeSrfNodePosition)), + PatchMethodType.Transpiler, + AccessTools.Method(typeof(Part), nameof(Part.SetHierarchyRoot)), this)); } - // In stock, this function is called after reversing a surface attachment during a re-root operation. - // it tries to alter a part's surface attachment so that it mirrors the surface attach node of its parent. - // But that's not a great idea, because a lot of things depend on the surface attach node never changing. - // For example, if the user then picks the part back up, it won't attach the same way to anything else - // To fix this, instead of using the child's actual srfAttachNode we create a new surface attach node and - // just stick it in the regular AttachNode list. - static bool AttachNode_ReverseSrfNodeDirection_Prefix(AttachNode __instance, AttachNode fromNode) + // skip the portion of that method that alter surface nodes position/orientation on re-rooting, + // by returning after the recursive SetHierarchyRoot() call. + private static IEnumerable Part_SetHierarchyRoot_Transpiler(IEnumerable instructions) { - // note that instead of cloning the child's srfAttachNode and using its properties, we use the fromNode - // because we want to mirror the previous state as much as possible - this node WAS the other part's srfAttachNode - AttachNode newSrfAttachNode = AttachNode.Clone(fromNode); - newSrfAttachNode.owner = __instance.owner; - newSrfAttachNode.attachedPart = fromNode.owner; - // the name here isn't important, but if anyone is debugging an issue I'd like to make it clear where it came from. - // I'm pretty sure the empty string has some special meaning, and we need to be sure it doesn't collide with any existing node IDs - newSrfAttachNode.id = "KSPCF-reroot-srfAttachNode"; - - // convert the position, orientation from the other part's local space into ours - Vector3 positionWorld = fromNode.owner.transform.TransformPoint(fromNode.position); - Vector3 orientationWorld = fromNode.owner.transform.TransformDirection(fromNode.orientation); - newSrfAttachNode.originalPosition = newSrfAttachNode.owner.transform.InverseTransformPoint(positionWorld); - newSrfAttachNode.originalOrientation = -newSrfAttachNode.owner.transform.InverseTransformDirection(orientationWorld); - newSrfAttachNode.position = newSrfAttachNode.originalPosition; - newSrfAttachNode.orientation = newSrfAttachNode.originalOrientation; - newSrfAttachNode.owner.attachNodes.Add(newSrfAttachNode); - - // now clear the srfAttachNodes from both parts - __instance.attachedPart = null; - fromNode.attachedPart = null; - - return false; - } + MethodInfo m_Part_SetHierarchyRoot = AccessTools.Method(typeof(Part), nameof(Part.SetHierarchyRoot)); - // this function is just horribly broken and no one could call it, ever - static bool AttachNode_ChangeSrfNodePosition_Prefix() - { - return false; + foreach (CodeInstruction instruction in instructions) + { + yield return instruction; + if (instruction.opcode == OpCodes.Callvirt && ReferenceEquals(instruction.operand, m_Part_SetHierarchyRoot)) + yield return new CodeInstruction(OpCodes.Ret); + } } } diff --git a/KSPCommunityFixes/KSPCommunityFixes.csproj b/KSPCommunityFixes/KSPCommunityFixes.csproj index 0b27596..3033c93 100644 --- a/KSPCommunityFixes/KSPCommunityFixes.csproj +++ b/KSPCommunityFixes/KSPCommunityFixes.csproj @@ -112,6 +112,7 @@ +