Use registry for cat and wolf variant #43
Conversation
WalkthroughTyped variant support is introduced for cats and wolves. CatMeta now maps its Variant enum to Mojang’s CatVariant and writes TYPED_CAT_VARIANT. WolfMeta adds getVariant/setVariant using TYPED_WOLF_VARIANT and increases MAX_OFFSET; imports updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant CatMeta
participant Metadata as Entity Metadata
rect rgba(230,245,255,0.6)
note over CatMeta: New typed cat variant flow
Caller->>CatMeta: setVariant(Variant v)
CatMeta->>CatMeta: v.getCatVariant()
CatMeta->>Metadata: write(TYPED_CAT_VARIANT, CatVariant)
Metadata-->>CatMeta: ack
CatMeta-->>Caller: return
end
sequenceDiagram
autonumber
participant Caller
participant WolfMeta
participant Metadata as Entity Metadata
rect rgba(235,255,235,0.6)
note over WolfMeta: New wolf variant accessors
Caller->>WolfMeta: getVariant()
WolfMeta->>Metadata: read(TYPED_WOLF_VARIANT, default=PALE)
Metadata-->>WolfMeta: WolfVariant
WolfMeta-->>Caller: WolfVariant
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/CatMeta.java (1)
23-30: Fix type mismatch in getVariant after switching to TYPED_CAT_VARIANTYou now write a CatVariant into metadata (via TYPED_CAT_VARIANT), but getVariant currently reads the index as CatMeta.Variant. This will cause a type mismatch at runtime. Read CatVariant from metadata, then map it to CatMeta.Variant.
Apply this diff to getVariant:
@NotNull public CatMeta.Variant getVariant() { - return super.metadata.getIndex(OFFSET, Variant.BLACK); + final CatVariant catVariant = super.metadata.getIndex(OFFSET, CatVariants.BLACK); + return Variant.fromCatVariant(catVariant); }And add a mapping helper in the enum:
private static final Variant[] VALUES = values(); public CatVariant getCatVariant() { return catVariant; } + + public static Variant fromCatVariant(final CatVariant catVariant) { + for (final Variant v : VALUES) { + if (v.catVariant == catVariant) { + return v; + } + } + return BLACK; + }
🧹 Nitpick comments (1)
api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/WolfMeta.java (1)
14-14: Confirm MAX_OFFSET (+5) is intentional; otherwise adjust to +4Current fields consume offsets 0..3:
- 0: begging
- 1: collar color
- 2: anger time
- 3: typed variant
If “sound variant” (mentioned in the PR summary) is not yet implemented at offset 4, consider setting MAX_OFFSET to OFFSET + 4 to reflect the next free slot. If you are intentionally reserving the slot, a short comment would help future readers.
Apply if unintentional:
- public static final byte MAX_OFFSET = OFFSET + 5; + public static final byte MAX_OFFSET = OFFSET + 4;
📜 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 (2)
api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/CatMeta.java(3 hunks)api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/WolfMeta.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/WolfMeta.java (1)
api/src/main/java/me/tofaa/entitylib/meta/types/TameableMeta.java (1)
TameableMeta(9-46)
⏰ 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 (2)
api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/WolfMeta.java (1)
20-27: Typed wolf variant read/write looks correctUsing TYPED_WOLF_VARIANT with WolfVariant default WolfVariants.PALE is aligned with the registry-based metadata change. API is clear with @NotNull annotations.
api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/CatMeta.java (1)
28-30: Switch to TYPED_CAT_VARIANT is the right moveWriting the registry entry via value.getCatVariant() correctly aligns with PacketEvents’ typed variant handling and resolves ordinal mismatch issues.
steveb05
force-pushed
the
fix/use-variant-registry
branch
from
a38d772 to
b7b69c3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/CatMeta.java (1)
78-90: Optional: Replace linear search with an O(1) lookup map.fromCatVariant currently does an O(n) scan across VALUES. Small n here, so it’s fine, but this can be simplified and made future-proof with a precomputed map.
Apply this diff to precompute the lookup map and streamline the method:
@@ - private static final Variant[] VALUES = values(); + private static final Variant[] VALUES = values(); + private static final java.util.Map<CatVariant, Variant> BY_CAT_VARIANT = new java.util.HashMap<>(); + static { + for (final Variant v : VALUES) { + BY_CAT_VARIIANT.put(v.getCatVariant(), v); + } + } @@ - public static Variant fromCatVariant(@NotNull final CatVariant catVariant) { - for (final Variant variant : VALUES) { - if (variant.getCatVariant().equals(catVariant)) { - return variant; - } - } - return BLACK; - } + public static Variant fromCatVariant(@NotNull final CatVariant catVariant) { + return BY_CAT_VARIANT.getOrDefault(catVariant, BLACK); + }
📜 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 (2)
api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/CatMeta.java(3 hunks)api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/WolfMeta.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/WolfMeta.java
⏰ 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 (5)
api/src/main/java/me/tofaa/entitylib/meta/mobs/tameable/CatMeta.java (5)
3-4: Good move to use registry-backed CatVariant.Switching to CatVariant/CatVariants aligns the API with the data-driven registry and avoids ordinal mismatches. No issues spotted here.
25-26: Reading typed variant looks correct and preserves a sensible default.Using a CatVariant default (CatVariants.BLACK) and mapping it back to the public Variant enum is a clean approach for compatibility.
29-31: Writing TYPED_CAT_VARIANT is the right fix.Persisting the typed CatVariant via EntityDataTypes.TYPED_CAT_VARIANT avoids the previous ordinal bug and brings this inline with the registry model.
58-68: Enum-to-registry mapping is comprehensive and explicit.The mappings cover all known cat variants and keep the public API stable. Looks good.
70-74: Encapsulation of the underlying CatVariant is clear.Storing the mapped CatVariant in the enum is straightforward and keeps responsibilities localized.
|
LGTM! Thank you for the PR <3 |
2 participants
To address an issue with cat variants, this pull request updates how their metadata is handled. Previously, the system used the enum's ordinal value for the metadata packet. This caused the wrong variant to be applied to the entity because cat variants are stored in the registry.
To fix this, the code now uses the cat variant registry provided by packet events. To maintain backward compatibility with older API versions, the enum has been mapped to the corresponding registry.
Additionally, since wolves also have variants, a corresponding method for them has been added. This required updating the maximum offset to account for two extra fields: one for the variant and one for the sound variant.
Summary by CodeRabbit
New Features
API Changes