Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ForceSyncSceneSwitch patch incompatibility with Universal Storage 2 #273

Open
gotmachine opened this issue Oct 20, 2024 · 2 comments
Open
Labels
bug Something isn't working

Comments

@gotmachine
Copy link
Contributor

gotmachine commented Oct 20, 2024

The ForceSyncSceneSwitch patch is causing US2 to break the PAW UI controls prefab.
Steps to reproduce :

  • Go to the VAB
  • Go to the flight scene

The following exception happens :

[EXC 20:21:43.454] NullReferenceException: Object reference not set to an instance of an object
	UIPartActionController.SetupItemControls () (at <39c0323fb6b449a4aaf3465c00ed3c8d>:0)
	UIPartActionController.Start () (at <39c0323fb6b449a4aaf3465c00ed3c8d>:0)
	UnityEngine.DebugLogHandler:LogException(Exception, Object)
	ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
	UnityEngine.Debug:CallOverridenDebugHandler(Exception, Object)

Then opening the PAW will trigger a flood of ItemPrefab for control type 'UI_xxx' not found. errors.

This is caused by US2 adding a new entry to the fieldPrefabs list in the UIPartActionController prefab every time the editor scene is loaded. That entry is a GameObject/Component lying around in the editor scene root.

Upon leaving the editor scene, that object is destroyed, leaving that destroyed entry in the prefab.
On exiting the editor, stock will do a double scene loading : first the loadingBuffer scene, then the requested scene. Somehow, this result in the destroyed object being cleaned up from the prefab list, I guess because the prefab is truly unloaded during the time the loadingBuffer scene is the only one loaded.

But with the ForceSyncSceneSwitch patch, the loadingBuffer scene isn't loaded at all (to avoid the redundant asset cleanup call). This result in the prefab not being unloaded/reloaded (I guess because it is also used in the flight scene), the null entry persisting, and finally in the nullref exception. Doing an editor > KSC > flight scene transition does prevent the issue, but trying to call Resources.UnloadUnusedAssets() after the scene transition request doesn't.

What US2 is doing is a bit sketchy to begin with, but I it could be possible to have plugins doing similar stuff and relying on the prefabs being unloaded between the scene transitions using the buffered load path.

The only way I found to reliably reproduce stock behavior and avoid the US2 issue is to keep loading the loadingBuffer scene in between the requested scene. The extra Resources.UnloadUnusedAssets() and the async loading don't seem necessary, but that still leave us with a double asset cleanup (due to the double scene load), and I would guess a large part of the gains were actually in avoiding to unload assets that are shared between the flight and editor scenes.

My position for now would be to put together a MM patch disabling the ForceSyncSceneSwitch patch when US2 is installed.

On a side note, adding custom PAW UI controls is cumbersome and messy, I won't really blame US2 for doing it that way. Maybe we should look into providing an API in KSPCF for this.

@gotmachine
Copy link
Contributor Author

gotmachine commented Oct 20, 2024

For reference, this is what I came with as a "safer" version of the patch :

static void HighLogic_LoadScene_Override(GameScenes sceneToLoad)
{
    if (HighLogic.LoadedSceneIsGame)
    {
        if (sceneToLoad == GameScenes.MAINMENU && PSystemSetup.Instance != null)
        {
            PSystemSetup.Instance.RemoveNonStockLaunchSites();
        }
        if (sceneToLoad == GameScenes.MAINMENU)
        {
            MissionSystem.RemoveMissionObjects(removeAll: true);
            if (HighLogic.CurrentGame != null && HighLogic.CurrentGame.missionToStart != null)
            {
                MissionSystem.RemoveMissonObject(HighLogic.CurrentGame.missionToStart.MissionInfo);
            }
        }
    }

    GameScenes currentScene = HighLogic.LoadedScene;
    bool transitionValue = HighLogic.fetch.sceneBufferTransitionMatrix.GetTransitionValue(currentScene, sceneToLoad);
    HighLogic.SetLoadSceneEventsAndFlags(sceneToLoad, false);

    if (transitionValue)
    {
        SceneManager.LoadScene("loadingBuffer");
        SceneManager.LoadScene((int)sceneToLoad);
    }
    else
    {
        SceneManager.LoadScene((int)sceneToLoad);
    }
}

Unfortunately, this mean a double asset cleanup is performed due to the double scene load, and that assets shared between the previous and next scene will be unloaded / reloaded, because they don't exist in the loadingBuffer scene (and which was likely a large part of why the current patch improves the VAB <> flight scene transitions significantly).

@JonnyOThan
Copy link
Contributor

On a side note, adding custom PAW UI controls is cumbersome and messy, I won't really blame US2 for doing it that way. Maybe we should look into providing an API in KSPCF for this.

TweakScale Rescaled does something like this: https://github.com/JonnyOThan/TweakScale/blob/4719b9ff96521463b8b31b8488c1c6e61b891845/Source/HarmonyPatching/Squad.cs#L99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants