From 2277da1af47f9a500fff266fa0eea1145bdb690f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:47:26 +0000 Subject: [PATCH] Fix duplicate crafting job dependencies when interfaces re-register Crafting interfaces re-register to the network when they become dirty, which added the dependencies of their jobs to the dependency graph again. This caused duplicated subtrees in crafting plans of running jobs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01C5iUzweRYoCkKB5bvJdH8Y --- .../crafting/CraftingJobDependencyGraph.java | 9 ++- .../GameTestsCraftingJobDependencies.java | 74 +++++++++++++++++++ .../TestCraftingJobDependencyGraph.java | 17 +++++ 3 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/cyclops/integratedcrafting/gametest/GameTestsCraftingJobDependencies.java diff --git a/src/main/java/org/cyclops/integratedcrafting/api/crafting/CraftingJobDependencyGraph.java b/src/main/java/org/cyclops/integratedcrafting/api/crafting/CraftingJobDependencyGraph.java index dda6cfe86..e6e8ff4e0 100644 --- a/src/main/java/org/cyclops/integratedcrafting/api/crafting/CraftingJobDependencyGraph.java +++ b/src/main/java/org/cyclops/integratedcrafting/api/crafting/CraftingJobDependencyGraph.java @@ -167,7 +167,10 @@ public void addDependency(CraftingJob craftingJob, int dependency) { jobDependencies = new IntArrayList(); dependencies.put(craftingJob.getId(), jobDependencies); } - jobDependencies.add(dependency); + // An edge is only stored once, as crafting interfaces add the edges of their jobs again when they re-register + if (!jobDependencies.contains(dependency)) { + jobDependencies.add(dependency); + } // Save reverse link IntCollection jobDependents = dependents.get(dependency); @@ -175,7 +178,9 @@ public void addDependency(CraftingJob craftingJob, int dependency) { jobDependents = new IntArrayList(); dependents.put(dependency, jobDependents); } - jobDependents.add(craftingJob.getId()); + if (!jobDependents.contains(craftingJob.getId())) { + jobDependents.add(craftingJob.getId()); + } } public void removeDependency(int craftingJob, int dependency) { diff --git a/src/main/java/org/cyclops/integratedcrafting/gametest/GameTestsCraftingJobDependencies.java b/src/main/java/org/cyclops/integratedcrafting/gametest/GameTestsCraftingJobDependencies.java new file mode 100644 index 000000000..d154a21c3 --- /dev/null +++ b/src/main/java/org/cyclops/integratedcrafting/gametest/GameTestsCraftingJobDependencies.java @@ -0,0 +1,74 @@ +package org.cyclops.integratedcrafting.gametest; + +import net.minecraft.core.BlockPos; +import net.minecraft.gametest.framework.GameTest; +import net.minecraft.gametest.framework.GameTestHelper; +import net.minecraft.resources.ResourceLocation; +import net.minecraft.world.item.ItemStack; +import net.minecraft.world.item.Items; +import net.minecraft.world.item.crafting.RecipeType; +import net.minecraft.world.level.block.entity.ChestBlockEntity; +import net.neoforged.neoforge.gametest.GameTestHolder; +import net.neoforged.neoforge.gametest.PrefixGameTestTemplate; +import org.apache.commons.lang3.tuple.Triple; +import org.cyclops.commoncapabilities.IngredientComponents; +import org.cyclops.commoncapabilities.api.capability.itemhandler.ItemMatch; +import org.cyclops.integratedcrafting.Reference; +import org.cyclops.integratedcrafting.api.crafting.CraftingJob; +import org.cyclops.integratedcrafting.api.crafting.CraftingJobDependencyGraph; +import org.cyclops.integratedcrafting.core.CraftingHelpers; +import org.cyclops.integratedcrafting.part.PartTypeInterfaceCrafting; +import org.cyclops.integrateddynamics.api.network.INetwork; +import org.cyclops.integrateddynamics.api.network.IPositionedAddonsNetwork; +import org.cyclops.integrateddynamics.core.helper.NetworkHelpers; + +import static org.cyclops.integratedcrafting.gametest.GameTestHelpersIntegratedCrafting.createBasicNetwork; + +/** + * Tests for the dependencies between crafting jobs in the network. + * + * @author rubensworks + */ +@GameTestHolder(Reference.MOD_ID) +@PrefixGameTestTemplate(false) +public class GameTestsCraftingJobDependencies { + + public static final String TEMPLATE_EMPTY = "empty10"; + public static final int TIMEOUT = 2000; + public static final BlockPos POS = BlockPos.ZERO.offset(2, 0, 2); + + /** + * A crafting interface that re-registers itself to the network must not duplicate the dependencies of its jobs. + */ + @GameTest(template = TEMPLATE_EMPTY, timeoutTicks = TIMEOUT) + public void testDependenciesNotDuplicatedOnInterfaceReRegister(GameTestHelper helper) { + GameTestHelpersIntegratedCrafting.INetworkPositions positions = createBasicNetwork(helper, POS); + ChestBlockEntity chestIn = helper.getBlockEntity(POS.east()); + chestIn.setItem(0, new ItemStack(Items.OAK_LOG, 2)); + positions.interfaceRecipeAdders().get(0).accept(Triple.of(0, RecipeType.CRAFTING, ResourceLocation.fromNamespaceAndPath("minecraft", "chest"))); + positions.interfaceRecipeAdders().get(0).accept(Triple.of(1, RecipeType.CRAFTING, ResourceLocation.fromNamespaceAndPath("minecraft", "oak_planks"))); + + helper.runAfterDelay(5, () -> { + INetwork network = NetworkHelpers.getNetworkChecked(positions.writer().getPos().getLevel(true), + positions.writer().getPos().getBlockPos(), positions.writer().getSide()); + CraftingJob craftingJob = CraftingHelpers.calculateAndScheduleCraftingJob(network, IPositionedAddonsNetwork.DEFAULT_CHANNEL, + IngredientComponents.ITEMSTACK, new ItemStack(Items.CHEST), ItemMatch.ITEM, true, false, + CraftingHelpers.getGlobalCraftingJobIdentifier(), null); + helper.assertTrue(craftingJob != null, "Crafting job could not be scheduled"); + CraftingJobDependencyGraph graph = CraftingHelpers.getCraftingNetworkChecked(network).getCraftingJobDependencyGraph(); + helper.assertValueEqual(graph.getDependencies(craftingJob).size(), 1, "Dependencies before re-registering"); + + // Re-registers the interface and its jobs to the network + positions.interfaceStates().get(0).onDirty(); + + helper.assertValueEqual(graph.getDependencies(craftingJob).size(), 1, "Dependencies after re-registering"); + CraftingJob dependency = graph.getDependencies(craftingJob).iterator().next(); + helper.assertValueEqual(graph.getDependents(dependency).size(), 1, "Dependents after re-registering"); + + helper.succeedWhen(() -> { + helper.assertValueEqual(chestIn.getItem(0).getItem(), Items.CHEST, "Slot 0 item is incorrect"); + helper.assertValueEqual(chestIn.getItem(0).getCount(), 1, "Slot 0 amount is incorrect"); + }); + }); + } +} diff --git a/src/test/java/org/cyclops/integratedcrafting/api/crafting/TestCraftingJobDependencyGraph.java b/src/test/java/org/cyclops/integratedcrafting/api/crafting/TestCraftingJobDependencyGraph.java index b56f723b9..09b6e5d20 100644 --- a/src/test/java/org/cyclops/integratedcrafting/api/crafting/TestCraftingJobDependencyGraph.java +++ b/src/test/java/org/cyclops/integratedcrafting/api/crafting/TestCraftingJobDependencyGraph.java @@ -71,6 +71,23 @@ public void testSingleDependencyDouble() { assertThat(Sets.newHashSet(g.getDependents(J1)), equalTo(Sets.newHashSet(J0))); } + @Test + public void testSingleDependencyDoubleStoredOnce() { + g.addDependency(J0, J1); + g.addDependency(J0, J1.getId()); + assertThat(Lists.newArrayList(g.getDependencies(J0)), equalTo(Lists.newArrayList(J1))); + assertThat(Lists.newArrayList(g.getDependents(J1)), equalTo(Lists.newArrayList(J0))); + } + + @Test + public void testSingleDependencyDoubleRemove() { + g.addDependency(J0, J1); + g.addDependency(J0, J1); + g.removeDependency(0, 1); + assertThat(Sets.newHashSet(g.getCraftingJobs()), equalTo(Sets.newHashSet())); + assertThat(g.hasDependencies(J0), equalTo(false)); + } + @Test public void testSingleDependencyRemove() { g.addDependency(J0, J1);