-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/add optional second item #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughCrusher now supports an optional auxiliary output: inventory expanded to four slots, crafting validates and writes primary and optional auxiliary outputs to separate output slots, extraction and slot access updated, and the UI shows a read-only auxiliary output slot. Changes
Sequence DiagramsequenceDiagram
participant Tick as Game Tick
participant BlockEnt as CrusherBlockEntity
participant Recipe as CrusherRecipe
participant Inv as Inventory
Tick->>BlockEnt: tick() / canCraft()
BlockEnt->>Recipe: findMatching(input)
Recipe-->>BlockEnt: primary(ItemStack), Optional(auxiliary)
BlockEnt->>BlockEnt: canInsertIntoSlot(OUTPUT_SLOT)
BlockEnt->>BlockEnt: canInsertIntoSlot(AUXILIARY_OUTPUT_SLOT)
alt Both slots can accept
BlockEnt->>Inv: craftItem()
Inv->>Inv: insertOrIncrement(OUTPUT_SLOT, primary)
opt auxiliary present
Inv->>Inv: insertOrIncrement(AUXILIARY_OUTPUT_SLOT, auxiliary)
end
Inv-->>BlockEnt: outputs stored
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/anya/pizza/houseki/block/entity/custom/CrusherBlockEntity.java (1)
234-236: Auxiliary output slot not exposed to hoppers.
getAvailableSlots()only returnsOUTPUT_SLOTforDirection.DOWN, butcanExtract()allows extraction from both output slots. This means hoppers below the crusher cannot extract items from the auxiliary output slot.If this is intentional, consider adding a comment. Otherwise, update to expose both slots:
Proposed fix (if hoppers should extract from both slots)
`@Override` public int[] getAvailableSlots(Direction side) { - return side == Direction.DOWN ? new int[]{OUTPUT_SLOT} : new int[]{INPUT_SLOT, FUEL_SLOT}; + return side == Direction.DOWN ? new int[]{OUTPUT_SLOT, AUXILARY_OUTPUT_SLOT} : new int[]{INPUT_SLOT, FUEL_SLOT}; }src/main/java/anya/pizza/houseki/screen/custom/CrusherScreenHandler.java (1)
29-29: Inventory size check is outdated.The inventory now has 4 slots, but
checkSizestill validates for 3. This should be updated to match the new inventory size.Proposed fix
- checkSize((Inventory) blockEntity, 3); + checkSize((Inventory) blockEntity, 4);
🤖 Fix all issues with AI agents
In
`@src/main/java/anya/pizza/houseki/block/entity/custom/CrusherBlockEntity.java`:
- Around line 194-199: The canInsertIntoSlot method currently uses
slotStack.getMaxCount() even when slotStack is empty, which incorrectly uses the
default 64; change the logic to compute the allowed max using the incoming
stack's max when the slot is empty. Specifically, in canInsertIntoSlot(int slot,
ItemStack stack) determine int max = slotStack.isEmpty() ? stack.getMaxCount() :
slotStack.getMaxCount() and use that max in the count comparison
(slotStack.getCount() + stack.getCount() <= max), keeping the existing
equality/emptiness checks for slotStack and stack.
In `@src/main/java/anya/pizza/houseki/recipe/CrusherRecipe.java`:
- Around line 89-91: The CODEC mapping in CrusherRecipe currently wraps
ItemStack.EMPTY into Optional.of, causing auxiliaryOutput to be present for
missing fields; change the xmap to produce Optional.empty() when the read
ItemStack is ItemStack.EMPTY and to write ItemStack.EMPTY when the Optional is
empty (i.e. replace Optional::of with a function that returns item.isEmpty() ?
Optional.empty() : Optional.of(item), and keep the writer as opt ->
opt.orElse(ItemStack.EMPTY)) so auxiliaryOutput correctly reflects an absent
auxiliary_result.
🧹 Nitpick comments (2)
src/main/java/anya/pizza/houseki/recipe/CrusherRecipe.java (1)
67-70: Redundant getter for record component.Java records auto-generate an accessor method
auxiliaryOutput()for each component. This explicitgetAuxiliaryOutput()method duplicates that functionality. Consider removing it and using the canonicalauxiliaryOutput()accessor throughout the codebase for consistency.src/main/java/anya/pizza/houseki/block/entity/custom/CrusherBlockEntity.java (1)
42-42: Typo:AUXILARYshould beAUXILIARY.The constant name has a spelling error. Consider renaming for consistency.
Proposed fix
- private static final int AUXILARY_OUTPUT_SLOT = 3; + private static final int AUXILIARY_OUTPUT_SLOT = 3;
src/main/java/anya/pizza/houseki/block/entity/custom/CrusherBlockEntity.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
src/main/java/anya/pizza/houseki/block/entity/custom/CrusherBlockEntity.java (1)
234-237: Auxiliary output slot is not accessible for hopper extraction.
canExtract()correctly allows extraction from both output slots (line 249), butgetAvailableSlots()only exposesOUTPUT_SLOTfor the DOWN direction. Hoppers won't be able to pull items from the auxiliary slot because it's not listed in the available slots array.Proposed fix
`@Override` public int[] getAvailableSlots(Direction side) { - return side == Direction.DOWN ? new int[]{OUTPUT_SLOT} : new int[]{INPUT_SLOT, FUEL_SLOT}; + return side == Direction.DOWN ? new int[]{OUTPUT_SLOT, AUXILARY_OUTPUT_SLOT} : new int[]{INPUT_SLOT, FUEL_SLOT}; }
🧹 Nitpick comments (2)
src/main/java/anya/pizza/houseki/block/entity/custom/CrusherBlockEntity.java (2)
42-42: Typo in constant name:AUXILARY→AUXILIARY.The constant name is misspelled. Consider fixing for consistency and readability.
Suggested fix
- private static final int AUXILARY_OUTPUT_SLOT = 3; + private static final int AUXILIARY_OUTPUT_SLOT = 3;Note: Update all references throughout the file accordingly.
206-214: Use consistent accessor method for recipe output in bothcanCraft()andcraftItem().
canCraft()usescrusherRecipe.getResult(null)(line 188) whilecraftItem()usescrusherRecipe.output()(line 209). Both access the same ItemStack, so using the same accessor method in both places improves code clarity and maintainability.Consider standardizing on a single accessor method:
Suggested fix
private void craftItem() { Optional<RecipeEntry<CrusherRecipe>> recipe = getCurrentRecipe(); if (recipe.isEmpty()) return; CrusherRecipe crusherRecipe = recipe.get().value(); // Handle Main Output - insertOrIncrement(OUTPUT_SLOT, crusherRecipe.output().copy()); + insertOrIncrement(OUTPUT_SLOT, crusherRecipe.getResult(null).copy()); // Handle Auxiliary Output crusherRecipe.getAuxiliaryOutput().ifPresent(stack -> { insertOrIncrement(AUXILARY_OUTPUT_SLOT, stack.copy()); });
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
|
@coderabbitai resolve |
Docstrings generation was requested by @The-Code-Monkey. * #1 (comment) The following files were modified: * `src/main/java/anya/pizza/houseki/block/entity/custom/CrusherBlockEntity.java` * `src/main/java/anya/pizza/houseki/recipe/CrusherRecipe.java` * `src/main/java/anya/pizza/houseki/screen/custom/CrusherScreenHandler.java`
…1e6c9 📝 Add docstrings to `feat/add-optional-second-item`
✅ Actions performedComments resolved. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.