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

ItemType - fix error in loops #7418

Open
wants to merge 4 commits into
base: dev/feature
Choose a base branch
from

Conversation

ShaneBeee
Copy link
Contributor

Description

This PR aims to fix a bug where looping over all item types would throw an error.

From what I gathered, the error is due to the container iterator where looping over the item datas was attempting to grab an ItemStack. If the ItemStack was not there (since it's a nullable getter), then it would throw an error.
This is due to the fact not all materials can be ItemStacks (ie: block only materials like WATER)

This PR is more created to be open for discussion. I stripped out the Container reference because it was returning ItemStacks??!?! Why exactly?!?!?
If I'm looping all ItemTypes I want ItemTypes not ItemStacks.

From what I can see the ONLY place the container stuff was referenced was in SecLoop and SecFor.
I don't believe this is needed, but I will leave it up to the team to decide.


Target Minecraft Versions: any
Requirements: none
Related Issues: #7414

@Efnilite Efnilite added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Jan 9, 2025
@ShaneBeee
Copy link
Contributor Author

Poor lil fella, you've been missed on two updates now, 😔

@sovdeeth
Copy link
Member

sovdeeth commented Feb 2, 2025

This is used to do loop all logs, for example, so I don't think we should outright remove it yet. Returning ItemTypes seems fine to me, though.
Honestly, the proper fix seems to be to do that little while loop in the hasNext instead of in Next and store the result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants