Slimefun4 icon indicating copy to clipboard operation
Slimefun4 copied to clipboard

Errors in SlimefunUtils

Open mcchampions opened this issue 1 year ago • 4 comments

❗ Checklist

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

📍 Description

From #4211

In a word,there is a wrong check.

Problem tracing

https://github.com/Slimefun/Slimefun4/blob/4bcce20249f3cdfed19eb17aae6d17208c4f492f/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java#L461-L493

This code checks Lore & Name,The problem arises

https://github.com/Slimefun/Slimefun4/blob/4bcce20249f3cdfed19eb17aae6d17208c4f492f/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java#L383-L414

Check here to see if you have overlooked a situation where it is a slimefun item and the two items are not the same slimefun item

https://github.com/Slimefun/Slimefun4/blob/4bcce20249f3cdfed19eb17aae6d17208c4f492f/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java#L383-L397

The variable name is sfItem, which means that sfItem is a slimefun item by default. In the case of id==null (i.e. the item is not a Slimefun item), it is't necessary to continue to determine if it is necessary to return false if one is a Slimefun item and the other is not.

Also, since sfitem.hasItemMeta() has been judged Why should the variable of this ItemMeta be named possibleSfItemMeta, and the same goes for possibleItemId

In a word,maybe change like:

                 if (id != null && id.equals(possibleItemId)) { 
                     Debug.log(TestCase.CARGO_INPUT_TESTING, "  Item IDs matched!"); 
  
                     /* 
                      * PR #3417 
                      * 
                      * Some items can't rely on just IDs matching and will implement Distinctive Item 
                      * in which case we want to use the method provided to compare 
                      */ 
                     Optional<DistinctiveItem> optionalDistinctive = getDistinctiveItem(id); 
                     if (optionalDistinctive.isPresent()) { 
                         return optionalDistinctive.get().canStack(possibleSfItemMeta, itemMeta); 
                     } 
                     return true; 
                 }
                 return false;

📑 Reproduction Steps

After the method is called, an error result is returned. There is a wrong checking.

💡 Expected Behavior

The method should run correctly.

📷 Screenshots / Videos

No response

📜 Server Log

No response

📂 /error-reports/ folder

No response

💻 Server Software

Purpur

🎮 Minecraft Version

1.20.x

⭐ Slimefun version

latest

🧭 Other plugins

No response

mcchampions avatar Jun 27 '24 16:06 mcchampions

https://github.com/Slimefun/Slimefun4/assets/129668183/bbeeaeca-2bbf-4c28-a3a1-bfc7d3538f4d

See more #4211

Environment:

git-Purpur-2018 (MC: 1.20.1) Slimefun DEV-1148 Metrics-Module 29 Java 17

I used the machine and the command to see NBT from InfinityExpansion to reproduce the issue clearly.

This video reproduces the issue by using a machine from InfinityExpansion, but it is not related to InfinityExpansion. The reason for using this machine is simply because it is easier to reproduce. Difficulty reproducing this issue in environments with only Slimefun.

balugaq avatar Jun 27 '24 17:06 balugaq

It is not as same as the video in #4211

balugaq avatar Jun 27 '24 17:06 balugaq

Sorry, I misjudged, but that fork is causing problems with the code. But, SlimefunUtils does have this problem, and this method is wrong.

mcchampions avatar Jun 28 '24 00:06 mcchampions

Sorry, I misjudged, but that fork is causing problems with the code. But, SlimefunUtils does have this problem, and this method is wrong.

Now,This issue has nothing to do with that fork anymore

mcchampions avatar Jun 28 '24 00:06 mcchampions