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

Fix extremely longstanding issues of PlayerEvent.ItemSmeltedEvent #1367

Closed
Tslat opened this issue Jul 24, 2024 · 3 comments · Fixed by #1440
Closed

Fix extremely longstanding issues of PlayerEvent.ItemSmeltedEvent #1367

Tslat opened this issue Jul 24, 2024 · 3 comments · Fixed by #1440
Labels
1.21 Targeted at Minecraft 1.21 bug A bug or error

Comments

@Tslat
Copy link

Tslat commented Jul 24, 2024

Forge patched in ItemSmeltedEvent into checkTakeAchievements of FurnaceResultSlot a long time ago, and ever since then the event has had one major flaw, that eliminates a huge chunk of use-cases.

image

It's been on my list of 'broken in forge' bugs for a long time, but now we have NeoForge yay

Foreword: There's actually two bugs below.. but only one is functionally critical

Basically, when extracting a stack from the FurnaceResultSlot, the stack provided is the exact stack the player attempts to extract, regardless of whether it's successful or not.
This means that the event is actually providing an inaccurate value to modders when attempting to use the stack to determine how many things were smelted.

Here's an example result table for a stack of 64 iron ingots smelted, and the resultant event stack size:
Left click - 64
Right click - 32
Shift click - 64
Shift click with only 5 spaces left in inventory - 64

Problem here? the stack is always the stack attempted, even if the player doesn't have space when shift clicking/quicktransferring

Solution: pass in the removeCount variable, which keeps track of exactly how much is being removed (move the event up a line so it resets removeCount after the event)

BUG 2: When shift-clicking the stack, MC fires checkTakeAchievements twice, with the second call being for an empty stack.
Don't call the event if the removecount or stack is 0. You're currently firing it twice for every shift-click extraction

@marchermans
Copy link
Contributor

Could you create a PR for this, cause yes this seems like a bug?

@marchermans marchermans added bug A bug or error 1.21 Targeted at Minecraft 1.21 and removed triage Needs triaging and confirmation labels Jul 24, 2024
@Technici4n
Copy link
Member

Fun fact: Mojang made the same mistake. See https://bugs.mojang.com/browse/MC-65198.

@neoforged-releases
Copy link

🚀 This issue has been resolved in NeoForge version 21.3.24-beta, as part of #1440.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21 Targeted at Minecraft 1.21 bug A bug or error
Projects
None yet
3 participants