- Home /
For() loop and AddListener() (already tried temporary variables...)
Hi! I'm trying to make a "list" of ui buttons; I create them and add the click event in a for loop:
for (int i = 0; i<profiles.Count; i++) {
string _tempString = profiles[i];
Debug.Log (_tempString);
GameObject button = (GameObject)Instantiate (Resources.Load ("ProfileButton"));
button.name = profiles[i];
Button _tempButton = button.GetComponent<Button>();
_tempButton.onClick.AddListener(() => GameManager.Profiles.SetCurrentProfile (_tempString));
}
The Debug.Log print the correct string, buttons have the correct name, however, when I click on any button it sets the same profile (the last in profiles List)
I'm puzzled.
If anyone has an idea.. :)
Thanks in advance!
To clarify, this will NOT work with a foreach
loop. I'm not sure why, but if someone could explain it that would be great!
Answer by Souk21 · Mar 22, 2015 at 09:03 PM
Apparently the problem was that the code was run inside a Coroutine. I changed for a public void() and now it works.
The parameter you pass with the function (_tempstring in this case) $$anonymous$$UST be local scope (aka declared inside the loop).
If it is outside the loop it will utilize the same reference for all of the Listeners and that reference will be set to the last value set in the loop.
"_tempString" is declared inside the for loop. However the problem here was that all that code was inside a coroutine. local variables inside a coroutine behave way different compared to normal methods. Coroutines (or generator methods) are translated into a statemachine class. So local variables become member variables of the statemachine. That's why declaring it "see$$anonymous$$gly local" doesn't work. A coroutine is already a closure.
It's actually a bug in / misbehaviour of the mono C# compiler. $$anonymous$$icrosoft's C# compiler only makes those variables member variables that need to stay between yield return statements.
Thank you, this solved my issue! Unity has some very frustrating quirks with object references...
Well, that's not Unity's fault ^^. As i said this is a bug in the $$anonymous$$ono compiler.
Answer by kozlice · Jun 20, 2018 at 07:56 AM
Okay, this is an old question, but it comes out in Google, so I think a better explanation won't hurt.
This is not a bug. It might be counter-intuitive, but every language with async capabilities will behave the same way. Here's the sample program:
using System;
using System.Threading.Tasks;
class Script
{
static public void Main(string[] args)
{
for (int i = 0; i < 5; i++) {
Console.WriteLine("Counter value: {0}", i);
}
Console.WriteLine("---");
for (int i = 0; i < 5; i++) {
Task.Run(() => Console.WriteLine("Counter value: {0}", i));
}
// Wait for user input so the program is not terminated
Console.ReadLine();
}
}
The output will be:
Counter value: 0
Counter value: 1
Counter value: 2
Counter value: 3
Counter value: 4
---
Counter value: 5
Counter value: 5
Counter value: 5
Counter value: 5
Counter value: 5
Why is that? When a for
loop is executed, variable i
is allocated on the stack, and it's incremented on every pass. In the first case, when we use it immediately, the current value of i
is used (and the variable itself is still kept on the stack).
Now, if you declare an action that will take place somewhere in the future (timeout, event listener, whatever), and this action will make use of i
, system will move i
to the heap and it will be accessed in async code by reference (i.e. address in memory). All 5 handlers declared in this loop will reference the same i
, which will have the value of 5
by the end of the loop (it's incremented from 4 to 5, thus stops satisfying condition i < 5
and only then loop breaks).
This might not be what you expected, but this is what it must be.
Now, the answer to the question "what do I do to have a distinct value in every async task" is simple: you need to copy the value of i
outside the async code on every iteration:
for (int i = 0; i < 5; i++) {
var j = i;
Task.Run(() => Console.WriteLine("Counter value: {0}", j));
}
A new variable j
will be created on every pass now, and its value will be the copy of current value of i
(note: this only works for scalars, i.e. integers, floats/doubles, booleans; if i
was an object, you would need to do var j = i.Clone()
, otherwise j
will be merely a reference to i
). The output in this example will be something like this:
Counter value: 1
Counter value: 3
Counter value: 2
Counter value: 0
Counter value: 4
Hope this helps.
While your explanation is mostly correct it doesn't answer the question here. As you can see in the code in question and read in the title he already uses a temporary local variable and it still doesn't work. That's because he hasn't mentioned the important detail that the code snippet is inside a coroutine / IEnumerator.
Btw: Stack memory can't be kept for later usage. That's why "i" is not allocated on the stack when you use it in a closure. Ins$$anonymous$$d a closure object is created on the heap which is used ins$$anonymous$$d.
The actual problem here is that the coroutine code is translated into a statemachine object and all local variables will be turned into member variables of the statemachine class. Therefore it's impossible to actually use a local variable in another closure inside a loop when that code is inside a coroutine.
Finally keep in $$anonymous$$d that a closure will always closes over variables, never over values. So it's irrelevant if the variable type is a value type or a reference type.
Reference/value types matters here becauseAs you can see in the code in question and read in the title he already uses a temporary local variable and it still doesn't work Finally keep in $$anonymous$$d that a closure will always closes over variables, never over values. So it's irrelevant if the variable type is a value type or a reference type.
_tempString
, despite being local, is not a "copy of profiles[current-value-of-i]", it's "a reference to profiles[whatever-is-now-in-heap-allocated-i]". If it was
var j = i; string _tempString = profiles[j];
, there would be 5 different integers with distinct values on the heap (one per coroutine), and it would've worked.
Well, that's what I said in the answer ¯_(ツ)_/¯Btw: Stack memory can't be kept for later usage. That's why "i" is not allocated on the stack when you use it in a closure.
I think you're confusing the variable itself with the content it holds. Yes _tempString is a reference type. Therefore when you assign something to that variable you will assign a reference to a string object to that variable. However closures close over the variable not over the variable content. So from the point of view of a closure it always deals with a reference. A reference to the memory location where the variable is stored. Again, what the variable containst is irrelevant for the closure problem.
If you actually want to pass a reference or a copy is something completely different and has nothing to do with this issue in general. Btw there is no generic way to "clone" or copy a reference type.
Apart from all this you still haven't understood the real issue in the question. The actual issue is the $$anonymous$$ono C# compiler and the way it implements iterator blocks. The $$anonymous$$S compiler can handle the above mentioned problem "correctly" though it highly depends on the further usage of the variables. The $$anonymous$$ono compiler generally translates all local variables of an iterator block into member variables of the statemachine class that is generated behind the scenes. Therefore you do not get a new variable / closure context each iteration. There is no way to get the above code working with the $$anonymous$$ono compiler besides running it outside a coroutine. The quick and easy fix is creating the closure inside a normal method which you call from your coroutine. That way you will allocate the variable inside the scope of the normal method and get the proper closure context.
Finally this makes no difference at all:
var j = i;
string _tempString = profiles[j];
As the closure does not capture the variable i or j but only the variable _tempString. The solution would be a method like this:
void AddListener(Button but, string profile)
{
but.onClick.AddListener(() => Game$$anonymous$$anager.Profiles.SetCurrentProfile (profile));
}
And inside the coroutine just do
AddListener(_tempButton, profiles[i]);
Your answer
Follow this Question
Related Questions
How can i get the coordinates of the point on an object I click on? 1 Answer
OnClick, parameter setup in editor becomes null?? 1 Answer
OnClick button does not work? Trying to make a pause menu 2 Answers
Two OnClicks on one button. One doesnt work all the time. 2 Answers
On Click GUI texture 0 Answers