DistinctiveItem/isSimilar + Backpack canStack

[Sefiraat]

trafficstars

Description

Supercedes #3384

SlimefunItemStacks that share an ID are currently automatically stacked when running through cargo. Items that vary by lore or another meta aspect are being 'merged' with items that happen to match on ID but are, otherwise, distinct. See issue #3379. This likely only effects backpacks within core Slimefun but also effects any similar SlimefunItems using similar distinctions - DankPacks and Tinkers parts (I'm so self centred!) being an example.

Proposed changes

New ItemAttribute called DistinctiveItem to act as an identifier for items that match this criterion. Distinctive item has a method that allows an implementation to say how two same-ID items can be distinguished. SlimefunBackpack has been amended to implement this interface. SlimefunUtils#isItemSimilar now checks if the stack's SlimefunItem is a DistinctiveItem and, if so, return the result of the canStack method.

Discussion with Walshy shows this is making an extra call to get the ID from the Meta that was already made in the previous getItemData that may mean this PR needs more work.

Related Issues (if applicable)

Resolves #3379

Checklist

• [x] I have fully tested the proposed changes and promise that they will not break everything into chaos.
• [x] I have also tested the proposed changes in combination with various popular addons and can confirm my changes do not break them.
• [x] I followed the existing code standards and didn't mess up the formatting.
• [x] I did my best to add documentation to any public classes or methods I added.
• [x] I have added Nonnull and Nullable annotations to my methods to indicate their behaviour for null values
• [ ] I added sufficient Unit Tests to cover my code.

[Sefiraat]

Comment from the previous PR I'm closing:

So, near as I can tell, the 5th branch (line 280) is never called by items moving through cargo though it will be reached when directly calling isItemSimilar. Given that meta is required to check distinction this will could add a lot of overhead to any other machine using this. Given the above, do we still want to check distinction outside of cargo - given that we do not currently and it, in itself, hasn't shown issues. Input welcome for how to add this in.

Also - changes in commit 92b45ad tested under the same conditions with expected and wanted results.

[Sefiraat]

I have added the check to both the SlimefunItemStack/SlimefunItemStack branch and to the ItemStack/SlimefunItemStack branches which should now cover all possible routes

[StarWishsama]

This change will cause backpack unable to upgrade

[Boomer-1]

any chance this is being pushed out? It may fix a couple other issues. #3341 and #3379

[J3fftw1]

any chance this is being pushed out? It may fix a couple other issues. #3341 and #3379

Patience :pansmug: @Sefiraat is working on a different fix which is way better then this iirc

[Boomer-1]

Awesome. it could fix a multitude of issues at once.

[Sefiraat]

It needs a touch up to fix issues with backpacks upgrading but it's not going to be worked on until I know if the general direction is viable/approvable. This isn't the PR in which I was working on a better idea though I'm afraid, that was one focused on networks :)

[Sefiraat]

It was mentioned a long while ago in discord that this is likely not the correct approach, but I'm not really sure I have a better one. I know this PR is live on the Chinese fork, not sure how they've worked around the backpack upgrading however

[github-actions[bot]]

Slimefun preview build

A Slimefun preview build is available for testing!

https://preview-builds.walshy.dev/download/Slimefun/3417/5420663511

Note: This is not a supported build and is only here for the purposes of testing. Do not run this on a live server and do not report bugs anywhere but this PR!

[Sefiraat]

Given how long it has been since this PR was first raised, if someone else other than myself wants to give the preview build a whirl before it's merged. Hopefully we can get this in finally over the weekend!