Conversation
Co-Authored-By: Stridey <19396085+StrideyGH@users.noreply.github.com>
WalkthroughRefactors WidgetArea to use Lombok-generated accessors and extends WidgetUtil with six new background texture constants plus a lines-based slot layout system for non-uniform, larger grid arrangements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/main/java/dev/tr7zw/itemswapper/util/WidgetUtil.java`:
- Around line 161-186: The new setupSlots method sets backgroundTextureSizeX but
never sets backgroundTextureSizeY, leaving it at 0 and breaking rendering;
update setupSlots to call widgetArea.setBackgroundTextureSizeY(...) using the
same threshold logic used for setBackgroundTextureSizeX (i.e., choose 256 vs 128
based on maxWidth or the equivalent condition used previously), so both
backgroundTextureSizeX and backgroundTextureSizeY are assigned appropriate
non-zero values when texture is provided or omitted (refer to setupSlots,
setBackgroundTextureSizeX, setBackgroundTextureSizeY and the maxWidth variable).
- Around line 146-158: The arrays passed to setupSlots for the length<=38 and
length<=30 branches don't match their handled capacities: the array used with
BACKGROUND_38_LOCATION currently sums to 37 so a length==38 item will have no
slot, and the array used with BACKGROUND_30_LOCATION sums to 36 which creates
extra empty slots; update the integer arrays passed to setupSlots in those
branches so their element counts sum to the handled length (e.g., change the
array passed alongside BACKGROUND_38_LOCATION so its elements total 38, and
optionally adjust the array for BACKGROUND_30_LOCATION to total 30 if you want
exact capacity) while keeping the call sites (setupSlots(widgetArea, slots, ...,
BACKGROUND_38_LOCATION) and setupSlots(widgetArea, slots, ...,
BACKGROUND_30_LOCATION)) and visual alignment with the background texture.
- Around line 208-220: The flip parameter in setupHalfGridSlots is ignored;
adjust the skip logic to mirror the two corner skips when flip is true so slots
are removed from the left side instead of the right. Specifically, in
setupHalfGridSlots change the skip condition that currently uses (x == width - 1
&& y == height - 1) || (x == width - 1 && y == 0) to branch on flip: when flip
is false keep the existing right-side corner checks, and when flip is true use
left-side checks (x == 0 && y == height - 1) || (x == 0 && y == 0); keep calling
setupSlots(widgetArea, slots, size, texture) after computing size.
- Around line 179-185: The slot rendering size is hardcoded to
ItemSwapperUI.tinySlotSize while grid spacing uses the variable sz, causing
visual mismatch; update the code in WidgetUtil (the loop creating GuiSlot
instances and the background dimension calculations just above it) to pass sz
instead of ItemSwapperUI.tinySlotSize so the GuiSlot constructors and background
dimensions use the same spacing variable (sz) for consistent layout and
rendering.
| public static void setupSlots(WidgetArea widgetArea, List<GuiSlot> slots, int[] lines, /*? >= 1.21.11 {*/ Identifier /*?} else {*//* ResourceLocation *//*?}*/ texture) { | ||
| int maxWidth = 0; | ||
| for (int line : lines) { | ||
| if (line > maxWidth) { | ||
| maxWidth = line; | ||
| } | ||
| } | ||
| widgetArea.setBackgroundTexture(texture); | ||
| widgetArea.setBackgroundSizeX(width * ItemSwapperUI.tinySlotSize + 6); | ||
| widgetArea.setBackgroundSizeY(height * ItemSwapperUI.tinySlotSize + 6); | ||
| widgetArea.setBackgroundSizeX(maxWidth * ItemSwapperUI.tinySlotSize + 6); | ||
| widgetArea.setBackgroundSizeY(lines.length * ItemSwapperUI.tinySlotSize + 6); | ||
| widgetArea.setBackgroundTextureSizeX(maxWidth >= 7 ? 256 : 128); | ||
| int sz = texture == null ? ItemSwapperUI.slotSize : ItemSwapperUI.tinySlotSize; | ||
| int lz = texture == null ? 11 : 9; | ||
| widgetArea.setMouseBoundsX(width * lz); | ||
| widgetArea.setMouseBoundsY(height * lz); | ||
| int originX = (int) (-width / 2d * sz - 2); | ||
| int originY = (int) (-height / 2d * sz - 1 - 2); | ||
| widgetArea.setMouseBoundsX(maxWidth * lz); | ||
| widgetArea.setMouseBoundsY(lines.length * lz); | ||
| int originX = (int) (-maxWidth / 2d * sz - 2); | ||
| int originY = (int) (-lines.length / 2d * sz - 1 - 2); | ||
| int slotId = 0; | ||
| for (int y = 0; y < lines.length; y++) { | ||
| int xOffset = (maxWidth - lines[y]) % 2 == 1 ? sz / 2 : 0; | ||
| for (int x = 0; x < lines[y]; x++) { | ||
| slots.add(new GuiSlot(originX + xOffset + ((maxWidth - lines[y]) / 2 + x) * sz, originY + y * sz, slotId, ItemSwapperUI.tinySlotSize)); | ||
| slotId++; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing setBackgroundTextureSizeY — all code paths now leave it at 0.
This overload sets backgroundTextureSizeX (Line 171) but never sets backgroundTextureSizeY. Since both the existing setupSlots(width, height, skipCorners, texture) (Line 200) and setupHalfGridSlots (Line 219) now delegate here, no code path sets backgroundTextureSizeY. It defaults to 0, which is then passed to graphics.blit(...) at Line 99 — this will produce broken rendering (likely a division-by-zero or zero-height texture mapping).
Proposed fix
widgetArea.setBackgroundTextureSizeX(maxWidth >= 7 ? 256 : 128);
+ widgetArea.setBackgroundTextureSizeY(lines.length >= 7 ? 256 : 128);Adjust the threshold to match whatever logic the old manual implementations used for backgroundTextureSizeY.
📝 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.
| public static void setupSlots(WidgetArea widgetArea, List<GuiSlot> slots, int[] lines, /*? >= 1.21.11 {*/ Identifier /*?} else {*//* ResourceLocation *//*?}*/ texture) { | |
| int maxWidth = 0; | |
| for (int line : lines) { | |
| if (line > maxWidth) { | |
| maxWidth = line; | |
| } | |
| } | |
| widgetArea.setBackgroundTexture(texture); | |
| widgetArea.setBackgroundSizeX(width * ItemSwapperUI.tinySlotSize + 6); | |
| widgetArea.setBackgroundSizeY(height * ItemSwapperUI.tinySlotSize + 6); | |
| widgetArea.setBackgroundSizeX(maxWidth * ItemSwapperUI.tinySlotSize + 6); | |
| widgetArea.setBackgroundSizeY(lines.length * ItemSwapperUI.tinySlotSize + 6); | |
| widgetArea.setBackgroundTextureSizeX(maxWidth >= 7 ? 256 : 128); | |
| int sz = texture == null ? ItemSwapperUI.slotSize : ItemSwapperUI.tinySlotSize; | |
| int lz = texture == null ? 11 : 9; | |
| widgetArea.setMouseBoundsX(width * lz); | |
| widgetArea.setMouseBoundsY(height * lz); | |
| int originX = (int) (-width / 2d * sz - 2); | |
| int originY = (int) (-height / 2d * sz - 1 - 2); | |
| widgetArea.setMouseBoundsX(maxWidth * lz); | |
| widgetArea.setMouseBoundsY(lines.length * lz); | |
| int originX = (int) (-maxWidth / 2d * sz - 2); | |
| int originY = (int) (-lines.length / 2d * sz - 1 - 2); | |
| int slotId = 0; | |
| for (int y = 0; y < lines.length; y++) { | |
| int xOffset = (maxWidth - lines[y]) % 2 == 1 ? sz / 2 : 0; | |
| for (int x = 0; x < lines[y]; x++) { | |
| slots.add(new GuiSlot(originX + xOffset + ((maxWidth - lines[y]) / 2 + x) * sz, originY + y * sz, slotId, ItemSwapperUI.tinySlotSize)); | |
| slotId++; | |
| } | |
| } | |
| } | |
| public static void setupSlots(WidgetArea widgetArea, List<GuiSlot> slots, int[] lines, /*? >= 1.21.11 {*/ Identifier /*?} else {*//* ResourceLocation *//*?}*/ texture) { | |
| int maxWidth = 0; | |
| for (int line : lines) { | |
| if (line > maxWidth) { | |
| maxWidth = line; | |
| } | |
| } | |
| widgetArea.setBackgroundTexture(texture); | |
| widgetArea.setBackgroundSizeX(maxWidth * ItemSwapperUI.tinySlotSize + 6); | |
| widgetArea.setBackgroundSizeY(lines.length * ItemSwapperUI.tinySlotSize + 6); | |
| widgetArea.setBackgroundTextureSizeX(maxWidth >= 7 ? 256 : 128); | |
| widgetArea.setBackgroundTextureSizeY(lines.length >= 7 ? 256 : 128); | |
| int sz = texture == null ? ItemSwapperUI.slotSize : ItemSwapperUI.tinySlotSize; | |
| int lz = texture == null ? 11 : 9; | |
| widgetArea.setMouseBoundsX(maxWidth * lz); | |
| widgetArea.setMouseBoundsY(lines.length * lz); | |
| int originX = (int) (-maxWidth / 2d * sz - 2); | |
| int originY = (int) (-lines.length / 2d * sz - 1 - 2); | |
| int slotId = 0; | |
| for (int y = 0; y < lines.length; y++) { | |
| int xOffset = (maxWidth - lines[y]) % 2 == 1 ? sz / 2 : 0; | |
| for (int x = 0; x < lines[y]; x++) { | |
| slots.add(new GuiSlot(originX + xOffset + ((maxWidth - lines[y]) / 2 + x) * sz, originY + y * sz, slotId, ItemSwapperUI.tinySlotSize)); | |
| slotId++; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/dev/tr7zw/itemswapper/util/WidgetUtil.java` around lines 161 -
186, The new setupSlots method sets backgroundTextureSizeX but never sets
backgroundTextureSizeY, leaving it at 0 and breaking rendering; update
setupSlots to call widgetArea.setBackgroundTextureSizeY(...) using the same
threshold logic used for setBackgroundTextureSizeX (i.e., choose 256 vs 128
based on maxWidth or the equivalent condition used previously), so both
backgroundTextureSizeX and backgroundTextureSizeY are assigned appropriate
non-zero values when texture is provided or omitted (refer to setupSlots,
setBackgroundTextureSizeX, setBackgroundTextureSizeY and the maxWidth variable).
| for (int y = 0; y < lines.length; y++) { | ||
| int xOffset = (maxWidth - lines[y]) % 2 == 1 ? sz / 2 : 0; | ||
| for (int x = 0; x < lines[y]; x++) { | ||
| slots.add(new GuiSlot(originX + xOffset + ((maxWidth - lines[y]) / 2 + x) * sz, originY + y * sz, slotId, ItemSwapperUI.tinySlotSize)); | ||
| slotId++; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src/main/java/dev/tr7zw/itemswapper/util && git log --oneline -p -S "new GuiSlot" -- WidgetUtil.java | head -150Repository: tr7zw/ItemSwapper
Length of output: 10457
🏁 Script executed:
# Also, let's look at the current full setupSlots method to understand the context
cat -n WidgetUtil.java | sed -n '150,190p'Repository: tr7zw/ItemSwapper
Length of output: 108
🏁 Script executed:
# Check if there are multiple setupSlots methods to understand the overloads
rg "void setupSlots" WidgetUtil.java -A 2Repository: tr7zw/ItemSwapper
Length of output: 116
🏁 Script executed:
find . -name "WidgetUtil.java" -type fRepository: tr7zw/ItemSwapper
Length of output: 119
🏁 Script executed:
cat -n src/main/java/dev/tr7zw/itemswapper/util/WidgetUtil.java | sed -n '160,200p'Repository: tr7zw/ItemSwapper
Length of output: 2413
🏁 Script executed:
# Check the sz variable assignment and the setupSlots method
cat -n src/main/java/dev/tr7zw/itemswapper/util/WidgetUtil.java | sed -n '165,190p'Repository: tr7zw/ItemSwapper
Length of output: 1602
🏁 Script executed:
# Verify that setupSlots is actually called with texture=null for small grids
cat -n src/main/java/dev/tr7zw/itemswapper/util/WidgetUtil.java | sed -n '102,110p'Repository: tr7zw/ItemSwapper
Length of output: 555
🏁 Script executed:
# Check the GuiSlot constructor to understand what the last parameter represents
rg "class GuiSlot" -A 10 src/Repository: tr7zw/ItemSwapper
Length of output: 43
🏁 Script executed:
# Search for GuiSlot definition in the entire codebase
find . -name "*.java" -type f | xargs grep -l "class GuiSlot"Repository: tr7zw/ItemSwapper
Length of output: 45
🏁 Script executed:
# Also check the import to understand where GuiSlot comes from
grep "import.*GuiSlot" src/main/java/dev/tr7zw/itemswapper/util/WidgetUtil.javaRepository: tr7zw/ItemSwapper
Length of output: 112
🏁 Script executed:
# Find GuiSlot.java
find . -name "GuiSlot.java" -type fRepository: tr7zw/ItemSwapper
Length of output: 125
🏁 Script executed:
# Also check if we can see the constructor signature
rg "public GuiSlot" -A 5Repository: tr7zw/ItemSwapper
Length of output: 1096
🏁 Script executed:
# Let's see the full GuiSlot constructor to confirm the 4th parameter is size
cat -n src/main/java/dev/tr7zw/itemswapper/overlay/logic/GuiSlot.java | head -30Repository: tr7zw/ItemSwapper
Length of output: 428
Slot size should match spacing variable for layout consistency.
Line 182 hardcodes ItemSwapperUI.tinySlotSize as the slot rendering size, but sz (determined at line 172) controls grid spacing. When texture == null (for layouts with 1–2 items), sz is set to slotSize for wider spacing, yet slots are still rendered at tinySlotSize. This creates a visual mismatch: large gaps with small slots.
Background dimensions (lines 169–170) are also hardcoded to tinySlotSize, amplifying the inconsistency. Use sz for the slot size parameter to align spacing with rendering:
Suggested fix
- slots.add(new GuiSlot(originX + xOffset + ((maxWidth - lines[y]) / 2 + x) * sz, originY + y * sz, slotId, ItemSwapperUI.tinySlotSize));
+ slots.add(new GuiSlot(originX + xOffset + ((maxWidth - lines[y]) / 2 + x) * sz, originY + y * sz, slotId, sz));📝 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.
| for (int y = 0; y < lines.length; y++) { | |
| int xOffset = (maxWidth - lines[y]) % 2 == 1 ? sz / 2 : 0; | |
| for (int x = 0; x < lines[y]; x++) { | |
| slots.add(new GuiSlot(originX + xOffset + ((maxWidth - lines[y]) / 2 + x) * sz, originY + y * sz, slotId, ItemSwapperUI.tinySlotSize)); | |
| slotId++; | |
| } | |
| } | |
| for (int y = 0; y < lines.length; y++) { | |
| int xOffset = (maxWidth - lines[y]) % 2 == 1 ? sz / 2 : 0; | |
| for (int x = 0; x < lines[y]; x++) { | |
| slots.add(new GuiSlot(originX + xOffset + ((maxWidth - lines[y]) / 2 + x) * sz, originY + y * sz, slotId, sz)); | |
| slotId++; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/dev/tr7zw/itemswapper/util/WidgetUtil.java` around lines 179 -
185, The slot rendering size is hardcoded to ItemSwapperUI.tinySlotSize while
grid spacing uses the variable sz, causing visual mismatch; update the code in
WidgetUtil (the loop creating GuiSlot instances and the background dimension
calculations just above it) to pass sz instead of ItemSwapperUI.tinySlotSize so
the GuiSlot constructors and background dimensions use the same spacing variable
(sz) for consistent layout and rendering.
| public static void setupHalfGridSlots(WidgetArea widgetArea, List<GuiSlot> slots, int width, int height, | ||
| /*? >= 1.21.11 {*/ Identifier /*?} else {*//* ResourceLocation *//*?}*/ texture, boolean flip) { | ||
| widgetArea.setBackgroundTexture(texture); | ||
| widgetArea.setBackgroundSizeX(width * ItemSwapperUI.tinySlotSize + 6); | ||
| widgetArea.setBackgroundSizeY(height * ItemSwapperUI.tinySlotSize + 6); | ||
| int sz = texture == null ? ItemSwapperUI.slotSize : ItemSwapperUI.tinySlotSize; | ||
| int lz = texture == null ? 11 : 9; | ||
| widgetArea.setMouseBoundsX(width * lz); | ||
| widgetArea.setMouseBoundsY(height * lz); | ||
| int originX = (int) (-width / 2d * sz - 2); | ||
| int originY = (int) (-height / 2d * sz - 1 - 2); | ||
| int slotId = 0; | ||
| int[] size = new int[height]; | ||
| for (int y = 0; y < height; y++) { | ||
| for (int x = 0; x < width; x++) { | ||
| boolean skip = (x == width - 1 && y == height - 1) || (x == width - 1 && y == 0); | ||
| boolean needsOffset = y == 0 || y == height - 1; | ||
| if (flip) { | ||
| skip = (x == width - 1 && y != height - 1 && y != 0); | ||
| needsOffset = !needsOffset; | ||
| } | ||
| int xOffset = needsOffset ? sz / 2 : 0; | ||
| if (!skip) { | ||
| slots.add(new GuiSlot(originX + xOffset + x * sz, originY + y * sz, slotId++, | ||
| ItemSwapperUI.tinySlotSize)); | ||
| size[y]++; | ||
| } | ||
| } | ||
| } | ||
| setupSlots(widgetArea, slots, size, texture); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all call sites of setupHalfGridSlots
rg -n 'setupHalfGridSlots' --type=java -B2 -A2Repository: tr7zw/ItemSwapper
Length of output: 4264
🏁 Script executed:
#!/bin/bash
# Check the setupHalfGridSlots method implementation and search for any use of 'flip' within it
rg -n 'flip' src/main/java/dev/tr7zw/itemswapper/util/WidgetUtil.java -B1 -A1Repository: tr7zw/ItemSwapper
Length of output: 328
🏁 Script executed:
#!/bin/bash
# Get the full context of the setupHalfGridSlots method to see if flip is used
sed -n '208,220p' src/main/java/dev/tr7zw/itemswapper/util/WidgetUtil.javaRepository: tr7zw/ItemSwapper
Length of output: 665
🏁 Script executed:
#!/bin/bash
# Check if there are any overloads of setupHalfGridSlots or if flip is used elsewhere
ast-grep --pattern 'setupHalfGridSlots($$$)'Repository: tr7zw/ItemSwapper
Length of output: 1056
flip parameter is accepted but never used — potential layout regression for length <= 11.
The flip parameter (line 209) is passed to the method but never consulted in the implementation. Only one call site (line 125) provides flip=true for the 11-item grid layout, yet the parameter has no effect on slot positioning. Meanwhile, other callers either use the 5-parameter overload (which defaults to flip=false) or ignore the flag entirely.
Re-implement the flip logic to control slot positioning, or remove the unused parameter and its call site to clarify intent.
🤖 Prompt for AI Agents
In `@src/main/java/dev/tr7zw/itemswapper/util/WidgetUtil.java` around lines 208 -
220, The flip parameter in setupHalfGridSlots is ignored; adjust the skip logic
to mirror the two corner skips when flip is true so slots are removed from the
left side instead of the right. Specifically, in setupHalfGridSlots change the
skip condition that currently uses (x == width - 1 && y == height - 1) || (x ==
width - 1 && y == 0) to branch on flip: when flip is false keep the existing
right-side corner checks, and when flip is true use left-side checks (x == 0 &&
y == height - 1) || (x == 0 && y == 0); keep calling setupSlots(widgetArea,
slots, size, texture) after computing size.
1 participant
No description provided.