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

Item comparisons are too specific #3618

Closed
wallxq opened this issue Dec 8, 2020 · 14 comments
Closed

Item comparisons are too specific #3618

wallxq opened this issue Dec 8, 2020 · 14 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: high Issues with potentially high impact that could be harmful to users.

Comments

@wallxq
Copy link

wallxq commented Dec 8, 2020

Description

I have updated Skript to the latest version and noticed that a certain function stopped working for me.
I migrate from 2.5 alpha5.

Steps to Reproduce

https://pastebin.com/mFVyYpec

Expected Behavior

I created just any script. Anything should happen. I want to show that any item that has its name changed color is undetectable.

Errors / Screenshots

There are no erros.

Server Information

I use Paper 1.16.4 version 318 with AdoptOpenJDK 11 HotSpot
I tested on skript 2.5.1 and 2.5.2. There is the same problem on both versions.

@ShaneBeee
Copy link
Contributor

I tested your code, both worked fine for me:
Screen Shot 2020-12-07 at 7 44 47 PM
Screen Shot 2020-12-07 at 7 45 39 PM

@wallxq
Copy link
Author

wallxq commented Dec 8, 2020

I already know what the problem is. Skript does not detect item by name if item has lore or uses flags to hide for example "attributes". In the old version of skript, there was no problem with this.

@APickledWalrus
Copy link
Member

Skript is explicitly checking for a diamond sword with the given name. If there are any other modifiers, the check will fail. I think you are meaning to check the name specifically. If so, you could do:

if the name of the event-item is "&6Sword +0":

@wallxq
Copy link
Author

wallxq commented Dec 9, 2020

Skript is explicitly checking for a diamond sword with the given name. If there are any other modifiers, the check will fail. I think you are meaning to check the name specifically. If so, you could do:

if the name of the event-item is "&6Sword +0":

That works! Thanks you 👍

@wallxq wallxq closed this as completed Dec 9, 2020
@wallxq
Copy link
Author

wallxq commented Dec 9, 2020

Okay but what if item is in inventory?
if player has 1 diamond sword named "&6Sword +0" with custom model data 81:
or
if player has 1 diamond sword named "&6Sword +0":
don't work, no errors

@wallxq wallxq reopened this Dec 9, 2020
@APickledWalrus
Copy link
Member

Are you sure the item doesn't have any other modifiers? It has to match exactly. If you aren't able to check for the specific items, you could loop through the player's inventory items and compare the type and name of each item to determine if it is a match.

@wallxq
Copy link
Author

wallxq commented Dec 9, 2020

The point is that I have quite complicated items that I wanted to easily extract and capture in a script. When upgrading, skript no longer works "named" or "has". Now I'm forced to loop all inventory and use "if name".

@APickledWalrus
Copy link
Member

APickledWalrus commented Dec 9, 2020

Recently changes were made to how item comparison works. A lot of previous behavior that did work no longer does, as it wasn't intentional. I could be wrong about how this is supposed to work though, so I'll leave this open for input from the other developers.

@wallxq
Copy link
Author

wallxq commented Dec 9, 2020

I believe this has more harm than good in it. This makes it a bit more difficult. For me, it means that I have to change a lot now and I have what because it is a few MB of scripts for over 400 players.

@APickledWalrus
Copy link
Member

Okay so, after some investigating and discussion, I was mistaken. This actually should work if the "second" item has at least the properties of the "first" item. Having more is okay. To explain that with an actual example:

player's tool = diamond sword of sharpness 1 named "Sharp Sword"

if the player's tool is a diamond sword of sharpness 1: # SHOULD WORK
if a diamond sword of sharpness 1 is the player's tool: # SHOULD NOT WORK - this is because the player's tool also has a name.

I think this is how it used to work, but the behavior was broken after a change to item comparisons.

@wallxq wallxq closed this as completed Dec 10, 2020
@APickledWalrus APickledWalrus changed the title Problem in detecting items with colored names. Item comparisons are too specific Dec 12, 2020
@APickledWalrus APickledWalrus added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. priority: high Issues with potentially high impact that could be harmful to users. labels Dec 12, 2020
@wallxq
Copy link
Author

wallxq commented Dec 30, 2020

I noticed another problem. Many plugins make items easy. Like here:
{display: {Name: '{"text": "Blood Pearl"}'}, CustomModelData: 103}

After saving such an item to a variable and checking it with has item, it works fine.
The problem occurs after a server restart. The item written in the variable becomes:
{display: {Name: '{"extra": [{"bold": true, "italic": false, "underlined": false, "strikethrough": false, "obfuscated": false, "color": "gold "," text ":" Blood Pearl "}]," text ":" "} '}, CustomModelData: 103}
Which means that an item created by plugins is not recognized by has item. Even more strange, if the player re-enters the server, the item will get similar values ​​and only then has the item work.

@wallxq
Copy link
Author

wallxq commented Dec 30, 2020

It would be great if script could compare these two items to each other because they are actually the same without looking at the metadata.

@wallxq
Copy link
Author

wallxq commented Dec 31, 2020

Paper converting the json color to the "another" color system after reconnect player or restart server. It would be good if Skript could recognize item only through custom model data. Because in the current case of using "if player has" he wants 1 to 1 to be accurately given the whole.

@wallxq
Copy link
Author

wallxq commented Dec 31, 2020

if items in player's inventory contains its i think addon?
Works as it should "if player has item".

@TPGamesNL TPGamesNL added the PR available Issues which have a yet-to-be merged PR resolving it label Mar 23, 2021
@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 May 9, 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: high Issues with potentially high impact that could be harmful to users.
Projects
None yet
Development

No branches or pull requests

4 participants