refactor: remove last Fabric API imports from common/#352
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a common ServerNetworking SPI and wires it to Fabric, migrates RequesterScreenHandler to use ServerNetworking.send(), updates FabricPacketRegistration to register the Fabric sender, refactors StirlingEngineBlockEntity to implement MenuBehavior.HasMenu with an anonymous MenuProvider, changes menu type registration to use MenuType with FeatureFlagSet.of(), and simplifies StirlingEngineScreenHandler’s client constructor signature. Sequence Diagram(s)sequenceDiagram
participant RequesterScreenHandler
participant ServerNetworking
participant ServerPlayNetworking
participant ServerPlayer
RequesterScreenHandler->>ServerNetworking: send(player, packet)
ServerNetworking->>ServerPlayNetworking: delegate (registered Sender)
ServerPlayNetworking->>ServerPlayer: deliver packet
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@common/src/main/java/com/logistics/core/lib/platform/ServerNetworking.java`:
- Around line 43-45: The send method in ServerNetworking currently drops packets
silently when the SPI field sender is null; change it to fail fast by throwing
an IllegalStateException with a clear message when sender is null (e.g., in
send(ServerPlayer, CustomPacketPayload) check sender and throw if null) and
ensure the register method that assigns sender (e.g., register(...) or
setSender(...)) validates its input with a null-check to prevent registering a
null sender; this surfaces wiring/order problems immediately rather than
silently dropping packets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 14df757a-4799-4efa-8727-3033dade8650
📒 Files selected for processing (6)
common/src/main/java/com/logistics/LogisticsPower.javacommon/src/main/java/com/logistics/core/lib/platform/ServerNetworking.javacommon/src/main/java/com/logistics/pipe/ui/RequesterScreenHandler.javacommon/src/main/java/com/logistics/power/engine/block/entity/StirlingEngineBlockEntity.javacommon/src/main/java/com/logistics/power/engine/ui/StirlingEngineScreenHandler.javafabric/src/main/java/com/logistics/fabric/FabricPacketRegistration.java
Summary
Eliminates the final three
net.fabricmc.*imports fromcommon/main sources, completing the acceptance criterion from #324::commonhas zero loader-specific imports.Changes
Power domain — drop unused
ExtendedMenuType/ExtendedMenuProvider:ExtendedMenuTypeto send aBlockPosto the client on screen open, but the client constructor had it labeledunusedPosand ignored it entirelyExtendedMenuTypewith vanillaMenuTypeinLogisticsPower.SCREEN.register()ExtendedMenuProvider<BlockPos>fromStirlingEngineBlockEntity's implements clause and droppedgetScreenOpeningData()StirlingEngineScreenHandler's client constructor to the vanilla(int syncId, Inventory)signaturePipe domain — abstract
ServerPlayNetworking.send()via SPI:ServerNetworkingincore.lib.platform(Pattern A — same shape asItemStorageLookup)FabricPacketRegistrationregistersServerPlayNetworking::sendas the implementationRequesterScreenHandler.broadcastChanges()now callsServerNetworking.send()instead of the Fabric API directlyNotes
common/src/mainnow has zeronet.fabricmc.*orteam.reborn.*importsCloses #324