search

Lothrazar / Cyclic

Minecraft mod written in Java
MIT License
160 stars 96 forks source link

Storage Bag causes item loss #1184

Closed ghost closed 5 years ago

ghost commented 5 years ago

Minecraft Version: 1.12.2

Forge Version: Reproduced on 14.23.5.2838.

Mod Version: Reproduced on 1.17.12-19.12.2, expect more.

Single Player or Server: Applies to both

Steps to Reproduce (Yeh i deleted the dscripte yor prblem thing):

  1. Give yourself a Storage Bag, open it up and set it to grab any items automatically;
  2. Give yourself any item, then look straight down, press Q to drop it;
  3. Open Storage Bag and wait for the item.

Observed result: Item won't appear. Expected result: Item should appear. Occurrence rate: 100%

Further reproduction:

  1. Look straight down, then drop the item;
  2. Open the Storage Bag, then spam click Bag's slots randomly for 5 seconds;
  3. Close the bag, then open it up again.

Observed result: Item is gone. Expected result: Item should be right there. Occurrence rate: 100%

Causes:

  1. Storage Bag GUI does not update properly when its inventory gets updated in server side;
  2. Storage Bag inventory is determined on client side only, and saved to server side regardless of server's tick update.

Video: Project 1.zip (I reproduced this on instances without other mods later, so don't conclude this as mod compatibility issue)

Workaround: Try not to open Storage Bag when picking up items or Disable Storage Bag temporarily until there's a patch addressing this issue.

Comment:

  1. Override inventory NBT information directly from client side is extremely dangerous, as hackers can make use of this by using a modified version of Cyclic and just give them any items they want.
  2. I've also experienced another issue where all items within Storage Bag are lost (I recovered it from server backup), suspecting it being the same issue.
Lothrazar commented 5 years ago

The first few times i couldnt recplicate this. then i got it to happen, just like the video it happens if you open the GUI during the pickup, during some desync type where the items picked up but not saved, but if you open the bag it overwrites something . Race condition maybe. or like you said overriding NBT directly is dangerous

Lothrazar commented 5 years ago

it should save to the server side data on picukp. And refresh server->client only when it needs to (ie when pickup completse or when gui opens)

Lothrazar commented 5 years ago

I recorded a gif of your video for convenience so i dont have to redownload . https://imgur.com/iGGV1yu Going to try adding a "gui-is-open" flag of sorts, and then the auto pickup will be disabled when open. should avoid the issue.

Lothrazar commented 5 years ago

https://www.curseforge.com/minecraft/mc-mods/cyclic/files/2755366

KernelDeimos commented 1 year ago

yikes. This can still happen when picking up ammo with mrcrayfishs gun mod - maybe it overrides this somehow.