Block update for connected multiblock at load
SirEdvin opened this issue · 7 comments
I am the developer of UnlimitedPeripheralWorks, CC:T add-on that provides integration for different mods. As part of my add-on, I have some generic fabric integrations as well as MI.
Recently, I received a bug report about multiblock fluid tanks that don't work correctly.
The reasoning behind it is that when a fluid tank multiblock loads, it replaces internal fluid storage with a new one. And if the computer requests a peripheral before it, the peripheral is created, but with empty fluid storage, which is replaced later. Usually, in such cases, blocks use block state updates to notify neighboring blocks (and computers) that block state is changed or use some sort of generic invalidation, which Fabric doesn't have right now.
Can I ask you to introduce some sort of block update when multiblock are loaded so that computers can request peripherals to be reevaluated?
In theory you are not supposed to cache objects retrieved from an API Lookup because they might become invalid at any time. If you re-query the fluid storage every time it should work, right? (BlockApiCache should make this efficient performance-wise).
Indeed, @SquidDev correctly identified the issue. This is not really an MI problem in the sense that what MI is doing is within the spec of API Lookup.
I think that adding cache invalidation to Fabric is unlikely, but BlockApiCache should give sufficient performance. Worst case, one of you (not sure who is using the transfer API here) can use an implementation of Storage that re-queries the target every time, and falls back to Storage.empty() if the target is not available anymore.
BlockApiCacheshould give sufficient performance
Just to clarify here, its not the performance of the cache here which is the issue, but rather when we choose to query the block lookup API at all. We use the result of the lookup API to derive peripheral methods, which are then used off-thread. This means we either check this data every tick (inefficient, even with caching!) or when peripheral methods are called (means peripheral methods are likely to be inconsistent with the current state).
In this case it's actually tricker. Because MI starts off with an empty storage, CC doesn't generate methods for it (though UPW does), so we wouldn't be able to invalidate when calling peripheral methods anyway!
To be clear, I'm not expecting this MI or Fabric to solve this problem, just trying to explain the context. ComputerCraft's use-case is esoteric to say the least - I've always known that we've done it Wrong(TM), just been able to get away with it so far :p.
Ah right it is actually an instance of Storage.empty(). In that case I can add the block update, because we should indeed notify neighbors when supportsInsertion and supportsExtraction change.
Until then I would add a specific type of peripheral that would use caching. I believe, in this mod there is also a multiblock energy and item storages as well?