(Basically copying from Discord, as requested by 2No2Name. Thanks for considering this.)

I have a mod called Cooldown Coordinator¹ which is used by my mod Ductwork and a few others to allow modded hopper-like things to interact with hoppers in the same manner hoppers do (it provides a safe method to attempt to notify any block entity of a transfer cooldown and will process the notification if the block entity implements its interface). Lithium now bypasses the entire vanilla hopper insert, which includes the point where CC inserts the call for abstract hopper extenders.

Being as CC is a library mod many users don't even know is installed, I can't reasonably ask folks to go disable Lithium's hopper optimizations in order to restore stable item transfer behavior. So I am looking for a way to get the functionality back into the vanilla hopper reimplementation in Lithium. The main issue is Lithium's implementation, like the vanilla implementation, only notifies other hopper blocks².

Is there f.e. some way Lithium devs would be willing to provide an event I could use to discover when the conditions for notification are met (excluding the one about it being a Hopper which is what I'm trying to get around)? The timing check also has to be a bit different³.

A couple of updates relative to the original Discord request:

1. squirl pointed out I could use Lithium's API to turn off features, so for the moment I have done that. Therefore this is not a big rush on my part.

2. I'm happy to try and do whatever would accommodate Lithium's needs in this case. I have discussed the possibility of eventually merging CC into Fabric's Transfer API with Technici4n and I want to try and keep CC generally in a state the Fabric folks might accept eventually.

3. The issue here is reproducible, but it is pretty technical. Moving items from a hopper to a Ductwork duct with a hopper beneath it is probably the simplest way to reproduce. The hopper below the duct should have time to acquire the item, but because the first hopper does not notify the duct of its cooldown, the duct will instead move the item along before the second hopper can acquire it. Disabling Lithium's hopper optimizations as expected resolves this issue.

I am considering to add an API like this. Would this make sense and help resolve the issue?

package me.jellysquid.mods.lithium.api.inventory;

import net.minecraft.util.math.Direction;

public interface HopperInteractedInventory {
    /**
     * Called after Lithium's hopper code transfers an item to the inventory.
     * Can be used to resolve mod compatibility issues that arise from injecting into vanilla hopper code while Lithium
     * replaces the hopper code.
     * Be aware that this function is only called by Lithium's hopper code and not by vanilla hopper code.
     *
     * Example implementation assuming that the Inventory implements Vanilla's Inventory interface:
     * (Not required to implement HopperInteractedInventory interface with implements-clause, but not harmful either)
     * void onItemInsertedByHopper(Direction fromDirection) {
     *      //Do stuff
     * }
     *
     * @param fromDirection The direction the hopper was transferring from
     */
    default void onItemInsertedByHopper(Direction fromDirection) {
    }

    /**
     * Called after Lithium's hopper code transfers an item from the inventory to the hopper.
     * Can be used to resolve mod compatibility issues that arise from injecting into vanilla hopper code while Lithium
     * replaces the hopper code.
     * Be aware that this function is only called by Lithium's hopper code and not by vanilla hopper code.
     *
     * Example implementation assuming that the Inventory implements Vanilla's Inventory interface:
     * (Not required to implement HopperInteractedInventory interface with implements-clause, but not harmful either)
     * void onItemExtractedByHopper(Direction fromDirection) {
     *      //Do stuff
     * }
     *
     * @param toDirection The direction the hopper was transferring to (In case of vanilla-like behavior: Direction.DOWN)
     */
    default void onItemExtractedByHopper(Direction toDirection) {
    }
}

Something like that should work, but the above looks like it is intended to be more general and would probably lack the pre-check I am needing. This seems like something intended to notify of every insertion (or extraction, but those do not involve setting a cooldown on the target).

How about adding an arg indicating whether the target was previously empty before the insertion? You'd have to break this big multi-state evaluation back up so you can later get at just whether it was empty and not also whether it was a hopper:
https://github.com/CaffeineMC/lithium-fabric/blob/1.19.x/dev/src/main/java/me/jellysquid/mods/lithium/mixin/block/hopper/HopperBlockEntityMixin.java#L175

The information whether the inventory was empty would need to be computed every time, which adds unnecessary lag. So I do not really want to do that for all inventories when it is only needed rarely. You could count the items in all slots and if there is one item in total, the inventory was empty before the transfer.

Hmm, that's a good point. Frankly, I do not think the optimizations Lithium makes here are going to prove feasible in the long run; they change vanilla assumptions too much. If it does work out, that's going to be mostly because folks aren't writing mods using vanilla Inventories anymore (and we're not there yet). For example, Lithium is assuming there is nothing interesting in the isEmpty() method of modded inventories.

I also dispute the suggestion the information is rarely needed; the normal case for a hopper network when moving items is moving a single item at a time with each hopper cycling between empty and one item, and that is the case for modded duct or item-based pipe mods as well. The worst optimization will be applied to the normal case, because CC will have to iterate over the entire Inventory with a custom method to answer a question which was previously answered by the target Inventory itself (via calls to isEmpty().

The reason the 7/8 tick difference exists is item transfer networks are not acyclic and item movement order has to be stabilized in some manner, so it is important to get this right or unexpected behavior does result. Suggesting mods should be satisfied with post-transfer evaluation is likely to cause changes in behavior (preventing which is the entire point of CC), and furthermore it is not thread-safe.

Can you give an example of a reasonable interesting thing that can happen inside isEmpty()? I think Lithium's assumption that isEmpty() doesn't have any side effects is reasonable.

The information is only needed when the hopper interacts with such modded inventories. This is a rare case from my point of view. I understand that you will always use the information in your special inventories and you don't like implementing a count function for all potential inventories. I think we have to think of another solution then.

I am thinking about a cooldown interface which I will implement for vanilla hoppers. Modded inventories could implement the same interface. This would require a compile dependency on lithium though

Inventories with hidden slots can lie about whether they are empty.

The reason the 7/8 tick difference exists is item transfer networks are not acyclic and item movement order has to be stabilized in some manner, so it is important to get this right or unexpected behavior does result. Suggesting mods should be satisfied with post-transfer evaluation is likely to cause changes in behavior (preventing which is the entire point of CC), and furthermore it is not thread-safe.

I don't understand how this is related to the issue. Your implementation does not seem to care about the 7/8 tick difference, as in your implementation the modded inventory seems to decide itself what happens to it when it receives the "The inventory was empty but now an item has been transferred into it"-event.

Okay the easiest way to do it on your side is to just mixin into lithium's code and add your hook the same way as you do in vanilla code.

On lithium's side I can

• get rid of optimizations
• offer some kind of interface

I can offer this I think:
Inventories that want to be set on cooldown like hoppers implement certain methods (without compile dependency):

• void setTransferCooldown() //Can be called by lithium and maybe by your vanilla hooks
• boolean canReceiveTransferCooldown() //False unless override

Lithium can check canReceiveTransferCooldown() before the transfer. If true, calculate whether the inventory is empty, then transfer and if successful, call setTransferCooldown. Would you think this is reasonable?

Also by compile dependency I did not mean that users have to install lithium. It just means that compiling the code requires lithium, which is already super annoying and not recommended.

I am thinking about a cooldown interface which I will implement for vanilla hoppers. Modded inventories could implement
the same interface. This would require a compile dependency on lithium though

This is going the wrong direction IMO. The last thing I want is to tell folks they have to use Lithium if they want to coordinate cooldowns, or to have two different implementations of cooldown coordination depending on whether Lithium is installed. Not everybody wants to use Lithium, especially when it starts making assumptions which may impact mod behavior.

I can do what you suggested (search inventories), I just categorically disagree the performance optimization Lithium is making is worth it, so of course I am arguing against this. It is easy enough for Lithium to force me to either implement the check-later-and-pray approach or suppress Lithium's hopper optimizations, either of which would be better than adding another standard for coordination.

https://xkcd.com/927/

I don't understand how this is related to the issue.

It's not. I'm doing too much at once and I apologize for that little non-sequitur.

Lithium can check canReceiveTransferCooldown() before the transfer. If true, calculate whether the inventory is empty,
then transfer and if successful, call setTransferCooldown. Would you think this is reasonable?

(CC is already opt-in, so) If I'm understanding this correctly, what would actually happen is CC would implement both methods and mods using CC would not have to do anything new (but could override) ... this seems feasible. Are you planning on using reflection for this? (Oh, it'll just be blind implementing an interface, probably.) I don't particularly mind depending on Lithium to build CC. Basically it amounts to something like:

boolean canReceiveTransferCooldown() {
    // Let Lithium handle cooldown notification between hoppers in whatever manner it expects will work...
    return (this instanceof HopperBlockEntity) ? false : true;
}

void setTransferCooldown() {
    CooldownCoordinator.notify(this);
}

Also by compile dependency I did not mean that users have to install lithium. It just means that compiling the code
requires lithium, which is already super annoying and not recommended.

They would have to install either Lithium or CC to get coordinated cooldowns (or both is usually what would happen).

Sorry for the delay, I forgot about this request until now. I will implement something now so you can test it

Build here: https://github.com/CaffeineMC/lithium-fabric/actions/runs/3769100431
Lithium code: bf9b43f
You can probably implement your thing it like this:

public class MyCustomInventory implements Inventory {
  boolean canReceiveTransferCooldown() {
      return true;
  }
  
  void setTransferCooldown(long currentTime) {
      CooldownCoordinator.notify(this);
  }
}

👍 I'll test this out but it'll probably take me a bit to get to it; I started in on the next project in line which is kind of fully occupying my brainspace at the moment.

Closing as this seems to be done. You can of course request more changes when needed