Skip to content

Fix Item Scroller trade list compatibility - #86

Open
SpaceSyt wants to merge 1 commit into
Ramixin:fabricfrom
SpaceSyt:agent/item-scroller-visible-trades-compat
Open

SpaceSyt wants to merge 1 commit into
Ramixin:fabricfrom
SpaceSyt:agent/item-scroller-visible-trades-compat

Conversation

@SpaceSyt

Copy link
Copy Markdown

Fixes #83.

Problem

Item Scroller replaces MerchantMenu#getOffers() with a custom trade list using a cancellable HEAD injection. Visible Traders previously used @ModifyReturnValue at RETURN, so its combined unlocked + locked offer list was bypassed whenever Item Scroller returned early.

This also interacts with TweakerMore's villagerOfferUsesDisplay feature. TweakerMore reads the offer list from an injected tooltip path. Once Visible Traders rendered a button for a locked offer, TweakerMore could use that button index against Item Scroller's shorter list and crash with an IndexOutOfBoundsException.

Fix

  • Wrap the entire MerchantMenu#getOffers() method with MixinExtras @WrapMethod, so the combined list can be returned even when another injection cancels the original method.
  • Add a nestable combined-offers scope to the client merchant menu.
  • Wrap the entire TradeOfferButton#extractToolTip call in that scope and release it in finally. This ensures vanilla and injected tooltip code, including TweakerMore, see the same list for the full tooltip call.
  • Keep ordinary calls outside the scope delegated to the original chain, preserving Item Scroller's normal favorites/reordering behavior and avoiding exposing locked indexes to actual trade submission.

Validation

  • In-game tested successfully on Minecraft 26.1.2 Fabric with:
    • Item Scroller 0.31.5
    • TweakerMore 3.31.0 with villagerOfferUsesDisplay enabled
    • Trade Cycling 1.0.21
  • Locked/future trades render and can be hovered without crashing.
  • Normal unlocked trades remain usable; locked trades remain inactive.
  • Current fabric branch checks passed locally:
    • compileJava
    • compileFabricJava
    • fabricJar

The change has not been game-tested on Minecraft 26.2 or NeoForge.

AI disclosure

The investigation and implementation were written with OpenAI Codex using GPT-5.6 Sol with high reasoning effort, followed by in-game validation on Minecraft 26.1.2 Fabric.

@Ramixin

Ramixin commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Hello @SpaceSyt ,

Thank you for taking the time to put this together. Since it was made using AI, I may take some extra time to verify the changes, but at a quick glance it looks good to go.

@SpaceSyt

Copy link
Copy Markdown
Author

Just re-checked all four open issues (#70, #81, #82, #83) . This PR actually fixes them all together. All stem from the same Item Scroller getOffers() injection breaking trade preview.

@SpaceSyt
SpaceSyt marked this pull request as ready for review July 24, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The VisibleTraders-2.3.0 mod cannot achieve trade preview in version fabirc 26.1.2

2 participants