From 662dbca3e1846494b0d9178ce57e8adff082a599 Mon Sep 17 00:00:00 2001 From: yueh Date: Mon, 27 Jul 2020 11:21:06 +0200 Subject: [PATCH] Fixes #4508: Correctly construct the sparse lists. (#4511) * Fixes #4508: Correctly construct the sparse lists. Also renamed all uncondensed getters to sparse and all condensed to the simple ones. * Apply suggestions from code review Co-authored-by: shartte * Streamlined collapsing the sparse lists * Extracted helper to condense stacks * Sort lists desc by stack size Co-authored-by: shartte --- .../crafting/ICraftingPatternDetails.java | 67 ++++++-- .../implementations/PatternTermContainer.java | 21 +-- .../appeng/crafting/CraftingTreeNode.java | 4 +- .../appeng/crafting/CraftingTreeProcess.java | 22 +-- .../helpers/CraftingPatternDetails.java | 145 ++++++++---------- .../appeng/items/misc/EncodedPatternItem.java | 7 +- .../implementations/CraftingCPUCluster.java | 14 +- .../parts/reporting/PatternTerminalPart.java | 8 +- 8 files changed, 152 insertions(+), 136 deletions(-) diff --git a/src/api/java/appeng/api/networking/crafting/ICraftingPatternDetails.java b/src/api/java/appeng/api/networking/crafting/ICraftingPatternDetails.java index f2ac98821..8a82fa609 100644 --- a/src/api/java/appeng/api/networking/crafting/ICraftingPatternDetails.java +++ b/src/api/java/appeng/api/networking/crafting/ICraftingPatternDetails.java @@ -23,6 +23,8 @@ package appeng.api.networking.crafting; +import java.util.List; + import net.minecraft.inventory.CraftingInventory; import net.minecraft.item.ItemStack; import net.minecraft.world.World; @@ -60,24 +62,65 @@ public interface ICraftingPatternDetails { boolean isCraftable(); /** + * Equal itemstacks will be aggregated into one, respectively 3*64 will be + * returned as one stack of 192, up to 576 of a single type. + *

+ * This should be the preferred way to deal with the list of inputs. + *

+ * The list will be sorted in descending order by stack size. However there is + * no guarantee about maintaining the placement order of the inputs in case of + * equal values. + * + * @return an immutable list of inputs without nulls + */ + List getInputs(); + + /** + * Equal itemstacks will be aggregated into one, respectively 2*32 will be + * returned as one stack of 64, up to 192 of a single type. + *

+ * This should be the preferred way to deal with the list of outputs. + *

+ * + * The list will be sorted in descending order by stack size. However there is + * no guarantee about maintaining the placement order of the outputs in case of + * equal values. + * + * @return an immutable list of outputs without nulls + */ + List getOutputs(); + + /** + * A sparse list representing the placement order of a crafting grid, left to + * right, then top to bottom. + *

+ * Only use when absolutely necessary, always prefer + * {@link ICraftingPatternDetails#getInputs()} + *

+ * This will contain exactly 9 entries. + *

+ * This can return a copy from the internal structure, so there are no + * guarantees about modifications. + * * @return a list of the inputs, will include nulls. */ - IAEItemStack[] getInputs(); - - /** - * @return a list of the inputs, will be clean - */ - IAEItemStack[] getCondensedInputs(); - - /** - * @return a list of the outputs, will be clean - */ - IAEItemStack[] getCondensedOutputs(); + IAEItemStack[] getSparseInputs(); /** + * A sparse list representing the placement order of the respective output + * slots. + *

+ * Only use when absolutely necessary, always prefer + * {@link ICraftingPatternDetails#getOutputs()} + *

+ * This will either contain 1 entry for crafting patterns or 3 for processing. + *

+ * This can return a copy from the internal structure, so there are no + * guarantees about modifications. + * * @return a list of the outputs, will include nulls. */ - IAEItemStack[] getOutputs(); + IAEItemStack[] getSparseOutputs(); /** * @return if this pattern is enabled to support substitutions. diff --git a/src/main/java/appeng/container/implementations/PatternTermContainer.java b/src/main/java/appeng/container/implementations/PatternTermContainer.java index 22fd63e5b..23f8d579b 100644 --- a/src/main/java/appeng/container/implementations/PatternTermContainer.java +++ b/src/main/java/appeng/container/implementations/PatternTermContainer.java @@ -18,9 +18,6 @@ package appeng.container.implementations; -import java.util.ArrayList; -import java.util.List; - import net.minecraft.entity.player.PlayerEntity; import net.minecraft.entity.player.PlayerInventory; import net.minecraft.entity.player.ServerPlayerEntity; @@ -264,21 +261,13 @@ public class PatternTermContainer extends MEMonitorableContainer return new ItemStack[] { out }; } } else { - final List list = new ArrayList<>(3); - boolean hasValue = false; + final ItemStack[] list = new ItemStack[3]; - for (final OptionalFakeSlot outputSlot : this.outputSlots) { - final ItemStack out = outputSlot.getStack(); - - if (!out.isEmpty() && out.getCount() > 0) { - list.add(out); - hasValue = true; - } - } - - if (hasValue) { - return list.toArray(new ItemStack[list.size()]); + for (int i = 0; i < this.outputSlots.length; i++) { + final ItemStack out = this.outputSlots[i].getStack(); + list[i] = out; } + return list; } return null; diff --git a/src/main/java/appeng/crafting/CraftingTreeNode.java b/src/main/java/appeng/crafting/CraftingTreeNode.java index 67b5382d1..5b636d5dd 100644 --- a/src/main/java/appeng/crafting/CraftingTreeNode.java +++ b/src/main/java/appeng/crafting/CraftingTreeNode.java @@ -85,7 +85,7 @@ public class CraftingTreeNode { } boolean notRecursive(final ICraftingPatternDetails details) { - IAEItemStack[] o = details.getCondensedOutputs(); + Collection o = details.getOutputs(); for (final IAEItemStack i : o) { if (i.equals(this.what)) { @@ -93,7 +93,7 @@ public class CraftingTreeNode { } } - o = details.getCondensedInputs(); + o = details.getInputs(); for (final IAEItemStack i : o) { if (i.equals(this.what)) { diff --git a/src/main/java/appeng/crafting/CraftingTreeProcess.java b/src/main/java/appeng/crafting/CraftingTreeProcess.java index 31019c314..6b77b480b 100644 --- a/src/main/java/appeng/crafting/CraftingTreeProcess.java +++ b/src/main/java/appeng/crafting/CraftingTreeProcess.java @@ -64,10 +64,10 @@ public class CraftingTreeProcess { final World world = job.getWorld(); if (details.isCraftable()) { - final IAEItemStack[] list = details.getInputs(); + final IAEItemStack[] list = details.getSparseInputs(); final CraftingInventory ic = new CraftingInventory(new ContainerNull(), 3, 3); - final IAEItemStack[] is = details.getInputs(); + final IAEItemStack[] is = details.getSparseInputs(); for (int x = 0; x < ic.getSizeInventory(); x++) { ic.setInventorySlotContents(x, is[x] == null ? ItemStack.EMPTY : is[x].createItemStack()); } @@ -82,11 +82,11 @@ public class CraftingTreeProcess { } } - for (final IAEItemStack part : details.getCondensedInputs()) { + for (final IAEItemStack part : details.getInputs()) { final ItemStack g = part.createItemStack(); boolean isAnInput = false; - for (final IAEItemStack a : details.getCondensedOutputs()) { + for (final IAEItemStack a : details.getOutputs()) { if (!g.isEmpty() && a != null && a.equals(g)) { isAnInput = true; } @@ -114,7 +114,7 @@ public class CraftingTreeProcess { } else { // this is minor different then below, this slot uses the pattern, but kinda // fudges it. - for (final IAEItemStack part : details.getCondensedInputs()) { + for (final IAEItemStack part : details.getInputs()) { for (int x = 0; x < list.length; x++) { final IAEItemStack comparePart = list[x]; if (part != null && part.equals(comparePart)) { @@ -127,11 +127,11 @@ public class CraftingTreeProcess { } } } else { - for (final IAEItemStack part : details.getCondensedInputs()) { + for (final IAEItemStack part : details.getInputs()) { final ItemStack g = part.createItemStack(); boolean isAnInput = false; - for (final IAEItemStack a : details.getCondensedOutputs()) { + for (final IAEItemStack a : details.getOutputs()) { if (!g.isEmpty() && a != null && a.equals(g)) { isAnInput = true; } @@ -142,7 +142,7 @@ public class CraftingTreeProcess { } } - for (final IAEItemStack part : details.getCondensedInputs()) { + for (final IAEItemStack part : details.getInputs()) { this.nodes.put(new CraftingTreeNode(cc, job, part.copy(), this, -1, depth + 1), part.getStackSize()); } } @@ -208,7 +208,7 @@ public class CraftingTreeProcess { // assume its possible. // add crafting results.. - for (final IAEItemStack out : this.details.getCondensedOutputs()) { + for (final IAEItemStack out : this.details.getOutputs()) { final IAEItemStack o = out.copy(); o.setStackSize(o.getStackSize() * i); inv.injectItems(o, Actionable.MODULATE, src); @@ -227,7 +227,7 @@ public class CraftingTreeProcess { } IAEItemStack getAmountCrafted(IAEItemStack what2) { - for (final IAEItemStack is : this.details.getCondensedOutputs()) { + for (final IAEItemStack is : this.details.getOutputs()) { if (is.equals(what2)) { what2 = what2.copy(); what2.setStackSize(is.getStackSize()); @@ -236,7 +236,7 @@ public class CraftingTreeProcess { } // more fuzzy! - for (final IAEItemStack is : this.details.getCondensedOutputs()) { + for (final IAEItemStack is : this.details.getOutputs()) { if (is.getItem() == what2.getItem() && (is.getItem().isDamageable() || is.getItemDamage() == what2.getItemDamage())) { what2 = is.copy(); diff --git a/src/main/java/appeng/helpers/CraftingPatternDetails.java b/src/main/java/appeng/helpers/CraftingPatternDetails.java index b02de8b9e..1a8a52174 100644 --- a/src/main/java/appeng/helpers/CraftingPatternDetails.java +++ b/src/main/java/appeng/helpers/CraftingPatternDetails.java @@ -19,13 +19,17 @@ package appeng.helpers; import java.util.ArrayList; -import java.util.HashMap; +import java.util.Collection; +import java.util.Comparator; import java.util.HashSet; import java.util.List; -import java.util.Map; +import java.util.Objects; import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import net.minecraft.inventory.CraftingInventory; import net.minecraft.item.Item; @@ -46,14 +50,21 @@ import appeng.util.Platform; public class CraftingPatternDetails implements ICraftingPatternDetails, Comparable { + private static final int ALL_INPUT_LIMIT = 9; + private static final int CRAFTING_OUTPUT_LIMIT = 1; + private static final int PROCESSING_OUTPUT_LIMIT = 3; + + private static final Comparator COMPARE_BY_STACKSIZE = (left, right) -> Long + .compare(right.getStackSize(), left.getStackSize()); + private final CraftingInventory crafting = new CraftingInventory(new ContainerNull(), 3, 3); private final CraftingInventory testFrame = new CraftingInventory(new ContainerNull(), 3, 3); private final ItemStack correctOutput; private final ICraftingRecipe standardRecipe; - private final IAEItemStack[] condensedInputs; - private final IAEItemStack[] condensedOutputs; - private final IAEItemStack[] inputs; - private final IAEItemStack[] outputs; + private final List inputs; + private final List outputs; + private final IAEItemStack[] sparseInputs; + private final IAEItemStack[] sparseOutputs; private final boolean isCraftable; private final boolean canSubstitute; private final Set failCache = new HashSet<>(); @@ -79,7 +90,7 @@ public class CraftingPatternDetails implements ICraftingPatternDetails, Comparab final List in = new ArrayList<>(); final List out = new ArrayList<>(); - for (int x = 0; x < 9; x++) { + for (int x = 0; x < ALL_INPUT_LIMIT; x++) { final IAEItemStack ais = ingredients.get(x); final ItemStack gs = ais != null ? ais.createItemStack() : ItemStack.EMPTY; @@ -109,70 +120,19 @@ public class CraftingPatternDetails implements ICraftingPatternDetails, Comparab this.standardRecipe = null; this.correctOutput = ItemStack.EMPTY; - for (int x = 0; x < products.size(); x++) { + for (int x = 0; x < PROCESSING_OUTPUT_LIMIT; x++) { final IAEItemStack ais = products.get(x); - final ItemStack gs = ais.createItemStack(); - if (!gs.isEmpty()) { - out.add(ais.copy()); - } + out.add(ais != null ? ais.copy() : null); } } - this.inputs = in.toArray(new IAEItemStack[0]); - this.outputs = out.toArray(new IAEItemStack[0]); + final int outputLength = this.isCraftable ? CRAFTING_OUTPUT_LIMIT : PROCESSING_OUTPUT_LIMIT; + this.sparseInputs = in.toArray(new IAEItemStack[ALL_INPUT_LIMIT]); + this.sparseOutputs = out.toArray(new IAEItemStack[outputLength]); - final Map tmpOutputs = new HashMap<>(); - - for (final IAEItemStack io : this.outputs) { - if (io == null) { - continue; - } - - final IAEItemStack g = tmpOutputs.get(io); - - if (g == null) { - tmpOutputs.put(io, io.copy()); - } else { - g.add(io); - } - } - - final Map tmpInputs = new HashMap<>(); - - for (final IAEItemStack io : this.inputs) { - if (io == null) { - continue; - } - - final IAEItemStack g = tmpInputs.get(io); - - if (g == null) { - tmpInputs.put(io, io.copy()); - } else { - g.add(io); - } - } - - if (tmpOutputs.isEmpty() || tmpInputs.isEmpty()) { - throw new IllegalStateException("No pattern here!"); - } - - this.condensedInputs = new IAEItemStack[tmpInputs.size()]; - int offset = 0; - - for (final IAEItemStack io : tmpInputs.values()) { - this.condensedInputs[offset] = io; - offset++; - } - - offset = 0; - this.condensedOutputs = new IAEItemStack[tmpOutputs.size()]; - - for (final IAEItemStack io : tmpOutputs.values()) { - this.condensedOutputs[offset] = io; - offset++; - } + this.inputs = this.condenseStacks(in); + this.outputs = this.condenseStacks(out); } private void markItemAs(final int slotIndex, final ItemStack i, final TestStatus b) { @@ -213,8 +173,8 @@ public class CraftingPatternDetails implements ICraftingPatternDetails, Comparab this.testFrame.setInventorySlotContents(slotIndex, i); // If we cannot substitute, the items must match exactly - if (!canSubstitute && slotIndex < inputs.length) { - if (!inputs[slotIndex].isSameType(i)) { + if (!canSubstitute && slotIndex < sparseInputs.length) { + if (!sparseInputs[slotIndex].isSameType(i)) { this.markItemAs(slotIndex, i, TestStatus.DECLINE); return false; } @@ -240,25 +200,25 @@ public class CraftingPatternDetails implements ICraftingPatternDetails, Comparab } @Override - public IAEItemStack[] getInputs() { + public IAEItemStack[] getSparseInputs() { + return this.sparseInputs; + } + + @Override + public List getInputs() { return this.inputs; } @Override - public IAEItemStack[] getCondensedInputs() { - return this.condensedInputs; - } - - @Override - public IAEItemStack[] getCondensedOutputs() { - return this.condensedOutputs; - } - - @Override - public IAEItemStack[] getOutputs() { + public List getOutputs() { return this.outputs; } + @Override + public IAEItemStack[] getSparseOutputs() { + return this.sparseOutputs; + } + @Override public boolean canSubstitute() { return this.canSubstitute; @@ -276,8 +236,8 @@ public class CraftingPatternDetails implements ICraftingPatternDetails, Comparab } } - if (this.outputs != null && this.outputs.length > 0) { - return this.outputs[0].createItemStack(); + if (this.sparseOutputs != null && this.sparseOutputs.length > 0) { + return this.sparseOutputs[0].createItemStack(); } return ItemStack.EMPTY; @@ -344,6 +304,29 @@ public class CraftingPatternDetails implements ICraftingPatternDetails, Comparab return false; } + /** + * Merges all equal entries into a single one while adding their total stack + * sizes. + * + * @throws IllegalStateException if the result would be empty. + * + * @param collection the collection to condense + * + * @return a non empty list of condensed stacks. + */ + private List condenseStacks(Collection collection) { + final List merged = collection.stream().filter(Objects::nonNull) + .collect(Collectors.toMap(Function.identity(), IAEItemStack::copy, + (left, right) -> left.setStackSize(left.getStackSize() + right.getStackSize()))) + .values().stream().sorted(COMPARE_BY_STACKSIZE).collect(ImmutableList.toImmutableList()); + + if (merged.isEmpty()) { + throw new IllegalStateException("No pattern here!"); + } + + return merged; + } + private enum TestStatus { ACCEPT, DECLINE, TEST } diff --git a/src/main/java/appeng/items/misc/EncodedPatternItem.java b/src/main/java/appeng/items/misc/EncodedPatternItem.java index d1bcdb0e2..bb6d90d10 100644 --- a/src/main/java/appeng/items/misc/EncodedPatternItem.java +++ b/src/main/java/appeng/items/misc/EncodedPatternItem.java @@ -19,6 +19,7 @@ package appeng.items.misc; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.WeakHashMap; @@ -155,8 +156,8 @@ public class EncodedPatternItem extends AEBaseItem { final boolean isCrafting = details.isCraftable(); final boolean substitute = details.canSubstitute(); - final IAEItemStack[] in = details.getCondensedInputs(); - final IAEItemStack[] out = details.getCondensedOutputs(); + final Collection in = details.getInputs(); + final Collection out = details.getOutputs(); final ITextComponent label = (isCrafting ? GuiText.Crafts.textComponent() : GuiText.Creates.textComponent()) .appendText(": "); @@ -208,7 +209,7 @@ public class EncodedPatternItem extends AEBaseItem { final ICraftingPatternDetails details = Api.instance().crafting().decodePattern(item, w); - out = details != null ? details.getOutputs()[0].createItemStack() : ItemStack.EMPTY; + out = details != null ? details.getOutputs().get(0).createItemStack() : ItemStack.EMPTY; SIMPLE_CACHE.put(item, out); return out; diff --git a/src/main/java/appeng/me/cluster/implementations/CraftingCPUCluster.java b/src/main/java/appeng/me/cluster/implementations/CraftingCPUCluster.java index 00b404b19..45fe9fcba 100644 --- a/src/main/java/appeng/me/cluster/implementations/CraftingCPUCluster.java +++ b/src/main/java/appeng/me/cluster/implementations/CraftingCPUCluster.java @@ -429,7 +429,7 @@ public final class CraftingCPUCluster implements IAECluster, ICraftingCPU { return null; } - private boolean canCraft(final ICraftingPatternDetails details, final IAEItemStack[] condensedInputs) { + private boolean canCraft(final ICraftingPatternDetails details, final Collection condensedInputs) { for (IAEItemStack g : condensedInputs) { if (details.isCraftable()) { @@ -557,7 +557,7 @@ public final class CraftingCPUCluster implements IAECluster, ICraftingCPU { final ICraftingPatternDetails details = e.getKey(); - if (this.canCraft(details, details.getCondensedInputs())) { + if (this.canCraft(details, details.getInputs())) { CraftingInventory ic = null; for (final ICraftingMedium m : cc.getMediums(e.getKey())) { @@ -567,7 +567,7 @@ public final class CraftingCPUCluster implements IAECluster, ICraftingCPU { if (!m.isBusy()) { if (ic == null) { - final IAEItemStack[] input = details.getInputs(); + final IAEItemStack[] input = details.getSparseInputs(); double sum = 0; for (final IAEItemStack anInput : input) { @@ -663,7 +663,7 @@ public final class CraftingCPUCluster implements IAECluster, ICraftingCPU { this.somethingChanged = true; this.remainingOperations--; - for (final IAEItemStack out : details.getCondensedOutputs()) { + for (final IAEItemStack out : details.getOutputs()) { this.postChange(out, this.machineSrc); this.waitingFor.add(out.copy()); this.postCraftingStatusChange(out.copy()); @@ -896,7 +896,7 @@ public final class CraftingCPUCluster implements IAECluster, ICraftingCPU { break; case PENDING: for (final Entry t : this.tasks.entrySet()) { - for (IAEItemStack ais : t.getKey().getCondensedOutputs()) { + for (IAEItemStack ais : t.getKey().getOutputs()) { ais = ais.copy(); ais.setStackSize(ais.getStackSize() * t.getValue().value); list.add(ais); @@ -915,7 +915,7 @@ public final class CraftingCPUCluster implements IAECluster, ICraftingCPU { } for (final Entry t : this.tasks.entrySet()) { - for (IAEItemStack ais : t.getKey().getCondensedOutputs()) { + for (IAEItemStack ais : t.getKey().getOutputs()) { ais = ais.copy(); ais.setStackSize(ais.getStackSize() * t.getValue().value); list.add(ais); @@ -960,7 +960,7 @@ public final class CraftingCPUCluster implements IAECluster, ICraftingCPU { is.setStackSize(0); for (final Entry t : this.tasks.entrySet()) { - for (final IAEItemStack ais : t.getKey().getCondensedOutputs()) { + for (final IAEItemStack ais : t.getKey().getOutputs()) { if (ais.equals(is)) { is.setStackSize(is.getStackSize() + ais.getStackSize() * t.getValue().value); } diff --git a/src/main/java/appeng/parts/reporting/PatternTerminalPart.java b/src/main/java/appeng/parts/reporting/PatternTerminalPart.java index 4576b158d..783040e2d 100644 --- a/src/main/java/appeng/parts/reporting/PatternTerminalPart.java +++ b/src/main/java/appeng/parts/reporting/PatternTerminalPart.java @@ -113,13 +113,13 @@ public class PatternTerminalPart extends AbstractTerminalPart { this.setCraftingRecipe(details.isCraftable()); this.setSubstitution(details.canSubstitute()); - for (int x = 0; x < this.crafting.getSlots() && x < details.getInputs().length; x++) { - final IAEItemStack item = details.getInputs()[x]; + for (int x = 0; x < this.crafting.getSlots() && x < details.getSparseInputs().length; x++) { + final IAEItemStack item = details.getSparseInputs()[x]; this.crafting.setStackInSlot(x, item == null ? ItemStack.EMPTY : item.createItemStack()); } - for (int x = 0; x < this.output.getSlots() && x < details.getOutputs().length; x++) { - final IAEItemStack item = details.getOutputs()[x]; + for (int x = 0; x < this.output.getSlots() && x < details.getSparseOutputs().length; x++) { + final IAEItemStack item = details.getSparseOutputs()[x]; this.output.setStackInSlot(x, item == null ? ItemStack.EMPTY : item.createItemStack()); } }