NeoForge icon indicating copy to clipboard operation
NeoForge copied to clipboard

[1.21] Storage Rework (ItemHandler, FluidHandler, ItemContext for item caps)

Open CodexAdrian opened this issue 1 month ago • 15 comments

The Big Storage Rework!

Motivation

NeoForge's storage situation is not great. ItemHandler and FluidHandler are both extremely helpful to devs trying to create a unified standard for storage of resources, but aren't consistent with each other and rely on the usage of mutable resources with amounts attached for transfer. This creates situations where devs have to return copies of their resources in order to safely ensure a dev isnt trying to manipulate their storage directly, or copy the stack passed into the insert method before its inserted in order to ensure the implementor of insert method doesn't attempt to modify the stack when its inserted. Not to mention the inconvenient issues that arise when the resource (item + components) are joined in the same object where so much copying and mutation needs to occur to the stack when most of the time the developer just wants to change the amount the stack contains or insert the same stack into a container with a different amount.

The ideal solution to these problems would be creating a single unified storage class (for consistency) that uses immutable resources (for the mutation problems) whose count is separated from the resource (for the inconvenience problems).

Implementation

Much of the implementation was inspired by https://github.com/FabricMC/fabric/pull/1356 and how it was subsequently built out in the later years, and is a continuation of Tech's initial PR to introduce immutable resources in https://github.com/neoforged/NeoForge/pull/715

IStorage

IStorage is a new interface that will eventually fully replace usages of IItemHandler and IFluidHandler. It is a slotted container that stores an integer amount of a resource of type T. This type doesn't necessarily have to be an IResource class, but utilities will be provided for storage classes that do. In this implementation, T should be immutable and should be countless, as the amount is retrieved in a separate getAmount(slot) method.

IResource and FluidResource/ItemResource

IResource is the basis for the Neo provided storage capabilities. Its an immutable data component holder with a utility methods that return a new resource with modified components without mutating the resource to make it easy to interact with. Lastly, it contains an isBlank method to indicate whether or not the resource should be considered blank or empty, like if the resource is of Items.AIR or Fluids.EMPTY. The default resources provided by Neo are FluidResource and ItemResource and represent immutable types of a fluid/item and data components.

ResourceStack

ResourceStack is a utility class that contains an instance of IResource and an integer amount. It provides the same utility methods IResource methods provides for copying with modifications while also providing similar utilities for amount based copying. ResourceStack should largely replace the usage of ItemStack or FluidStack in the usage of code.

IItemContext

ItemContext is a new interface for general usage when interacting with inventories. Its centered around a main slot, whose contents IItemContext can be used to get its current state (resource and amount). This context also allows devs to manipulate the ItemResource and amount in the main slot, by extracting, inserting or exchanging items into it. Extracting will extract items of a given ItemResource and amount from the slot, returning how much was extracted from the main slot. Inserting into the context will not only insert into the main slot, but is also intended to insert whatever does not fit into the main slot into some kind of outer container. This can be an outer container like a buffer or a context could choose to drop the excess onto the ground, its up to the implementor of IItemContext to say where items go. Exchanging allows devs to extract however many items that are requested to be exchanged and then inserting a new resource in its place up to the amount extracted. Devs should also dump excess items that were not successfully inserted into the main slot into some kind of overflow as well (if you choose to support having an overflow for your items).

This new class largely replaces the need for any item specific capabilities, like IFluidHandlerItem, while also providing devs with much needed utility to insert excess into a slot other than itself. This should make it possible to, for example, have a stacked fluid container where only 1 of 16 items are drained. The fluid storage can simply call exchange with 1 empty storage container, without needing to worry about whether or not the stack is of stack size 1.

ItemContext also contains a helpful utility which you can use to retrieve the capability found on item in the main slot without needing to pass in an ItemStack

The Fallen

This PR also sets IItemHandler and IFluidHandler for deprecation and marked them for removal in 1.22. Currently these classes have been retrofit to extend IStorage<ItemResource> and IStorage<FluidResource> respectively, and will eventually be phased out entirely for just using the base storage classes. While consideration to keeping some of the old API around has been made, this PR does still make a breaking change in the removal of IFluidHandlerItem, and the modification of existing Item capabilities to have the aforementioned IItemContext as its new context class (which also replaces the need of having the getContainer method in IFluidHandlerItem).

Interaction changes

With immutable resources and IStorage<ItemResource> and IItemContext, its safe to say that modifying the ItemStack in which a capability was retrieved from will not always achieve the desired effect. As such, developers should now take care to use ItemContext to make any modifications needed to the main stack using the methods provided in ItemContext, even if those changes are just component or size related changes. Developers should treat that ItemStack given to them in the capability provider as read only, and should also take care to note it is only accurate at the time of creation of the cap class, and may not be updated to reflect changes made with IItemContext. Devs should use the resource and amount provided in IItemContext as an accurate image of what the item in its slot currently looks like

Notes

This was a doozy to write, and might not be a complete overlook of this PR when it is time to merge it. I'll do my best to update the description as needed. This PR is (as of May 8th, 2024) not complete, but was opened so others could see what the changes I was making were and could comment on them while in dev.

CodexAdrian avatar May 09 '24 03:05 CodexAdrian

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar May 09 '24 03:05 CLAassistant

  • [ ] Publish PR to GitHub Packages

@CodexAdrian isBlank -> isEmpty?

Spinoscythe avatar May 09 '24 17:05 Spinoscythe

@CodexAdrian isBlank -> isEmpty?

Yeah i can make that change

CodexAdrian avatar May 10 '24 00:05 CodexAdrian

I disagree. isBlank was chosen for immutable resources because it does not have any connotation of an "amount" with it.

Technici4n avatar May 10 '24 09:05 Technici4n

I disagree. isBlank was chosen for immutable resources because it does not have any connotation of an "amount" with it.

isAbsent maybe then?

Spinoscythe avatar May 10 '24 15:05 Spinoscythe

isBlank is better than isAbsent, I do personally think isEmpty works best for Minecraft related content. It's what Mojang uses for the empty fluid as well.

CodexAdrian avatar May 10 '24 15:05 CodexAdrian

isBlank just sounds blegh to me

Spinoscythe avatar May 10 '24 15:05 Spinoscythe

@Spinoscythe you'll get used to it. Trust me.

Technici4n avatar May 12 '24 18:05 Technici4n

+1 for isEmpty over isBlank/isAbsent - akin to ItemStack.EMPTY and FluidStack.EMPTY

robotgryphon avatar May 13 '24 15:05 robotgryphon

FluidStack and ItemStack have the notion of amount that fluid resources do not have. Hence isEmpty will get two subtly different meanings. This is closer to Item.isAir() (if it exists). Technically you can have 0 of a non-blank resource. Yet that would be an empty slot.

Technici4n avatar May 13 '24 16:05 Technici4n

Any reason for not merging this in 1.21?

Spinoscythe avatar May 13 '24 16:05 Spinoscythe

I'm not sure what you mean, the PR is targeting 1.21

CodexAdrian avatar May 13 '24 16:05 CodexAdrian

image

Spinoscythe avatar May 13 '24 18:05 Spinoscythe

The idea is to allow the older containers to work to make it easier on devs to transition, but to remove them on the next major version.

CodexAdrian avatar May 13 '24 18:05 CodexAdrian