[MC-7058] Brewing stand comparator output inconsistent

The comparator output from a brewing stand is inconsistent between the different slots in the stand.

What I expected to happen was if the top slot in the brewing stand had a full stack of items but the three lower slots were empty, the comparator output would be about a quarter of the full fifteen - say four. This would have been consistent with the comparator output from a chest where, if a third of the slots are full of stacks of items, the output is five.

What actually happened was the comparator output was fifteen. However, if the top slot is empty but one of the lower slots has a water bottle, the output is only four, as expected.

Steps to Reproduce:
1. Place a brewing stand with an adjacent comparator and enough redstone to see the output level.
2. Put a stack of nether wart (for example) in the top slot of the brewing stand's GUI.
3. Check the output level - fifteen.

This also affected the previous snapshot.

UPDATE: Having read Dinnerbone's explanation of the algorithm in MC-6165, the problem appears to be that the top slot is assumed to only be able to hold a single item. If four items are placed in the slot it is calculated as being 400% full, giving an average over the four slots of 100% and hence an output level of fifteen. With anything over four items in the top slot, the brewing stand is calculated as over 100% full and the output is capped at fifteen.

This may be related to MC-2034 - the top slot does not work as expected when shift-clicking items into the brewing stand.

Linked issues

is duplicated by 1

MC-11272 Brewing Stand and Comparator bug Resolved

relates to 1

MC-12315 Comparator will not distinguish between 3 and 4 items in a brewing stand Resolved

Comments 18

Tails 2013-02-06T15:58:11Z

Confirmed.

Phssthpok Pak 2013-02-07T17:50:44Z

Nice to see this has been looked at even with zero votes.

Pippo 2013-02-10T00:20:26Z

Totally true, was tryng to make a potion making machine but i stumbled on this problem

Not Public 2013-03-08T12:34:29Z

well in my opinion this fix is plain useless, my programmable potionmaker is now completly broken!
there should be at least a change of ONE redstonelevel in between NOTHING in the top slot and MORE THAN NOTHING! This is a broken fix!

Preston Crary 2013-03-14T03:02:27Z

Please revert this fix.

8 more comments

Kevin Moser 2013-03-17T03:54:57Z

I'm going to agree with Frank.
This fix, while it applies the correct algorithm, makes it impossible to tell when a potion has started brewing or when it has stopped.
Maybe a special method can be called when a comparator is checking a brewing stand, with it's own algorithm. Something like:
signal strength = truncate(1+ [(number of items in bottom slots/3) + (number of items in top slot/64)]*7 )

In my equation, the output will be 8 if the brewing stand has only bottles in the bottom slots, 9 when the first item is added to the top, and 15 when it is completely filled. Inversely, 8 if the brewing stand's top slot has 64 items, 9 with one bottle, 11 with two, 15 when completely filled.

This would fix the initial bug report as well as the unintended feature from the fix.

Preston Crary 2013-03-18T01:04:03Z

I think Kevin's fix would be enough for me to be happy.
However, I think the correct fix would be to revert this and make it so that brewing stands can only contain a single ingredient. The fact that Dinnerbone's algorithm was originally based on the assumption that they could only contain a single ingredient supports this.

Dave G 2013-03-18T05:10:31Z

This really needs to be addressed. I think Preston's fix would be most ideal since there's no legitimate reason for placing more than one of an item in the ingredient slot.

I had built a multi-stand brewing machine during the snapshots, and just returned to it today only to find that this fix broke it completely. The only workaround I can think of would vastly increase the complexity of the control circuits.

Mathias pedersen 2013-03-18T18:46:49Z

This really needs to be reverted. Most creations possible to make using brewingstands and restone comparators need the mechanism that detects if there is a single incredient in the slot or not.

Kevin Moser 2013-03-18T23:16:54Z

I could see some instances where having a stack of items in the top slot could be used, so I doubt they will change that interaction by limiting the top slot to 1 item like the enchanting table. Would like to see some kind of change to the comparator-brewing stand interaction. Just some way of detecting an item in the top slot or not when there are 3 bottles in the brewing stand.

The work-around that I've developed for my auto-potion-maker detects when the hopper above the brewing stand is empty, then initiate a 25 second delay. Works fine, just needed more redstone circuit components than the pre-fix version.

Preston Crary 2013-03-19T01:27:56Z

"I could see some instances where having a stack of items in the top slot could be used"

Could you give one example please?

Kevin Moser 2013-03-19T03:34:19Z

The best thing I can think of is having multiple brewing stands with a stack of a particular ingredient in each one. The user would then move the bottles from stand to stand until the potion is complete. Thus using the stands as a storage device that seconds as a brewing step. Not very practical and very intensive, but someone might like it that way.

I don't see a full revert of this fix to be the best solution as that will re-introduce 4 items in the top slot to equal full redstone output.
My best solution would be a middle ground of keeping the brewing stand as is and changing how the comparator treats it as a container. Like before, but different.

Just trying to come up with solutions that doesn't change the brewing stand that was introduced more than a year ago. Do that and there will be people coming out of the woodwork to argue for the way it used to work, just like we are doing for the comparator now. The comparator is still new enough to change how it interacts, that is where we should be focusing our attention.

Preston Crary 2013-03-19T05:48:40Z

Okay, I can respect that.
Sorry, I just don't like to when someone says they can think of some reason for something but then they don't express one.

I don't think the formula you gave as an example would work though:
floor(1 + [(3/3) + (1/64)] * 7) = floor(1 + [(3/3 + (0/64) * 7)
floor(1 + 7 + 7/64) = floor(1 + 7 + 0)
floor(8 + 0.109...) = floor(8)

Maybe use Math.ceil instead of truncate?

Kevin Moser 2013-03-19T06:54:57Z

You're right there, error on my part. Math.ceil would be what the equation in my head was using.
So, Math.ceil[ ( Number of bottles in bottom slots /3) * ( Weight of Bottom Slots ) + ( Number of items in the top slot /64) * ( 15- Weight of Bottom Slots ) ]

My previous example had the Weight of Bottom Slots set at 7. Changed the formula a bit to allow for inequality of a brewing stand with just bottles in it and a brewing stand with just a stack of items. Weight of Bottom Slots set at 11 is the closest value to have all slots to have equal weight overall, since 15*3/4 = 11.25. If only we could submit RFEs on this site instead of just bugs. Could post something on the Suggestions Board, but I fear this topic will be drowned out by the abstract requests. An official response would be helpful to know if we're not wasting our time.

Preston Crary 2013-03-19T15:39:31Z

Just a thought. Instead of using a formula, it may be better to just use a table whenever a comparator is used with a container. Something like:

Percent Full	RS Output
0	0
(0, 5]	1
(5, 10]	2
(10, 20]	3
(20, 25]	4
(25, 30]	5
(30, 40]	6
(40, 50]	7
(50, 60]	8
(60, 70]	9
(70, 75]	10
(75, 80]	11
(80, 90]	12
(90, 95]	13
(95, 100)	14
100	15

This way every output change would be meaningful and since 75% would output 10 and above 75% would output 11 it would solve this problem too.

Kevin Moser 2013-03-20T06:29:29Z

I like the idea of using a table to determine the output strength. It would fix the issue and can be applied to all containers. The problem with it is coding it for efficiency. As far as my programming skill get me, I can only come up with a block of if statements to determine the output. We'll be dealing with a continuous variable, so a pre-rendered array or table can't be used. So it will have to be checked during run time, every time there is an update. It'll add at most 16 code routines when a one-off equation will do it faster. Might cause unnecessary lag.

I've just set up MCP on my computer and I'm starting to look at the source code to see if I can gleam some ideas from there. With the 1.5.1 update coming out soon, I don't think our issue will be addressed. Maybe a mod will come out to fix this.

Not Public 2013-03-20T09:06:47Z

accessing a table is pretty as fast as making a floatingpoint calculation imho! But more a burden to the memory if not used in a static way. But i must admit that i never tested it for myself i just heared it from my professor years ago XD

Preston Crary 2013-03-20T15:49:50Z

Comparisons aren't that bad on performance. I would think in some cases having a complex formula is worse. They could also use a switch statement which would be slightly faster.

If this actually caused some lag they could rewrite the code to do a bisection search of the table. Worse case it would have to compare four times. However, I don't think the size of the table really warrants it.