0

Suppose I have a canvas in Unity and I want to create 5 buttons in it. Every time I press a button, it will state This is button n.i, where i goes from 0 to 4. This is the code I've been using so far:

public Button buttonPrefab; //set from inspector
void SetButtons()
{
    Button newObj;
    for (int i = 0; i < 5; i++)
    { //loop 5 times, creates a button using buttonPrefab and assign ButtonNumber() method to it
        newObj = Instantiate(buttonPrefab, transform);
        newObj.onClick.AddListener(delegate { ButtonNumber(i); });
    }
}
void ButtonNumber(int nButton)
{
    Debug.Log($"This is button n.{nButton}");
}

I use onClick.AddListener to assign a Method to the button, and since the Method requires argument I've been passing them using delegate.

The problem is that, for every button I press, the result is always This is button n.5.

All buttons always seem to be updating according to the latest value of i, even though I'd want the Methods to save the i value at the time it was assigned.

I could make the script work by using this script:

public Button buttonPrefab; //set from inspector
void SetButtons()
{
    Button newObj;
    for (int i = 0; i < 5; i++)
    {
        newObj = Instantiate(buttonPrefab, transform);
        switch (i)
        {
            case 0:
                newObj.onClick.AddListener(delegate { ButtonNumber(0); });
                break;
            case 1:
                newObj.onClick.AddListener(delegate { ButtonNumber(1); });
                break;
            case 2:
                newObj.onClick.AddListener(delegate { ButtonNumber(2); });
                break;
            case 3:
                newObj.onClick.AddListener(delegate { ButtonNumber(3); });
                break;
            case 4:
                newObj.onClick.AddListener(delegate { ButtonNumber(4); });
                break;
        }
    }
}
void ButtonNumber(int nButton)
{
    Debug.Log($"This is buton n.{nButton}");
}

But I wouldn't call it a "smart" solution.

Is there anything I can do to make the buttons work in the first script?

man-teiv
  • 419
  • 3
  • 16

1 Answers1

1

Just change to:

  var x = i;
  newObj.onClick.AddListener(delegate { ButtonNumber(x); });

Voila.

What's going on is called "capturing" of the i variable, and you can find tons of questions and answers on it, for example: Captured variable in a loop in C#

Dan Byström
  • 9,067
  • 5
  • 38
  • 68
  • I feel so stupid for not having tried that! Thank you so much. – man-teiv Jun 24 '20 at 07:25
  • No need to. Bright programmers have even blogged about finding a c# bug when they encountered this for the first time! You're in good company! :-) – Dan Byström Jun 24 '20 at 07:27
  • I'm curious, is this an *intended* feature? It looks like it could lead to more errors than benefits. Where can it be beneficial? – man-teiv Jun 24 '20 at 07:32
  • 1
    Few things in this world is so thoroughly well-reasoned as the C# specifications, so yes, it is intended. And, yes, this has baffled many programmers. May I suggest this for further reading: https://ericlippert.com/2009/11/12/closing-over-the-loop-variable-considered-harmful-part-one/ – Dan Byström Jun 24 '20 at 07:40