Search Unity

  1. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Bug Setting Selectabe.Navigation.wrapAround does not work when setting it two times in a row

Discussion in 'UGUI & TextMesh Pro' started by CortiWins, Jul 1, 2023.

  1. CortiWins

    CortiWins

    Joined:
    Sep 24, 2018
    Posts:
    150
    Unity 2022.3.3f1 LTS

    The purpose of the function is:
    • Takes an array of UnityEngine.UI.Selectable
    • Sets active components to Navigation.Mode.Vertical, and inacive ones to Navigation.Mode.None.
    • Sets first and last active component to navStruct.wrapAround = true and all others to false.
    Provided
    • Code Snipped #1 Function that fails the unit test
    • Code Snipped #2 Function that passes the unit test
    • Code Snipped #3 Unit Test
    Version 1 Fail: My first attempt was that i don't want two loops and i didnt want linq because of allocations, so this functions looks like it does. I don't say it's beautiful ( its not ) , but thats not the issue here.

    Code (CSharp):
    1. /// <summary>
    2. /// NOT WORKING:
    3. /// </summary>
    4. public static void SetNavigationFail(Selectable[] controls)
    5. {
    6.     var indexFirstActive = -1;
    7.     var indexLastActive = -1;
    8.  
    9.     for (var index = 0; index < controls.Length; index++)
    10.     {
    11.         var control = controls[index];
    12.  
    13.         var navStruct = control.navigation;
    14.         if (control.gameObject.activeSelf)
    15.         {
    16.             navStruct.mode = Navigation.Mode.Vertical;
    17.             indexLastActive = index;
    18.             if (indexFirstActive == -1)
    19.             {
    20.                 indexFirstActive = index;
    21.                 navStruct.wrapAround = true;
    22.             }
    23.             else
    24.             {
    25.                 navStruct.wrapAround = false;
    26.             }
    27.         }
    28.         else
    29.         {
    30.             navStruct.mode = Navigation.Mode.None;
    31.             navStruct.wrapAround = false;
    32.         }
    33.  
    34.         control.navigation = navStruct;
    35.     }
    36.  
    37.     if (indexLastActive >= 0)
    38.     {
    39.         var selectableControl = controls[indexLastActive];
    40.         var navStruct = selectableControl.navigation;
    41.         navStruct.wrapAround = true;
    42.         // THIS IS IGNORED
    43.         selectableControl.navigation = navStruct;
    44.     }
    45. }
    46.  
    The unit test ( code down below ) fails. The last selectable is not wrapAround = true. Debugging into the code shows that at Line 42, the struct is set to the Selectable.Navigation-Property, but the wrapAround value is not updated.

    Version 2 Pass: My second attempt is reworked around setting the Selectable.Navigation struct just once per instance. And this time it works.

    Code (CSharp):
    1. /// <summary>
    2. /// WORKING:
    3. /// </summary>
    4. public static void SetNavigationPass(Selectable[] controls)
    5. {
    6.     var indexFirstActive = -1;
    7.     var indexLastActive = -1;
    8.  
    9.     // Find first and last
    10.     for (var index = 0; index < controls.Length; index++)
    11.     {
    12.         var control = controls[index];
    13.         if (control.gameObject.activeSelf)
    14.         {
    15.             indexLastActive = index;
    16.             if (indexFirstActive == -1)
    17.             {
    18.                 indexFirstActive = index;
    19.             }
    20.         }
    21.     }
    22.  
    23.     // Set each component once.
    24.     for (var index = 0; index < controls.Length; index++)
    25.     {
    26.         var control = controls[index];
    27.  
    28.         var navStruct = control.navigation;
    29.         navStruct.mode = control.gameObject.activeSelf ? Navigation.Mode.Vertical : Navigation.Mode.None;
    30.         navStruct.wrapAround = index == indexLastActive || index == indexFirstActive;
    31.         control.navigation = navStruct;
    32.     }
    33. }
    34.  
    Here's the test that shows the bug for me.
    Code (CSharp):
    1. [Test]
    2. public void MenuNavigationLinksTestMethod()
    3. {
    4.     // Use the Assert class to test conditions
    5.  
    6.     var selects = new UnityEngine.UI.Selectable[6];
    7.     for (int i = 0; i < 6; i++)
    8.     {
    9.         var go = new GameObject(i.ToString());
    10.         go.SetActive(true);
    11.         selects[i] = go.AddComponent<UnityEngine.UI.Button>();
    12.     }
    13.  
    14.     CortiWins.BasePage.SetNavigationFail(selects);
    15.     Assert.IsTrue(selects[0].navigation.mode == UnityEngine.UI.Navigation.Mode.Vertical);
    16.     Assert.IsTrue(selects[1].navigation.mode == UnityEngine.UI.Navigation.Mode.Vertical);
    17.     Assert.IsTrue(selects[2].navigation.mode == UnityEngine.UI.Navigation.Mode.Vertical);
    18.     Assert.IsTrue(selects[3].navigation.mode == UnityEngine.UI.Navigation.Mode.Vertical);
    19.     Assert.IsTrue(selects[4].navigation.mode == UnityEngine.UI.Navigation.Mode.Vertical);
    20.     Assert.IsTrue(selects[5].navigation.mode == UnityEngine.UI.Navigation.Mode.Vertical);
    21.  
    22.     Assert.IsTrue(selects[0].navigation.wrapAround);
    23.     Assert.IsTrue(selects[5].navigation.wrapAround);
    24.  
    25.     Assert.IsFalse(selects[1].navigation.wrapAround);
    26.     Assert.IsFalse(selects[2].navigation.wrapAround);
    27.     Assert.IsFalse(selects[3].navigation.wrapAround);
    28.     Assert.IsFalse(selects[4].navigation.wrapAround);
    29. }
    30.  
    I didn't find any traces of "you may only set this once per frame" etc. If that's something i should know, please tell me.