Error stack and some incorrect use of variables in the code
mcchampions opened this issue · 22 comments
❗ 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 SlimefunGuguProject/Slimefun4#912,This fork does not change the code related to this issue.
The following is machine translation:
Original content:For certain items, slimefun will directly make them stack, even if they have different slimefun IDs
Specific items: Same name, same lore, but different slimefun ID (i.e. NBT exists differently, you don't need to consider the full NBT to fix the bug, just consider the slimefun ID)
Problem tracing
Herein lies the problem——equalsItemMetamethod
Here's a thing to say about the checkCustomModelData variable in this method called bypassCustomModelCheck, which actually ignores checking, so it should be reversed when it is passed in, but it is not reversed when it is passed in
This code checks Lore & Name,The problem arises
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
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;In a word,there is a wrong check and some incorrect use of variables(or wrong name?)
📑 Reproduction Steps
Put different slimefun items with the same lore and name into the charging bench
And then they stacks.
💡 Expected Behavior
they shouldn't stack
📷 Screenshots / Videos
343782159-dc0ad5a8-745b-4829-8c6e-148c2552e470.mp4
📜 Server Log
No response
📂 /error-reports/ folder
No response
💻 Server Software
Paper
🎮 Minecraft Version
1.20.x
⭐ Slimefun version
latest
🧭 Other plugins
No response
as part of the review process the dev's can look into it to see if the root cause is addressed in the PR or if other changes need to be made to look at #4212
Boomer he is meaning different issues, it sounds like some other menus/methods combine slimefun items that shouldn't combine like how cargo does
It's completely different, and the charging bench is just a convenient demonstration machine, not just the charging bench with this issue.
then as i said, open a seperate issue
If you read it carefully, you will see that this is not the same.
if the cause of the other issue is what you identified then it's the same. open a seperate issue not related to the charging bench
if the cause of the other issue is what you identified then it's the same. open a seperate issue not related to the charging bench
That's not the cause of the problem, it's talking about items that can't be stacked but stacked,but i will open a new issue
That PR maybe just fix the charging bench. But it is SlimefunUtils's wrong in fact.
charging bench is a duplicate issue. also please report issues on seperate report. also we aren't responsible for any forks of slimefun.
charging bench is a duplicate issue. also please report issues on seperate report. also we aren't responsible for any forks of slimefun.
No,Actually, I'm talking about the problem with the charging bench , but I found a lot of errors in this logic in the process.and the problem was found on this branch, but the branch doesn't change the code, so it's safe to assume that the problem works on the main branch as well(In fact, it does)
again, it's a duplicate issue. it's already been reported, and it's never safe to assume a fork doesn't affect code.
again, it's a duplicate issue. it's already been reported, and it's never safe to assume a fork doesn't affect code.
Yes,It's reported in other fork,but it isn't reported in here.And,in fact,is does't affect code,this question exists
again, it's a duplicate issue. it's already been reported, and it's never safe to assume a fork doesn't affect code.
Yes,It's reported in other fork,but it isn't reported in here.And,in fact,is does't affect code,this question exists
In fact,this issue is not quite the same as that one
it is reported here. #4183
it is reported here. #4183
Emm.no same
it is reported here. #4183
Emm.no same
you said you have items stacking in the charging bench on the official version. that was already reported in 4183. this is closed. move on