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

Cargo ignores lore of slimefun items, causing backpacks to stack. #3379

Closed
4 tasks done
Boomer-1 opened this issue Dec 20, 2021 · 16 comments · Fixed by #3417
Closed
4 tasks done

Cargo ignores lore of slimefun items, causing backpacks to stack. #3379

Boomer-1 opened this issue Dec 20, 2021 · 16 comments · Fixed by #3417
Assignees
Labels
🐞 Bug Report A bug that needs to be fixed. 🧨 Confirmed Bug This bug has been confirmed by our team. ✔ Resolved This Issue has been resolved.

Comments

@Boomer-1
Copy link

❗ Checklist

  • I am using the official english version of Slimefun and did not modify the jar.
  • I am using an up to date "DEV" (not "RC") version of Slimefun.
  • I am aware that issues related to Slimefun addons need to be reported on their bug trackers and not here.
  • I searched for similar open issues and could not find an existing bug report on this.

📍 Description

When sending newly crafted backpacks of the same size thru cargo from one chest to another the backpacks became stacked and all had the same id #.

📑 Reproduction Steps

Put a dozen or so newly crafted backpacks into a chest.
setup a cargo node to blacklist to send to another chest
put a same sized backpack into the cargo node on whitelist
as items transfer they will stack.

💡 Expected Behavior

I would expect backpacks to keep their lore and not stack.

📷 Screenshots / Videos

https://youtu.be/O00yWZRFdno

📜 Server Log

No response

📂 /error-reports/ folder

No response

💻 Server Software

Paper

🎮 Minecraft Version

1.17.x

⭐ Slimefun version

dev 987

🧭 Other plugins

No response

@Boomer-1 Boomer-1 added 🎯 Needs testing This Issue needs to be tested by our team to see if it can be reproduced. 🐞 Bug Report A bug that needs to be fixed. labels Dec 20, 2021
@variananora variananora added 🧨 Confirmed Bug This bug has been confirmed by our team. and removed 🎯 Needs testing This Issue needs to be tested by our team to see if it can be reproduced. labels Dec 21, 2021
@variananora
Copy link
Member

Just speculating, since backpacks are Slimefun items, the optimization on #3258 probably have caused this side effects.

@Sfiguz7
Copy link
Member

Sfiguz7 commented Dec 21, 2021

I believe this is a much longer standing issue, ever since IDs were added in 1.14: to optimize comparisons between SlimefunItems we have a quick ID check here https://github.com/Slimefun/Slimefun4/blob/master/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java#L279, which would see two backpacks being the same even though they have different lore (#3258 does make this happen more often but it is not the root cause).
I am honestly unsure how to fix this: we could add a specific check for backpacks and fall back to lore checks in that case before returning true but this would probably be hard to expand - I can't think of any other item that would use meta information AND is stackable (most are armour/tools) but more may come, we could add them to a Set to check Set#contains() and fall back to lore checks in those cases but it seems... Unwise.
I summon le Cookie and le Blob for advice on how to work around it in a better way if possible @TheBusyBiscuit @WalshyDev

@Sefiraat
Copy link
Member

I have just tested this with my DankPacks from dt2 and they are effected in the same way. SlimeTinker tools/repair kits would likely be effected if they were itemstacks that could stack.

Cargo.Stacking.mp4

@WalshyDev
Copy link
Member

Hmm, I'm not sure how to handle this. I'd like to try and not revert but how do we know if it's an item to check lore of. Any ideas or opinions?

@Sefiraat
Copy link
Member

Uhm, I might be a big dummy but something in SlimefunItem like boolean enforceLoreCheck that defaults false and have cargo check this then only do the previous/extended check as/when told to do so? I assume we have the SlimefunItem at this point?

If dumb - sorry xD

@char3210
Copy link
Contributor

char3210 commented Dec 21, 2021

Is SlimefunUtils.isItemSimilar supposed to check for equal values or are the items allowed to differ in meta or nbt?

@NCBPFluffyBear
Copy link
Contributor

NCBPFluffyBear commented Dec 21, 2021

Is SlimefunUtils.isItemSimilar supposed to check for equal values or are the items allowed to differ in meta or nbt?

For vanilla items, it checks ItemMeta. For Slimefun items, the IDs are compared (In the context of cargo).

@char3210
Copy link
Contributor

the problem is that slimefun backpacks have the same id

@J3fftw1
Copy link
Member

J3fftw1 commented Jan 2, 2022

This also happens on other items that are stack able for example in LiteXpansion you can allow the glasscutter with 0 joules to merge with 1 with 300 joules.
Also tested it with Crysta and Brilliance Scoop when the item misses an use and you put it in cargo with an item with full uses it also combines

@Sefiraat
Copy link
Member

Sefiraat commented Jan 2, 2022

Ah. I could probably have rechargeable extend Distinct and the scoop is limited use so I could make limited-use implement distinct also? I’ll modify when I get home

@kuzuanpa
Copy link

kuzuanpa commented Feb 19, 2022

all kinds of memory card (in addon Network ) are also affected by this bug.
it can store a single type of item,this bug could copy the items in it
so I have to change the Material of these item into chestplate (items that can't merge)to use it normally : )

@kuzuanpa
Copy link

so broken&fixed spawner and staff_elemental_storm is affected too.

@variananora
Copy link
Member

@kuzuanpa We already aware of the issue, you dont need to report every broken item.

@LaLlorrana
Copy link

LaLlorrana commented Oct 29, 2022

Any updates on this issue? Have run across several servers disabling the cargo manager and rendering the automation of Slimefun useless.

@Boomer-1
Copy link
Author

I believe that #3341 is also related to this.

@test137E29B
Copy link
Contributor

test137E29B commented May 8, 2023

This causes also a major duplication exploit with the Network's Addon (not caused by Network itself, but cargo).

  • Place an item into a Network Quantum storage that you want to dupe
  • Place that Quantum storage into the output chest
  • Place the same type of Quantum (but empty) into a chest that cargo will pull it out of and put it into the same chest as the other.

Cargo will create a stack of 2x the first Quantum instead of understanding based on Lore that they are different Quantums and do not both store the same item.

You can then place the Quantums down and have double the items. It works on any item, custom, unique, rare, keys, anything that's an ItemStack and can be placed into a Quantum can be duped because of this bug in Cargo.

@variananora variananora added the ✔ Resolved This Issue has been resolved. label Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug Report A bug that needs to be fixed. 🧨 Confirmed Bug This bug has been confirmed by our team. ✔ Resolved This Issue has been resolved.
Projects
None yet