net-lisias-ksp / KSP-Recall

Recall for KSP blunders, screw ups and borks.
GNU General Public License v2.0
25 stars 2 forks source link

**Correctly** handle KSP 1.9 (and later) borking while loading Crafts on Editor with scaled Variants #32

Closed Lisias closed 2 years ago

Lisias commented 2 years ago

Easting your own dog-food is the ultimate QAS tool. :)

That's the thing: the code that closed net-lisias-ksp/TweakScale#219 reopened net-lisias-ksp/TweakScale#129. But with a catch: only on editing mode.

So, we have three different situations on KSP's guts that affects the nodes of a variant part:

1) OnCopy (#219) 2) While Editing (#139) 3) OnLoad (detected while troubleshooting net-lisias-ksp/TweakScale#219).


EDIT: WRONG. The problem is happening only on Editor, and on PartModule.OnStart.


Apparently the code that fixed net-lisias-ksp/TweakScale#219 is related to OnLoad too. But it regressed net-lisias-ksp/TweakScale#139.


EDIT: That code fixed net-lisias-ksp/TweakScale#219 by pure accident. The regression on net-lisias-ksp/TweakScale#139 happened due colateral effects induced by net-lisias-ksp/TweakScale#219 that fixed a problem that happens only on Editor, and then screw up things on Flight.


Problem: if I put that code back to fix net-lisias-ksp/TweakScale#139, I screw up OnCopy and OnLoad. Clearly, there's something inside KSP that it's out of control, suggesting that Fellow Kerbonaut darthgently was not wrong on suggesting that the problem was KSP and that I didn't, in fact screwed up.

Lisias commented 2 years ago

The problem appears to be OnLoad and OnCopy triggering the event onEditorVariantApplied (what's nuts, it's my understanding that this event should be called only when the User selected a new Variant) - but OnLoad and OnCopy already have the nodes on their places, and so TweakScale's code that handles the user changing Variants ends up borking the job.

This apparently explains why the net-lisias-ksp/TweakScale#219 bug affected loading crafts on Editor, but not on the Flight Scene (something that scared the sheet out of me at that time!).

Lisias commented 2 years ago

CRAP, CRAP, CRAP, CRAP How I could be so naive?

While debugging the damned thing, after implementing the hack to suppress the GameEvents.onEditorVariantApplied, I noticed that it was not making no effect at all - not to mention the problem not being present!

I took some good hours, confused, trying to figure out what was happening when I suddenly realised I was running KSP 1.8.1 . I do tests on many different KSP versions, to the point sometimes I forget what KSP I'm running at one moment.

So I did what I should had done since the Day Zero: I did a regression test on every KSP available on my machine. And this is what I found:

So at least from KSP 1.9.1 (probably 1.9.0, but I don't have it installed anymore), something changed on Editor that started to screw me up. What's not a surprise, KSP-Recall has a fix for a problem on the Attachment Nodes (the same way it does for Resources), but I ended up not using it because at that time I thought that TweakScale would handle this better. Famous last words. :P

SO, yeah - definitivelly this is a badly handled KSP bug, and not a TweakScale one!!!

Lisias commented 2 years ago

Setting it as Not my Fault, because TweakScale *is doing everything right. It's KSP since 1.9 that it's screwing up things.

Lisias commented 2 years ago

There's a good chance that I may handle this on KSP-Recall, after all. Because this is not screwing up only TweakScale, this is screwing up any PartModule that changes the Attachment Nodes, and I got reports for some 3rd parties add'ons that appears to be similar in effect to what's happening here.

Lisias commented 2 years ago

As I explained on Forum, this is a KSP problem. It's that old screwup from KSP 1.9.0 again, but this time I correctly diagnosed the problem.

The crap is happening on OnStart, not on OnLoad or OnCopy as it happened once with Resources:

[LOG 11:04:46.765] [TweakScale] TRACE: Called by    at TweakScale.Log.stackdump(System.String msg, System.Object[] params)
   at TweakScale.PartDB.GameEventEditorVariantAppliedListener.isSpuriousCall()
   at TweakScale.PartDB.GameEventEditorVariantAppliedListener.EditorVariantAppliedHandler(.Part part, .PartVariant partVariant)
   at EventData`2[[Part, Assembly-CSharp, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null],[PartVariant, Assembly-CSharp, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null]].Fire(.Part data0, .PartVariant data1)
   at ModulePartVariants.ApplyVariant(.Part part, UnityEngine.Transform meshRoot, .PartVariant variant, UnityEngine.Material[] materials, Boolean skipShader)
   at ModulePartVariants.RefreshVariant()
   at ModulePartVariants.OnStart(StartState state)
   at Part.ModulesOnStart()
   at Part+<Start>c__Iterator0.MoveNext()
   at UnityEngine.SetupCoroutine.InvokeMoveNext(IEnumerator enumerator, IntPtr returnValueAddress)

For the sake of completude this is what happens when change the variant using the PAW:

[LOG 15:52:11.242] [TweakScale] DETAIL: isSpuriousCall
[LOG 15:52:11.249] [TweakScale] TRACE: Called by   at TweakScale.Log.stackdump (System.String msg, System.Object[] params) [0x00000] in <5128b97299ae4d6db38986bd3a7846c3>:0
  at TweakScale.PartDB.GameEventEditorVariantAppliedListener.isSpuriousCall () [0x00000] in 42e2243022f74a25b18528b4bfa5d1f2>:0
  at TweakScale.PartDB.GameEventEditorVariantAppliedListener.EditorVariantAppliedHandler (Part part, PartVariant partVariant) [0x00000] in <42e2243022f74a25b18528b4bfa5d1f2>:0
  at EventData`2[T,U].Fire (T data0, U data1) [0x00000] in <48dcb08e2e1542e2af1286b02d2eb072>:0
  at ModulePartVariants.ApplyVariant (Part part, UnityEngine.Transform meshRoot, PartVariant variant, UnityEngine.Material[] materials, System.Boolean skipShader) [0x00000] in <48dcb08e2e1542e2af1286b02d2eb072>:0
  at ModulePartVariants.RefreshVariant () [0x00000] in <48dcb08e2e1542e2af1286b02d2eb072>:0
  at ModulePartVariants.SetVariant (System.String variantName) [0x00000] in <48dcb08e2e1542e2af1286b02d2eb072>:0
  at EditorLogic.SpawnPart (AvailablePart partInfo) [0x00000] in <48dcb08e2e1542e2af1286b02d2eb072>:0
  at EditorLogic.OnPartListIconTap (AvailablePart p) [0x00000] in <48dcb08e2e1542e2af1286b02d2eb072>:0
  at KSP.UI.Screens.EditorPartList.TapIcon (AvailablePart part) [0x00000] in <48dcb08e2e1542e2af1286b02d2eb072>:0
  at KSP.UI.Screens.EditorPartIcon.MouseInput_SpawnPart () [0x00000] in <48dcb08e2e1542e2af1286b02d2eb072>:0
  at UnityEngine.Events.InvokableCall.Invoke () [0x00000] in <7d9ec060e791409ab3eb85c61e312ed6>:0
  at UnityEngine.Events.UnityEvent.Invoke () [0x00000] in <7d9ec060e791409ab3eb85c61e312ed6>:0
  at UnityEngine.UI.Button.Press () [0x00000] in <edc19f239d8642a2b533c214a7b029d5>:0
  at UnityEngine.UI.Button.OnPointerClick (UnityEngine.EventSystems.PointerEventData eventData) [0x00000] in <edc19f239d8642a2b533c214a7b029d5>:0
  at UnityEngine.EventSystems.ExecuteEvents.Execute (UnityEngine.EventSystems.IPointerClickHandler handler, UnityEngine.EventSystems.BaseEventData eventData) [0x00000] in <edc19f239d8642a2b533c214a7b029d5>:0
  at UnityEngine.EventSystems.ExecuteEvents.Execute[T] (UnityEngine.GameObject target, UnityEngine.EventSystems.BaseEventData eventData, UnityEngine.EventSystems.ExecuteEvents+EventFunction`1[T1] functor) [0x00000] in <edc19f239d8642a2b533c214a7b029d5>:0
  at UnityEngine.EventSystems.StandaloneInputModule.ReleaseMouse (UnityEngine.EventSystems.PointerEventData pointerEvent, UnityEngine.GameObject currentOverGo) [0x00000] in <edc19f239d8642a2b533c214a7b029d5>:0
  at UnityEngine.EventSystems.StandaloneInputModule.ProcessMousePress (UnityEngine.EventSystems.PointerInputModule+MouseButtonEventData data) [0x00000] in <edc19f239d8642a2b533c214a7b029d5>:0
  at UnityEngine.EventSystems.StandaloneInputModule.ProcessMouseEvent (System.Int32 id) [0x00000] in <edc19f239d8642a2b533c214a7b029d5>:0
  at UnityEngine.EventSystems.StandaloneInputModule.ProcessMouseEvent () [0x00000] in <edc19f239d8642a2b533c214a7b029d5>:0
  at UnityEngine.EventSystems.StandaloneInputModule.Process () [0x00000] in <edc19f239d8642a2b533c214a7b029d5>:0
  at UnityEngine.EventSystems.EventSystem.Update () [0x00000] in <edc19f239d8642a2b533c214a7b029d5>:0
Lisias commented 2 years ago

That's interesting… When loading a craft made from an earlier KSP version by the first time, things works fine. If you save the craft and load it, the problem starts to happen!

And this explains why I took so much time to be aware of this crap - most of my test crafts were made on earlier KSP versions - and I really don't play on KSP >= 1.8… :/

Lisias commented 2 years ago

Jesus Fscking Christ…. #facePalm

The problem was not on the attachment node, it is on the part position itself!!

[LOG 09:51:24.288] [KSP-Recall-AttachedOnEditor] TRACE: OnAwake Rockomax64.BW(Clone):FFF65482
[LOG 09:51:24.291] [KSP-Recall-AttachedOnEditor] TRACE: OnLoad Rockomax64.BW:FFF65482 True
[LOG 09:51:24.291] [KSP-Recall-AttachedOnEditor] TRACE: Before base.OnLoad Rockomax64.BW:FFF65482 POS:(0.0, 11.4, -2.0)
[LOG 09:51:24.291] [KSP-Recall-AttachedOnEditor] TRACE: After base.OnLoad Rockomax64.BW:FFF65482 POS:(0.0, 11.4, -2.0)
[LOG 09:51:24.306] [KSP-Recall-AttachedOnEditor] TRACE: OnSave Rockomax64.BW:FFF65482 True
[LOG 09:51:24.307] [KSP-Recall-AttachedOnEditor] TRACE: Before base.OnSave Rockomax64.BW:FFF65482 POS:(0.0, 11.4, -2.0)
[LOG 09:51:24.307] [KSP-Recall-AttachedOnEditor] TRACE: After base.OnSave Rockomax64.BW:FFF65482 POS:(0.0, 11.4, -2.0)
[LOG 09:51:24.362] [KSP-Recall-AttachedOnEditor] TRACE: OnStart Rockomax64.BW:FFF65482 Editor False
[LOG 09:51:24.362] [KSP-Recall-AttachedOnEditor] TRACE: Before base.OnStart Rockomax64.BW:FFF65482 POS:(0.0, 12.0, -2.0)
[LOG 09:51:24.362] [KSP-Recall-AttachedOnEditor] TRACE: After base.OnStart Rockomax64.BW:FFF65482 POS:(0.0, 12.0, -2.0)
[LOG 09:51:39.302] [KSP-Recall-AttachedOnEditor] TRACE: OnDestroy Rockomax64.BW:FFF65482

The problem is happening after the OnSave that happens immediately after the OnLoad and before the OnStart. Observe how the craft position changes from 11.4 to 12.0 .

Additionally, the problem is cumulative. Every time you launch the thing from the Editor and Revert to Editor, the drift is increased. At least the damned thing is deterministic.

Lisias commented 2 years ago

Movendo de https://github.com/net-lisias-ksp/TweakScale para https://github.com/net-lisias-ksp/KSP-Recall .

Agora que eu entendi onde está o problema, a solução é trivial.

Lisias commented 2 years ago

Fellow Kerbonaut Krazy1 found this gem on the KSP 1.9's release notes:

Fix surface attach node handling on Part Variants

I don't know what they intended to do, but whatever it was, screwed me up royally.

Lisias commented 2 years ago

Implemented on commit https://github.com/net-lisias-ksp/KSP-Recall/commit/d7405b6efee2d0f1aa866197c40a5437c176cf7a

Lisias commented 2 years ago

On commit https://github.com/net-lisias-ksp/KSP-Recall/commit/c2d91be7f1c27aaf3673497a79a1d3166b31e3ab , I locked up the patch to be applied only when TweakScale is applied on the part.

Since this problem is plaguing KSP since 1.9.0 (2020-0212 - more than a year and a half ago), it's reasonable to assume that the most used add'ons were not affected, or had already applied their own fix to the problem and this workaround may break them again.

Of course I may be wrong, but KSP-Recall can easily be patched by the user or 3rd parties to add this workaround to any other part, so I think this is a good compromise - at least, until further and better information is gathered.

Lisias commented 2 years ago

Oukey, the stunt worked on KSP 1.9.1 and 1.12.3. :)

So I'm closing this.

Lisias commented 2 years ago

I'm reopening this because I forgot to handle the PartModule.OnCopy callback… #facePalm

Lisias commented 2 years ago

Fixed (again) on commit https://github.com/net-lisias-ksp/KSP-Recall/commit/11c53a7bb9c499e57010246f7be1b50ebcae47b6

Lisias commented 2 years ago

Now it works with symmetries too! So I'm closing this. Again. (Hopefully) by the last time.

Lisias commented 2 years ago

Since I didn't managed to invest time on learning how to code a new KSP Upgrade Pipeline thingy (that would allow a seamless upgrade of SubAssemblies without AttachedToEditor), I had to find a (viable) workaround to make easier for the users to update their SubAssembly files.

Well.. Craft Manager to the rescue!!!

Screen Shot 2022-03-08 at 13 23 37

Load a SubAssembly as it was a craft using the depicted option above, and then just drag and drop the whole shebang into a new SubAssembly! :)