- Home /
add button listener via for loop
I'm trying to add functionality to my buttons via for loop, because I don't want to assign them in inspector.
Here is what I'm trying to do:
for (int i = 0; i < players.Length; i++)
{
playersSpellers[i].spellOne_txt = spellOne_txts[i].GetComponent<Text>();
spellOnePlus_btns[i].GetComponent<Button>().onClick.AddListener(() => playersSpellers[i].AddRemoveSpell(0, adjustSpellCount));
}
And here is what I had to do in order to have it working in Unity
for (int i = 0; i < players.Length; i++)
{
playersSpellers[i].spellOne_txt = spellOne_txts[i].GetComponent<Text>();
}
spellOnePlus_btns[0].GetComponent<Button>().onClick.AddListener(() => playersSpellers[0].AddRemoveSpell(0, adjustSpellCount));
spellOnePlus_btns[1].GetComponent<Button>().onClick.AddListener(() => playersSpellers[1].AddRemoveSpell(0, adjustSpellCount));
spellOnePlus_btns[2].GetComponent<Button>().onClick.AddListener(() => playersSpellers[2].AddRemoveSpell(0, adjustSpellCount));
spellOnePlus_btns[3].GetComponent<Button>().onClick.AddListener(() => playersSpellers[3].AddRemoveSpell(0, adjustSpellCount));
spellOnePlus_btns[4].GetComponent<Button>().onClick.AddListener(() => playersSpellers[4].AddRemoveSpell(0, adjustSpellCount));
spellOnePlus_btns[5].GetComponent<Button>().onClick.AddListener(() => playersSpellers[5].AddRemoveSpell(0, adjustSpellCount));
I learned that it has to do something with issues with "loop closures" that are present in .NET v3.5 and that this was fixed in .NET v4.0 (and so my adding listeners using for loop would work as shown above)
I presume we won't be seeing Unity jump to .NET 4.0 anytime soon, or am I wrong? Is there a nicer solution at the moment (a workaround) to this?
Btw: this hasn't been "fixed" in the latest C# version as there's nothing to fix. Everything works as expected. What has changed is the behaviour of the "foreach loop". The behaviour of the for loop is still the same and is the expected behaviour. Even the behaviour of the foreach loop is not "an issue" as it worked exactly as it was specified. They just decided that it might be convenient for the foreach loop to get a new loop variable each iteration. So they actually changed the specification for this "new feature".
The problem is, here you create a closure:
AddListener(() => playersSpellers[i].AddRemoveSpell(0, adjustSpellCount));
A closure is an anonymous method that can "capture" variables from the current scope and use them later. Note that i said variables and not values. Each of your closure will close over the variable "i" (as well as the variable "playersSpellers" and "adjustSpellCount" but those are the same for each so it doesn't matter).
Every one uses the same variable. After the for loop is finished the variable "i" will have the value "players.Length" since that was the last value the for loop has checked when it ter$$anonymous$$ated. So whenever one of those anonymous methods get called, it will see the last value that "i" was set to and not what i was while the closure was created.
Like fafase said you simply have to declare the variable you want to close over inside the for loop body. That way, each iteration you get a "new" variable and each closure will use a different one. So if you look at his solution he created the variable "ps" and the closure will capture this "ps" variable each iteration. Since each iteration there's a new ps variable the content of the last one isn't changed. So when the callback is called it will still use the right value.
ps: Closures are actually not a .NET thing but something that the compiler implements. So in this case the C# compiler. Under the hood a closure is just a compiler generated class which the compiler instantiates when needed. Actually every anonymous method actually becomes a member of a hidden compiler generated class. In the end they are just the same kind of methods as every else.
pps: don't forget to upvote his answer and accept it if helped you to solve your problem. If it doesn't solve your problem you might create those in a coroutine which will make thiis solution fail.
You're using a lot of words I don't understand :) but that will change one day, I hope.
Answer by fafase · Dec 11, 2016 at 03:48 PM
for (int i = 0; i < players.Length; i++)
{
var ps= playersSpellers[i];
Text t = spellOne_txts[i].GetComponent<Text>();
ps.spellOne_txt = t;
var clickEvent = spellOnePlus_btns[i].GetComponent<Button>().onClick;
clickEvent.AddListener(() => ps.AddRemoveSpell(0, adjustSpellCount));
}
By using a local variables, you should be able to bypass the issue. At least, I do it like this and it works for me.
Thank you @fafase for providing such a simple and concise answer to a problem that I have been struggling with for months.
Answer by PaxForce · Dec 12, 2016 at 07:19 AM
I decided to accept fafase's answer, because the idea is good, even though fafase put into local variables members of spellOne_txts, that is not neccessary at all.
I formatted my code a bit more nicely :) so this is what I used:
for (int i = 0; i < players.Length; i++)
{
playersSpellers[i].spellOne_txt = spellOne_txts[i].GetComponent<Text>();
var tempPlayerSpeller = playersSpellers[i];
var tempSpellOnePlusButton = spellOnePlus_btns[i].GetComponent<Button>();
tempSpellOnePlusButton.onClick.AddListener(() => tempPlayerSpeller.AddRemoveSpell(0, adjustSpellCount));
}
Yes I just got taken away with making things local...over excitement...
Your answer
Follow this Question
Related Questions
Create DoTween from for loop? 1 Answer
Move Button OnClick listeners to another button 3 Answers
Functions aren't appearing in the onClick editor..... 1 Answer
Onclick.Addlistener issues in for statement. (also network related). 1 Answer
[Bug?] int parameter on dynamically added onClick-Event is wrong 1 Answer