Skip to content

Commit

Permalink
fix: resources not being used up correctly when there are missing items
Browse files Browse the repository at this point in the history
We always have to add the resulting output
resources to the internal storage, even if there
are missing resources.
If we would not do this, then, in the case of
missing resources, it would re-calculate
the same ingredient from scratch everytime and
use way too much resources.

However, if we always add resulting resources,
even if there are missing items, we must still
use up the yields from the recursive calculation.
Otherwise, if we would not use up the yields
from the recursive calculation, the crafting
calculation would incorrectly use up resources
from the internal storage for other ingredients.

What is the difference between not adding the yields
in case of missing resources and using up the recursive
yields when there were missing resources?
The difference lies in the fact that, when we do not
add *any* yields, the crafting calculation would start
from scratch everytime for each ingredient.
Only using up what we needed in the first place from the
recursive yield ensures that any _leftover_ yields
can still be used up by other ingredients that happen
to need the same resource.
So, the difference lies in not having any yields at
all versus still allowing access to the leftovers.
  • Loading branch information
raoulvdberge committed Dec 26, 2024
1 parent 36bbd76 commit 7cb2e3a
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ CalculationResult calculate() {
result = CalculationResult.MISSING_RESOURCES;
}
}
if (result == CalculationResult.SUCCESS) {
craftingState.addOutputsToInternalStorage(pattern, amount);
}
craftingState.addOutputsToInternalStorage(pattern, amount);
return result;
}

Expand All @@ -81,9 +79,16 @@ 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) {
craftingState.extractFromInternalStorage(resourceState.resource(), remaining);
return CalculationResult.MISSING_RESOURCES;
} else {
resourceState = newState;
}
}
}
Expand Down Expand Up @@ -116,14 +121,14 @@ private CraftingState.ResourceState calculateChild(final IngredientState ingredi
final CraftingState.ResourceState resourceState) {
final ChildCalculationResult<T> 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;
}
Expand Down Expand Up @@ -153,37 +158,34 @@ private ChildCalculationResult<T> 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<T> result) {
final ChildCalculationResult<T> 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;
});
}

private record ChildCalculationResult<T>(boolean success,
long amountCrafted,
CraftingCalculatorListener<T> childListener,
CraftingState childCraftingState) {
CraftingTree<T> childTree) {
}

enum CalculationResult {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package com.refinedmods.refinedstorage.api.autocrafting.calculation;
package com.refinedmods.refinedstorage.api.autocrafting.preview;

import com.refinedmods.refinedstorage.api.autocrafting.preview.Preview;
import com.refinedmods.refinedstorage.api.autocrafting.preview.PreviewBuilder;
import com.refinedmods.refinedstorage.api.autocrafting.preview.PreviewType;
import com.refinedmods.refinedstorage.api.autocrafting.calculation.CraftingCalculatorListener;
import com.refinedmods.refinedstorage.api.resource.ResourceKey;
import com.refinedmods.refinedstorage.api.resource.list.MutableResourceList;
import com.refinedmods.refinedstorage.api.resource.list.MutableResourceListImpl;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package com.refinedmods.refinedstorage.api.autocrafting.calculation;
package com.refinedmods.refinedstorage.api.autocrafting.preview;

import com.refinedmods.refinedstorage.api.autocrafting.Pattern;
import com.refinedmods.refinedstorage.api.autocrafting.PatternRepository;
import com.refinedmods.refinedstorage.api.autocrafting.PatternRepositoryImpl;
import com.refinedmods.refinedstorage.api.autocrafting.preview.Preview;
import com.refinedmods.refinedstorage.api.autocrafting.preview.PreviewBuilder;
import com.refinedmods.refinedstorage.api.autocrafting.calculation.CraftingCalculator;
import com.refinedmods.refinedstorage.api.autocrafting.calculation.CraftingCalculatorImpl;
import com.refinedmods.refinedstorage.api.core.Action;
import com.refinedmods.refinedstorage.api.resource.ResourceAmount;
import com.refinedmods.refinedstorage.api.resource.ResourceKey;
Expand Down Expand Up @@ -36,7 +36,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;

class CraftingCalculatorImplTest {
class PreviewTest {
private static final RecursiveComparisonConfiguration PREVIEW_CONFIG = RecursiveComparisonConfiguration.builder()
.withIgnoredCollectionOrderInFields("items")
.build();
Expand Down Expand Up @@ -97,6 +97,39 @@ 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
import com.refinedmods.refinedstorage.api.autocrafting.TaskId;
import com.refinedmods.refinedstorage.api.autocrafting.calculation.CraftingCalculator;
import com.refinedmods.refinedstorage.api.autocrafting.calculation.CraftingCalculatorImpl;
import com.refinedmods.refinedstorage.api.autocrafting.calculation.PreviewCraftingCalculatorListener;
import com.refinedmods.refinedstorage.api.autocrafting.preview.Preview;
import com.refinedmods.refinedstorage.api.autocrafting.preview.PreviewCraftingCalculatorListener;
import com.refinedmods.refinedstorage.api.autocrafting.status.TaskStatus;
import com.refinedmods.refinedstorage.api.autocrafting.status.TaskStatusListener;
import com.refinedmods.refinedstorage.api.autocrafting.status.TaskStatusProvider;
Expand Down

0 comments on commit 7cb2e3a

Please sign in to comment.