Skip to content

Fix duplicate crafting job dependencies when interfaces re-register - #248

Merged
rubensworks merged 1 commit into
master-1.21-ltsfrom
fix/duplicate-dependency-edges
Oct 14, 2026
Merged

rubensworks merged 1 commit into
master-1.21-ltsfrom
fix/duplicate-dependency-edges

Conversation

@rubensworks

Copy link
Copy Markdown
Member

When a crafting interface becomes dirty (e.g. a recipe or setting changes), PartTypeInterfaceCraftingVariableBase.State.onDirty() unregisters and re-registers it to the crafting network. removeCraftingInterface only removes the job ids from the dependency graph and keeps the edges, while addCraftingInterface adds the edges of each job again. CraftingJobDependencyGraph.addDependency did not check for existing edges, so each re-registration duplicated them. In Integrated Terminals this shows up as duplicated subtrees in the plan of a running job.

addDependency now stores each edge (and its reverse link) only once.

Tests:

  • Unit tests for adding the same dependency twice, and removing it afterwards.
  • Game test that schedules a chest job with a planks dependency, re-registers the interface via onDirty(), and checks the dependencies are not duplicated. It fails without the fix (2 instead of 1).

🤖 Generated with Claude Code

https://claude.ai/code/session_01C5iUzweRYoCkKB5bvJdH8Y


Generated by Claude Code

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5iUzweRYoCkKB5bvJdH8Y

Copy link
Copy Markdown
Member Author

coverage/coveralls is red with a -0.09% coverage drop. This PR does not cause it: the only changed main code is the two new contains checks in CraftingJobDependencyGraph.addDependency, and the new unit tests and game test cover both branches of each. The new game test also adds covered lines of its own. A drop this small comes from code paths in other game tests that depend on tick timing and so vary from run to run. I couldn't open the per-file Coveralls breakdown (403/500 from here) to point at the exact lines. The Build job, which includes unit and game tests, is green.


Generated by Claude Code

@rubensworks
rubensworks merged commit 660c68c into master-1.21-lts Oct 14, 2026
7 checks passed
@rubensworks
rubensworks deleted the fix/duplicate-dependency-edges branch October 14, 2026 18:07
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.

2 participants