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

Hoppers try to transfer items out twice per tick if the first attempt fails #1770

Open
2No2Name opened this issue Dec 14, 2024 · 0 comments · May be fixed by #1787
Open

Hoppers try to transfer items out twice per tick if the first attempt fails #1770

2No2Name opened this issue Dec 14, 2024 · 0 comments · May be fixed by #1787
Assignees
Labels
triage Needs triaging and confirmation

Comments

@2No2Name
Copy link

Minecraft Version: 1.21.4

NeoForge Version: 21.4.22-beta

Steps to Reproduce:
Image

World download:
HopperTest.zip
Demo Video:
Cannot post a video here, but demonstration video on the neoforge discord: 1836 ticks (91 seconds) on vanilla vs 1072 ticks (53 seconds) on neoforge The video shows a measurement of the timing. It is not necessary to understand the issue, but may be helpful.

  1. Place a hopper facing into multiple inventory entities (in the world download: 19 chest carts and one chest boat, chest boat sits on a soul sand behind the rail)
  2. Make sure all but one entity inventory is filled entirely (in the world download: chest carts full, chest boat empty)
  3. Put items into the hopper and check how long the transfer takes on average. Compare this to the average time on vanilla. (World download: 86 seconds for 64 items in vanilla, 58 seconds for 64 items on neoforge)

Description of issue:
Neoforge's hopper item insertion logic is broken. Neoforge adds a "hook" at the start of the insertion code that duplicates the insertion code in case the insertion attempt fails. For successful attempts, it cancels the execution of the vanilla insertion code. In defail, the hook returns false for unsuccessful insertion attempts and the vanilla attempt is only cancelled when true is returned. (This is implemented differently for the extraction hook.)
Item transfers to inventory entities work by uniformly choosing a random inventory entity that intersects the 1x1x1 volume at the hopper output.
The chance of picking the chest boat in the example is 1/20.
After not choosing the chest boat (19/20 chance), the item transfer attempt fails.
With neoforge a second attempt takes place: Another 1/20 chance to pick the chest boat and successfully transfer.
This significantly speeds up the transfer to the chest boat from ~86 seconds to ~58 seconds for 64 items.

The fix should probably include always cancelling the vanilla transfer code or if possible, avoiding to duplicate the transfer code in the first place.

@2No2Name 2No2Name added the triage Needs triaging and confirmation label Dec 14, 2024
@Technici4n Technici4n self-assigned this Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs triaging and confirmation
Projects
None yet
2 participants