mojira.dev
MC-171079

Comparators no longer work as expected reading containers through powered blocks

Moderator Notice

Please do not spam the comments. This ticket has already been assigned to a developer and is likely to be fixed. Further discussion should take place on the Mojira subreddit.

Previously in 1.15.2 and below, a comparator that reads the contents of a container through a block would let the container's signal be overridden by a signal of the block itself, but only if it gives a strength of 15.
Since 20w06a, the container's signal strength is now always overridden by the block's signal strength if it is higher.

This feature was very useful in 1-wide tileable contraptions, where it can be used to detect full containers, etc.

Linked issues

Attachments

Comments

RedCMD
migrated

This is used in doors too, for example, the smallest 7x7 and 10x10 observerless doors.

Nethonos

Confirmed for 20w07a

migrated

Confirmed for 20w08a

wobst.michael

People please stop spamming this ticket.

migrated

請只寫英文。僅添加有用的信息。我們知道這是影響Redstone的變化。請保持冷靜。謝謝。

Please write only English. Only add useful information. We know this is a change that affects Redstone. Please stay calm. Thank you.

migrated

Confirmed for 20w09a

migrated

I'm not redstone player but I understand these players anxiety about this change. Still, please speak English in this forum.

Sonicwave

Please take further discussion of this ticket to the Mojira subreddit. This ticket has already been assigned and is likely to be fixed, spamming the comments won't make it get fixed any faster.

migrated

Confirmed for 20w09a

migrated

On a side note, the feature related to this bug has been first reported in MC-64394, which (the feature) has been resolved as working as intended.

migrated

Some code analysis:
Old logic (FabricMC yarn 1.15-pre4 build 1)

protected int getPower(World world, BlockPos pos, BlockState state) {
      int i = super.getPower(world, pos, state);
      Direction direction = (Direction)state.get(FACING);
      BlockPos blockPos = pos.offset(direction);
      BlockState blockState = world.getBlockState(blockPos);
      if (blockState.hasComparatorOutput()) {
         i = blockState.getComparatorOutput(world, blockPos);
      } else if (i < 15 && blockState.isSimpleFullBlock(world, blockPos)) {
         blockPos = blockPos.offset(direction);
         blockState = world.getBlockState(blockPos);
         if (blockState.hasComparatorOutput()) {
            i = blockState.getComparatorOutput(world, blockPos);
         } else if (blockState.isAir()) {
            ItemFrameEntity itemFrameEntity = this.getAttachedItemFrame(world, direction, blockPos);
            if (itemFrameEntity != null) {
               i = itemFrameEntity.getComparatorPower();
            }
         }
      }

      return i;
   }

New logic (FabricMC yarn 20w08a build 6) with my comments:

protected int getPower(World world, BlockPos pos, BlockState state) {
      int i = super.getPower(world, pos, state);
      Direction direction = (Direction)state.get(FACING);
      BlockPos blockPos = pos.offset(direction);
      BlockState blockState = world.getBlockState(blockPos);
      if (blockState.hasComparatorOutput()) {
         i = blockState.getComparatorOutput(world, blockPos);
      } else if (i < 15 && blockState.isSimpleFullBlock(world, blockPos)) {
         blockPos = blockPos.offset(direction);
         blockState = world.getBlockState(blockPos);
         ItemFrameEntity itemFrameEntity = this.getAttachedItemFrame(world, direction, blockPos); // also the air check is now removed, most likely intended
         int j = itemFrameEntity == null ? 0 : itemFrameEntity.getComparatorPower();
         int k = blockState.hasComparatorOutput() ? blockState.getComparatorOutput(world, blockPos) : 0;
         i = Math.max(Math.max(j, k), i); // i = Math.max(j, k) may be a fix, but it still changes old behavior as previously only frames in air outputs, and has lower priority than outputing blocks
      }

      return i;
   }

The max out calculation is all new. The old logic is that it replaces the power obtained as a regular block with custom comparator output if i < 15.

Is maxing things out wrong?

This i < 15 statement makes me think that maxing out power later is the intended initial behavior (given if i == 15, extra check will not return a higher value and will waste cpu/memory when we are actually doing maxing out logic).

Is maxing things out right?

If it is, the getting power not through wall code probably should max with ambient block power as well, but it's clear that if the comparator outputing block has redstone output itself, things are clearly messed up.

My conclusion is that the final result is up to mojang to decide. In my opinion, the current status (i.e. in 20w09a) should be the most correct one (again this is only my opinion). The other side effects of the recent change (item frame) may be less noticed. If we do determine what should be regarded as "correct", we should determine what should be the power when both item frame and a comparator outputing block exists.

Uriel Salischiker

Moderator Notice

Please do not spam the comments. This ticket has already been assigned to a developer and is likely to be fixed. Further discussion should take place on the Mojira subreddit or our discord server.

migrated

A Redditpost for this bugpost has been added, could any of you link it directly in the bugpost above where the Mojira Subreddit is mentioned, and maybe also @unknown' comment?
https://www.reddit.com/r/Mojira/comments/fa6ngm/mc171079_why_it_shouldnt_be_fixed

Jack McKalling

I rephrased the description a bit to express the problem better, because the previous "before and after" description was a little bit ambiguous to me, about what exacly the issue is.

FaRo1

Isn't this WAI? Just like a piece of redstone dust gets the highest adjacent power level minus 1, a comparator in this way gets the highest measured power level. If this was not the case, either the incoming redstone signal or the container would always be ignored or there would be the weird 15/not 15 distinction mentioned in the description. The behaviour as described here is the only consistent one.

(The Reddit post also argues that this should be kept, but for performance or code cleanliness, which is not a valid argument for gameplay changes.)

migrated

Confirmed 20w10a

ZYX_2D

I just checked the screenshots, then I found that's really a hard problem 2 decide fix it or not QWQ.

migrated

Felix Jones

Confirmed

Important

Redstone

20w06a, 20w07a, 20w08a, 20w09a, 20w10a

20w11a

Retrieved