lambda in foreach is capturing loop variable DESPITE LOCAL COPY

Unity Engine
1

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:

        Transform newToggleTransform;
        RectTransform newToggleRect;
        Toggle newToggle;
        Text newToggleText;
        ToggleId newToggleId;
        float xPos = 0;
        foreach (KeyValuePair<int,string> pair in retails) {
            newToggleTransform = Instantiate(retailTogglePrefab, new Vector3(xPos, 0, 0), Quaternion.identity) as Transform;
            newToggleTransform.SetParent(retailGroupPanel,false);
            newToggle = newToggleTransform.GetComponent<Toggle> ();
            newToggle.isOn = false;
            if (xPos == 0) {
                newToggle.isOn = true; // doesn't fire, as expected
                activateRetail(true,pair.Key); // so we call the callback manually. This works :)
                Debug.Log ("toggled because xPos == 0");
            }
            newToggle.group = retailGroupPanel.GetComponent<ToggleGroup> ();
            newToggleText = newToggleTransform.GetComponent<Text> ();
            newToggleText.text = pair.Value; // retailName
            newToggleId = newToggleTransform.GetComponent<ToggleId> ();
            newToggleId.toggleId = pair.Key; // retailId <-- do we even need this?
            newToggleRect = newToggleTransform.GetComponent<RectTransform> ();
            int temp = pair.Key;
            newToggle.onValueChanged.AddListener((value) => {
                Debug.Log ("listener fired, calling with "+temp.ToString()); // called only once, with the last key :(
                activateRetail(value,temp);
            });
            xPos += newToggleRect.rect.width;
        }
2

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

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(). 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:

        float xPos = 0;
        foreach (KeyValuePair<int,string> pair in retails) 
        {
            Transform newToggleTransform = Instantiate(retailTogglePrefab, new Vector3(xPos, 0, 0), Quaternion.identity) as Transform;
            newToggleTransform.SetParent(retailGroupPanel,false);
          
            Toggle newToggle = newToggleTransform.GetComponent<Toggle> ();
            newToggle.isOn = false;
            if (xPos == 0) {
                newToggle.isOn = true; // doesn't fire, as expected
                activateRetail(true,pair.Key); // so we call the callback manually. This works :)
                Debug.Log ("toggled because xPos == 0");
            }

            newToggle.group = retailGroupPanel.GetComponent<ToggleGroup> ();

            Text newToggleText = newToggleTransform.GetComponent<Text> ();
            newToggleText.text = pair.Value; // retailName

            ToggleId newToggleId = newToggleTransform.GetComponent<ToggleId> ();
            newToggleId.toggleId = pair.Key; // retailId <-- do we even need this?

            RectTransform newToggleRect = newToggleTransform.GetComponent<RectTransform> ();

            int temp = pair.Key;
            newToggle.onValueChanged.AddListener((value) => {
                Debug.Log ("listener fired, calling with "+temp.ToString()); // called only once, with the last key :(
                activateRetail(value,temp);
            });

            xPos += newToggleRect.rect.width;
        }

and tell us if it makes any difference. Thanks!

2 Likes
4

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

Are you doing this inside a coroutine by chance?

6

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.