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

Removing item from inventory ignores extra data on the item #2354

Closed
piotrskibapl opened this issue Aug 8, 2019 · 6 comments
Closed

Removing item from inventory ignores extra data on the item #2354

piotrskibapl opened this issue Aug 8, 2019 · 6 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

@piotrskibapl
Copy link

Description

I found it impossible to remove only not enchanted items from the inventory.

Steps to Reproduce

  1. Install this script:
command /test:
	trigger:
		clear player's inventory
		give player diamond pickaxe
		give player diamond pickaxe of efficiency 1
		give player diamond pickaxe of efficiency 2
		
		set {_item} to "diamond pickaxe" parsed as item
		
		remove 3 of {_item} from player's inventory
  1. Run /test as a player

Expected Behavior

Only one pickaxe should be removed from inventory, as the {_item} variable contains the exact item, not itemtype. Instead, all 3 pickaxes are removed.

Server Information

  • Server version/platform: git-Paper-134
  • Skript version: 2.4-beta5

Additional Context

In this issue I am assuming that diamond pickaxe and diamond pickaxe of efficiency 1 are not the same things when parsed as item - at least that's what I found in the documentation:

Unlike item type an item can only represent exactly one item

@piotrskibapl
Copy link
Author

I just also found out that this issue appeared in 2.4-beta5 version. In 2.4-beta4 the code above works properly and the issue is not present.

@Wealthyturtle
Copy link
Member

Seems like it's removing an item type rather than an item

@TheBentoBox TheBentoBox 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 Aug 26, 2019
@TheBentoBox
Copy link
Member

I can confirm this as an issue and it can cause huge issues in some cases, hence the high priority.

Personal example: on my server I have a script which allows players to sent items directly to each other (e.g. /itemsend <player> [<amount>] ). I calculate what they're trying to send, store it in a variable, validate that they have it, then remove it from their inventory and add it to a queue for the target player to claim.

Due to this bug, If a player is trying to send a newly crafted diamond pickaxe, it will instead save the plain pickaxe in a variable, give that to the target player (putting it in their queue), and remove the first diamond pickaxe it finds in the user's inventory instead of the correct one that they're holding. Often that will end up being their main pick over in one of their first couple of slots, which ends up being straight up deleted as a result.

I had to update multiple scripts to use skript-mirror to remove the item from the player's inventory instead of using Skript's remove effect.

@TheBentoBox TheBentoBox changed the title Removing item from inventory ignores if the item is enchanted Removing item from inventory ignores extra data on the item Aug 29, 2019
@bensku bensku added the completed The issue has been fully resolved and the change will be in the next Skript update. label Sep 1, 2019
@bensku
Copy link
Member

bensku commented Sep 1, 2019

I hope my quick fix doesn't break anything else.

bensku added a commit that referenced this issue Sep 1, 2019
@bluelhf
Copy link
Contributor

bluelhf commented Sep 1, 2019

@bensku How reassuring! 😅
I'm sure it's fine.

@rudde0
Copy link

rudde0 commented Sep 6, 2019

It was fixed in beta6 but appeared again in beta7. #2404

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

6 participants