Backport: hopper-xtreme — block-entity base extraction + review fixes (26.1.2) - #83
Merged
Merged
Conversation
Wave 1 of docs/backport-26.1.2. Everything Wave 2's mods compile against.
Bug fixes:
- ImplementedInventory.isMatchingPartialStack compared with ItemStack.matches,
which also compares counts, so two otherwise-identical partial stacks only
merged when their counts happened to be equal. Every tryInsert consumer
(minekea shelves/armoires/glass jars, the block painter, the hopper filter)
silently failed to merge partials. Use isSameItemSameComponents. (1.6)
- clearContent now preserves the fixed slot count of a NonNullList.withSize. (4.7)
- BlockConfig.getTexture() used Map.getOrDefault, whose default argument is
evaluated unconditionally, so a config with an explicit texture but no
ingredient threw "No default ingredient set" from a fallback it never needed.
Look the texture up first. (2.4)
New shared API (the Wave 2 dependency surface):
- screen/InventoryScreenHandler - base for fixed-grid container menus (layout,
quickMoveStack, removed()/stopOpen()). Simple/DoubleWide collapse to thin
subclasses that only pin the column count. (3.2, 2.6)
- inventories/ContainerOpenersCounters - factory replacing three hand-rolled
anonymous counters. Takes the menu class as a required parameter and confirms
ownership against the block entity, which is what makes shulker-stuff's 2.5
unrepeatable. (3.5)
- item/AbstractWrenchItem - the wrench logic duplicated byte-for-byte between
minekea and hopper-xtreme. (3.4)
- blocks/BlockUtils - moved here from sponj. (3.9)
- neoforge/loot/LootModifierHelper.createRegister(modId). (3.6)
ColorHelpers: palette arrays are private and handed out only as defensive
copies via a new getTints(String); getTint gains a lower bound. The 26.1.2 flat
Blocks/Items constants are kept - main's ColorCollection accessors
(Blocks.WOOL.red()) do not exist here. (4.7)
Test harness: common/testFixtures publishes BootstrapMinecraft plus
GameTestContainers/Entities/Menus; 9 JUnit classes (43 tests) and 5 fabric
GameTest classes (11 tests) in the never-shipped gametest source set.
Two adaptations the plan did not predict, both now recorded in it:
1. Data components ARE lazily bound on 26.1.2. The plan said the
"Components not bound yet" bake was 26.2-only and should be deleted; that was
inferred from `git grep DATA_COMPONENT_INITIALIZERS 26.1.2` returning nothing,
which only proves the repo never used it. Without the bake, 8 of 43 tests fail;
javap on the 26.1.2 jar shows BuiltInRegistries.DATA_COMPONENT_INITIALIZERS and
DataComponentInitializers.build(Provider) -> List<PendingComponents> with
apply(), identically shaped to 26.2. BootstrapMinecraft keeps main's bake.
This also means minekea must KEEP its datagen component bind - minekea.md,
README.md, docs/TESTING.md and CLAUDE.md are all corrected here.
2. new BlockEntityType<>(factory, blocks) is private on 26.1.2 (public on 26.2).
Mods that call it directly each widen it in their own access widener;
chimeric-lib has none enabled. Rather than add a shipping access widener to
serve a test fixture, the gametest builds the type with fabric-api's public
FabricBlockEntityTypeBuilder - that source set is fabric-only and never ships.
Gate: :chimeric-lib:{common,fabric,neoforge}:build green;
:chimeric-lib:fabric:test 43/43; :chimeric-lib:fabric:runGameTest 11/11.
Backport of e5233d8, e69d6e8, 60a8169, 3aaa3bb, 5315faa, 8d7fa89,
875eaa2, 5f70e69, d0ed03c, 8acc6af, e8aaffb, c392de4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eview fixes
The six *BlockEntity classes were ~80% identical copies of a vanilla-hopper
fork (~3,700 lines). They collapse into a base-class hierarchy (3.1):
AbstractXtremeHopperBlockEntity shared tick/insert/extract/
+- AbstractSingleFacingXtremeHopperBlockEntity transfer/filter/load-save,
| +- XtremeHopperBlockEntity parameterized by hooks:
| +- XtremeHupperBlockEntity storageSlotCount /
| +- GlazedHopperBlockEntity extractSide /
+- AbstractMultiXtremeHopperBlockEntity inputBlockYOffset /
+- AbstractDownMultiXtremeHopperBlockEntity levelYOffset / pushOutput
| +- XtremeMultiHopperBlockEntity
| +- GlazedMultiHopperBlockEntity
+- XtremeMultiHupperBlockEntity
Output strategy (insert into a facing container vs. drop in front) and
targeting (single FACING vs. round-robin) live in the intermediates; every leaf
is now 52-107 lines of constants and hook overrides. The hupper/multi-hupper UP
pull geometry (SUCK_AABB, the +-0.5/0.0 Y offsets) is preserved verbatim.
Review fixes, previously duplicated per copy, now centralized:
- 1.2/1.3 the filter slot is the last container slot, but the server BE hides it
from the Container API while the client builds the menu over a dummy
SimpleContainer that does not. NonFilterSlot read the filter at
getContainerSize()-1 - a real storage slot server-side, so an ordinary item
put there was treated as the filter and the GUI rejected every insert - and
quickMoveStack used getContainerSize()+1 as the hopper/player boundary, 6
server-side vs 7 client-side, desyncing shift-click routing. The index is
passed explicitly and the boundary uses the fixed hopper-slot count.
- 2.11 canExtract cast its Hopper argument straight to this mod's BE, but
extract(Level, Hopper) accepts any vanilla Hopper - a hopper minecart threw
ClassCastException. Now an instanceof pattern, with foreign hoppers falling
back to vanilla pull-from-above.
- 2.12 isFull() iterates getContainerSize() rather than the backing list, so it
no longer counts the filter slot as storage.
- 2.6 menu open/close balance via chimeric-lib's InventoryScreenHandler.
Behaviour changes that are not refactors:
- XtremeMultiHupperBlockEntity was the only variant gating canExtract on
`isFilter == (slot == 5)`, so a non-filtered multi-hupper refused to pull
anything but the filter item out of a source's slot 5. Dead code from an older
filter design; the override is removed and it inherits the standard
passesExtractFilter like the other five. (The 3.1 commit kept it verbatim
behind an overridable hook first, so the removal is a separate, visible step.)
- The multi-hopper round-robin cursor was memory-only, so an unload/reload reset
it to the vertical direction and the hopper repeated the side it had just
handed off to. It now round-trips via Direction.CODEC, falling back to
verticalDirection() for fresh placements and old saves.
4.4 - HopperItemFilterItem.use had an outer
`player.level() != null && !player.level().isClientSide()` (where the `world`
param IS player.level()) wrapping an inner isClientSide check; collapsed to one
gate. FilterSlot.mayPlace simplified alongside.
3.4 - WrenchItem drops to a thin AbstractWrenchItem subclass supplying only
ITEM_ID and its registration properties.
Block classes and screen handlers are deliberately untouched - that collapse is
step 4 in REFACTOR-3.1-PLAN.md and stays deferred.
Adaptation: ExtractGuardTest used EntityTypes.HOPPER_MINECART (26.2-only); on
26.1.2 the static instances are still on EntityType.
Gametests stay in the main source set (fabric/src/main/.../fabric/test/), which
is this mod's pre-existing exception - TEST_PLAN.md tracks migrating them.
Gate: :hopper-xtreme:{common,fabric,neoforge}:build green;
:hopper-xtreme:fabric:runGameTest 14/14 before the patch, 20/20 after.
Wave 2 of docs/backport-26.1.2; see hopper-xtreme.md.
Backport of 496e148 + 54fc6a6 + 8acc6af + 1b8d84e + a96de32 + 9d09bbf +
bc0359e + 084eb51 + 1b6eaa7 + 04bfc97 + 78a23bc + 41ab0ad + 2b53182.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
chimericdream
force-pushed
the
backport/26.1.2/chimeric-lib
branch
from
July 27, 2026 00:43
7c945fb to
c213381
Compare
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
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.
Wave 2, stacked on #82. Plan:
docs/backport-26.1.2/hopper-xtreme.md.−3,484 / +1,933 lines. The largest refactor in the backport.
The 3.1 extraction
Six
*BlockEntityclasses were ~80% identical copies of a vanilla-hopper fork (~3,700 lines):Every leaf is now 52–107 lines. The hupper/multi-hupper
UPpull geometry (SUCK_AABB, the ±0.5/0.0 Y offsets) is preserved verbatim.Review fixes, previously duplicated six times
ContainerAPI while the client builds the menu over a dummySimpleContainerthat does not.NonFilterSlotread the filter atgetContainerSize()-1— a real storage slot server-side, so an ordinary item put there was treated as the filter and the GUI rejected every insert. AndquickMoveStackusedgetContainerSize()+1as the hopper/player boundary: 6 server-side vs 7 client-side, desyncing shift-click routing.canExtractcast itsHopperargument straight to this mod's BE, butextract(Level, Hopper)accepts any vanillaHopper— a hopper minecart threwClassCastException.isFull()counted the filter slot as storage.Behaviour changes (not refactors)
XtremeMultiHupperBlockEntitywas the only variant gatingcanExtractonisFilter == (slot == 5), so a non-filtered multi-hupper refused to pull anything but the filter item out of a source's slot 5. Dead code from an older filter design — removed. (Kept verbatim behind a hook in the extraction commit first, so the removal is a separate, reviewable step.)Direction.CODEC.Adaptation
ExtractGuardTestusedEntityTypes.HOPPER_MINECART(26.2-only); on 26.1.2 the static instances are still onEntityType.Verification
:hopper-xtreme:fabric:runGameTest— 14/14 before the patch, 20/20 after. Both loaders build.Block classes and screen handlers are deliberately untouched — that's step 4 in
REFACTOR-3.1-PLAN.mdand stays deferred. Gametests stay in the main source set, this mod's pre-existing exception.🤖 Generated with Claude Code