[1.12] Tile entity doesn't save itemstacks

[That_Martin_Guy]

Whenever I right click my custom block with an item I want to store it in its tile entity. When right clicked again it's supposed to give the itemstack to the player. For some reason it doesn't, however. It doesn't say it's empty unless I haven't given it an item, like it is supposed to. It doesn't log the itemstack either. What is the problem here?

[V0idWa1k3r]

cards.add(card);
card.shrink(1);

If your stacksize is 1 this will effectively add an empty itemstack to the list. 

 

ItemStackHelper.loadAllItems(compound, cards);

private NonNullList<ItemStack> cards = NonNullList.create();

ItemStackHelper only puts the deserialized ItemStack into the list if the slot of the itemstack is greater or equals to 0 and less than the size of the list passed. As the size of your list is always 0 at TE init this method will never load your items into the list when you re-enter the world. Vanilla uses fixed size NonNullLists(there is a method in the NonNullList to create one). If you do not want your list to have a fixed size you will need to create your own deserialization method.

[That_Martin_Guy]

I made the size fixed since I eventually want to give it a chest-like gui, but now it crashes the game when I try and add an item to it. Updated TE

[V0idWa1k3r]

If your list has a fixed size then adding/removing things from it will indeed throw an exception as it's delegate list will be a Arrays.ArrayList(not to be confused with java.util.ArrayList) which is essentially a wrapper around an array. It is not designed to resize an array so it throws an exception. The reason it was not crashing wit a list of not fixed size is because that constructor sets an empty java.util.ArrayList as it's delegate.

If you are using an array list of a fixed size you must use it's set method to modify any of it's elements.

Edited July 11, 2017 by V0idWa1k3r

[That_Martin_Guy]

I tried to update it, but now it always says it's empty.

[V0idWa1k3r]

if(cards.get(i) == ItemStack.EMPTY)

Don't compare it to an empty stack directly, use ItemStack::isEmpty. Same goes for your getRandomCard and getCardAmount methods.

 

In your addCard method you actually never stop your for loop and end up insterting your card into every possible list position that is not empty.

 

Your getCardAmount actually does the opposite of what it is supposed to do.

[That_Martin_Guy]

2 minutes ago, V0idWa1k3r said:

In your addCard method you actually never stop your for loop and end up insterting your card into every possible list position that is not empty.

 

Your getCardAmount actually does the opposite of what it is supposed to do.

Is this because I use the == operator instead of isEmpty? If not, what is the cause of it? Also, why does the following code crash the game? Sorry for so many questions, but I'm a bit stressed and need to go to sleep very soon.

 

    public int getCardAmount()
    {
        int i = 0;
        while(cards.get(i).isEmpty() && i < cards.size() - 1)
        {
            i++;
        }
        return i;
    }

 

[V0idWa1k3r]

23 minutes ago, That_Martin_Guy said:

Is this because I use the == operator instead of isEmpty? If not, what is the cause of it?

Why are you never breaking your loop is a question for you, the equality check is not responsible for it.

The getCardAmount currenty checks if the itemstack in a slot is an empty stack and increments the counter. It should do the opposite though considering it's usage.

 

I am not sure why the code you've posted crashes the game. The cards list is not null, it can't contain a null element and you are not going out of bounds. Additionally I am unable to encounter any exception with this method. What is the exception exactly?

It still will only count empty items though. And will stop counting them as soon as it hits a non-empty item.

 

Edit: after a bit of debugging I found why your items are never added to the box. You are still shrinking the itemstack, just this time in your block code. You are still effectively adding an empty stack to the list. Adding a copy of the stack into the list instead of the stack itself fixes the issue and cards can be inserted/extracted just fine after fixing the getCardAmount method that is. I also could not get any exceptions even without the method not being fixed so I really do not know what that was about.

 

 

 

Edited July 11, 2017 by V0idWa1k3r

[That_Martin_Guy]

The game is crashing with an ArrayIndexOutOfBoundsException. 27 is too much(?) apparently. I also tried this code, but it also resulted in a similar crash

   public int getCardAmount()
    {
        int i = cards.size();
        while(i > 0)
        {
            if(cards.get(i).isEmpty())
            {
                i--;
            }
        }
        return i;
    }

 

[Animefan8888]

39 minutes ago, That_Martin_Guy said:

The game is crashing with an ArrayIndexOutOfBoundsException. 27 is too much(?) apparently. I also tried this code, but it also resulted in a similar crash

   public int getCardAmount()
    {
        int i = cards.size();
        while(i > 0)
        {
            if(cards.get(i).isEmpty())
            {
                i--;
            }
        }
        return i;
    }

 

Arrays index at zero.

[V0idWa1k3r]

4 hours ago, That_Martin_Guy said:

int i = cards.size(); while(i > 0) { if(cards.get(i).isEmpty())

This will surely crash you with an ArrayIndexOutOfBounds as i equals to the size of the array. Honestly I would say to drop this while loop completely here and instead either use streams

return this.cards.stream().filter(e -> !e.isEmpty()).count();

Or a foreach loop

int i;
for (Card c : this.cards)
{
    if (!c.isEmpty())
    {
        ++i;
    }
}

return i;

 

Edited July 12, 2017 by V0idWa1k3r
Fixed wrong quote

[Animefan8888]

1 minute ago, V0idWa1k3r said:

This will surely crash you with an ArrayIndexOutOfBounds as i equals to the size of the array. Honestly I would say to drop this while loop completely here and instead either use streams

return this.cards.stream().filter(e -> !e.isEmpty()).count();

Or a foreach loop

int i;
for (Card c : this.cards)
{
    if (!c.isEmpty())
    {
        ++i;
    }
}

return i;

 

Why did you quote me?

[V0idWa1k3r]

Oh, sorry, I haven't notice that I am quoting a quote in your message, derp. 

Edited July 12, 2017 by V0idWa1k3r

[That_Martin_Guy]

Thanks a ton guys! It works properly now. FYI, I used the foreach loop voidwalker commented.