-
-
Notifications
You must be signed in to change notification settings - Fork 184
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
Let vanilla try to handle Container interactions before checking for item handlers #1787
base: 1.21.x
Are you sure you want to change the base?
Conversation
Last commit published: 5cea5dce44481ed60c264df1f03d01d2132b3e96. PR PublishingThe artifacts published by this PR:
Repository DeclarationIn order to use the artifacts published by the PR, add the following repository to your buildscript: repositories {
maven {
name 'Maven for PR #1787' // https://github.com/neoforged/NeoForge/pull/1787
url 'https://prmaven.neoforged.net/NeoForge/pr1787'
content {
includeModule('net.neoforged', 'neoforge')
includeModule('net.neoforged', 'testframework')
}
}
} MDK installationIn order to setup a MDK using the latest PR version, run the following commands in a terminal. mkdir NeoForge-pr1787
cd NeoForge-pr1787
curl -L https://prmaven.neoforged.net/NeoForge/pr1787/net/neoforged/neoforge/21.4.46-beta-pr-1787-fix-vanilla-inv-hooks/mdk-pr1787.zip -o mdk.zip
jar xf mdk.zip
rm mdk.zip || del mdk.zip To test a production environment, you can download the installer from here. |
@Technici4n, this PR introduces breaking changes.
|
How difficult would that be to implement? I can see what you mean by that being preferrable -- ensuring that blocks are always checked before entities in either case -- but I wonder how the impl. look like if you have to interweave the item handler check between container blocks and entities. |
I managed to implement it locally and am planning to update the PR tomorrow. |
PR updated with a more modder-friendly lookup order. 😄 @sciwhiz12 |
Move our
IItemHandler
interaction hooks after vanilla checks forContainer
interactions. This gives vanilla control over interactions withContainer
s. Fixes #1750. Fixes #1770. (Did not test the latter but I assume it gets fixed).As a side effect, this greatly simplifies our hooks since we don't need to handle vanilla edge cases such as hopper - hopper interactions having special interactions when it comes to cooldowns.
Breaking change
Modders need to be careful that their
Container
s will be interacted with asContainer
s before an item handler is even attempted to be found. I'd suggest not implementingContainer
directly on block entities to avoid this problem. Nevertheless, this counts as a breaking change.Lookup order
From highest priority to lowest priority:
Container block > IItemHandler block > Container entity > IItemHandler entity
.