Adding listeners to an array of buttons not working

Unity Engine
1

Hello there. I have searched for an answer to this but can’t find anyone with my specific problem.

I am populating an array of buttons in a vertical layout, the data coming from scriptable objects. I am trying to add unique listeners to each button. When I run the code, the buttons ALL have the information from the last scriptable object in the list. The buttons are supposed to add or subtract values depending on which one you pick.

Here is my code. I think the problem is with newObj.GetComponent.onclick.addlistener. It seems to be overriding all previous buttons with the latest version.

    void Populate()
    {
        GameObject newObj;
        int MoneyChange;
        int FoodChange;
        int HealthChange;

        for (int i = 0; i < EarnMoneyOptions.Count; i++)
        {
            newObj = (GameObject)Instantiate(EarnOption, transform);

            newObj.GetComponentInChildren<Text>().text = EarnMoneyOptions[i].description;
            MoneyChange = EarnMoneyOptions[i].MoneyChange;
            FoodChange = EarnMoneyOptions[i].FoodChange;
            HealthChange = EarnMoneyOptions[i].HealthChange;

            newObj.GetComponent<Button>().onClick.AddListener(() => ButtonClicked(MoneyChange, FoodChange, HealthChange));
            Debug.Log("Populate Script: " + EarnMoneyOptions[i].MoneyChange);


        }
    }

    void ButtonClicked(int MoneyChange, int FoodChange, int HealthChange)
    {
        Debug.Log(MoneyChange + " " + HealthChange);
        PlayerPrefs.SetInt("Health", PlayerPrefs.GetInt("Health") + HealthChange);
        PlayerPrefs.SetInt("Money", PlayerPrefs.GetInt("Money") + MoneyChange);
        PlayerPrefs.SetInt("Food", PlayerPrefs.GetInt("Food") + FoodChange);
    }
2

This is actually an extremely common pitfall when adding lambda listeners in a for loop. First, to fix it, simply remove these lines of code on lines 4-6 of your snippet:

int MoneyChange;
int FoodChange;
int HealthChange;

Instead, declare these variables inline in the for loop:

        for (int i = 0; i < EarnMoneyOptions.Count; i++)
        {
            newObj = (GameObject)Instantiate(EarnOption, transform);
            newObj.GetComponentInChildren<Text>().text = EarnMoneyOptions[i].description;
            int MoneyChange = EarnMoneyOptions[i].MoneyChange;
            int FoodChange = EarnMoneyOptions[i].FoodChange;
            int HealthChange = EarnMoneyOptions[i].HealthChange;
            newObj.GetComponent<Button>().onClick.AddListener(() => ButtonClicked(MoneyChange, FoodChange, HealthChange));
            Debug.Log("Populate Script: " + EarnMoneyOptions[i].MoneyChange);
        }

The problem is something called variable capture. When you set up the lambda it is not putting the current value of those variables into the lambda. It is actually using references to those variables. So all of your lambdas are referencing the same 3 reused variables, which will end up having the value they get during the final iteration of the for loop. By moving the declaration of the variables inside the loop, the variables are now individualized to each individual lambda with the actual value you want and not changing afterwards.

Just a small stylistic nit: typical convention for variable names is that local variables and parameter names should be camelCased, not TitleCased. That is of course completely up to you though.

5 Likes
3

Holy MOLY as easy as that, its solved. Thank you so much!! First try and its what I expected.

Your explanation makes sense, it felt like the reference to the value was getting passed on, not the value itself.

Many thanks :slight_smile:

1 Like
4

yeah lambda capturing is tricky. it’s easy to get lulled into thinking data are just magically accessible.
they’re accessible by reference. :slight_smile: