Rework travel api to be POI instead #1173
Conversation
WalkthroughReplaces the TravelTarget API with a new EnderPOI API across the codebase. Introduces EnderPOI interfaces, serializers, types, renderers, events, storage, networking, and service loading. Migrates client and server flows to EnderPOI.onActivation. Updates registries and packets. Removes legacy travel API and related render/event classes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Player
participant Input as Input/Event
participant Handler as TravelHandler
participant API as EnderPOIApi
participant POI as EnderPOI
participant Server as Server/Level
Player->>Input: Jump/Crouch/Right-click
Input->>Handler: Query POI (getEnderPOIs / getElevatorAnchorTarget)
Handler->>API: getInItemRange / get(...)
API-->>Handler: Optional<EnderPOI>
alt POI present
Handler->>POI: onActivation(level, player)
alt AnchorTravelTarget
POI->>Handler: request teleport position
Handler->>Server: teleportEvent(player, pos)
Server-->>Player: Teleport and sound
else Other EnderPOI
POI-->>Player: Perform POI-specific action (e.g., open screen)
end
else No POI
Input-->>Player: No action / short teleport paths as applicable
end
sequenceDiagram
rect rgba(230,240,255,0.5)
note over Client,Server: Network update of POIs
participant Server
participant Net as EIONetwork
participant Client
Server->>Net: ClientboundEnderPOIUpdatedPacket
Net->>Client: Dispatch to ClientPayloadHandler
Client->>Client: EnderPOIApi.set/remove
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| distance -= dy * 15; | ||
| } else { | ||
| yaw -= dx * 180; | ||
| yaw += dx * 180; |
Check failure
Code scanning / CodeQL
Implicit narrowing conversion in compound assignment High
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
enderio/src/main/java/com/enderio/enderio/content/travel/TravelHandler.java (1)
264-271: Propagate teleport cancellation to the caller.
teleportEventreturns empty when cancelled, butshortTeleport(...)still returnstruein that case, which can cause upstream logic (e.g., resource consumption, cooldowns) to behave as if a teleport succeeded.Apply this change in
shortTeleportto returneventPos.isPresent()instead of alwaystrueon a computed position:- if (player instanceof ServerPlayer serverPlayer) { + if (player instanceof ServerPlayer serverPlayer) { Optional<Vec3> eventPos = teleportEvent(player, pos.get()); if (eventPos.isPresent()) { player.teleportTo(eventPos.get().x(), eventPos.get().y(), eventPos.get().z()); serverPlayer.connection.resetPosition(); player.fallDistance = 0; if (player.isInWall()) { player.setPose(Pose.SWIMMING); } player.playNotifySound(SoundEvents.ENDERMAN_TELEPORT, SoundSource.PLAYERS, 1F, 1F); } else { player.playNotifySound(SoundEvents.DISPENSER_FAIL, SoundSource.PLAYERS, 1F, 1F); } - } - return true; + return eventPos.isPresent(); + } + return false; // non-server context: no teleport performedIf you prefer to keep client invocation returning
trueon servers only, return the boolean immediately after event handling and move the outerreturn true/falseaccordingly.enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelTargetRendering.java (1)
82-83: Use value equality, not reference equality, for active target detection.activeTarget == target may fail if the instances are equal by value but not the same object, causing incorrect highlighting. Use equals.
Apply this diff (fully-qualified to avoid a new import):
- boolean active = activeTarget == target; + boolean active = java.util.Objects.equals(activeTarget, target);
🧹 Nitpick comments (12)
enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOIType.java (1)
5-14: Consider equals/hashCode for proper type identity.The interface and factory method are well-implemented. The anonymous class captures the
toStringvalue efficiently. However, consider whetherequals()andhashCode()should be overridden based on theResourceLocationto ensure proper type identity when used in maps or sets. Currently, the anonymous instances will use reference equality, which may be the intended behaviour if these types are singletons.If value-based equality is needed (comparing by
ResourceLocationrather than reference), apply this diff:public interface EnderPOIType<T extends EnderPOI> { static <T extends EnderPOI> EnderPOIType<T> simple(final ResourceLocation name) { final String toString = name.toString(); return new EnderPOIType<>() { @Override public String toString() { return toString; } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (!(obj instanceof EnderPOIType)) return false; + return toString.equals(obj.toString()); + } + + @Override + public int hashCode() { + return toString.hashCode(); + } }; } }enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceBlockEntity.java (1)
61-65: TODO: Implement saving of UI state.The
saveAdditionalSyncedmethod contains a TODO comment indicating thatlastUiPitch,lastUiYaw, andlastUiDistanceshould be saved, but they're not currently persisted. Consider implementing this to maintain UI state across restarts.enderio/src/main/java/com/enderio/enderio/client/content/enderface/EnderfaceRenderer.java (1)
29-29: Fix spacing around operator.Missing space before the equality operator:
getColor()== nullshould begetColor() == null.Apply this diff to fix the spacing:
- int color = ChatFormatting.GREEN.getColor()== null ? 0xFFFFFF : ChatFormatting.GREEN.getColor(); + int color = ChatFormatting.GREEN.getColor() == null ? 0xFFFFFF : ChatFormatting.GREEN.getColor();enderio/src/main/java/com/enderio/enderio/foundation/network/EIONetwork.java (1)
57-58: Update handler method name to align with EnderPOI terminology.The packet type has been updated to
ClientboundEnderPOIUpdatedPacket, but the handler method is still namedhandleAddTravelTarget. For consistency with the EnderPOI refactor, consider renaming this tohandleAddEnderPOIorhandleEnderPOIUpdated.enderio/src/main/java/com/enderio/enderio/foundation/network/ClientPayloadHandler.java (1)
24-29: Consider renaming method to align with EnderPOI terminology.The method
handleAddTravelTargetnow handlesClientboundEnderPOIUpdatedPacketbut retains the old "TravelTarget" naming. Consider renaming tohandleEnderPOIUpdatedorhandleAddEnderPOIfor consistency with the refactored API.enderio/src/main/java/com/enderio/enderio/content/travel/TravelHandler.java (2)
188-204: Add a line‑of‑sight check to avoid selecting POIs through walls (unless intentional).Current filters use distance and FOV only; without a raycast, POIs can be targeted through solid blocks. If LOS is required, add a clip test from eye to POI center before accepting it. Also consider making the 15° FOV a config.
Would you like me to wire a non‑allocating LOS check using ClipContext between player.eyePosition and target.pos().getCenter() and gate on MISS vs BLOCK?
206-231: Elevator Y bounds are exclusive; consider inclusive checks.Using
>/<excludes POIs exactly atlowerY/upperY. If anchors sit at the range edges, they won’t be found.Apply this diff if inclusivity is desired:
- .filter(target -> target.pos().getY() > lowerY && target.pos().getY() < upperY) + .filter(target -> target.pos().getY() >= lowerY && target.pos().getY() <= upperY)enderio/src/main/java/com/enderio/enderio/content/travel/TravelTargetSavedData.java (2)
120-123: Always-dirty SavedData can cause unnecessary saves.Overriding
isDirty()to always returntruewrites every save cycle. Consider tracking adirtyflag and callingsetDirty()on mutations (set/remove), then returningsuper.isDirty().
75-83: Align packet naming between update and removal
ClientboundEnderPOIUpdatedPacketvsClientboundTravelTargetRemovedPacketare inconsistent. Rename the removal packet toClientboundEnderPOIRemovedPacket(and update its imports/references in TravelTargetSavedData.java, ClientPayloadHandler.java, and EIONetwork.java), or add a TODO explaining why the existing name is retained.enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceTravelTarget.java (1)
40-45: Safer client cast and follow‑up on TODO.
- Use an
instanceof ClientLevelguard before casting to avoid edge‑caseClassCastExceptions.- There’s a TODO for packet work; can help stub and wire the interaction packet if needed.
Apply:
- if (level.isClientSide) { - Client.openScreen(level, pos); - } + if (level.isClientSide) { + Client.openScreen(level, pos); + } ... - public static void openScreen(Level level, BlockPos pos) { - Minecraft.getInstance().setScreen(new EnderfaceScreen(pos.immutable(), (ClientLevel) level)); - } + public static void openScreen(Level level, BlockPos pos) { + if (!(level instanceof ClientLevel clientLevel)) return; + Minecraft.getInstance().setScreen(new EnderfaceScreen(pos.immutable(), clientLevel)); + }Happy to draft the client/server packets and handlers for the Enderface interaction.
Also applies to: 77-84
enderio/src/main/java/com/enderio/enderio/content/poi/EnderPOIApiImpl.java (1)
13-38: Return an unmodifiable view from getAll
Replace the live collection with a defensive copy:- public Collection<EnderPOI> getAll(Level level) { - return TravelTargetSavedData.getTravelData(level).getTravelTargets(); - } + public Collection<EnderPOI> getAll(Level level) { + return List.copyOf(TravelTargetSavedData.getTravelData(level).getTravelTargets()); + }enderio/src/main/java/com/enderio/enderio/content/travel/travel_anchor/AnchorTravelTarget.java (1)
80-85: Consider pose fix to avoid 1-tick suffocation like shortTeleport.In TravelHandler.shortTeleport, after teleport the code sets Pose.SWIMMING when the player is in a wall to avoid a tick of damage. Mirroring that here will improve reliability around tight spaces.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
enderio-armory/src/main/java/com/enderio/armory/common/item/darksteel/upgrades/travel/TravelUpgrade.java(2 hunks)enderio/src/main/java/com/enderio/enderio/api/EnderIORegistries.java(2 hunks)enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOI.java(1 hunks)enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOIApi.java(1 hunks)enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOISerializer.java(1 hunks)enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOIType.java(1 hunks)enderio/src/main/java/com/enderio/enderio/api/poi/POIRenderer.java(1 hunks)enderio/src/main/java/com/enderio/enderio/api/poi/RegisterEnderPOIRenderersEvent.java(1 hunks)enderio/src/main/java/com/enderio/enderio/api/poi/package-info.java(1 hunks)enderio/src/main/java/com/enderio/enderio/api/travel/RegisterTravelRenderersEvent.java(0 hunks)enderio/src/main/java/com/enderio/enderio/api/travel/TravelRenderer.java(0 hunks)enderio/src/main/java/com/enderio/enderio/api/travel/TravelTarget.java(0 hunks)enderio/src/main/java/com/enderio/enderio/api/travel/TravelTargetApi.java(0 hunks)enderio/src/main/java/com/enderio/enderio/api/travel/TravelTargetType.java(0 hunks)enderio/src/main/java/com/enderio/enderio/client/EnderIOClient.java(2 hunks)enderio/src/main/java/com/enderio/enderio/client/content/enderface/EnderfaceRenderer.java(3 hunks)enderio/src/main/java/com/enderio/enderio/client/content/enderface/package-info.java(1 hunks)enderio/src/main/java/com/enderio/enderio/client/content/machines/gui/screen/EnderfaceScreen.java(2 hunks)enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelAnchorHud.java(2 hunks)enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelAnchorRenderer.java(2 hunks)enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelClientEventHandler.java(6 hunks)enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelTargetRendering.java(3 hunks)enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceBlock.java(3 hunks)enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceBlockEntity.java(4 hunks)enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceTravelTarget.java(3 hunks)enderio/src/main/java/com/enderio/enderio/content/poi/EnderPOIApiImpl.java(2 hunks)enderio/src/main/java/com/enderio/enderio/content/travel/TravelHandler.java(5 hunks)enderio/src/main/java/com/enderio/enderio/content/travel/TravelStaffItem.java(3 hunks)enderio/src/main/java/com/enderio/enderio/content/travel/TravelTargetSavedData.java(6 hunks)enderio/src/main/java/com/enderio/enderio/content/travel/travel_anchor/AnchorTravelTarget.java(4 hunks)enderio/src/main/java/com/enderio/enderio/content/travel/travel_anchor/TravelAnchorBlock.java(2 hunks)enderio/src/main/java/com/enderio/enderio/content/travel/travel_anchor/TravelAnchorBlockEntity.java(3 hunks)enderio/src/main/java/com/enderio/enderio/foundation/network/ClientPayloadHandler.java(2 hunks)enderio/src/main/java/com/enderio/enderio/foundation/network/EIONetwork.java(2 hunks)enderio/src/main/java/com/enderio/enderio/foundation/network/ServerPayloadHandler.java(3 hunks)enderio/src/main/java/com/enderio/enderio/foundation/network/packets/ClientboundEnderPOIUpdatedPacket.java(1 hunks)enderio/src/main/java/com/enderio/enderio/init/EIOTravelTargets.java(2 hunks)enderio/src/main/resources/META-INF/services/com.enderio.enderio.api.poi.EnderPOIApi(1 hunks)enderio/src/main/resources/META-INF/services/com.enderio.enderio.api.travel.TravelTargetApi(0 hunks)
💤 Files with no reviewable changes (6)
- enderio/src/main/java/com/enderio/enderio/api/travel/TravelTarget.java
- enderio/src/main/java/com/enderio/enderio/api/travel/RegisterTravelRenderersEvent.java
- enderio/src/main/java/com/enderio/enderio/api/travel/TravelTargetApi.java
- enderio/src/main/java/com/enderio/enderio/api/travel/TravelTargetType.java
- enderio/src/main/resources/META-INF/services/com.enderio.enderio.api.travel.TravelTargetApi
- enderio/src/main/java/com/enderio/enderio/api/travel/TravelRenderer.java
🧰 Additional context used
🧬 Code graph analysis (12)
enderio/src/main/java/com/enderio/enderio/client/EnderIOClient.java (1)
enderio/src/main/java/com/enderio/enderio/api/poi/RegisterEnderPOIRenderersEvent.java (1)
RegisterEnderPOIRenderersEvent(9-24)
enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelClientEventHandler.java (1)
enderio/src/main/java/com/enderio/enderio/content/travel/TravelHandler.java (1)
TravelHandler(39-272)
enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOI.java (1)
enderio/src/main/java/com/enderio/enderio/api/EnderIORegistries.java (2)
EnderIORegistries(16-69)Keys(43-68)
enderio/src/main/java/com/enderio/enderio/content/travel/TravelStaffItem.java (1)
enderio/src/main/java/com/enderio/enderio/content/travel/TravelHandler.java (1)
TravelHandler(39-272)
enderio/src/main/java/com/enderio/enderio/content/travel/travel_anchor/AnchorTravelTarget.java (3)
enderio/src/main/java/com/enderio/enderio/content/travel/TravelHandler.java (1)
TravelHandler(39-272)enderio/src/main/java/com/enderio/enderio/init/EIOTravelTargets.java (1)
EIOTravelTargets(15-39)enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceTravelTarget.java (1)
Serializer(57-75)
enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceBlock.java (1)
enderio/src/main/java/com/enderio/enderio/init/EIOBlockEntities.java (1)
EIOBlockEntities(61-333)
enderio/src/main/java/com/enderio/enderio/foundation/network/ServerPayloadHandler.java (1)
enderio/src/main/java/com/enderio/enderio/content/travel/TravelHandler.java (1)
TravelHandler(39-272)
enderio/src/main/java/com/enderio/enderio/init/EIOTravelTargets.java (3)
enderio/src/main/java/com/enderio/enderio/api/EnderIORegistries.java (1)
EnderIORegistries(16-69)enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceTravelTarget.java (1)
Serializer(57-75)enderio/src/main/java/com/enderio/enderio/content/travel/travel_anchor/AnchorTravelTarget.java (1)
Serializer(105-128)
enderio-armory/src/main/java/com/enderio/armory/common/item/darksteel/upgrades/travel/TravelUpgrade.java (1)
enderio/src/main/java/com/enderio/enderio/content/travel/TravelHandler.java (1)
TravelHandler(39-272)
enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceTravelTarget.java (3)
enderio/src/main/java/com/enderio/enderio/client/content/machines/gui/screen/EnderfaceScreen.java (1)
EnderfaceScreen(50-543)enderio/src/main/java/com/enderio/enderio/init/EIOTravelTargets.java (1)
EIOTravelTargets(15-39)enderio/src/main/java/com/enderio/enderio/content/travel/travel_anchor/AnchorTravelTarget.java (1)
Serializer(105-128)
enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelTargetRendering.java (2)
enderio/src/main/java/com/enderio/enderio/api/poi/RegisterEnderPOIRenderersEvent.java (1)
RegisterEnderPOIRenderersEvent(9-24)enderio/src/main/java/com/enderio/enderio/content/travel/TravelHandler.java (1)
TravelHandler(39-272)
enderio/src/main/java/com/enderio/enderio/content/travel/TravelHandler.java (1)
enderio/src/main/java/com/enderio/enderio/foundation/tag/EIOTags.java (1)
EIOTags(21-203)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (37)
enderio/src/main/java/com/enderio/enderio/client/content/machines/gui/screen/EnderfaceScreen.java (2)
67-67: Verify the initial pitch orientation change.The initial pitch value changed from -45 to 45, reversing the default vertical camera orientation. Ensure this change is intentional and provides the desired user experience when first opening the Enderface screen.
170-172: Improved mouse drag behavior.The changes enhance the camera controls:
- Horizontal rotation now moves in the natural direction (yaw increases with rightward mouse movement)
- Unnecessary float cast removed from pitch clamping
These adjustments improve the intuitiveness of the camera manipulation.
enderio/src/main/java/com/enderio/enderio/client/content/enderface/package-info.java (1)
1-4: Approve package-info.java nullability annotations. net.minecraft.MethodsReturnNonnullByDefault is used extensively across the codebase; no further verification required.enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceBlock.java (3)
3-3: LGTM!The import has been correctly updated from
TravelTargetApitoEnderPOIApias part of the API migration.
20-23: LGTM!The
getRenderShapemethod is correctly implemented, returningRenderShape.MODELto ensure the block renders as a 3D model.
33-33: LGTM!The removal logic has been correctly updated to use the new
EnderPOIApi.INSTANCE.removeAt()method, maintaining the same behaviour as before.enderio/src/main/java/com/enderio/enderio/content/travel/travel_anchor/TravelAnchorBlock.java (2)
3-3: LGTM!The import has been correctly updated from
TravelTargetApitoEnderPOIApias part of the API migration.
20-20: LGTM!The removal logic has been correctly updated to use the new
EnderPOIApi.INSTANCE.removeAt()method, maintaining the same behaviour as before.enderio/src/main/java/com/enderio/enderio/foundation/network/packets/ClientboundEnderPOIUpdatedPacket.java (2)
4-4: LGTM!The import has been correctly updated from
TravelTargettoEnderPOIas part of the API migration.
10-15: LGTM!The packet has been correctly refactored:
- Record name updated from
ClientboundTravelTargetUpdatedPackettoClientboundEnderPOIUpdatedPacket- Payload type changed from
TravelTargettoEnderPOI- Packet identifier updated from
add_travel_targettoadd_ender_poi- Stream codec correctly references
EnderPOI.STREAM_CODECAll changes are consistent with the API migration.
enderio/src/main/java/com/enderio/enderio/content/travel/TravelStaffItem.java (2)
8-8: LGTM!The import for
EnderPOIis correctly added to support the API migration.
28-28: LGTM!The import for
Optionalis correctly added to support the newgetEnderPOIsmethod signature.enderio/src/main/java/com/enderio/enderio/client/EnderIOClient.java (2)
7-7: LGTM!The import has been correctly updated from
RegisterTravelRenderersEventtoRegisterEnderPOIRenderersEventas part of the API migration.
392-395: Registry types already use EnderPOIType correctly
BothTRAVEL_ANCHOR_TYPEandENDERFACE_TYPEregister viaEnderPOIType::simple, matching the new EnderPOI system.enderio/src/main/resources/META-INF/services/com.enderio.enderio.api.poi.EnderPOIApi (1)
1-2: EnderPOIApiImpl implementation verified. The class exists in com.enderio.enderio.content.poi and correctly implements EnderPOIApi.enderio/src/main/java/com/enderio/enderio/api/poi/package-info.java (1)
4-4: Package rename applied consistently
rgsearch returned no matches forcom.enderio.enderio.api.travel—all imports updated tocom.enderio.enderio.api.poi.enderio-armory/src/main/java/com/enderio/armory/common/item/darksteel/upgrades/travel/TravelUpgrade.java (1)
105-118: LGTM! API migration is correct.The migration from
TravelTargettoEnderPOIis properly implemented. The newonActivationmethod correctly replaces the previous teleportation logic whilst maintaining the same control flow.enderio/src/main/java/com/enderio/enderio/content/enderface/EnderfaceBlockEntity.java (2)
24-24: Verify the sign change forlastUiPitch.The default value changed from
-45to45. Ensure this pitch direction change is intentional and doesn't invert the UI camera orientation unexpectedly.
67-80: LGTM! API migration is correct.The transition from
TravelTargetApitoEnderPOIApiis properly implemented in both the getter and setter methods.enderio/src/main/java/com/enderio/enderio/content/travel/travel_anchor/TravelAnchorBlockEntity.java (1)
104-117: LGTM! API migration is correct.The migration from
TravelTargetApitoEnderPOIApimaintains the existing logic whilst aligning with the new API surface.enderio/src/main/java/com/enderio/enderio/api/poi/POIRenderer.java (1)
1-8: LGTM! Clean interface design.The
POIRendererinterface provides a clear contract for renderingEnderPOIinstances with appropriate rendering context parameters.enderio/src/main/java/com/enderio/enderio/foundation/network/ServerPayloadHandler.java (1)
37-57: LGTM! API migration and error handling improved.The migration to
EnderPOIApiand the use ofonActivationaligns with the new API design. The change fromcanBlockTeleporttocanTeleportbroadens the check to include both item and block teleportation, which is more appropriate for this general handler. The addition of the empty target guard at lines 44-47 improves error handling.enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelAnchorRenderer.java (1)
36-36: LGTM! Renderer interface migration is correct.The transition from
TravelRenderertoPOIRendereraligns with the API refactor whilst preserving the existing rendering logic.enderio/src/main/java/com/enderio/enderio/client/content/enderface/EnderfaceRenderer.java (2)
18-18: LGTM! Renderer interface migration is correct.The transition to
POIRendereraligns with the API refactor.
29-32: LGTM! Colour scheme updated.The colour changes to GREEN (default) and DARK_GREEN (active) appear intentional and provide visual distinction for the Enderface renderer.
enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOISerializer.java (1)
1-10: LGTM! Serialiser interface renamed and relocated.The rename from
TravelTargetSerializertoEnderPOISerializerand the package relocation fromapi.traveltoapi.poialign with the broader API refactor whilst preserving the serialisation contract.enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelAnchorHud.java (1)
44-89: LGTM!The method signature correctly updated to accept
EnderPOIinstead ofTravelTarget, and the implementation properly handles the type check forAnchorTravelTarget.enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelClientEventHandler.java (4)
38-48: LGTM!The jump and crouch handling correctly migrated to the Optional-based EnderPOI activation pattern, with appropriate cooldown management.
64-70: LGTM!The empty-click event handler correctly retrieves and activates the EnderPOI, preserving the swing behaviour when activated.
79-83: LGTM!The block-click event handler correctly retrieves and activates the EnderPOI, setting cancellation result only on successful activation.
92-96: LGTM!The item-click event handler correctly retrieves and activates the EnderPOI, setting cancellation result only on successful activation.
enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOI.java (1)
19-31: LGTM!The interface methods are well-defined with clear contracts for position, range, activation status, and activation behaviour.
enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOIApi.java (1)
12-18: LGTM!The ServiceLoader-based singleton pattern is appropriate for this API, and the method contracts provide a clear and complete interface for EnderPOI management.
enderio/src/main/java/com/enderio/enderio/api/poi/RegisterEnderPOIRenderersEvent.java (1)
15-23: LGTM!The use of
ConcurrentHashMapand returning an immutable copy viaMap.copyOfensures thread safety and prevents external modification of the registry.enderio/src/main/java/com/enderio/enderio/init/EIOTravelTargets.java (1)
15-33: Registry refactor looks consistent.Types/serializers now use EnderPOI; helper signature is correct. No issues spotted.
Confirm
EIOTravelTargets.register(bus)is invoked during mod init so the registries are created (e.g., in your main mod class).enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelTargetRendering.java (2)
66-69: Confirm EnderPOIApi iteration matches getEnderPOIs() source to keep identity stable.activeTarget comes from TravelHandler.getEnderPOIs(player) whereas the loop uses EnderPOIApi.INSTANCE.getAll(level). If these produce distinct instances per tick, identity comparisons will never match. The equals() fix above mitigates this, but please verify both sources produce consistent value objects.
29-38: init() is called in EnderIOClient.clientSetup, ensuring RENDERERS is initialised before use.
| public static final Registry<EnderPOIType<?>> TRAVEL_TARGET_TYPES = new RegistryBuilder<>(Keys.TRAVEL_TARGET_TYPES) | ||
| .sync(true) | ||
| .create(); | ||
|
|
||
| public static final Registry<TravelTargetSerializer<?>> TRAVEL_TARGET_SERIALIZERS = new RegistryBuilder<>(Keys.TRAVEL_TARGET_SERIALIZERS) | ||
| public static final Registry<EnderPOISerializer<?>> TRAVEL_TARGET_SERIALIZERS = new RegistryBuilder<>(Keys.TRAVEL_TARGET_SERIALIZERS) | ||
| .sync(true) | ||
| .create(); |
There was a problem hiding this comment.
Registry naming inconsistency with EnderPOI refactor.
The registry types have been correctly updated to EnderPOIType<?> and EnderPOISerializer<?>, but the registry field names still use the TRAVEL_TARGET prefix. For consistency with the EnderPOI refactor, consider renaming:
TRAVEL_TARGET_TYPES→ENDER_POI_TYPESTRAVEL_TARGET_SERIALIZERS→ENDER_POI_SERIALIZERS
The same applies to the registry key names in the Keys class (lines 44-45) and the string literals passed to createKey().
🤖 Prompt for AI Agents
In enderio/src/main/java/com/enderio/enderio/api/EnderIORegistries.java around
lines 18 to 24, the registry field names still use the old TRAVEL_TARGET prefix
even though types were refactored to EnderPOI; rename TRAVEL_TARGET_TYPES →
ENDER_POI_TYPES and TRAVEL_TARGET_SERIALIZERS → ENDER_POI_SERIALIZERS, update
any references/usages to the new names, and in the Keys class (around lines
44-45) rename the corresponding key constants and the string arguments passed to
createKey() to use "ender_poi_types" and "ender_poi_serializers" (or your
project's canonical snake_case), ensuring all call sites and imports are updated
to the new identifiers to keep naming consistent.
| Codec<EnderPOI> CODEC = EnderIORegistries.TRAVEL_TARGET_SERIALIZERS.byNameCodec() | ||
| .dispatch(EnderPOI::serializer, EnderPOISerializer::codec); | ||
| StreamCodec<RegistryFriendlyByteBuf, EnderPOI> STREAM_CODEC = ByteBufCodecs | ||
| .registry(EnderIORegistries.Keys.TRAVEL_TARGET_SERIALIZERS) | ||
| .dispatch(EnderPOI::serializer, EnderPOISerializer::streamCodec); |
There was a problem hiding this comment.
Registry naming inconsistency with EnderPOI refactor.
The codecs reference TRAVEL_TARGET_SERIALIZERS which retains the old "TravelTarget" naming. For consistency with the EnderPOI refactor, the registry should be renamed to ENDER_POI_SERIALIZERS (and similarly ENDER_POI_TYPES for the types registry). This naming discrepancy is also present in EnderIORegistries.java.
🤖 Prompt for AI Agents
In enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOI.java around lines
13 to 17, the codec references use the old registry constant
TRAVEL_TARGET_SERIALIZERS; update these to the new ENDER_POI_SERIALIZERS name
(and any related registry constant rename ENDER_POI_TYPES where applicable) to
match the EnderPOI refactor. Also update EnderIORegistries.java to rename the
TRAVEL_TARGET_SERIALIZERS and TRAVEL_TARGET_TYPES constants/keys to
ENDER_POI_SERIALIZERS and ENDER_POI_TYPES respectively, and update any usages
(including ByteBufCodecs.registry(...) calls and byNameCodec() references)
across the codebase to the new names so all registry lookups and codec
dispatches compile and remain consistent.
| EnderPOIApi INSTANCE = ServiceLoader.load(EnderPOIApi.class).findFirst().orElseThrow(); | ||
|
|
||
| Optional<EnderPOI> get(Level level, BlockPos pos); | ||
| <T extends EnderPOI> void set(Level level, T travelTarget); |
There was a problem hiding this comment.
Update parameter name to match type.
The parameter is named travelTarget but the type is T extends EnderPOI. For consistency, rename the parameter to enderPOI or poi.
Apply this diff:
- <T extends EnderPOI> void set(Level level, T travelTarget);
+ <T extends EnderPOI> void set(Level level, T enderPOI);📝 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.
| <T extends EnderPOI> void set(Level level, T travelTarget); | |
| <T extends EnderPOI> void set(Level level, T enderPOI); |
🤖 Prompt for AI Agents
In enderio/src/main/java/com/enderio/enderio/api/poi/EnderPOIApi.java around
line 15 the method parameter is named travelTarget while the generic type is T
extends EnderPOI; rename the parameter to a name matching the type (e.g.,
enderPOI or poi) and update the method signature accordingly so it reads
something like <T extends EnderPOI> void set(Level level, T enderPOI); ensure
the parameter name change is reflected in any implementing classes and usages.
| private static <T extends EnderPOI> void render(T target, LevelRenderer levelRender, PoseStack poseStack, | ||
| double distanceSquared, boolean isActive, float partialTick) { | ||
| // noinspection unchecked | ||
| getRenderer((TravelTargetType<T>) target.type()).render(target, levelRender, poseStack, distanceSquared, | ||
| getRenderer((EnderPOIType<T>) target.type()).render(target, levelRender, poseStack, distanceSquared, | ||
| isActive, partialTick); | ||
| } |
There was a problem hiding this comment.
Null-guard the renderer lookup to avoid NPEs for unregistered types.
If no renderer was registered for a given EnderPOIType, getRenderer(...) returns null and this line will NPE.
Apply this diff:
- // noinspection unchecked
- getRenderer((EnderPOIType<T>) target.type()).render(target, levelRender, poseStack, distanceSquared,
- isActive, partialTick);
+ // noinspection unchecked
+ POIRenderer<T> renderer = (POIRenderer<T>) getRenderer((EnderPOIType<T>) target.type());
+ if (renderer != null) {
+ renderer.render(target, levelRender, poseStack, distanceSquared, isActive, partialTick);
+ }📝 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 EnderPOI> void render(T target, LevelRenderer levelRender, PoseStack poseStack, | |
| double distanceSquared, boolean isActive, float partialTick) { | |
| // noinspection unchecked | |
| getRenderer((TravelTargetType<T>) target.type()).render(target, levelRender, poseStack, distanceSquared, | |
| getRenderer((EnderPOIType<T>) target.type()).render(target, levelRender, poseStack, distanceSquared, | |
| isActive, partialTick); | |
| } | |
| private static <T extends EnderPOI> void render(T target, LevelRenderer levelRender, PoseStack poseStack, | |
| double distanceSquared, boolean isActive, float partialTick) { | |
| // noinspection unchecked | |
| POIRenderer<T> renderer = (POIRenderer<T>) getRenderer((EnderPOIType<T>) target.type()); | |
| if (renderer != null) { | |
| renderer.render(target, levelRender, poseStack, distanceSquared, isActive, partialTick); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
enderio/src/main/java/com/enderio/enderio/client/content/travel/TravelTargetRendering.java
around lines 45 to 50, the direct call to getRenderer(...) can return null and
cause an NPE; change the code to assign the result of getRenderer(...) to a
local variable, check if it's null, and if so bail out (return) or skip
rendering (optionally log a warning), otherwise call render on the non-null
renderer with the existing parameters.
| @Override | ||
| public boolean onActivation(Level level, Player player) { | ||
| Optional<Double> height = TravelHandler.isTeleportPositionClear(level, this.pos()); | ||
| if (height.isEmpty()) { | ||
| return false; | ||
| } | ||
| BlockPos blockPos = this.pos(); | ||
| Vec3 teleportPosition = new Vec3(blockPos.getX() + 0.5f, blockPos.getY() + height.get() + 1, | ||
| blockPos.getZ() + 0.5f); | ||
| teleportPosition = TravelHandler.teleportEvent(player, teleportPosition).orElse(null); | ||
| if (teleportPosition != null) { | ||
| if (player instanceof ServerPlayer serverPlayer) { | ||
| player.teleportTo(teleportPosition.x(), teleportPosition.y(), teleportPosition.z()); | ||
| // Stop "moved too quickly" warnings | ||
| serverPlayer.connection.resetPosition(); | ||
| player.playNotifySound(SoundEvents.ENDERMAN_TELEPORT, SoundSource.PLAYERS, 0.75F, 1F); | ||
| } else { | ||
| PacketDistributor.sendToServer(new ServerboundRequestTravelPacket(this.pos())); | ||
| } | ||
|
|
||
| player.resetFallDistance(); | ||
| return true; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Fire teleport event server-side only; gate client packet send on client without event.
EntityTeleportEvent should not be posted on the client. Currently Line 78 calls TravelHandler.teleportEvent(...) before the server/client check, which can trigger client-side handlers, desync logic, and suppress the server request if “cancelled” on client. Move event posting into the server branch and send the request unconditionally on client.
Apply this diff:
- teleportPosition = TravelHandler.teleportEvent(player, teleportPosition).orElse(null);
- if (teleportPosition != null) {
- if (player instanceof ServerPlayer serverPlayer) {
- player.teleportTo(teleportPosition.x(), teleportPosition.y(), teleportPosition.z());
- // Stop "moved too quickly" warnings
- serverPlayer.connection.resetPosition();
- player.playNotifySound(SoundEvents.ENDERMAN_TELEPORT, SoundSource.PLAYERS, 0.75F, 1F);
- } else {
- PacketDistributor.sendToServer(new ServerboundRequestTravelPacket(this.pos()));
- }
-
- player.resetFallDistance();
- return true;
- }
- return false;
+ if (player instanceof ServerPlayer serverPlayer) {
+ var eventPos = TravelHandler.teleportEvent(player, teleportPosition);
+ if (eventPos.isEmpty()) {
+ player.playNotifySound(SoundEvents.DISPENSER_FAIL, SoundSource.PLAYERS, 0.75F, 1F);
+ return false;
+ }
+ Vec3 finalPos = eventPos.get();
+ player.teleportTo(finalPos.x(), finalPos.y(), finalPos.z());
+ // Stop "moved too quickly" warnings
+ serverPlayer.connection.resetPosition();
+ player.playNotifySound(SoundEvents.ENDERMAN_TELEPORT, SoundSource.PLAYERS, 0.75F, 1F);
+ player.resetFallDistance();
+ return true;
+ } else {
+ PacketDistributor.sendToServer(new ServerboundRequestTravelPacket(this.pos()));
+ // Server will validate and perform the teleport.
+ return true;
+ }🤖 Prompt for AI Agents
In
enderio/src/main/java/com/enderio/enderio/content/travel/travel_anchor/AnchorTravelTarget.java
around lines 69 to 93, the teleport event is currently fired before checking
whether the player is server-side, which can post EntityTeleportEvent on the
client and allow client-side cancellation to suppress the server request; move
the TravelHandler.teleportEvent(...) call inside the serverPlayer branch so the
event is only posted server-side, and in the client branch always send
PacketDistributor.sendToServer(new ServerboundRequestTravelPacket(this.pos()))
without invoking the event; ensure the server branch still respects the event's
optional result (skip teleport if empty) and that fall distance reset, sound,
and connection.resetPosition() run only after a successful server-side
event/teleport result.
| Optional<EnderPOI> enderPOIs = TravelHandler.getEnderPOIs(player); | ||
| if (enderPOIs.isPresent() && enderPOIs.get().onActivation(level, player)) { | ||
| player.getCooldowns().addCooldown(item, BaseConfig.COMMON.ITEMS.TRAVELLING_BLINK_DISABLED_TIME.get()); | ||
| return true; | ||
| } else if (TravelHandler.interact(level, player)) { | ||
| player.getCooldowns().addCooldown(this, BaseConfig.COMMON.ITEMS.TRAVELLING_BLINK_DISABLED_TIME.get()); | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing return statement after successful activation.
When onActivation succeeds, the cooldown is applied but the method doesn't return true. This means resources won't be consumed (see line 68-74 in the caller). The method should return true when activation succeeds to ensure resources are properly consumed.
Apply this diff to return true after successful activation:
} else {
Optional<EnderPOI> enderPOIs = TravelHandler.getEnderPOIs(player);
if (enderPOIs.isPresent() && enderPOIs.get().onActivation(level, player)) {
player.getCooldowns().addCooldown(item, BaseConfig.COMMON.ITEMS.TRAVELLING_BLINK_DISABLED_TIME.get());
+ return true;
}
}📝 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.
| Optional<EnderPOI> enderPOIs = TravelHandler.getEnderPOIs(player); | |
| if (enderPOIs.isPresent() && enderPOIs.get().onActivation(level, player)) { | |
| player.getCooldowns().addCooldown(item, BaseConfig.COMMON.ITEMS.TRAVELLING_BLINK_DISABLED_TIME.get()); | |
| return true; | |
| } else if (TravelHandler.interact(level, player)) { | |
| player.getCooldowns().addCooldown(this, BaseConfig.COMMON.ITEMS.TRAVELLING_BLINK_DISABLED_TIME.get()); | |
| return true; | |
| } | |
| } | |
| Optional<EnderPOI> enderPOIs = TravelHandler.getEnderPOIs(player); | |
| if (enderPOIs.isPresent() && enderPOIs.get().onActivation(level, player)) { | |
| player.getCooldowns().addCooldown(item, BaseConfig.COMMON.ITEMS.TRAVELLING_BLINK_DISABLED_TIME.get()); | |
| return true; | |
| } | |
| } |
🤖 Prompt for AI Agents
In enderio/src/main/java/com/enderio/enderio/content/travel/TravelStaffItem.java
around lines 93 to 97, after applying the cooldown when enderPOIs.isPresent() &&
enderPOIs.get().onActivation(level, player) is true, the method currently does
not return true; update the control flow to return true immediately after adding
the cooldown so the caller knows activation succeeded and resources will be
consumed.
| private EnderPOI loadTarget(HolderLookup.Provider lookupProvider, Tag tag) { | ||
| return EnderPOI.CODEC.decode(lookupProvider.createSerializationContext(NbtOps.INSTANCE), tag) | ||
| .getOrThrow() | ||
| .getFirst(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make deserialisation tolerant to bad data to avoid world-load crashes.
EnderPOI.CODEC.decode(...).getOrThrow() will crash on malformed or legacy entries. Prefer resultOrPartial to log and skip invalid entries.
Example:
private EnderPOI loadTarget(HolderLookup.Provider lookupProvider, Tag tag) {
var ctx = lookupProvider.createSerializationContext(NbtOps.INSTANCE);
return EnderPOI.CODEC.decode(ctx, tag)
.result()
.map(com.mojang.datafixers.util.Pair::getFirst)
.orElse(null);
}
// And in loadNBT:
targets.stream()
.map(t -> loadTarget(lookupProvider, (CompoundTag) t))
.filter(Objects::nonNull)
.forEach(target -> travelTargets.put(target.pos(), target));This avoids hard failures and drops corrupt entries with a log (add one if desired).
🤖 Prompt for AI Agents
In
enderio/src/main/java/com/enderio/enderio/content/travel/TravelTargetSavedData.java
around lines 114-118, the current loadTarget call uses
EnderPOI.CODEC.decode(...).getOrThrow(), which will crash on malformed or legacy
data; change it to use the safe decode result (result() or resultOrPartial) and
map the Pair to its first value, returning null when decoding fails, and update
the caller (loadNBT) to filter out nulls before inserting into travelTargets so
corrupt entries are skipped; optionally log the decoding error in the
resultOrPartial consumer for diagnostics.
1 participant
Description
Change the current travel target api to accept POI instead to generalize the api for the travel anchors and "the enderio".
TODO
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.
Checklist
Summary by CodeRabbit
New Features
Improvements
UI
Refactor