Search Unity

lambda in foreach is capturing loop variable DESPITE LOCAL COPY

Discussion in 'Scripting' started by Undertaker-Infinity, Apr 30, 2015.

  1. Undertaker-Infinity

    Undertaker-Infinity

    Joined:
    May 2, 2014
    Posts:
    112
    Hi.

    By now I've read a bunch about this issue.
    I have a foreach iterating over a dictionary, instantiating objects with a Toggle component and I'm adding an event handler for the ValueChanged event in it, passing the dictionary key as a parameter.
    Of course, the loop Pair<int,string> is captured. I read about that. The callback always gets the last key, because we've been changing the same variable that it has.
    So I did what everybody said: take a local copy inside of the loop.
    It doesn't work. I get the same result as when using the pair.Key directly.

    Edit: I thought even't wasn't firing and it was (since log had the same number, it was grouped!), so I've removed all references to that issue. Lambda issue is still there though.

    Summary:
    -Local copy to break out of the lambda closure is not working. Why? How should it be done?

    Here's the code:

    Code (CSharp):
    1.         Transform newToggleTransform;
    2.         RectTransform newToggleRect;
    3.         Toggle newToggle;
    4.         Text newToggleText;
    5.         ToggleId newToggleId;
    6.         float xPos = 0;
    7.         foreach (KeyValuePair<int,string> pair in retails) {
    8.             newToggleTransform = Instantiate(retailTogglePrefab, new Vector3(xPos, 0, 0), Quaternion.identity) as Transform;
    9.             newToggleTransform.SetParent(retailGroupPanel,false);
    10.             newToggle = newToggleTransform.GetComponent<Toggle> ();
    11.             newToggle.isOn = false;
    12.             if (xPos == 0) {
    13.                 newToggle.isOn = true; // doesn't fire, as expected
    14.                 activateRetail(true,pair.Key); // so we call the callback manually. This works :)
    15.                 Debug.Log ("toggled because xPos == 0");
    16.             }
    17.             newToggle.group = retailGroupPanel.GetComponent<ToggleGroup> ();
    18.             newToggleText = newToggleTransform.GetComponent<Text> ();
    19.             newToggleText.text = pair.Value; // retailName
    20.             newToggleId = newToggleTransform.GetComponent<ToggleId> ();
    21.             newToggleId.toggleId = pair.Key; // retailId <-- do we even need this?
    22.             newToggleRect = newToggleTransform.GetComponent<RectTransform> ();
    23.             int temp = pair.Key;
    24.             newToggle.onValueChanged.AddListener((value) => {
    25.                 Debug.Log ("listener fired, calling with "+temp.ToString()); // called only once, with the last key :(
    26.                 activateRetail(value,temp);
    27.             });
    28.             xPos += newToggleRect.rect.width;
    29.         }
    30.  
     
    Last edited: Apr 30, 2015
  2. Undertaker-Infinity

    Undertaker-Infinity

    Joined:
    May 2, 2014
    Posts:
    112
    I ended up working around it, by just ignoring the callback parameter and searching through the list of toggles looking for the one that is on.

    The question remains though, why is the above local copy not working?
     
  3. Fajlworks

    Fajlworks

    Joined:
    Sep 8, 2014
    Posts:
    344
    It could be a stretch, but perhaps this occurred because you declared the variable outside for loop and reused the same variable? What you wrote is fine by all means since you always overwrite the reference with GetComponent<Toggle>(). But I had a similar problem with lambdas, where I thought the gameObject I retrieved in for loop was captured properly.

    Just curious for future reference, could you test the same code but a bit refactored like:
    Code (CSharp):
    1.  
    2.         float xPos = 0;
    3.         foreach (KeyValuePair<int,string> pair in retails)
    4.         {
    5.             Transform newToggleTransform = Instantiate(retailTogglePrefab, new Vector3(xPos, 0, 0), Quaternion.identity) as Transform;
    6.             newToggleTransform.SetParent(retailGroupPanel,false);
    7.          
    8.             Toggle newToggle = newToggleTransform.GetComponent<Toggle> ();
    9.             newToggle.isOn = false;
    10.             if (xPos == 0) {
    11.                 newToggle.isOn = true; // doesn't fire, as expected
    12.                 activateRetail(true,pair.Key); // so we call the callback manually. This works :)
    13.                 Debug.Log ("toggled because xPos == 0");
    14.             }
    15.  
    16.             newToggle.group = retailGroupPanel.GetComponent<ToggleGroup> ();
    17.  
    18.             Text newToggleText = newToggleTransform.GetComponent<Text> ();
    19.             newToggleText.text = pair.Value; // retailName
    20.  
    21.             ToggleId newToggleId = newToggleTransform.GetComponent<ToggleId> ();
    22.             newToggleId.toggleId = pair.Key; // retailId <-- do we even need this?
    23.  
    24.             RectTransform newToggleRect = newToggleTransform.GetComponent<RectTransform> ();
    25.  
    26.             int temp = pair.Key;
    27.             newToggle.onValueChanged.AddListener((value) => {
    28.                 Debug.Log ("listener fired, calling with "+temp.ToString()); // called only once, with the last key :(
    29.                 activateRetail(value,temp);
    30.             });
    31.  
    32.             xPos += newToggleRect.rect.width;
    33.         }
    and tell us if it makes any difference. Thanks!
     
    Undertaker-Infinity and BenZed like this.
  4. KelsoMRK

    KelsoMRK

    Joined:
    Jul 18, 2010
    Posts:
    5,539
    That is rather odd - especially considering the fact that KeyValuePair is a struct and your key and value are both primitive types.

    So you're saying that regardless of which toggle you click the message is the same?
     
  5. KyleStaves

    KyleStaves

    Joined:
    Nov 4, 2009
    Posts:
    821
    Are you doing this inside a coroutine by chance?
     
  6. Undertaker-Infinity

    Undertaker-Infinity

    Joined:
    May 2, 2014
    Posts:
    112
    I had tried that too and it didn't make a difference. By now I'm just curious too, I'm pretty sure I'll have to do this again at some point. Plus the workaround isn't very elegant, stomping all over the toggles while the event just calls the right one.

    yes. Even though I can see the number in their ToggleId (an empty component with just a public int) is different for each in the Inspector and I can see them toggling properly.

    Yes indeed. I read that it makes it more difficult and I did not find much more detail, let alone a solution.