Inbound 26.1 item wire + PacketDecoder MixinExtras; snapshot-2#3
Merged
Inbound 26.1 item wire + PacketDecoder MixinExtras; snapshot-2#3
Conversation
- Build legacy→server inverse map in LegacyItemIdTable (collision → stone). - Scope legacy decode to PacketDecoder#decode via LegacyInboundDecoding (reentrant-safe). - LegacyItemStackWireCodec: decode/encode optional stacks with trusted + delimited patches. - Wrap HashedStack.ActualItem.STREAM_CODEC for ServerboundContainerClickPacket. - Version 0.1.1; README + prebuilt jar. Made-with: Cursor
- Replace @Inject @at(THROW) (unsupported on this Mixin) with @WrapMethod + try/finally so LegacyInboundDecoding always leaves decode cleanly. - Add MixinExtras 0.5.3 (annotationProcessor + jar-in-jar) and preLaunch entrypoint (MixinExtrasBootstrap.init()). - Compile against Minecraft 26.2-snapshot-2; document MixinExtras in README. Made-with: Cursor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR bumps the project from v0.1.0 → v0.1.1, adds MixinExtras and a pre-launch entrypoint, updates the Minecraft snapshot baseline to 26.2-snapshot-2, and implements inbound legacy wire-level item-id translation via thread-local decoder context, new codecs, mixins, and an inverse legacy→server item-id map. Changes
Sequence Diagram(s)sequenceDiagram
participant Netty as Netty I/O
participant PD as PacketDecoderMixin
participant LID as LegacyInboundDecoding
participant Codec as LegacyItemStackWireCodec
participant LIT as LegacyItemIdTable
participant Reg as BuiltInRegistries.ITEM
Netty->>PD: decode(ctx, input, out)
PD->>LID: enterDecode(ctx.channel())
PD->>PD: original.decode(ctx, input, out)
PD->>Codec: decodeOptional(...) (item stacks)
Codec->>LID: connection()
alt legacy connection active
Codec->>LIT: serverItemIdFromLegacyWire(legacyWireId)
LIT->>Reg: lookup item holder by server id (fallback STONE)
Codec->>Codec: decode DataComponentPatch / construct ItemStack
else non-legacy or no connection
Codec->>Codec: delegate to vanilla codec
end
PD->>LID: leaveDecode()
PD->>Netty: return decoded packets
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/dev/ohno/legacylink/mapping/LegacyItemIdTable.java (1)
131-138: Minor: Redundant negative check on line 137.The
sid >= 0check is always true given howbuildLegacyToServerInverseconstructs the array (all values are eitherstoneServerIdor valid server IDs fromtoLegacyTableindices). However, this is acceptable defensive coding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/dev/ohno/legacylink/mapping/LegacyItemIdTable.java` around lines 131 - 138, The conditional in serverItemIdFromLegacyWire redundantly checks "sid >= 0" even though buildLegacyToServerInverse guarantees entries are either valid server IDs or stoneServerItemId; simplify the method by removing the unnecessary negative check and directly return the mapped value (use legacyToServer and stoneServerItemId as currently referenced), or if you prefer defensive coding keep the bounds check for legacyWireId but change the final return to just "return sid;" to avoid the redundant comparison.src/main/java/dev/ohno/legacylink/mixin/HashedStackActualItemMixin.java (1)
45-54: Consolidate legacy item-id remap logic to avoid divergence.The decode/encode remap + fallback logic here duplicates the same pattern already centralized in
src/main/java/dev/ohno/legacylink/encoding/LegacyItemStackWireCodec.java. Extracting shared helpers (e.g., id translation + fallback resolution) would reduce protocol drift risk in future snapshot bumps.Also applies to: 64-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/dev/ohno/legacylink/mixin/HashedStackActualItemMixin.java` around lines 45 - 54, The decode path in HashedStackActualItemMixin duplicates the legacy wire-id remap + fallback logic (wireItemId -> LegacyItemIdTable.serverItemIdFromLegacyWire -> BuiltInRegistries.ITEM.byId -> fallback to Items.STONE) that already exists in LegacyItemStackWireCodec; add a shared helper in LegacyItemStackWireCodec (e.g., resolveHolderFromWireId(int wireId) or resolveItemHolder(int wireId)) to encapsulate translation and fallback, then replace the duplicated code in HashedStackActualItemMixin (and the similar block at the 64-67 range) to call that helper (use the same Holder<Item> return to construct HashedStack.ActualItem), ensuring all remap/fallback behavior is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/java/dev/ohno/legacylink/mapping/LegacyItemIdTable.java`:
- Around line 131-138: The conditional in serverItemIdFromLegacyWire redundantly
checks "sid >= 0" even though buildLegacyToServerInverse guarantees entries are
either valid server IDs or stoneServerItemId; simplify the method by removing
the unnecessary negative check and directly return the mapped value (use
legacyToServer and stoneServerItemId as currently referenced), or if you prefer
defensive coding keep the bounds check for legacyWireId but change the final
return to just "return sid;" to avoid the redundant comparison.
In `@src/main/java/dev/ohno/legacylink/mixin/HashedStackActualItemMixin.java`:
- Around line 45-54: The decode path in HashedStackActualItemMixin duplicates
the legacy wire-id remap + fallback logic (wireItemId ->
LegacyItemIdTable.serverItemIdFromLegacyWire -> BuiltInRegistries.ITEM.byId ->
fallback to Items.STONE) that already exists in LegacyItemStackWireCodec; add a
shared helper in LegacyItemStackWireCodec (e.g., resolveHolderFromWireId(int
wireId) or resolveItemHolder(int wireId)) to encapsulate translation and
fallback, then replace the duplicated code in HashedStackActualItemMixin (and
the similar block at the 64-67 range) to call that helper (use the same
Holder<Item> return to construct HashedStack.ActualItem), ensuring all
remap/fallback behavior is centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5186567e-a312-49b0-ad20-2445f5a92d2a
⛔ Files ignored due to path filters (1)
prebuilt/legacylink-0.1.1.jaris excluded by!**/*.jar
📒 Files selected for processing (12)
README.mdbuild.gradle.ktsgradle.propertiessrc/main/java/dev/ohno/legacylink/LegacyLinkPreLaunch.javasrc/main/java/dev/ohno/legacylink/encoding/LegacyInboundDecoding.javasrc/main/java/dev/ohno/legacylink/encoding/LegacyItemStackWireCodec.javasrc/main/java/dev/ohno/legacylink/mapping/LegacyItemIdTable.javasrc/main/java/dev/ohno/legacylink/mixin/HashedStackActualItemMixin.javasrc/main/java/dev/ohno/legacylink/mixin/ItemStackOptionalCodecMixin.javasrc/main/java/dev/ohno/legacylink/mixin/PacketDecoderMixin.javasrc/main/resources/fabric.mod.jsonsrc/main/resources/legacylink.mixins.json
- serverItemIdFromLegacyWire: drop redundant sid>=0 (inverse table is always valid) - LegacyItemStackWireCodec.holderFromLegacyWireId for remap + byId + stone fallback - HashedStackActualItemMixin decode uses shared helper (matches optional stack decode) Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings 26.1→26.2 item stack wire handling for serverbound traffic and fixes a runtime mixin crash that broke new connections when the first player/channel loaded
PacketDecoder.What changed
Inbound item wire (already on branch, commit
ef3aeb3)LegacyItemIdTable: legacy wire id ↔ server item id;serverItemIdFromLegacyWire()for inbound.LegacyInboundDecoding: thread-local + re-entrant depth for inbound decode scope.PacketDecoderMixin: establish decode context for legacy codecs (see below for implementation fix).LegacyItemStackWireCodec,ItemStackOptionalCodecMixin(optional + untrusted),HashedStackActualItemMixinfor container clicks.legacylink.mixins.json: registers decoder + hashed stack mixins.This PR (
8e7a364)@At("THROW")removed: Fabric’s Mixin rejectsTHROWas an injection point →PacketDecoderfailed to transform on first connection (latest.logshowedInvalidInjectionException).@WrapMethodonPacketDecoder#decode:enterDecode/leaveDecodeintry/finallyso depth is correct on success and on codec exceptions.LegacyLinkPreLaunch:MixinExtrasBootstrap.init()viafabric.mod.jsonpreLaunchentrypoint.mixinextras-fabric0.5.3 (annotationProcessor+implementation(include(...))for jar-in-jar).minecraft_version: 26.2-snapshot-2 (align compile target with production); README table + MixinExtras row.Test notes
./gradlew buildpasses.Links
Made with Cursor
Summary by CodeRabbit
New Features
Chores