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.

Sign up for GitHub

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

Jump to bottom

Invalidate Ingredients on tags update, make getItems() check ingredient invalidation #9688

Merged
merged 1 commit into from Nov 8, 2023
Merged

Invalidate Ingredients on tags update, make getItems() check ingredient invalidation #9688

merged 1 commit into from Nov 8, 2023

Conversation

DBotThePony
Copy link
Contributor

@DBotThePony DBotThePony commented Jul 31, 2023 •
edited

Since it doesn't make much sense that invalidate() erasing cached itemstacks, but method which build itemstack cache doesn't check for invalidation.

And since tag ingredients depend on tags, they should be rechecked when tags get updated.

This fixes issue with recipes bricking IF they network Ingredients themselves rather than final ItemStacks of these ingredients.

@CLAassistant
Copy link

CLAassistant commented Jul 31, 2023 •
edited

CLA assistant check
All committers have signed the CLA.

@autoforge autoforge bot added Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. 1.20 labels Jul 31, 2023
@autoforge autoforge bot requested a review from a team July 31, 2023 09:57
@autoforge autoforge bot added the Needs Rebase This PR requires a rebase to implement upstream changes (usually to fix merge conflicts). label Sep 21, 2023
@autoforge
Copy link

autoforge bot commented Sep 21, 2023

@DBotThePony, this pull request has conflicts, please resolve them for this PR to move forward.

@LexManos
Copy link
Member

LexManos commented Oct 7, 2023

This fixes issue with recipes bricking IF they network Ingredients themselves rather than final ItemStacks of these ingredients.

Did a bunch of rework to actually get custom Ingredients working again.
Can you still reproduce this issue and if so, please provide that case so i can test it and be sure its supported/fixed.

@DBotThePony
Copy link
Contributor Author

DBotThePony commented Oct 7, 2023 •
edited

This fixes issue with recipes bricking IF they network Ingredients themselves rather than final ItemStacks of these ingredients.

Did a bunch of rework to actually get custom Ingredients working again. Can you still reproduce this issue and if so, please provide that case so i can test it and be sure its supported/fixed.

I already went around this issue by doing what vanilla does - sending itemstacks to clients instead of actual ingredients. However, this PR's functional change is still relevant, since getItems() even in current commit (https://github.com/MinecraftForge/MinecraftForge/tree/ab544a78d3b24786b07b421689a6f20853a49f80) is not patched to check for ingredient invalidation, which is inconsistent with getStackingIds() patch.

@autoforge autoforge bot removed the Needs Rebase This PR requires a rebase to implement upstream changes (usually to fix merge conflicts). label Oct 10, 2023
@DBotThePony
Copy link
Contributor Author

Updated PR to match trunk

@LexManos LexManos merged commit da34d97 into MinecraftForge:1.20.x Nov 8, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
1.20 Triage This request requires the active attention of the Triage Team. Requires labelling or reviews.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@DBotThePony @CLAassistant @LexManos