Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Condition "Name" of certain items in player's inventory is broken when item have lore #4054

Closed
tiradorus opened this issue Jun 3, 2021 · 9 comments
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: medium Issues that are detrimental to user experience (prohibitive bugs or lack of useful implementation).

Comments

@tiradorus
Copy link

tiradorus commented Jun 3, 2021

Description

When you have a script, the all item's name check (or certains items name) into an inventory, the Name condition is considered as false if the item have lore on it.
However, it works if the item does not have any lores.

I'll send you the script that i've done. You can try it yourself, it works, does not send any errors.
But if your named item have a lore, then it's not counted into the condition, even if the name is the same.

command /changes:		
    trigger:
        set {_diampur} to amount of diamond named "&f&lDiamant épuré" in player's inventory
        remove all diamond named "&f&lDiamant épuré" from player's inventory
        wait 5 ticks
        make console execute command "/mi give material DIAMANTPLUS %player% %{_diampur}%"
        message "%{_diampur}%" to player

On the code above, it count all the diamonds named "&f&lDiamant épuré" into player's inventory and removes them.
Afterwards, it gives to the player, the number of the items removed, to another item type.
It's an echange with NameCondition on items...

BUT! If the diamonds (On the script) have a lore, then, they are not counted into "{_diampur}", even if the name is still "&f&lDiamant épuré"

Without lore on the items, it works.
With lore, it's not working.

Skript version: 2.5.3
Minecraft version: 1.16.5

@TPGamesNL
Copy link
Member

Please fill in the entire format:

### Description
<!--- What happened? --->

### Steps to Reproduce
<!--- What did you do to make it happen? Provide as accurate instructions as possible! --->

### Expected Behavior
<!--- What should have happened? --->

### Errors / Screenshots
<!--- Console errors and screenshots of visual bugs in game, if you have any --->

<!---
If you have console errors, copy them to a paste service which won't delete them.
DON'T use Hastebin!
--->

<!--
Screenshots of bugs visible in-game can also be attached.
--->

### Server Information
* **Server version/platform:** <!-- /version -->
* **Skript version:** <!-- /version Skript ("latest" is not useful, we need the version number) -->

### Additional Context
<!--- Anything you'd like to add? This can be left empty. --->

@TPGamesNL TPGamesNL added the waiting for reply The report needs a response from the reporter to determine course of action. label Jun 3, 2021
@tiradorus
Copy link
Author

tiradorus commented Jun 3, 2021

Description

When you have a script that count all items with a certain name into an inventory, the name condition isn't considered if the item have lore on it.
However, it works if the item does not have any lores.

I'll send you the script that i've done. You can try it yourself, it works, does not send any errors.
But if your named item have a lore, then it's not counted into the condition, even if the name is the same.

Steps to Reproduce

Use condition named item into a player's inventory, like that:
set {_diampur} to amount of diamond named "&f&lDiamant épuré" in player's inventory
Then try with an item with correct name, without lore, and another with a lore.

Expected Behavior

All items with the name should have been counted, with or without lores.

Actual Behavior

Only items without lores are counted

Errors / Screenshots

None, because there is no errors.

Server Information

  • Server version/platform: 1.16.5
  • Skript version: tryed on 2.5.3 and 2.6.0

Additional Context

I use this script, you can use it too and see what happens.

command /changes:		
    trigger:
        set {_diampur} to amount of diamond named "&f&lDiamant épuré" in player's inventory
        remove all diamond named "&f&lDiamant épuré" from player's inventory
        make console execute command "/mi give material DIAMANTPLUS %player% %{_diampur}%"
        message "%{_diampur}%" to player

@tiradorus tiradorus reopened this Jun 3, 2021
@tiradorus
Copy link
Author

However, there is a little change between 2.5.3 and 2.6.0.
On 2.5.3, the remove all diamond named "&f&lDiamant épuré" from player's inventory was corrupted by lores.
On 2.6.0 the remove all diamond named "&f&lDiamant épuré" from player's inventory isn't corrupted by lores

@TPGamesNL
Copy link
Member

Okay, so it works as expected on 2.6?

@tiradorus
Copy link
Author

tiradorus commented Jun 13, 2021

Nope, only the "Remove all diamonds ..." has been solved on 2.6.0
However the "set {_diampur} to amount of diamond named "&f&lDiamant épuré" in player's inventory" is still buged

That means 50% of the post has been solved with the 2.6.0

@tiradorus
Copy link
Author

Bump?

@TPGamesNL
Copy link
Member

@APickledWalrus pinging you since this is related to item comparisons

@APickledWalrus
Copy link
Member

This is because ExprAmountOfItems uses ItemType#isOfType(ItemStack). It should probably use a comparator instead where the ItemStack is the first argument and ItemType is the second argument (ItemStack comes first as we are verifying that it has at least the same qualities at the ItemType, but that it may also have more).

Ideally we could improve the situation of comparing items in ItemType directly (it's not really logical to have the important item comparisons in the comparators class) but because of how tied together these methods are with blocks and everything it becomes very difficult in terms of preventing bugs and making sure everything works the same.

I will add this to #4162 with the comparators fix as that will be the quickest for the time being (it is not a necessarily bad fix either).

@APickledWalrus APickledWalrus added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. PR available Issues which have a yet-to-be merged PR resolving it priority: medium Issues that are detrimental to user experience (prohibitive bugs or lack of useful implementation). and removed waiting for reply The report needs a response from the reporter to determine course of action. labels Jul 21, 2021
@APickledWalrus
Copy link
Member

Should be resolved in the PR shown above. If anyone reading this tests the nightly build, please leave feedback/problems there :)

@TPGamesNL TPGamesNL added completed The issue has been fully resolved and the change will be in the next Skript update. and removed PR available Issues which have a yet-to-be merged PR resolving it labels Oct 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: medium Issues that are detrimental to user experience (prohibitive bugs or lack of useful implementation).
Projects
None yet
Development

No branches or pull requests

3 participants