Fix three jukebox item bugs in pipes - #1372
Open
RasmusKD wants to merge 2 commits into
Open
Conversation
The jukebox branch created a new disc ItemStack and only emptied the jukebox if every item was delivered. When the network was full or missing, the copy dropped at the piston while the original kept playing, so a clocked piston printed discs. It was also the only branch that never fed leftovers, so an empty jukebox silently destroyed any items a pipe request delivered into it. Now the disc is removed from the jukebox before the event fires, and undelivered items go through leftovers like the other branches.
The put-path test was inverted: a disc was only inserted when the jukebox was already playing, so an empty jukebox never accepted one, and when it did fire, setPlaying overwrote the playing disc and destroyed it. Now only an empty jukebox accepts a disc, and a disc that does not fit flows on through the pipe like any other undelivered item.
RasmusKD
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three separate bugs in how Pipes handles jukeboxes, all reproducible on a test server.
1. Disc duplication (suck path). The branch creates a new disc ItemStack and only empties the jukebox if every item was delivered. If the network is full or missing, the copy drops at the piston while the original keeps playing. A clocked sticky piston facing a playing jukebox prints discs.
Repro: jukebox with a disc playing, sticky piston facing it, no pipe network. Pulse the piston. A disc drops and the jukebox keeps playing. Repeat for more discs.
2. Item loss (suck path). It is the only branch that never feeds leftovers, so when the jukebox is empty, any items a pipe request delivers into it are cleared at the end of startPipe and destroyed.
3. Item loss (put path). The insert test is inverted: a disc is only inserted when the jukebox is already playing, so an empty jukebox never accepts one. When it does fire, setPlaying overwrites the playing disc and destroys it.
Repro: chest with a disc, pipe into a jukebox that is already playing a different disc. Two discs before, one after.
Fixes. For the suck path, take the disc out of the jukebox before firing the event and route undelivered items through leftovers like the other branches. For the put path, only insert into an empty jukebox and let discs that do not fit flow on through the pipe.
One behavior change worth noting: a disc the network refuses now drops at the piston instead of staying in the jukebox. That is what stops the duplication.