Buuz135 / FunctionalStorage

FunctionalStorage
MIT License
30 stars 33 forks source link

[FEATURE REQUEST] Don't handle compacting drawer interactions client-side #159

Closed rlnt closed 1 year ago

rlnt commented 1 year ago

Description: CompactingDrawerBlock#use calls CompactingDrawerTile#onSlotActivated client-side and server-side. As we try to make more and more mods compatible with Almost Unified and submit pull requests so they get integration directly, this now has a very huge disadvantage when something like your Compacting Drawer requests information on the client side.

When the Compacting Drawer processes the player interaction and its block entity tries to find the lower tier in CompactingUtil#findLowerTier, it will cause a crash because other libraries/mods use the AlmostUnifiedLookup API to find the preferred item for a specific tag. When this happens client-side, the game will crash as the Almost Unified runtime is not available on the client. Making it available to the client would be difficult. Configs and a ton of information would have to be synced to the client or the whole unification process would have to happen on the client as well.

While being pretty challenging, this would be very inefficient too. The unification happens only on the server. Clients joining the server get the server recipes synced through the network in the recipe serializers fromNetwork method. That means recipes are already unified upon arrival and the process doesn't need to happen again. So the runtime on the client side would be quite useless.

Since Functional Storage took inspiration from Storage Drawers, I tested it there as well and the crash doesn't happen on their end because they don't handle the Compacting Drawer interaction on the client. And I also don't really see a reason why you would do that. I didn't spend a lot of time looking at your mod's internals but why does the client need to know about the lower and upper tiers of the inserted material? When the item is inserted and the inventory of the drawer is updated on the server, you could simply inform the client about an inventory change and sync it. That way, information is only processed on the server and clients would also save some performance since they wouldn't need to do the tier lookup on their own.