PssbleTrngle / DataTrades

Allows easy customization of villager trades using datapacks
Creative Commons Zero v1.0 Universal
1 stars 0 forks source link

Trades with failed conditions remain selected within the table #12

Open CraftyG opened 11 months ago

CraftyG commented 11 months ago

What happened?

The Issue

Trades that have failed to meet conditions remain selected within the table, but they don't appear when interacting with an affected villager/trader

How to Reproduce

  1. I've created a data pack that helps demonstrate the issue. It overwrites the wandering trader's trades accordingly:
    • selling a redstone block if it's on red wool
    • selling a copper block if it's on orange wool
    • selling a gold block if it's on yellow wool
    • selling a diamond block if it's on light blue wool
    • only one of the above trades will be selected
    • emerald trade is only there for the sake of having a rare trade report_pack.zip
  2. Summon four wandering traders, each standing on one of the wool blocks mentioned above.
  3. Install the data pack, then reload the world by exiting and re-entering it.
  4. Use /villagers refresh to reload the traders' trades
    • Expectation: "Failed" trades would be re-rolled until there's a "valid" trade; All wandering traders having the one trade corresponding to the conditions met.
    • Observation: Some of the traders only have the one rare trade displayed. Rarely does a valid trade appear.
  5. Repeat step 3 and continue observing the trades displayed from the traders.

    The Proposal

    Despite it being a bug report, this behavior could be proven useful in some cases and thus can be maintined. Having an optional field for a trade to re-roll if the conditions don't meet fixes the issue while not completely removing this behavior.

Loader

forge

Mod Version

1.1.1

Minecraft Version

1.19

Relevant log output

No response

PssbleTrngle commented 11 months ago

Hi, you're totally right, I would also like it more for "invalid" trades to be re-rolled until a valid trade is found, but the way the villager logic itself and where I intercept it does not really make that possible right now, which is why there is this half-baked solution right now. I am personally also not content with it, which is why I have also have not documented the condition part of the json model yet.