Onclick inside foreach loop only returns last iteration?

Unity Engine
1

I am instantiating buttons inside a foreach loop that represent the rooms available but when i try to add a onclick listener every button gets the last iterations onclick. Not sure why this is happening and could use some help understanding whats going on here. All of the other code works just fine its only the onclick code that is giving me issues.

using UnityEngine;
using System.Collections;
using UnityEngine.UI;

public class RoomList : MonoBehaviour {
    private GameObject roomButton;
    private Button b;
    public Text [] t; //Holds all texts in the gameobject roomButton and stores them in a array
    private Text roomNameText;
    private Text gamemodeText;
    private Text playerCountText;
    private int roomAmount = 0;
    private bool roomsLoaded = false;
   

   
    public void JoinRoom()
    {
        PhotonNetwork.JoinRoom (roomNameText.text);
    }
   
    public void OnReceivedRoomListUpdate()
    {
        foreach (RoomInfo room in PhotonNetwork.GetRoomList())
        {
            roomAmount++;

            if(roomsLoaded == false)
            {
                roomButton = Instantiate (Resources.Load ("_Prefabs/RoomButton") as GameObject);
                roomButton.transform.SetParent(GameObject.FindObjectOfType<Canvas>().gameObject.transform, false);
                b = roomButton.gameObject.GetComponentInChildren <Button> ();
                //Get all texts in roomButton and store them in t
                t = roomButton.gameObject.GetComponentsInChildren <Text> ();
               
                roomNameText = t [0];
                gamemodeText = t [1];
                playerCountText = t [2];
               
                roomNameText.text = room.name;
                gamemodeText.text = "Empty";
                playerCountText.text = room.playerCount + "/" + room.maxPlayers;
               
                b.onClick.AddListener(JoinRoom);
                   
                for(int i = 1; i < roomAmount; i++)
                {
                    roomButton.transform.position = new Vector3(roomButton.transform.position.x, roomButton.transform.position.y - 54, roomButton.transform.position.z);
                }
            }
        }

        roomsLoaded = true;
    }
}
2

Your JoinRoom method is dependent on the text in the roomNameText variable. Since you update that in every run of the foreach loop, every button is going to call JoinRoom with that variable set to the last value it was given in the loop.

3

Even if i do this i get the same thing happening? Im confused as to how i could do this properly?

                b.onClick.AddListener(() => {
                //Handle Button Click
                PhotonNetwork.JoinRoom (room.name);
              
                 });
4

Ah! Now you ran into the most common issue with writing delegates within for loops. There’s a writeup of why this happens here, which I recommend that you read.

Note that this is going to change in C# 5, which Unity will probably move to sometime in the 2080’s.

The fix is to simply create a local copy of the variable:

string roomName = room.name;
b.onClick.AddListener(() => {
    //Handle Button Click
    PhotonNetwork.JoinRoom (roomName);
});
3 Likes
5

Interesting read. Thank you very much!