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

Improve Item Comparisons #3960

Merged

Conversation

APickledWalrus
Copy link
Member

Description

Same as #3633 (I think I ported it fine)

This PR aims to fix an issue I created with item comparisons in #3419.
That PR made the comparison checks end up only working for EXACT matches. Really, I should've been using something like SAME_ITEM.

However, when I made my changes, it fixed an issue with comparing for unmodified items. You could do if player's tool is a diamond sword, but that would match ANY diamond sword. If you wanted to match to an unmodified diamond sword, you could do if player's tool is a diamond sword named "", but in the end, that feels kind of hacky.

To solve this issue, I've also added ExprPlain. The expression takes in an ItemType and returns an unmodified version that is tagged as plain through ItemData.

I've improved the metadata checks and comparators to properly use SAME_ITEM matches AND to handle the "new" plain items. I have also expanded and updated the current item comparison test accordingly.

I'm thinking some parts may be able to be improved, so any feedback is appreciated. While I tested this PR as best I could (through the test suite and in game), more rigorous in game testing would be beneficial.

Right now, I am going to target 2.5, but this PR very well may end up in 2.6.


Target Minecraft Versions: Any
Requirements: None
Related Issues: #3618

@APickledWalrus APickledWalrus added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label May 8, 2021
@FranKusmiruk FranKusmiruk merged commit da6f6a3 into SkriptLang:master May 9, 2021
@APickledWalrus APickledWalrus deleted the fixes/item-comparisons branch September 6, 2023 21:21
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants