From 6bd658a127fcf779f7d635b393df2949c5e5724d Mon Sep 17 00:00:00 2001 From: raoulvdberge Date: Thu, 26 Dec 2024 20:49:10 +0100 Subject: [PATCH] fix: resources not being used up correctly when there are missing items This now always makes it so that output resources are being added to the internal storage. If we do not do this, when there are missing resources, and we have the same ingredient being used multiple times, it would start from scratch for each of that same ingredient, causing a calculation that needs too many resources. So we have to *always* return resources so they can be reused later down the line, even if there are missing resources. However, we must use these resources up correctly if we do end up with missing resources! Otherwise, if another ingredient needs this resource later down the calculation, it will use them when it's not allowed to (the yields of the child calculation must be used up...) --- .../calculation/CraftingTree.java | 41 +++++++++++-------- .../CraftingCalculatorImplTest.java | 34 +++++++++++++++ .../common/autocrafting/CraftingPattern.java | 2 +- 3 files changed, 59 insertions(+), 18 deletions(-) diff --git a/refinedstorage-autocrafting-api/src/main/java/com/refinedmods/refinedstorage/api/autocrafting/calculation/CraftingTree.java b/refinedstorage-autocrafting-api/src/main/java/com/refinedmods/refinedstorage/api/autocrafting/calculation/CraftingTree.java index eb6d2b5b5..256acdbc0 100644 --- a/refinedstorage-autocrafting-api/src/main/java/com/refinedmods/refinedstorage/api/autocrafting/calculation/CraftingTree.java +++ b/refinedstorage-autocrafting-api/src/main/java/com/refinedmods/refinedstorage/api/autocrafting/calculation/CraftingTree.java @@ -59,9 +59,7 @@ CalculationResult calculate() { result = CalculationResult.MISSING_RESOURCES; } } - if (result == CalculationResult.SUCCESS) { - craftingState.addOutputsToInternalStorage(pattern, amount); - } + craftingState.addOutputsToInternalStorage(pattern, amount); return result; } @@ -81,9 +79,21 @@ private CalculationResult calculateIngredient(final IngredientState ingredientSt remaining -= toTake; } if (remaining > 0) { - resourceState = tryCalculateChild(ingredientState, resourceState, remaining); - if (resourceState == null) { + final CraftingState.ResourceState newState = tryCalculateChild( + ingredientState, + resourceState, + remaining + ); + if (newState == null) { + // If we end up with missing resources, we need to use up all the resulting + // resources from the internal storage so that it cannot be used later for other ingredients + // that happen to use the same resource. + // The goal was to use up the resources created by the child calculation in the next iteration, + // but since we have missing resources, we need to use them up now. + craftingState.extractFromInternalStorage(resourceState.resource(), remaining); return CalculationResult.MISSING_RESOURCES; + } else { + resourceState = newState; } } } @@ -116,14 +126,14 @@ private CraftingState.ResourceState calculateChild(final IngredientState ingredi final CraftingState.ResourceState resourceState) { final ChildCalculationResult result = calculateChild(remaining, childPatterns, resourceState); if (result.success) { - this.craftingState = result.childCraftingState; + this.craftingState = result.childTree.craftingState; final CraftingState.ResourceState updatedResourceState = craftingState.getResource( resourceState.resource() ); listener.childCalculationCompleted( updatedResourceState.resource(), updatedResourceState.inInternalStorage(), - result.childListener + result.childTree.listener ); return updatedResourceState; } @@ -153,28 +163,26 @@ private ChildCalculationResult calculateChild(final long remaining, return new ChildCalculationResult<>( true, craftingState.getResource(resourceState.resource()).inInternalStorage(), - childTree.listener, - childTree.craftingState + childTree ); } return new ChildCalculationResult<>( false, requireNonNull(lastChildAmount).getTotal(), - requireNonNull(lastChildTree).listener, - lastChildTree.craftingState + requireNonNull(lastChildTree) ); } @Nullable private CraftingState.ResourceState cycleToNextIngredientOrFail(final IngredientState ingredientState, final CraftingState.ResourceState resourceState, - final ChildCalculationResult result) { + final ChildCalculationResult childResult) { return ingredientState.cycle().map(craftingState::getResource).orElseGet(() -> { - this.craftingState = result.childCraftingState; + this.craftingState = childResult.childTree.craftingState; listener.childCalculationCompleted( resourceState.resource(), - result.amountCrafted, - result.childListener + childResult.amountCrafted, + childResult.childTree.listener ); return null; }); @@ -182,8 +190,7 @@ private CraftingState.ResourceState cycleToNextIngredientOrFail(final Ingredient private record ChildCalculationResult(boolean success, long amountCrafted, - CraftingCalculatorListener childListener, - CraftingState childCraftingState) { + CraftingTree childTree) { } enum CalculationResult { diff --git a/refinedstorage-autocrafting-api/src/test/java/com/refinedmods/refinedstorage/api/autocrafting/calculation/CraftingCalculatorImplTest.java b/refinedstorage-autocrafting-api/src/test/java/com/refinedmods/refinedstorage/api/autocrafting/calculation/CraftingCalculatorImplTest.java index 918b882dd..383d9c443 100644 --- a/refinedstorage-autocrafting-api/src/test/java/com/refinedmods/refinedstorage/api/autocrafting/calculation/CraftingCalculatorImplTest.java +++ b/refinedstorage-autocrafting-api/src/test/java/com/refinedmods/refinedstorage/api/autocrafting/calculation/CraftingCalculatorImplTest.java @@ -97,6 +97,40 @@ void shouldCalculateForSingleRootPatternSingleIngredientAndAllResourcesAreAvaila .build()); } + @ParameterizedTest + @ValueSource(longs = {1, 2}) + void shouldCalculateForSingleRootPatternSingleIngredientSpreadOutOverMultipleIngredientsAndThereAreMissingResources( + final long requestedAmount + ) { + // Arrange + final RootStorage storage = storage(); + final PatternRepository patterns = patterns( + pattern() + .ingredient(OAK_LOG, 1) + .output(OAK_PLANKS, 4) + .build(), + pattern() + .ingredient(OAK_PLANKS, 1) + .ingredient(OAK_PLANKS, 1) + .ingredient(OAK_PLANKS, 1) + .ingredient(OAK_PLANKS, 1) + .output(CRAFTING_TABLE, 1) + .build() + ); + final CraftingCalculator sut = new CraftingCalculatorImpl(patterns, storage); + + // Act + final Preview preview = calculateAndGetPreview(sut, CRAFTING_TABLE, requestedAmount); + + // Assert + assertThat(preview).usingRecursiveComparison(PREVIEW_CONFIG).isEqualTo(PreviewBuilder.ofType(MISSING_RESOURCES) + .addToCraft(CRAFTING_TABLE, requestedAmount) + .addToCraft(OAK_PLANKS, requestedAmount * 4) + .addMissing(OAK_LOG, requestedAmount) + .build()); + } + + @Test void shouldNotCalculateForSingleRootPatternSingleIngredientAndAlmostAllResourcesAreAvailable() { // Arrange diff --git a/refinedstorage-common/src/main/java/com/refinedmods/refinedstorage/common/autocrafting/CraftingPattern.java b/refinedstorage-common/src/main/java/com/refinedmods/refinedstorage/common/autocrafting/CraftingPattern.java index 0e5c42426..d2da98a6d 100644 --- a/refinedstorage-common/src/main/java/com/refinedmods/refinedstorage/common/autocrafting/CraftingPattern.java +++ b/refinedstorage-common/src/main/java/com/refinedmods/refinedstorage/common/autocrafting/CraftingPattern.java @@ -29,7 +29,7 @@ class CraftingPattern implements Pattern { this.output = output; this.inputResources = inputs.stream().flatMap(List::stream).collect(Collectors.toSet()); this.outputResources = Set.of(output.resource()); - this.ingredients = inputs.stream().map(i -> new Ingredient(1, i)).toList(); + this.ingredients = inputs.stream().map(i -> new Ingredient(i.isEmpty() ? 0 : 1, i)).toList(); this.outputs = Stream.concat(Stream.of(output), byproducts.stream()).toList(); }