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
– Abion47, Commented Oct 21, 2016 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
– turnipinrut, Commented Oct 21, 2016 at 19:17
Possible duplicate : stackoverflow.com/questions/40156493/… — Hellium
– Hellium, Commented Oct 21, 2016 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
– Abion47, Commented Oct 21, 2016 at 20:18
Thank you all for your help! Indeed, it was necessary to "temp" a variable "i" and after this use it. — Igor
– Igor, Commented Oct 22, 2016 at 16:19

Programmer · Accepted Answer · 2016-10-22 05:41:41Z

2

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));

answered Oct 22, 2016 at 5:41

Programmer

126k22 gold badges253 silver badges345 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Community · Accepted Answer · 2017-05-23 11:48:35Z

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, 2017 at 11:48

CommunityBot

11 silver badge

answered Oct 21, 2016 at 19:55

Hellium

7,3662 gold badges25 silver badges50 bronze badges

3 Comments

Programmer Over a year ago

Basically what you did is just copy my answer, rename the tempI variable to index then put it as an answer.

Hellium Over a year ago

Check the dates my friend ;)

Programmer Over a year ago

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.

Collectives™ on Stack Overflow

C# Unity Wrong argument of the function in onClick event

2 Answers 2

Comments

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related