IllusionMods / IllusionFixes

A collection of fixes for common issues found in games by Illusion
GNU General Public License v3.0
157 stars 29 forks source link

Add fix scene load gui #53

Closed takahiro0327 closed 6 months ago

takahiro0327 commented 6 months ago

Fixes a problem in the UI for loading scenes where page and scrollbar positions are not saved. This seems to occur only on KK/KKS. Please merge.

How to reproduce the problem

  1. Open the scene load UI
  2. Select any page
  3. Close the scene load UI
  4. Open the scene load UI again KK does not save the page and scrollbars. In KKS, the page is saved but the scrollbars are not. HS2 saves both.

image

takahiro0327 commented 6 months ago

I don't mind, but is this optimization? I too feel it's overkill, but I just didn't feel it was right to put it in a place named "optimization".

takahiro0327 commented 6 months ago

Sorry to waste your time reviewing it! I'm off to make a KKS version of StudioSceneNavigation. That will accomplish my goal.

takahiro0327 commented 6 months ago

Next time I want something, I'll check KK's behavior first.

I made a PR for the KKS version. https://github.com/GeBo1/GeBoPlugins/pull/12 KKS_StudioSceneNavigation.zip

ManlyMarco commented 6 months ago

StudioSceneNavigation doesn't keep the scroll bar position so there might still be merit in keeping this.

I don't mind, but is this optimization? I too feel it's overkill, but I just didn't feel it was right to put it in a place named "optimization".

I think it's fine to add this. You could say that it optimizes the user experience.

takahiro0327 commented 6 months ago

Ah, I thought it was the CPU/memory performance optimization one.

Were you in charge of the repository there, too?

StudioSceneNavigation doesn't keep the scroll bar position so there might still be merit in keeping this.

What? At least the scrollbars are retained in KKS. Am I misunderstanding something? I guess I still have the modified plugin on my local again.

https://github.com/GeBo1/GeBoPlugins/blob/master/src/StudioSceneNavigation/Core_StudioSceneNavigation/Core.StudioSceneNavigation.cs#L752

ManlyMarco commented 6 months ago

You're right, I made a mistake while testing it.

takahiro0327 commented 6 months ago

No, the scroll position is kept regardless of my changes.

I added the following hook to close and open the UI, scrolling to near the center. It was set only from StudioSceneNavigation and the system.

            [HarmonyPostfix]
            [HarmonyPatch(typeof(UnityEngine.UI.Scrollbar), nameof(UnityEngine.UI.Scrollbar.value), MethodType.Setter)]
            internal static void PostScrollbarValue(UnityEngine.UI.Scrollbar __instance, float value )
            {
                if( __instance.gameObject.GetComponentInParent<SceneLoadScene>() != null )
                {
                    System.Console.WriteLine($"@@@ Scrollbar.value = {value};");
                    System.Console.WriteLine(new System.Diagnostics.StackTrace());
                }
            }
@@@ Scrollbar.value = 1;
  at StudioSceneNavigationPlugin.StudioSceneNavigation+Hooks.PostScrollbarValue (UnityEngine.UI.Scrollbar __instance, System.Single value) [0x00000] in <584af79af356409fb76ab852fcbff4dc>:0
  at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::set_value> (UnityEngine.UI.Scrollbar , System.Single ) [0x00000] in <1c4c7ec3463840f298c6bedffa7311b7>:0
  at UnityEngine.UI.ScrollRect.UpdateScrollbars (UnityEngine.Vector2 offset) [0x00000] in <1c4c7ec3463840f298c6bedffa7311b7>:0
  at UnityEngine.UI.ScrollRect.Rebuild (UnityEngine.UI.CanvasUpdate executing) [0x00000] in <1c4c7ec3463840f298c6bedffa7311b7>:0
  at UnityEngine.UI.CanvasUpdateRegistry.PerformUpdate () [0x00000] in <1c4c7ec3463840f298c6bedffa7311b7>:0
  at UnityEngine.Canvas.SendWillRenderCanvases () [0x00000] in <cb8d7d5bd77b485bb139e6889ac07328>:0
  at UnityEngine.Canvas.ForceUpdateCanvases () [0x00000] in <cb8d7d5bd77b485bb139e6889ac07328>:0
  at UnityEngine.UI.ScrollRect.EnsureLayoutHasRebuilt () [0x00000] in <1c4c7ec3463840f298c6bedffa7311b7>:0
  at UnityEngine.UI.ScrollRect.LateUpdate () [0x00000] in <1c4c7ec3463840f298c6bedffa7311b7>:0
@@@ Scrollbar.value = 0.4506451;
  at StudioSceneNavigationPlugin.StudioSceneNavigation+Hooks.PostScrollbarValue (UnityEngine.UI.Scrollbar __instance, System.Single value) [0x00000] in <584af79af356409fb76ab852fcbff4dc>:0
  at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::set_value> (UnityEngine.UI.Scrollbar , System.Single ) [0x00000] in <1c4c7ec3463840f298c6bedffa7311b7>:0
  at StudioSceneNavigationPlugin.StudioSceneNavigation+<>c__DisplayClass96_1.<ScrollToCurrentPageCoroutine>b__1 (UnityEngine.UI.Scrollbar sb) [0x00000] in <584af79af356409fb76ab852fcbff4dc>:0
  at NullCheck.SafeProc[T] (T self, System.Action`1[T] act) [0x00000] in <8b09ac0ba803494aafbed35d2609270f>:0
  at StudioSceneNavigationPlugin.StudioSceneNavigation+<>c__DisplayClass96_0.<ScrollToCurrentPageCoroutine>b__0 (UnityEngine.UI.ScrollRect r) [0x00000] in <584af79af356409fb76ab852fcbff4dc>:0
  at NullCheck.SafeProc[T] (System.Collections.Generic.IReadOnlyList`1[T] list, System.Int32 index, System.Action`1[T] act) [0x00000] in <8b09ac0ba803494aafbed35d2609270f>:0
  at StudioSceneNavigationPlugin.StudioSceneNavigation+<ScrollToCurrentPageCoroutine>d__96.MoveNext () [0x00000] in <584af79af356409fb76ab852fcbff4dc>:0
  at UnityEngine.SetupCoroutine.InvokeMoveNext (System.Collections.IEnumerator enumerator, System.IntPtr returnValueAddress) [0x00000] in <548b4fa0e7e04f27a1b7580930bfb7dc>:0
@@@ Scrollbar.value = 0.4506451;
  at StudioSceneNavigationPlugin.StudioSceneNavigation+Hooks.PostScrollbarValue (UnityEngine.UI.Scrollbar __instance, System.Single value) [0x00000] in <584af79af356409fb76ab852fcbff4dc>:0
  at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::set_value> (UnityEngine.UI.Scrollbar , System.Single ) [0x00000] in <1c4c7ec3463840f298c6bedffa7311b7>:0
  at UnityEngine.UI.ScrollRect.UpdateScrollbars (UnityEngine.Vector2 offset) [0x00000] in <1c4c7ec3463840f298c6bedffa7311b7>:0
  at UnityEngine.UI.ScrollRect.LateUpdate () [0x00000] in <1c4c7ec3463840f298c6bedffa7311b7>:0
takahiro0327 commented 6 months ago

I changed the hook to something more fundamental and looked at it in KK. I still don't see any interference from anything other than the system and StudioSceneNavigation.

            [HarmonyPostfix]
            [HarmonyPatch(typeof(UnityEngine.UI.Scrollbar), nameof(UnityEngine.UI.Scrollbar.Set), typeof(float), typeof(bool))]
            internal static void PostScrollbarValue(UnityEngine.UI.Scrollbar __instance, float input, bool sendCallback)
            {
                if( __instance.gameObject.GetComponentInParent<SceneLoadScene>() != null )
                {
                    System.Console.WriteLine($"@@@ Scrollbar.Set({input},{sendCallback});");
                    System.Console.WriteLine(new System.Diagnostics.StackTrace());
                }
            }

#if KK
            [HarmonyPostfix]
            [HarmonyPatch(typeof(UnityEngine.UI.Scrollbar), nameof(UnityEngine.UI.Scrollbar.Set), typeof(float))]
            internal static void PostScrollbarValue(UnityEngine.UI.Scrollbar __instance, float input)
            {
                if (__instance.gameObject.GetComponentInParent<SceneLoadScene>() != null)
                {
                    System.Console.WriteLine($"@@@ Scrollbar.Set({input});");
                    System.Console.WriteLine(new System.Diagnostics.StackTrace());
                }
            }
#endif
@@@ Scrollbar.Set(1,True);
   at StudioSceneNavigationPlugin.StudioSceneNavigation+Hooks.PostScrollbarValue(UnityEngine.UI.Scrollbar __instance, Single input, Boolean sendCallback)
   at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::Set>(UnityEngine.UI.Scrollbar , Single , Boolean )
   at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::Set>(UnityEngine.UI.Scrollbar , Single )
   at UnityEngine.UI.Scrollbar.set_value(Single value)
   at UnityEngine.UI.ScrollRect.UpdateScrollbars(Vector2 offset)
   at UnityEngine.UI.ScrollRect.Rebuild(CanvasUpdate executing)
   at UnityEngine.UI.CanvasUpdateRegistry.PerformUpdate()
   at UnityEngine.Canvas.SendWillRenderCanvases()
   at UnityEngine.Canvas.ForceUpdateCanvases()
   at UnityEngine.UI.ScrollRect.EnsureLayoutHasRebuilt()
   at UnityEngine.UI.ScrollRect.LateUpdate()
@@@ Scrollbar.Set(1);
   at StudioSceneNavigationPlugin.StudioSceneNavigation+Hooks.PostScrollbarValue(UnityEngine.UI.Scrollbar __instance, Single input)
   at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::Set>(UnityEngine.UI.Scrollbar , Single )
   at UnityEngine.UI.Scrollbar.set_value(Single value)
   at UnityEngine.UI.ScrollRect.UpdateScrollbars(Vector2 offset)
   at UnityEngine.UI.ScrollRect.Rebuild(CanvasUpdate executing)
   at UnityEngine.UI.CanvasUpdateRegistry.PerformUpdate()
   at UnityEngine.Canvas.SendWillRenderCanvases()
   at UnityEngine.Canvas.ForceUpdateCanvases()
   at UnityEngine.UI.ScrollRect.EnsureLayoutHasRebuilt()
   at UnityEngine.UI.ScrollRect.LateUpdate()
@@@ Scrollbar.Set(0.4968966,True);
   at StudioSceneNavigationPlugin.StudioSceneNavigation+Hooks.PostScrollbarValue(UnityEngine.UI.Scrollbar __instance, Single input, Boolean sendCallback)
   at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::Set>(UnityEngine.UI.Scrollbar , Single , Boolean )
   at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::Set>(UnityEngine.UI.Scrollbar , Single )
   at UnityEngine.UI.Scrollbar.set_value(Single value)
   at StudioSceneNavigationPlugin.StudioSceneNavigation+<>c__DisplayClass96_1.<ScrollToCurrentPageCoroutine>b__1(UnityEngine.UI.Scrollbar sb)
   at NullCheck.Call(System.Action`1 action, UnityEngine.UI.Scrollbar arg)
   at NullCheck.SafeProc(UnityEngine.UI.Scrollbar self, System.Action`1 act)
   at StudioSceneNavigationPlugin.StudioSceneNavigation+<>c__DisplayClass96_0.<ScrollToCurrentPageCoroutine>b__0(UnityEngine.UI.ScrollRect r)
   at NullCheck.Call(System.Action`1 action, UnityEngine.UI.ScrollRect arg)
   at NullCheck.SafeProc(UnityEngine.UI.ScrollRect self, System.Action`1 act)
   at NullCheck.SafeProc(UnityEngine.UI.ScrollRect[] array, Int32 index, System.Action`1 act)
   at StudioSceneNavigationPlugin.StudioSceneNavigation+<ScrollToCurrentPageCoroutine>d__96.MoveNext()
   at UnityEngine.SetupCoroutine.InvokeMoveNext(IEnumerator enumerator, IntPtr returnValueAddress)
@@@ Scrollbar.Set(0.4968966);
   at StudioSceneNavigationPlugin.StudioSceneNavigation+Hooks.PostScrollbarValue(UnityEngine.UI.Scrollbar __instance, Single input)
   at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::Set>(UnityEngine.UI.Scrollbar , Single )
   at UnityEngine.UI.Scrollbar.set_value(Single value)
   at StudioSceneNavigationPlugin.StudioSceneNavigation+<>c__DisplayClass96_1.<ScrollToCurrentPageCoroutine>b__1(UnityEngine.UI.Scrollbar sb)
   at NullCheck.Call(System.Action`1 action, UnityEngine.UI.Scrollbar arg)
   at NullCheck.SafeProc(UnityEngine.UI.Scrollbar self, System.Action`1 act)
   at StudioSceneNavigationPlugin.StudioSceneNavigation+<>c__DisplayClass96_0.<ScrollToCurrentPageCoroutine>b__0(UnityEngine.UI.ScrollRect r)
   at NullCheck.Call(System.Action`1 action, UnityEngine.UI.ScrollRect arg)
   at NullCheck.SafeProc(UnityEngine.UI.ScrollRect self, System.Action`1 act)
   at NullCheck.SafeProc(UnityEngine.UI.ScrollRect[] array, Int32 index, System.Action`1 act)
   at StudioSceneNavigationPlugin.StudioSceneNavigation+<ScrollToCurrentPageCoroutine>d__96.MoveNext()
   at UnityEngine.SetupCoroutine.InvokeMoveNext(IEnumerator enumerator, IntPtr returnValueAddress)
@@@ Scrollbar.Set(0.4968966,True);
   at StudioSceneNavigationPlugin.StudioSceneNavigation+Hooks.PostScrollbarValue(UnityEngine.UI.Scrollbar __instance, Single input, Boolean sendCallback)
   at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::Set>(UnityEngine.UI.Scrollbar , Single , Boolean )
   at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::Set>(UnityEngine.UI.Scrollbar , Single )
   at UnityEngine.UI.Scrollbar.set_value(Single value)
   at UnityEngine.UI.ScrollRect.UpdateScrollbars(Vector2 offset)
   at UnityEngine.UI.ScrollRect.LateUpdate()
@@@ Scrollbar.Set(0.4968966);
   at StudioSceneNavigationPlugin.StudioSceneNavigation+Hooks.PostScrollbarValue(UnityEngine.UI.Scrollbar __instance, Single input)
   at UnityEngine.UI.Scrollbar.DMD<UnityEngine.UI.Scrollbar::Set>(UnityEngine.UI.Scrollbar , Single )
   at UnityEngine.UI.Scrollbar.set_value(Single value)
   at UnityEngine.UI.ScrollRect.UpdateScrollbars(Vector2 offset)
   at UnityEngine.UI.ScrollRect.LateUpdate()

https://github.com/Unity-Technologies/uGUI/blob/5ab4c0fee7cd5b3267672d877ec4051da525913c/UnityEngine.UI/UI/Core/Scrollbar.cs#L230 https://github.com/Unity-Technologies/uGUI/blob/5ab4c0fee7cd5b3267672d877ec4051da525913c/UnityEngine.UI/UI/Core/ScrollRect.cs