[8.0] Conduit block rewrite #1025

2025-03-07T20:21:32Z

Rover656 commented

2025-03-07 20:21:32 +00:00

(Migrated from github.com)

Description

Replaces #958 which had a nasty commit history and became difficult to rebase.

TODO

Add the ability to open filter GUIs from the conduit screen (and go back).
Address any newly introduced TODOs.
Many other things, I'm very busy so will keep updating this list as I trawl through.

Breaking Changes

List any breaking changes in this section, such as: changed/removed APIs, changed or removed items/blocks or modifications to recipes and gameplay mechanics.

Major API breaks for mods that add or interact with conduits.
Has supporting logic for loading existing conduit bundles, but it is not saved in a backward-compatible format (i.e. 7.2 cannot be safely downgraded from)

Checklist

My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you).
I have made corresponding changes to the documentation.
My changes are ready for review from a contributor.

Summary by CodeRabbit

New Features
- Introduced advanced conduit API for improved flexibility and extensibility, including new interfaces for bundles, connections, and screen types.
- Added new GUI components for conduits, such as dynamic tooltips, enhanced button widgets, and improved screen layout management.
- Implemented refined filter APIs for items, fluids, and entities, supporting more granular filtering and menu interactions.
- Enhanced visual effects for conduit bundles, including custom hit and destruction particles.
Improvements
- Refined enum and method naming for clarity and consistency across the API.
- Improved tooltip localisation and display for dye colours and toggle buttons.
- Streamlined conduit registration, network, and data management, with new registries and deprecation of legacy systems.
- Updated redstone control and sensitivity handling for more precise behaviour.
Bug Fixes
- Fixed tooltip synchronisation issues on toggle buttons.
- Improved tooltip suppression logic for overlays in GUIs.
Removals/Deprecations
- Removed legacy and redundant interfaces/classes, including old filter abstractions and deprecated conduit data structures.
- Deprecated or removed obsolete registry keys and data types.
Refactor
- Refactored package structure for better modularity and maintainability.
- Simplified slot creation in menus to enable easier customisation.
Chores
- Updated build configuration and dependency management for enhanced compatibility and formatting.
Style
- Improved code formatting and readability in various components and language files.
Documentation
- Enhanced documentation and annotations for experimental and deprecated features.

# Description Replaces #958 which had a nasty commit history and became difficult to rebase.  # TODO - [x] Add the ability to open filter GUIs from the conduit screen (and go back). - [x] Address any newly introduced TODOs. - [x] Many other things, I'm very busy so will keep updating this list as I trawl through. # Breaking Changes List any breaking changes in this section, such as: changed/removed APIs, changed or removed items/blocks or modifications to recipes and gameplay mechanics. Major API breaks for mods that add or interact with conduits. Has supporting logic for loading existing conduit bundles, but it is not saved in a backward-compatible format (i.e. 7.2 cannot be safely downgraded from)  # Checklist - [ ] My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you). - [ ] I have made corresponding changes to the documentation. - [ ] My changes are ready for review from a contributor.   ## Summary by CodeRabbit - **New Features** - Introduced advanced conduit API for improved flexibility and extensibility, including new interfaces for bundles, connections, and screen types. - Added new GUI components for conduits, such as dynamic tooltips, enhanced button widgets, and improved screen layout management. - Implemented refined filter APIs for items, fluids, and entities, supporting more granular filtering and menu interactions. - Enhanced visual effects for conduit bundles, including custom hit and destruction particles. - **Improvements** - Refined enum and method naming for clarity and consistency across the API. - Improved tooltip localisation and display for dye colours and toggle buttons. - Streamlined conduit registration, network, and data management, with new registries and deprecation of legacy systems. - Updated redstone control and sensitivity handling for more precise behaviour. - **Bug Fixes** - Fixed tooltip synchronisation issues on toggle buttons. - Improved tooltip suppression logic for overlays in GUIs. - **Removals/Deprecations** - Removed legacy and redundant interfaces/classes, including old filter abstractions and deprecated conduit data structures. - Deprecated or removed obsolete registry keys and data types. - **Refactor** - Refactored package structure for better modularity and maintainability. - Simplified slot creation in menus to enable easier customisation. - **Chores** - Updated build configuration and dependency management for enhanced compatibility and formatting. - **Style** - Improved code formatting and readability in various components and language files. - **Documentation** - Enhanced documentation and annotations for experimental and deprecated features.

👍 1

Rover656 commented

2025-03-11 00:22:23 +00:00

(Migrated from github.com)

@coderabbitai summary

coderabbitai[bot] commented

2025-03-11 00:22:28 +00:00

(Migrated from github.com)

✅ Actions performed

Summary regeneration triggered.

<details> <summary>:white_check_mark: Actions performed</summary> Summary regeneration triggered. </details>

coderabbitai[bot] commented

2025-03-11 00:22:33 +00:00

(Migrated from github.com)

Walkthrough

This update introduces a substantial refactor and expansion of the conduit API and client systems. It adds new interfaces, records, enums, and classes for conduit bundles, connections, menus, models, and tickers, while deprecating or removing legacy types. Numerous package reorganisations and method signature changes improve type flexibility and modularity. The client setup is updated to register new model modifiers and screen types, and visual/particle effects for conduit bundles are enhanced. Several filtering interfaces are replaced or consolidated, and new filter APIs are introduced for items, fluids, and entities. Miscellaneous improvements include tooltip handling, slot creation customisation, and dependency updates.

Changes

File(s) / Path(s)	Change Summary
`enderio-conduits/api/`, `enderio-conduits/api/bundle/`, `enderio-conduits/api/connection/`, `enderio-conduits/api/network/`, `enderio-conduits/api/model/`, `enderio-conduits/api/screen/`, `enderio-conduits/api/ticker/`	Major API refactor: new interfaces, records, enums, and classes for conduit bundles, connections, node data, menus, models, and tickers; deprecation/removal of legacy types; package reorganisations; enhanced type parameters and modularity.
`enderio-conduits/client/`	Added `ConduitBundleExtension` for particle effects; updated `ConduitClientSetup` to register new model modifiers, screen types, and client extensions; import and registration logic updated.
`enderio-conduits/common/conduit/block/` → `common/conduit/bundle/`	Refactored imports to new package structure for conduit bundle block entities.
`enderio-conduits-modded/src/main/java/com/enderio/modconduits/mods/mekanism/`, `enderio-conduits-modded/src/main/java/com/enderio/modconduits/mods/refinedstorage/`	Updated imports and method signatures to match new conduit API and package structure.
`enderio-base/api/filter/`	Introduced new filter interfaces: `ItemFilter`, `FluidFilter`, `FilterMenuProvider`; removed legacy interfaces (`ResourceFilter`, `ItemStackFilter`, `FluidStackFilter`); refined `EntityFilter` methods.
`enderio-base/api/misc/RedstoneControl.java`	Added `isRedstoneSensitive` property to enum, updated constants and constructor, added getter.
`enderio-base/client/gui/widget/DyeColorPickerWidget.java`, `enderio-base/common/lang/EIOEnumLang.java`	Improved dye colour tooltip support and added translation map for dye colours.
`enderio-base/common/lang/EIOLang.java`	Formatting changes and removal of unused component declarations.
`enderio-base/common/blockentity/Wrenchable.java`, `enderio-base/common/item/tool/YetaWrenchItem.java`	Refactored wrench interaction to use `UseOnContext`; removed legacy method usage.
`endercore/client/gui/widgets/`	Added new `IconButton` class; removed legacy `ToggleImageButton`; improved tooltip and toggle logic in related widgets.
`endercore/common/menu/BaseEnderMenu.java`	Refactored slot creation to use overridable factory methods for inventory and armour slots.
`endercore/common/util/TooltipUtil.java`	Changed return type of `style` method to `MutableComponent`.
`endercore/common/blockentity/EnderBlockEntity.java`	Removed deferred change notification logic and simplified capability cache logic.
`build.gradle.kts`, `enderio-conduits/build.gradle.kts`	Updated Spotless/Cleanthat plugin version; added Jade dependency by property.
Miscellaneous (various files)	Parameter renaming for clarity, import updates, formatting, and minor logic improvements.

Sequence Diagram(s)

sequenceDiagram
    participant Game as Game Engine
    participant Bundle as ConduitBundle
    participant Conduit as Conduit
    participant Node as ConduitNode
    participant Menu as ConduitMenuDataAccess
    participant Screen as ConduitScreenType

    Game->>Bundle: Add/Remove conduit
    Bundle->>Conduit: Validate addition/removal
    Bundle->>Node: Update node data/config
    Node->>Conduit: Query connection/config
    Game->>Menu: Open conduit menu
    Menu->>Screen: Provide config and extra data
    Screen->>Menu: Update config via modifier

Team-EnderIO/EnderIO#1035: Updates to BaseEnumPickerWidget for scrolling and ordinal usage, related to tooltip and value management changes in this PR.

Suggested labels

P-0-High

Poem

In tunnels deep where conduits run,
The rabbits code—refactor begun!
New bundles, nodes, and menus bright,
Models, filters, widgets—what a sight!
With particles and tooltips too,
This warren’s code feels fresh and new.
🐇✨

✨ Finishing Touches

📝 Generate Docstrings

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:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- 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.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in 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 pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

## Walkthrough This update introduces a substantial refactor and expansion of the conduit API and client systems. It adds new interfaces, records, enums, and classes for conduit bundles, connections, menus, models, and tickers, while deprecating or removing legacy types. Numerous package reorganisations and method signature changes improve type flexibility and modularity. The client setup is updated to register new model modifiers and screen types, and visual/particle effects for conduit bundles are enhanced. Several filtering interfaces are replaced or consolidated, and new filter APIs are introduced for items, fluids, and entities. Miscellaneous improvements include tooltip handling, slot creation customisation, and dependency updates. ## Changes | File(s) / Path(s) | Change Summary | |-------------------------------------------------------------------------------------------------------------|---------------| | `enderio-conduits/api/`, `enderio-conduits/api/bundle/`, `enderio-conduits/api/connection/`, `enderio-conduits/api/network/`, `enderio-conduits/api/model/`, `enderio-conduits/api/screen/`, `enderio-conduits/api/ticker/` | Major API refactor: new interfaces, records, enums, and classes for conduit bundles, connections, node data, menus, models, and tickers; deprecation/removal of legacy types; package reorganisations; enhanced type parameters and modularity. | | `enderio-conduits/client/` | Added `ConduitBundleExtension` for particle effects; updated `ConduitClientSetup` to register new model modifiers, screen types, and client extensions; import and registration logic updated. | | `enderio-conduits/common/conduit/block/` → `common/conduit/bundle/` | Refactored imports to new package structure for conduit bundle block entities. | | `enderio-conduits-modded/src/main/java/com/enderio/modconduits/mods/mekanism/`, `enderio-conduits-modded/src/main/java/com/enderio/modconduits/mods/refinedstorage/` | Updated imports and method signatures to match new conduit API and package structure. | | `enderio-base/api/filter/` | Introduced new filter interfaces: `ItemFilter`, `FluidFilter`, `FilterMenuProvider`; removed legacy interfaces (`ResourceFilter`, `ItemStackFilter`, `FluidStackFilter`); refined `EntityFilter` methods. | | `enderio-base/api/misc/RedstoneControl.java` | Added `isRedstoneSensitive` property to enum, updated constants and constructor, added getter. | | `enderio-base/client/gui/widget/DyeColorPickerWidget.java`, `enderio-base/common/lang/EIOEnumLang.java` | Improved dye colour tooltip support and added translation map for dye colours. | | `enderio-base/common/lang/EIOLang.java` | Formatting changes and removal of unused component declarations. | | `enderio-base/common/blockentity/Wrenchable.java`, `enderio-base/common/item/tool/YetaWrenchItem.java` | Refactored wrench interaction to use `UseOnContext`; removed legacy method usage. | | `endercore/client/gui/widgets/` | Added new `IconButton` class; removed legacy `ToggleImageButton`; improved tooltip and toggle logic in related widgets. | | `endercore/common/menu/BaseEnderMenu.java` | Refactored slot creation to use overridable factory methods for inventory and armour slots. | | `endercore/common/util/TooltipUtil.java` | Changed return type of `style` method to `MutableComponent`. | | `endercore/common/blockentity/EnderBlockEntity.java` | Removed deferred change notification logic and simplified capability cache logic. | | `build.gradle.kts`, `enderio-conduits/build.gradle.kts` | Updated Spotless/Cleanthat plugin version; added Jade dependency by property. | | Miscellaneous (various files) | Parameter renaming for clarity, import updates, formatting, and minor logic improvements. | ## Sequence Diagram(s) ```mermaid sequenceDiagram participant Game as Game Engine participant Bundle as ConduitBundle participant Conduit as Conduit participant Node as ConduitNode participant Menu as ConduitMenuDataAccess participant Screen as ConduitScreenType Game->>Bundle: Add/Remove conduit Bundle->>Conduit: Validate addition/removal Bundle->>Node: Update node data/config Node->>Conduit: Query connection/config Game->>Menu: Open conduit menu Menu->>Screen: Provide config and extra data Screen->>Menu: Update config via modifier ``` ## Possibly related PRs - [Team-EnderIO/EnderIO#1035](https://github.com/Team-EnderIO/EnderIO/pull/1035): Updates to `BaseEnumPickerWidget` for scrolling and ordinal usage, related to tooltip and value management changes in this PR. ## Suggested labels `P-0-High` ## Poem > In tunnels deep where conduits run, > The rabbits code—refactor begun! > New bundles, nodes, and menus bright, > Models, filters, widgets—what a sight! > With particles and tooltips too, > This warren’s code feels fresh and new. > 🐇✨      <details open="true"> <summary>✨ Finishing Touches</summary> - [ ]  📝 Generate Docstrings </details>   --- 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. <details> <summary>❤️ Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> <details> <summary>🪧 Tips</summary> ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=Team-EnderIO/EnderIO&utm_content=1025): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `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.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in 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](https://www.coderabbit.ai/contact-us/support) 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 pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai generate docstrings` to [generate docstrings](https://docs.coderabbit.ai/finishing-touches/docstrings) for this PR. - `@coderabbitai generate sequence diagram` to generate a sequence diagram of the changes in this PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://docs.coderabbit.ai) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements. </details>

Rover656 commented

2025-03-11 00:40:44 +00:00

(Migrated from github.com)

@coderabbitai review

coderabbitai[bot] commented

2025-03-11 00:40:50 +00:00

(Migrated from github.com)

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

<details> <summary>:white_check_mark: Actions performed</summary> Review triggered. > Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused. </details>

coderabbitai[bot] (Migrated from github.com) reviewed

2025-03-11 01:02:09 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

🛑 Comments failed to post (28)

enderio-conduits/src/main/java/com/enderio/conduits/integration/jade/package-info.java (1)

4-4: 🛠️ Refactor suggestion

Updated Package Declaration

The package declaration has been adjusted to reflect the new organisational structure, moving the file into com.enderio.conduits.integration.jade as intended by this refactor. Please ensure that all related modules, documentation, and references are updated accordingly to maintain consistency across the codebase.

enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduitNodeData.java (1)

18-22: 🛠️ Refactor suggestion

Consider limiting or validating the map entries.

By using Codec.unboundedMap(Direction.CODEC, Codec.INT), there is no explicit upper bound on map size. Realistically, only 6 entries are needed (one per Direction). You might consider using an enum-based map (e.g. EnumMap<Direction, Integer>) or validating the map during decoding to ensure data doesn't exceed expected sizes.
enderio-conduits/src/main/java/com/enderio/conduits/client/model/conduit/modifier/RedstoneConduitModelModifier.java (1)
14-14: 🛠️ Refactor suggestion

Add type checking before casting.

This direct cast assumes the conduit is a RedstoneConduit type. If a different conduit type is passed, this would throw a ClassCastException. Consider adding a type check before casting:
- RedstoneConduit redstoneConduit = (RedstoneConduit) conduit.value();
+ if (!(conduit.value() instanceof RedstoneConduit)) {
+ return null; // Or a default texture
+ }
+ RedstoneConduit redstoneConduit = (RedstoneConduit) conduit.value();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 if (!(conduit.value() instanceof RedstoneConduit)) {
 return null; // Or a default texture
 }
 RedstoneConduit redstoneConduit = (RedstoneConduit) conduit.value();
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphContext.java (2)

109-113: 🛠️ Refactor suggestion

Review: Encoding Context via Codec

The encodeContext method utilises encodeStart(...).getOrThrow() to perform serialization. While this approach ensures immediate feedback by throwing on error, it might be beneficial to consider more graceful error handling in future iterations, especially if codec failures are expected to occur under normal operation.

131-146: 🛠️ Refactor suggestion

Review: Robustness in loadNetworkContext Parsing

The private static loadNetworkContext method has been refactored to use the wildcard type for Conduit<?, ?> and includes critical null checks via Objects.requireNonNull. However, the repeated use of getOrThrow() (also noted in the TODO comment) indicates that error handling could be made more robust and flexible. Consider revisiting this approach to gracefully manage potential codec parsing failures.
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitConnectionHost.java (2)
12-12: ⚠️ Potential issue

Address TODO comment before finalising PR

This TODO comment indicates uncertainty about whether this should be an interface or a concrete class. As interfaces are part of your public API contract (even if not marked as public API), changing this later could be disruptive. Consider resolving this architectural decision before merging.

20-22: 💡 Verification agent
🧩 Analysis chain

Verify error handling for connection configuration operations

The interface doesn't specify how errors should be handled when getting or setting connection configurations. Consider documenting what happens with invalid directions or null configuration values. Should implementers throw exceptions or handle errors gracefully?

🏁 Script executed:
#!/bin/bash
# Search for implementations of these methods to understand how errors are handled

echo "Searching for implementations of getConnectionConfig and setConnectionConfig..."
rg -A 5 -B 5 "getConnectionConfig\(Direction" --type java
rg -A 5 -B 5 "setConnectionConfig\(Direction" --type java
Length of output: 14470
Update: Document Explicit Exception Contracts for Connection Configuration Methods

The investigation shows that implementations consistently handle error conditions by throwing exceptions (e.g. an IllegalStateException when the connection host is missing, or an IllegalArgumentException as documented in the API). However, the interface in ConduitConnectionHost.java does not explicitly specify the expected behaviour for invalid directions or null configuration values.

Please update the method documentation (Javadoc) to explicitly state:

That an IllegalArgumentException (or the appropriate exception) will be thrown if an invalid direction is provided.

What behaviour is expected if a null configuration value is passed to setConnectionConfig.

This clearer contract will help implementers ensure consistent error handling and improve the API’s usability.
enderio-conduits/src/main/java/com/enderio/conduits/api/screen/RegisterConduitScreenTypesEvent.java (1)
13-15: 💡 Verification agent
🧩 Analysis chain

Consider handling duplicate registrations.

The current implementation silently overwrites existing screen types for a conduit type. Depending on the intended behavior, you might want to log a warning or throw an exception when a duplicate registration occurs.

🏁 Script executed:
#!/bin/bash
# Check if there are any existing usages of this event that might be affected by this behavior
rg -m 5 -A 3 "RegisterConduitScreenTypesEvent" --type java
Length of output: 3204
Duplicate Registration Handling Required

The current implementation of the register method in
enderio-conduits/src/main/java/com/enderio/conduits/api/screen/RegisterConduitScreenTypesEvent.java
silently overwrites any prior registration for a given conduit type. Given that the event is subsequently used in methods such as the one in ConduitClientSetup.java to register screen types, it would be prudent to explicitly handle duplicate registrations. For example, you might want to add a check to verify if a registration already exists and then log a warning or throw an exception before overwriting.

A suggested approach:
public void register(ConduitType<? extends Conduit<?, ?>> conduitType, ConduitScreenType<?> screenType) {
 if(screenTypes.containsKey(conduitType)) {
 // Optionally log a warning or throw an exception to flag duplicate registration.
 // For instance:
 // throw new IllegalArgumentException("Duplicate registration for conduit type: " + conduitType);
 // Or alternatively:
 // Logger.warn("Duplicate registration for conduit type: " + conduitType + ". Overwriting existing screen type.");
 }
 screenTypes.put(conduitType, screenType);
}
This change would ensure that accidental duplicate registrations do not lead to silent overwrites and make debugging registration issues easier.
enderio-conduits/src/main/java/com/enderio/conduits/client/model/conduit/bundle/ConduitBundleRenderState.java (2)
92-94: ⚠️ Potential issue

Potential null pointer exception in isConnectionEndpoint

This method assumes that conduitConnections.get(side) will never return null, but there's no guarantee of this. Consider adding a null check or using getOrDefault() as done in the getConnectedConduits method.
public boolean isConnectionEndpoint(Direction side) {
- return !conduitConnections.get(side).isEmpty();
+ var connections = conduitConnections.get(side);
+ return connections != null && !connections.isEmpty();
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 public boolean isConnectionEndpoint(Direction side) {
 var connections = conduitConnections.get(side);
 return connections != null && !connections.isEmpty();
 }
96-98: ⚠️ Potential issue

Missing null checks in getConnectionState

Similar to the issue above, this method may throw NullPointerExceptions if either conduitConnections.get(side) or the inner map's get returns null.
public ConduitConnectionRenderState getConnectionState(Direction side, Holder<Conduit<?, ?>> conduit) {
- return conduitConnections.get(side).get(conduit);
+ Map<Holder<Conduit<?, ?>>, ConduitConnectionRenderState> sideConnections = conduitConnections.get(side);
+ return sideConnections != null ? sideConnections.get(conduit) : null;
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 public ConduitConnectionRenderState getConnectionState(Direction side, Holder<Conduit<?, ?>> conduit) {
 Map<Holder<Conduit<?, ?>>, ConduitConnectionRenderState> sideConnections = conduitConnections.get(side);
 return sideConnections != null ? sideConnections.get(conduit) : null;
 }
enderio-conduits/src/main/java/com/enderio/conduits/client/gui/ConduitSelectionButton.java (1)

47-48: 🛠️ Refactor suggestion

Reference check vs. logical equality.
Using conduit != currentConduit.get() is a valid reference check, but consider whether a logical comparison (e.g., .equals()) might be more appropriate if multiple Holder<Conduit<?, ?>> instances could refer to the same underlying conduit.

enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitData.java (1)

24-25: 🛠️ Refactor suggestion

Revisit parameter usage in withClientChanges.
This method currently ignores the guiData parameter, raising doubts about its relevance. Consider removing the parameter if truly unneeded, or implement the intended logic if appropriate.
enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/IOAwareConduitTicker.java (2)
44-56: 🛠️ Refactor suggestion

Avoid creating duplicate connection objects for send/receive.

Calling createConnection(...) twice for the same node and side is wasteful if both canSend(...) and canReceive(...) return true. Refactor to create a single connection object, then add it to both sender and receiver lists if needed.
- if (canSend(node, config)) {
- var connection = createConnection(level, node, side);
- if (connection != null) {
- senders.get(config.sendColor()).add(connection);
- }
- }
-
- if (canReceive(node, config)) {
- var connection = createConnection(level, node, side);
- if (connection != null) {
- receivers.get(config.receiveColor()).add(connection);
- }
- }
+ var canSend = canSend(node, config);
+ var canReceive = canReceive(node, config);
+ if (canSend || canReceive) {
+ var connection = createConnection(level, node, side);
+ if (connection != null) {
+ if (canSend) {
+ senders.get(config.sendColor()).add(connection);
+ }
+ if (canReceive) {
+ receivers.get(config.receiveColor()).add(connection);
+ }
+ }
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 var canSend = canSend(node, config);
 var canReceive = canReceive(node, config);
 if (canSend || canReceive) {
 var connection = createConnection(level, node, side);
 if (connection != null) {
 if (canSend) {
 senders.get(config.sendColor()).add(connection);
 }
 if (canReceive) {
 receivers.get(config.receiveColor()).add(connection);
 }
 }
 }
129-130: 💡 Verification agent
🧩 Analysis chain

Check for a potentially null inventory reference.

node.getInventory(side) could potentially return null, causing a NullPointerException if callers use inventory() without checking. Confirm that this is safe or add a null check to avoid unexpected runtime errors.

🏁 Script executed:
#!/bin/bash
# Description: Search the codebase for usage of inventory() to confirm if null checks are in place.
fd -e java --exec rg --context 5 'inventory'
Length of output: 2774
Attention: Potential Null-Reference Risk in Inventory Access

The method implementation for inventory() simply returns node.getInventory(side) without any null-check. Our codebase usage—such as in:

var inventory = receiver.inventory();

Subsequent calls like inventory.getStackInSlot(...)

—suggests that callers assume the return value will never be null. This could be risky if node.getInventory(side) can indeed return null, as it would lead to a NullPointerException at runtime.

Please either:

Confirm that node.getInventory(side) is guaranteed never to return null (for example, via a non-null invariant or appropriate annotations/documentation), or

Add an explicit null-check within the inventory() method (or at all call sites) to handle a potential null value safely.
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/DynamicConnectionState.java (1)
53-63: 🛠️ Refactor suggestion

Handle the STUB or remove the redundant code
The provided parameters (level, pos, direction, type) are not used, and the method simply returns a fixed DynamicConnectionState. Consider implementing the intended logic or removing this STUB.
- // STUB
- return new DynamicConnectionState(false, DyeColor.GREEN, true, DyeColor.GREEN, RedstoneControl.NEVER_ACTIVE,
- DyeColor.RED, ItemStack.EMPTY, ItemStack.EMPTY, ItemStack.EMPTY);
+Conduit.ConduitConnectionData defaultConnection = type.value().getDefaultConnection(level, pos, direction);
+return new DynamicConnectionState(
+ defaultConnection.isInsert(),
+ DyeColor.GREEN,
+ defaultConnection.isExtract(),
+ DyeColor.GREEN,
+ defaultConnection.control(),
+ DyeColor.RED,
+ ItemStack.EMPTY,
+ ItemStack.EMPTY,
+ ItemStack.EMPTY
+);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 Holder<Conduit<?, ?>> type) {
 Conduit.ConduitConnectionData defaultConnection = type.value().getDefaultConnection(level, pos, direction);
 return new DynamicConnectionState(
 defaultConnection.isInsert(),
 DyeColor.GREEN,
 defaultConnection.isExtract(),
 DyeColor.GREEN,
 defaultConnection.control(),
 DyeColor.RED,
 ItemStack.EMPTY,
 ItemStack.EMPTY,
 ItemStack.EMPTY
 );
 }
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduit.java (1)
109-128: ⚠️ Potential issue

Safeguard against null legacy side config.
When retrieving legacy data, there's a risk oldSideConfig might be null if the map lacks entries for certain sides. Dereferencing it could result in a null pointer exception.

To address this, consider a null check as shown below:
- var oldSideConfig = legacyData.get(side);
- node.setConnectionConfig(side,
- currentConfig.withIsRoundRobin(oldSideConfig.isRoundRobin)
- .withIsSelfFeed(oldSideConfig.isSelfFeed)
- .withPriority(oldSideConfig.priority));
+ var oldSideConfig = legacyData.get(side);
+ if (oldSideConfig != null) {
+ node.setConnectionConfig(side,
+ currentConfig.withIsRoundRobin(oldSideConfig.isRoundRobin)
+ .withIsSelfFeed(oldSideConfig.isSelfFeed)
+ .withPriority(oldSideConfig.priority));
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 @Override
 public void copyLegacyData(ConduitNode node, ConduitDataAccessor legacyDataAccessor) {
 var legacyData = legacyDataAccessor.getData(ConduitTypes.Data.ITEM.get());
 if (legacyData == null) {
 return;
 }

 // Copy connection config
 for (Direction side : Direction.values()) {
 if (node.isConnectedTo(side)) {
 var oldSideConfig = legacyData.get(side);
 var currentConfig = node.getConnectionConfig(side, ItemConduitConnectionConfig.TYPE);

 if (oldSideConfig != null) {
 node.setConnectionConfig(side,
 currentConfig.withIsRoundRobin(oldSideConfig.isRoundRobin)
 .withIsSelfFeed(oldSideConfig.isSelfFeed)
 .withPriority(oldSideConfig.priority));
 }
 }
 }
 }
enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/FluidConduitScreenType.java (2)
111-122: 🛠️ Refactor suggestion

Handle invalid resource locations when retrieving locked fluid.
A malformed or missing fluid identifier could throw an exception.

Consider adding a try/catch block:
- return BuiltInRegistries.FLUID.get(ResourceLocation.parse(tag.getString("LockedFluid")));
+ try {
+ return BuiltInRegistries.FLUID.get(ResourceLocation.parse(tag.getString("LockedFluid")));
+ } catch (Exception e) {
+ // Log and gracefully handle invalid fluid name
+ return Fluids.EMPTY;
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 private Fluid getLockedFluid(ConduitMenuDataAccess<FluidConduitConnectionConfig> dataAccess) {
 var tag = dataAccess.getExtraGuiData();
 if (tag == null) {
 return Fluids.EMPTY;
 }

 if (!tag.contains("LockedFluid")) {
 return Fluids.EMPTY;
 }

 try {
 return BuiltInRegistries.FLUID.get(ResourceLocation.parse(tag.getString("LockedFluid")));
 } catch (Exception e) {
 // Log and gracefully handle invalid fluid name
 return Fluids.EMPTY;
 }
 }
124-193: 🛠️ Refactor suggestion

Confirm fluid supplier reliability.
Ensure currentFluid.get() is never null. If null is possible, add a safeguard check to prevent potential NullPointerExceptions.

Example adjustment:
- if (!currentFluid.get().isSame(Fluids.EMPTY)) {
+ var fluid = currentFluid.get();
+ if (fluid != null && !fluid.isSame(Fluids.EMPTY)) {
+ // ...
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 private static class FluidWidget extends AbstractWidget {
 private static final ResourceLocation WIDGET_TEXTURE = EnderIO.loc("textures/gui/fluidbackground.png");

 private final Runnable onPress;
 private final Supplier<Fluid> currentFluid;

 FluidWidget(int x, int y, Supplier<Fluid> fluid, Runnable onPress) {
 super(x, y, 14, 14, Component.empty());
 this.onPress = onPress;
 this.currentFluid = fluid;
 }

 @Override
 public void updateWidgetNarration(NarrationElementOutput pNarrationElementOutput) {
 }

 @Override
 public void renderWidget(GuiGraphics guiGraphics, int pMouseX, int pMouseY, float pPartialTick) {
 if (isHoveredOrFocused()) {
 MutableComponent tooltip = ConduitLang.FLUID_CONDUIT_CHANGE_FLUID1.copy();
 tooltip.append("\n").append(ConduitLang.FLUID_CONDUIT_CHANGE_FLUID2);
 var fluid = currentFluid.get();
 if (fluid != null && !fluid.isSame(Fluids.EMPTY)) {
 tooltip.append("\n")
 .append(TooltipUtil.withArgs(ConduitLang.FLUID_CONDUIT_CHANGE_FLUID3,
 fluid.getFluidType().getDescription()));
 }
 setTooltip(Tooltip.create(TooltipUtil.style(tooltip)));
 }

 RenderSystem.enableBlend();
 RenderSystem.defaultBlendFunc();
 RenderSystem.enableDepthTest();
 guiGraphics.blit(WIDGET_TEXTURE, getX(), getY(), 0, 0, this.width, this.height);
 if (currentFluid.get().isSame(Fluids.EMPTY)) {
 return;
 }

 IClientFluidTypeExtensions props = IClientFluidTypeExtensions.of(currentFluid.get());
 ResourceLocation still = props.getStillTexture();
 AbstractTexture texture = Minecraft.getInstance()
 .getTextureManager()
 .getTexture(TextureAtlas.LOCATION_BLOCKS);
 if (texture instanceof TextureAtlas atlas) {
 TextureAtlasSprite sprite = atlas.getSprite(still);

 int color = props.getTintColor();
 RenderSystem.setShaderColor(FastColor.ARGB32.red(color) / 255.0F,
 FastColor.ARGB32.green(color) / 255.0F, FastColor.ARGB32.blue(color) / 255.0F,
 FastColor.ARGB32.alpha(color) / 255.0F);
 RenderSystem.enableBlend();

 int atlasWidth = (int) (sprite.contents().width() / (sprite.getU1() - sprite.getU0()));
 int atlasHeight = (int) (sprite.contents().height() / (sprite.getV1() - sprite.getV0()));

 guiGraphics.blit(TextureAtlas.LOCATION_BLOCKS, getX() + 1, getY() + 1, 0, sprite.getU0() * atlasWidth,
 sprite.getV0() * atlasHeight, 12, 12, atlasWidth, atlasHeight);

 RenderSystem.setShaderColor(1, 1, 1, 1);
 }

 RenderSystem.disableBlend();
 RenderSystem.disableDepthTest();
 }

 @Override
 public void onClick(double pMouseX, double pMouseY) {
 onPress.run();
 }
 }
}
enderio-conduits/src/main/java/com/enderio/conduits/common/network/ConduitCommonPayloadHandler.java (1)
6-22: ⚠️ Potential issue

Review of ConduitCommonPayloadHandler implementation

The singleton implementation looks good, but there appears to be a potential issue with the permissions check in the handler method. Currently, only spectator players can update the connection configuration, which seems counterintuitive - spectators typically shouldn't be able to modify game state.

Consider the following fix:
- if (context.player().isSpectator() && context.player().containerMenu instanceof ConduitMenu menu) {
+ if (!context.player().isSpectator() && context.player().containerMenu instanceof ConduitMenu menu) {
 menu.setConnectionConfig(packet.connectionConfig());
 }
Also, consider adding validation for the packet data and error logging for debugging purposes.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
public class ConduitCommonPayloadHandler {
 private static final ConduitCommonPayloadHandler INSTANCE = new ConduitCommonPayloadHandler();

 public void handle(SetConduitConnectionConfigPacket packet, IPayloadContext context) {
 context.enqueueWork(() -> {
 if (packet.containerId() == context.player().containerMenu.containerId) {
- if (context.player().isSpectator() && context.player().containerMenu instanceof ConduitMenu menu) {
+ if (!context.player().isSpectator() && context.player().containerMenu instanceof ConduitMenu menu) {
 menu.setConnectionConfig(packet.connectionConfig());
 }
 }
 });
 }

 public static ConduitCommonPayloadHandler getInstance() {
 return INSTANCE;
 }
}
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/bundle/ConduitBlockEvent.java (1)

36-43: 🛠️ Refactor suggestion

Review spectator usage.
Allowing spectators to open conduit GUIs can be a design choice, but it may inadvertently disclose system internals. Please confirm that this aligns with your gameplay or security requirements.
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitTicker.java (1)
28-29: ⚠️ Potential issue

Possible naming confusion leading to incorrect fluid flow.
The parameter receiver is drained, while senders are used for insertion. This may invert the intuitive usage of "receiver" and "sender". If the intention is to push fluid into “receiver”, rename these parameters or reverse the logic to avoid confusion about which side is actually providing fluid.
- private int doFluidTransfer(FluidStack fluid, Connection receiver, List<Connection> senders) {
- FluidStack extractedFluid = receiver.fluidHandler().drain(fluid, IFluidHandler.FluidAction.SIMULATE);
- ...
- FluidStack transferredFluid = FluidUtil.tryFluidTransfer(insert.fluidHandler(), receiver.fluidHandler(), fluid, true);
+ private int doFluidTransfer(FluidStack fluid, Connection supplier, List<Connection> consumers) {
+ FluidStack extractedFluid = supplier.fluidHandler().drain(fluid, IFluidHandler.FluidAction.SIMULATE);
+ ...
+ FluidStack transferredFluid = FluidUtil.tryFluidTransfer(consumer.fluidHandler(), supplier.fluidHandler(), fluid, true);
Also applies to: 56-57
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitBlockItem.java (1)
133-135: 🛠️ Refactor suggestion

Potential ClassCastException risk.

Casting o2 to (T) may cause a runtime error if they do not share the same type. Consider verifying compatibility or adding a defensive check to ensure correctness.
 private static <T extends Conduit<T, ?>> int compareConduitTo(Conduit<T, ?> o1, Conduit<?, ?> o2) {
- return o1.compareTo((T) o2);
+ if (o2.getClass().isAssignableFrom(o1.getClass())) {
+ return o1.compareTo((T) o2);
+ } else {
+ // Fallback logic or safe handling
+ return 0;
+ }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 private static <T extends Conduit<T, ?>> int compareConduitTo(Conduit<T, ?> o1, Conduit<?, ?> o2) {
 if (o2.getClass().isAssignableFrom(o1.getClass())) {
 return o1.compareTo((T) o2);
 } else {
 // Fallback logic or safe handling
 return 0;
 }
 }
enderio-conduits/src/main/java/com/enderio/conduits/api/ConduitType.java (1)

34-40: 🛠️ Refactor suggestion

Check all call sites for the updated method signatures.
These methods now accept a more flexible type parameter T extends Conduit<T, ?>. Confirm that all code referencing these factory or builder methods has been updated to avoid compile-time errors.

enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduit.java (1)

99-105: ⚠️ Potential issue

isItemValid uses stack.getCapability(...) instanceof ...
getCapability returns a LazyOptional, so direct instanceof checks typically fail at runtime. Ensure the code resolves the capability instance first.
enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/ConduitScreenTypes.java (1)
23-29: 🛠️ Refactor suggestion

Add null check in get method

The get method doesn't verify if the SCREEN_TYPES map has been initialized or if the lookup returns null. This could lead to unexpected NullPointerExceptions.
 @EnsureSide(EnsureSide.Side.CLIENT)
 @Nullable
 public static <T extends Conduit<T, U>, U extends ConnectionConfig> ConduitScreenType get(
 ConduitType<T> conduitType) {
+ if (SCREEN_TYPES == null) {
+ return null;
+ }
 // noinspection unchecked
 return (ConduitScreenType) SCREEN_TYPES.get(conduitType);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 @EnsureSide(EnsureSide.Side.CLIENT)
 @Nullable
 public static <T extends Conduit<T, U>, U extends ConnectionConfig> ConduitScreenType get(
 ConduitType<T> conduitType) {
 if (SCREEN_TYPES == null) {
 return null;
 }
 // noinspection unchecked
 return (ConduitScreenType) SCREEN_TYPES.get(conduitType);
 }
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduit.java (1)

33-34: ⚠️ Potential issue

Evaluate fallback scenario for multi-fluid.
If the conduit is multi-fluid, canConnectConduits() returns true without further checks. Ensure this does not allow unintended fluid mixing in the network.

Do you need a fallback check to disallow mixing incompatible fluids?
enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/menu/ConduitMenu.java (1)
85-103: 💡 Verification agent
❓ Verification inconclusive

Verify buffer decoding for potential errors.
Decoding from the buffer can throw runtime exceptions if the data is malformed or from a mismatched mod version. Including minimal error handling or fallback logic could protect against unexpected data corruption and prevent crashes.

Please run a quick search throughout the codebase to confirm if any buffer decoding exceptions are caught or handled:

🏁 Script executed:
#!/bin/bash
rg -A 5 "decode(buf)"
Length of output: 23
Buffer Decoding Error Handling Required

It has been confirmed that the buffer decoding in the constructor of ConduitMenu (lines 85–103) is unguarded—that is, there is no try-catch or other form of error handling in place for the Conduit.STREAM_CODEC.decode(buf) call. Without such safeguards, a malformed buffer or a version mismatch could indeed result in runtime exceptions and potentially crash the application.

Please consider adding minimal error handling or fallback logic at this stage. A potential solution could involve wrapping the decode call in a try-catch block to handle any exceptions gracefully, thereby ensuring the UI remains stable even in the event of data corruption.

Affected file:
• enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/menu/ConduitMenu.java (lines 85–103)
enderio-conduits/src/main/java/com/enderio/conduits/api/Conduit.java (1)

196-198: 🛠️ Refactor suggestion

Inform users about item inventory size changes on conduit upgrades.
A note is present regarding item loss. It might be beneficial to present an in-game warning if the user tries to transition from a larger inventory conduit to a smaller one.

> [!CAUTION] > Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings. <details> <summary>🛑 Comments failed to post (28)</summary><blockquote> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/integration/jade/package-info.java (1)</summary><blockquote> 4-4: _🛠️ Refactor suggestion_ **Updated Package Declaration** The package declaration has been adjusted to reflect the new organisational structure, moving the file into `com.enderio.conduits.integration.jade` as intended by this refactor. Please ensure that all related modules, documentation, and references are updated accordingly to maintain consistency across the codebase. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduitNodeData.java (1)</summary><blockquote> 18-22: _🛠️ Refactor suggestion_ **Consider limiting or validating the map entries.** By using `Codec.unboundedMap(Direction.CODEC, Codec.INT)`, there is no explicit upper bound on map size. Realistically, only 6 entries are needed (one per `Direction`). You might consider using an enum-based map (e.g. `EnumMap<Direction, Integer>`) or validating the map during decoding to ensure data doesn't exceed expected sizes. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/client/model/conduit/modifier/RedstoneConduitModelModifier.java (1)</summary><blockquote> 14-14: _🛠️ Refactor suggestion_ **Add type checking before casting.** This direct cast assumes the conduit is a RedstoneConduit type. If a different conduit type is passed, this would throw a ClassCastException. Consider adding a type check before casting: ```diff - RedstoneConduit redstoneConduit = (RedstoneConduit) conduit.value(); + if (!(conduit.value() instanceof RedstoneConduit)) { + return null; // Or a default texture + } + RedstoneConduit redstoneConduit = (RedstoneConduit) conduit.value(); ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion if (!(conduit.value() instanceof RedstoneConduit)) { return null; // Or a default texture } RedstoneConduit redstoneConduit = (RedstoneConduit) conduit.value(); ````` </details>  </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitGraphContext.java (2)</summary><blockquote> 109-113: _🛠️ Refactor suggestion_ **Review: Encoding Context via Codec** The `encodeContext` method utilises `encodeStart(...).getOrThrow()` to perform serialization. While this approach ensures immediate feedback by throwing on error, it might be beneficial to consider more graceful error handling in future iterations, especially if codec failures are expected to occur under normal operation. --- 131-146: _🛠️ Refactor suggestion_ **Review: Robustness in loadNetworkContext Parsing** The private static `loadNetworkContext` method has been refactored to use the wildcard type for `Conduit<?, ?>` and includes critical null checks via `Objects.requireNonNull`. However, the repeated use of `getOrThrow()` (also noted in the TODO comment) indicates that error handling could be made more robust and flexible. Consider revisiting this approach to gracefully manage potential codec parsing failures. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/graph/ConduitConnectionHost.java (2)</summary><blockquote> 12-12: _⚠️ Potential issue_ **Address TODO comment before finalising PR** This TODO comment indicates uncertainty about whether this should be an interface or a concrete class. As interfaces are part of your public API contract (even if not marked as public API), changing this later could be disruptive. Consider resolving this architectural decision before merging. --- 20-22: _💡 Verification agent_ <details> <summary>🧩 Analysis chain</summary> **Verify error handling for connection configuration operations** The interface doesn't specify how errors should be handled when getting or setting connection configurations. Consider documenting what happens with invalid directions or null configuration values. Should implementers throw exceptions or handle errors gracefully? --- 🏁 Script executed: ```shell #!/bin/bash # Search for implementations of these methods to understand how errors are handled echo "Searching for implementations of getConnectionConfig and setConnectionConfig..." rg -A 5 -B 5 "getConnectionConfig\(Direction" --type java rg -A 5 -B 5 "setConnectionConfig\(Direction" --type java ``` Length of output: 14470 --- </details> **Update: Document Explicit Exception Contracts for Connection Configuration Methods** The investigation shows that implementations consistently handle error conditions by throwing exceptions (e.g. an IllegalStateException when the connection host is missing, or an IllegalArgumentException as documented in the API). However, the interface in *ConduitConnectionHost.java* does not explicitly specify the expected behaviour for invalid directions or null configuration values. - Please update the method documentation (Javadoc) to explicitly state: - That an IllegalArgumentException (or the appropriate exception) will be thrown if an invalid direction is provided. - What behaviour is expected if a null configuration value is passed to setConnectionConfig. - This clearer contract will help implementers ensure consistent error handling and improve the API’s usability. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/api/screen/RegisterConduitScreenTypesEvent.java (1)</summary><blockquote> 13-15: _💡 Verification agent_ <details> <summary>🧩 Analysis chain</summary> **Consider handling duplicate registrations.** The current implementation silently overwrites existing screen types for a conduit type. Depending on the intended behavior, you might want to log a warning or throw an exception when a duplicate registration occurs. --- 🏁 Script executed: ```shell #!/bin/bash # Check if there are any existing usages of this event that might be affected by this behavior rg -m 5 -A 3 "RegisterConduitScreenTypesEvent" --type java ``` Length of output: 3204 --- </details> **Duplicate Registration Handling Required** The current implementation of the register method in `enderio-conduits/src/main/java/com/enderio/conduits/api/screen/RegisterConduitScreenTypesEvent.java` silently overwrites any prior registration for a given conduit type. Given that the event is subsequently used in methods such as the one in `ConduitClientSetup.java` to register screen types, it would be prudent to explicitly handle duplicate registrations. For example, you might want to add a check to verify if a registration already exists and then log a warning or throw an exception before overwriting. A suggested approach: ```java public void register(ConduitType<? extends Conduit<?, ?>> conduitType, ConduitScreenType<?> screenType) { if(screenTypes.containsKey(conduitType)) { // Optionally log a warning or throw an exception to flag duplicate registration. // For instance: // throw new IllegalArgumentException("Duplicate registration for conduit type: " + conduitType); // Or alternatively: // Logger.warn("Duplicate registration for conduit type: " + conduitType + ". Overwriting existing screen type."); } screenTypes.put(conduitType, screenType); } ``` This change would ensure that accidental duplicate registrations do not lead to silent overwrites and make debugging registration issues easier. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/client/model/conduit/bundle/ConduitBundleRenderState.java (2)</summary><blockquote> 92-94: _⚠️ Potential issue_ **Potential null pointer exception in isConnectionEndpoint** This method assumes that `conduitConnections.get(side)` will never return null, but there's no guarantee of this. Consider adding a null check or using getOrDefault() as done in the `getConnectedConduits` method. ```diff public boolean isConnectionEndpoint(Direction side) { - return !conduitConnections.get(side).isEmpty(); + var connections = conduitConnections.get(side); + return connections != null && !connections.isEmpty(); } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion public boolean isConnectionEndpoint(Direction side) { var connections = conduitConnections.get(side); return connections != null && !connections.isEmpty(); } ````` </details>  --- 96-98: _⚠️ Potential issue_ **Missing null checks in getConnectionState** Similar to the issue above, this method may throw NullPointerExceptions if either `conduitConnections.get(side)` or the inner map's get returns null. ```diff public ConduitConnectionRenderState getConnectionState(Direction side, Holder<Conduit<?, ?>> conduit) { - return conduitConnections.get(side).get(conduit); + Map<Holder<Conduit<?, ?>>, ConduitConnectionRenderState> sideConnections = conduitConnections.get(side); + return sideConnections != null ? sideConnections.get(conduit) : null; } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion public ConduitConnectionRenderState getConnectionState(Direction side, Holder<Conduit<?, ?>> conduit) { Map<Holder<Conduit<?, ?>>, ConduitConnectionRenderState> sideConnections = conduitConnections.get(side); return sideConnections != null ? sideConnections.get(conduit) : null; } ````` </details>  </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/client/gui/ConduitSelectionButton.java (1)</summary><blockquote> 47-48: _🛠️ Refactor suggestion_ **Reference check vs. logical equality.** Using `conduit != currentConduit.get()` is a valid reference check, but consider whether a logical comparison (e.g., `.equals()`) might be more appropriate if multiple `Holder<Conduit<?, ?>>` instances could refer to the same underlying conduit. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/api/network/node/legacy/ConduitData.java (1)</summary><blockquote> 24-25: _🛠️ Refactor suggestion_ **Revisit parameter usage in withClientChanges.** This method currently ignores the `guiData` parameter, raising doubts about its relevance. Consider removing the parameter if truly unneeded, or implement the intended logic if appropriate. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/api/ticker/IOAwareConduitTicker.java (2)</summary><blockquote> 44-56: _🛠️ Refactor suggestion_ **Avoid creating duplicate connection objects for send/receive.** Calling `createConnection(...)` twice for the same node and side is wasteful if both `canSend(...)` and `canReceive(...)` return true. Refactor to create a single connection object, then add it to both sender and receiver lists if needed. ```diff - if (canSend(node, config)) { - var connection = createConnection(level, node, side); - if (connection != null) { - senders.get(config.sendColor()).add(connection); - } - } - - if (canReceive(node, config)) { - var connection = createConnection(level, node, side); - if (connection != null) { - receivers.get(config.receiveColor()).add(connection); - } - } + var canSend = canSend(node, config); + var canReceive = canReceive(node, config); + if (canSend || canReceive) { + var connection = createConnection(level, node, side); + if (connection != null) { + if (canSend) { + senders.get(config.sendColor()).add(connection); + } + if (canReceive) { + receivers.get(config.receiveColor()).add(connection); + } + } + } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion var canSend = canSend(node, config); var canReceive = canReceive(node, config); if (canSend || canReceive) { var connection = createConnection(level, node, side); if (connection != null) { if (canSend) { senders.get(config.sendColor()).add(connection); } if (canReceive) { receivers.get(config.receiveColor()).add(connection); } } } ````` </details>  --- 129-130: _💡 Verification agent_ <details> <summary>🧩 Analysis chain</summary> **Check for a potentially null inventory reference.** `node.getInventory(side)` could potentially return `null`, causing a NullPointerException if callers use `inventory()` without checking. Confirm that this is safe or add a null check to avoid unexpected runtime errors. --- 🏁 Script executed: ```shell #!/bin/bash # Description: Search the codebase for usage of inventory() to confirm if null checks are in place. fd -e java --exec rg --context 5 'inventory' ``` Length of output: 2774 --- </details> **Attention: Potential Null-Reference Risk in Inventory Access** The method implementation for `inventory()` simply returns `node.getInventory(side)` without any null-check. Our codebase usage—such as in: - `var inventory = receiver.inventory();` - Subsequent calls like `inventory.getStackInSlot(...)` —suggests that callers assume the return value will never be null. This could be risky if `node.getInventory(side)` can indeed return null, as it would lead to a NullPointerException at runtime. Please either: - Confirm that `node.getInventory(side)` is guaranteed never to return null (for example, via a non-null invariant or appropriate annotations/documentation), or - Add an explicit null-check within the `inventory()` method (or at all call sites) to handle a potential null value safely. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/legacy/DynamicConnectionState.java (1)</summary><blockquote> 53-63: _🛠️ Refactor suggestion_ **Handle the STUB or remove the redundant code** The provided parameters (`level`, `pos`, `direction`, `type`) are not used, and the method simply returns a fixed `DynamicConnectionState`. Consider implementing the intended logic or removing this STUB. ```diff - // STUB - return new DynamicConnectionState(false, DyeColor.GREEN, true, DyeColor.GREEN, RedstoneControl.NEVER_ACTIVE, - DyeColor.RED, ItemStack.EMPTY, ItemStack.EMPTY, ItemStack.EMPTY); +Conduit.ConduitConnectionData defaultConnection = type.value().getDefaultConnection(level, pos, direction); +return new DynamicConnectionState( + defaultConnection.isInsert(), + DyeColor.GREEN, + defaultConnection.isExtract(), + DyeColor.GREEN, + defaultConnection.control(), + DyeColor.RED, + ItemStack.EMPTY, + ItemStack.EMPTY, + ItemStack.EMPTY +); ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion Holder<Conduit<?, ?>> type) { Conduit.ConduitConnectionData defaultConnection = type.value().getDefaultConnection(level, pos, direction); return new DynamicConnectionState( defaultConnection.isInsert(), DyeColor.GREEN, defaultConnection.isExtract(), DyeColor.GREEN, defaultConnection.control(), DyeColor.RED, ItemStack.EMPTY, ItemStack.EMPTY, ItemStack.EMPTY ); } ````` </details>  </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/item/ItemConduit.java (1)</summary><blockquote> 109-128: _⚠️ Potential issue_ **Safeguard against null legacy side config.** When retrieving legacy data, there's a risk `oldSideConfig` might be null if the map lacks entries for certain sides. Dereferencing it could result in a null pointer exception. To address this, consider a null check as shown below: ```diff - var oldSideConfig = legacyData.get(side); - node.setConnectionConfig(side, - currentConfig.withIsRoundRobin(oldSideConfig.isRoundRobin) - .withIsSelfFeed(oldSideConfig.isSelfFeed) - .withPriority(oldSideConfig.priority)); + var oldSideConfig = legacyData.get(side); + if (oldSideConfig != null) { + node.setConnectionConfig(side, + currentConfig.withIsRoundRobin(oldSideConfig.isRoundRobin) + .withIsSelfFeed(oldSideConfig.isSelfFeed) + .withPriority(oldSideConfig.priority)); + } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion @Override public void copyLegacyData(ConduitNode node, ConduitDataAccessor legacyDataAccessor) { var legacyData = legacyDataAccessor.getData(ConduitTypes.Data.ITEM.get()); if (legacyData == null) { return; } // Copy connection config for (Direction side : Direction.values()) { if (node.isConnectedTo(side)) { var oldSideConfig = legacyData.get(side); var currentConfig = node.getConnectionConfig(side, ItemConduitConnectionConfig.TYPE); if (oldSideConfig != null) { node.setConnectionConfig(side, currentConfig.withIsRoundRobin(oldSideConfig.isRoundRobin) .withIsSelfFeed(oldSideConfig.isSelfFeed) .withPriority(oldSideConfig.priority)); } } } } ````` </details>  </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/FluidConduitScreenType.java (2)</summary><blockquote> 111-122: _🛠️ Refactor suggestion_ **Handle invalid resource locations when retrieving locked fluid.** A malformed or missing fluid identifier could throw an exception. Consider adding a try/catch block: ```diff - return BuiltInRegistries.FLUID.get(ResourceLocation.parse(tag.getString("LockedFluid"))); + try { + return BuiltInRegistries.FLUID.get(ResourceLocation.parse(tag.getString("LockedFluid"))); + } catch (Exception e) { + // Log and gracefully handle invalid fluid name + return Fluids.EMPTY; + } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion private Fluid getLockedFluid(ConduitMenuDataAccess<FluidConduitConnectionConfig> dataAccess) { var tag = dataAccess.getExtraGuiData(); if (tag == null) { return Fluids.EMPTY; } if (!tag.contains("LockedFluid")) { return Fluids.EMPTY; } try { return BuiltInRegistries.FLUID.get(ResourceLocation.parse(tag.getString("LockedFluid"))); } catch (Exception e) { // Log and gracefully handle invalid fluid name return Fluids.EMPTY; } } ````` </details>  --- 124-193: _🛠️ Refactor suggestion_ **Confirm fluid supplier reliability.** Ensure `currentFluid.get()` is never null. If null is possible, add a safeguard check to prevent potential NullPointerExceptions. Example adjustment: ```diff - if (!currentFluid.get().isSame(Fluids.EMPTY)) { + var fluid = currentFluid.get(); + if (fluid != null && !fluid.isSame(Fluids.EMPTY)) { + // ... + } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion private static class FluidWidget extends AbstractWidget { private static final ResourceLocation WIDGET_TEXTURE = EnderIO.loc("textures/gui/fluidbackground.png"); private final Runnable onPress; private final Supplier<Fluid> currentFluid; FluidWidget(int x, int y, Supplier<Fluid> fluid, Runnable onPress) { super(x, y, 14, 14, Component.empty()); this.onPress = onPress; this.currentFluid = fluid; } @Override public void updateWidgetNarration(NarrationElementOutput pNarrationElementOutput) { } @Override public void renderWidget(GuiGraphics guiGraphics, int pMouseX, int pMouseY, float pPartialTick) { if (isHoveredOrFocused()) { MutableComponent tooltip = ConduitLang.FLUID_CONDUIT_CHANGE_FLUID1.copy(); tooltip.append("\n").append(ConduitLang.FLUID_CONDUIT_CHANGE_FLUID2); var fluid = currentFluid.get(); if (fluid != null && !fluid.isSame(Fluids.EMPTY)) { tooltip.append("\n") .append(TooltipUtil.withArgs(ConduitLang.FLUID_CONDUIT_CHANGE_FLUID3, fluid.getFluidType().getDescription())); } setTooltip(Tooltip.create(TooltipUtil.style(tooltip))); } RenderSystem.enableBlend(); RenderSystem.defaultBlendFunc(); RenderSystem.enableDepthTest(); guiGraphics.blit(WIDGET_TEXTURE, getX(), getY(), 0, 0, this.width, this.height); if (currentFluid.get().isSame(Fluids.EMPTY)) { return; } IClientFluidTypeExtensions props = IClientFluidTypeExtensions.of(currentFluid.get()); ResourceLocation still = props.getStillTexture(); AbstractTexture texture = Minecraft.getInstance() .getTextureManager() .getTexture(TextureAtlas.LOCATION_BLOCKS); if (texture instanceof TextureAtlas atlas) { TextureAtlasSprite sprite = atlas.getSprite(still); int color = props.getTintColor(); RenderSystem.setShaderColor(FastColor.ARGB32.red(color) / 255.0F, FastColor.ARGB32.green(color) / 255.0F, FastColor.ARGB32.blue(color) / 255.0F, FastColor.ARGB32.alpha(color) / 255.0F); RenderSystem.enableBlend(); int atlasWidth = (int) (sprite.contents().width() / (sprite.getU1() - sprite.getU0())); int atlasHeight = (int) (sprite.contents().height() / (sprite.getV1() - sprite.getV0())); guiGraphics.blit(TextureAtlas.LOCATION_BLOCKS, getX() + 1, getY() + 1, 0, sprite.getU0() * atlasWidth, sprite.getV0() * atlasHeight, 12, 12, atlasWidth, atlasHeight); RenderSystem.setShaderColor(1, 1, 1, 1); } RenderSystem.disableBlend(); RenderSystem.disableDepthTest(); } @Override public void onClick(double pMouseX, double pMouseY) { onPress.run(); } } } ````` </details>  </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/network/ConduitCommonPayloadHandler.java (1)</summary><blockquote> 6-22: _⚠️ Potential issue_ **Review of ConduitCommonPayloadHandler implementation** The singleton implementation looks good, but there appears to be a potential issue with the permissions check in the handler method. Currently, only spectator players can update the connection configuration, which seems counterintuitive - spectators typically shouldn't be able to modify game state. Consider the following fix: ```diff - if (context.player().isSpectator() && context.player().containerMenu instanceof ConduitMenu menu) { + if (!context.player().isSpectator() && context.player().containerMenu instanceof ConduitMenu menu) { menu.setConnectionConfig(packet.connectionConfig()); } ``` Also, consider adding validation for the packet data and error logging for debugging purposes.  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion public class ConduitCommonPayloadHandler { private static final ConduitCommonPayloadHandler INSTANCE = new ConduitCommonPayloadHandler(); public void handle(SetConduitConnectionConfigPacket packet, IPayloadContext context) { context.enqueueWork(() -> { if (packet.containerId() == context.player().containerMenu.containerId) { - if (context.player().isSpectator() && context.player().containerMenu instanceof ConduitMenu menu) { + if (!context.player().isSpectator() && context.player().containerMenu instanceof ConduitMenu menu) { menu.setConnectionConfig(packet.connectionConfig()); } } }); } public static ConduitCommonPayloadHandler getInstance() { return INSTANCE; } } ````` </details>  </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/bundle/ConduitBlockEvent.java (1)</summary><blockquote> 36-43: _🛠️ Refactor suggestion_ **Review spectator usage.** Allowing spectators to open conduit GUIs can be a design choice, but it may inadvertently disclose system internals. Please confirm that this aligns with your gameplay or security requirements. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduitTicker.java (1)</summary><blockquote> 28-29: _⚠️ Potential issue_ **Possible naming confusion leading to incorrect fluid flow.** The parameter `receiver` is drained, while `senders` are used for insertion. This may invert the intuitive usage of "receiver" and "sender". If the intention is to push fluid into “receiver”, rename these parameters or reverse the logic to avoid confusion about which side is actually providing fluid. ```diff - private int doFluidTransfer(FluidStack fluid, Connection receiver, List<Connection> senders) { - FluidStack extractedFluid = receiver.fluidHandler().drain(fluid, IFluidHandler.FluidAction.SIMULATE); - ... - FluidStack transferredFluid = FluidUtil.tryFluidTransfer(insert.fluidHandler(), receiver.fluidHandler(), fluid, true); + private int doFluidTransfer(FluidStack fluid, Connection supplier, List<Connection> consumers) { + FluidStack extractedFluid = supplier.fluidHandler().drain(fluid, IFluidHandler.FluidAction.SIMULATE); + ... + FluidStack transferredFluid = FluidUtil.tryFluidTransfer(consumer.fluidHandler(), supplier.fluidHandler(), fluid, true); ``` Also applies to: 56-57 </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/ConduitBlockItem.java (1)</summary><blockquote> 133-135: _🛠️ Refactor suggestion_ **Potential ClassCastException risk.** Casting `o2` to `(T)` may cause a runtime error if they do not share the same type. Consider verifying compatibility or adding a defensive check to ensure correctness. ```diff private static <T extends Conduit<T, ?>> int compareConduitTo(Conduit<T, ?> o1, Conduit<?, ?> o2) { - return o1.compareTo((T) o2); + if (o2.getClass().isAssignableFrom(o1.getClass())) { + return o1.compareTo((T) o2); + } else { + // Fallback logic or safe handling + return 0; + } } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion private static <T extends Conduit<T, ?>> int compareConduitTo(Conduit<T, ?> o1, Conduit<?, ?> o2) { if (o2.getClass().isAssignableFrom(o1.getClass())) { return o1.compareTo((T) o2); } else { // Fallback logic or safe handling return 0; } } ````` </details>  </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/api/ConduitType.java (1)</summary><blockquote> 34-40: _🛠️ Refactor suggestion_ **Check all call sites for the updated method signatures.** These methods now accept a more flexible type parameter `T extends Conduit<T, ?>`. Confirm that all code referencing these factory or builder methods has been updated to avoid compile-time errors. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/redstone/RedstoneConduit.java (1)</summary><blockquote> 99-105: _⚠️ Potential issue_ **`isItemValid` uses `stack.getCapability(...) instanceof ...`** `getCapability` returns a `LazyOptional`, so direct `instanceof` checks typically fail at runtime. Ensure the code resolves the capability instance first. </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/client/gui/screen/types/ConduitScreenTypes.java (1)</summary><blockquote> 23-29: _🛠️ Refactor suggestion_ **Add null check in get method** The `get` method doesn't verify if the `SCREEN_TYPES` map has been initialized or if the lookup returns null. This could lead to unexpected NullPointerExceptions. ```diff @EnsureSide(EnsureSide.Side.CLIENT) @Nullable public static <T extends Conduit<T, U>, U extends ConnectionConfig> ConduitScreenType get( ConduitType<T> conduitType) { + if (SCREEN_TYPES == null) { + return null; + } // noinspection unchecked return (ConduitScreenType) SCREEN_TYPES.get(conduitType); } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion @EnsureSide(EnsureSide.Side.CLIENT) @Nullable public static <T extends Conduit<T, U>, U extends ConnectionConfig> ConduitScreenType get( ConduitType<T> conduitType) { if (SCREEN_TYPES == null) { return null; } // noinspection unchecked return (ConduitScreenType) SCREEN_TYPES.get(conduitType); } ````` </details>  </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/type/fluid/FluidConduit.java (1)</summary><blockquote> 33-34: _⚠️ Potential issue_ **Evaluate fallback scenario for multi-fluid.** If the conduit is multi-fluid, `canConnectConduits()` returns `true` without further checks. Ensure this does not allow unintended fluid mixing in the network. Do you need a fallback check to disallow mixing incompatible fluids? </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/menu/ConduitMenu.java (1)</summary><blockquote> 85-103: _💡 Verification agent_ <details> <summary>❓ Verification inconclusive</summary> **Verify buffer decoding for potential errors.** Decoding from the buffer can throw runtime exceptions if the data is malformed or from a mismatched mod version. Including minimal error handling or fallback logic could protect against unexpected data corruption and prevent crashes. Please run a quick search throughout the codebase to confirm if any buffer decoding exceptions are caught or handled: --- 🏁 Script executed: ```shell #!/bin/bash rg -A 5 "decode(buf)" ``` Length of output: 23 --- </details> **Buffer Decoding Error Handling Required** It has been confirmed that the buffer decoding in the constructor of `ConduitMenu` (lines 85–103) is unguarded—that is, there is no try-catch or other form of error handling in place for the `Conduit.STREAM_CODEC.decode(buf)` call. Without such safeguards, a malformed buffer or a version mismatch could indeed result in runtime exceptions and potentially crash the application. Please consider adding minimal error handling or fallback logic at this stage. A potential solution could involve wrapping the decode call in a try-catch block to handle any exceptions gracefully, thereby ensuring the UI remains stable even in the event of data corruption. Affected file: • `enderio-conduits/src/main/java/com/enderio/conduits/common/conduit/menu/ConduitMenu.java` (lines 85–103) </blockquote></details> <details> <summary>enderio-conduits/src/main/java/com/enderio/conduits/api/Conduit.java (1)</summary><blockquote> 196-198: _🛠️ Refactor suggestion_ **Inform users about item inventory size changes on conduit upgrades.** A note is present regarding item loss. It might be beneficial to present an in-game warning if the user tries to transition from a larger inventory conduit to a smaller one. </blockquote></details> </blockquote></details>

justliliandev (Migrated from github.com) reviewed

2025-03-17 22:06:45 +00:00

justliliandev (Migrated from github.com) left a comment

there are some nice improvements there, well done

enderio-base/src/main/java/com/enderio/base/common/menu/AbstractFilterMenu.java