C# Unity Wrong argument of the function in onClick event

Question

When I'm adding methods to onClick buttons the argument of the function is always const = array of buttons.length + 1. Where did I go wrong?

all_buttons not empty. I clicked on three different buttons. Unity log screenshot: link

Button[] all_buttons = GetComponentsInChildren<Button>();
for (int i = 0; i < all_buttons.Length; i++) {
    Debug.LogWarning(all_buttons[i]+" => addLoad with index "+ (m_LvlStartIndex + i));
    if (levelScript)
        all_buttons[i].onClick.AddListener(() => Load(m_LvlStartIndex+i));
}

public void Load(int level) {
    Debug.LogWarning("Loading "+level+" level...");
    Application.LoadLevel(level);
}

Update: change this

all_buttons[i].onClick.AddListener(() => Load(m_LvlStartIndex+i));

to

int tempI = i;
all_buttons[i].onClick.AddListener(() => Load(m_LvlStartIndex+tempI));

Thanks to all!!

You pass `m_LvlStartIndex + i` as a parameter in all the event handlers. By the time those handlers actually get called, `i` is equal to `all_buttons.Length`. So if `m_LvlStartIndex` is equal to 1, the parameter will be exactly what you're getting. — Abion47, Oct 21 '16 at 19:10
@Abion47 how come t works at all? i would expect it to throw an error because 'i' is gone after the for loop is finished right? — turnipinrut, Oct 21 '16 at 19:17
Possible duplicate : http://stackoverflow.com/questions/40156493/c-sharp-anonymous-function-scope-in-unity3d/40157738#40157738 — Hellium, Oct 21 '16 at 19:52
@turnipinindia It's because of closure. In a nutshell, because the event handlers are declared in the loop scope they are considered part of the scope, so `i` won't get disposed as long as the event handlers exist. — Abion47, Oct 21 '16 at 20:18
Thank you all for your help! Indeed, it was necessary to "temp" a variable "i" and after this use it. — Igor, Oct 22 '16 at 16:19

score 2 · Answer 1 · edited May 23 '17 at 11:48

2

You have a problem of closure (See What are 'closures' in C#?). Try this instead :

for (int i = 0; i < all_buttons.Length; i++) {
    int index = i ;
    if (levelScript)
        all_buttons[index].onClick.AddListener(() => Load(m_LvlStartIndex+index));
}

edited May 23 '17 at 11:48

Community

1
1

answered Oct 21 '16 at 19:55

Hellium

7,206
2
19
49

Basically what you did is just copy my answer, rename the `tempI` variable to `index` then put it as an answer. – Programmer Oct 24 '16 at 13:38
Check the dates my friend ;) – Hellium Oct 24 '16 at 15:51
I clicked on the oldest bar and my answer is the oldest. We will verify this in a couple of days when it starts to show the actual date and time. I will apologize if I am wrong. – Programmer Oct 24 '16 at 15:59

score 2 · Accepted Answer · answered Oct 22 '16 at 05:41

The problem is on this line of code:

all_buttons[i].onClick.AddListener(() => Load(m_LvlStartIndex+i));

You are supposed to save i to a temporary variable before using it with the AddListener function. The code below should fix it:

int tempI = i;
all_buttons[i].onClick.AddListener(() => Load(m_LvlStartIndex+tempI));

C# Unity Wrong argument of the function in onClick event

2 Answers2