[8.0] Custom Graph Solution #1066
No reviewers
Labels
No labels
Area-Assets
Area-Backend
Area-Conduits
Area-Datapacks
Area-Lang
Area-Mod Compat
Area-Parity
Area-Rendering
Good first issue
MC-1.19.2
MC-1.20.1
MC-1.20.4
MC-1.20.6
MC-1.21
MC-1.21.1
Modtoberfest
P-0-High
P-1-Medium
P-2-Low
Status-Awaiting Response
Status-Behind-Flag
Status-Blocked
Status-Cannot Reproduce
Status-Duplicate
Status-Help Wanted
Status-Incomplete Report
Status-Invalid
Status-Needs LTS Backport
Status-Needs Updating
Status-Stale
Status-To Implement
Status-Triage
Status-Wontfix
Status-Wontmerge
Type-Backport
Type-Bug
Type-Documentation
Type-Enhancement
Type-Question
Type-RFC
Type-Suggestion
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: Team-EnderIO/EnderIO#1066
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/graph-library"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Description
This implements a custom graph network system in EnderCore, which is then used and expanded by conduits to cache commonly queried information about networks for tickers.
This results in much simpler conduit tickers, and they should be much faster than previous versions as they do not need to filter nodes every tick.
Closes: #920
TODO
Explore partial cache updates rather than removing and re-adding a node.Deferred to "later"Networktests are comprehensive enoughAfter this PR
After this PR, I'll revisit #922 and recreate the relevant bugfixes with our
Networksystem.Breaking Changes
Changes the save format of our conduit network saved data somewhat, it will not be possible to load a v7.1 world after loading v8 (this is acceptable as this was already the case).
Checklist
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores
Tests
Walkthrough
This update replaces the old graph-based conduit network system with a new, custom network and node implementation. It introduces new APIs (
IConduitNetwork,IConduitNode), rewrites network persistence and ticking, and refactors all conduit types, tickers, and GUIs to use the new abstractions. Legacy graph classes and interfaces are removed, and comprehensive automated and game tests are added for validation.Changes
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/*,.../network/ConduitNetwork.java,.../network/ConduitNode.java,.../network/ConduitNetworkSavedData.java,.../network/IConduitNetwork.java,.../network/ConduitBlockConnection.java,.../network/node/IConduitNode.java,.../network/node/NodeDataType.java,.../network/node/NodeDataAccessor.java,.../network/node/ConduitNode.java,.../network/ConduitNetworkContextType.java,.../network/ConduitNetworkContext.javaenderio-conduits/src/main/java/com/enderio/conduits/api/Conduit.java,.../bundle/ConduitBundle.java,.../connection/config/IOConnectionConfig.java,.../ticker/ConduitTicker.java,.../ticker/IOAwareConduitTicker.java,.../ticker/LoadedAwareConduitTicker.javaenderio-conduits/src/main/java/com/enderio/conduits/common/conduit/bundle/ConduitBundleBlockEntity.java,.../common/conduit/menu/ConduitMenu.java,.../common/conduit/ConduitBlockItem.javaenderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/*enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/*Ticker.javaConduitTickerinterface and operate onIConduitNetwork, removing legacy connection logic.enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/*,.../client/model/conduit/bundle/ConduitConnectionRenderState.javaenderio-conduits/src/main/java/com/enderio/conduits/server/tests/*enderio-conduits/build.gradle.kts,endercore/build.gradle.kts,enderio/build.gradle.kts,.github/workflows/verify.ymlendercore/src/main/java/com/enderio/core/common/graph/*,endercore/src/test/java/com/enderio/core/*enderio-machines/src/main/java/com/enderio/machines/common/travel/EnderfaceTravelTarget.javaenderio-conduitsforRemoval = true; legacy code retained for compatibility but not for removal.Sequence Diagram(s)
Assessment against linked issues
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
❤️ Share
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
I pushed a fix in commit <commit_id>, please review it.Generate unit testing code for this file.Open a follow-up GitHub issue for this discussion.@coderabbitaiin a new review comment at the desired location with your query. Examples:@coderabbitai generate unit testing code for this file.@coderabbitai modularize this function.@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read src/utils.ts and generate unit testing code.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.@coderabbitai help me debug CodeRabbit configuration file.Support
Need help? Create a ticket on our support page for assistance with any issues or questions.
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.
CodeRabbit Commands (Invoked using PR comments)
@coderabbitai pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai generate docstringsto generate docstrings for this PR.@coderabbitai generate sequence diagramto generate a sequence diagram of the changes in this PR.@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.Other keywords and placeholders
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed.@coderabbitai summaryto generate the high-level summary at a specific location in the PR description.@coderabbitaianywhere in the PR title to generate the title automatically.CodeRabbit Configuration File (
.coderabbit.yaml).coderabbit.yamlfile to the root of your repository.# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.jsonDocumentation and Community
@ -1,30 +1,52 @@name: Verify Buildname: Game and Unit TestsWorkflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
Show more details
@ -1,30 +1,52 @@name: Verify Buildname: Game and Unit TestsWorkflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
Show more details
We don't have full conduit test coverage, but I think we're in a good enough position to consider this for merge now.
Actionable comments posted: 33
🔭 Outside diff range comments (7)
🧹 Nitpick comments (50)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between
4e6e0501bfand 7d1bf03e24a43c72c05ab8410df14e022d055b46.📒 Files selected for processing (100)
.github/workflows/verify.yml(1 hunks)endercore/build.gradle.kts(2 hunks)endercore/src/main/java/com/enderio/core/common/graph/BasicNetwork.java(1 hunks)endercore/src/main/java/com/enderio/core/common/graph/INetworkNode.java(1 hunks)endercore/src/main/java/com/enderio/core/common/graph/Network.java(1 hunks)endercore/src/main/java/com/enderio/core/common/graph/package-info.java(1 hunks)endercore/src/test/java/com/enderio/core/NetworkTests.java(1 hunks)endercore/src/test/java/com/enderio/core/TestNode.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/client/modules/mekanism/screens/ChemicalConduitScreenType.java(4 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/client/modules/mekanism/screens/HeatConduitScreenType.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/appeng/MEConduit.java(5 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalConduit.java(7 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalConduitConnectionConfig.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalConduitNetworkContext.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalTicker.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/heat/HeatConduitConnectionConfig.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/heat/HeatTicker.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/refinedstorage/RSConduit.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/refinedstorage/RSConduitNodeData.java(3 hunks)enderio-conduits/build.gradle.kts(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ColoredRedstoneProvider.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/Conduit.java(9 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/EnderIOConduitsRegistries.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/bundle/ConduitBundle.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/bundle/SlotType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/connection/config/IOConnectionConfig.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/model/ConduitModelModifier.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitBlockConnection.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetwork.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContext.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContextAccessor.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContextType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/IConduitNetwork.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/ConduitNode.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/IConduitNode.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/NodeData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/NodeDataAccessor.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/NodeDataType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitDataAccessor.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitDataType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/screen/IOConduitScreenType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/ConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/IOAwareConduitTicker.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/LoadedAwareConduitTicker.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/EnergyConduitScreenType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/FluidConduitScreenType.java(4 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/ItemConduitScreenType.java(3 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/RedstoneConduitScreenType.java(3 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/model/conduit/bundle/ConduitBundleModel.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/model/conduit/bundle/ConduitConnectionRenderState.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitBlockItem.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitSavedData.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/bundle/ConduitBundleBlock.java(3 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/bundle/ConduitBundleBlockEntity.java(27 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitConnectionHost.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphContext.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphObject.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphUtility.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/WrappedConduitNetwork.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/ConduitDataContainer.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/ConnectionState.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/DynamicConnectionState.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/LegacyFluidConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/LegacyItemConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/LegacyRedstoneConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/StaticConnectionStates.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/menu/ConduitMenu.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNetwork.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNetworkSavedData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNode.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/package-info.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduit.java(5 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitConnectionConfig.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitNetworkContext.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitStorage.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitTicker.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduit.java(6 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitConnectionConfig.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitNetworkContext.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduit.java(7 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduitConnectionConfig.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduit.java(4 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduitConnectionConfig.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduitNetworkContext.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/init/ConduitLang.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/integrations/cctweaked/EIOBundledRedstoneProvider.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/network/ConduitCommonPayloadHandler.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/EnderIOConduitsServer.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/BasicConduitTests.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/ConduitGameTestHelper.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/item/ItemConduitGameTestHelper.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/item/ItemConduitTests.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/item/package-info.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/package-info.java(1 hunks)enderio-machines/src/main/java/com/enderio/machines/common/travel/EnderfaceTravelTarget.java(1 hunks)enderio/build.gradle.kts(1 hunks)💤 Files with no reviewable changes (13)
🧰 Additional context used
🧬 Code Graph Analysis (15)
enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/NodeData.java (2)
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/ConduitDataContainer.java (3)
enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/FluidConduitScreenType.java (1)
enderio-conduits/src/main/java/com/enderio/conduits/common/init/ConduitLang.java (1)
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitTicker.java (1)
enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContext.java (1)
enderio-conduits-modded/src/main/java/com/enderio/modconduits/client/modules/mekanism/screens/HeatConduitScreenType.java (1)
enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalConduit.java (1)
enderio-conduits-modded/src/main/java/com/enderio/modconduits/client/modules/mekanism/screens/ChemicalConduitScreenType.java (1)
enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/ItemConduitScreenType.java (1)
enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/refinedstorage/RSConduit.java (1)
enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/EnergyConduitScreenType.java (1)
endercore/src/test/java/com/enderio/core/NetworkTests.java (1)
enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalTicker.java (1)
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNetwork.java (1)
🪛 YAMLlint (1.35.1)
.github/workflows/verify.yml
[error] 42-42: trailing spaces
(trailing-spaces)
🔇 Additional comments (116)
@ -0,0 +19,4 @@* Do not call this method directly.* @param network The network this node is a member of, or null if this node is being invalidated.*/void setNetwork(@Nullable TNet network);🛠️ Refactor suggestion
setNetworkis public – contradicts the “do not call directly” contractBecause interface methods are implicitly
public, nothing prevents third-party code (or even your own conduit logic) from callingsetNetworkand breaking network integrity.Two possible mitigations:
or move the write-side contract into a separate package-private interface that only the network implementation sees.
Either will make misuse harder and signal intent clearly in IDEs.
📝 Committable suggestion
@ -0,0 +63,4 @@var node = nodes.getFirst();graph.addNode(node);node.setNetwork(self());onNodeAdded(node);🛠️ Refactor suggestion
List#getFirst()requires Java 21List#getFirst()was introduced in Java 21, but the NeoForge toolchain targets Java 17.Using it will break compilation for anyone not on the latest toolchain.
Perform the same replacement at line 350.
📝 Committable suggestion
@ -15,38 +15,39 @@ import net.minecraft.network.codec.ByteBufCodecs;import net.minecraft.network.codec.StreamCodec;⚠️ Potential issue
STREAM_CODEC uses the same getter ordering issue
The byte-buffer codec must match the
MapCodec, otherwise network syncs between server and client will desynchronise.(only correct if you rename the field in the previous comment).
⚠️ Potential issue
Mismatch between serialised field name and getter – will break world-load compatibility
extractColoris written under the key"extract_channel", but the getter supplied isextractColor().When the config is (de)serialised the colour will silently end up in the wrong JSON/NBT field, causing old saves to fail to load once the key is corrected, or – worse – produce silent data loss.
📝 Committable suggestion
🛠️ Refactor suggestion
reconnected()/disconnected()override the user’s settingsBoth methods hard-code
isInsert/isExtracttoDEFAULT(orfalse), discarding the state that existed before the block was broken and re-placed. Players will be surprised to find their carefully set IO-modes reset.Apply the same idea to
disconnected().@ -24,0 +31,4 @@if (extractHandler == null) {continue;}⚠️ Potential issue
Integer overflow when computing
transferRatetransferRatePerTick()andnetworkTickRate()areints.int * intoverflows before the value is widened tolong, producing negative or wrapped results for large buffers.📝 Committable suggestion
@ -91,28 +91,28 @@ public class ItemConduitScreenType extends IOConduitScreenType<ItemConduitConnec// Redstone control⚠️ Potential issue
GUI still fetches
receiveRedstoneChannelfor extract side – causes wrong colour to displayThe redstone channel widget for the extract panel reads the receive value, so the picker can display (and later serialise) the wrong colour.
Make the same change wherever the supplier lambda is used across screen types.
@ -1036,6 +968,93 @@ public final class ConduitBundleBlockEntity extends EnderBlockEntity⚠️ Potential issue
Null-check order causes unavoidable
NullPointerExceptionWhen
levelisnullthe right-hand side of the&&is still evaluated, dereferencingleveland crashing.Swap the order or split the condition:
🛠️ Refactor suggestion
Neighbour-capability caches never cleared
NeighboringCapabilityCachessurvive the full lifetime of the bundle and are not invalidated on chunk unload, neighbour removal, or conduit removal. This can retain staleBlockCapabilityCacheinstances referencing unloaded chunks, blocking garbage collection and potentially leaking memory over long sessions (e.g. on large servers).Provide an explicit
clear()/invalidate()path and invoke it from:removeConduitonChunkUnloadedupdateConnections)@ -0,0 +1,580 @@package com.enderio.conduits.common.conduit.network;🛠️ Refactor suggestion
Connection-list duplication & ordering costs
addLoadedNode()pushes every newConduitBlockConnectioninto all existingaccessibleBlockConnectionsMapvalue lists.For large dense networks this becomes O(n²) memory and time, and duplicates are possible when two nodes share multiple endpoints.
A
Set(LinkedHashSet to preserve order) eliminates duplicates and drops complexity to O(n) for insertion, at the cost of marginally slower sorting later.⚠️ Potential issue
onChunkCoverageChangedcan be null – defensively check beforeaccept()addNodeToPositionMaps()invokesonChunkCoverageChanged.accept(this)without verifying that the consumer has been registered.On servers that never wire up the callback this leads to an immediate
NullPointerExceptiononce the first network spreads over more than one chunk.📝 Committable suggestion
@ -0,0 +1,341 @@package com.enderio.conduits.common.conduit.network;⚠️ Potential issue
Removing empty networks here breaks internal chunk-tracking maps
networks.values().removeIf(network -> network.isValid() && network.isEmpty());only removes the reference fromnetworks.networksByChunkandchunksByNetworkkeep dangling entries, so later look-ups (e.g., inonChunkTicketLevelUpdated) will still hand out a discarded network, leading to NPEs or logic errors.Prefer using existing disposal logic so all maps stay in sync:
📝 Committable suggestion
@ -0,0 +166,4 @@public boolean isDirty() {// Always save networks when the opportunity arisesreturn true;}🛠️ Refactor suggestion
isDirty()returningtrueevery tick will spam world-saving I/OAlways returning
truecauses the Minecraft data storage layer to write this file to disk on every save cycle (normally each tick).Large worlds with many conduits will suffer significant, needless disk I/O and TPS loss.
Introduce an internal
dirtyflag and callsetDirty()(inherited) whenever networks are added/removed or their data actually mutates.@ -0,0 +1,302 @@package com.enderio.conduits.common.conduit.network;🛠️ Refactor suggestion
markDirty()throws on perfectly legitimate calls while the chunk is still loadingmarkDirty()callsensureValid()which in turn checksisLoaded().During world-load (e.g. when the bundle is constructed but the chunk is not yet ticking) any innocent attempt to mark the node dirty will blow up with an
IllegalStateException. Nothing in the Javadoc warns callers about this pre-condition.Consider relaxing the contract so that marking dirty is always safe (no-op when not loaded), or – at the very least – document the restriction prominently and keep the public API exception-free.
@ -0,0 +298,4 @@(side, config) -> conduitBundle.setConnectionConfig(conduit, side, config));legacyDataContainer = null;}}⚠️ Potential issue
Potential NPE in
tryCopyLegacyData()whenconduitBundleis stillnullcopyLegacyData()invokesconduitBundle.setConnectionConfig(...), butconduitBundleis only set onceattach()has run.If
setNetwork()is called beforeattach()(which happens when the node is merged into an existing graph during deserialisation) the dereference will crash.Guard with a null-check or call this helper from
attach()instead, once the bundle reference is guaranteed.@ -102,18 +101,17 @@ public record EnergyConduit(ResourceLocation texture, Component description, int}💡 Verification agent
🧩 Analysis chain
Redstone channel accessor mismatch will block extraction
proxyCapabilitynow checks extraction settings but still queriesconfig.receiveRedstoneChannel().When a player configures an extract side and assigns an extract redstone channel, that channel will be ignored – the conduit falls back to the default and may stay disabled.
Please update the getter to
extractRedstoneChannel()(and any related references) to ensure the redstone gate works as intended.🏁 Script executed:
Length of output: 1714
Update extract-side redstone channel getters
The extract logic still reads from
receiveRedstoneChannel(), so any configured extract channel will be ignored at runtime and in the GUI. Please update all of the following to useextractRedstoneChannel()instead:This ensures the redstone gate for extraction respects the player-selected channel.
@ -15,93 +15,94 @@ import net.minecraft.network.codec.ByteBufCodecs;import net.minecraft.network.codec.StreamCodec;🛠️ Refactor suggestion
STREAM_CODEC uses the same inconsistent getter
Same mismatch as above – replace the getter with the correctly named one to stay in sync with the record fields.
⚠️ Potential issue
Compilation error: direct field access on a record
DEFAULT.isInsertandDEFAULT.isExtractaccess the synthetic fields, which areprivatein a record.Use the accessor methods instead.
📝 Committable suggestion
🛠️ Refactor suggestion
Missing accessor calls in
withIsExtractFor consistency and to avoid future access-level issues, prefer the accessor methods.
📝 Committable suggestion
⚠️ Potential issue
Codec field names and getters are inconsistent
The codec keys were changed to the new “insert/extract” terminology, but the getter for the channel still points to
receiveRedstoneChannel, which is now semantically “extract”.Rename the record component and getter to
extractRedstoneChannel(or adjust the codec mapping) to avoid confusion and future maintenance errors.📝 Committable suggestion
⚠️ Potential issue
withIsInsertflips the wrong valueThe parameter is named
isExtract, and the new instance places it into the insert slot, effectively swapping the two flags.@ -55,8 +58,17 @@ public class EnergyConduitNetworkContext implements ConduitNetworkContext<Energy}⚠️ Potential issue
Potential divide-by-zero and energy loss during split
allNetworks.stream()...reduce(0, Integer::sum)may return0when the list is empty (or all networks report0nodes), which leads to a division by zero andInfinityproportion, corrupting stored energy.Secondly,
Math.flooron each network can drop residual joules, making the sum of children<parent.This guards against edge cases and conserves total energy across all split networks.
📝 Committable suggestion
@ -15,38 +15,38 @@ import net.minecraft.network.codec.ByteBufCodecs;import net.minecraft.network.codec.StreamCodec;🛠️ Refactor suggestion
Same key mismatch as the chemical config
extractColoris stored under"extract_channel"– please rename to"extract_color"to avoid corrupting saved data.@ -55,72 +55,72 @@ public record FluidConduitConnectionConfig(boolean isSend, DyeColor sendColor, b🛠️ Refactor suggestion
reconnected()/disconnected()overwrite previous IO flagsSame issue as in ChemicalConduitConnectionConfig – the player’s insert/extract settings are lost.
⚠️ Potential issue
splitloseslastLockedFluidhistoryWhen a network is split, you create a new context with only
lockedFluid, resettinglastLockedFluidtoEMPTY. Any ticker logic relying onlastLockedFluidto mark nodes dirty will miss the change. Should the split copy both fields?📝 Committable suggestion
@ -1,16 +1,12 @@package com.enderio.conduits.common.conduit.type.fluid;⚠️ Potential issue
Incorrect filter slot used for inserts – may bypass insert-side filters
insertConnection.inventory().getStackInSlot(FluidConduit.EXTRACT_FILTER_SLOT)queries the extract filter slot while validating an insert action.If
INSERT_FILTER_SLOTexists (as it does for the other conduit types) this line will silently ignore player-configured insert filters and could allow unwanted fluids into inventories.📝 Committable suggestion
@ -16,33 +16,34 @@ import net.minecraft.network.codec.ByteBufCodecs;import net.minecraft.network.codec.StreamCodec;⚠️ Potential issue
Inconsistent “receive” vs “extract” nomenclature breaks serialisation and API coherence
The field is still called
receiveRedstoneChannel, yet every other identifier was migrated to the newextract*terminology.•
Recordparameter 6, the corresponding accessor, the codec key ("extract_redstone_channel"), theStreamCodecgetter, and thewithExtractRedstoneChannelbuilder now reference different names.• Aside from the mental overhead, this will desynchronise data between server ↔ client and disk ↔ RAM because codecs write
"extract_redstone_channel"but read viareceiveRedstoneChannel().Apply the rename consistently:
extractRedstoneChannel.DEFAULT, builder methods, etc.) → pass the renamed variable.ItemConduitConnectionConfig::extractRedstoneChannel.Failing to do this will silently corrupt saved worlds.
Also applies to: 28-36, 41-48, 118-121
@ -1,114 +1,110 @@package com.enderio.conduits.common.conduit.type.item;🛠️ Refactor suggestion
Round-robin exit condition may skip remaining items
isEmpty(extractHandler, i + 1)ignores the current slot (i).If only part of the stack in the current slot was moved (e.g., transfer rate not reached or destination full) the ticker will prematurely abandon this extract connection and leave items unprocessed.
A simpler, safer exit check is to rely solely on the transfer-rate cap:
If you still want the early-out optimisation, scan from slot 0 so the current slot is included.
📝 Committable suggestion
@ -1,97 +1,61 @@package com.enderio.conduits.common.conduit.type.redstone;🛠️ Refactor suggestion
Neighbour updates executed redundantly for every channel
network.insertConnections()is iterated for every channel whenever any channel’s signal changes, resulting inupdateNeighborsAtbeing calledchannels × insertstimes per tick.On larger networks this scales poorly and causes needless block-update spam.
Suggestion: collect all unique
insertConnections whose output has actually changed and update them once:This preserves behaviour while cutting per-tick work dramatically.
⚠️ Potential issue
Potential NPE on missing inventory or capability
extractConnection.inventory()may legitimately returnnull(e.g., if the block entity was removed between ticks).Accessing
getStackInSlot(...).getCapability(...)without a preceding null-check will crash the server in that scenario.Alternatively, make
extractConnection.inventory()returnOptional<IItemHandler>to enforce safe access throughout the codebase.📝 Committable suggestion
@ -0,0 +34,4 @@.set(2, 0, 2, Blocks.CHEST.defaultBlockState()));// @formatter:on⚠️ Potential issue
Template height too small for later y=1 edits
All three tests place or edit blocks at
y = 1, but the registered template is only one block tall (height = 1).StructureTemplateBuilderbounds the test area; attempting to touchy = 1will therefore throw an out-of-bounds exception and fail the test before the logic runs.Do the same for the ad-hoc template in
itemConduitBasicTransfer.📝 Committable suggestion
@ -0,0 +107,4 @@// Ensure the item moves to the closer chest.thenExecuteAfter(tickRate, () -> helper.assertContainerHasExactly(2, 1, 2, Items.DIRT, 1))// Place an item into the first chest.thenSucceed();🛠️ Refactor suggestion
Chest y-coordinate off by one
The template places all chests at
y = 0, yet the assertions referencey = 1(e.g.,assertContainerHasExactly(2, 1, 2, …)).With the corrected template height this will point at air blocks and the tests will still fail.
Update every assertion and helper call that targets a chest to use
y = 0.Also applies to: 141-144, 178-180
💡 Verification agent
🧩 Analysis chain
Good fix for server-side compatibility issue.
The commented-out client-side screen code correctly addresses the classloading errors on dedicated servers. This avoids attempting to load client-only classes in a server context.
Consider implementing a proper client-server communication pattern to replace this temporary solution:
🏁 Script executed:
Length of output: 66
🏁 Script executed:
Length of output: 272
Implement proper client–server communication to open the Enderface screen
I didn’t find any registered
SimpleChannelor activeNetworkHooksusage for opening GUIs (aside from the commented line inCapacitorBankBlock.java:95). To replace the temporary client-only call, please:• Create and register a
SimpleChannel(e.g. incom.enderio.machines.network) during mod setup usingNetworkRegistry.newSimpleChannel.• Define an
OpenEnderfaceScreenPacketwith encode/decode methods and a handler.• From the server side (in
EnderfaceTravelTarget), send this packet when the travel target is activated.• In the client handler, call
to open the GUI without causing class-load errors on dedicated servers.
This will safely bridge server logic to the client UI and fully resolve the compatibility issue.
This is intended - if both values flip to false it will disconnect. These methods are used to revive the config to a valid state.
@Rover656 I understand now - thanks for the clarification! The methods are serving their intended purpose by ensuring connections return to valid states rather than preserving previous configurations. Makes sense that disconnected connections would have both values set to false.
✏️ Learnings added
Almost everything you said here was wrong
@Rover656 You're right. Looking more closely at the code, I can see the capability caches are already properly handled - they're removed when conduits are removed, and the entire entity (with all caches) is unloaded when chunks unload. Sorry about the inaccurate suggestion.
Actionable comments posted: 5
♻️ Duplicate comments (17)
🧹 Nitpick comments (17)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 7d1bf03e24a43c72c05ab8410df14e022d055b46 and 48826cbada07dc827ec68d569cd98c7228eea868.
📒 Files selected for processing (100)
.github/workflows/verify.yml(1 hunks)endercore/build.gradle.kts(2 hunks)endercore/src/main/java/com/enderio/core/common/graph/BasicNetwork.java(1 hunks)endercore/src/main/java/com/enderio/core/common/graph/INetworkNode.java(1 hunks)endercore/src/main/java/com/enderio/core/common/graph/Network.java(1 hunks)endercore/src/main/java/com/enderio/core/common/graph/package-info.java(1 hunks)endercore/src/test/java/com/enderio/core/NetworkTests.java(1 hunks)endercore/src/test/java/com/enderio/core/TestNode.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/client/modules/mekanism/screens/ChemicalConduitScreenType.java(4 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/client/modules/mekanism/screens/HeatConduitScreenType.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/appeng/MEConduit.java(5 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalConduit.java(7 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalConduitConnectionConfig.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalConduitNetworkContext.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalTicker.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/heat/HeatConduitConnectionConfig.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/heat/HeatTicker.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/refinedstorage/RSConduit.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/refinedstorage/RSConduitNodeData.java(3 hunks)enderio-conduits/build.gradle.kts(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ColoredRedstoneProvider.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/Conduit.java(9 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/EnderIOConduitsRegistries.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/bundle/ConduitBundle.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/bundle/SlotType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/connection/config/IOConnectionConfig.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/model/ConduitModelModifier.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitBlockConnection.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetwork.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContext.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContextAccessor.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContextType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/IConduitNetwork.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/ConduitNode.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/IConduitNode.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/NodeData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/NodeDataAccessor.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/NodeDataType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitDataAccessor.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitDataType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/screen/IOConduitScreenType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/ConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/IOAwareConduitTicker.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/LoadedAwareConduitTicker.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/EnergyConduitScreenType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/FluidConduitScreenType.java(4 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/ItemConduitScreenType.java(3 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/RedstoneConduitScreenType.java(3 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/model/conduit/bundle/ConduitBundleModel.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/model/conduit/bundle/ConduitConnectionRenderState.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitBlockItem.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitSavedData.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/bundle/ConduitBundleBlock.java(3 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/bundle/ConduitBundleBlockEntity.java(27 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitConnectionHost.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphContext.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphObject.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphUtility.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/WrappedConduitNetwork.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/ConduitDataContainer.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/ConnectionState.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/DynamicConnectionState.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/LegacyFluidConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/LegacyItemConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/LegacyRedstoneConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/StaticConnectionStates.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/menu/ConduitMenu.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNetwork.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNetworkSavedData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNode.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/package-info.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduit.java(5 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitConnectionConfig.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitNetworkContext.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitStorage.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitTicker.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduit.java(6 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitConnectionConfig.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitNetworkContext.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduit.java(6 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduitConnectionConfig.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduit.java(4 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduitConnectionConfig.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduitNetworkContext.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/init/ConduitLang.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/integrations/cctweaked/EIOBundledRedstoneProvider.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/network/ConduitCommonPayloadHandler.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/EnderIOConduitsServer.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/BasicConduitTests.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/ConduitGameTestHelper.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/item/ItemConduitGameTestHelper.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/item/ItemConduitTests.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/item/package-info.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/package-info.java(1 hunks)enderio-machines/src/main/java/com/enderio/machines/common/travel/EnderfaceTravelTarget.java(1 hunks)enderio/build.gradle.kts(1 hunks)💤 Files with no reviewable changes (13)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (67)
🧰 Additional context used
🧬 Code Graph Analysis (3)
enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContext.java (1)
enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalTicker.java (1)
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNetwork.java (1)
🪛 YAMLlint (1.35.1)
.github/workflows/verify.yml
[error] 42-42: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (31)
@ -0,0 +511,4 @@if (shouldRebuildCache) {return;}💡 Verification agent
🧩 Analysis chain
Review null check and early return in onNodeAdded
The TODO comment suggests you need to review this behavior more thoroughly.
Consider refactoring to avoid the null check by ensuring the field is always initialized before this method is called, or document why this special case is needed.
🏁 Script executed:
Length of output: 1506
I’d like to inspect how nodesByChunkPos is declared, initialised, and used before onNodeAdded. Let’s pull in the class header, constructor(s) and onNodeAdded signature:
🏁 Script executed:
Length of output: 2433
Refactor onNodeAdded guard to avoid relying on a null-check
The
if (nodesByChunkPos == null)is only there to dodge an NPE when the superclass constructor calls your override before Java has run your field initialisers. Rather than leaving this as a “magic” null-check, either:• Introduce an explicit “construction complete” flag:
– Declare
private boolean constructed;– In each constructor, after
super(…)and once all fields are in place, setconstructed = true;– At the top of
onNodeAdded, replace the null-check withif (!constructed) return;• Or, better yet, remove the overridable call in the superclass constructor (if you can change
Network), and invoke your cache-rebuild logic only once the network is fully built.Either way, add a comment explaining that this guard exists because Java initialises instance fields only after the superclass constructor returns. This makes the intent explicit and removes the reliance on a null field.
Relevant location:
• enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNetwork.java: lines 506–514
@ -0,0 +126,4 @@if (network != bundle.getConduitNode(conduit).getNetwork()) {throw new GameTestAssertException("Conduit node networks differ at " + x + ", " + y + ", " + z);}⚠️ Potential issue
Possible
NullPointerExceptionwhen a bundle does not contain the requested conduitassertAllNetworksMatchassumes that every non-null bundle in the region owns the conduit type under test.When the assumption is violated,
bundle.getConduitNode(conduit)will throw, terminating the entire test with aNullPointerExceptioninstead of a descriptive assertion failure.📝 Committable suggestion
@ -0,0 +64,4 @@// Put some dirt in the chest we'll extract from.thenExecute(() -> helper.insertIntoContainer(0, 1, 0, Items.DIRT, 1))// Ensure the item is still there.thenExecuteAfter(tickRate, () -> helper.assertContainerHasExactly(0, 1, 0, Items.DIRT, 1))⚠️ Potential issue
Chest y-coordinate off by one.
In
itemConduitBasicTransfertest, the assertion for the source chest also uses incorrect y-coordinate.📝 Committable suggestion
@ -0,0 +72,4 @@.withIsInsert(false).withExtractRedstoneControl(RedstoneControl.ALWAYS_ACTIVE)))// Ensure the item moves.thenExecuteAfter(tickRate, () -> helper.assertContainerHasExactly(0, 1, 2, Items.DIRT, 1))⚠️ Potential issue
Chest y-coordinate off by one.
The insertion result assertion here also uses wrong y-coordinate.
📝 Committable suggestion
@ -0,0 +103,4 @@.withIsInsert(false).withExtractRedstoneControl(RedstoneControl.ALWAYS_ACTIVE)))// Put some dirt in the chest we'll extract from.thenExecute(() -> helper.insertIntoContainer(2, 1, 0, Items.DIRT, 1))⚠️ Potential issue
Chest y-coordinate off by one.
The insertion to the container uses wrong y-coordinate in this test as well.
📝 Committable suggestion
Actionable comments posted: 1
🔭 Outside diff range comments (2)
♻️ Duplicate comments (2)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 48826cbada07dc827ec68d569cd98c7228eea868 and 69f36cb2807f8d026cbcaddd63a7e686a1832595.
📒 Files selected for processing (5)
enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalTicker.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/heat/HeatTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitTicker.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduitTicker.java(1 hunks)🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (7)
@ -29,0 +33,4 @@IFluidHandler extractHandler = extractConnection.getSidedCapability(Capabilities.FluidHandler.BLOCK);if (extractHandler == null) {continue;🛠️ Refactor suggestion
Check for potential integer overflow.
The multiplication of two integers without explicit type conversion could potentially overflow if both values are large.
📝 Committable suggestion
Actionable comments posted: 1
🔭 Outside diff range comments (2)
♻️ Duplicate comments (17)
🧹 Nitpick comments (17)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 69f36cb2807f8d026cbcaddd63a7e686a1832595 and
1563c55d06.📒 Files selected for processing (100)
.github/workflows/verify.yml(1 hunks)endercore/build.gradle.kts(2 hunks)endercore/src/main/java/com/enderio/core/common/graph/BasicNetwork.java(1 hunks)endercore/src/main/java/com/enderio/core/common/graph/INetworkNode.java(1 hunks)endercore/src/main/java/com/enderio/core/common/graph/Network.java(1 hunks)endercore/src/main/java/com/enderio/core/common/graph/package-info.java(1 hunks)endercore/src/test/java/com/enderio/core/NetworkTests.java(1 hunks)endercore/src/test/java/com/enderio/core/TestNode.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/client/modules/mekanism/screens/ChemicalConduitScreenType.java(4 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/client/modules/mekanism/screens/HeatConduitScreenType.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/appeng/MEConduit.java(5 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalConduit.java(7 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalConduitConnectionConfig.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalConduitNetworkContext.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/chemical/ChemicalTicker.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/heat/HeatConduitConnectionConfig.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/mekanism/heat/HeatTicker.java(1 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/refinedstorage/RSConduit.java(2 hunks)enderio-conduits-modded/src/main/java/com/enderio/modconduits/common/modules/refinedstorage/RSConduitNodeData.java(3 hunks)enderio-conduits/build.gradle.kts(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ColoredRedstoneProvider.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/Conduit.java(9 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/EnderIOConduitsRegistries.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/bundle/ConduitBundle.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/bundle/SlotType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/connection/config/IOConnectionConfig.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/model/ConduitModelModifier.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitBlockConnection.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetwork.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContext.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContextAccessor.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/ConduitNetworkContextType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/IConduitNetwork.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/ConduitNode.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/IConduitNode.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/NodeData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/NodeDataAccessor.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/NodeDataType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitDataAccessor.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitDataType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/screen/IOConduitScreenType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/ConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/IOAwareConduitTicker.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/LoadedAwareConduitTicker.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/EnergyConduitScreenType.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/FluidConduitScreenType.java(4 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/ItemConduitScreenType.java(3 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/RedstoneConduitScreenType.java(3 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/model/conduit/bundle/ConduitBundleModel.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/client/model/conduit/bundle/ConduitConnectionRenderState.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitBlockItem.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitSavedData.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/bundle/ConduitBundleBlock.java(3 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/bundle/ConduitBundleBlockEntity.java(27 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitConnectionHost.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphContext.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphObject.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphUtility.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/WrappedConduitNetwork.java(0 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/ConduitDataContainer.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/ConnectionState.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/DynamicConnectionState.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/LegacyFluidConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/LegacyItemConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/LegacyRedstoneConduitData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/StaticConnectionStates.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/menu/ConduitMenu.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNetwork.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNetworkSavedData.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNode.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/package-info.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduit.java(5 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitConnectionConfig.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitNetworkContext.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitStorage.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/energy/EnergyConduitTicker.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduit.java(6 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitConnectionConfig.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitNetworkContext.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduit.java(6 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduitConnectionConfig.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduit.java(4 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduitConnectionConfig.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduitNetworkContext.java(2 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduitTicker.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/init/ConduitLang.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/integrations/cctweaked/EIOBundledRedstoneProvider.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/common/network/ConduitCommonPayloadHandler.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/EnderIOConduitsServer.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/BasicConduitTests.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/ConduitGameTestHelper.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/item/ItemConduitGameTestHelper.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/item/ItemConduitTests.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/item/package-info.java(1 hunks)enderio-conduits/src/main/java/com/enderio/conduits/server/tests/package-info.java(1 hunks)enderio-machines/src/main/java/com/enderio/machines/common/travel/EnderfaceTravelTarget.java(1 hunks)enderio/build.gradle.kts(1 hunks)💤 Files with no reviewable changes (13)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (71)
🧰 Additional context used
🧬 Code Graph Analysis (1)
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNetwork.java (1)
🪛 YAMLlint (1.35.1)
.github/workflows/verify.yml
[error] 43-43: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (29)
@ -0,0 +1,302 @@package com.enderio.conduits.common.conduit.network;⚠️ Potential issue
Fix potential IllegalStateException in markDirty()
The current implementation calls
ensureValid()which throws when the chunk is not loaded, but then still accessesconduitBundleeven whenisLoaded()is false.📝 Committable suggestion
Actionable comments posted: 1
♻️ Duplicate comments (1)
🧹 Nitpick comments (4)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between
1563c55d06andb641d7636e.📒 Files selected for processing (1)
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/network/ConduitNode.java(1 hunks)⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (4)
@ -0,0 +95,4 @@this.legacyDataContainer = legacyDataContainer;this.nodeData = oldData.toNodeData();}}💡 Verification agent
🧩 Analysis chain
Ensure constructors properly initialize all fields
The constructors handling legacy data conversion are well structured, but consider adding documentation to clarify the initialization flow, especially regarding when
networkis created versus when it's expected to be set later.🏁 Script executed:
Length of output: 13646
Clarify ConduitNode constructor initialisation behaviour
Please add JavaDoc (or inline comments) to each
ConduitNodeconstructor to make it explicit when thenetworkfield is initialised versus when it’s left null for later setup:public ConduitNode(Holder<Conduit<?, ?>> conduit, BlockPos pos, @Nullable NodeData nodeData)note that
this.network = new ConduitNetwork(conduit, this)is performed immediately.public ConduitNode(Holder<Conduit<?, ?>> conduit, BlockPos pos, ConduitDataContainer legacyDataContainer)explain that it delegates to the above constructor, then extracts legacy data before the network exists.
(BlockPos pos, Optional<NodeData>)and(BlockPos pos, ConduitDataContainer)document that they do not create a network (it will be set later by
ConduitNetworkSavedData).No functional changes are necessary beyond improving the constructor documentation.