Slimefun

Slimefun

Bukkit Plugins
3M Downloads

Errors in SlimefunUtils

mcchampions opened this issue · 4 comments

commented

❗ 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

From #4211

In a word,there is a wrong check.

Problem tracing

Slimefun4/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java

Lines 461 to 493 in 4bcce20

private static boolean equalsItemMeta(@Nonnull ItemMeta itemMeta, @Nonnull ItemMeta sfitemMeta, boolean checkLore) {
if (itemMeta.hasDisplayName() != sfitemMeta.hasDisplayName()) {
return false;
} else if (itemMeta.hasDisplayName() && sfitemMeta.hasDisplayName() && !itemMeta.getDisplayName().equals(sfitemMeta.getDisplayName())) {
return false;
} else if (checkLore) {
boolean hasItemMetaLore = itemMeta.hasLore();
boolean hasSfItemMetaLore = sfitemMeta.hasLore();
if (hasItemMetaLore && hasSfItemMetaLore) {
if (!equalsLore(itemMeta.getLore(), sfitemMeta.getLore())) {
return false;
}
} else if (hasItemMetaLore != hasSfItemMetaLore) {
return false;
}
}
// Fixes #3133: name and lore are not enough
boolean hasItemMetaCustomModelData = itemMeta.hasCustomModelData();
boolean hasSfItemMetaCustomModelData = sfitemMeta.hasCustomModelData();
if (hasItemMetaCustomModelData && hasSfItemMetaCustomModelData && itemMeta.getCustomModelData() != sfitemMeta.getCustomModelData()) {
return false;
} else if (hasItemMetaCustomModelData != hasSfItemMetaCustomModelData) {
return false;
}
if (itemMeta instanceof PotionMeta && sfitemMeta instanceof PotionMeta) {
return ((PotionMeta) itemMeta).getBasePotionData().equals(((PotionMeta) sfitemMeta).getBasePotionData());
}
return true;
}

This code checks Lore & Name,The problem arises

Slimefun4/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java

Lines 383 to 414 in 4bcce20

} else if (sfitem instanceof ItemStackWrapper && sfitem.hasItemMeta()) {
Debug.log(TestCase.CARGO_INPUT_TESTING, " is wrapper");
/*
* Cargo optimization (PR #3258)
*
* Slimefun items may be ItemStackWrapper's in the context of cargo
* so let's try to do an ID comparison before meta comparison
*/
Debug.log(TestCase.CARGO_INPUT_TESTING, " sfitem is ItemStackWrapper - possible SF Item: {}", sfitem);
ItemMeta possibleSfItemMeta = sfitem.getItemMeta();
String id = Slimefun.getItemDataService().getItemData(itemMeta).orElse(null);
String possibleItemId = Slimefun.getItemDataService().getItemData(possibleSfItemMeta).orElse(null);
// Prioritize SlimefunItem id comparison over ItemMeta comparison
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 {@link DistinctiveItem}
* 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;
} else {
Debug.log(TestCase.CARGO_INPUT_TESTING, " Item IDs don't match, checking meta {} == {} (lore: {})", itemMeta, possibleSfItemMeta, checkLore);
return equalsItemMeta(itemMeta, possibleSfItemMeta, checkLore);
}

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

Slimefun4/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java

Lines 383 to 397 in 4bcce20

} else if (sfitem instanceof ItemStackWrapper && sfitem.hasItemMeta()) {
Debug.log(TestCase.CARGO_INPUT_TESTING, " is wrapper");
/*
* Cargo optimization (PR #3258)
*
* Slimefun items may be ItemStackWrapper's in the context of cargo
* so let's try to do an ID comparison before meta comparison
*/
Debug.log(TestCase.CARGO_INPUT_TESTING, " sfitem is ItemStackWrapper - possible SF Item: {}", sfitem);
ItemMeta possibleSfItemMeta = sfitem.getItemMeta();
String id = Slimefun.getItemDataService().getItemData(itemMeta).orElse(null);
String possibleItemId = Slimefun.getItemDataService().getItemData(possibleSfItemMeta).orElse(null);
// Prioritize SlimefunItem id comparison over ItemMeta comparison
if (id != null && id.equals(possibleItemId)) {

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

commented
QQ2024628-05254.mp4

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.

commented

It is not as same as the video in #4211

commented

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

commented

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