Skip to content

Commit

Permalink
Fix usage of FluidStack as keys in maps or sets not properly comparing
Browse files Browse the repository at this point in the history
  • Loading branch information
pupnewfster committed Apr 29, 2024
1 parent dd10f08 commit 9ea3942
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import com.mojang.blaze3d.vertex.VertexConsumer;
import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
import java.util.HashMap;
import it.unimi.dsi.fastutil.objects.Object2ObjectOpenCustomHashMap;
import java.util.Map;
import mekanism.api.annotations.NothingNullByDefault;
import mekanism.client.render.MekanismRenderer;
Expand All @@ -13,6 +13,7 @@
import mekanism.client.render.ModelRenderer;
import mekanism.client.render.RenderResizableCuboid.FaceDisplay;
import mekanism.common.base.ProfilerConstants;
import mekanism.common.lib.collection.FluidHashStrategy;
import mekanism.common.tile.TileEntityFluidTank;
import mekanism.common.util.MekanismUtils;
import net.minecraft.client.renderer.MultiBufferSource;
Expand All @@ -26,8 +27,8 @@
@NothingNullByDefault
public class RenderFluidTank extends MekanismTileEntityRenderer<TileEntityFluidTank> {

private static final Map<FluidStack, Int2ObjectMap<Model3D>> cachedCenterFluids = new HashMap<>();
private static final Map<FluidStack, Int2ObjectMap<Model3D>> cachedValveFluids = new HashMap<>();
private static final Map<FluidStack, Int2ObjectMap<Model3D>> cachedCenterFluids = new Object2ObjectOpenCustomHashMap<>(FluidHashStrategy.INSTANCE);
private static final Map<FluidStack, Int2ObjectMap<Model3D>> cachedValveFluids = new Object2ObjectOpenCustomHashMap<>(FluidHashStrategy.INSTANCE);

private static final int stages = 1_400;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
import it.unimi.dsi.fastutil.ints.Int2ObjectArrayMap;
import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
import it.unimi.dsi.fastutil.objects.Object2ObjectOpenCustomHashMap;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
Expand All @@ -20,6 +20,7 @@
import mekanism.common.base.ProfilerConstants;
import mekanism.common.content.network.FluidNetwork;
import mekanism.common.content.network.transmitter.MechanicalPipe;
import mekanism.common.lib.collection.FluidHashStrategy;
import mekanism.common.lib.transmitter.ConnectionType;
import mekanism.common.tile.transmitter.TileEntityMechanicalPipe;
import mekanism.common.util.EnumUtils;
Expand All @@ -43,7 +44,6 @@ public class RenderMechanicalPipe extends RenderTransmitterBase<TileEntityMechan
private static final float offset = 0.02F;
//Note: this is basically used as an enum map (Direction), but null key is possible, which EnumMap doesn't support.
// 6 is used for null side, and 7 is used for null side but flowing vertically
//TODO - 1.20.5: Look at all maps and sets that use FluidStacks as keys as they no longer are hashed
private static final Int2ObjectMap<Map<FluidStack, Int2ObjectMap<Model3D>>> cachedLiquids = new Int2ObjectArrayMap<>(8);

public RenderMechanicalPipe(BlockEntityRendererProvider.Context context) {
Expand Down Expand Up @@ -142,7 +142,7 @@ private Model3D getModel(@Nullable Direction side, FluidStack fluid, int stage,
} else {
sideOrdinal = side.ordinal();
}
Int2ObjectMap<Model3D> modelMap = cachedLiquids.computeIfAbsent(sideOrdinal, s -> new HashMap<>())
Int2ObjectMap<Model3D> modelMap = cachedLiquids.computeIfAbsent(sideOrdinal, s -> new Object2ObjectOpenCustomHashMap<>(FluidHashStrategy.INSTANCE))
.computeIfAbsent(fluid, f -> new Int2ObjectOpenHashMap<>());
Model3D model = modelMap.get(stage);
if (model == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
package mekanism.common.inventory.slot;

import java.util.HashMap;
import it.unimi.dsi.fastutil.objects.Object2ObjectOpenCustomHashMap;
import java.util.Collection;
import java.util.Map;
import java.util.Set;
import mekanism.api.Action;
import mekanism.api.AutomationType;
import mekanism.api.fluid.IExtendedFluidTank;
import mekanism.api.inventory.IInventorySlot;
import mekanism.common.capabilities.Capabilities;
import mekanism.common.lib.collection.FluidHashStrategy;
import mekanism.common.tile.interfaces.IFluidContainerManager.ContainerEditMode;
import mekanism.common.util.MekanismUtils;
import net.minecraft.world.item.ItemStack;
import net.neoforged.neoforge.fluids.FluidStack;
import net.neoforged.neoforge.fluids.capability.IFluidHandler;
import net.neoforged.neoforge.fluids.capability.IFluidHandler.FluidAction;
import net.neoforged.neoforge.fluids.capability.IFluidHandlerItem;
import net.neoforged.neoforge.items.ItemHandlerHelper;

public interface IFluidHandlerSlot extends IInventorySlot {

Expand Down Expand Up @@ -88,9 +88,8 @@ default void fillTank(IInventorySlot outputSlot) {
//If we have more than one tank in our item then handle calculating the different drains that will occur for filling our fluid handler
// We start by gathering all the fluids in the item that we are able to drain and are valid for the tank,
// combining same fluid types into a single fluid stack
Set<FluidStack> knownFluids = gatherKnownFluids(itemFluidHandler, itemTanks);
//If we found any fluids that we can drain, attempt to drain them into our item
for (FluidStack knownFluid : knownFluids) {
for (FluidStack knownFluid : gatherKnownFluids(itemFluidHandler, itemTanks)) {
if (drainItemAndMove(outputSlot, knownFluid) && isEmpty()) {
//If we moved the item after draining it and we now don't have an item to try and fill
// then just exit instead of checking the other types of fluids
Expand Down Expand Up @@ -261,7 +260,7 @@ default boolean fillTank() {
//If we have more than one tank in our item then handle calculating the different drains that will occur for filling our fluid handler
// We start by gathering all the fluids in the item that we are able to drain and are valid for the tank,
// combining same fluid types into a single fluid stack
Set<FluidStack> knownFluids = gatherKnownFluids(itemFluidHandler, tanks);
Collection<FluidStack> knownFluids = gatherKnownFluids(itemFluidHandler, tanks);
if (!knownFluids.isEmpty()) {
//If we found any fluids that we can drain, attempt to drain them into our item
boolean changed = false;
Expand All @@ -282,8 +281,8 @@ default boolean fillTank() {
return false;
}

private Set<FluidStack> gatherKnownFluids(IFluidHandlerItem itemFluidHandler, int tanks) {
Map<FluidStack, FluidStack> knownFluids = new HashMap<>();
private Collection<FluidStack> gatherKnownFluids(IFluidHandlerItem itemFluidHandler, int tanks) {
Map<FluidStack, FluidStack> knownFluids = new Object2ObjectOpenCustomHashMap<>(FluidHashStrategy.INSTANCE);
for (int tank = 0; tank < tanks; tank++) {
FluidStack fluidInItem = itemFluidHandler.getFluidInTank(tank);
if (!fluidInItem.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package mekanism.common.lib.collection;

import it.unimi.dsi.fastutil.Hash.Strategy;
import net.neoforged.neoforge.fluids.FluidStack;

public class FluidHashStrategy implements Strategy<FluidStack> {

public static final FluidHashStrategy INSTANCE = new FluidHashStrategy();

private FluidHashStrategy() {
}

@Override
public int hashCode(FluidStack stack) {
return FluidStack.hashFluidAndComponents(stack);
}

@Override
public boolean equals(FluidStack a, FluidStack b) {
return FluidStack.isSameFluidSameComponents(a, b);
}
}

0 comments on commit 9ea3942

Please sign in to comment.