SlimefunGuguProject / Slimefun4

Slimefun4 (粘液科技) 汉化版 | Slimefun modified version in Simplified Chinese
GNU General Public License v3.0
255 stars 59 forks source link

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

Closed balugaq closed 2 months ago

balugaq commented 3 months ago

问题描述

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

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

问题复现率

必现

复现步骤

https://github.com/SlimefunGuguProject/Slimefun4/assets/129668183/dc0ad5a8-745b-4829-8c6e-148c2552e470

服务端类型

Purpur

Minecraft 版本

1.20.x

Slimefun 版本

f5a446fc873c4ae405f3a57dc9fdde91

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

其他插件信息

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

补充信息

No response

mcchampions commented 3 months ago
问题溯源 https://github.com/SlimefunGuguProject/Slimefun4/blob/226af9be61d1bfc582cbe235909579b9f4f605cd/src/main/java/me/mrCookieSlime/Slimefun/api/inventory/DirtyChestMenu.java#L178-L183 https://github.com/SlimefunGuguProject/Slimefun4/blob/226af9be61d1bfc582cbe235909579b9f4f605cd/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java#L306-L322 ~~我不理解,正常来说应该不能堆叠吧,除非这里它`SlimefunItem.getByItem(item) == null`或者是有一个ItemStack不能转换成SlimefunItemStack(还真是,ItemStackWrapper转不了)~~ https://github.com/SlimefunGuguProject/Slimefun4/blob/226af9be61d1bfc582cbe235909579b9f4f605cd/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java#L348-L387 前半部分乍一看没啥毛病,检查了粘液科技的id,那么问题肯定出在后半部分上的`equalsItemMeta`方法 https://github.com/SlimefunGuguProject/Slimefun4/blob/226af9be61d1bfc582cbe235909579b9f4f605cd/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java#L459-L501 **这里有一说一这个传入的`checkCustomModelData`变量在这个方法里叫做`bypassCustomModelCheck`,实际用法上也是忽略检查的意思,那它传入时理应逆转,可是它传入时并没有逆转诶** 这段代码就是检查Lore和Name然后就过了,问题就来了 https://github.com/SlimefunGuguProject/Slimefun4/blob/226af9be61d1bfc582cbe235909579b9f4f605cd/src/main/java/io/github/thebusybiscuit/slimefun4/utils/SlimefunUtils.java#L364-L386 这里检查是不是忽略了一种情况,它是粘液物品并且两个物品并不是同一种粘液物品 建议大概改成这样: ```java 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 optionalDistinctive = getDistinctiveItem(id); if (optionalDistinctive.isPresent()) { return optionalDistinctive.get().canStack(possibleSfItemMeta, itemMeta); } return true; } return false; } else { ``` 后面的Debug输出的内容也许要改下?
mcchampions commented 3 months ago

问题溯源

发现了个问题,在这里,这里传入的变量名是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 commented 3 months ago

问题溯源

发现了个问题,在这里,这里传入的变量名是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 commented 3 months ago

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

mcchampions commented 3 months ago

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

mcchampions commented 3 months ago

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

StarWishsama commented 3 months ago

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

mcchampions commented 2 months ago

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 commented 2 months ago

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

StarWishsama commented 2 months ago

尝试新 Insider 版

mcchampions commented 2 months ago

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