TeamDman / SuperFactoryManager

Mozilla Public License 2.0
35 stars 15 forks source link

Split does not appear to be functioning correctly #68

Closed sei-mwd closed 3 years ago

sei-mwd commented 3 years ago

Create a simple experiment: one manager, a redstone receiver, three chests, and a button.

Designate one chest as an input and the other two chests as outputs. Connect the redstone receiver to the manager, and put a button on the receiver. Connect a redstone trigger (high) from the receiver to the input, the input to a flow in split-2 mode, and each output of the flow to one of the output chests. Set the split mode to split (not sequential).

Put two items in the input chest. Click button. One goes to one output, and one gets left behind. Expected: each output gets one item.

Put three items in the input chest. Click button. One goes to each output chest. One gets left behind. Expected: one output gets one, the second gets two.

Put four items in the input chest. Click button. One item goes in one output, two go into the second output, and one remains behind. Expected: each output gets two.

An example of how it's supposed to work can be found at: https://youtu.be/NGoNZ_h0zpQ?t=1476

This is with SFM 2.0.24 in Minecraft 1.12.2.

sei-mwd commented 3 years ago

I've concluded what causes this. If you look at splitFlow in CommandExecutor, you find that at the split it calculates the number of valid outputs and puts that in amount. Then it loops over the output connections and creates a divided ItemBufferElement using element.getSplitElement, then uses that as the itemBuffer in a new CommandExecutor, which it runs on the connection from that output. That executor will pull half the elements from the inventory into the first output. On the second iteration of the loop it regenerates the split on the second element, using the same amount, but the number of items in the stach in each element from the itemBuffer is now halved, as they were moved in the first loop. So it then ends up halving again (1/4) and moving half of the remaining items into the second inventory.

So that's what's happening. The question is how to fix it. This part of the code is really overly complicated. Decrementing amount each loop is not a valid solution, since it will cause "fair splits" to become unfair. A possible solution would be to generate the splits for each connection in one loop, then apply the splits in a second loop. I'm going to try to prototype that and see what comes of it.

TeamDman commented 3 years ago

Fixed in #69 Thanks for the help! Latest build will be available here once approved

md5i commented 3 years ago

Great! I'll see if I can't track down the periodic "every selected inventory is shifted by one" problem I keep on encountering.