EssentialsX / Essentials

The modern Essentials suite for Spigot and Paper.
https://essentialsx.net
GNU General Public License v3.0
1.94k stars 979 forks source link

Items are randomly swapped and deleted when using invsee #3065

Open emkirsh opened 4 years ago

emkirsh commented 4 years ago

Information

Full output of /ess version:

EssentialsX version: 2.17.1.62
LuckPerms version: 5.0.72
PlaceholderAPI version: 2.10.4
Vault version: 1.7.2-b107
Towny version: 0.95.2.0
EssentialsXChat version: 2.17.1.62
EssentialsXGeoIP version: 2.17.1.62
EssentialsXSpawn version: 2.17.1.62

Server log: I can't begin to explain why, but for some reason, logs haven't generated in my logs folder since the 3rd of March. It's the 12th and I've been running the server daily for about a week. Luckily, I kept the terminal open and I will copy and paste from there and call it latest.log.

EssentialsX config: My EssentialsX Config

Details

Description
So, pretty much, I was invsee-ing my alt, which was AFK fishing, to transfer items without going through the teleporting and re-setting the AFK fisher. I saw a few items glitching, but then it was fine. (or so I thought). Then, a few minutes later, I noticed my pickaxe had the wrong durability. After looking through and double-checking my inventory, I realized I only had one! I had originally had two pickaxes because I was planning on merging enchants in an anvil. I didn't even have the good one! So, I recreated the problem and found out that when moving around items from one slot to another, sometimes they will be placed, seemingly at random, in another spot, and if there's already an item there, the original item gets deleted. Permanently. This happens more when you are moving the items quickly.

This doesn't appear to happen when shift-clicking to move items.
One other thing, when looking at the emeralds in the video clip: When I accidentally discovered this, my emeralds disappeared. I found them in my alt's off-hand, which is quite strange considering the fact that off-hands aren't supported in /invsee.

Steps to reproduce

  1. /invsee someone.
  2. move an item in your inventory from one spot to another, preferably quickly.
  3. watch as the items are randomly switched to a different spot
    1. if the items are switched to a slot that already has an item, it will replace the original item, deleting it permanently. clicking all of the empty slots brings no luck.
  4. hope you record it so you can spawn back the right enchants on your items, or you're out of luck 😉!
  5. report it so it gets fixed.

Expected behavior
I would expect the /invsee command to work normally (as intended), and not delete my items when I move them.

Screenshots
Here is a gif hosted on gyazo of this happening. Please disregard the log of me enchanting items. That was just me respawning the items from my first time.

https://i.gyazo.com/0535a911d0976ca4bace071829c0a37f.mp4

Let me know if you need anything else! Thanks and have a nice night. I hope everything's okay with Coronavirus and stuff, and I understand if the response is a bit slow amidst all this. Thanks!

-emkirsh

emkirsh commented 4 years ago

Oh, I logged back in with my alt that I was testing on. There was a bow as a helmet and a fishing rod as a chestplate. The bucket was also in the offhand. So, I guess it doesn't all go away completely, but most of it does.

pop4959 commented 4 years ago

Confirmed on the following setup:

[00:44:33 INFO]: Server version: 1.15.2-R0.1-SNAPSHOT git-Paper-118 (MC: 1.15.2)
[00:44:33 INFO]: EssentialsX version: 2.17.2.1
[00:44:33 INFO]: LuckPerms version: 5.0.72
[00:44:33 INFO]: Vault version: 1.7.2-b107
[00:44:33 INFO]: EssentialsXProtect version: 2.17.2.1
[00:44:33 INFO]: EssentialsXChat version: 2.17.2.1
[00:44:33 INFO]: EssentialsXGeoIP version: 2.17.2.1
[00:44:33 INFO]: EssentialsXAntiBuild version: 2.17.2.1
[00:44:33 INFO]: EssentialsXSpawn version: 2.17.2.1

It seems the slots are incorrectly mapped when you do the movement action very quickly. Very strange. I've never noticed this because I typically take a second or two to move items. Also, it seems they are mapped to the same location For example, the top middle slot of your own inventory appears to teleport to the observed player's legging slot.

emkirsh commented 4 years ago

Yeah, and some just get flat-out deleted!

Chew commented 4 years ago

Maybe this is asking for too much, but would it be better to convert this to a GUI, and also add a row for the armour and off hand? "Could" fix things and also improve on it

emkirsh commented 4 years ago

Well, considering this is a plugin, not a mod, the only GUIs available are those in vanilla Minecraft. As far as I know, the only way to make a fully-integrated GUI with custom graphics and is not in the form of a vanilla inventory in Minecraft is to have a client-side mod and a server-side equivalent mod installed.

emkirsh commented 4 years ago

plugins were made so that the players don't have to install mods to begin with, and even the most sophisticated GUIs on a server accessible by a vanilla client still make use of the chest-like inventory function.

Chew commented 4 years ago

I wasn't clear, OpenInv for example has this: image

Top 4 rows mimic inventory, bottom row is armor + off hand. Something like this could work. Plus this works for offline players

emkirsh commented 4 years ago

oh cool! and the armor would be great for punishing people offline! i used to manually go into my friend's playerdata files to paste in a cursed pumpkin with vanishing and binding on their heads.

donvdp commented 4 years ago

I am also experiencing this issue on my servers.

Running paper-136 with latest essentialsx dev build 2.18.0.22. Also experiencing this issue on the combination paper-133 with 2.18.0.0.

donvdp commented 4 years ago

bugg still excists in version 2.18.1.0. ive created a small movie about it: https://youtu.be/ihI5M5T4I0M

niqoar commented 3 years ago

Same here.. with /ec also happens

12.09 10:09:04 [Server] INFO Server version: 1.16.2-R0.1-SNAPSHOT git-Tuinity-"0b86de3" (MC: 1.16.2) 12.09 10:09:04 [Server] INFO EssentialsX version: 2.18.1.0 12.09 10:09:04 [Server] INFO PlaceholderAPI version: 2.10.10-DEV-108 12.09 10:09:04 [Server] INFO LuckPerms version: 5.1.96 12.09 10:09:04 [Server] INFO Vault version: 1.7.3-b131 12.09 10:09:04 [Server] INFO EssentialsXChat version: 2.18.1.0 12.09 10:09:04 [Server] INFO EssentialsXSpawn version: 2.18.1.0

JRoy commented 3 years ago

Closing this issue as there is nothing we can do about it but I will leave it pinned to the issue tracker

mdcfe commented 3 years ago

Unpinning - see https://github.com/EssentialsX/Essentials/issues/3956#issuecomment-771694504 for recommendations.

tanyaofei commented 10 months ago

Recently, I also used the openInventory provided by Bukkit to access another player's inventory and has this same issue. However, I've identified the cause:

The player's inventory has a total of 40 slots. Indexes 0 through 8 refer to the hotbar. 9 throught 35 refer to the main inventory, indexes 36 throught 39 refer to the armor slots, index 40 refers to the off hand. However, when the client opens a player's ineventory, it can only seen slots 0 through 35, indexs greater than 36 refer to clientside player's inventory;

If a player uses a regular click (InventoryClickEvent), this issue doesn't arise. However, when a player drags items (or performs actions too quickly, causing the client to interpret it as a drag operation), it triggers an InventoryDragEvent. For instance, if a player accidentally uses dragging to place an item into their inventory's first slot (index 36), Bukkit, upon receiving this event, thinks index 36 referers to the player's helmet slot, equipping the item as a helmet. Because the clientside player cannot see items beyond index 36, it assumes the item has disappeared.

Here is my solution: Listen to InventoryDragEvent and cancel it if player drag items to their inventory

@EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
public void onDropFakePlayerInventory(@NotNull InventoryDragEvent event) {
    var top = event.getView().getTopInventory();
    if (top.getType() == InventoryType.PLAYER && top.getHolder() instanceof Player player ) {
        if (event.getInventorySlots().stream().anyMatch(slot -> slot > 35)) {
            event.setCancelled(true);
        }
    }
}
pop4959 commented 10 months ago

Hey thanks for letting us know - we'll take a look (or if you like, feel free to make a PR yourself with the proposed fix).

tanyaofei commented 4 weeks ago

Hey thanks for letting us know - we'll take a look (or if you like, feel free to make a PR yourself with the proposed fix).

Hey, hello! Has the bug been fixed? if not, i'd be happy to submit a PR to fix it.