Search Unity

Resolved IndexOutOfRangeException: Index was outside the bounds of the array.

Discussion in 'Scripting' started by Jacktardean, May 3, 2023.

  1. Jacktardean

    Jacktardean

    Joined:
    May 3, 2023
    Posts:
    5
    Hi,

    I'm pretty new to this so you'll have to bare with

    I'm trying to get my hotbar to update to show each line of my inventory when a key is pressed. The inventory is 36 slots with the hotbar being 12.

    The hotbar item slot is updated to the item within the item slot which the index is increased by 12 for each refresh.

    The actual functionality works I just get the above error along with it

    Code (CSharp):
    1. public void ToggleHotbar()
    2.     {
    3.         for (int i = 0; i < itemSlots.Length; i++)
    4.         {
    5.             if (itemSlots[i] != null && increaseHotbar <=24)
    6.             {
    7.                 HotBarSlot[i].Item = itemSlots[i+ increaseHotbar].Item;
    8.             }
    9.             else
    10.             {
    11.                 increaseHotbar = 0;
    12.                 HotBarSlot[i].Item = itemSlots[i + increaseHotbar].Item;
    13.             }
    14.         }
    15.     }

    Code (CSharp):
    1.  private void Update()
    2.     {
    3.         for (int i = 0; i < toggleInventoryKeys.Length; i++)
    4.         {
    5.             if (Input.GetKeyDown(toggleInventoryKeys[I]))
    6.             {
    7.                 inventoryGameobject.SetActive(!inventoryGameobject.activeSelf);
    8.  
    9.                 break;
    10.             }
    11.         }
    12.  
    13.         for (int i = 0; i < toggleHotbarKeys.Length; i++)
    14.         {
    15.             if (Input.GetKeyDown(toggleHotbarKeys[I]))
    16.             {
    17.                 for (int b = 0; i < 3; b++)
    18.                 {
    19.                     inventory.increaseHotbar += 12;
    20.                     inventory.ToggleHotbar();
    21.                 }
    22.                 break;
    23.             }
    24.         }
    25.     }[/I][/I]
     
    Last edited: May 4, 2023
  2. Reaktion

    Reaktion

    Joined:
    Nov 8, 2019
    Posts:
    53
    Hi,

    If your array of itemSlots is bigger than your array of HotBarSlot, then you're gonna get an error trying to access HotBarSlot.Item if you're looping from 0 to itemSlots.Length. (line 7.)

    I don't really know what you're trying to do with your for loop from 0 to 3 inside the update function, to cycle through an other loop frop 0 to itemSlots.Length, but I think you may find your way using the modulo operator.
    https://learn.microsoft.com/dotnet/csharp/language-reference/operators/arithmetic-operators

    This can be a way for you to cycle through 36 iterations, and to never access outside of your array (of size 12) by using
    HotBarSlot[i % HotBarSlot.Length].Item


    There are other ways to prevent this type of error, but you'll have to think from your initial problem to find the best solution for you.
     
  3. Bunny83

    Bunny83

    Joined:
    Oct 18, 2010
    Posts:
    4,011
    It's hard to tell what the code actually is supposed to do. You have some strange things going on which are hard to follow. For example what is the uppercase "I" in this line?
    Code (CSharp):
    1.  if (Input.GetKeyDown(toggleInventoryKeys[I]))
    is that actually a lower case "i"? Since there are those BB code fragments at the end:
    }[/I][/I]
    it may just be a formatting issue here in the forum? Anyways, you probably should edit your post and fix that. At the moment it's really confusing.

    Apart from that you have a glaring logical issue in your code. Specifically the combination of
    Code (CSharp):
    1. for (int i = 0; i < itemSlots.Length; i++)
    and
    Code (CSharp):
    1. itemSlots[i+ increaseHotbar]
    This code will ALWAYS cause an IndexOutOfBounds exception whenever "increaseHotbar" is greater than 0. So what do you think this is supposed to achieve? The for loop variable iterates through all valid indices of the itemSlots array. So adding or removing something to / from that value will of course result in invalid indices. I guess you actually wanted to iterate through the hotbar slots and not through all slots of your inventory? So your for loop should look like this:

    Code (CSharp):
    1. for (int i = 0; i < HotBarSlot.Length; i++)
    Another huge issue is this for loop:

    Code (CSharp):
    1.  for (int b = 0; i < 3; b++)
    You wrongly check the "i" variable instead of "b". That means that the loop either doesn't run at all (if i is 3 or greater) or you're stuck in an infinite loop since i does not change inside that loop.

    I would also suggest that you put a bit more efford in naming things. A method called "ToggleHotbar" would suggest that it toggles something. However calling it twice in a row does not really change anything. This is more like an UpdateHotbar. Your "increaseHotbar" variable is also misleading as nothing is increased here. This is essentially an offset. So it indicates at which inventory slot your hotbar currently should start.

    So I would suggest:
    Code (CSharp):
    1.  
    2.         public void UpdateHotbar()
    3.         {
    4.             if (hotbarOffset > 24)
    5.                 hotbarOffset = 0;
    6.             for (int i = 0; i < HotBarSlot[.Length; i++)
    7.             {
    8.                 if (itemSlots[i] != null)
    9.                     HotBarSlot[i].Item = itemSlots[i+ hotbarOffset].Item;
    10.             }
    11.         }
    and

    Code (CSharp):
    1.  
    2.     private void Update()
    3.     {
    4.         for (int i = 0; i < toggleInventoryKeys.Length; i++)
    5.         {
    6.             if (Input.GetKeyDown(toggleInventoryKeys[i]))
    7.             {
    8.                 inventoryGameobject.SetActive(!inventoryGameobject.activeSelf);
    9.                 break;
    10.             }
    11.         }
    12.         for (int i = 0; i < toggleHotbarKeys.Length; i++)
    13.         {
    14.             if (Input.GetKeyDown(toggleHotbarKeys[i]))
    15.             {
    16.                 inventory.hotbarOffset += 12;
    17.                 inventory.UpdateHotbar();
    18.                 break;
    19.             }
    20.         }
    21.     }
    Note that when you have more than 2 states you usually don't use "toggle" but "cycle" (through). I would highly recommend to refactor your code more often. Whenever you see a misnamed thing, you should fix it right away. Code just gets harder to read and understand if things are named confusingly or wrong. Code always change over time and the purpose of a variable of a method may also change. This may require to give it a new name that actually reflects its purpose.

    ps: That hardcoded
    += 12
    is also bad. It assumes a fix hotbar size of 12. You may use
    += HotBarSlot.Length
    instead. Also if a "line" of the inventory is always the same length as the hotbar, the offset may just be the "line index" (0, 1, 2) and gets multiplied by the hotbar length only when actually used as offset. Currently you could set the offset to for example "1" which would just shift the hotbar by 1. Maybe that's what you actually want, so the hotbar could scroll horizontally through the whole inventory. However it would be quite uncommon. Though it's up to you.
     
  4. Jacktardean

    Jacktardean

    Joined:
    May 3, 2023
    Posts:
    5


    Hello and thank you so much for the detailed response!

    I've read through your comments and it all makes sense, I apologise for any confusion caused. I've gone through the code and taken your suggestions, especially around the naming of things as I agree that can make things tidier

    It now works perfectly without the errors after changing

    Code (CSharp):
    1. for (int i = 0; i < itemSlots.Length; i++)
    to
    Code (CSharp):
    1. for (int i = 0; i < hotBarSlot.Length; i++)
    Really appreciate your help!
     
    Reaktion likes this.