堆叠物品疑似并不检查粘液id

[balugaq]

问题描述

如题
对于特定物品，粘液会直接让他们堆叠，即使他们粘液id不同

特定物品：名字相同，lore相同，但粘液id不同（即nbt存在不同，修复bug不需要考虑完整的nbt，只考虑粘液id就够了）

问题复现率

必现

复现步骤

QQ2024627-205635.mp4

服务端类型

Purpur

Minecraft 版本

1.20.x

Slimefun 版本

构建 180
https://builds.guizhanss.com/StarWishsama/Slimefun4/master/builds/180

其他插件信息

同name同lore但nbt不同的物品使用SlimeCustomizer生成
nbt使用InfinityExpansion查看

补充信息

No response

[mcchampions]

问题溯源

Slimefun4/src/main/java/me/mrCookieSlime/Slimefun/api/inventory/DirtyChestMenu.java

Lines 178 to 183 in 226af9b

	if (SlimefunItem.getByItem(item) != null) {
	// Patch: use sf item check
	if (!SlimefunUtils.isItemSimilar(stack, wrapper, true, false)) {
	continue;
	}
	} else {

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

Lines 306 to 322 in 226af9b

	} else if (checkDistinctiveItem
	&& sfitem instanceof SlimefunItemStack stackOne
	&& item instanceof SlimefunItemStack stackTwo) {
	if (stackOne.getItemId().equals(stackTwo.getItemId())) {
	/*
	* 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
	*/
	if (stackOne instanceof DistinctiveItem && stackTwo instanceof DistinctiveItem distinctiveItem) {
	return distinctiveItem.canStack(stackOne.getItemMeta(), stackTwo.getItemMeta());
	}
	return true;
	}
	return false;
	} else if (item.hasItemMeta()) {

我不理解，正常来说应该不能堆叠吧，除非这里它SlimefunItem.getByItem(item) == null或者是有一个ItemStack不能转换成SlimefunItemStack(还真是，ItemStackWrapper转不了)

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

Lines 348 to 387 in 226af9b

	} 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 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;
	} else {
	Debug.log(
	TestCase.CARGO_INPUT_TESTING,
	" Item IDs don't match, checking meta {} == {} (lore: {})",
	itemMeta,
	possibleSfItemMeta,
	checkLore);
	return equalsItemMeta(itemMeta, possibleSfItemMeta, checkLore, checkCustomModelData);
	}
	} else if (sfitem.hasItemMeta()) {

前半部分乍一看没啥毛病，检查了粘液科技的id，那么问题肯定出在后半部分上的equalsItemMeta方法

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

Lines 459 to 501 in 226af9b

	private static boolean equalsItemMeta(
	@Nonnull ItemMeta itemMeta,
	@Nonnull ItemMeta sfitemMeta,
	boolean checkLore,
	boolean bypassCustomModelCheck) {
	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;
	}
	}
	if (!bypassCustomModelCheck) {
	// 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;
	}

这里有一说一这个传入的checkCustomModelData变量在这个方法里叫做bypassCustomModelCheck，实际用法上也是忽略检查的意思，那它传入时理应逆转，可是它传入时并没有逆转诶

这段代码就是检查Lore和Name然后就过了，问题就来了

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

Lines 364 to 386 in 226af9b

	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;
	} else {
	Debug.log(
	TestCase.CARGO_INPUT_TESTING,
	" Item IDs don't match, checking meta {} == {} (lore: {})",
	itemMeta,
	possibleSfItemMeta,
	checkLore);
	return equalsItemMeta(itemMeta, possibleSfItemMeta, checkLore, checkCustomModelData);
	}

这里检查是不是忽略了一种情况，它是粘液物品并且两个物品并不是同一种粘液物品
建议大概改成这样:

                if (id != null) {
                    if (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;
                } else {

后面的Debug输出的内容也许要改下？

[mcchampions]

问题溯源

发现了个问题，在这里，这里传入的变量名是sfItem,也就意味着默认sfItem是粘液物品了。在id==null(即item不是粘液科技物品的情况下)，是否有必要继续判断呢，在一个是粘液物品一个不是的情况下直接返回false就行了吧，最后应该这样修改

                 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;

还有，既然已经判断sfitem.hasItemMeta()了
为什么还要将这个ItemMeta的变量命名为possibleSfItemMeta,possibleItemId同理

[balugaq]

问题溯源

发现了个问题，在这里，这里传入的变量名是sfItem,也就意味着默认sfItem是粘液物品了。在id==null(即item不是粘液科技物品的情况下)，是否有必要继续判断呢，在一个是粘液物品一个不是的情况下直接返回false就行了吧，最后应该这样修改

                 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;

还有，既然已经判断sfitem.hasItemMeta()了 为什么还要将这个ItemMeta的变量命名为possibleSfItemMeta,possibleItemId同理

但是在视频中，我给出的光源方块都是粘液物品，从nbt里就可以看出

[balugaq]

反馈上游的话我可能得另外录制一个视频

[mcchampions]

不，我发现这个isItemSimilar是汉化版自己加上去的，有一说一，完全没必要，移除了这个就可以正常运行。但是上游的那个方法里还是有问题。还有这个checkCustomModelData也是汉化版自己加上去的，是汉化版没有逆转用法

[mcchampions]

问题是汉化版的问题，但汉化版出问题又可以通过改上游进行解决

[StarWishsama]

SlimeCustomizer 物品配置发一个，我没怎么用过附属

[mcchampions]

SlimeCustomizer 物品配置发一个，我没怎么用过附属

SLIMEFUN_ITEM_1: #id不同
  category: slime_customizer
  item-type: CUSTOM
  item-name: "&b名称相同"
  item-lore:
  - "LORE相同"
  item-id: STICK
  item-amount: 1
  placeable: false
  crafting-recipe-type: NONE 
  crafting-recipe:
    1:
      type: NONE
      id: N/A
      amount: 1
    2:
      type: NONE
      id: N/A
      amount: 1
    3:
      type: NONE
      id: N/A
      amount: 1
    4:
      type: NONE
      id: N/A
      amount: 1
    5:
      type: NONE
      id: N/A
      amount: 1
    6:
      type: NONE
      id: N/A
      amount: 1
    7:
      type: NONE
      id: N/A
      amount: 1
    8:
      type: NONE
      id: N/A
      amount: 1
    9:
      type: NONE
      id: N/A
      amount: 1
SLIMEFUN_ITEM_2: #id不同
  category: slime_customizer
  item-type: CUSTOM
  item-name: "&b名称相同"
  item-lore:
  - "LORE相同"
  item-id: STICK
  item-amount: 1
  placeable: false
  crafting-recipe-type: NONE 
  crafting-recipe:
    1:
      type: NONE
      id: N/A
      amount: 1
    2:
      type: NONE
      id: N/A
      amount: 1
    3:
      type: NONE
      id: N/A
      amount: 1
    4:
      type: NONE
      id: N/A
      amount: 1
    5:
      type: NONE
      id: N/A
      amount: 1
    6:
      type: NONE
      id: N/A
      amount: 1
    7:
      type: NONE
      id: N/A
      amount: 1
    8:
      type: NONE
      id: N/A
      amount: 1
    9:
      type: NONE
      id: N/A
      amount: 1      

大致就这样，事实上这个问题出自汉化版自己加的检测（说实话这个检测本来就有问题）

[mcchampions]

对了，通过仔细阅读代码发现理论上这种符合name相同lore相同的粘液物品在其使用的原版物品不同的情况下也会堆叠

[StarWishsama]

尝试新 Insider 版

[mcchampions]

对了，由此延伸出去可以发现不同使用次数的元素拐杖可以堆叠