GlowstoneMC / Glowstone

A fast, customizable and compatible open source server for Minecraft: Java Edition
https://glowstone.net
Other
1.89k stars 271 forks source link

Inventory sync issue #181

Closed Paulomart closed 8 years ago

Paulomart commented 8 years ago

Because images say more then words. I dobble click on in the inventory, gif http://i.imgur.com/EkdKMbW.gif

Console claims that it was rejected:

2016/01/25 22:31:14 [INFORMATION] Paulomart: [rejected] WindowClickMessage(id=7, slot=-999, button=0, transaction=4, mode=4, item=null)
2016/01/25 22:31:51 [INFORMATION] Paulomart: [rejected] WindowClickMessage(id=8, slot=3, button=0, transaction=1, mode=0, item=ItemStack{STAINED_GLASS_PANE x 1, UNSPECIFIC_META:{meta-type=UNSPECIFIC, display-name=§f}})
2016/01/25 22:31:51 [INFORMATION] Paulomart: [rejected] WindowClickMessage(id=8, slot=3, button=0, transaction=2, mode=6, item=null)
2016/01/25 22:31:53 [INFORMATION] Paulomart: [rejected] WindowClickMessage(id=8, slot=-999, button=2, transaction=5, mode=5, item=null)
2016/01/25 22:31:54 [INFORMATION] Paulomart: [rejected] WindowClickMessage(id=8, slot=66, button=0, transaction=6, mode=0, item=ItemStack{STAINED_GLASS_PANE x 10, UNSPECIFIC_META:{meta-type=UNSPECIFIC, display-name=§f}})
2016/01/25 22:31:54 [INFORMATION] Paulomart: [rejected] WindowClickMessage(id=8, slot=-999, button=2, transaction=17, mode=5, item=null)
2016/01/25 22:31:55 [INFORMATION] Paulomart: [rejected] WindowClickMessage(id=8, slot=85, button=0, transaction=18, mode=0, item=null)
2016/01/25 22:31:56 [INFORMATION] Paulomart: [rejected] WindowClickMessage(id=8, slot=49, button=0, transaction=19, mode=0, item=ItemStack{STAINED_GLASS_PANE x 1, UNSPECIFIC_META:{meta-type=UNSPECIFIC, display-name=§f}})
2016/01/25 22:31:56 [INFORMATION] Paulomart: [rejected] WindowClickMessage(id=8, slot=49, button=0, transaction=20, mode=6, item=null)
2016/01/25 22:31:57 [INFORMATION] Paulomart: [rejected] WindowClickMessage(id=8, slot=-999, button=2, transaction=25, mode=5, item=null)

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Paulomart commented 8 years ago

Hotbar updates instand because of a plugin. Items that you put into you player inventory, are gone when reopening the inventory (self or container)

mastercoms commented 8 years ago

Could I see the plugin's code?

Paulomart commented 8 years ago

Sure, I created a Test plugin: https://github.com/Mystalion/GlowTest/tree/inventory-sync

mastercoms commented 8 years ago

Not sure if we use events correctly for the click handler, then. Or, we aren't telling the client that they can't take that item.

Paulomart commented 8 years ago

Handler seems to be fine, the server is not accepting the transaction. Maybe a client side bug that is worked around in craftbukkit?

Paulomart commented 8 years ago

When clicken outsite it now gives a exception:

2016/02/11 17:57:53 [SCHWERWIEGEND] Error while handling WindowClickMessage(id=4, slot=-999, button=0, transaction=50, mode=0, item=null) (handler: WindowClickHandler)
java.lang.ArrayIndexOutOfBoundsException: -999
    at java.util.Arrays$ArrayList.get(Unknown Source)
    at net.glowstone.inventory.GlowInventory.getItem(GlowInventory.java:320)
    at net.glowstone.net.handler.play.inv.WindowClickHandler.process(WindowClickHandler.java:234)
    at net.glowstone.net.handler.play.inv.WindowClickHandler.handle(WindowClickHandler.java:31)
    at net.glowstone.net.handler.play.inv.WindowClickHandler.handle(WindowClickHandler.java:26)
    at net.glowstone.libs.com.flowpowered.networking.session.BasicSession.handleMessage(BasicSession.java:80)
    at net.glowstone.libs.com.flowpowered.networking.session.BasicSession.messageReceived(BasicSession.java:139)
    at net.glowstone.net.GlowSession.pulse(GlowSession.java:428)
    at net.glowstone.net.SessionRegistry.pulse(SessionRegistry.java:23)
    at net.glowstone.scheduler.GlowScheduler.pulse(GlowScheduler.java:173)
    at net.glowstone.scheduler.GlowScheduler.access$100(GlowScheduler.java:24)
    at net.glowstone.scheduler.GlowScheduler$2.run(GlowScheduler.java:110)
    at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source)
    at java.util.concurrent.FutureTask.runAndReset(Unknown Source)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(Unknown Source)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(Unknown Source)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
    at java.lang.Thread.run(Unknown Source)
Paulomart commented 8 years ago

glowtest

Also the items can get in the bottom inventory if you shift click.

mastercoms commented 8 years ago

As of this commit: https://github.com/GlowstonePlusPlus/GlowstonePlusPlus/commit/930a3c8156ad7e200fee959d5ceb9dc5fc1e2e53, the outside slot should work, but I'm not sure if or how the cursor item should be reset.

Also, the shift click issue seems to be client side only - is there another hack bukkit did to resolve it?

The bug also happens with the number keys.

Paulomart commented 8 years ago

Not sure, what CraftBukkit did there, maybe updating the whole inventory would be faster and better?

mastercoms commented 8 years ago

maybe we could guess where the item went for the client and then tell it to revert the change?

Paulomart commented 8 years ago

Would have to mess with the DragTracker to do that

Paulomart commented 8 years ago

Also happing when open a new inventory on click, the item is still there then.

mastercoms commented 8 years ago

What do you mean?

Paulomart commented 8 years ago

Maybe adapt this change Mystalion/MysticStone@db6895debe9ce25af7feaf714dbea58ca9238ce5

Paulomart commented 8 years ago

Cannot provide a test case now, ask @summsi if he has time

mastercoms commented 8 years ago

Is there a GIF of the issue that you could show me?

EDIT: ok, @summsi could you please provide a video, or gif of the issue?

summsi commented 8 years ago

ezgif com-video-to-gif 1

mastercoms commented 8 years ago

Are you sure the inventory is cancelling events for the bottom inventory as well?

Paulomart commented 8 years ago

No, because we care only about the top one

mastercoms commented 8 years ago

Could I see the code for this plugin?

Paulomart commented 8 years ago

Really big lib, will try to extract the bug and put in Mystalion/GlowTest

Paulomart commented 8 years ago

https://github.com/Mystalion/GlowTest/blob/inventory-sync/src/main/java/com/mystalion/glowtest/GlowTestPlugin.java

inventory2

Paulomart commented 8 years ago

The class plane shuld not be there.

Paulomart commented 8 years ago

Note: I fixed this in my patch: https://github.com/Mystalion/MysticStone/blob/master/patches/0003-Fix-inventory-sync-issues.patch

mastercoms commented 8 years ago

And all window click logic works? If you want, send it as a PR.

Paulomart commented 8 years ago

It only calls updateInventory so I guess it should not break anything. Will PR tomorrow