Uploaded image for project: 'Spigot'
  1. Spigot
  2. SPIGOT-6478

Close animation missing for double chest inventories opened by a plugin

    XMLWordPrintable

Details

    • This server is running CraftBukkit version dev-Spigot-9fb885e-fd905ab (MC: 1.16.5) (Implementing API version 1.16.5-R0.1-SNAPSHOT)
    • Yes

    Description

      When a plugin opens the inventory of a double chest for a player, the double chest remains in its open animation state even after the player has closed the inventory.

      This issue matches the descriptions of the following past issues:

      One can use the code provided in https://hub.spigotmc.org/jira/browse/SPIGOT-4871 to reproduce this issue.

      My current assumption of what is causing this issue is as follows:
      I assume that this regression has been caused by https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/b2de47d581bd20a11ba4a667342e8f08f16ee564#src/main/java/org/bukkit/craftbukkit/entity/CraftHumanEntity.java
      For unknown reasons, this commit changed the check for whether the passed inventory can be opened via EntityHuman#openContainer from

      if (iinventory instanceof ITileInventory) {
          // Open via openContainer
      } else {
          // Open as custom inventory
      }

      to

      if (iinventory instanceof TileEntityContainer) {
          // Open via openContainer
      } else {
          // Open as custom inventory
      }
      

      Since a double chest (BlockChest.DoubleInventory) is of type ITileInventory, but not TileEntityContainer, this opens the double chest inventory via the 'openCustomInventory' method.

      (Side note: Many custom tile entity inventories probably use openContainer just fine currently although they are 'custom', i.e. not backed by a TileEntity in the world, but a virtual TileEntity. So this openCustomInventory method is currently mainly used for custom chest-like inventories).

      The (custom) inventories that are opened this way use CraftContainer. This type of container internally creates a 'delegate' Container to achieve some of its functionality related to handling shift clicks. This also explains why the double chest open animation apparently plays fine: During construction of this delegate ChestContainer, it informs the underling double chest inventory about the container being opened by a player, which informs the two involved chest tile entities, which update their state and play their open animation.

      However, CraftContainer never delegates the call

      b(EntityHuman entityhuman)

      (i.e. the callback for when the player closes the container) to the 'delegate' Container.

      In the case of a double chest inventory, this delegate Container would probably further delegate this call to the two chest tile entities that are wrapped by the double chest inventory, which would then update their viewer counts and chest animation states; similar to how the Container behaves that is used when the double chest inventory is opened via openContainer, as it is the case when a player interacts with a double chest block in Minecraft.

      However, since CraftHumanEntity#openCustomInventory is probably only meant for inventories that are actually custom and not backed by a TileEntity, and which would therefore usually not require to delegate this container-closed-callback to any underlying tile entities, it probably makes more sense to just revert the above mentioned change and open the double chest inventory via openContainer again in the first place. I will prepare a small PR for this.

      Attachments

        Activity

          People

            Unassigned Unassigned
            blablubbabc blablubbabc
            Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: