Ender IO Conduits

Ender IO Conduits

2M Downloads

Slow chargePlayersItems method because of detectAndSendChanges

Sir-Will opened this issue ยท 4 comments

commented

Issue Description:

The chargePlayersItems method calls detectAndSendChanges every player tick (if something is being charged) which makes it use slower than it should be because vanilla checks the inventory for advancements every time detectAndSendChanges is called which shouldn't be needed here because only the charge level is being changed and not a new item being added.

For which reason is detectAndSendChanges being called here? Is there maybe a better way which doesn't trigger the advancements check?

https://github.com/SleepyTrousers/EnderIO/blob/master/enderio-base/src/main/java/crazypants/enderio/base/power/wireless/WirelessChargerController.java#L80


Affected Versions (Do not use "latest"):

  • EnderIO: 1.3.10
  • EnderCore: 5.2.59
  • Minecraft: 1.12.2
  • Forge: 14.23.5.284
commented

The important bit behind that call is this.connection.sendPacket(new SPacketSetSlot(containerToSend.windowId, slotInd, stack));, without that the changed charge state of items won't propagate to the client reliably. In theory it shouldn't be necessary because detectAndSendChanges() also is called from onUpdate(), but for some reason that alone is not enough.

commented

PS: About a better way: We could copy the complete detectAndSendChanges() method and make a version that doesn't notify each IContainerListener but only the player. But I don't really like copying vanilla code for such changes, that's code that is next to impossible to maintain when updating to newer versions.

commented

Wouldn't make more sense to call this.connection.sendPacket(new SPacketSetSlot(containerToSend.windowId, slotInd, stack)); for each slot where an item was charged instead of going through the whole inventory and detecting changes?

commented

Yes, it would make complete sense to ignore all of Vanilla and Forge and implement more performant version of all of their code. But that's "out of scope" as the buzzwordians say nowadays.