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

Adjacent parallel stained pipes with different colours get added to the visitedPipes set despite not being relevant to the pipe system (making adjacent attached loops not work) #1313

Open
6 tasks done
ANRAR4 opened this issue May 7, 2023 · 2 comments
Labels
status:pending Pending acceptance or closure. type:bug Incorrect behavior, not working as intended

Comments

@ANRAR4
Copy link

ANRAR4 commented May 7, 2023

CraftBook Version

3.10.8

Platform Version

paper

Confirmations

  • I am using the most recent Minecraft release.
  • I am using a version of WorldEdit compatible with my Minecraft version.
  • I am using a version of CraftBook compatible with my Minecraft version.
  • I am using the latest or recommended version of my platform software.
  • I am NOT using a hybrid server, e.g. a server that combines Bukkit and Forge. Examples include Arclight, Mohist, and Cardboard.
  • I am NOT using a fork of WorldEdit, such as FastAsyncWorldEdit (FAWE) or AsyncWorldEdit (AWE)

Bug Description

Adjacent parallel stained pipes with different colours get added to the visitedPipes set despite not being relevant to the pipe system, causing adjacent loops (attached on one end of the current pipe) of a different colour to be ignored in further processing of the pipe system. The same behaviour would occur when using (stained) glass panes instead of stained glass blocks.

Expected Behavior

In reference to the reproduction steps:
The items are introduced to the pipe system in the black stained glass block. Now CraftBook checks all surrounding blocks. The only block relevant to the pipe system is the unstained glass block beneath.
After reaching the unstained glass block CraftBook should find any adjacent stained or unstained glass block (in this case the adjacent orange stained glass block).
After that it should find the orange stained glass block adjacent to the black stained glass block. With the current behavior this block isn't found as it was marked as visited when checking adjacent blocks to the black stained glass block.
After which it should find the piston and reach the left chest.

Reproduction Steps

  1. CraftBook_Pipe_Bug1
  2. Try to send items from the input chest to the output chest

Anything Else?

In https://github.com/EngineHub/CraftBook/blob/master/src/main/java/com/sk89q/craftbook/mechanics/pipe/Pipes.java#L277 the adjacent glass blocks are added to the list of visitedPipes. The check whether these glass blocks are relevant to the current pipe occurs 2 lines later in https://github.com/EngineHub/CraftBook/blob/master/src/main/java/com/sk89q/craftbook/mechanics/pipe/Pipes.java#L279. These codeblocks would need to swap positions. To incorporate the expected behaviour for stained glass panes the code would need further modifications.

@ANRAR4 ANRAR4 added status:pending Pending acceptance or closure. type:bug Incorrect behavior, not working as intended labels May 7, 2023
@me4502
Copy link
Member

me4502 commented May 10, 2023

I can move adding to the visitedPipes list to after the check to allow this, it was originally just done for performance reasons. What are the further changes you're referring to?

@ANRAR4
Copy link
Author

ANRAR4 commented May 11, 2023

The further changes were refering to

The same behaviour would occur when using (stained) glass panes instead of stained glass blocks.

But I just noticed that glass panes (at least the first one) are not checked regularly, so the check for adjacent pipe blocks is skipped, so this problem wouldn't occur there.

While testing i also noticed that the following setup works for transfering items(which might not be intended). If this were to be fixed(or rather if glass_panes could bridge more than one block retaining their functionality, the original problem would also occur for them, as they aren't handled by ItemUtil.isStainedGlass() in Line 279.
CraftBook_Pipe_Bug2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:pending Pending acceptance or closure. type:bug Incorrect behavior, not working as intended
Projects
None yet
Development

No branches or pull requests

2 participants