Search Unity

SendMessage incorrectly assigning i iteration?

Discussion in 'Scripting' started by JamieRoss95, Nov 17, 2019.

  1. JamieRoss95

    JamieRoss95

    Joined:
    Apr 5, 2016
    Posts:
    28
    Hey there,

    I implemented a dynamic dropdown menu for my inventory system today and all is working well, however, I have run into an odd bug when assigning the listener for my buttons.

    The dropdown item has 3 inactive buttons that are activated, text is changed, and listerner assigned when the dropdown is called on the appropriate item. The odd thing is that the SendMessage call I am using to add the listener seems to be assigning an incorrect function call seperate from the i iteration that it should be using. Hopefully, my code clears up my problem as it is difficult to explain.

    Code (CSharp):
    1.     public void ShowMenu(ClassItemUI targetItem)
    2.     {
    3.         _menuIsActive = true;
    4.         curParentItem = targetItem;
    5.      
    6.         //ACTIVATE ALL APPROPRIATE BUTTONS
    7.         for (int i = 0; i < curParentItem.dropdownButtonNames.Length; i++)
    8.         {
    9.             print("Message: " + (i));
    10.             actionButtons[i].gameObject.SetActive(true);
    11.             //SET NAME & FUNCTION
    12.             actionButtons[i].GetComponentInChildren<TextMeshProUGUI>().text = curParentItem.dropdownButtonNames[i];
    13.             actionButtons[i].onClick.AddListener(() => curParentItem.SendMessage("UseButton" + (i)));
    14.         }
    15.      
    16.         MenuPosition();
    17.     }
    When I click on a button it prints the corresponding "UseButton" function. When I use an item with 2 buttons this the command log that I get in return:

    Code (CSharp):
    1. Message: 0
    2. Message: 1
    3. Use Button 3
    4. Use Button 3
    The Use Button log should be "Use Button 0" followed by "Use Button 1". It is as if the AddListener is using the total iterations in the for loop rather than the current.

    Any help is greatly appreciated thanks!
     
  2. Adrian

    Adrian

    Joined:
    Apr 5, 2008
    Posts:
    1,066
    The problem comes from the fact that the SendMessage is part of a closure function that is registered as the onClick event handler. This event handler gets called after the for loop has completed and only captures a reference to the i variable, not the value of i at the point the closure was created. At that time the for loop has counted up i until the end and so all event handlers get the value 3.

    To illustrate, here i is immediately logged and you'll get 0, 1, 2 printed to the console:
    Code (CSharp):
    1. for (int i = 0; i < 3; i++) {
    2.     Debug.Log(i);
    3. }
    On the other hand, here the closures are only created in the loop but called later, at wich point i is 3 and 3, 3, 3 will be printed:
    Code (CSharp):
    1. var closures = new List<Action>();
    2. for (int i = 0; i < 3; i++) {
    3.     closures.Add(() => Debug.Log(i));
    4. }
    5. closures.ForEach(a => a());
    To fix this, create a variable inside the for loop and assign it the value of i. Since this variable is inside the for loop's scope (curly braces always create a new variable scope), each iteration will create a distinct variable, even if they have the same name. The closures then capture this distinct variable, wich contains the value of i from the loop's iteration and 0, 1, 2 will be printed:
    Code (CSharp):
    1. var closures = new List<Action>();
    2. for (int i = 0; i < 3; i++) {
    3.     var k = i;
    4.     closures.Add(() => Debug.Log(k));
    5. }
    6. closures.ForEach(a => a());
     
  3. JamieRoss95

    JamieRoss95

    Joined:
    Apr 5, 2016
    Posts:
    28
    You're a legend! Tbh I should've thought of just turning i into a variable on my own lol.

    Thanks a bunch man!