Fix the rendered packet overwrites the original metadata #44
Conversation
WalkthroughIntroduces a defensive copy of entity metadata in PacketUtil.renderPacket by duplicating each EntityData into a new ArrayList and setting it on the metadata before translation/mutation. Subsequent processing modifies the copied list rather than the original. No public APIs or external behavior are changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant PacketUtil
participant Metadata
participant Translator
Caller->>PacketUtil: renderPacket(packet, metadata, ...)
rect rgba(230,240,255,0.5)
note over PacketUtil: Create defensive copy of EntityData list
PacketUtil->>Metadata: getEntityMetadata()
PacketUtil->>PacketUtil: copy each EntityData(index,type,value)
PacketUtil->>Metadata: setEntityMetadata(copiedList)
end
rect rgba(240,255,240,0.5)
note over PacketUtil,Translator: Translate/mutate ADV_COMPONENT fields on copied list
PacketUtil->>Translator: translate(copiedList)
Translator-->>PacketUtil: translated list
end
PacketUtil-->>Caller: return processed packet/metadata
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
api/src/main/java/me/tofaa/entitylib/utils/PacketUtil.java (3)
19-23: Add null/empty guard and pre-size the copy; program to List.Avoid NPE when metadata has no entries, and trim allocations by pre-sizing. Also reduce coupling by using the List interface.
- final ArrayList<EntityData<?>> copiedEntityData = new ArrayList<>(); - metadata.getEntityMetadata().forEach(entityData -> - copiedEntityData.add(new EntityData(entityData.getIndex(), entityData.getType(), entityData.getValue())) - ); - metadata.setEntityMetadata(copiedEntityData); + final java.util.List<EntityData<?>> original = metadata.getEntityMetadata(); + if (original == null || original.isEmpty()) { + return; + } + final java.util.List<EntityData<?>> copiedEntityData = new ArrayList<>(original.size()); + original.forEach(entityData -> + copiedEntityData.add(new EntityData(entityData.getIndex(), entityData.getType(), entityData.getValue())) + ); + metadata.setEntityMetadata(copiedEntityData);
21-21: Nit: avoid raw type constructor.If it type-checks in your target Java/Packetevents versions, prefer the diamond to suppress raw-type warnings:
- copiedEntityData.add(new EntityData(entityData.getIndex(), entityData.getType(), entityData.getValue())) + copiedEntityData.add(new EntityData<>(entityData.getIndex(), entityData.getType(), entityData.getValue()))
7-7: Import List if you adopt the List-typed locals.Add:
+import java.util.List;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
api/src/main/java/me/tofaa/entitylib/utils/PacketUtil.java(2 hunks)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (1)
api/src/main/java/me/tofaa/entitylib/utils/PacketUtil.java (1)
19-23: Defensive copy prevents original metadata mutations — good fix.Replacing the metadata list with a freshly copied list of EntityData objects ensures downstream rendering doesn’t overwrite the caller’s originals. This directly addresses the PR’s objective.
2 participants
Summary by CodeRabbit