Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove items module #98

Open
Technici4n opened this issue Sep 27, 2021 · 3 comments
Open

Remove items module #98

Technici4n opened this issue Sep 27, 2021 · 3 comments
Labels
components-item talk Discussion or feedback thread

Comments

@Technici4n
Copy link

I would suggest deprecating for removal in 1.18 the itemstack module. Item stack components work with direct modifications of the stack, whereas I think the preferred paradigm should be to ask the container of the stacks for any modification. While worlds, entities, block entities, ... exist on their own and have a clear identity, for stacks it is not clear.

A consequence of this is that the itemstack module is fundamentally incompatible with the fabric transfer API on multiple levels: I think that components are currently destroyed when stacks are moved with the transfer API. Patching this can be possible, however components are still fundamentally incompatible with item-provided APIs such as the "fluid-containing" API or the reworked TR Energy API which require all modifications to happen through the ContainerItemContext. To prevent people using CCA and then running into these issues, I suggest removing the feature entirely.

Despite the special behavior of item stacks, a CCA module used to be necessary - not necessarily to store data, but also to be able to expose APIs. However, that is not necessary anymore now that ItemApiLookup exists.

@UpcraftLP UpcraftLP added components-item talk Discussion or feedback thread labels Sep 27, 2021
@Pyrofab
Copy link
Member

Pyrofab commented Sep 29, 2021

You're late lmao

The plan was to drop the external data storage for a while, the question now is whether there is a point in item components existing at all. Technically item components are still useful for caching live data for eg. rendering purposes, although I'm not sure how often this use case would come up. In any case I made a point in the changelog that API Lookup API should be preferred where applicable.

@UpcraftLP thoughts ?

@UpcraftLP
Copy link
Member

UpcraftLP commented Sep 29, 2021

Technically item components are still useful for caching live data for eg. rendering purposes, although I'm not sure how often this use case would come up.

I don't think components would be the way to go for that. There's a lot more potential for involuntarily causing issues with other mods as @Technici4n pointed out.
Besides, 'caching live data for rendering purposes' is trivial enough to implement using a weak-keyed map with ItemStack keys.

@Technici4n
Copy link
Author

@Pyrofab I'm really not sure what the new item components are supposed to achieve. :P
Are they just supposed to operate on a subtag directly? If that's the case then they still have the second issue I mentioned above, and don't seem to bring much compared to direct stack manipulation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
components-item talk Discussion or feedback thread
Projects
None yet
Development

No branches or pull requests

3 participants