koobas.hobune.stream
Index out of range when using delegates to set onClick at runtime
I am writing a script that passes 2 arrays into a menu creation method- the arrays hold the text and the models (which the buttons spawn) for the buttons that the menu will create. However, due to the fact that I would like to make the whole thing dynamic I need to set all the references to the gameobjects at runtime.
Below is the method which sets up the panel with buttons
public void Choice(string upgradeQuestion, /*List<UnityAction>*/ string[] upgradeText, GameObject[] buildings)
{
Button[] buttonList = null;
buttonList = new Button[upgradeText.Length];
modalPanelObject.SetActive(true);
for (int i = 0; i < upgradeText.Length; i++)
{
Button newbut = Instantiate(button);
Debug.Log(newbut);
newbut.transform.SetParent(buttonPanel.transform);
buttonList[i] = newbut;
//Remove previous listeners
buttonList[i].onClick.RemoveAllListeners();
//Add Panelclose listener
AddOnClickListener(buttonList[i], ClosePanel);
//Add Custom Listener
Debug.Log(buildings[i]);
buttonList[i].onClick.AddListener(delegate { BuildingPlacer(buildings[i]); });
buttonList[i].GetComponentInChildren<Text>().text = upgradeText[i];
Debug.Log("button created");
}
this.question.text = upgradeQuestion;
this.iconImage.gameObject.SetActive(false);
foreach (Button b in buttonList)
{
b.gameObject.SetActive(true);
}
}
The problem is that
buttonList[i].onClick.AddListener(delegate { BuildingPlacer(buildings[i]); });
returns an IndexOutOfRangeException when the buttons are clicked. The arrays are not empty and are of the same size, the buttons get created but the issue lies when the onclick is assigned.
Any idea why i am getting an index out of range
The whole loop is run to completion when you call this method, after that, at some point you press the button, and by then the value of i is upgradeText.Length because that's the exit condition for your loop.
When you use delegates or such in this manner, the variable i gets wrapped into the delegate rather than just its value, making it sort of act like a reference type.
It should be easily fixable by passing a variable you don't change later into the delegate.
var i2 = i;
buttonList[i].onClick.AddListener(delegate { BuildingPlacer(buildings[i2]); });
Thank you!!! I had the EXACT same problem. How did you know about that? I don't understand it at all.
$$anonymous$$y solution was to cache the needed parameter beforehand and pass it directly to the delegate, ins$$anonymous$$d of passing it using i.
Well, every programmer should know about closures and why closing over for loop variables is never a good idea. You may want to read Eric Lipperts post closing over the loop variable considered harmful. Eric was on the $$anonymous$$m who developed the $$anonymous$$S C# compiler.
Another reason you should know about this is because there are an endless stream of questions about that topic on SO but also here on UnityAnswers. Note that the $$anonymous$$ono C# compiler does has some additional issues when it comes to coroutines. See this question for more details. Using a local variable inside the for loop wouldn't work when the for loop is inside a coroutine because the mono compiler turns all local variables inside the coroutine into member variables of the internally created statemachine object. So creating a closure around such a variable will always close over the same variable. Using a nested lambda expression does work. Though i would generally recommend to avoid such cases. Those are the tiny bits you should know to not get caught in such issues ;)
Thank you very much Bunny83. This is very useful background knowledge. Also the extra bit about the coroutines.
So when you say you would generally avoid such cases, you were referring to the closure within Coroutines right? Not the closure in normal for loops?
What 'ovlay' and I are doing, i.e. assigning listeners to buttons in a for loop, that is generally okay using a local variable, isn't it?
$$anonymous$$aybe some day I will actually be able to call myself programmer, thanks to people like you!
