Skip to content

Commit

Permalink
fix: external storage not connecting properly to fluid storages
Browse files Browse the repository at this point in the history
Also fixes interface not respecting maximum stack size.
(Reproduceable in the Mekanism addon with creative chemical tank)

Also fixes crash potential crash when building cable shapes.
On Fabric it crashed with a ConcurrentModificationException.
  • Loading branch information
raoulvdberge committed Nov 30, 2024
1 parent ae4166f commit b869509
Show file tree
Hide file tree
Showing 15 changed files with 70 additions and 31 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- The Autocrafting Monitor now has a sidebar with all tasks instead of using tabs.
- The auto-selected search box mode is now a global option used in the Autocrafter Manager as well.

### Fixed

- Fixed External Fluid not connecting properly to fluid storages.
- Fixed Interface filter not respecting maximum stack size of a resource.
- Fixed potential crash when trying to build cable shapes.

### Removed

- Block of Quartz Enriched Iron (has been moved to addon mod)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,8 @@ private void registerItemStorageItems(final RegistryCallback<Item> callback,
if (variant != ItemStorageVariant.CREATIVE) {
Items.INSTANCE.setItemStoragePart(variant, callback.register(
ContentIds.forItemStoragePart(variant),
SimpleItem::new)
);
SimpleItem::new
));
}
Items.INSTANCE.setItemStorageDisk(variant, callback.register(
ContentIds.forStorageDisk(variant),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
import com.refinedmods.refinedstorage.common.support.ColorableBlock;
import com.refinedmods.refinedstorage.common.support.DirectionalCableBlockShapes;

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;

import net.minecraft.core.Direction;
Expand All @@ -31,7 +30,8 @@ public abstract class AbstractConstructorDestructorBlock<T extends Block & Block
implements ColorableBlock<T, I>, EntityBlock {
public static final BooleanProperty ACTIVE = BooleanProperty.create("active");

private static final Map<DirectionalCacheShapeCacheKey, VoxelShape> SHAPE_CACHE = new HashMap<>();
private static final ConcurrentHashMap<DirectionalCacheShapeCacheKey, VoxelShape> SHAPE_CACHE =
new ConcurrentHashMap<>();

private final AbstractBlockEntityTicker<B> ticker;
private final DyeColor color;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import com.refinedmods.refinedstorage.common.support.NetworkNodeBlockItem;
import com.refinedmods.refinedstorage.common.support.network.NetworkNodeBlockEntityTicker;

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;

Expand All @@ -35,7 +34,8 @@
public class ExporterBlock extends AbstractDirectionalCableBlock
implements ColorableBlock<ExporterBlock, BaseBlockItem>, EntityBlock, BlockItemProvider<BaseBlockItem> {
private static final Component HELP = createTranslation("item", "exporter.help");
private static final Map<DirectionalCacheShapeCacheKey, VoxelShape> SHAPE_CACHE = new ConcurrentHashMap<>();
private static final ConcurrentHashMap<DirectionalCacheShapeCacheKey, VoxelShape> SHAPE_CACHE =
new ConcurrentHashMap<>();
private static final AbstractBlockEntityTicker<AbstractExporterBlockEntity> TICKER =
new NetworkNodeBlockEntityTicker<>(BlockEntities.INSTANCE::getExporter);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,13 @@ public class InterfacePlatformExternalStorageProviderFactory implements Platform
public Optional<ExternalStorageProvider> create(final ServerLevel level,
final BlockPos pos,
final Direction direction) {
if (!(level.getBlockEntity(pos) instanceof InterfaceBlockEntity)) {
return Optional.empty();
}
return Optional.of(new InterfaceProxyExternalStorageProvider(level, pos));
return Optional.ofNullable(level.getBlockEntity(pos))
.filter(blockEntity -> blockEntity instanceof InterfaceBlockEntity)
.map(blockEntity -> new InterfaceProxyExternalStorageProvider(level, pos));
}

@Override
public int getPriority() {
return -1;
return Integer.MIN_VALUE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
import com.refinedmods.refinedstorage.common.support.NetworkNodeBlockItem;
import com.refinedmods.refinedstorage.common.support.network.NetworkNodeBlockEntityTicker;

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;

import net.minecraft.core.BlockPos;
Expand All @@ -35,7 +34,8 @@
public class ImporterBlock extends AbstractDirectionalCableBlock implements
ColorableBlock<ImporterBlock, BaseBlockItem>, EntityBlock, BlockItemProvider<BaseBlockItem> {
private static final Component HELP = createTranslation("item", "importer.help");
private static final Map<DirectionalCacheShapeCacheKey, VoxelShape> SHAPE_CACHE = new HashMap<>();
private static final ConcurrentHashMap<DirectionalCacheShapeCacheKey, VoxelShape> SHAPE_CACHE =
new ConcurrentHashMap<>();
private static final AbstractBlockEntityTicker<AbstractImporterBlockEntity> TICKER =
new NetworkNodeBlockEntityTicker<>(BlockEntities.INSTANCE::getImporter);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ public StorageContainerUpgradeRecipe(final T[] variants,
containerProvider.apply(to).asItem().getDefaultInstance(),
NonNullList.of(
Ingredient.of(getValidSourceContainers(to, variants, containerProvider)
.stream().map(ItemLike::asItem).map(Item::getDefaultInstance)),
.stream()
.map(ItemLike::asItem)
.toArray(Item[]::new)),
Ingredient.of(to.getStoragePart())
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
import com.refinedmods.refinedstorage.common.support.NetworkNodeBlockItem;
import com.refinedmods.refinedstorage.common.support.network.NetworkNodeBlockEntityTicker;

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;

import net.minecraft.core.BlockPos;
Expand All @@ -39,7 +38,8 @@
public class ExternalStorageBlock extends AbstractDirectionalCableBlock
implements ColorableBlock<ExternalStorageBlock, BaseBlockItem>, EntityBlock, BlockItemProvider<BaseBlockItem> {
private static final Component HELP = createTranslation("item", "external_storage.help");
private static final Map<DirectionalCacheShapeCacheKey, VoxelShape> SHAPE_CACHE = new HashMap<>();
private static final ConcurrentHashMap<DirectionalCacheShapeCacheKey, VoxelShape> SHAPE_CACHE =
new ConcurrentHashMap<>();
private static final AbstractBlockEntityTicker<AbstractExternalStorageBlockEntity> TICKER =
new NetworkNodeBlockEntityTicker<>(BlockEntities.INSTANCE::getExternalStorage);
private static final Logger LOGGER = LoggerFactory.getLogger(ExternalStorageBlock.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import com.refinedmods.refinedstorage.common.support.direction.DirectionType;
import com.refinedmods.refinedstorage.common.util.PlatformUtil;

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;

import net.minecraft.core.BlockPos;
Expand All @@ -31,9 +31,10 @@

public abstract class AbstractDirectionalCableBlock extends AbstractDirectionalBlock<Direction>
implements SimpleWaterloggedBlock {
private final Map<DirectionalCacheShapeCacheKey, VoxelShape> shapeCache;
private final ConcurrentHashMap<DirectionalCacheShapeCacheKey, VoxelShape> shapeCache;

protected AbstractDirectionalCableBlock(final Map<DirectionalCacheShapeCacheKey, VoxelShape> shapeCache) {
protected AbstractDirectionalCableBlock(final ConcurrentHashMap<DirectionalCacheShapeCacheKey, VoxelShape>
shapeCache) {
super(BlockConstants.CABLE_PROPERTIES);
this.shapeCache = shapeCache;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

import com.refinedmods.refinedstorage.common.networking.CableConnections;

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import net.minecraft.world.phys.shapes.Shapes;
import net.minecraft.world.phys.shapes.VoxelShape;
Expand All @@ -18,7 +18,7 @@ public final class CableShapes {
private static final VoxelShape WEST = box(0, 6, 6, 6, 10, 10);
private static final VoxelShape UP = box(6, 10, 6, 10, 16, 10);
private static final VoxelShape DOWN = box(6, 0, 6, 10, 6, 10);
private static final Map<CableConnections, VoxelShape> CACHE = new HashMap<>();
private static final Map<CableConnections, VoxelShape> CACHE = new ConcurrentHashMap<>();

private CableShapes() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,27 @@ public void setListener(@Nullable final Runnable listener) {
public void change(final int index, final ItemStack stack, final boolean tryAlternatives) {
if (tryAlternatives) {
for (final ResourceFactory resourceFactory : alternativeResourceFactories) {
final var result = resourceFactory.create(stack);
final var result = resourceFactory.create(stack).map(this::respectMaxAmount);
if (result.isPresent()) {
set(index, result.get());
return;
}
}
}
primaryResourceFactory.create(stack).ifPresentOrElse(
primaryResourceFactory.create(stack).map(this::respectMaxAmount).ifPresentOrElse(
resource -> set(index, resource),
() -> remove(index)
);
}

private ResourceAmount respectMaxAmount(final ResourceAmount resourceAmount) {
final long maxAmount = getMaxAmount(resourceAmount.resource());
if (resourceAmount.amount() > maxAmount) {
return new ResourceAmount(resourceAmount.resource(), maxAmount);
}
return resourceAmount;
}

@Override
public void set(final int index, final ResourceAmount resourceAmount) {
setSilently(index, resourceAmount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,15 +279,17 @@ private void registerExternalStorageProviderFactories() {
ItemStorage.SIDED,
VariantUtil::ofItemVariant,
resource -> resource instanceof ItemResource itemResource
? toItemVariant(itemResource) : null
? toItemVariant(itemResource) : null,
0
)
);
RefinedStorageApi.INSTANCE.addExternalStorageProviderFactory(
new FabricStoragePlatformExternalStorageProviderFactory<>(
FluidStorage.SIDED,
VariantUtil::ofFluidVariant,
resource -> resource instanceof FluidResource fluidResource
? toFluidVariant(fluidResource) : null
? toFluidVariant(fluidResource) : null,
-1
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,26 @@ public class FabricStoragePlatformExternalStorageProviderFactory<T>
private final BlockApiLookup<Storage<T>, Direction> lookup;
private final Function<T, ResourceKey> fromPlatformMapper;
private final Function<ResourceKey, @NullableType T> toPlatformMapper;
private final int priority;

public FabricStoragePlatformExternalStorageProviderFactory(final BlockApiLookup<Storage<T>, Direction> lookup,
final Function<T, ResourceKey> fromPlatformMapper,
@NullableType final Function<ResourceKey, T>
toPlatformMapper) {
toPlatformMapper,
final int priority) {
this.lookup = lookup;
this.fromPlatformMapper = fromPlatformMapper;
this.toPlatformMapper = toPlatformMapper;
this.priority = priority;
}

@Override
public Optional<ExternalStorageProvider> create(final ServerLevel level,
final BlockPos pos,
final Direction direction) {
if (lookup.find(level, pos, direction) == null) {
return Optional.empty();
}
return Optional.of(new FabricStorageExternalStorageProvider<>(
lookup,
fromPlatformMapper,
Expand All @@ -42,4 +48,9 @@ public Optional<ExternalStorageProvider> create(final ServerLevel level,
direction
));
}

@Override
public int getPriority() {
return priority;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.refinedmods.refinedstorage.api.storage.external.ExternalStorageProvider;
import com.refinedmods.refinedstorage.common.api.storage.externalstorage.PlatformExternalStorageProviderFactory;
import com.refinedmods.refinedstorage.neoforge.storage.CapabilityCache;
import com.refinedmods.refinedstorage.neoforge.storage.CapabilityCacheImpl;

import java.util.Optional;
Expand All @@ -15,6 +16,13 @@ public class FluidHandlerPlatformExternalStorageProviderFactory implements Platf
public Optional<ExternalStorageProvider> create(final ServerLevel level,
final BlockPos pos,
final Direction direction) {
return Optional.of(new FluidHandlerExternalStorageProvider(new CapabilityCacheImpl(level, pos, direction)));
final CapabilityCache capabilityCache = new CapabilityCacheImpl(level, pos, direction);
return capabilityCache.getFluidHandler()
.map(handler -> new FluidHandlerExternalStorageProvider(capabilityCache));
}

@Override
public int getPriority() {
return -1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.refinedmods.refinedstorage.api.storage.external.ExternalStorageProvider;
import com.refinedmods.refinedstorage.common.api.storage.externalstorage.PlatformExternalStorageProviderFactory;
import com.refinedmods.refinedstorage.neoforge.storage.CapabilityCache;
import com.refinedmods.refinedstorage.neoforge.storage.CapabilityCacheImpl;

import java.util.Optional;
Expand All @@ -15,6 +16,7 @@ public class ItemHandlerPlatformExternalStorageProviderFactory implements Platfo
public Optional<ExternalStorageProvider> create(final ServerLevel level,
final BlockPos pos,
final Direction direction) {
return Optional.of(new ItemHandlerExternalStorageProvider(new CapabilityCacheImpl(level, pos, direction)));
final CapabilityCache capabilityCache = new CapabilityCacheImpl(level, pos, direction);
return capabilityCache.getItemHandler().map(handler -> new ItemHandlerExternalStorageProvider(capabilityCache));
}
}

0 comments on commit b869509

Please sign in to comment.