Jump to content

ItemStackHandler not a direct replacement for IInventory?


TheGreyGhost

Recommended Posts

Howdy all

 

I'm working on a Container based on a TileEntity and on the advice of more experienced folks than myself I have tried to avoid using MyTileEntity implements IInventory.

But I find that IItemHandler (ItemStackHandler) is not a direct replacement for IInventory because IInventory is also used by the Container to communicate with its parent TileEntity 

For example openInventory(), closeInventory(), isUsableByPlayer().

Is there a drop-in Forge replacement for those methods?

I can of course provide these to the Container as lambdas or a callback interface or give an Optional reference to the parent TileEntity directly, but it seems a bit clumsy.

-TGG

 

Link to comment
Share on other sites

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable.  If you think this is the case, JUST REPORT ME.  Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

 

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

 

DO NOT PM ME WITH PROBLEMS. No help will be given.

Link to comment
Share on other sites

Hi

Thanks for the replies

Yeah I just realised it's even worse than I thought because SlotItemHandler throws away / ignores the markDirty() notification, which vanilla appears to use to tell the TileEntity it needs to be saved/retransmitted to the client.

 

I think I'll just stick with the vanilla TileEntity implements IInventory, it appears to be simpler to write a few composition wrappers for an itemStackHandler held in my TileEntity than to tightly couple my TileEntity to the container and to write a customised Slot which handles markDirty() properly.

 

-TGG

 

 

 

Link to comment
Share on other sites

7 hours ago, TheGreyGhost said:

I can of course provide these to the Container as lambdas or a callback interface or give an Optional reference to the parent TileEntity directly, but it seems a bit clumsy.

If you haven't given your container a reference to the TileEntity I have only one question for you. Why? Also why would it be an Optional? Your container can't exist without the TileEntity right? At least in most cases. You may have a few that are reused.

6 hours ago, TheGreyGhost said:

than to tightly couple my TileEntity to the container and to write a customised Slot which handles markDirty() properly.

You don't need to do that. Just override ItemStackHandler::onContentsChanged to call TileEntity::markDirty(). Then just use SlotItemHandler for all your slot needs that are not limited.

VANILLA MINECRAFT CLASSES ARE THE BEST RESOURCES WHEN MODDING

I will be posting 1.15.2 modding tutorials on this channel. If you want to be notified of it do the normal YouTube stuff like subscribing, ect.

Forge and vanilla BlockState generator.

Link to comment
Share on other sites

Hi Anime

 

Thanks for the suggestions.

2 hours ago, Animefan8888 said:

If you haven't given your container a reference to the TileEntity I have only one question for you. Why? Also why would it be an Optional? Your container can't exist without the TileEntity right? At least in most cases. You may have a few that are reused.

You don't need to do that. Just override ItemStackHandler::onContentsChanged to call TileEntity::markDirty(). Then just use SlotItemHandler for all your slot needs that are not limited.

Re TileEntity can't exist with the TileEntity: yeah, I used to think that too, but after studying the vanilla code a bit more I realised that the TileEntity is only available on the server.  On the client side, the container does not get a TileEntity, it gets a temporary IInventory instead that isn't linked to any TileEntity.

 

Re override ItemStackHandler:  I don't think that ItemStackHandler::onContentsChanged is called every time that the Slot::onSlotChanged is called.  Many places in vanilla call Slot::onSlotChanged directly and it's not clear that ItemStackHandler::onContentsChanged would also be called in those situations (why would a manual call be needed if the other Slot methods had already marked as dirty?).  SlotItemHandler::onSlotChanged just discards the notification without passing it to ItemStackHandler.

 

Maybe it would be cleaner in some ways to separate a "notifications" interface from the more-general manipulation of a group of ItemStacks, or to code an adapter to convert ItemStackHandler to an IInventory with Notifications, but I'm thinking it really isn't worth the extra complexity.

 

-TGG

 

 

Link to comment
Share on other sites

11 minutes ago, TheGreyGhost said:

On the client side, the container does not get a TileEntity, it gets a temporary IInventory instead that isn't linked to any TileEntity.

What are you talking about. This is all about how you setup your containers. The client does have the TileEntities in the world so you can just get it from the world. If you create your ContainerType like this you have access to your TileEntity.

IForgeContainerType.create((id, playerInven, buffer) -> new MyContainer(id, inventory.player.world.getTileEntity(buffer.readBlockPos()), inventory)).setRegistryName("modid:my_container");

 

18 minutes ago, TheGreyGhost said:

I don't think that ItemStackHandler::onContentsChanged is called every time that the Slot::onSlotChanged is called.

It is called whenever an ItemStack inside the ItemStackHandler is changed from one to another or insert/extract has been called. However if you instead do something like ItemStackHandler::getStackInSlot().setTag(someNBTTag);

 

It will not be called in this case make sure to call it yourself.

VANILLA MINECRAFT CLASSES ARE THE BEST RESOURCES WHEN MODDING

I will be posting 1.15.2 modding tutorials on this channel. If you want to be notified of it do the normal YouTube stuff like subscribing, ect.

Forge and vanilla BlockState generator.

Link to comment
Share on other sites

Hi Anime

8 minutes ago, Animefan8888 said:

What are you talking about. This is all about how you setup your containers. The client does have the TileEntities in the world so you can just get it from the world. If you create your ContainerType like this you have access to your TileEntity.

IForgeContainerType.create((id, playerInven, buffer) -> new MyContainer(id, inventory.player.world.getTileEntity(buffer.readBlockPos()), inventory)).setRegistryName("modid:my_container");

 

Perhaps this page will explain what I mean better

https://greyminecraftcoder.blogspot.com/2020/04/containers-1144.html

 

I understand what you're saying about giving your client container a copy of the client TileEntity.  The problem is that Vanilla doesn't do that, probably for a good reason (i.e. you then wind up with two independent synchronisation methods trying to fight each other: the vanilla tileentity synchronisation in parallel with the container directly poking around in the client's contents.)  There's also no guarantee that the client side TileEntity will exist if the client and server worlds are slightly out of synch for some reason

 

13 minutes ago, Animefan8888 said:

It is called whenever an ItemStack inside the ItemStackHandler is changed from one to another or insert/extract has been called. However if you instead do something like ItemStackHandler::getStackInSlot().setTag(someNBTTag);

 

It will not be called in this case make sure to call it yourself.

Here's an example of what I mean, from Container::mergeItemStack:

               int maxSize = Math.min(slot.getSlotStackLimit(), stack.getMaxStackSize());
               if (j <= maxSize) {
                  stack.setCount(0);
                  itemstack.setCount(j);
                  slot.onSlotChanged();
                  flag = true;
               } else if (itemstack.getCount() < maxSize) {

It looks to me like this vanilla code is directly manipulating the ItemStack, not via the Slot methods for changing/insertion/deletion.  Hence the reason the vanilla code must call onSlotChanged directly.  Seems to me that relying on ItemStackHandler::onContentsChanged for this situation may lead to data loss or mis-synchronisation because onContentsChanged is not triggered and hence the TileEntity has been altered but not marked as dirty.

 

-TGG

 

 

 

 

Link to comment
Share on other sites

1 hour ago, TheGreyGhost said:

There's also no guarantee that the client side TileEntity will exist if the client and server worlds are slightly out of synch for some reason

Then that is a problem in it of itself. Solve the root of the problem not the visible one.

1 hour ago, TheGreyGhost said:

It looks to me like this vanilla code is directly manipulating the ItemStack, not via the Slot methods for changing/insertion/deletion.  Hence the reason the vanilla code must call onSlotChanged directly.

Actually even this is not a problem both experimentally and in the code. Because when a slot is clicked while it does call mergeItemstack later on it still calls Container::detectAndSendChanges on ServerPlayerNethandler 1201.

Edited by Animefan8888

VANILLA MINECRAFT CLASSES ARE THE BEST RESOURCES WHEN MODDING

I will be posting 1.15.2 modding tutorials on this channel. If you want to be notified of it do the normal YouTube stuff like subscribing, ect.

Forge and vanilla BlockState generator.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.



×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.