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

[1.21.3] Add remove count context to ItemSmeltedEvent and fix firing multiple times with shift-click #1440

Merged
merged 2 commits into from
Nov 10, 2024

Conversation

dhyces
Copy link
Contributor

@dhyces dhyces commented Aug 10, 2024

Closes #1367.

This PR does two different things in regard to the event.

  1. Moves the event firing up above the removeCount and passes it as an integer field for context on how much of the item stack was removed, since the item stack given in the event is only representative of the stack that was present in the slot before splitting it.
  2. Adds a guard around the hook call to only fire if the removeCount is not zero. This is because AbstractFurnaceMenu#quickMoveStack calls Slot#onTake for the result slot while also earlier calling Slot#onQuickCraft. FurnaceResultSlot calls checkTakeAchievements for both onTake and onQuickCraft, which would fire the event twice.

This is up for review now, though it should be merged during the next BC window due to mods with custom furnaces/machines more than likely calling the event hook themselves.

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@neoforged-compatibility-checks
Copy link

neoforged-compatibility-checks bot commented Aug 10, 2024

@dhyces, this PR introduces breaking changes.
Fortunately, this project is currently accepting breaking changes, but if they are not intentional, please revert them.
Last checked commit: 3384671d6ef724aa49245b0bbc716f8d957121bb.

neoforge (:neoforge)

  • net/neoforged/neoforge/event/entity/player/PlayerEvent$ItemSmeltedEvent
    • <init>(Lnet/minecraft/world/entity/player/Player;Lnet/minecraft/world/item/ItemStack;)V: ❗ API method was removed
  • net/neoforged/neoforge/event/EventHooks
    • firePlayerSmeltedEvent(Lnet/minecraft/world/entity/player/Player;Lnet/minecraft/world/item/ItemStack;)V: ❗ API method was removed

@Matyrobbrt Matyrobbrt added enhancement New (or improvement to existing) feature or request regression Worked previously but doesn't anymore breaking change Breaks binary compatibility 1.21 Targeted at Minecraft 1.21 bug A bug or error 1.21.2 Targeted at Minecraft 1.21.2 and removed regression Worked previously but doesn't anymore 1.21 Targeted at Minecraft 1.21 labels Aug 11, 2024
@dhyces dhyces force-pushed the fix/item-smelted-event branch from 56fe79c to df2a4c7 Compare October 22, 2024 19:33
@dhyces dhyces changed the title [BC 1.21.x] Add remove count context to ItemSmeltedEvent and fix firing multiple times with shift-click [1.21.2] Add remove count context to ItemSmeltedEvent and fix firing multiple times with shift-click Oct 22, 2024
@Tslat
Copy link

Tslat commented Oct 22, 2024

@dhyces bump for 1.21.23

@dhyces dhyces force-pushed the fix/item-smelted-event branch from df2a4c7 to be93868 Compare October 24, 2024 22:20
@dhyces dhyces changed the title [1.21.2] Add remove count context to ItemSmeltedEvent and fix firing multiple times with shift-click [1.21.3] Add remove count context to ItemSmeltedEvent and fix firing multiple times with shift-click Oct 24, 2024
@TelepathicGrunt TelepathicGrunt merged commit 08a40d2 into neoforged:1.21.x Nov 10, 2024
6 checks passed
@neoforged-releases
Copy link

🚀 This PR has been released as NeoForge version 21.3.24-beta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21.2 Targeted at Minecraft 1.21.2 breaking change Breaks binary compatibility bug A bug or error enhancement New (or improvement to existing) feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix extremely longstanding issues of PlayerEvent.ItemSmeltedEvent
4 participants