fix(meta): add version support for PlayerMeta in 1.21.10+ #47
Conversation
📝 WalkthroughWalkthroughThis change introduces per-version metadata indexing for the PlayerMeta class, expanding the metadata offset range and adding version-aware logic. New metadata index constants are introduced, and getters/setters for various player attributes now include conditional checks to use appropriate indices based on server version (>= 1.21.10 vs. older versions). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@api/src/main/java/me/tofaa/entitylib/meta/types/PlayerMeta.java`:
- Around line 214-221: The setter setLeftShoulderData is missing the same
version guard used by getLeftShoulderData; add an early check using
isVersionNewer(ServerVersion.V_1_11) at the top of setLeftShoulderData and skip
(return) when the server version is older than V_1_11 so metadata isn't written
on unsupported versions, keeping the existing logic that chooses between
PLAYER_LEFT_SHOULDER_INDEX and offset(OFFSET,4) intact.
- Around line 231-238: The setter setRightShoulderData is missing the same
version guard used in the getter; update setRightShoulderData to check
isVersionNewer(ServerVersion.V_1_11) (like setLeftShoulderData does) and only
use PLAYER_RIGHT_SHOULDER_INDEX when that check passes, otherwise fall back to
using offset(OFFSET, 5) as before; keep the NBTCompound defaulting logic and the
super.metadata.setIndex(EntityDataTypes.NBT, value) calls intact.
| public void setLeftShoulderData(@Nullable NBTCompound value) { | ||
| if (value == null) value = new NBTCompound(); | ||
| super.metadata.setIndex(offset(OFFSET,4), EntityDataTypes.NBT, value); | ||
| if (isVersion(ServerVersion.V_1_21_10, VersionComparison.NEWER_THAN_OR_EQUALS)) { | ||
| super.metadata.setIndex(PLAYER_LEFT_SHOULDER_INDEX, EntityDataTypes.NBT, value); | ||
| } else { | ||
| super.metadata.setIndex(offset(OFFSET,4), EntityDataTypes.NBT, value); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing version check in setter.
The getter getLeftShoulderData() has isVersionNewer(ServerVersion.V_1_11) at line 207, but the setter is missing this guard. This could cause invalid metadata to be set on servers older than V_1_11.
Proposed fix
public void setLeftShoulderData(`@Nullable` NBTCompound value) {
+ isVersionNewer(ServerVersion.V_1_11);
if (value == null) value = new NBTCompound();
if (isVersion(ServerVersion.V_1_21_10, VersionComparison.NEWER_THAN_OR_EQUALS)) {🤖 Prompt for AI Agents
In `@api/src/main/java/me/tofaa/entitylib/meta/types/PlayerMeta.java` around lines
214 - 221, The setter setLeftShoulderData is missing the same version guard used
by getLeftShoulderData; add an early check using
isVersionNewer(ServerVersion.V_1_11) at the top of setLeftShoulderData and skip
(return) when the server version is older than V_1_11 so metadata isn't written
on unsupported versions, keeping the existing logic that chooses between
PLAYER_LEFT_SHOULDER_INDEX and offset(OFFSET,4) intact.
| public void setRightShoulderData(@Nullable NBTCompound value) { | ||
| if (value == null) value = new NBTCompound(); | ||
| super.metadata.setIndex(offset(OFFSET,5), EntityDataTypes.NBT, value); | ||
| if (isVersion(ServerVersion.V_1_21_10, VersionComparison.NEWER_THAN_OR_EQUALS)) { | ||
| super.metadata.setIndex(PLAYER_RIGHT_SHOULDER_INDEX, EntityDataTypes.NBT, value); | ||
| } else { | ||
| super.metadata.setIndex(offset(OFFSET,5), EntityDataTypes.NBT, value); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing version check in setter.
Same issue as setLeftShoulderData — the getter at line 224 has isVersionNewer(ServerVersion.V_1_11), but the setter is missing this guard.
Proposed fix
public void setRightShoulderData(`@Nullable` NBTCompound value) {
+ isVersionNewer(ServerVersion.V_1_11);
if (value == null) value = new NBTCompound();
if (isVersion(ServerVersion.V_1_21_10, VersionComparison.NEWER_THAN_OR_EQUALS)) {🤖 Prompt for AI Agents
In `@api/src/main/java/me/tofaa/entitylib/meta/types/PlayerMeta.java` around lines
231 - 238, The setter setRightShoulderData is missing the same version guard
used in the getter; update setRightShoulderData to check
isVersionNewer(ServerVersion.V_1_11) (like setLeftShoulderData does) and only
use PLAYER_RIGHT_SHOULDER_INDEX when that check passes, otherwise fall back to
using offset(OFFSET, 5) as before; keep the NBTCompound defaulting logic and the
super.metadata.setIndex(EntityDataTypes.NBT, value) calls intact.
|
Have you tested these changes? |
|
Yes, it actually works on 1.21.8-1.21.11 |
|
I discussed this PR with Tofaa in DMs, and it looks like we primarily target the latest protocol specification. Given that, it might make more sense to properly implement the new Avatar entity within the Player entity rather than skipping its offset. Could you take a look at #48 and share your thoughts? |
|
Okay, it looks good. I think this would be a better solution |
|
Closing in favor of #48. Thanks for your help nonetheless! Forced me to actually look into it besides other work I have :-) |
2 participants
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.