Solution to button.onClick.AddListener() with indexed argument. Argument out of range exception

Recently I was fixing up my inventory system and the display that generates on screen when a user clicks an item which he has collected. However when I collected several items and attempted to select one from the inventory, either the wrong item was selected or I got an argument / index out of range / bounds exception.

Here was my code to generate clickable buttons based on what items were in inventory while also giving the buttons a different sprite when highligthed by mouse, programmatically (The code that caused exception) . Note line 31:

        int idxItemSprite = -1;
        // Items collected and sprites for U.I. display need to have the same enum name and sprite name
        // to be auto loaded when item aquired by player!

        for (int x = 0; x < _collectedItems.Count(); x++)
        {
            if (_collectedItems[x]._displayInUI)
            {
                if (_collectedItems[x]._displayedInUI == false)
                {
                    Button button;
                    GameObject itemBox = Instantiate(_itemBox) as GameObject;

                    for (int s = 0; s < _invSprites.Count; s++)
                    {
                        if (_collectedItems[x]._item.ToString() == _invSprites[s].name)
                        {
                            idxItemSprite = s;
                        }
                    }

                    button = itemBox.GetComponent<Button>();

                    //Handle highlights with SpriteState (create new temp variable to change values)
                    UnityEngine.UI.SpriteState spriteState = button.spriteState;
                    spriteState.highlightedSprite = _invSpritesHighlighted[idxItemSprite];
                    button.spriteState = spriteState;

                    //Add on Click call back method events
                    button.onClick.AddListener(() => InventoryItemClicked(_collectedItems[x]));
                    _buttons.Add(button);

                    itemBox.transform.SetParent(_invHorzGrp.transform, false);
                    itemBox.GetComponent<Image>().sprite = _invSprites[idxItemSprite];
                    itemBox.layer = 5;

                    _collectedItems[x]._displayedInUI = true;
                    _itemBoxes.Add(itemBox);
                }
            }
        }

Why? In the past this would piss me off and hold me up for a few days but I know better. _collectedItems[×] is an element in a List of Class called Item. Worse, onClick.AddListener is accepting an anonymous function as an argument WITH a reference type that’s a dynamically indexed element from a list.

What’s happening was since I was using an anonymous function and X as the index to specify to the callback method which element i wanted from my list of collected items, a pointer to the memory location for X is being passed to the onClick event, instead of a concrete value integer. Each iteration of the for loop, X changes… post hoc ergo propter hoc, the memory location to the onClick listener

To get around this I added a temp variable whose scope is only within the current iteration of the loop, and will not be modified. This way the index is a value type and doesn’t change.

Here is the working code:

        int idxItemSprite = -1;
        // Items collected and sprites for U.I. display need to have the same enum name and sprite name
        // to be auto loaded when item aquired by player!

        for (int x = 0; x < _collectedItems.Count(); x++)
        {
            if (_collectedItems[x]._displayInUI)
            {
                if (_collectedItems[x]._displayedInUI == false)
                {
                    Button button;
                    GameObject itemBox = Instantiate(_itemBox) as GameObject;

                    for (int s = 0; s < _invSprites.Count; s++)
                    {
                        if (_collectedItems[x]._item.ToString() == _invSprites[s].name)
                        {
                            idxItemSprite = s;
                        }
                    }

                    button = itemBox.GetComponent<Button>();

                    //Handle highlights with SpriteState (create new temp variable to change values)
                    UnityEngine.UI.SpriteState spriteState = button.spriteState;
                    spriteState.highlightedSprite = _invSpritesHighlighted[idxItemSprite];
                    button.spriteState = spriteState;

                    //Add on Click call back method events
                    Enums.CollectableItems itemEnum = _collectedItems[x]._item;
                    button.onClick.AddListener(() => InventoryItemClicked(itemEnum));
                    _buttons.Add(button);

                    itemBox.transform.SetParent(_invHorzGrp.transform, false);
                    itemBox.GetComponent<Image>().sprite = _invSprites[idxItemSprite];
                    itemBox.layer = 5;

                    _collectedItems[x]._displayedInUI = true;
                    _itemBoxes.Add(itemBox);
                }
            }
        }

Hope this can be useful to somebody else because it drove me up a wall for a while in the past.

3 Likes

All is true. In case someone is interested in the theory behind this, you should look for “variable capturing”. For example, here’s a post that discusses it: Lambda expressions - Lambda expressions and anonymous functions - C# reference | Microsoft Learn

1 Like

Here’s another example of Variable Capturing. Thanks for putting a name to it:

https://blogs.msdn.microsoft.com/matt/2008/03/01/understanding-variable-capturing-in-c/

It’s hilarious that this is 7 years later, but this literally saved my life. I was doing precisely the same thing trying to dynamically add listeners to buttons with a loop, since my friend and I are building an RPG that may have many skills (with corresponding buttons) in the character creation menu, and I realized that it was not capturing the index of i in my loop when I added the button’s listener, but when the lambda was called (after it had iterated all elements). This was useful to someone. Thanks for the post :). I literally logged into my Unity account to comment for the first time ever because this was so niche but so useful to know.