Failed inventory transaction does not rollback cursor inventory on client's end

[Muqsit]

Issue description

• Expected result: What were you expecting to happen? When an inventory transaction involving a cursor inventory fails, the cursor inventory gets rolled back.
• Actual result: What actually happened? When an inventory transaction involving a cursor inventory fails, the cursor inventory gets rolled back on the server's side, but not on the client's side.

Steps to reproduce the issue

1. Cancel an InventoryTransactionEvent where the inventory transaction involves a cursor inventory. The video shows the transaction rolling back (causing the item to be placed back in the chest), however the cursor inventory still holds the item ("ghost item"). On interacting with an inventory slot, the server corrects back the contents of the cursor inventory.

https://user-images.githubusercontent.com/15074389/109976664-da9a4900-7d1d-11eb-8f5d-bdca1510a30d.mp4

OS and versions

• PocketMine-MP: 3907ae6726fd9f46914832d2608cdb9f5617667d
• PHP: 7.4.15
• Server OS: Ubuntu 20.04.1 LTS
• Game version: Win10

Plugins

• Can you provide sample, minimal reproducing code for the issue? If so, paste it in the bottom section
  1. Using this script plugin https://gist.github.com/Muqsit/d0dca479cdcdbc48711ec42ee5a86e79, run /itetest as a player, which will spawn a chest at the location of the command sender. The chest contains a stained glass pane.
  2. Move the stained glass pane to your cursor inventory.
  3. The plugin cancels all inventory transaction events, so attempting to move the pane will result in the transaction rolling back. However, there is still a ghost glass pane item in the cursor inventory on the client's end.

Debugging

While the server does send the client an inventory update... https://github.com/pmmp/PocketMine-MP/blob/bbae02264ddf02c3b4f0b1e2ec2ac337e3516f40/src/network/mcpe/handler/InGamePacketHandler.php#L327) ...it seems like InvManager::syncContents() does not trigger a slot update for PlayerCursorInventory on client's end. InvManager::syncSlot() on the other hand seems to do the trick.

/**
 * @param InventoryTransactionEvent $event
 * @priority MONITOR
 * @handleCancelled true
 */
public function fixCursorInventoryNotUpdatingClientSide(InventoryTransactionEvent $event) : void{
    if($event->isCancelled()){
        $transaction = $event->getTransaction();
        $inv_manager = $transaction->getSource()->getNetworkSession()->getInvManager();
        foreach($transaction->getActions() as $action){
            if($action instanceof SlotChangeAction){
                $inv_manager->syncSlot($action->getInventory(), $action->getSlot());
            }
        }
    }
}

https://user-images.githubusercontent.com/15074389/109978759-061e3300-7d20-11eb-91d7-b42626d08416.mp4

[dktapps]

This has been broken since 1.13 because of the clusterfuck with "UI inventory".

[ghost]

It is also possible to fix by setting the players cursor inventory to air when the transaction is failed