ChangeInventoryEvent item duplication & weird behavior

[CDFN]

I am currently running

• SpongeForge version: -
• SpongeVanilla version: The one from commit https://github.com/SpongePowered/Sponge/commit/e20a7d1091e6d9b58966d8ff872d97ee98c7aedf
• Java version: 15.0.1
• Operating System: Windows 10

• Plugins/Mods:

  [17:43:13] [Server thread/INFO]:     Minecraft v1.16.5
  [17:43:13] [Server thread/INFO]:     SpongeAPI v8.0.0
  [17:43:13] [Server thread/INFO]:     Sponge v1.16.5-8.0.0
  [17:43:13] [Server thread/INFO]:     SpongeVanilla v1.16.5-8.0.0
  [17:43:13] [Server thread/INFO]:     Hub Plugin v1.0.0

Issue Description Today I encountered weird issue with ChangeInventoryEvent. Hard to explain what happens so I made short video showing problem: https://youtu.be/AV9kfiCUt7I Basically items are duping when moving them around in inventory using SHIFT+LMB. I was dropping them out using CTRL+Q. Normal clicking LMB in inventory moves items either. It started happening after adding listener for ChangeInventoryEvent.

Code:

public class PlayerItemListener {

  private final HubPlayerRepository hubPlayerRepository;

  @Inject
  public PlayerItemListener(@Named("nonpersistent") HubPlayerRepository hubPlayerRepository) {
    this.hubPlayerRepository = hubPlayerRepository;
  }

  @Listener
  public void onUse(InteractItemEvent.Secondary event, @First Player player) {
    var itemType = event.getItemStack().getType();

    hubPlayerRepository.getPlayer(player.getUniqueId()).thenAccept(hubPlayer -> {
      var optionalStatefulItem = hubPlayer.getStatefulItems()
          .stream()
          .filter(statefulItem -> statefulItem.getItemType().equals(itemType))
          .findFirst();

      if (optionalStatefulItem.isEmpty()) {
        return;
      }

      var statefulItem = optionalStatefulItem.get();
      var useResult = statefulItem.onUse(event.getItemStack());

      //TODO: Use InteractEvent#handType once https://github.com/SpongePowered/SpongeAPI/pull/2312 gets merged
      var optionalHandType = event.getCause().getContext().get(EventContextKeys.USED_HAND);
      if (optionalHandType.isEmpty()) {
        return;
      }

      var handType = optionalHandType.get();
      var inventory = player.getInventory();

      if (HandTypes.MAIN_HAND.get().equals(handType)) {
        var selectedSlot = inventory.getHotbar().getSelectedSlotIndex();
        inventory.set(selectedSlot, useResult);
        return;
      }
      inventory.getOffhand().set(useResult);

    });
  }

  @Listener
  @Exclude({ChangeInventoryEvent.Held.class})
  public void onItemMove(ChangeInventoryEvent event){
    event.setCancelled(true);
  }
}

[ImMorpheus]

Please follow the issue template, it's actually rather important if you want to get dev attention to fix your issue.

Edit this issue with the required details / template, and I'll reopen and hide this comment.

[CDFN]

@ImMorpheus Done 👌 Sorry for not following the template, I was creating issue in SpongeAPI and I think templates aren't configured there 👀

[JBYoshi]

I was able to reproduce the dropped items part on commit 02135c2a9b6d824725a44813c819de705b6242b9 using just the onItemMove listener, although I can't reproduce the shift-clicking bug anymore. (EDIT: The same code does reproduce the bug on the older build RC378, so I'll declare that part of the issue fixed.)

The cause of the dropped items bug seems to that API 8 no longer waits to spawn entities until the current phase completes. Instead, it spawns them immediately and relies on the phase to despawn them. (So putting a call to processSpawnedEntities only when the event is not cancelled doesn't prevent them from spawning.)

[gabizou]

Ah yes, I was beginning to wonder whether there'd be some conflict of organization here, It's difficult to say what could be the better solution. I may have to have a think on this.