Skip to content

Fix parallel crafting jobs all completing on a single produced output - #226

Open
rubensworks wants to merge 1 commit into
master-1.21-ltsfrom
fix/parallel-jobs-single-output
Open

Fix parallel crafting jobs all completing on a single produced output#226
rubensworks wants to merge 1 commit into
master-1.21-ltsfrom
fix/parallel-jobs-single-output

Conversation

@rubensworks

Copy link
Copy Markdown
Member

Important

Depends on CyclopsMC/IntegratedDynamics#1720 and does not compile until that is merged and published. Two dependency bumps in gradle.properties are deliberately left out of this PR, because the Integrated Dynamics build number does not exist yet:

  • integrateddynamics_version to the build produced by that PR
  • neoforge_version to at least 21.1.228, which that branch of Integrated Dynamics requires

Both were applied locally to run everything below.

What was going wrong

integratedcrafting:gametestsitemscraft_testitemscraftironingotsparallelfromdeppartial failed rarely and randomly. Reproduced by running 40 copies of it per game test run, over 12 runs: 1 failure in 480, with the chest ending on

[chest: 0=1xminecraft:raw_iron 1=10xminecraft:iron_ingot]

instead of the expected 6 raw iron and 5 iron ingots. It crafted a whole second batch of 5 ingots.

Root cause

Every crafting interface registers its own PendingCraftingJobResultIndexObserver as an insert pre-consumer, and all of them observe the same insertion. Resolving a pending crafting job output does not consume the instance (it still has to end up in storage), so the instance was passed on to the next pre-consumer unchanged, and every one of them resolved its own job's pending output from that one item.

In this test that means the first iron ingot returned by any of the five furnaces marked all five parallel jobs as finished. Those jobs then left the crafting network within an update interval, while the other four ingots were still on their way, and CraftingAspectWriteBuilders.PROP_CRAFT re-triggers as soon as it sees neither a running job nor enough of the requested ingredient in storage. So it scheduled the job again, which ate the leftover raw iron.

It is a race only because the remaining ingots normally arrive before the last job is removed from the network. The over-crafting itself is not specific to this test: it applies to any parallelised crafting job.

Fix

PendingCraftingJobResultIndexObserver now only resolves pending outputs with the part of an insertion that no other crafting interface has accounted for yet, using the pre-consumer API added in CyclopsMC/IntegratedDynamics#1720.

PartTypeInterfaceCraftingBase offers a crafting result to its own jobs before flushing it into the network, so flushIngredientToNetwork now carries that accounting into the network insertion as well, otherwise the same double-accounting would happen on that path.

Testing

  • New TestPendingCraftingJobResultIndexObserver runs two crafting interfaces with a parallel job each through the real pre-consumer chain and asserts one produced output resolves exactly one of them, while still ending up in storage. It fails on the code before this fix ("the output does not resolve the second job: Expected: <1> but: was <0>") and passes after.
  • ./gradlew build passes.
  • ./gradlew runGameTestServer passes, all 62 tests.
  • The 40-copy reproduction above was rerun 15 times against the fix: 600 samples of the flaky test, no failures.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EypmxuPjaJ4cFGh2DvZYbf


Generated by Claude Code

Every crafting interface registers its own insert pre-consumer on the network's
ingredient channel, and all of them observe the same insertion. Because a
pending crafting job output is resolved without consuming the instance, the
instance passed on to the next pre-consumer unchanged, so one produced item
resolved the pending output of every parallel job at once.

Those jobs then reported completion before their outputs were actually in
storage. As soon as the last of them left the crafting network, a crafting
writer saw neither a running job nor enough of the requested ingredient, and
scheduled the whole job all over again.

This showed up as a rare failure of
testItemsCraftIronIngotsParallelFromDepPartial, which crafted 10 iron ingots
instead of 5, but it applies to any parallelised crafting job.

The observer now only resolves pending outputs with the part of an insertion
that no other crafting interface has accounted for yet. Crafting results that a
crafting interface first offers to its own jobs carry that accounting along into
the network insertion, so they can not be accounted for a second time there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EypmxuPjaJ4cFGh2DvZYbf

Copy link
Copy Markdown
Member Author

CI (Build) is red on :compileJava, as expected and as noted at the top of the description:

core/PendingCraftingJobResultIndexObserver.java:54: error: cannot find symbol
    public IIngredientChannelInsertPreConsumer.Result<T> addIngredient(T instance, T unaccounted, int channel, boolean simulate) {
                                              ^
  symbol:   class Result

IIngredientChannelInsertPreConsumer.Result and INetworkIngredientsChannel.insert(ingredient, unaccounted, simulate) are added by CyclopsMC/IntegratedDynamics#1720, which is green but not merged or published yet. The released integrateddynamics_version=1.32.0-1630 this branch builds against does not have them, so there is nothing to fix on this side.

Two gradle.properties bumps are needed once that PR is published, and I left them out rather than commit a build number that does not exist yet:

With both applied locally, ./gradlew build and ./gradlew runGameTestServer pass here. Happy to push the bumps as soon as the build number exists.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant