KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
696 stars 229 forks source link

Changing the active vessel using `kuniverse` from IVA mode will break the internal camera until the next scene change #3095

Open JonnyOThan opened 11 months ago

JonnyOThan commented 11 months ago

Repro steps:

  1. Get 2 ships within close proximity of each other (simply decoupling a probe core on the launchpad is sufficient). Make sure one of them is manned and has an IVA.
  2. Enter IVA mode in the manned ship
  3. Open the KOS terminal and use it to switch to the other vessel using kuniverse:activevessel or kuniverse:forceactive
  4. Note the camera is pretty broken, but pressing C makes it fairly normal
  5. Press square bracket to swap to the manned ship again
  6. Try to enter IVA mode by pressing C or clicking the "view" button on the portrait

Results: NREs are spewed:

[EXC 20:38:22.752] NullReferenceException: Object reference not set to an instance of an object
    CameraManager.SetCameraIVA (Kerbal kerbal, System.Boolean resetCamera) (at C:/Users/Jon/source/repos/ksp-assembly-csharp/CameraManager.cs:319)
    KSP.UI.Screens.Flight.KerbalPortrait.ClickIVA () (at C:/Users/Jon/source/repos/ksp-assembly-csharp/KSP.UI.Screens.Flight/KerbalPortrait.cs:191)
    UnityEngine.Events.InvokableCall.Invoke () (at <12e76cd50cc64cf19e759e981cb725af>:0)
    UnityEngine.Events.UnityEvent.Invoke () (at <12e76cd50cc64cf19e759e981cb725af>:0)
    UnityEngine.UI.Button.Press () (at <5336a8686ff14f17888ce9a9f44f29bc>:0)
    UnityEngine.UI.Button.OnPointerClick (UnityEngine.EventSystems.PointerEventData eventData) (at <5336a8686ff14f17888ce9a9f44f29bc>:0)
    UnityEngine.EventSystems.ExecuteEvents.Execute (UnityEngine.EventSystems.IPointerClickHandler handler, UnityEngine.EventSystems.BaseEventData eventData) (at <5336a8686ff14f17888ce9a9f44f29bc>:0)
    UnityEngine.EventSystems.ExecuteEvents.Execute[T] (UnityEngine.GameObject target, UnityEngine.EventSystems.BaseEventData eventData, UnityEngine.EventSystems.ExecuteEvents+EventFunction`1[T1] functor) (at <5336a8686ff14f17888ce9a9f44f29bc>:0)
    UnityEngine.DebugLogHandler:LogException(Exception, Object)
    ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
    UnityEngine.EventSystems.EventSystem:Update()

This does NOT occur if you use square brackets to change vessels in step 3. I'm pretty sure the root issue here is that the InternalCamera.Instance object is a child object of the kerbal inside the IVA. Normally the camera would be detached when changing the active vessel, but somehow KOS is changing vessels without detaching the camera so it is destroyed along with the old IVA. The only way to fix it is a scene change.

nuggreat commented 11 months ago

As all kOS does is call FlightGlobals.SetActiveVessel(vessel); or FlightGlobals.ForceSetActiveVessel(vessel); when you set those suffixes (code here) this sounds to me like KSP is doing something extra when you change vessel from within the IVA view that they really should have built into the methods.

SofieBrink commented 11 months ago

While true, since ksp won’t be getting those fixes anymore it does seem like something kOS will have to check for

Dunbaratu commented 11 months ago

Chances are the stock KSP API never expected the UX to let a player switch scenes from inside IVA, so the problem it causes is never experienced in stock play.

There's a lot of these messy cases in KSP, where protections against Bad Things that really belong down in the API are only enforced by the UX code not providing the player with a button or keypress that would invoke the Bad Thing. (Thus why we ended up just banning NaN and Infinity in kOS mathematics because otherwise a player script can just take that NaN or Infinity and send it into a KSP API call that throws exception when it tries using the value.)

JonnyOThan commented 11 months ago

@Dunbaratu see the note on my comment. You CAN switch vessels from IVA in stock (use square bracket keys) and the bug does not occur. It’s specific to whatever KOS is doing. More likely, the stock code is doing some other bookkeeping inside the button press handler in addition to actually switching vessels.