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

Diamond Pipe doesn't distinguish cables from ic2 #4553

Closed
ashlanderr opened this issue Apr 7, 2020 · 8 comments · Fixed by #4555
Closed

Diamond Pipe doesn't distinguish cables from ic2 #4553

ashlanderr opened this issue Apr 7, 2020 · 8 comments · Fixed by #4555
Labels
priority: low status: fixed in release Fully fixed and released. type: bug Something isn't behaving as expected, from a developer perspective. (Except crashes are always bugs) type: mod compat This only happens with another mod. version: 1.12.2

Comments

@ashlanderr
Copy link
Contributor

@ashlanderr ashlanderr commented Apr 7, 2020

  • Minecraft Version: 1.12.2
  • Forge Version: 14.23.5.2847
  • BuildCraft Version: buildcraft-all-7.99.24.5
  • IndustrialCraft Version: industrialcraft-2-2.8.176-ex112

Steps to reproduce the behavior:

  1. Create a diamond pipe.
  2. Attach two cobblestone pipes for output.
  3. Attach some pipe for input.
  4. In diamond pipe filters, set "Copper Cable" for the first pipe, and "Insulated Copper Cable" for the second.
  5. Pass multiple cables into the diamond pipe.

Expected: The "Copper Cable" must go out from the first pipe, and the "Insulated Copper Cable" - from the second pipe.
Actual: The items choose pipes randomly.

This also works with "Tin Cable".

@ashlanderr ashlanderr added the auto: bug This issue was based on the "bug" issue type, and hasn't been looked at yet. label Apr 7, 2020
@ashlanderr
Copy link
Contributor Author

@ashlanderr ashlanderr commented Apr 7, 2020

I think the problem is here.

if (base.getItem() != comparison.getItem()) {
    return false;
}

Somehow "Copper Cable" and "Insulated Copper Cable" have the same stack.getItem() object.

I've changed the condition, and now it works.

if (!Objects.equals(base.getDisplayName(), comparison.getDisplayName())) {
    return false;
}

I know, it's not a good idea to compare items by name, but now we know where is the problem.

@AlexIIL
Copy link
Member

@AlexIIL AlexIIL commented Apr 7, 2020

For some reason ic2 stores insulated cable variants in NBT, rather than in the items metadata or as a different item, however the diamond pipe doesn't filter by NBT - instead you can use a list, and click the P button to make it compare items precisely.

We don't filter by NBT by default because NBT is normally used for things with large variance - like written books, or the amount of liquid in a container, etc. For ic2 however it might make sense to compare the specific NBT tags that ic2 uses, as insulated cables should probably be sortable by default. (However that would need to go in compat somehow rather than in base BC).

@AlexIIL AlexIIL added priority: low type: bug Something isn't behaving as expected, from a developer perspective. (Except crashes are always bugs) type: mod compat This only happens with another mod. version: 1.12.2 and removed auto: bug This issue was based on the "bug" issue type, and hasn't been looked at yet. labels Apr 7, 2020
@ashlanderr
Copy link
Contributor Author

@ashlanderr ashlanderr commented Apr 9, 2020

Thank you for the reference to a list. It was hard to find any info about the list and its usage with diamond pipes.

If you don't mind, I want to try to fix the issue. My idea is to add some registry of comparison rules to the StackUtils. Other mods, like BuildCraftCompat, can use this registry to add their own rules. These rules can represent some "common sense comparison", like the difference between insulated and simple cables.

@AlexIIL
Copy link
Member

@AlexIIL AlexIIL commented Apr 10, 2020

Sure!

@AlexIIL AlexIIL linked a pull request Apr 11, 2020 that will close this issue
@AlexIIL AlexIIL reopened this Apr 14, 2020
@ChangeOtaku
Copy link

@ChangeOtaku ChangeOtaku commented Oct 3, 2020

what do you guys mean by "list"?

@AlexIIL
Copy link
Member

@AlexIIL AlexIIL commented Oct 5, 2020

I've finally released this fix in 7.99.24.7 - sorry for the delay.

@AlexIIL AlexIIL closed this as completed Oct 5, 2020
@AlexIIL
Copy link
Member

@AlexIIL AlexIIL commented Oct 5, 2020

ChangeOtaku: A list is an item that contains "ghost" copies of other items, and allows diamond pipes to filter up to 18 items per slot (so up to 162 per side) rather than just 9. It looks like this:
image

@AlexIIL AlexIIL added the status: fixed in release Fully fixed and released. label Oct 5, 2020
@ChangeOtaku
Copy link

@ChangeOtaku ChangeOtaku commented Oct 6, 2020

oh, i´ve never seen this "list item", neither this P-T-M buttons... im playing on 1.12.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low status: fixed in release Fully fixed and released. type: bug Something isn't behaving as expected, from a developer perspective. (Except crashes are always bugs) type: mod compat This only happens with another mod. version: 1.12.2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants