Registry for custom behaviour with external storage for specific blocks

[lucasser]

Implements #976

[Nodrance]

based

[lucasser]

Will let us integrate with QIO and AE2 very easily.

[Nodrance]

"is rs2 or ae2 better" we're doing both now keep up

[raoulvdberge]

I'm not sure that this will work properly. The external storage API is designed to just be active for all providers all the time, regardless of block type. We can't be block type dependant. The reason for this is: if we are block type dependant, that means you need to reload the storage if the connected block changes (this currently doesn't happen, by design). If you reload when the connected block changes, this causes 2 problems:

• Items are flickering in the Grid
• Server strain

Listening to connected block changes isn't bad per se, but some blocks constantly send block updates. See #886
And yes, indeed, interfaces get an exception for the neighbor changed event and do cause a reload.

Regarding possible AE integration:

Refined Storage shouldn't have integration with Applied Energistics, there is no use case for it and encourages people to create weird setups for no reason that create problems.

[raoulvdberge]

If we were to merge this, only for QIO integration, ExternalStorageBlock#neighborChanged needs to be updated as well for block change detection.

[lucasser]

I have two solutions. I changed the neighbourChanged method to re initialize if the changed block is in the registry. This might cause problems if the special block always sends updates. The other option is we store the block the storage is currently sitting on, and if it changes:

if (savedBlock.getLootTable().registry() != newBlock.getLootTable().registry()) {
    blockEntity.loadStorage(serverLevel);
} else...

then we change the provider.

[lucasser]

I am trying to prevent hardcoded blocks/values so that if there is something else that need a custom provider, it'll be easy to integrate (for example create storage system)

[raoulvdberge]

I have two solutions. I changed the neighbourChanged method to re initialize if the changed block is in the registry. This might cause problems if the special block always sends updates.

Yes, but that is less of a problem since we explicitly whitelist these blocks and can check them.

The other option is we store the block the storage is currently sitting on, and if it changes:

That is bound to cause issues: you have to deal with syncing world state & cache invalidation.

[raoulvdberge]

brokeSpecialBlock should probably check isPresent instead? Please thoroughly test these changes!

I do not want to create more instability in a phase of development where we already have so many bugs.

[lucasser]

Also, this doesn't have to be an urgent thing, I'm just submitting PRs as I finish working on them for you to review whenever you can.

[lucasser]

Is there a reason for updating external storage provider when an interface/special block is broken as opposed to just update when something is placed

[lucasser]

I updated this PR to 2.0.0, and stress tested. It correctly updates the provider on world load, when a block gets placed, and when a block gets instant replaced in creative mode. The one thing that doesn't work is entangled blocks, since they will load the composite provider when you place it and not update the provider depending on what's inside (but they won't work with an interface either). To fix it there would have to be an EntangledBlockExternalStorageProvider which would correctly update it. Everything else works fine.

[raoulvdberge]

Please rebase on latest develop

[lucasser]

Done

[raoulvdberge]

The class is called ExternalStorageProviderFactory, so the method should be called addExternalStorageProviderFactory.

[lucasser]

There are two different externalStorageProvider groups.

    private final List<ExternalStorageProviderFactory> externalStorageProviderFactories = new ArrayList<>();
    private final PlatformRegistry<ExternalStorageProviderFactory> customExternalStorageProviderBlocks =

one is for generic resource-based providers: item storage, fluid storage (chemical storage) and is an array
The other is for specific blocks (interface, QIO dash) and is a registry which can be queried for block id. Both of them consist of ExternalStorageProviderFactories but they are sepparate things. I could try to combine them, but i dont think that will be very helpful, or I can rename the methods/collections to resourceExternalStorageProviderFactories and blockExternalStorageProviderFactories, or i can add a class for block provider factories only.

[raoulvdberge]

Instead of taking in an Identifier I'd prefer to take in a ResourceKey<Block>, is that possible?

[lucasser]

Right now the providers are stored in a PlatformRegistry, so no. But I can change that. Is there an example registry in refinedStorageAPI that I can copy follow?

[lucasser]

If you mean the function takes a ResourceKey and then converts it into an Identifier, then i can do that

[raoulvdberge]

Just stop storing these block providers in a registry?

[lucasser]

And store them in a map instead?

[raoulvdberge]

So why is the check for didBreakInterfaceBlock gone without alternative?

[lucasser]

We don't need it. The only time the provider might have to change is when the targeted block gets changed. If it gets broken, that's irrelevant because air is not targeted either way. So the only time we need to reload the provider is if a block is placed. I tested different ways of changing blocks (piston, creative mode replacing) and all of them will first put an air block there, and then add the new block.
Tho only time this will not work with is with entangled blocks, but there could be an addon making an entangledBlockProvider to fix that.